Re: [PATCH 1/4] Documentation: tps65086: Add DT bindings for the TPS65086 PMIC

[Lee Jones <lee.jones@xxxxxxxxxx>]

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@xxxxxx>
> >>>>>>---
> >>>>>>  .../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-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html