Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver
On 21 June 2013 22:03, Alan Stern wrote:
> On Fri, 21 Jun 2013, Manjunath Goudar wrote:
>
> > On 20 June 2013 22:23, Alan Stern wrote:
> >
> > > On Thu, 20 Jun 2013, Manjunath Goudar wrote:
> > >
> > > > > > @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct
> platform_device
> > > > > *pdev, pm_message_t mesg)
> > > > > >* REVISIT: some boards will be able to turn VBUS off...
> > > > > >*/
> > > > > > if (at91_suspend_entering_slow_clock()) {
> > > > > > - ohci_usb_reset (ohci);
> > > > > > + ohci_restart(ohci);
> > > > >
> > > > > Why did you change this? Did we discuss it earlier?
> > > > >
> > > >
> > > > We are not discussed regarding this,I think we need to call
> > > > use ohci_resume() instead of ohci_restart().
> > >
> > > Why? Don't you think the current code has a good reason for calling
> > > ohci_usb_reset()?
> > >
> > >
> > Here ohci_usb_reset() is static function,that is what I am planing to
> call
> > ohci_setup() or ohci_restart() because it is calling ohci_usb_reset(),
> > If not calling, we can make ohci_usb_reset() function as non-static
> > function
> > or use directly ohci_usb_reset() function code here.
> >
> > Let me know which one is good approach.
>
> As a general rule, you should never change code that you don't
> understand. Do you _know_ that it will be safe to call ohci_setup() or
> ohci_restart() at this point?
>
>
>From David Brownell comment I am understanding,instead of calling
ohci_setup()
or ohci_restart(),we can use directly below code.
ohci->hc_control &= OHCI_CTRL_RWC;
ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
ohci->rh_state = OHCI_RH_HALTED;
the 3rd line code is written by you,I want to know what exactly it is doing.
Is it required here?
> It might be a good idea to get in touch with the person who wrote that
> routine originally and ask why they used ohci_usb_reset().
>
> yes I will.
Hi David,
As I understood ohci_usb_reset() is calling for to notice
disconnect,reconnect,
or wakeup without the 48 MHz clock active.
After fix power management hanging(869aa98c) issue by Patrice Vilchez,I
think
ohci_usb_reset() is not required to call. what is your opinion about this.
Manjunath Goudar
> Alan Stern
>
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver
On Sun, 23 Jun 2013, Manjunath Goudar wrote: > > As a general rule, you should never change code that you don't > > understand. Do you _know_ that it will be safe to call ohci_setup() or > > ohci_restart() at this point? > > > > > From David Brownell comment I am understanding,instead of calling > ohci_setup() > or ohci_restart(),we can use directly below code. > > ohci->hc_control &= OHCI_CTRL_RWC; > ohci_writel (ohci, ohci->hc_control, &ohci->regs->control); > ohci->rh_state = OHCI_RH_HALTED; Of course you can use that code; that's exactly what ohci_usb_reset() does now. > the 3rd line code is written by you,I want to know what exactly it is doing. > Is it required here? Yes, it is required. ohci->rh_state stores the current state of the root hub. When the controller is reset, the root hub goes into the HALTED state; this line records that fact. > > It might be a good idea to get in touch with the person who wrote that > > routine originally and ask why they used ohci_usb_reset(). > > > > yes I will. > > Hi David, > > As I understood ohci_usb_reset() is calling for to notice > disconnect,reconnect, > or wakeup without the 48 MHz clock active. > After fix power management hanging(869aa98c) issue by Patrice Vilchez,I > think > ohci_usb_reset() is not required to call. what is your opinion about this. Sadly, David Brownell died in 2011. I can tell you, though, that commit 869aa98c does not eliminate the need to call ohci_usb_reset(). Alan Stern ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver
On 23 June 2013 00:38, Alan Stern wrote: > On Sun, 23 Jun 2013, Manjunath Goudar wrote: > > > > As a general rule, you should never change code that you don't > > > understand. Do you _know_ that it will be safe to call ohci_setup() or > > > ohci_restart() at this point? > > > > > > > > From David Brownell comment I am understanding,instead of calling > > ohci_setup() > > or ohci_restart(),we can use directly below code. > > > > ohci->hc_control &= OHCI_CTRL_RWC; > > ohci_writel (ohci, ohci->hc_control, &ohci->regs->control); > > ohci->rh_state = OHCI_RH_HALTED; > > Of course you can use that code; that's exactly what ohci_usb_reset() > does now. > > OK then I will use above three line code for ohci_usb_reset() alternative. > the 3rd line code is written by you,I want to know what exactly it is > doing. > > Is it required here? > > Yes, it is required. ohci->rh_state stores the current state of the > root hub. When the controller is reset, the root hub goes into the > HALTED state; this line records that fact. > > Thank you for spending your valuable time, answering my doubt. > > > It might be a good idea to get in touch with the person who wrote that > > > routine originally and ask why they used ohci_usb_reset(). > > > > > > yes I will. > > > > Hi David, > > > > As I understood ohci_usb_reset() is calling for to notice > > disconnect,reconnect, > > or wakeup without the 48 MHz clock active. > > After fix power management hanging(869aa98c) issue by Patrice Vilchez,I > > think > > ohci_usb_reset() is not required to call. what is your opinion about > this. > > Sadly, David Brownell died in 2011. I can tell you, though, that > commit 869aa98c does not eliminate the need to call ohci_usb_reset(). > > So sad :( :( > Alan Stern > > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
