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);
...
After this delay, ID state becomes correct("0") and real ID "change" should
already happened, so I think this is the right place to clear this unexpected
ID interrupt.
Li Jun
> --
>
> Best Regards,
> Peter Chen
>
--
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
