lkml.org 
[lkml]   [2016]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs
Date
On Sunday, May 08, 2016 07:17:13 PM Joachim Eastwood wrote:
> > +#define ADD(_name, _func) { .compatible = _name, .data = _func }
>
> I don't see the point in having a macro for such a simple data
> structure, but since this v8 and Linus hasn't complained I guess it's
> fine.
>
> Using a macro here makes it impossible to grep for 'compatible'. Doing
> 'git grep compatible drivers/gpio/' is sometimes very useful to see
> which hardware the driver actually supports.

Ok. I'll definitely picking it up. I'll wait until Tuesday/Wednesday
for more comments and then release a new series.

> > +static const struct of_device_id bgpio_of_match[] = {
> > + ADD("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
> > + { }
> > +};
> > +#undef ADD
> > +MODULE_DEVICE_TABLE(of, bgpio_of_match);
> > +
> > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> > + unsigned long *flags)
> > +{
[...]
> > + of_id = of_match_node(bgpio_of_match, node);
> > + if (!of_id)
> > + return NULL;
[...]
> You can retrieve OF match data using of_device_get_match_data().
> Saves you a couple of lines and better explains what your doing.
Yes thanks that's useful too since I don't need the of_id variable
anymore.

Both improvements save a total of 8 lines. so it's 328 insertions(+)
vs 380 deletions(-).

Regards,
Christian

\
 
 \ /
  Last update: 2016-05-08 20:41    [W:0.091 / U:1.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site