| 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
| ||
|
Message-ID: <56264990.8050501@ti.com>
Date: Tue, 20 Oct 2015 09:02:56 -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>,
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 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).
>>>>>> + - reg : Slave address.
>>>>>
>>>>> I2C/SPI?
>>>>>
>>>>
>>>> ACK
>>>>
>>>>>> + - interrupt-parent : The parent interrupt controller.
>>>>>
>>>>> Phandled to ...
>>>>>
>>>>
>>>> ACK
>>>>
>>>>>> + - interrupts : The interrupt line the device is connected to.
>>>>>> + - interrupt-controller : Marks the device node as an interrupt controller.
>>>>>> + - #interrupt-cells : The number of cells to describe an IRQ, this
>>>>>> + should be 2. The first cell is the IRQ number.
>>>>>> + The second cell is the flags, encoded as the trigger
>>>>>> + masks from ../interrupt-controller/interrupts.txt.
>>>>>
>>>>> Masks? What masks?
>>>>>
>>>>> Best to make a link to the header where the flags are defined here.
>>>>>
>>>>
>>>> ACK
>>>>
>>>>>> +Additional nodes defined in:
>>>>>> + - Regulators : ../regulator/tps65086-regulator.txt.
>>>>>> + - GPIO : ../gpio/gpio-tps65086.txt.
>>>>>
>>>>> I'd suggest removing the full stops from all of the lines above.
>>>>>
>>>>> Just treat them as bullet points like we normally do.
>>>>>
>>>>
>>>> ACK
>>>>
>>>>>> +Example:
>>>>>> +
>>>>>> + pmic: tps65086@5e {
>>>>>> + compatible = "ti,tps65086";
>>>>>> + reg = <0x5e>;
>>>>>> + interrupt-parent = <&gpio1>;
>>>>>> + interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
>>>>>> + interrupt-controller;
>>>>>> + #interrupt-cells = <2>;
>>>>>> +
>>>>>> + regulators {
>>>>>> + compatible = "ti,tps65086-regulator";
>>>>>> +
>>>>>> + buck1 {
>>>>>> + regulator-name = "vcc1";
>>>>>> + regulator-min-microvolt = <1600000>;
>>>>>> + regulator-max-microvolt = <1600000>;
>>>>>> + regulator-boot-on;
>>>>>> + ti,regulator-decay;
>>>>>> + ti,regulator-step-size-25mv;
>>>>>> + };
>>>>>> + };
>>>>>> +
>>>>>> + gpio4: tps65086_gpio {
>>>>>> + compatible = "ti,tps65086-gpio";
>>>>>> + gpio-controller;
>>>>>> + #gpio-cells = <2>;
>>>>>> + };
>>>>>> + };
>>>>>
>>>>> [...]
>>>>>
>>>>
>>>
>>
>
--
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/
Powered by blists - more mailing lists