RE: [PATCH v10 00/11] Add USB OTG HNP and SRP support on Chipidea usb driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 
> On Tue, Apr 15, 2014 at 02:48:44PM +0800, Peter Chen wrote:
> > On Tue, Apr 15, 2014 at 09:43:04AM +0800, Li Jun wrote:
> > > On Mon, Apr 14, 2014 at 08:45:13PM +0800, Peter Chen wrote:
> > > > On Mon, Apr 14, 2014 at 10:22:32AM +0800, Li Jun wrote:
> > > > > On Mon, Apr 14, 2014 at 09:37:50AM +0800, Li Jun wrote:
> > > > > > From: Li Jun <b47624@xxxxxxxxxxxxx>
> > > > > >
> > > > > > This patchset adds USB OTG HNP and SRP support on chipidea usb
> > > > > > driver, existing OTG port role swtich function by ID pin
> > > > > > status kept unchanged, based on that, if select
> > > > > > CONFIG_USB_OTG_FSM, OTG HNP and SRP will be supported.
> > > > > >
> > > > > > Reference to:
> > > > > > "On-The-Go and Embedded Host Supplement to the USB Revision
> > > > > > 2.0 Specification July 27, 2012 Revision 2.0 version 1.1a"
> > > > > >
> > > > > > Changes since v9:
> > > > > > - Move xxx_fsm_start to be after request irq since xxx_fsm_work
> need call to
> > > > > >   disable_irq_nosync(ci->irq).
> > > >
> > > > Then, the interrupt will be on during the host initialization, it
> > > > may have unexpected interrupt.
> > > >
> > >
> > > Maybe for some devices which need not vbus from host, the enum will
> > > start when host init. one possible solution is split the current
> > > fsm_start into 2 parts, the first part is put into fsm_init(thus
> > > before request_irq), the 2nd part is put into fsm_start, any idea? if
> no objection, I can go in this way.
> > >
> >
> > Try to defer ci_otg_fsm_work using queue_work, and disable irq before
> > it.
> >
> 
> Can work in this way, I will change.
> 
> > >
> > > > > > - Add fsm handling in xxx_fsm_work for regsiter gadget driver
> with vbus on, instead
> > > > > >   of rely on B_SE0_SRP timer(which is not directly linked to
> this case and need wait
> > > > > >   1s).
> > > >
> > > > The B_SE0_SRP timer still will queue otg fsm work or not? If it
> > > > is, any side effects?
> > > >
> > >
> > > yes, still queue otg fsm work, but no side effects, as B-device is
> > > already queue one fsm work before B_SE0_SRP timer time out, then the
> > > later one will just try to change state but do nothing.
> >
> > Get it.
> >
> > >
> > > > > > - Add comments on a_idle to a_wait_vrise due to ID change,
> which is out of OTG spec
> > > > > >   but make sense for user experience.
> > > > > > - Remove blank line introduced in v9.
> > > > > >
> > > > > Sorry, missed one update in v10 changes summary:
> > > > > - Clear ID interrupt status before enable ID irq in
> ci_hdrc_probe().
> > > >
> > > > At ci_get_otg_capable, it will clear all otg interrupt status.
> > > >
> > >
> > > I do still find there is one ID change irq pending there when power
> up with ID is low.
> > > if I need rework the fsm start and put host init before request irq,
> > > this problem will not exist.
> >
> > Try to find the root cause for it please, we can't accept the code
> > which still not find the root cause.
> >
> 
> After more check on this,
> - The default state of ID is "1"
> - It's too early to clear ID change irq stats in current sequence, my
> simple log
>   information shows the ID is still in default state("1") when clear and
> disable
>   OTG irq in ci_get_otg_capable().
> - There is already safe check of ID status by 2ms delay in current code:
> 		...
> 	if (ci->is_otg) {
> 		/*
> 		 * ID pin needs 1ms debouce time,
> 		 * we delay 2ms for safe.
> 		 */
> 		mdelay(2);
> 		ci->role = ci_otg_role(ci);
> 		...
> 
 
I think I know the reason why you see this problem, your PHY is in low power mode
during the initialization, so after PHY's initialization has finished, we need to
delay 2ms for controller to reflect phy's status, and delay at ci_hdrc_enter_lpm are too much,
Please omit this ID status clear change in your patch, I will submit a patch to fix it soon.

Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux