linux-kernel - Re: [PATCH 1/4] mfd: DT bindings for TPS65218 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:	Tue, 3 Dec 2013 16:22:21 +0530
From:	Keerthy <a0393675@...com>
To:	Lee Jones <lee.jones@...aro.org>
CC:	Keerthy <j-keerthy@...com>, <rob.herring@...xeda.com>,
	<pawel.moll@....com>, <mark.rutland@....com>,
	<swarren@...dotorg.org>, <ijc+devicetree@...lion.org.uk>,
	<rob@...dley.net>, <sameo@...ux.intel.com>,
	<grant.likely@...aro.org>, <lgirdwood@...il.com>,
	<broonie@...nel.org>, <devicetree@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>
Subject: Re: [PATCH 1/4] mfd: DT bindings for TPS65218 PMIC

On Tuesday 03 December 2013 03:06 PM, Lee Jones wrote:
>> Add DT bindings for TPS65218 PMIC.
>>
>> Signed-off-by: Keerthy <j-keerthy@...com>
>> ---
>>   Documentation/devicetree/bindings/mfd/tps65218.txt |   27 ++++++++++++++++++++
>>   .../devicetree/bindings/regulator/tps65218.txt     |   22 ++++++++++++++++
>>   2 files changed, 49 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/tps65218.txt
>>   create mode 100644 Documentation/devicetree/bindings/regulator/tps65218.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/tps65218.txt b/Documentation/devicetree/bindings/mfd/tps65218.txt
>> new file mode 100644
>> index 0000000..87cb7a8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/tps65218.txt
>> @@ -0,0 +1,27 @@
>> +The TPS65218 Integrated Power Management Chips.
>> +These chips are connected to an i2c bus.
>                                     I2C

Ok.

>> +Required properties:
>> +- compatible : Must be "ti,tps65218";
>> +- interrupts : This i2c device has an IRQ line connected to the main SoC
>                         I2C

Ok.

>> +- interrupt-controller : Since the tps65218 support several interrupts
>                                                 support(s) (hosts?)

Yes Supports multiple interrupts from the sub-modules.

>> +  internally, it is considered as an interrupt controller cascaded to the SoC.
>> +- #interrupt-cells = <2>;
>> +- interrupt-parent : The parent interrupt controller.
>             Phandle to ...
>
>> +Optional node:
>              node(s):

Ok.

>
>> +- Child nodes contain in the tps65218.
>> +  It supports a number of features.
> Please re-phase the above two sentences to something decipherable.

Ok.

>> +  The children nodes will thus depend of the capability of the variant.
> Please, go on ...

Ok.

>> +Example:
>> +/*
>> + * Integrated Power Management Chip
>> + */
>> +tps@24 {
>> +    compatible = "ti,tps65218";
>> +    reg = <0x24>;
>> +    interrupt-controller;
>> +    #interrupt-cells = <2>;
>> +    interrupt-parent = <&gic>;
>> +};
> Perhaps it would be better to centralise the documentation inclusive
> of the regulator contingent. Or at least provide a _full_ example in
> each document.

Ok.

>
>> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> new file mode 100644
>> index 0000000..1ccf170
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> @@ -0,0 +1,22 @@
>> +TPS65218 family of regulators
>> +
>> +Required properties:
>> +For tps65218 regulators/LDOs
>> +- compatible:
>> +  - "ti,tps65218-dcdc1" for DCDC1
>> +  - "ti,tps65218-dcdc2" for DCDC2
>> +  - "ti,tps65218-dcdc3" for DCDC3
>> +  - "ti,tps65218-dcdc4" for DCDC4
>> +  - "ti,tps65218-dcdc5" for DCDC5
>> +  - "ti,tps65218-dcdc6" for DCDC6
>> +  - "ti,tps65218-ldo1" for LDO1 LDO
> Why aren't you using 'regulator-compatible'?

I referred tps65217.txt the predecessors and used the compatible.
I also referred twl-regulator.txt.

>
>> +Optional properties:
>> +- Any optional property defined in bindings/regulator/regulator.txt
>> +
>> +Example:
>> +	xyz: regulator@0 {
> Genuine question: Is the @0 meaningful?

@0 is not the best example. @24 is what i use on the am43x.
I will change it to @24.

>> +		compatible = "ti,tps65218-dcdc1";
>> +		regulator-min-microvolt  = <1000000>;
>> +		regulator-max-microvolt  = <3000000>;
>> +	};

Thanks for reviewing.

Kind Regards,
Keerthy
--
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/