[PATCH] media: fdp1: Reduce FCP not found message level to debug

[PATCH] media: fdp1: Reduce FCP not found message level to debug

[Geert Uytterhoeven]

When support for the IPMMU is not enabled, the FDP driver may be
probe-deferred multiple times, causing several messages to be printed
like:

    rcar_fdp1 fe940000.fdp1: FCP not found (-517)
    rcar_fdp1 fe944000.fdp1: FCP not found (-517)

Fix this by reducing the message level to debug level, as is done in the
VSP1 driver.

Fixes: 4710b752e029f3f8 ("[media] v4l: Add Renesas R-Car FDP1 Driver")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
The alternative would be to add an explicit check for -EPROBE_DEFER.
---
 drivers/media/platform/rcar_fdp1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
index 43aae9b6bb20e3e8..c23ec127c2776f51 100644
--- a/drivers/media/platform/rcar_fdp1.c
+++ b/drivers/media/platform/rcar_fdp1.c
@@ -2306,7 +2306,7 @@ static int fdp1_probe(struct platform_device *pdev)
 		fdp1->fcp = rcar_fcp_get(fcp_node);
 		of_node_put(fcp_node);
 		if (IS_ERR(fdp1->fcp)) {
-			dev_err(&pdev->dev, "FCP not found (%ld)\n",
+			dev_dbg(&pdev->dev, "FCP not found (%ld)\n",
 				PTR_ERR(fdp1->fcp));
 			return PTR_ERR(fdp1->fcp);
 		}
-- 
2.17.1

Re: [PATCH] media: fdp1: Reduce FCP not found message level to debug

[Kieran Bingham]

Hi Geert,

On 09/07/2019 10:59, Geert Uytterhoeven wrote:
> When support for the IPMMU is not enabled, the FDP driver may be
> probe-deferred multiple times, causing several messages to be printed
> like:
> 
>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> 
> Fix this by reducing the message level to debug level, as is done in the
> VSP1 driver.

Does the lack of IPMMU prevent the FDP1 being loaded entirely?

If so is that a problem for us? (I thought we were able to run without
the IPMMU)


> Fixes: 4710b752e029f3f8 ("[media] v4l: Add Renesas R-Car FDP1 Driver")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> The alternative would be to add an explicit check for -EPROBE_DEFER.
> ---
>  drivers/media/platform/rcar_fdp1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c
> index 43aae9b6bb20e3e8..c23ec127c2776f51 100644
> --- a/drivers/media/platform/rcar_fdp1.c
> +++ b/drivers/media/platform/rcar_fdp1.c
> @@ -2306,7 +2306,7 @@ static int fdp1_probe(struct platform_device *pdev)
>  		fdp1->fcp = rcar_fcp_get(fcp_node);
>  		of_node_put(fcp_node);
>  		if (IS_ERR(fdp1->fcp)) {
> -			dev_err(&pdev->dev, "FCP not found (%ld)\n",
> +			dev_dbg(&pdev->dev, "FCP not found (%ld)\n",
>  				PTR_ERR(fdp1->fcp));

Should we be doing something differently here in the event of no IPMMU then?

Otherwise, if the IPMMU has become a hard dependency, should we add it
to the Kconfig dependencies?


>  			return PTR_ERR(fdp1->fcp);
>  		}
> 

--
Regards

Kieran

Re: [PATCH] media: fdp1: Reduce FCP not found message level to debug

[Geert Uytterhoeven]

Hi Kieran,

On Tue, Jul 9, 2019 at 12:07 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> On 09/07/2019 10:59, Geert Uytterhoeven wrote:
> > When support for the IPMMU is not enabled, the FDP driver may be
> > probe-deferred multiple times, causing several messages to be printed
> > like:
> >
> >     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> >
> > Fix this by reducing the message level to debug level, as is done in the
> > VSP1 driver.
>
> Does the lack of IPMMU prevent the FDP1 being loaded entirely?

No it doesn't.
If CONFIG_IPMMU_VMSA=n,

    rcar_fdp1 fe940000.fdp1: FCP not found (-517)
    rcar_fdp1 fe944000.fdp1: FCP not found (-517)
    rcar_fdp1 fe940000.fdp1: FCP not found (-517)
    rcar_fdp1 fe944000.fdp1: FCP not found (-517)
    rcar_fdp1 fe940000.fdp1: FCP not found (-517)
    rcar_fdp1 fe944000.fdp1: FCP not found (-517)
    rcar_fdp1 fe940000.fdp1: Device registered as /dev/video8
    rcar_fdp1 fe944000.fdp1: Device registered as /dev/video9

So the driver succeeds, eventually.

If CONFIG_IPMMU_VMSA=y, it succeeds sooner:

    rcar_fdp1 fe940000.fdp1: Device registered as /dev/video0
    rcar_fdp1 fe944000.fdp1: Device registered as /dev/video1

Always be prepared to handle -EPROBE_DEFER.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] media: fdp1: Reduce FCP not found message level to debug

[Kieran Bingham]

Hi Geert,

On 09/07/2019 11:13, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Tue, Jul 9, 2019 at 12:07 PM Kieran Bingham
> <kieran.bingham+renesas@ideasonboard.com> wrote:
>> On 09/07/2019 10:59, Geert Uytterhoeven wrote:
>>> When support for the IPMMU is not enabled, the FDP driver may be
>>> probe-deferred multiple times, causing several messages to be printed
>>> like:
>>>
>>>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
>>>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
>>>
>>> Fix this by reducing the message level to debug level, as is done in the
>>> VSP1 driver.
>>
>> Does the lack of IPMMU prevent the FDP1 being loaded entirely?
> 
> No it doesn't.
> If CONFIG_IPMMU_VMSA=n,
> 
>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
>     rcar_fdp1 fe940000.fdp1: Device registered as /dev/video8
>     rcar_fdp1 fe944000.fdp1: Device registered as /dev/video9
> 
> So the driver succeeds, eventually.
> 
> If CONFIG_IPMMU_VMSA=y, it succeeds sooner:
> 
>     rcar_fdp1 fe940000.fdp1: Device registered as /dev/video0
>     rcar_fdp1 fe944000.fdp1: Device registered as /dev/video1
> 
> Always be prepared to handle -EPROBE_DEFER.
> 
> Gr{oetje,eeting}s,

On the basis that the driver is not prevented from loading, then the
message does indeed become more of a debug print.

I wonder if it's better to print something different in the event of
EPROBE_DEFER vs another actual error preventing the loading of the
driver, but in either case if someone hits an issue they're likely to
start adding/enabling debug.

So, with that and a precedent for this change in VSP1, I'm happy here.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>




> 
>                         Geert
> 

Re: [PATCH] media: fdp1: Reduce FCP not found message level to debug

[Geert Uytterhoeven]

Hi Kieran,

On Tue, Jul 9, 2019 at 12:18 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> On 09/07/2019 11:13, Geert Uytterhoeven wrote:
> > On Tue, Jul 9, 2019 at 12:07 PM Kieran Bingham
> > <kieran.bingham+renesas@ideasonboard.com> wrote:
> >> On 09/07/2019 10:59, Geert Uytterhoeven wrote:
> >>> When support for the IPMMU is not enabled, the FDP driver may be
> >>> probe-deferred multiple times, causing several messages to be printed
> >>> like:
> >>>
> >>>     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
> >>>     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> >>>
> >>> Fix this by reducing the message level to debug level, as is done in the
> >>> VSP1 driver.
> >>
> >> Does the lack of IPMMU prevent the FDP1 being loaded entirely?
> >
> > No it doesn't.
> > If CONFIG_IPMMU_VMSA=n,
> >
> >     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe940000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe944000.fdp1: FCP not found (-517)
> >     rcar_fdp1 fe940000.fdp1: Device registered as /dev/video8
> >     rcar_fdp1 fe944000.fdp1: Device registered as /dev/video9
> >
> > So the driver succeeds, eventually.
> >
> > If CONFIG_IPMMU_VMSA=y, it succeeds sooner:
> >
> >     rcar_fdp1 fe940000.fdp1: Device registered as /dev/video0
> >     rcar_fdp1 fe944000.fdp1: Device registered as /dev/video1
> >
> > Always be prepared to handle -EPROBE_DEFER.
>
> On the basis that the driver is not prevented from loading, then the
> message does indeed become more of a debug print.
>
> I wonder if it's better to print something different in the event of
> EPROBE_DEFER vs another actual error preventing the loading of the
> driver, but in either case if someone hits an issue they're likely to
> start adding/enabling debug.

That's why some driver suppress printing errors for -EPROBE_DEFER.

> So, with that and a precedent for this change in VSP1, I'm happy here.
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds