linux-kernel - Re: [PATCH v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 08 May 2016 20:27:24 +0200
From:	Christian Lamparter <chunkeey@...glemail.com>
To:	Joachim Eastwood <manabian@...il.com>
Cc:	linux-gpio@...r.kernel.org,
	devicetree <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Álvaro Fernández Rojas <noltari@...il.com>,
	Kumar Gala <galak@...eaurora.org>,
	Alexander Shiyan <shc_work@...l.ru>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Mark Rutland <mark.rutland@....com>,
	Pawel Moll <pawel.moll@....com>,
	Rob Herring <robh+dt@...nel.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Andy Shevchenko <andy.shevchenko@...il.com>
Subject: Re: [PATCH v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

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