[PATCH] USB: ehci-omap: Select USB_PHY

[PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

Hi Greg,

The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
is enabled.

Patch is based on your usb-next branch and is needed for 3.10.

From: Roger Quadros <rogerq@ti.com>
Date: Thu, 11 Apr 2013 12:08:19 +0300
Subject: [PATCH] USB: ehci-omap: Select USB_PHY

As we need NOP_USB_XCEIV which depends on USB_PHY
we need to select USB_PHY as well.

Gets rid of the below warnings when USB_EHCI_HCD_OMAP
is enabled.

warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index c0be25c..931b437 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -150,6 +150,7 @@ config USB_EHCI_MXC
 config USB_EHCI_HCD_OMAP
 	tristate "EHCI support for OMAP3 and later chips"
 	depends on ARCH_OMAP
+	select USB_PHY
 	select NOP_USB_XCEIV
 	default y
 	---help---
-- 
1.7.4.1

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Hi,

On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
> Hi Greg,
> 
> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
> is enabled.
> 
> Patch is based on your usb-next branch and is needed for 3.10.
> 
> From: Roger Quadros <rogerq@ti.com>
> Date: Thu, 11 Apr 2013 12:08:19 +0300
> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
> 
> As we need NOP_USB_XCEIV which depends on USB_PHY
> we need to select USB_PHY as well.
> 
> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
> is enabled.
> 
> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Ideally, however, we wouldn't select any PHY in particular as different
boards might need a different PHY driver, even on OMAP ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

On 04/11/2013 01:04 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
>> Hi Greg,
>>
>> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
>> is enabled.
>>
>> Patch is based on your usb-next branch and is needed for 3.10.
>>
>> From: Roger Quadros <rogerq@ti.com>
>> Date: Thu, 11 Apr 2013 12:08:19 +0300
>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
>>
>> As we need NOP_USB_XCEIV which depends on USB_PHY
>> we need to select USB_PHY as well.
>>
>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
>> is enabled.
>>
>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Ideally, however, we wouldn't select any PHY in particular as different
> boards might need a different PHY driver, even on OMAP ;-)
> 
Right, but we need to select USB_PHY here as the driver uses
the USB_PHY APIs.

The NOP_USB_XCEIV selection could be done by the board config.

cheers,
-roger

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

Hi,

On Thu, Apr 11, 2013 at 01:51:16PM +0300, Roger Quadros wrote:
> On 04/11/2013 01:04 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
> >> Hi Greg,
> >>
> >> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
> >> is enabled.
> >>
> >> Patch is based on your usb-next branch and is needed for 3.10.
> >>
> >> From: Roger Quadros <rogerq@ti.com>
> >> Date: Thu, 11 Apr 2013 12:08:19 +0300
> >> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
> >>
> >> As we need NOP_USB_XCEIV which depends on USB_PHY
> >> we need to select USB_PHY as well.
> >>
> >> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
> >> is enabled.
> >>
> >> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> > 
> > Ideally, however, we wouldn't select any PHY in particular as different
> > boards might need a different PHY driver, even on OMAP ;-)
> > 
> Right, but we need to select USB_PHY here as the driver uses
> the USB_PHY APIs.
> 
> The NOP_USB_XCEIV selection could be done by the board config.

I would avoid 'select' completely and just update omap2plus_defconfig
adding those two as modules.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

On 04/11/2013 01:55 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Apr 11, 2013 at 01:51:16PM +0300, Roger Quadros wrote:
>> On 04/11/2013 01:04 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
>>>> Hi Greg,
>>>>
>>>> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
>>>> is enabled.
>>>>
>>>> Patch is based on your usb-next branch and is needed for 3.10.
>>>>
>>>> From: Roger Quadros <rogerq@ti.com>
>>>> Date: Thu, 11 Apr 2013 12:08:19 +0300
>>>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
>>>>
>>>> As we need NOP_USB_XCEIV which depends on USB_PHY
>>>> we need to select USB_PHY as well.
>>>>
>>>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
>>>> is enabled.
>>>>
>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>
>>> Ideally, however, we wouldn't select any PHY in particular as different
>>> boards might need a different PHY driver, even on OMAP ;-)
>>>
>> Right, but we need to select USB_PHY here as the driver uses
>> the USB_PHY APIs.
>>
>> The NOP_USB_XCEIV selection could be done by the board config.
> 
> I would avoid 'select' completely and just update omap2plus_defconfig
> adding those two as modules.
> 

OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".

cheers,
-roger

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Alexander Holler]

Am 11.04.2013 14:42, schrieb Roger Quadros:
> On 04/11/2013 01:55 PM, Felipe Balbi wrote:

>> I would avoid 'select' completely and just update omap2plus_defconfig
>> adding those two as modules.
>>
> 
> OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".

Sorry, but this just will end up with many users having broken configs
because of disabled stuff they don't know why they have to enable them.
And thus with a never ending stream of questions and thus with a needed
FAQ entry.

Regards,

Alexander

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

Felipe,

On 04/11/2013 04:02 PM, Alexander Holler wrote:
> Am 11.04.2013 14:42, schrieb Roger Quadros:
>> On 04/11/2013 01:55 PM, Felipe Balbi wrote:
> 
>>> I would avoid 'select' completely and just update omap2plus_defconfig
>>> adding those two as modules.

Setting USB_PHY as a module gives rise to these problems

arch/arm/mach-omap2/built-in.o: In function `usbhs_init_phys':
/work/linux-2.6/arch/arm/mach-omap2/usb-host.c:652: undefined reference to `usb_bind_phy'
arch/arm/mach-omap2/built-in.o: In function `omap_2430sdp_init':
/work/linux-2.6/arch/arm/mach-omap2/board-2430sdp.c:236: undefined reference to `usb_bind_phy'
arch/arm/mach-omap2/built-in.o: In function `omap3_beagle_init':
/work/linux-2.6/arch/arm/mach-omap2/board-omap3beagle.c:554: undefined reference to `usb_bind_phy'
arch/arm/mach-omap2/built-in.o: In function `devkit8000_init':
/work/linux-2.6/arch/arm/mach-omap2/board-devkit8000.c:596: undefined reference to `usb_bind_phy'
arch/arm/mach-omap2/built-in.o: In function `omap_ldp_init':
/work/linux-2.6/arch/arm/mach-omap2/board-ldp.c:379: undefined reference to `usb_bind_phy'

So USB_PHY shouldn't be tristate IMO or at least the platform registration stuff
as the usb_bind_phy() part is used by platform code.

>>>
>>
>> OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".
> 
> Sorry, but this just will end up with many users having broken configs
> because of disabled stuff they don't know why they have to enable them.
> And thus with a never ending stream of questions and thus with a needed
> FAQ entry.
> 

Alexander,

I agree with you that it can get difficult with users. But it is best for
users do not disable anything they are not familiar with.

As the USB_PHY option doesn't depend on anything, it is safe to select it
from USB_EHCI_HCD_OMAP.

However, the PHY drivers themselves must be selected from the board configs.

cheers,
-roger

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

Hi,

On Thu, Apr 11, 2013 at 05:44:00PM +0300, Roger Quadros wrote:
> Felipe,
> 
> On 04/11/2013 04:02 PM, Alexander Holler wrote:
> > Am 11.04.2013 14:42, schrieb Roger Quadros:
> >> On 04/11/2013 01:55 PM, Felipe Balbi wrote:
> > 
> >>> I would avoid 'select' completely and just update omap2plus_defconfig
> >>> adding those two as modules.
> 
> Setting USB_PHY as a module gives rise to these problems
> 
> arch/arm/mach-omap2/built-in.o: In function `usbhs_init_phys':
> /work/linux-2.6/arch/arm/mach-omap2/usb-host.c:652: undefined reference to `usb_bind_phy'
> arch/arm/mach-omap2/built-in.o: In function `omap_2430sdp_init':
> /work/linux-2.6/arch/arm/mach-omap2/board-2430sdp.c:236: undefined reference to `usb_bind_phy'
> arch/arm/mach-omap2/built-in.o: In function `omap3_beagle_init':
> /work/linux-2.6/arch/arm/mach-omap2/board-omap3beagle.c:554: undefined reference to `usb_bind_phy'
> arch/arm/mach-omap2/built-in.o: In function `devkit8000_init':
> /work/linux-2.6/arch/arm/mach-omap2/board-devkit8000.c:596: undefined reference to `usb_bind_phy'
> arch/arm/mach-omap2/built-in.o: In function `omap_ldp_init':
> /work/linux-2.6/arch/arm/mach-omap2/board-ldp.c:379: undefined reference to `usb_bind_phy'
> 
> So USB_PHY shouldn't be tristate IMO or at least the platform registration stuff
> as the usb_bind_phy() part is used by platform code.

alright, let's send a patch to fix the check in the header. We should be
using IS_ENABLED() anyway.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Alexander Holler]

Am 11.04.2013 16:44, schrieb Roger Quadros:

>> Sorry, but this just will end up with many users having broken configs
>> because of disabled stuff they don't know why they have to enable them.
>> And thus with a never ending stream of questions and thus with a needed
>> FAQ entry.
>>
>
> Alexander,
>
> I agree with you that it can get difficult with users. But it is best for
> users do not disable anything they are not familiar with.
>
> As the USB_PHY option doesn't depend on anything, it is safe to select it
> from USB_EHCI_HCD_OMAP.
>
> However, the PHY drivers themselves must be selected from the board configs.

Maybe I understood something wrong, but if the OMAP USB driver requires 
CONFIG_USB_PHY (or similiar), it should be selected automatically and 
not just enabled in the board config.

I just had it to often, that I needed to search around why a driver 
doesn't work (or even compiles), just to find out that I need to enable 
some strange config option which wasn't selected automatically. 
Especially with OMAPs. ;)

And this not only occured when disabling options, it often occured by 
just updating the kernel. Suddenly some obscur option was needed too, it 
wasn't selected automatically by make oldconfig, bang. So the argument 
to not remove anything from a board config doesn't help much.

Sorry for the rant. ;)

Regards,

Alexander

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]

Hi,

On Thu, Apr 11, 2013 at 07:33:32PM +0200, Alexander Holler wrote:
> >>Sorry, but this just will end up with many users having broken configs
> >>because of disabled stuff they don't know why they have to enable them.
> >>And thus with a never ending stream of questions and thus with a needed
> >>FAQ entry.
> >>
> >
> >Alexander,
> >
> >I agree with you that it can get difficult with users. But it is best for
> >users do not disable anything they are not familiar with.
> >
> >As the USB_PHY option doesn't depend on anything, it is safe to select it
> >from USB_EHCI_HCD_OMAP.
> >
> >However, the PHY drivers themselves must be selected from the board configs.
> 
> Maybe I understood something wrong, but if the OMAP USB driver
> requires CONFIG_USB_PHY (or similiar), it should be selected
> automatically and not just enabled in the board config.

and who said OMAP USB depends on CONFIG_USB_PHY ? Some platforms need to
control a PHY and some don't.

> I just had it to often, that I needed to search around why a driver
> doesn't work (or even compiles), just to find out that I need to
> enable some strange config option which wasn't selected
> automatically. Especially with OMAPs. ;)

so ? Send a patch fixing it, those are really welcome. You see, it's
very difficult to get all of this perfectly right and if people continue
to rant rather than fix, we will get nowhere.

> And this not only occured when disabling options, it often occured by
> just updating the kernel. Suddenly some obscur option was needed too,
> it wasn't selected automatically by make oldconfig, bang. So the
> argument to not remove anything from a board config doesn't help
> much.

blablabla, Kconfig changes are *always* necessary. Specially when we
need to re-design an entire API because the previous one was just a
*SINGLE* global pointer.

Go check out kernel 2.6.39 (maybe even 3.1 and 3.2) and you'll see that
we're much better off today where we can actually have multiple PHY
drivers and multiple UDC drivers enabled (either as modules or
built-in), but the fact is that changing all of this over takes time and
sometimes people make mistakes, but that's alright, since we have the
-rc series to catch those unwanted errors.

Without the help of the rest of the community, though, it'll just get
slower and slower. With the whole single zImage effort going on in the
ARM land, things have gotten much more critical WRT getting rid of
"selects" and turning legacy "drivers" into real drivers, not just a
bunch of exported functions which a single architecture uses.

Add to that all the rework going on in the Gadget Framework, PHY layer
and EHCI drivers (which now has a core re-usable library thanks to Alan
Stern) and you have a lot of work to do.

Next time you consider ranting about something, use that 'frustration'
and turn it into motivation to write patches, then we all win. Also
consider that under drivers/usb/ alone we have 270K LOCs, and that's
quite a lot of code to handle with only a few of us (Greg, Sarah, Alan,
Alex and myself) being the gateway towards mainline.

With all that comes the responsibility of revieweing drivers for
architectures we, most of the times, don't have access to with drivers
that only compile in some ceratin ways.

Cleaning all of that takes time. Look at all the effort it's taking the
Chipidea folks just to get rid of a couple copies of the chipidea
driver. It takes time, it takes sweat and we can all use some help
rather than some random rant.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Alexander Holler]

Am 11.04.2013 20:29, schrieb Felipe Balbi:

> and who said OMAP USB depends on CONFIG_USB_PHY ? Some platforms need to
> control a PHY and some don't.

I've read that so.

> Go check out kernel 2.6.39 (maybe even 3.1 and 3.2) and you'll see that
> we're much better off today where we can actually have multiple PHY
> drivers and multiple UDC drivers enabled (either as modules or
> built-in), but the fact is that changing all of this over takes time and
> sometimes people make mistakes, but that's alright, since we have the
> -rc series to catch those unwanted errors.

I'll still have to stick to 3.2 because nothing afterwards boots from
EHCI on my beagleboard.

> 
> Without the help of the rest of the community, though, it'll just get
> slower and slower. With the whole single zImage effort going on in the
> ARM land, things have gotten much more critical WRT getting rid of
> "selects" and turning legacy "drivers" into real drivers, not just a
> bunch of exported functions which a single architecture uses.
> 
> Add to that all the rework going on in the Gadget Framework, PHY layer
> and EHCI drivers (which now has a core re-usable library thanks to Alan
> Stern) and you have a lot of work to do.
> 
> Next time you consider ranting about something, use that 'frustration'
> and turn it into motivation to write patches, then we all win. Also

I frequently tried and gave up. But thanks for the good suggestions.

Regards,

Alexander

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

On 04/11/2013 03:42 PM, Roger Quadros wrote:
> On 04/11/2013 01:55 PM, Felipe Balbi wrote:
>> Hi,
>>
>> On Thu, Apr 11, 2013 at 01:51:16PM +0300, Roger Quadros wrote:
>>> On 04/11/2013 01:04 PM, Felipe Balbi wrote:
>>>> Hi,
>>>>
>>>> On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
>>>>> Hi Greg,
>>>>>
>>>>> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
>>>>> is enabled.
>>>>>
>>>>> Patch is based on your usb-next branch and is needed for 3.10.
>>>>>
>>>>> From: Roger Quadros <rogerq@ti.com>
>>>>> Date: Thu, 11 Apr 2013 12:08:19 +0300
>>>>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
>>>>>
>>>>> As we need NOP_USB_XCEIV which depends on USB_PHY
>>>>> we need to select USB_PHY as well.
>>>>>
>>>>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
>>>>> is enabled.
>>>>>
>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>
>>>> Ideally, however, we wouldn't select any PHY in particular as different
>>>> boards might need a different PHY driver, even on OMAP ;-)
>>>>
>>> Right, but we need to select USB_PHY here as the driver uses
>>> the USB_PHY APIs.
>>>
>>> The NOP_USB_XCEIV selection could be done by the board config.
>>
>> I would avoid 'select' completely and just update omap2plus_defconfig
>> adding those two as modules.
>>
> 
> OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".
> 

One more issue to clarify.

if USB_PHY is not enabled, then all phy_get() API's should return NULL and not
-ENXIO as it does now.

This way the drivers need not treat it as an error and all PHY ops can be NOPs.

This will make it behave like other frameworks. e.g. clk.

cheers,
-roger.

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

Hi,

On Thu, Apr 11, 2013 at 04:18:33PM +0300, Roger Quadros wrote:
> On 04/11/2013 03:42 PM, Roger Quadros wrote:
> > On 04/11/2013 01:55 PM, Felipe Balbi wrote:
> >> Hi,
> >>
> >> On Thu, Apr 11, 2013 at 01:51:16PM +0300, Roger Quadros wrote:
> >>> On 04/11/2013 01:04 PM, Felipe Balbi wrote:
> >>>> Hi,
> >>>>
> >>>> On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
> >>>>> Hi Greg,
> >>>>>
> >>>>> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
> >>>>> is enabled.
> >>>>>
> >>>>> Patch is based on your usb-next branch and is needed for 3.10.
> >>>>>
> >>>>> From: Roger Quadros <rogerq@ti.com>
> >>>>> Date: Thu, 11 Apr 2013 12:08:19 +0300
> >>>>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
> >>>>>
> >>>>> As we need NOP_USB_XCEIV which depends on USB_PHY
> >>>>> we need to select USB_PHY as well.
> >>>>>
> >>>>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
> >>>>> is enabled.
> >>>>>
> >>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >>>>>
> >>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >>>>
> >>>> Ideally, however, we wouldn't select any PHY in particular as different
> >>>> boards might need a different PHY driver, even on OMAP ;-)
> >>>>
> >>> Right, but we need to select USB_PHY here as the driver uses
> >>> the USB_PHY APIs.
> >>>
> >>> The NOP_USB_XCEIV selection could be done by the board config.
> >>
> >> I would avoid 'select' completely and just update omap2plus_defconfig
> >> adding those two as modules.
> >>
> > 
> > OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".
> > 
> 
> One more issue to clarify.
> 
> if USB_PHY is not enabled, then all phy_get() API's should return NULL and not
> -ENXIO as it does now.

ENXIO means "No such device or address", looks alright to me ;-)

> This way the drivers need not treat it as an error and all PHY ops can
> be NOPs.

drivers will already be using if (IS_ERR()) construction, returning
-ENXIO when the API is disabled gives them an oportunity to *not*
request probe deferral since the API isn't enabled anyway.

> This will make it behave like other frameworks. e.g. clk.

if we return NULL we will need IS_ERR_OR_NULL() which will cause
problems with people who aren't careful enough.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Roger Quadros]

On 04/11/2013 05:34 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Apr 11, 2013 at 04:18:33PM +0300, Roger Quadros wrote:
>> On 04/11/2013 03:42 PM, Roger Quadros wrote:
>>> On 04/11/2013 01:55 PM, Felipe Balbi wrote:
>>>> Hi,
>>>>
>>>> On Thu, Apr 11, 2013 at 01:51:16PM +0300, Roger Quadros wrote:
>>>>> On 04/11/2013 01:04 PM, Felipe Balbi wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Apr 11, 2013 at 12:42:04PM +0300, Roger Quadros wrote:
>>>>>>> Hi Greg,
>>>>>>>
>>>>>>> The following patch gets rid of Kbuild warnings when USB_EHCI_HCD_OMAP
>>>>>>> is enabled.
>>>>>>>
>>>>>>> Patch is based on your usb-next branch and is needed for 3.10.
>>>>>>>
>>>>>>> From: Roger Quadros <rogerq@ti.com>
>>>>>>> Date: Thu, 11 Apr 2013 12:08:19 +0300
>>>>>>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
>>>>>>>
>>>>>>> As we need NOP_USB_XCEIV which depends on USB_PHY
>>>>>>> we need to select USB_PHY as well.
>>>>>>>
>>>>>>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
>>>>>>> is enabled.
>>>>>>>
>>>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
>>>>>>>
>>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>>
>>>>>> Ideally, however, we wouldn't select any PHY in particular as different
>>>>>> boards might need a different PHY driver, even on OMAP ;-)
>>>>>>
>>>>> Right, but we need to select USB_PHY here as the driver uses
>>>>> the USB_PHY APIs.
>>>>>
>>>>> The NOP_USB_XCEIV selection could be done by the board config.
>>>>
>>>> I would avoid 'select' completely and just update omap2plus_defconfig
>>>> adding those two as modules.
>>>>
>>>
>>> OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".
>>>
>>
>> One more issue to clarify.
>>
>> if USB_PHY is not enabled, then all phy_get() API's should return NULL and not
>> -ENXIO as it does now.
> 
> ENXIO means "No such device or address", looks alright to me ;-)
> 
>> This way the drivers need not treat it as an error and all PHY ops can
>> be NOPs.
> 
> drivers will already be using if (IS_ERR()) construction, returning
> -ENXIO when the API is disabled gives them an oportunity to *not*
> request probe deferral since the API isn't enabled anyway.

on second thoughts I agree with you. So the general understanding is
that USB_PHY users without USB_PHY enabled is an error case.

This means we need to allow controller drivers to select USB_PHY
and minimize this possibility.

> 
>> This will make it behave like other frameworks. e.g. clk.
> 
> if we return NULL we will need IS_ERR_OR_NULL() which will cause
> problems with people who aren't careful enough.
> 
OK.

cheers,
-roger

Re: [PATCH] USB: ehci-omap: Select USB_PHY

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2512 bytes --]

Hi,

On Thu, Apr 11, 2013 at 05:53:10PM +0300, Roger Quadros wrote:
> >>>>>>> From: Roger Quadros <rogerq@ti.com>
> >>>>>>> Date: Thu, 11 Apr 2013 12:08:19 +0300
> >>>>>>> Subject: [PATCH] USB: ehci-omap: Select USB_PHY
> >>>>>>>
> >>>>>>> As we need NOP_USB_XCEIV which depends on USB_PHY
> >>>>>>> we need to select USB_PHY as well.
> >>>>>>>
> >>>>>>> Gets rid of the below warnings when USB_EHCI_HCD_OMAP
> >>>>>>> is enabled.
> >>>>>>>
> >>>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >>>>>>> warning: (USB_EHCI_HCD_OMAP) selects NOP_USB_XCEIV which has unmet direct dependencies (USB_SUPPORT && USB_PHY)
> >>>>>>>
> >>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >>>>>>
> >>>>>> Ideally, however, we wouldn't select any PHY in particular as different
> >>>>>> boards might need a different PHY driver, even on OMAP ;-)
> >>>>>>
> >>>>> Right, but we need to select USB_PHY here as the driver uses
> >>>>> the USB_PHY APIs.
> >>>>>
> >>>>> The NOP_USB_XCEIV selection could be done by the board config.
> >>>>
> >>>> I would avoid 'select' completely and just update omap2plus_defconfig
> >>>> adding those two as modules.
> >>>>
> >>>
> >>> OK, makes sense. I will update the patch to remove "select NOP_USB_XCEIV".
> >>>
> >>
> >> One more issue to clarify.
> >>
> >> if USB_PHY is not enabled, then all phy_get() API's should return NULL and not
> >> -ENXIO as it does now.
> > 
> > ENXIO means "No such device or address", looks alright to me ;-)
> > 
> >> This way the drivers need not treat it as an error and all PHY ops can
> >> be NOPs.
> > 
> > drivers will already be using if (IS_ERR()) construction, returning
> > -ENXIO when the API is disabled gives them an oportunity to *not*
> > request probe deferral since the API isn't enabled anyway.
> 
> on second thoughts I agree with you. So the general understanding is
> that USB_PHY users without USB_PHY enabled is an error case.
> 
> This means we need to allow controller drivers to select USB_PHY
> and minimize this possibility.

perhaps but OTOH careless select will also cause lots of problems.
distro-like kernels will just put all those as modules and product-like
kernels will only enable exactly what they need, so it makes no
difference if we select or not. Except that select will enable that PHY
even in e.g. beaglebone derivative which is, for now, using the same DTS
file.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]