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

[Geert Uytterhoeven <geert@linux-m68k.org>]

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