[PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine Barshak]

USB phy controls USB channels 0 and 2 which are shared between
PCI USB host controllers and USBHS/USBSS respectively.

This Initializes USB phy driver earlier because we need it
before PCI USB host controllers are initialized.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/usb/phy/phy-rcar-gen2-usb.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c
index db3ab34..32c8847 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -241,7 +241,17 @@ static struct platform_driver rcar_gen2_usb_phy_driver = {
 	.remove = rcar_gen2_usb_phy_remove,
 };
 
-module_platform_driver(rcar_gen2_usb_phy_driver);
+static int __init rcar_gen2_usb_phy_mod_init(void)
+{
+	return platform_driver_register(&rcar_gen2_usb_phy_driver);
+}
+postcore_initcall(rcar_gen2_usb_phy_mod_init);
+
+static void __exit rcar_gen2_usb_phy_mod_exit(void)
+{
+	platform_driver_unregister(&rcar_gen2_usb_phy_driver);
+}
+module_exit(rcar_gen2_usb_phy_mod_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
-- 
1.8.3.1

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Laurent Pinchart]

Hi Valentine,

Thank you for the patch.

On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
> USB phy controls USB channels 0 and 2 which are shared between
> PCI USB host controllers and USBHS/USBSS respectively.
> 
> This Initializes USB phy driver earlier because we need it
> before PCI USB host controllers are initialized.

Can't you use deferred probing ?

> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  drivers/usb/phy/phy-rcar-gen2-usb.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
> b/drivers/usb/phy/phy-rcar-gen2-usb.c index db3ab34..32c8847 100644
> --- a/drivers/usb/phy/phy-rcar-gen2-usb.c
> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
> @@ -241,7 +241,17 @@ static struct platform_driver rcar_gen2_usb_phy_driver
> = { .remove = rcar_gen2_usb_phy_remove,
>  };
> 
> -module_platform_driver(rcar_gen2_usb_phy_driver);
> +static int __init rcar_gen2_usb_phy_mod_init(void)
> +{
> +	return platform_driver_register(&rcar_gen2_usb_phy_driver);
> +}
> +postcore_initcall(rcar_gen2_usb_phy_mod_init);
> +
> +static void __exit rcar_gen2_usb_phy_mod_exit(void)
> +{
> +	platform_driver_unregister(&rcar_gen2_usb_phy_driver);
> +}
> +module_exit(rcar_gen2_usb_phy_mod_exit);
> 
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");
-- 
Regards,

Laurent Pinchart

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
> Hi Valentine,
>
> Thank you for the patch.
>
> On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
>> USB phy controls USB channels 0 and 2 which are shared between
>> PCI USB host controllers and USBHS/USBSS respectively.
>>
>> This Initializes USB phy driver earlier because we need it
>> before PCI USB host controllers are initialized.
>
> Can't you use deferred probing ?

No, unfortunately this doesn't work with PCI.
We need the USB PHY set up before the PCI driver starts.
PCI controllers should be initialized via subsys_initcall and can't be built as a module.
Deferred probing is done at late_initcall and that's far too late in this case.

The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.
I haven't observed any issues with it on R-Car Gen2 phy as well.

Thanks,
Val.

>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   drivers/usb/phy/phy-rcar-gen2-usb.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
>> b/drivers/usb/phy/phy-rcar-gen2-usb.c index db3ab34..32c8847 100644
>> --- a/drivers/usb/phy/phy-rcar-gen2-usb.c
>> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
>> @@ -241,7 +241,17 @@ static struct platform_driver rcar_gen2_usb_phy_driver
>> = { .remove = rcar_gen2_usb_phy_remove,
>>   };
>>
>> -module_platform_driver(rcar_gen2_usb_phy_driver);
>> +static int __init rcar_gen2_usb_phy_mod_init(void)
>> +{
>> +	return platform_driver_register(&rcar_gen2_usb_phy_driver);
>> +}
>> +postcore_initcall(rcar_gen2_usb_phy_mod_init);
>> +
>> +static void __exit rcar_gen2_usb_phy_mod_exit(void)
>> +{
>> +	platform_driver_unregister(&rcar_gen2_usb_phy_driver);
>> +}
>> +module_exit(rcar_gen2_usb_phy_mod_exit);
>>
>>   MODULE_LICENSE("GPL v2");
>>   MODULE_DESCRIPTION("Renesas R-Car Gen2 USB phy");

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Greg KH]

On Tue, Oct 29, 2013 at 09:19:09PM +0400, Valentine wrote:
> On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
> >Hi Valentine,
> >
> >Thank you for the patch.
> >
> >On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
> >>USB phy controls USB channels 0 and 2 which are shared between
> >>PCI USB host controllers and USBHS/USBSS respectively.
> >>
> >>This Initializes USB phy driver earlier because we need it
> >>before PCI USB host controllers are initialized.
> >
> >Can't you use deferred probing ?
> 
> No, unfortunately this doesn't work with PCI.
> We need the USB PHY set up before the PCI driver starts.
> PCI controllers should be initialized via subsys_initcall and can't be built as a module.
> Deferred probing is done at late_initcall and that's far too late in this case.
> 
> The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.

I _HATE_ this "make this driver later than the others" mess.  I'll not
take patches that move drivers to different initcalls, sorry.  Please
fix up the link order, or use deferred probing, as that is why it was
created.

sorry,

greg k-h

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/30/2013 03:57 AM, Greg KH wrote:
> On Tue, Oct 29, 2013 at 09:19:09PM +0400, Valentine wrote:
>> On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
>>> Hi Valentine,
>>>
>>> Thank you for the patch.
>>>
>>> On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
>>>> USB phy controls USB channels 0 and 2 which are shared between
>>>> PCI USB host controllers and USBHS/USBSS respectively.
>>>>
>>>> This Initializes USB phy driver earlier because we need it
>>>> before PCI USB host controllers are initialized.
>>>
>>> Can't you use deferred probing ?
>>
>> No, unfortunately this doesn't work with PCI.
>> We need the USB PHY set up before the PCI driver starts.
>> PCI controllers should be initialized via subsys_initcall and can't be built as a module.
>> Deferred probing is done at late_initcall and that's far too late in this case.
>>
>> The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.
>
> I _HATE_ this "make this driver later than the others" mess.  I'll not

This is actually "make this driver before the others" mess.

> take patches that move drivers to different initcalls, sorry.  Please
> fix up the link order, or use deferred probing, as that is why it was
> created.
>

This is needed for the PCI EHCI/OHCI drivers to operate correctly on R-Car Gen platform
since it has built-in PCI host controllers that share USB I/O with USBHS/USBSS devices.
The phy has to be configured before the PCI EHCI/OHCI devices are probed.

I'd really appreciate if you could give me a hint on how to fix the PCI EHCI/OHCI driver
to use deferred probing, or fix the link order without creating a bigger mess.

> sorry,
>
> greg k-h
>

Thanks,
Val.

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Greg KH]

On Wed, Oct 30, 2013 at 01:56:25PM +0400, Valentine wrote:
> On 10/30/2013 03:57 AM, Greg KH wrote:
> > On Tue, Oct 29, 2013 at 09:19:09PM +0400, Valentine wrote:
> >> On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
> >>> Hi Valentine,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
> >>>> USB phy controls USB channels 0 and 2 which are shared between
> >>>> PCI USB host controllers and USBHS/USBSS respectively.
> >>>>
> >>>> This Initializes USB phy driver earlier because we need it
> >>>> before PCI USB host controllers are initialized.
> >>>
> >>> Can't you use deferred probing ?
> >>
> >> No, unfortunately this doesn't work with PCI.
> >> We need the USB PHY set up before the PCI driver starts.
> >> PCI controllers should be initialized via subsys_initcall and can't be built as a module.
> >> Deferred probing is done at late_initcall and that's far too late in this case.
> >>
> >> The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.
> >
> > I _HATE_ this "make this driver later than the others" mess.  I'll not
> 
> This is actually "make this driver before the others" mess.
> 
> > take patches that move drivers to different initcalls, sorry.  Please
> > fix up the link order, or use deferred probing, as that is why it was
> > created.
> >
> 
> This is needed for the PCI EHCI/OHCI drivers to operate correctly on R-Car Gen platform
> since it has built-in PCI host controllers that share USB I/O with USBHS/USBSS devices.
> The phy has to be configured before the PCI EHCI/OHCI devices are probed.
> 
> I'd really appreciate if you could give me a hint on how to fix the PCI EHCI/OHCI driver
> to use deferred probing, or fix the link order without creating a bigger mess.

Just change the PCI driver to use deferred probing, it isn't that hard
to do so.

good luck,

greg k-h

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/30/2013 06:12 PM, Greg KH wrote:
> On Wed, Oct 30, 2013 at 01:56:25PM +0400, Valentine wrote:
>> On 10/30/2013 03:57 AM, Greg KH wrote:
>>> On Tue, Oct 29, 2013 at 09:19:09PM +0400, Valentine wrote:
>>>> On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
>>>>> Hi Valentine,
>>>>>
>>>>> Thank you for the patch.
>>>>>
>>>>> On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
>>>>>> USB phy controls USB channels 0 and 2 which are shared between
>>>>>> PCI USB host controllers and USBHS/USBSS respectively.
>>>>>>
>>>>>> This Initializes USB phy driver earlier because we need it
>>>>>> before PCI USB host controllers are initialized.
>>>>>
>>>>> Can't you use deferred probing ?
>>>>
>>>> No, unfortunately this doesn't work with PCI.
>>>> We need the USB PHY set up before the PCI driver starts.
>>>> PCI controllers should be initialized via subsys_initcall and can't be built as a module.
>>>> Deferred probing is done at late_initcall and that's far too late in this case.
>>>>
>>>> The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.
>>>
>>> I _HATE_ this "make this driver later than the others" mess.  I'll not
>>
>> This is actually "make this driver before the others" mess.
>>
>>> take patches that move drivers to different initcalls, sorry.  Please
>>> fix up the link order, or use deferred probing, as that is why it was
>>> created.
>>>
>>
>> This is needed for the PCI EHCI/OHCI drivers to operate correctly on R-Car Gen platform
>> since it has built-in PCI host controllers that share USB I/O with USBHS/USBSS devices.
>> The phy has to be configured before the PCI EHCI/OHCI devices are probed.
>>
>> I'd really appreciate if you could give me a hint on how to fix the PCI EHCI/OHCI driver
>> to use deferred probing, or fix the link order without creating a bigger mess.
>
> Just change the PCI driver to use deferred probing, it isn't that hard
> to do so.
>

Do you mean to change usb_hcd_pci_probe() to return -EPROBE_DEFER if the phy is not ready?
Or should I defer the whole PCI subsystem initialization (pci_common_int)?

> good luck,

Thanks,
Val.

>
> greg k-h
>

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/31/2013 03:36 AM, Valentine wrote:
> On 10/30/2013 06:12 PM, Greg KH wrote:
>> On Wed, Oct 30, 2013 at 01:56:25PM +0400, Valentine wrote:
>>> On 10/30/2013 03:57 AM, Greg KH wrote:
>>>> On Tue, Oct 29, 2013 at 09:19:09PM +0400, Valentine wrote:
>>>>> On 10/29/2013 09:00 PM, Laurent Pinchart wrote:
>>>>>> Hi Valentine,
>>>>>>
>>>>>> Thank you for the patch.
>>>>>>
>>>>>> On Tuesday 29 October 2013 20:21:06 Valentine Barshak wrote:
>>>>>>> USB phy controls USB channels 0 and 2 which are shared between
>>>>>>> PCI USB host controllers and USBHS/USBSS respectively.
>>>>>>>
>>>>>>> This Initializes USB phy driver earlier because we need it
>>>>>>> before PCI USB host controllers are initialized.
>>>>>>
>>>>>> Can't you use deferred probing ?
>>>>>
>>>>> No, unfortunately this doesn't work with PCI.
>>>>> We need the USB PHY set up before the PCI driver starts.
>>>>> PCI controllers should be initialized via subsys_initcall and can't be built as a module.
>>>>> Deferred probing is done at late_initcall and that's far too late in this case.
>>>>>
>>>>> The MXS USB phy uses the similar approach, initializing the driver via postcore_initcall.
>>>>
>>>> I _HATE_ this "make this driver later than the others" mess.  I'll not
>>>
>>> This is actually "make this driver before the others" mess.
>>>
>>>> take patches that move drivers to different initcalls, sorry.  Please
>>>> fix up the link order, or use deferred probing, as that is why it was
>>>> created.
>>>>
>>>
>>> This is needed for the PCI EHCI/OHCI drivers to operate correctly on R-Car Gen platform
>>> since it has built-in PCI host controllers that share USB I/O with USBHS/USBSS devices.
>>> The phy has to be configured before the PCI EHCI/OHCI devices are probed.
>>>
>>> I'd really appreciate if you could give me a hint on how to fix the PCI EHCI/OHCI driver
>>> to use deferred probing, or fix the link order without creating a bigger mess.
>>
>> Just change the PCI driver to use deferred probing, it isn't that hard
>> to do so.
>>
>
> Do you mean to change usb_hcd_pci_probe() to return -EPROBE_DEFER if the phy is not ready?
> Or should I defer the whole PCI subsystem initialization (pci_common_int)?

Greg,
the reason I ask is that it doesn't seem that simple to me.

Here's some details:
The h/w is an ARM SoC that has 3 internal PCI controllers with a single EHCI/OHCI on each one.
This gives us 3 USB channels as this is called in the h/w manual.
Channel 0 is shared with USBHS (USB function) device.
Channel 2 is shared with USBSS (USB3.0 host).
Both channels are configured by a single USB phy.
USB PHY is a platform device, while EHCI/OCHI are located on the PCI busses.

If PCI USB host is probed before USB phy, the EHCI/OHCI device is detected, but nothing works.

We could change the USB HDC PCI driver and make usb_hcd_pci_probe() return -EPROBE_DEFER,
but I'm not sure how the condition for that should be phrased.
We could check the PCI vendor/device id, but there's no guarantee the same PCI device
is not used on other Renesas h/w which do not require any PHY setup. More over we only need
to setup the phy for the USB hosts connected to PCI controller 0/2.

We could try to defer the PCI driver and call pci_common_init() after the USB phy is initialized.
However, it seems that it's gonna be a bigger mess.

1. We need to add custom PCI bus scan callback to the hw_pci structure.
We can't use default (pci_scan_root_bus) because it will try to probe/enable pci devices before
their resources are fixed up when called late (after the PCI USB host drivers are registered).

2. All of the internal PCI controllers are initialized using a single call to the pci_common_init_dev().
So we can't easily defer just PCI controller 0/2. Even if we try to defer the whole PCI subsystem
initialization, we probably need some sort of a dummy device that will defer pci_common_init_dev call
untill USB phy is initialized.

3. The data structures and functions that are used to setup PCI are called at subsys_init() and
marked __initdata/__init, so that we can easily get rid of them once the PCI is registered.
Using deferred probe we'd need to keep them forever.

Using late_initcall() instead of subsys_initcall() for the PCI registration might work
if we implement custom PCI bus scan callback,
But that's just doing the same thing that I've tried to do with USB phy here
with additional PCI bus scan quirk.

On the other hand, postcore_initcall is used in drivers/usb/phy/phy-mxs-usb.c which is in your tree.
I agree that it's not that pretty, but it doesn't seem that bad considering other options we have.

>
>> good luck,
>
> Thanks,
> Val.
>

Please, let me know if you see better options.
Thanks,
Val.

>>
>> greg k-h
>>
>

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Ulrich Hecht]

On Thu, Oct 31, 2013 at 12:43 PM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> Please, let me know if you see better options.

How about disregarding the whole PCI aspect? I mean, yes, those are
PCI busses, but they are internal, with a fixed set of devices
attached, and as far as I can tell there are no other PCI busses in
the system. So how about treating the USB host controllers as platform
devices the way it's done in the vendor kernel, saving us the trouble
of pulling in the entire PCI subsystem without any tangible benefit?

CU
Uli

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/31/2013 08:12 PM, Ulrich Hecht wrote:
> On Thu, Oct 31, 2013 at 12:43 PM, Valentine
> <valentine.barshak@cogentembedded.com> wrote:
>> Please, let me know if you see better options.
>
> How about disregarding the whole PCI aspect? I mean, yes, those are
> PCI busses, but they are internal, with a fixed set of devices
> attached, and as far as I can tell there are no other PCI busses in
> the system. So how about treating the USB host controllers as platform
> devices the way it's done in the vendor kernel, saving us the trouble
> of pulling in the entire PCI subsystem without any tangible benefit?

There's also a PCIe bus available on the SoC. It is not yet supported,
but we'll eventually need the entire PCI subsystem, which is not that huge as it may seem, btw.

Besides, the internal PCI driver has already been submitted to the PCI mailing list
and applied to Bjorn's tree.

I don't see how treating the PCI devices as platform devices is better than
adjusting the initialization order of the USB phy.

>
> CU
> Uli
>

Thanks,
Val.

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Alan Stern]

On Thu, 31 Oct 2013, Valentine wrote:

> > Do you mean to change usb_hcd_pci_probe() to return -EPROBE_DEFER if the phy is not ready?
> > Or should I defer the whole PCI subsystem initialization (pci_common_int)?
> 
> Greg,
> the reason I ask is that it doesn't seem that simple to me.
> 
> Here's some details:
> The h/w is an ARM SoC that has 3 internal PCI controllers with a single EHCI/OHCI on each one.
> This gives us 3 USB channels as this is called in the h/w manual.
> Channel 0 is shared with USBHS (USB function) device.
> Channel 2 is shared with USBSS (USB3.0 host).
> Both channels are configured by a single USB phy.
> USB PHY is a platform device, while EHCI/OCHI are located on the PCI busses.
> 
> If PCI USB host is probed before USB phy, the EHCI/OHCI device is
> detected, but nothing works.
> 
> We could change the USB HDC PCI driver and make usb_hcd_pci_probe() return -EPROBE_DEFER,
> but I'm not sure how the condition for that should be phrased.

You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
to be the proper solution to your problem.

The difficulty is that you have a discoverable device (the PCI EHCI
controller) which needs to wait for a platform device (the PHY).  The
kernel doesn't have a good way to describe such a constraint between
two different kinds of device like that, as far as I know.

This is similar to the problems facing Runtime Interpreted Power 
Sequences, although not quite the same.

Alan Stern

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 10/31/2013 08:54 PM, Alan Stern wrote:
> On Thu, 31 Oct 2013, Valentine wrote:
>
>>> Do you mean to change usb_hcd_pci_probe() to return -EPROBE_DEFER if the phy is not ready?
>>> Or should I defer the whole PCI subsystem initialization (pci_common_int)?
>>
>> Greg,
>> the reason I ask is that it doesn't seem that simple to me.
>>
>> Here's some details:
>> The h/w is an ARM SoC that has 3 internal PCI controllers with a single EHCI/OHCI on each one.
>> This gives us 3 USB channels as this is called in the h/w manual.
>> Channel 0 is shared with USBHS (USB function) device.
>> Channel 2 is shared with USBSS (USB3.0 host).
>> Both channels are configured by a single USB phy.
>> USB PHY is a platform device, while EHCI/OCHI are located on the PCI busses.
>>
>> If PCI USB host is probed before USB phy, the EHCI/OHCI device is
>> detected, but nothing works.
>>
>> We could change the USB HDC PCI driver and make usb_hcd_pci_probe() return -EPROBE_DEFER,
>> but I'm not sure how the condition for that should be phrased.
>
> You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
> to be the proper solution to your problem.
>
> The difficulty is that you have a discoverable device (the PCI EHCI
> controller) which needs to wait for a platform device (the PHY).  The
> kernel doesn't have a good way to describe such a constraint between
> two different kinds of device like that, as far as I know.

Thanks, unfortunately this doesn't help.
I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.
However, at the same time I see that six usb phy drivers use subsys_initcall and one
uses postcore_initcall to adjust the initialization order.

The same approach is used with other drivers quite often. Take I2C, for example.
I'm not sure why we can't use it here with the R-Car Gen2 phy.
This driver is used only with R-Car SoC and the approach is trivial and  working fine.

Why can't we use it instead of trying to create a bigger mess in the USB HCD PCI driver,
which is used on quite a number of platforms, to workaround the PHY initialization order
that is only relevant to R-Car Gen2 SoC?

>
> This is similar to the problems facing Runtime Interpreted Power
> Sequences, although not quite the same.
>
> Alan Stern
>

Thanks,
Val.

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Alan Stern]

On Fri, 1 Nov 2013, Valentine wrote:

> > You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
> > to be the proper solution to your problem.
> >
> > The difficulty is that you have a discoverable device (the PCI EHCI
> > controller) which needs to wait for a platform device (the PHY).  The
> > kernel doesn't have a good way to describe such a constraint between
> > two different kinds of device like that, as far as I know.
> 
> Thanks, unfortunately this doesn't help.
> I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.

I'm not sure either.  It requires further discussion, and it is an 
important problem.  Not a trivial one, as you seem to think.

> However, at the same time I see that six usb phy drivers use subsys_initcall and one
> uses postcore_initcall to adjust the initialization order.
> 
> The same approach is used with other drivers quite often. Take I2C, for example.
> I'm not sure why we can't use it here with the R-Car Gen2 phy.

The fact that other drivers do it doesn't mean it is the right thing to
do.  (Besides, I2C is different from PCI because it isn't discoverable,
right?)

Greg KH can explain further, if you ask him.

> This driver is used only with R-Car SoC and the approach is trivial and  working fine.
> 
> Why can't we use it instead of trying to create a bigger mess in the USB HCD PCI driver,
> which is used on quite a number of platforms, to workaround the PHY initialization order
> that is only relevant to R-Car Gen2 SoC?

Because other platforms are likely to experience the same problem in 
the future.  And as you pointed out, other platforms already _are_ 
experiencing this problem (although perhaps for other drivers).

We should implement a proper solution.  One that can be used
everywhere, not an initcall-order hack.

Alan Stern

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Greg KH]

On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:
> I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.
> However, at the same time I see that six usb phy drivers use subsys_initcall and one
> uses postcore_initcall to adjust the initialization order.

Then they are all wrong and should be fixed.  Again, this is _why_ we
created the deferred probing logic, and it should be used for this type
of thing, as trying to juggle init call levels is madness and you will
loose in the end (think multi-system kernel images, how is that going to
work?)

thanks,

greg k-h

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 11/01/2013 06:17 PM, Alan Stern wrote:
> On Fri, 1 Nov 2013, Valentine wrote:
>
>>> You need to tell usb_hcd_pci_probe() to wait for the PHY.  That seems
>>> to be the proper solution to your problem.
>>>
>>> The difficulty is that you have a discoverable device (the PCI EHCI
>>> controller) which needs to wait for a platform device (the PHY).  The
>>> kernel doesn't have a good way to describe such a constraint between
>>> two different kinds of device like that, as far as I know.
>>
>> Thanks, unfortunately this doesn't help.
>> I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.
>
> I'm not sure either.  It requires further discussion, and it is an
> important problem.  Not a trivial one, as you seem to think.
>
>> However, at the same time I see that six usb phy drivers use subsys_initcall and one
>> uses postcore_initcall to adjust the initialization order.
>>
>> The same approach is used with other drivers quite often. Take I2C, for example.
>> I'm not sure why we can't use it here with the R-Car Gen2 phy.
>
> The fact that other drivers do it doesn't mean it is the right thing to
> do.  (Besides, I2C is different from PCI because it isn't discoverable,
> right?)
>

I'm not using postcore_init with PCI. I'm doing it for a platform USB phy driver.
Anyway why can't we use postcore_initcall to register a driver for a discoverable device?

> Greg KH can explain further, if you ask him.

Yes, I'd like to hear from Greg as well.
Explanaitions and suggestions are greatly appreciated.

>
>> This driver is used only with R-Car SoC and the approach is trivial and  working fine.
>>
>> Why can't we use it instead of trying to create a bigger mess in the USB HCD PCI driver,
>> which is used on quite a number of platforms, to workaround the PHY initialization order
>> that is only relevant to R-Car Gen2 SoC?
>
> Because other platforms are likely to experience the same problem in
> the future.  And as you pointed out, other platforms already _are_
> experiencing this problem (although perhaps for other drivers).
>
> We should implement a proper solution.  One that can be used
> everywhere, not an initcall-order hack.

That would be great, but I don't think we can implement a universal solution for all of those drivers.
The solution that may suit R-Car may not be good for other USB phy drivers since most likely they have
their init order fixed-up for other reasons not related to PCI USB hosts.

Fixing all the drivers so that they all can use deferred probing is, in my opinion, a bit out of scope
here.

The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how to make a particular
PCI USB HDC device attached to a particular PCI host controller on a particular SoC defer its probing
while waiting for the USB phy. At the same time other identical PCI USB HCD devices (with the same PCI id)
on the same SoC should be probed as usual.

We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither can we use PCI busses
to distinguish between PCI host controllers, since bus numbers are assigned dynamically.

It looks that it's quite hard to do that without bigger hacks in the PCI USB HCD driver that are
most likely not to be used on any other platforms except for R-Car.

I just can't see why changing init order is considered such a bad hack in this case,
while it is being used fine with other USB phy drivers.

Can't we use it for R-Car until a universal solution to the deferred probing is found?

Probably, moving the whole phy subsystem to the earlier initialization stage would be a good-enough
solution since that many PHY drivers have already been using it?
You know, just like the PCI host drivers are initialized at subsys_initcall, or is it considered wrong?

>
> Alan Stern
>

Thanks,
Val.

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 11/01/2013 06:32 PM, Greg KH wrote:
> On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:
>> I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.
>> However, at the same time I see that six usb phy drivers use subsys_initcall and one
>> uses postcore_initcall to adjust the initialization order.
>
> Then they are all wrong and should be fixed.  Again, this is _why_ we
> created the deferred probing logic, and it should be used for this type
> of thing, as trying to juggle init call levels is madness and you will
> loose in the end (think multi-system kernel images, how is that going to
> work?)

I'm sorry, I don't see how moving driver registration from
device_initcall to postcore_initcall alone breaks multi-system kernel image.

>
> thanks,
>
> greg k-h
>

Thanks,
Val.

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Greg KH]

On Fri, Nov 01, 2013 at 07:26:51PM +0400, Valentine wrote:
> On 11/01/2013 06:32 PM, Greg KH wrote:
> >On Fri, Nov 01, 2013 at 05:59:40PM +0400, Valentine wrote:
> >>I'm not sure how this problem should be addressed using USB HCD PCI deferred probing.
> >>However, at the same time I see that six usb phy drivers use subsys_initcall and one
> >>uses postcore_initcall to adjust the initialization order.
> >
> >Then they are all wrong and should be fixed.  Again, this is _why_ we
> >created the deferred probing logic, and it should be used for this type
> >of thing, as trying to juggle init call levels is madness and you will
> >loose in the end (think multi-system kernel images, how is that going to
> >work?)
> 
> I'm sorry, I don't see how moving driver registration from
> device_initcall to postcore_initcall alone breaks multi-system kernel
> image.

Ok, in this specific case, it probably doesn't make a difference, but
it's still a mess, right?  What happens if everyone uses
postcore_initcall, then we need to add another level after that because
someone wants to be there instead?

Again, that is why we added the deferred probing logic, use it please.

I should just go rip those changes out and put everything at
device_initcall in order to get people to fix the code properly...

thanks,

greg k-h

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Greg KH]

On Fri, Nov 01, 2013 at 07:04:00PM +0400, Valentine wrote:
> >We should implement a proper solution.  One that can be used
> >everywhere, not an initcall-order hack.
> 
> That would be great, but I don't think we can implement a universal solution for all of those drivers.
> The solution that may suit R-Car may not be good for other USB phy drivers since most likely they have
> their init order fixed-up for other reasons not related to PCI USB hosts.

That's the best reason I've heard to fix this properly, and NOT use init
call ordering.  So glad we agree :)

> Fixing all the drivers so that they all can use deferred probing is, in my opinion, a bit out of scope
> here.

Not at all.  I understand you feel a little put-apon due to this, but
that's how kernel development works, sorry.

> The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how to make a particular
> PCI USB HDC device attached to a particular PCI host controller on a particular SoC defer its probing
> while waiting for the USB phy. At the same time other identical PCI USB HCD devices (with the same PCI id)
> on the same SoC should be probed as usual.
> 
> We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither can we use PCI busses
> to distinguish between PCI host controllers, since bus numbers are assigned dynamically.
> 
> It looks that it's quite hard to do that without bigger hacks in the PCI USB HCD driver that are
> most likely not to be used on any other platforms except for R-Car.
> 
> I just can't see why changing init order is considered such a bad hack in this case,
> while it is being used fine with other USB phy drivers.

It isn't fine for those other drivers, I just happened to miss that they
were doing this, and you got "caught" trying to make the same change.
I'll go break them as well if it makes you feel any better.

> Can't we use it for R-Car until a universal solution to the deferred probing is found?

The "universal" solution is to implement deferred probing, why not just
do that?

greg k-h

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Alan Stern]

On Fri, 1 Nov 2013, Valentine wrote:

> The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how to make a particular
> PCI USB HDC device attached to a particular PCI host controller on a particular SoC defer its probing
> while waiting for the USB phy. At the same time other identical PCI USB HCD devices (with the same PCI id)
> on the same SoC should be probed as usual.

That is the hard part.  That's what we need to discuss.

And not just on the linux-usb mailing list.  Get other people 
(especially PCI people) involved too.

> We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither can we use PCI busses
> to distinguish between PCI host controllers, since bus numbers are assigned dynamically.

There must be some way for you to tell which PCI devices use the PHY.  
But I'm not a PCI expert, and I'm not familiar with your platform.

> It looks that it's quite hard to do that without bigger hacks in the PCI USB HCD driver that are
> most likely not to be used on any other platforms except for R-Car.

You never know what will happen in the future.  Besides, even if nobody 
else needs to do this for a PCI device, you can be certain that the 
same sort of thing will be needed for devices on other buses.  A 
suitably general solution would help them too.

Alan Stern

Re: [PATCH] usb: phy: Move R-Car Gen2 driver registration to postcore_inictall

[Valentine]

On 11/01/2013 07:55 PM, Alan Stern wrote:
> On Fri, 1 Nov 2013, Valentine wrote:
>
>> The USB HDC PCI deferred probing could be used on R-Car. But I'm not sure how to make a particular
>> PCI USB HDC device attached to a particular PCI host controller on a particular SoC defer its probing
>> while waiting for the USB phy. At the same time other identical PCI USB HCD devices (with the same PCI id)
>> on the same SoC should be probed as usual.
>
> That is the hard part.  That's what we need to discuss.
>
> And not just on the linux-usb mailing list.  Get other people
> (especially PCI people) involved too.
>
>> We can't use PCI ids here to distinguish between PCI USB HCD devices. Neither can we use PCI busses
>> to distinguish between PCI host controllers, since bus numbers are assigned dynamically.
>
> There must be some way for you to tell which PCI devices use the PHY.
> But I'm not a PCI expert, and I'm not familiar with your platform.
>
>> It looks that it's quite hard to do that without bigger hacks in the PCI USB HCD driver that are
>> most likely not to be used on any other platforms except for R-Car.
>
> You never know what will happen in the future.  Besides, even if nobody
> else needs to do this for a PCI device, you can be certain that the
> same sort of thing will be needed for devices on other buses.  A
> suitably general solution would help them too.
>
> Alan Stern
>

OK, I've tried to come up with more or less universal solution to defer HCD probing
until the external USB phy is ready. I'll start a new thread with the RFC patches in a bit.

Thanks,
Val.