Re: [PATCH 1/2] regulator: Add brcm,bcm63xx-regulator device tree binding

From: Simon Arlott <simon@fire.lp0.eu>
To: Mark Brown <broonie@kernel.org>
Cc: devicetree@vger.kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	linux-kernel@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jonas Gorski <jogo@openwrt.org>
Subject: Re: [PATCH 1/2] regulator: Add brcm,bcm63xx-regulator device tree binding
Date: Thu, 3 Dec 2015 08:14:33 +0000	[thread overview]
Message-ID: <565FF9E9.1090503@simon.arlott.org.uk> (raw)
In-Reply-To: <20151203000631.GM1929@sirena.org.uk>

On 03/12/15 00:06, Mark Brown wrote:
> On Wed, Dec 02, 2015 at 08:26:36PM +0000, Simon Arlott wrote:
>> On 02/12/15 12:53, Mark Brown wrote:
> 
>> > This is the sort of thing you can pick up from the SoC compatible
>> > strings.  As things stand there is zero content in this driver that
>> > relates to this SoC.
> 
>> There's always going to be very little content in the driver that
>> relates to this SoC, given that a single bit flip enables/disables
>> power.
> 
>> All other device tree drivers allow a register address to be specified
>> for the device, how is an offset in the regmap any different?
> 
> It's not that it's an offset in regmap, it's that you're trying to make
> a device driver with literally *no* content that is specific to the
> actual device.  If you're making a driver for a specific device like

The device that only has a single register, I don't want to make it
specific to the device by hard coding a register address in the driver
that changes when the same device is used on different SoCs.

> this it should know at least something about how to control the device
> from the compatible string.  If you're making a generic driver it should
> not make reference to specific devices.

Will you accept a generic driver for a simple enable regulator device on
a regmap? What should its compatible string be?

> In general I would prefer to have a driver with a SoC specific
> compatible string and the data tables in the kernel, that way we keep
> the device trees stable and our ABI more robust.
> 
>> >> The mask is used as there's one bit per regulator in the register, but
>> >> there's more than one way to express this in the DT:
> 
>> > I wouldn't expect to see it in the device tree at all for a device
>> > specific driver.
> 
>> If there isn't an individual entry in DT for each regulator, how is it
>> supposed to work? There's no #regulator-cells property.
> 
> There could be one if it would help, but we do normally manage to do
> this without - look at how other regulator drivers work.

Several of them have a fixed list of supported regulator names in the
driver. The regulator names for this device are meaningless to the
driver because all of the regulators look the same. They don't have a
known or controllable voltage and can only be turned on or off.

Any table mapping regulator names to bits in the register would be
different for each SoC making the list of regulator names in the device
tree redundant. If they're not listed in the device tree then I can't
use them as a phandle for other devices.

-- 
Simon Arlott