| Message ID | 1390836466-16007-1-git-send-email-balbi@ti.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Mon, Jan 27, 2014 at 11:44:16PM +0300, Sergei Shtylyov wrote: > >diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c > >index e6f61e4..db18011 100644 > >--- a/drivers/usb/phy/phy.c > >+++ b/drivers/usb/phy/phy.c > >@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type) > > > > phy = __usb_find_phy(&phy_list, type); > > if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) { > >- pr_err("unable to find transceiver of type %s\n", > >+ dev_dbg(phy->dev, "unable to find transceiver of type %s\n", > > 'phy' is possibly invalid (error ptr) at this point, you cannot > dereference it. right, this one can't be a dev_dbg(), unfortunately. > >@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index) > > > > phy = __usb_find_phy_dev(dev, &phy_bind_list, index); > > if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) { > >- pr_err("unable to find transceiver\n"); > >+ dev_dbg(dev, "unable to find transceiver\n"); > > Same here. here he's using the caller's dev pointer, which is just fine.
On Mon, Jan 27, 2014 at 02:46:40PM -0500, Josh Boyer wrote: > On Mon, Jan 27, 2014 at 3:44 PM, Sergei Shtylyov > <sergei.shtylyov@cogentembedded.com> wrote: > > Hello. > > > > > > On 01/27/2014 10:23 PM, Josh Boyer wrote: > > > >> the PHY layer is supposed to be optional, > >> considering some PHY have no control bus > >> for SW to poke around. > > > > > >> After commit 1ae5799 (usb: hcd: Initialize > >> USB phy if needed) any HCD which didn't provide > >> a PHY driver would emit annoying error messages. > > > > > >> In this patch we're decreasing those messages > >> to dev_dbg for debugging only and so we know where > >> they're coming from. > > > > > >> Reported-by: Josh Boyer <jwboyer@fedoraproject.org> > >> Signed-off-by: Felipe Balbi <balbi@ti.com> > >> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > >> --- > > > > > >> v2: Switch to using dev_dbg > > > > > >> drivers/usb/phy/phy.c | 8 +++----- > >> 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c > >> index e6f61e4..db18011 100644 > >> --- a/drivers/usb/phy/phy.c > >> +++ b/drivers/usb/phy/phy.c > >> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type) > >> > >> phy = __usb_find_phy(&phy_list, type); > >> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) { > >> - pr_err("unable to find transceiver of type %s\n", > >> + dev_dbg(phy->dev, "unable to find transceiver of type > >> %s\n", > > > > > > 'phy' is possibly invalid (error ptr) at this point, you cannot > > dereference it. > > Oh, yes. Duh, I should have spotted that. > > Felipe, can we just go with your original patch? It avoids having to > worry about the dev parameter to dev_dbg. fine by me, no problem. Can I get a Tested-by or Reviewed-by on that ? (Tested-by gets more points heh)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c index e6f61e4..29840c2 100644 --- a/drivers/usb/phy/phy.c +++ b/drivers/usb/phy/phy.c @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type) phy = __usb_find_phy(&phy_list, type); if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) { - pr_err("unable to find transceiver of type %s\n", + pr_debug("PHY: unable to find transceiver of type %s\n", usb_phy_type_string(type)); goto err0; } @@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8 index) phy = __usb_find_phy_dev(dev, &phy_bind_list, index); if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) { - pr_err("unable to find transceiver\n"); + pr_debug("PHY: unable to find transceiver\n"); goto err0; } @@ -424,10 +424,8 @@ int usb_bind_phy(const char *dev_name, u8 index, unsigned long flags; phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL); - if (!phy_bind) { - pr_err("phy_bind(): No memory for phy_bind"); + if (!phy_bind) return -ENOMEM; - } phy_bind->dev_name = dev_name; phy_bind->phy_dev_name = phy_dev_name;
the PHY layer is supposed to be optional, considering some PHY have no control bus for SW to poke around. After commit 1ae5799 (usb: hcd: Initialize USB phy if needed) any HCD which didn't provide a PHY driver would emit annoying error messages. In this patch we're decreasing those messages to debugging only and we also add a PHY prefix so we know where they're coming from. Reported-by: Josh Boyer <jwboyer@fedoraproject.org> Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/phy/phy.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)