diff mbox

[3/3] mfd: max77686: Split out regulator part from the DT binding

Message ID 1436773348-15316-4-git-send-email-javier@osg.samsung.com
State Superseded, archived
Headers show

Commit Message

Javier Martinez Canillas July 13, 2015, 7:42 a.m. UTC
The Maxim MAX77686 PMIC is a multi-function device with regulators,
clocks and a RTC. The DT bindings for the clocks are in a separate
file but the bindings for the regulators are inside the mfd part.

To make it consistent with the clocks portion of the binding and
because is more natural to look for regulator bindings under the
bindings/regulator sub-directory, split the regulator portion of
the DT binding and add it as a separate file.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 Documentation/devicetree/bindings/mfd/max77686.txt | 58 +-----------------
 .../devicetree/bindings/regulator/max77686.txt     | 71 ++++++++++++++++++++++
 2 files changed, 74 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/max77686.txt

Comments

Krzysztof Kozlowski July 13, 2015, 8:03 a.m. UTC | #1
On 13.07.2015 16:42, Javier Martinez Canillas wrote:
> The Maxim MAX77686 PMIC is a multi-function device with regulators,
> clocks and a RTC. The DT bindings for the clocks are in a separate
> file but the bindings for the regulators are inside the mfd part.
> 
> To make it consistent with the clocks portion of the binding and
> because is more natural to look for regulator bindings under the
> bindings/regulator sub-directory, split the regulator portion of
> the DT binding and add it as a separate file.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  Documentation/devicetree/bindings/mfd/max77686.txt | 58 +-----------------
>  .../devicetree/bindings/regulator/max77686.txt     | 71 ++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 55 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/max77686.txt

Actually I would prefer the opposite - merging everything into one file
(clocks, regulators -> mfd) because:

1. Separate files introduce some duplication (like introduction and
common part of example node).

2. It is easier to track the changes and update them. For example when
adding a new chipset to the driver one may forgot about updating other
files. When moving files one may forgot to update hard-coded path in
some other file.

3. When comparing existing DTS with documentation or when creating new
DTS for the device it is just faster to fetch everything (knowledge,
example node) from one file.

However I understand that such opinion may be not suited for the idea of
MFD...

Best regards,
Krzysztof

> 
> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> index 8221102d3fc2..8a276dd5ea46 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -8,7 +8,8 @@ client while probing.This document describes the binding for mfd device and
>  PMIC submodule.
>  
>  Binding for the built-in 32k clock generator block is defined separately
> -in bindings/clk/maxim,max77686.txt file.
> +in the bindings/clk/maxim,max77686.txt file and binding for the regulators
> +is defined in the bindings/regulator/max77686.txt file.
>  
>  Required properties:
>  - compatible : Must be "maxim,max77686";
> @@ -16,36 +17,6 @@ Required properties:
>  - interrupts : This i2c device has an IRQ line connected to the main SoC.
>  - interrupt-parent : The parent interrupt controller.
>  
> -Optional node:
> -- voltage-regulators : The regulators of max77686 have to be instantiated
> -  under subnode named "voltage-regulators" using the following format.
> -
> -	regulator_name {
> -		regulator-compatible = LDOn/BUCKn
> -		standard regulator constraints....
> -	};
> -	refer Documentation/devicetree/bindings/regulator/regulator.txt
> -
> -  The regulator node's name should be initialized with a string
> -to get matched with their hardware counterparts as follow:
> -
> -	-LDOn 	:	for LDOs, where n can lie in range 1 to 26.
> -		 	example: LDO1, LDO2, LDO26.
> -	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 9.
> -			example: BUCK1, BUCK5, BUCK9.
> -
> -  Regulators which can be turned off during system suspend:
> -	-LDOn	:	2, 6-8, 10-12, 14-16,
> -	-BUCKn	:	1-4.
> -  Use standard regulator bindings for it ('regulator-off-in-suspend').
> -
> -  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
> -  control. To turn this feature on this property must be added to the regulator
> -  sub-node:
> -	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
> -				flags are actually ignored and always
> -				ACTIVE_HIGH is used)
> -
>  Example:
>  
>  	max77686@09 {
> @@ -53,27 +24,4 @@ Example:
>  		interrupt-parent = <&wakeup_eint>;
>  		interrupts = <26 0>;
>  		reg = <0x09>;
> -
> -		voltage-regulators {
> -			ldo11_reg: LDO11 {
> -				regulator-name = "vdd_ldo11";
> -				regulator-min-microvolt = <1900000>;
> -				regulator-max-microvolt = <1900000>;
> -				regulator-always-on;
> -			};
> -
> -			buck1_reg: BUCK1 {
> -				regulator-name = "vdd_mif";
> -				regulator-min-microvolt = <950000>;
> -				regulator-max-microvolt = <1300000>;
> -				regulator-always-on;
> -				regulator-boot-on;
> -			};
> -
> -			buck9_reg: BUCK9 {
> -				regulator-name = "CAM_ISP_CORE_1.2V";
> -				regulator-min-microvolt = <1000000>;
> -				regulator-max-microvolt = <1200000>;
> -				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
> -			};
> -	}
> +	};
> diff --git a/Documentation/devicetree/bindings/regulator/max77686.txt b/Documentation/devicetree/bindings/regulator/max77686.txt
> new file mode 100644
> index 000000000000..04b7604c219e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/max77686.txt
> @@ -0,0 +1,71 @@
> +Binding for Maxim MAX77686 regulators
> +
> +This is a part of the device tree bindings of MAX77686 multi-function device.
> +More information can be found in bindings/mfd/max77686.txt file.
> +
> +The MAX77802 PMIC has 9 high-efficiency Buck and 26 Low-dropout (LDO)
> +regulators that can be controlled over I2C.
> +
> +Following properties should be present in main device node of the MFD chip.
> +
> +Optional node:
> +- voltage-regulators : The regulators of max77686 have to be instantiated
> +  under subnode named "voltage-regulators" using the following format.
> +
> +	regulator_name {
> +		regulator-compatible = LDOn/BUCKn
> +		standard regulator constraints....
> +	};
> +	refer Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +  The regulator node's name should be initialized with a string
> +to get matched with their hardware counterparts as follow:
> +
> +	-LDOn 	:	for LDOs, where n can lie in range 1 to 26.
> +			example: LDO1, LDO2, LDO26.
> +	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 9.
> +			example: BUCK1, BUCK5, BUCK9.
> +
> +  Regulators which can be turned off during system suspend:
> +	-LDOn	:	2, 6-8, 10-12, 14-16,
> +	-BUCKn	:	1-4.
> +  Use standard regulator bindings for it ('regulator-off-in-suspend').
> +
> +  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
> +  control. To turn this feature on this property must be added to the regulator
> +  sub-node:
> +	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
> +				flags are actually ignored and always
> +				ACTIVE_HIGH is used)
> +
> +Example:
> +
> +	max77686@09 {
> +		compatible = "maxim,max77686";
> +		interrupt-parent = <&wakeup_eint>;
> +		interrupts = <26 0>;
> +		reg = <0x09>;
> +
> +		voltage-regulators {
> +			ldo11_reg: LDO11 {
> +				regulator-name = "vdd_ldo11";
> +				regulator-min-microvolt = <1900000>;
> +				regulator-max-microvolt = <1900000>;
> +				regulator-always-on;
> +			};
> +
> +			buck1_reg: BUCK1 {
> +				regulator-name = "vdd_mif";
> +				regulator-min-microvolt = <950000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +
> +			buck9_reg: BUCK9 {
> +				regulator-name = "CAM_ISP_CORE_1.2V";
> +				regulator-min-microvolt = <1000000>;
> +				regulator-max-microvolt = <1200000>;
> +				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
> +			};
> +	};
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas July 13, 2015, 8:20 a.m. UTC | #2
Hello Krzysztof,

On 07/13/2015 10:03 AM, Krzysztof Kozlowski wrote:
> On 13.07.2015 16:42, Javier Martinez Canillas wrote:
>> The Maxim MAX77686 PMIC is a multi-function device with regulators,
>> clocks and a RTC. The DT bindings for the clocks are in a separate
>> file but the bindings for the regulators are inside the mfd part.
>>
>> To make it consistent with the clocks portion of the binding and
>> because is more natural to look for regulator bindings under the
>> bindings/regulator sub-directory, split the regulator portion of
>> the DT binding and add it as a separate file.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>>  Documentation/devicetree/bindings/mfd/max77686.txt | 58 +-----------------
>>  .../devicetree/bindings/regulator/max77686.txt     | 71 ++++++++++++++++++++++
>>  2 files changed, 74 insertions(+), 55 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/regulator/max77686.txt
> 
> Actually I would prefer the opposite - merging everything into one file
> (clocks, regulators -> mfd) because:
> 
> 1. Separate files introduce some duplication (like introduction and
> common part of example node).
> 
> 2. It is easier to track the changes and update them. For example when
> adding a new chipset to the driver one may forgot about updating other
> files. When moving files one may forgot to update hard-coded path in
> some other file.
> 
> 3. When comparing existing DTS with documentation or when creating new
> DTS for the device it is just faster to fetch everything (knowledge,
> example node) from one file.
>

Yes, I also wondered about going the opposite way but then thought
that is more natural to look for clock bindings under bindings/clock
and for regulators drivers under bindings/regulator...
 
> However I understand that such opinion may be not suited for the idea of
> MFD...
>

...but I don't have a strong opinion and can do the other way if folks
are more fond with that single file approach.

What I think is that it should be consistent, either everything is in mfd
for all PMICs or everything is split.

The max77686 for example has a somehow arbitrary split since the clocks
are in another file but the regulators are in the same mfd binding doc.

> Best regards,
> Krzysztof
>

Best regards,
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
index 8221102d3fc2..8a276dd5ea46 100644
--- a/Documentation/devicetree/bindings/mfd/max77686.txt
+++ b/Documentation/devicetree/bindings/mfd/max77686.txt
@@ -8,7 +8,8 @@  client while probing.This document describes the binding for mfd device and
 PMIC submodule.
 
 Binding for the built-in 32k clock generator block is defined separately
-in bindings/clk/maxim,max77686.txt file.
+in the bindings/clk/maxim,max77686.txt file and binding for the regulators
+is defined in the bindings/regulator/max77686.txt file.
 
 Required properties:
 - compatible : Must be "maxim,max77686";
@@ -16,36 +17,6 @@  Required properties:
 - interrupts : This i2c device has an IRQ line connected to the main SoC.
 - interrupt-parent : The parent interrupt controller.
 
-Optional node:
-- voltage-regulators : The regulators of max77686 have to be instantiated
-  under subnode named "voltage-regulators" using the following format.
-
-	regulator_name {
-		regulator-compatible = LDOn/BUCKn
-		standard regulator constraints....
-	};
-	refer Documentation/devicetree/bindings/regulator/regulator.txt
-
-  The regulator node's name should be initialized with a string
-to get matched with their hardware counterparts as follow:
-
-	-LDOn 	:	for LDOs, where n can lie in range 1 to 26.
-		 	example: LDO1, LDO2, LDO26.
-	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 9.
-			example: BUCK1, BUCK5, BUCK9.
-
-  Regulators which can be turned off during system suspend:
-	-LDOn	:	2, 6-8, 10-12, 14-16,
-	-BUCKn	:	1-4.
-  Use standard regulator bindings for it ('regulator-off-in-suspend').
-
-  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
-  control. To turn this feature on this property must be added to the regulator
-  sub-node:
-	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
-				flags are actually ignored and always
-				ACTIVE_HIGH is used)
-
 Example:
 
 	max77686@09 {
@@ -53,27 +24,4 @@  Example:
 		interrupt-parent = <&wakeup_eint>;
 		interrupts = <26 0>;
 		reg = <0x09>;
-
-		voltage-regulators {
-			ldo11_reg: LDO11 {
-				regulator-name = "vdd_ldo11";
-				regulator-min-microvolt = <1900000>;
-				regulator-max-microvolt = <1900000>;
-				regulator-always-on;
-			};
-
-			buck1_reg: BUCK1 {
-				regulator-name = "vdd_mif";
-				regulator-min-microvolt = <950000>;
-				regulator-max-microvolt = <1300000>;
-				regulator-always-on;
-				regulator-boot-on;
-			};
-
-			buck9_reg: BUCK9 {
-				regulator-name = "CAM_ISP_CORE_1.2V";
-				regulator-min-microvolt = <1000000>;
-				regulator-max-microvolt = <1200000>;
-				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
-			};
-	}
+	};
diff --git a/Documentation/devicetree/bindings/regulator/max77686.txt b/Documentation/devicetree/bindings/regulator/max77686.txt
new file mode 100644
index 000000000000..04b7604c219e
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max77686.txt
@@ -0,0 +1,71 @@ 
+Binding for Maxim MAX77686 regulators
+
+This is a part of the device tree bindings of MAX77686 multi-function device.
+More information can be found in bindings/mfd/max77686.txt file.
+
+The MAX77802 PMIC has 9 high-efficiency Buck and 26 Low-dropout (LDO)
+regulators that can be controlled over I2C.
+
+Following properties should be present in main device node of the MFD chip.
+
+Optional node:
+- voltage-regulators : The regulators of max77686 have to be instantiated
+  under subnode named "voltage-regulators" using the following format.
+
+	regulator_name {
+		regulator-compatible = LDOn/BUCKn
+		standard regulator constraints....
+	};
+	refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+  The regulator node's name should be initialized with a string
+to get matched with their hardware counterparts as follow:
+
+	-LDOn 	:	for LDOs, where n can lie in range 1 to 26.
+			example: LDO1, LDO2, LDO26.
+	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 9.
+			example: BUCK1, BUCK5, BUCK9.
+
+  Regulators which can be turned off during system suspend:
+	-LDOn	:	2, 6-8, 10-12, 14-16,
+	-BUCKn	:	1-4.
+  Use standard regulator bindings for it ('regulator-off-in-suspend').
+
+  LDO20, LDO21, LDO22, BUCK8 and BUCK9 can be configured to GPIO enable
+  control. To turn this feature on this property must be added to the regulator
+  sub-node:
+	- maxim,ena-gpios :	one GPIO specifier enable control (the gpio
+				flags are actually ignored and always
+				ACTIVE_HIGH is used)
+
+Example:
+
+	max77686@09 {
+		compatible = "maxim,max77686";
+		interrupt-parent = <&wakeup_eint>;
+		interrupts = <26 0>;
+		reg = <0x09>;
+
+		voltage-regulators {
+			ldo11_reg: LDO11 {
+				regulator-name = "vdd_ldo11";
+				regulator-min-microvolt = <1900000>;
+				regulator-max-microvolt = <1900000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "vdd_mif";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+
+			buck9_reg: BUCK9 {
+				regulator-name = "CAM_ISP_CORE_1.2V";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1200000>;
+				maxim,ena-gpios = <&gpm0 3 GPIO_ACTIVE_HIGH>;
+			};
+	};