linux-kernel - Re: [PATCH v2 3/4] iio: potentiometer: mcp4531: Add device tree binding

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] [day] [month] [year] [list]

Message-ID: <8d501966-d2f3-7b0b-2d70-3f0a60802d99@kernel.org>
Date:	Sun, 3 Jul 2016 10:25:23 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Florian Vaussard <florian.vaussard@...g-vd.ch>,
	Peter Rosin <peda@...ntia.se>,
	Florian Vaussard <florian.vaussard@...il.com>,
	devicetree@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>
Cc:	Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Slawomir Stepien <sst@...zta.fm>,
	Joachim Eastwood <manabian@...il.com>,
	Matt Ranostay <mranostay@...il.com>,
	Cristina Moraru <cristina.moraru09@...il.com>,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] iio: potentiometer: mcp4531: Add device tree
 binding

On 27/06/16 06:30, Florian Vaussard wrote:
> Hi Peter,
> 
> Le 27. 06. 16 à 00:12, Peter Rosin a écrit :
>> Hi Florian,
>>
>> On 2016-06-26 22:22, Florian Vaussard wrote:
>>> This patch adds the necessary device tree binding to allow DT probing of
>>> currently supported parts.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@...g-vd.ch>
>>> ---
>>>  drivers/iio/potentiometer/mcp4531.c | 273 +++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 272 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
>>> index 2251173..bf7b853 100644
>>> --- a/drivers/iio/potentiometer/mcp4531.c
>>> +++ b/drivers/iio/potentiometer/mcp4531.c
>>> @@ -31,6 +31,8 @@
>>>  #include <linux/module.h>
>>>  #include <linux/i2c.h>
>>>  #include <linux/err.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>>  
>>>  #include <linux/iio/iio.h>
>>>  
>>> @@ -188,12 +190,275 @@ static const struct iio_info mcp4531_info = {
>>>  	.driver_module = THIS_MODULE,
>>>  };
>>>  
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id mcp4531_of_match[] = {
>>> +	{
>>> +		.compatible = "microchip,mcp4531-502",
>>> +		.data = &mcp4531_cfg[MCP453x_502]
>>> +	},
>>
>> All this vertical whitespace makes this unreadable. I'd be
>> happier with either ignoring the 80 char rule, or skipping
>> the leading tab. I.e.
>>
>> 	{ .compatible = "microchip,mcp4531-502", .data = &mcp4531_cfg[MCP453x_502] },
>> 	{ .compatible = "microchip,mcp4531-103", .data = &mcp4531_cfg[MCP453x_103] },
>> 	{ .compatible = "microchip,mcp4531-503", .data = &mcp4531_cfg[MCP453x_503] },
>> 	...
>>
>> or
>>
>> { .compatible = "microchip,mcp4531-502", .data = &mcp4531_cfg[MCP453x_502] },
>> { .compatible = "microchip,mcp4531-103", .data = &mcp4531_cfg[MCP453x_103] },
>> { .compatible = "microchip,mcp4531-503", .data = &mcp4531_cfg[MCP453x_503] },
>> ...
>>
>> Or perhaps using a macro?
>>
>> #define MCP4531_COMPATIBLE(of_compatible, cfg) {	\
>> 		.compatible = of_compatible,		\
>> 		.data = &mcp4531_cfg[cfg],		\
>> }
>>
>> and then
>>
>> 	MCP4531_COMPATIBLE("microchip,mcp4531-502", MCP453x_502),
>> 	MCP4531_COMPATIBLE("microchip,mcp4531-103", MCP453x_103),
>> 	MCP4531_COMPATIBLE("microchip,mcp4531-503", MCP453x_503),
>> 	...
>>
>> Pick any of those, and you have my ack. Maybe Jonathan has an opinion
>> on which is best?
>>
> 
> The macro is my preferred one, as it makes things easier to read. Jonathan?
I'm fine with any of the options (or indeed the original line break heavy
approach). Take your pick!

Jonathan
> 
> Thanks for the suggestion!
> 
> Best,
> Florian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>