Hello Again Eric,

On Tue, Jun 26, 2018 at 01:40:40PM +0200, Enric Balletbo Serra wrote:
> Hi Matti,
> Missatge de Matti Vaittinen <[email protected]> del
> dia dt., 26 de juny 2018 a les 13:25:
> > On Tue, Jun 26, 2018 at 11:06:33AM +0200, Enric Balletbo Serra wrote:
> > > Missatge de Matti Vaittinen <[email protected]> del
> > > dia dt., 19 de juny 2018 a les 12:57:
> >
> > > > +static const struct of_device_id bd71837_of_match[] = {
> > > > +       { .compatible = "rohm,bd71837", .data = (void *)0},
> > > > +       { },
> > >
> > > nit: { /* sentinel */ }
> >
> > I am sorry but I didn't quite get the point here. Could you please
> > explain what did you expect to be added here?
> >
> 
> It's a nit but It is a good practice to specify that the last entry is
> a sentinel. Just this.
> 
> +static const struct of_device_id bd71837_of_match[] = {
> +       { .compatible = "rohm,bd71837", .data = (void *)0},
> +       { /* sentinel */ }
> +};

Oh, I see. Finally something I can disagree =) I quickly opened few
random drivers which declare match table. None of them practiced this
good practice. So I guess it is not such a standard after all. And I
guess the meaning of last entry in match table should be quite obvious.
Adding the comment /* sentinel */ sounds like stating the obvious at
best - at worst it gets one just to wonder what the "sentinel" means =)

> 
> And just noticed, is .data = (void *)0 really required?
 
As static structs should be initialized to zero I'd say it is not
required. Will remove this. Thanks for pointing this out.

Br,
        Matti Vaittinen

Reply via email to