Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0

From	Roger Quadros <rogerq@ti.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0
Date	2016-04-08 09:20 +0200
Message-ID	<rlyRY-eV-7@gated-at.bofh.it> (permalink)
References	(2 earlier) <rkOYO-880-9@gated-at.bofh.it> <rkPia-8gZ-9@gated-at.bofh.it> <rleJA-1Ne-7@gated-at.bofh.it> <rlfFE-2v3-7@gated-at.bofh.it> <rlt5T-4eY-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On 08/04/16 04:01, Peter Chen wrote:
> On Thu, Apr 07, 2016 at 01:40:21PM +0300, Roger Quadros wrote:
>> On 07/04/16 12:42, Peter Chen wrote:
>>> On Wed, Apr 06, 2016 at 09:32:22AM +0300, Roger Quadros wrote:
>>>> On 06/04/16 09:09, Felipe Balbi wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>>>>>> index 2ca2cef..6b1930d 100644
>>>>>> --- a/drivers/usb/core/hcd.c
>>>>>> +++ b/drivers/usb/core/hcd.c
>>>>>> @@ -2706,6 +2706,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>>>>>  	int retval;
>>>>>>  	struct usb_device *rhdev;
>>>>>>  
>>>>>> +	hcd->flags = 0;
>>>>>
>>>
>>> I am not sure if this usb_add(remove)_hcd pair is safe and clean enough
>>> for start/stop host role. From my point, we may need to do like
>>> .probe/.remove host platform driver interface. In that case, we can make
>>
>> probe and remove are meant to be called from bus layer.
>> I do not see a way how OTG framework can call probe/remove of HCD driver.
>> Some HCDs may be platform devices, some PCI, so different entities are calling
>> the HCD .probe hook.
>>
>>> sure the clocks and regulators are off, and hcd will be zero-initialized
>>
>> why can't we make that sure that is taken care of within the hcd_ops?
>> Why should some driver keep its regulators and clocks enabled when hcd is stopped?
>> It doesn't need to. If it is doing so now, it needs to be fixed.
>>
> 
> Well, you may misunderstand me. I mean your hcd_ops->start or ->stop
> is hard to be a general one which only calls usb_hcd_add or
> usb_hcd_remove. It needs to implement like .probe or .remove at platform
> driver, some example code like host_start and host_stop at
> drivers/usb/chipidea/host.c.
> 
The only extra thing the host_start/stop() of that driver is doing is
enabling/disabling the VBUS regulator.
In the OTG/dual role scope VBUS regulator handling has to be done via the
OTG driver using the otg ops otg_drv_vbus() and not at HCD level. Do you agree?

I don't want to complicate the OTG to HCD interface by adding new hooks there.
If HCD driver wants to do something special for OTG case it can always do that
within the struct hc_driver interface. But ideally OTG specific handling must
be done in the OTG driver. All that the HCD driver should care about is making sure
all used resources are disabled once usb_remove_hcd() is called.

cheers,
-roger

Thread

[PATCH v6 00/12] USB OTG/dual-role framework Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 10/12] usb: doc: dt-binding: Add otg-controller property Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 12/12] usb: host: xhci-plat: Add otg device to platform data Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
    RE: [PATCH v6 12/12] usb: host: xhci-plat: Add otg device to platform  data Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-06 05:30 +0200
      Re: [PATCH v6 12/12] usb: host: xhci-plat: Add otg device to platform  data Roger Quadros <rogerq@ti.com> - 2016-04-06 08:40 +0200
  [PATCH v6 06/12] usb: otg: get rid of CONFIG_USB_OTG_FSM in favour of CONFIG_USB_OTG Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
    RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-07 11:00 +0200
      Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-07 13:50 +0200
        RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-08 13:30 +0200
          Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-11 13:00 +0200
            RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-14 10:40 +0200
              Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-14 13:10 +0200
                RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-14 13:20 +0200
                Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-14 13:40 +0200
                RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-15 12:00 +0200
                Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-15 13:00 +0200
                RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> - 2016-04-15 12:10 +0200
    Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Peter Chen <hzpeterchen@gmail.com> - 2016-04-15 11:40 +0200
      Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core Roger Quadros <rogerq@ti.com> - 2016-04-15 13:10 +0200
  [PATCH v6 05/12] usb: gadget.h: Add OTG to gadget interface Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 02/12] usb: hcd.h: Add OTG to HCD interface Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 04/12] usb: otg-fsm: move host controller operations into usb_otg->hcd_ops Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 11/12] usb: core: hub: Notify OTG fsm when A device sets b_hnp_enable Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
  [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-04-05 16:10 +0200
    Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Felipe Balbi <balbi@kernel.org> - 2016-04-06 08:20 +0200
      Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-04-06 08:40 +0200
        Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Peter Chen <hzpeterchen@gmail.com> - 2016-04-07 11:50 +0200
          Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-04-07 12:50 +0200
            Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Peter Chen <hzpeterchen@gmail.com> - 2016-04-08 03:10 +0200
              Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Roger Quadros <rogerq@ti.com> - 2016-04-08 09:20 +0200
                Re: [PATCH v6 01/12] usb: hcd: Initialize hcd->flags to 0 Peter Chen <hzpeterchen@gmail.com> - 2016-04-08 10:00 +0200
  [PATCH v6 08/12] usb: hcd: Adapt to OTG core Roger Quadros <rogerq@ti.com> - 2016-04-05 16:20 +0200

csiph-web