diff mbox

[v3] usb: ehci-omap: don't complain on -EPROBE_DEFER when no PHY found

Message ID 20180118080453.GA2205@lenoch (mailing list archive)
State New, archived
Headers show

Commit Message

Ladislav Michl Jan. 18, 2018, 8:04 a.m. UTC
Don't complain on -EPROBE_DEFER when no PHY found, the driver
probe will be retried later.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Acked-by: Roger Quadros <rogerq@ti.com>
---
 Changes:
 - v2: None, just added Tony's ack
 - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack

 drivers/usb/host/ehci-omap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Alan Stern Jan. 18, 2018, 3:18 p.m. UTC | #1
On Thu, 18 Jan 2018, Ladislav Michl wrote:

> Don't complain on -EPROBE_DEFER when no PHY found, the driver
> probe will be retried later.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Acked-by: Roger Quadros <rogerq@ti.com>
> ---
>  Changes:
>  - v2: None, just added Tony's ack
>  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack

Although there's nothing wrong with making this change, people don't 
like to see character strings broken up just to avoid long lines.  Can 
you redo the patch without breaking the error message string?

Alan Stern

>  drivers/usb/host/ehci-omap.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index 854b146a457d..fd6170962a1a 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -167,8 +167,9 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>  				continue;
>  
>  			ret = PTR_ERR(phy);
> -			dev_err(dev, "Can't get PHY device for port %d: %d\n",
> -					i, ret);
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(dev, "Can't get PHY device for port "
> +					"%d: %d\n", i, ret);
>  			goto err_phy;
>  		}

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ladislav Michl Jan. 18, 2018, 3:26 p.m. UTC | #2
On Thu, Jan 18, 2018 at 10:18:41AM -0500, Alan Stern wrote:
> On Thu, 18 Jan 2018, Ladislav Michl wrote:
> 
> > Don't complain on -EPROBE_DEFER when no PHY found, the driver
> > probe will be retried later.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > Acked-by: Roger Quadros <rogerq@ti.com>
> > ---
> >  Changes:
> >  - v2: None, just added Tony's ack
> >  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack
> 
> Although there's nothing wrong with making this change, people don't 
> like to see character strings broken up just to avoid long lines.  Can 
> you redo the patch without breaking the error message string?

Sure I could, but I broke that long line intentionaly. Line lenght rule is
normally broken for string just to allow people grepping for error message in
source files and I guess everyone sane would grep for "Can't get PHY device for
port" which is on single line. So this way we:
1) respect 80 chars limit
2) have error message grepable.

Do you still insist on making this one long line? (I have no problem sending
v4, just wanted to give some rationale before I eventually do so)

Best regards,
	ladis

> >  drivers/usb/host/ehci-omap.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> > index 854b146a457d..fd6170962a1a 100644
> > --- a/drivers/usb/host/ehci-omap.c
> > +++ b/drivers/usb/host/ehci-omap.c
> > @@ -167,8 +167,9 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> >  				continue;
> >  
> >  			ret = PTR_ERR(phy);
> > -			dev_err(dev, "Can't get PHY device for port %d: %d\n",
> > -					i, ret);
> > +			if (ret != -EPROBE_DEFER)
> > +				dev_err(dev, "Can't get PHY device for port "
> > +					"%d: %d\n", i, ret);
> >  			goto err_phy;
> >  		}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Stern Jan. 18, 2018, 6:50 p.m. UTC | #3
On Thu, 18 Jan 2018, Ladislav Michl wrote:

> On Thu, Jan 18, 2018 at 10:18:41AM -0500, Alan Stern wrote:
> > On Thu, 18 Jan 2018, Ladislav Michl wrote:
> > 
> > > Don't complain on -EPROBE_DEFER when no PHY found, the driver
> > > probe will be retried later.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > > Acked-by: Tony Lindgren <tony@atomide.com>
> > > Acked-by: Roger Quadros <rogerq@ti.com>
> > > ---
> > >  Changes:
> > >  - v2: None, just added Tony's ack
> > >  - v3: Fixed commit message typo spotted by Jaejoong Kim, added Roger's ack
> > 
> > Although there's nothing wrong with making this change, people don't 
> > like to see character strings broken up just to avoid long lines.  Can 
> > you redo the patch without breaking the error message string?
> 
> Sure I could, but I broke that long line intentionaly. Line lenght rule is
> normally broken for string just to allow people grepping for error message in
> source files and I guess everyone sane would grep for "Can't get PHY device for
> port" which is on single line. So this way we:
> 1) respect 80 chars limit
> 2) have error message grepable.
> 
> Do you still insist on making this one long line? (I have no problem sending
> v4, just wanted to give some rationale before I eventually do so)

I don't have hard, set-in-stone views on this.  And I have broken quite 
a few printk strings in my time -- but that was before people reached a 
consensus on whether it should be done.

Even with the rationale, it's quite likely that somebody will complain
if we merge a patch that splits the string.  And it's good practice to
make sure that checkpatch.pl doesn't complain, in any case.

So yes, please do resubmit the patch with one long line.  When you do,
you may add

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 854b146a457d..fd6170962a1a 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -167,8 +167,9 @@  static int ehci_hcd_omap_probe(struct platform_device *pdev)
 				continue;
 
 			ret = PTR_ERR(phy);
-			dev_err(dev, "Can't get PHY device for port %d: %d\n",
-					i, ret);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Can't get PHY device for port "
+					"%d: %d\n", i, ret);
 			goto err_phy;
 		}