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
