Re: [PATCH] usb:phy: propagate __of_usb_find_phy()'s error on failure

[Felipe Balbi]

Hi again,

On Mon, Nov 24, 2014 at 09:36:21AM -0600, Felipe Balbi wrote:
> > > > > When __of_usb_find_phy() fails, it returns -ENODEV - its
> > > > > error code has to be returned by devm_usb_get_phy_by_phandle().
> > > > > Only when the former function succeeds and try_module_get()
> > > > > fails should -EPROBE_DEFER be returned.
> > > > > 
> > > > > Signed-off-by: Arjun Sreedharan <arjun024@xxxxxxxxx>
> > > > > ---
> > > > >  drivers/usb/phy/phy.c | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > This causes a boot regression on at least NVIDIA Dalmore (I boot over
> > > > NFS using a USB network adapter).
> > > > 
> > > > The commit message is somewhat insufficient because while it explains
> > > > what the code does and asserts that it is the right thing to do, it
> > > > fails to explain why.
> > > 
> > > you also fail to explain it causes a regressions with Dalmore.
> > 
> > I thought my explanation below was sufficient, but maybe I should say it
> > in other words: __of_usb_find_phy() returns -ENODEV if no PHY was found
> > to be registered for a given phandle. That causes the driver to abort
> > probing with a -ENODEV error and does not trigger the probe deferral
> > that'd be necessary to get the host controller to find the PHY the next
> > time it was triggered.
> 
> right, and before $subject dev_usb_get_phy_by_phandle() was overwriting
> whatever error code passed by __of_usb_find_phy() to -EPROBE_DEFER.
> 
> > > This is really the correct patch, we shouldn't be overwritting the
> > > error passed in by upper layers.
> > 
> > No, it's very obviously not the correct patch if it causes a regression.
> 
> or it exposes a bug elsewhere :-)

still, if you send your patch as a proper patch, I'll queue it as it
definitely makes sense to not return -ENODEV when we have a phandle.

-- 
balbi

Attachment: signature.asc
Description: Digital signature