Re: [PATCH 3/4] pinctrl: sh-pfc: Add check for empty pinmux groups/functions

[Simon Horman <horms@verge.net.au>]

On Fri, Apr 26, 2019 at 11:42:11AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Apr 26, 2019 at 11:35 AM Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Apr 25, 2019 at 11:55:41AM +0200, Geert Uytterhoeven wrote:
> > > The pinmux groups and functions arrays may contain two parts, to ease
> > > supporting SoCs that expose pin subsets of other related SoCs.  Both
> > > parts need to be declared with explicit sizes, which thus need to be
> > > updated when adding support for more groups and functions.
> > >
> > > If a size is too small, the compiler will detect this at build time
> > > ("excess elements in array initializer").
> > > If a size is too large, this may go undetected (for pin groups), lead to
> > > pin controller registration failures (for pin functions: "pinmux ops has
> > > no name for functionN"), or crash the optional run-time debug code (for
> > > pin groups).
> > >
> > > Extend the run-time debug code with checks to detect this, to help
> > > catching bugs early.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thank you!
> 
> > > --- a/drivers/pinctrl/sh-pfc/core.c
> > > +++ b/drivers/pinctrl/sh-pfc/core.c
> > > @@ -780,9 +780,15 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> > >
> > >       for (i = 0; i < info->nr_functions; i++) {
> > >               func = &info->functions[i];
> > > +             if (!func->name) {
> > > +                     pr_err("%s: empty function %u\n", drvname, i);
> > > +                     sh_pfc_errors++;
> >
> > Not strictly related to this patch but did
> > you consider not having sh_pfc_errors global to this file?
> 
> Do you have an alternative?
> I could hide it, and combine it with the pr_err() in a sh_pfc_report_error()
> function, but it would still be global.

I didn't look carefully for an alternative.
If its not practical to pass around a local variable then
I'm happy with the current setup.