diff mbox

[v2] usb: musb: musb_cppi41: Update an error message

Message ID 20170505125233.27015-1-abailon@baylibre.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandre Bailon May 5, 2017, 12:52 p.m. UTC
If dma_request_slave_channel() failed to return a channel,
then the driver will print an error and request to defer probe,
regardless of the cause of the failure.
Only defer if the DMA is not ready yet and print an error to explain
that we are deferring probe.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/usb/musb/musb_cppi41.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Bin Liu May 10, 2017, 7:45 p.m. UTC | #1
On Fri, May 05, 2017 at 07:52:33AM -0500, Alexandre Bailon wrote:
> If dma_request_slave_channel() failed to return a channel,
> then the driver will print an error and request to defer probe,
> regardless of the cause of the failure.
> Only defer if the DMA is not ready yet and print an error to explain
> that we are deferring probe.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Applied. Thanks.
-Bin.

> ---
>  drivers/usb/musb/musb_cppi41.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index e7c8b1b..4718d0d 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -673,10 +673,12 @@ static int cppi41_dma_controller_start(struct cppi41_dma_controller *controller)
>  		musb_dma->status = MUSB_DMA_STATUS_FREE;
>  		musb_dma->max_len = SZ_4M;
>  
> -		dc = dma_request_slave_channel(dev->parent, str);
> -		if (!dc) {
> -			dev_err(dev, "Failed to request %s.\n", str);
> -			ret = -EPROBE_DEFER;
> +		dc = dma_request_chan(dev->parent, str);
> +		if (IS_ERR(dc)) {
> +			ret = PTR_ERR(dc);
> +			dev_err(dev, "Failed to request %s%s.\n", str,
> +				ret == -EPROBE_DEFER
> +				? ", deferring probe" : "");
>  			goto err;
>  		}
>  		cppi41_channel->dc = dc;
> -- 
> 2.10.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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
Johan Hovold May 11, 2017, 7:51 a.m. UTC | #2
On Wed, May 10, 2017 at 02:45:13PM -0500, Bin Liu wrote:
> On Fri, May 05, 2017 at 07:52:33AM -0500, Alexandre Bailon wrote:
> > If dma_request_slave_channel() failed to return a channel,
> > then the driver will print an error and request to defer probe,
> > regardless of the cause of the failure.
> > Only defer if the DMA is not ready yet and print an error to explain
> > that we are deferring probe.
> > 
> > Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> 
> Applied. Thanks.
> 
> > ---
> >  drivers/usb/musb/musb_cppi41.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> > index e7c8b1b..4718d0d 100644
> > --- a/drivers/usb/musb/musb_cppi41.c
> > +++ b/drivers/usb/musb/musb_cppi41.c
> > @@ -673,10 +673,12 @@ static int cppi41_dma_controller_start(struct cppi41_dma_controller *controller)
> >  		musb_dma->status = MUSB_DMA_STATUS_FREE;
> >  		musb_dma->max_len = SZ_4M;
> >  
> > -		dc = dma_request_slave_channel(dev->parent, str);
> > -		if (!dc) {
> > -			dev_err(dev, "Failed to request %s.\n", str);
> > -			ret = -EPROBE_DEFER;
> > +		dc = dma_request_chan(dev->parent, str);
> > +		if (IS_ERR(dc)) {
> > +			ret = PTR_ERR(dc);
> > +			dev_err(dev, "Failed to request %s%s.\n", str,
> > +				ret == -EPROBE_DEFER
> > +				? ", deferring probe" : "");

Probe deferral is not really an error, and we typically suppress any
error messages otherwise printed when a subsystem requests it to avoid
noise during boot. (There is a debug statement in driver core that
anyone interested can enable instead).

> >  			goto err;
> >  		}

So this should really be

	if (IS_ERR(DC)) {
		ret = PTR_ERR(dc);
		if (ret != -EPROBE_DEFER)
			dev_err(dev, ...);
		goto err;
	}

Johan
--
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
Bin Liu May 11, 2017, 2:06 p.m. UTC | #3
On Thu, May 11, 2017 at 09:51:11AM +0200, Johan Hovold wrote:
> On Wed, May 10, 2017 at 02:45:13PM -0500, Bin Liu wrote:
> > On Fri, May 05, 2017 at 07:52:33AM -0500, Alexandre Bailon wrote:
> > > If dma_request_slave_channel() failed to return a channel,
> > > then the driver will print an error and request to defer probe,
> > > regardless of the cause of the failure.
> > > Only defer if the DMA is not ready yet and print an error to explain
> > > that we are deferring probe.
> > > 
> > > Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> > 
> > Applied. Thanks.
> > 
> > > ---
> > >  drivers/usb/musb/musb_cppi41.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> > > index e7c8b1b..4718d0d 100644
> > > --- a/drivers/usb/musb/musb_cppi41.c
> > > +++ b/drivers/usb/musb/musb_cppi41.c
> > > @@ -673,10 +673,12 @@ static int cppi41_dma_controller_start(struct cppi41_dma_controller *controller)
> > >  		musb_dma->status = MUSB_DMA_STATUS_FREE;
> > >  		musb_dma->max_len = SZ_4M;
> > >  
> > > -		dc = dma_request_slave_channel(dev->parent, str);
> > > -		if (!dc) {
> > > -			dev_err(dev, "Failed to request %s.\n", str);
> > > -			ret = -EPROBE_DEFER;
> > > +		dc = dma_request_chan(dev->parent, str);
> > > +		if (IS_ERR(dc)) {
> > > +			ret = PTR_ERR(dc);
> > > +			dev_err(dev, "Failed to request %s%s.\n", str,
> > > +				ret == -EPROBE_DEFER
> > > +				? ", deferring probe" : "");
> 
> Probe deferral is not really an error, and we typically suppress any
> error messages otherwise printed when a subsystem requests it to avoid
> noise during boot. (There is a debug statement in driver core that

Valid point, the deferral log shouldn't use the error level.
So dropped this patch from my tree.

> anyone interested can enable instead).
> 
> > >  			goto err;
> > >  		}
> 
> So this should really be
> 
> 	if (IS_ERR(DC)) {
> 		ret = PTR_ERR(dc);
> 		if (ret != -EPROBE_DEFER)
> 			dev_err(dev, ...);
> 		goto err;
> 	}

Regards,
-Bin.
--
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/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index e7c8b1b..4718d0d 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -673,10 +673,12 @@  static int cppi41_dma_controller_start(struct cppi41_dma_controller *controller)
 		musb_dma->status = MUSB_DMA_STATUS_FREE;
 		musb_dma->max_len = SZ_4M;
 
-		dc = dma_request_slave_channel(dev->parent, str);
-		if (!dc) {
-			dev_err(dev, "Failed to request %s.\n", str);
-			ret = -EPROBE_DEFER;
+		dc = dma_request_chan(dev->parent, str);
+		if (IS_ERR(dc)) {
+			ret = PTR_ERR(dc);
+			dev_err(dev, "Failed to request %s%s.\n", str,
+				ret == -EPROBE_DEFER
+				? ", deferring probe" : "");
 			goto err;
 		}
 		cppi41_channel->dc = dc;