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

[Felipe Balbi]

Hi,

On Thu, Apr 11, 2013 at 05:53:10PM +0300, Roger Quadros wrote:
> >>>>>>> From: Roger Quadros <rogerq@xxxxxx>
> >>>>>>> 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@xxxxxx>
> >>>>>>
> >>>>>> 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: signature.asc
Description: Digital signature