linux-kernel - Re: [PATCH v3 3/5] mfd: tps65912: Add driver for the TPS65912 PMIC

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 28 Sep 2015 10:43:12 -0500
From:	"Andrew F. Davis" <afd@...com>
To:	Lee Jones <lee.jones@...aro.org>
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>,
	Mark Brown <broonie@...nel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	<linux-gpio@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/5] mfd: tps65912: Add driver for the TPS65912 PMIC

On 09/25/2015 04:24 PM, Lee Jones wrote:
> On Fri, 25 Sep 2015, Andrew F. Davis wrote:
>
>> On 09/25/2015 11:50 AM, Lee Jones wrote:
>>> On Thu, 24 Sep 2015, Andrew F. Davis wrote:
>>>
>>>> This patch adds support for TPS65912 mfd device. It provides
>>>> communication through the I2C and SPI interfaces. It contains
>>>> the following components:
>>>>
>>>>   - Regulators
>>>>   - GPIO controller
>>>>
>>>> Signed-off-by: Andrew F. Davis <afd@...com>
>>>> ---
>>>>   drivers/mfd/Kconfig          |  24 +++
>>>>   drivers/mfd/Makefile         |   3 +
>>>>   drivers/mfd/tps65912-core.c  | 114 +++++++++++++
>>>>   drivers/mfd/tps65912-i2c.c   |  86 ++++++++++
>>>>   drivers/mfd/tps65912-spi.c   |  85 ++++++++++
>>>>   include/linux/mfd/tps65912.h | 393 +++++++++++++++++++++++++++++++++++++++++++
>>>>   6 files changed, 705 insertions(+)
>>>>   create mode 100644 drivers/mfd/tps65912-core.c
>>>>   create mode 100644 drivers/mfd/tps65912-i2c.c
>>>>   create mode 100644 drivers/mfd/tps65912-spi.c
>>>>   create mode 100644 include/linux/mfd/tps65912.h
>>>
>>> [...]
>>>
>>>> +#define TPS65912_IRQ(_name, _reg, _offset)			\
>>>> +	[TPS65912_IRQ_ ## _name] = {				\
>>>> +		.mask = TPS65912_ ## _reg ## _ ## _name,	\
>>>> +		.reg_offset = _offset,				\
>>>> +	}
>>>
>>> I told you about this already.
>>>
>>> If you want this set to be merged for v4.3 then you'll need commit
>>> b4fe8ba ("regmap: Add generic macro to define regmap_irq") from my
>>> tree.
>>
>> You asked me to submit this to Mark Brown, I didn't realize you also
>> wanted me to use the one in your tree. Using yours will make my lines
>> over 80 chars and so it kind of defeats the purpose but I'll do it
>> for v4.
>
> I asked two of you to upstream a generic incarnation of your
> implementation.  The other guy got there first.  Now I want everyone
> to use it instead of hand-rolling their own.
>

Got it.

>>> [...]
>>>
>>>> +static struct i2c_driver tps65912_i2c_driver = {
>>>> +	.driver		= {
>>>> +		.name	= "tps65912",
>>>> +		.of_match_table = tps65912_i2c_of_match_table,
>>>
>>> of_match_ptr()
>>>
>>
>> Why? tps65912_i2c_of_match_table is always compiled in.
>
> But it needn't be.
>

This driver is DT only, would it make more sense to just add a dependency
on CONFIG_OF in the Kconfig? I don't see many other drivers that are DT
only doing this, the driver should compile just fine without OF enabled,
it just wont have DT functionality, so is it really dependent on OF?
The of_match_ptr(x) macro looks to be useful when the x is conditionally
compiled in, but in my case the x (my of_device_id table) is always compiled.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/