linux-kernel - Re: [PATCH 1/4] Documentation: tps65086: Add DT bindings for the TPS65086 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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 21 Oct 2015 09:46:33 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	"Andrew F. Davis" <afd@...com>
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>,
	Alexandre Courbot <gnurou@...il.com>,
	Grygorii Strashko <grygorii.strashko@...com>,
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] Documentation: tps65086: Add DT bindings for the
 TPS65086 PMIC

On Tue, 20 Oct 2015, Andrew F. Davis wrote:

> On 10/20/2015 06:31 AM, Lee Jones wrote:
> >On Mon, 19 Oct 2015, Andrew F. Davis wrote:
> >
> >>On 10/19/2015 10:21 AM, Lee Jones wrote:
> >>>On Mon, 19 Oct 2015, Andrew F. Davis wrote:
> >>>
> >>>>On 10/19/2015 04:13 AM, Lee Jones wrote:
> >>>>>On Fri, 16 Oct 2015, Andrew F. Davis wrote:
> >>>>>
> >>>>>>The TPS65086 PMIC contains several regulators and a GPO controller.
> >>>>>>Add bindings for the TPS65086 PMIC.
> >>>>>>
> >>>>>>Signed-off-by: Andrew F. Davis <afd@...com>
> >>>>>>---
> >>>>>>  .../devicetree/bindings/gpio/gpio-tps65086.txt     | 17 ++++++++
> >>>>>>  Documentation/devicetree/bindings/mfd/tps65086.txt | 46 ++++++++++++++++++++++
> >>>>>>  .../bindings/regulator/tps65086-regulator.txt      | 36 +++++++++++++++++
> >>>>>
> >>>>>Please split these up into separate patches.
> >>>>>
> >>>>>There is no functional reason to bundle them up.
> >>>>>
> >>>>
> >>>>ACK
> >>>>
> >>>>>>  3 files changed, 99 insertions(+)
> >>>>>>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65086.txt
> >>>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt
> >>>>>>  create mode 100644 Documentation/devicetree/bindings/regulator/tps65086-regulator.txt
> >>>>>
> >>>>>[...]
> >>>>>
> >>>>>>diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
> >>>>>>new file mode 100644
> >>>>>>index 0000000..4b6aeb4
> >>>>>>--- /dev/null
> >>>>>>+++ b/Documentation/devicetree/bindings/mfd/tps65086.txt
> >>>>>>@@ -0,0 +1,46 @@
> >>>>>>+* TPS65086 Power Management Integrated Circuit bindings
> >>>>>>+
> >>>>>>+Required properties:
> >>>>>>+ - compatible		: Should be "ti,tps65086".
> >>>>>
> >>>>>Any indication that it's a PMIC?
> >>>>>
> >>>>
> >>>>In the compatible string?
> >>>
> >>>Ya.
> >>>
> >>
> >>Not sure what you mean then?, no one else seems to be doing that,
> >>"xx,xxxxxxx-pmic" is usually used for matching the regulator node,
> >>not the device itself.
> >
> >Either the driver is MFD is the PMIC or it's not.
> >
> >If it is, the compatible should reflect that, if isn't not then the
> >description in the header comment and the one above is not correct.
> >
> >IMO, 'pmic' should not be used in the regulator compatible strings, as
> >it's a general description of the overall device.  The regulators are
> >just a component of that device.
> >
> 
> I agree about not using compatible in regulator nodes (Mark Brown and I had
> a discussion on this topic), what I mean with "xxxxxxx-pmic" is for the MFD
> core to match sub-drivers (mfd_cells). For example drivers/mfd/tps80031.c,
> matches the regulator with "tps80031-pmic", the clock with "tps80031-clock",
> the charger with "tps80031-charger", etc..
> 
> The core device itself is just matched with "ti,tps65086" through the I2C bus
> matching.
> 
> I could change the core to be "tps65086-pmic", then call the regulator
> driver "tps65086-regulator" if this works for you, this seems to be the
> way new drivers name the regulator driver (max77843.c).

Yes, this is what I was thinking of.

ti,tps65086 is uninformative/undescriptive and tells me nothing.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/