diff mbox

[v7,3/3] mfd: max14577: Add device tree bindings document

Message ID 1386329534-14186-4-git-send-email-k.kozlowski@samsung.com
State Superseded, archived
Headers show

Commit Message

Krzysztof Kozlowski Dec. 6, 2013, 11:32 a.m. UTC
Add document describing device tree bindings for MAX14577 MFD driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max14577.txt |   65 ++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt

Comments

Lee Jones Jan. 6, 2014, 10:46 a.m. UTC | #1
> Add document describing device tree bindings for MAX14577 MFD driver.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max14577.txt |   65 ++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt

You still need a DT Ack before I can apply this.
Kyungmin Park Jan. 6, 2014, 11:26 a.m. UTC | #2
On Mon, Jan 6, 2014 at 7:46 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> Add document describing device tree bindings for MAX14577 MFD driver.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/max14577.txt |   65 ++++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
>
> You still need a DT Ack before I can apply this.

Tomasz, can you ack it?

Thank you,
Kyungmin Park
--
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
Tomasz Figa Jan. 7, 2014, 2:10 p.m. UTC | #3
Hi,

On Friday 06 of December 2013 12:32:14 Krzysztof Kozlowski wrote:
> Add document describing device tree bindings for MAX14577 MFD driver.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max14577.txt |   65 ++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
> new file mode 100644
> index 000000000000..0b2449aaf8bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max14577.txt
> @@ -0,0 +1,65 @@
> +Maxim MAX14577 Multi-Function Device
> +
> +MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
> +Battery Charger and SFOUT LDO output for powering USB devices. It is
> +interfaced to host controller using I2C.
> +
> +
> +Required properties:
> +- compatible : Must be "maxim,max14577".
> +- reg : I2C slave address for the max14577 chip.

If this chip has a predefined set of addresses it might be a good idea to
list them here.

> +- interrupts : IRQ line for the max14577 chip.
> +- interrupt-parent :  The parent interrupt controller.
> +
> +
> +Optional nodes:
> +- max14577-muic :
> +	Node used only by extcon consumers.

Do you need a dedicated node to do this? If you don't need any additional
data for MUIC cell, why couldn't the parent node be used to register the
extcon provider?

> +	Required properties:
> +		- compatible : "maxim,max14577-muic"
> +
> +- regulators :
> +	Required properties:
> +		- compatible : "maxim,max14577-regulator"
> +
> +	May contain a sub-node per regulator from the list below. Each

Is "May" the correct word? Wouldn't it be better to always have
configuration specified for all regulators of the cell?

> +	sub-node should contain the constraints and initialization information
> +	for that regulator. See regulator.txt for a description of standard
> +	properties for these sub-nodes.
> +
> +	List of valid regulator names: CHARGER, SAFEOUT.
> +
> +	The SAFEOUT is a fixed voltage regulator so there is no need to specify
> +	voltages for it.
> +
> +
> +Example:
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +max14577@25 {
> +	compatible = "maxim,max14577";
> +	reg = <0x25>;
> +	interrupt-parent = <&gpx1>;
> +	interrupts = <5 IRQ_TYPE_NONE>;
> +
> +	muic: max14577-muic {
> +		compatible = "maxim,max14577-muic";
> +	};
> +
> +	regulators {
> +		compatible = "maxim,max14577-regulator";
> +
> +		safeout_reg: regulator@1 {
> +			regulator-compatible = "SAFEOUT";
> +			regulator-name = "SAFEOUT";
> +		};
> +		charger_reg: regulator@0 {
> +			regulator-compatible = "CHARGER";
> +			regulator-name = "CHARGER";
> +			regulator-min-microamp = <90000>;
> +			regulator-max-microamp = <950000>;
> +			regulator-boot-on;
> +		};
> +	};
> +};
> 

Now this patch creates a question whether we should keep the existing
black-box MFD scheme, where the list of cells is determined by an array
hardcoded inside the driver or rather we should completely move to DT
based description where of_mfd_populate() could create all MFD cells
using description from DT.

Best regards,
Tomasz
--
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
Mark Brown Jan. 7, 2014, 2:35 p.m. UTC | #4
On Tue, Jan 07, 2014 at 03:10:04PM +0100, Tomasz Figa wrote:
> On Friday 06 of December 2013 12:32:14 Krzysztof Kozlowski wrote:

> > +- regulators :
> > +	Required properties:
> > +		- compatible : "maxim,max14577-regulator"

> > +	May contain a sub-node per regulator from the list below. Each

> Is "May" the correct word? Wouldn't it be better to always have
> configuration specified for all regulators of the cell?

It doesn't achieve anything to force people to include unused regulators
in the DT - the node is only needed if the kernel needs to do something
with the regulator.  This means that all the individual regulators end
up being optional.  The set of 

> Now this patch creates a question whether we should keep the existing
> black-box MFD scheme, where the list of cells is determined by an array
> hardcoded inside the driver or rather we should completely move to DT
> based description where of_mfd_populate() could create all MFD cells
> using description from DT.

This has been discussed before.  The biggest issue is that the Linux
idea of what cells it wants depends very much on whatever the current
set of subsystems Linux has and the way we divide the hardware between
them.  That's not a static thing, we're seeing flux with things like
extcon and clk at the minute for example.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 000000000000..0b2449aaf8bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,65 @@ 
+Maxim MAX14577 Multi-Function Device
+
+MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+
+Required properties:
+- compatible : Must be "maxim,max14577".
+- reg : I2C slave address for the max14577 chip.
+- interrupts : IRQ line for the max14577 chip.
+- interrupt-parent :  The parent interrupt controller.
+
+
+Optional nodes:
+- max14577-muic :
+	Node used only by extcon consumers.
+	Required properties:
+		- compatible : "maxim,max14577-muic"
+
+- regulators :
+	Required properties:
+		- compatible : "maxim,max14577-regulator"
+
+	May contain a sub-node per regulator from the list below. Each
+	sub-node should contain the constraints and initialization information
+	for that regulator. See regulator.txt for a description of standard
+	properties for these sub-nodes.
+
+	List of valid regulator names: CHARGER, SAFEOUT.
+
+	The SAFEOUT is a fixed voltage regulator so there is no need to specify
+	voltages for it.
+
+
+Example:
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+max14577@25 {
+	compatible = "maxim,max14577";
+	reg = <0x25>;
+	interrupt-parent = <&gpx1>;
+	interrupts = <5 IRQ_TYPE_NONE>;
+
+	muic: max14577-muic {
+		compatible = "maxim,max14577-muic";
+	};
+
+	regulators {
+		compatible = "maxim,max14577-regulator";
+
+		safeout_reg: regulator@1 {
+			regulator-compatible = "SAFEOUT";
+			regulator-name = "SAFEOUT";
+		};
+		charger_reg: regulator@0 {
+			regulator-compatible = "CHARGER";
+			regulator-name = "CHARGER";
+			regulator-min-microamp = <90000>;
+			regulator-max-microamp = <950000>;
+			regulator-boot-on;
+		};
+	};
+};