| Message ID | 1386153615-25456-4-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Krzysztof, On 12/04/2013 07:40 PM, 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 | 48 ++++++++++++++++++++ > 1 file changed, 48 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..f4fd16360b6b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/max14577.txt > @@ -0,0 +1,48 @@ > +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: > +- regulators : > + Required child node properties: > + - compatible : "maxim,max14577-regulator" > + > + Optional child nodes: > + Each child node representing a regulator, following standard regulator > + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". > + The SAFEOUT is a constant voltage regulator so there is no need to specify > + voltages for it. > + > + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt > + > +Example: > + max14577@25 { > + compatible = "maxim,max14577"; > + reg = <0x25>; > + interrupt-parent = <&gpx1>; > + interrupts = <5 0>; > + > + regulators { > + compatible = "maxim,max14577-regulator"; > + > + safeout_reg: safeout@1 { > + regulator-compatible = "SAFEOUT"; > + regulator-name = "SAFEOUT"; > + }; > + charger_reg: charger@0 { > + regulator-compatible = "CHARGER"; > + regulator-name = "CHARGER"; > + regulator-min-microamp = <90000>; > + regulator-max-microamp = <950000>; > + regulator-boot-on; > + }; > + }; I prefer to add dt data about max14577-muic on following: If extcon consumer driver need to use muic device, dts should provide following dt data. muic: max14577-muic { compatible = "maxim, max14577-muic"; }; > + }; > For example, If fuel-gauge want to use extcon subsystem, dt data of fuel-gauge add 'extcon' property as following: fuel-gague@ { compatible = "..."; extcon = <&muic>; } Thanks, Chanwoo Choi
On Wed, 2013-12-04 at 19:50 +0900, Chanwoo Choi wrote: > Hi Krzysztof, > > On 12/04/2013 07:40 PM, 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 | 48 ++++++++++++++++++++ > > 1 file changed, 48 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..f4fd16360b6b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/max14577.txt > > @@ -0,0 +1,48 @@ > > +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: > > +- regulators : > > + Required child node properties: > > + - compatible : "maxim,max14577-regulator" > > + > > + Optional child nodes: > > + Each child node representing a regulator, following standard regulator > > + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". > > + The SAFEOUT is a constant voltage regulator so there is no need to specify > > + voltages for it. > > + > > + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt > > + > > +Example: > > + max14577@25 { > > + compatible = "maxim,max14577"; > > + reg = <0x25>; > > + interrupt-parent = <&gpx1>; > > + interrupts = <5 0>; > > + > > + regulators { > > + compatible = "maxim,max14577-regulator"; > > + > > + safeout_reg: safeout@1 { > > + regulator-compatible = "SAFEOUT"; > > + regulator-name = "SAFEOUT"; > > + }; > > + charger_reg: charger@0 { > > + regulator-compatible = "CHARGER"; > > + regulator-name = "CHARGER"; > > + regulator-min-microamp = <90000>; > > + regulator-max-microamp = <950000>; > > + regulator-boot-on; > > + }; > > + }; > > I prefer to add dt data about max14577-muic on following: > If extcon consumer driver need to use muic device, dts should provide following dt data. > > muic: max14577-muic { > compatible = "maxim, max14577-muic"; > }; > > + }; > > > For example, > If fuel-gauge want to use extcon subsystem, dt data of fuel-gauge add 'extcon' property as following: > > fuel-gague@ { > compatible = "..."; > extcon = <&muic>; > } > Hi, The extcon driver itself does not use the binding. It is only needed for consumers. Is it a common practice to add to documentation such notice for consumers only? Best regards, Krzysztof
On 12/04/2013 07:56 PM, Krzysztof Kozlowski wrote: > On Wed, 2013-12-04 at 19:50 +0900, Chanwoo Choi wrote: >> Hi Krzysztof, >> >> On 12/04/2013 07:40 PM, 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 | 48 ++++++++++++++++++++ >>> 1 file changed, 48 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..f4fd16360b6b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mfd/max14577.txt >>> @@ -0,0 +1,48 @@ >>> +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: >>> +- regulators : >>> + Required child node properties: >>> + - compatible : "maxim,max14577-regulator" >>> + >>> + Optional child nodes: >>> + Each child node representing a regulator, following standard regulator >>> + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". >>> + The SAFEOUT is a constant voltage regulator so there is no need to specify >>> + voltages for it. >>> + >>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt >>> + >>> +Example: >>> + max14577@25 { >>> + compatible = "maxim,max14577"; >>> + reg = <0x25>; >>> + interrupt-parent = <&gpx1>; >>> + interrupts = <5 0>; >>> + >>> + regulators { >>> + compatible = "maxim,max14577-regulator"; >>> + >>> + safeout_reg: safeout@1 { >>> + regulator-compatible = "SAFEOUT"; >>> + regulator-name = "SAFEOUT"; >>> + }; >>> + charger_reg: charger@0 { >>> + regulator-compatible = "CHARGER"; >>> + regulator-name = "CHARGER"; >>> + regulator-min-microamp = <90000>; >>> + regulator-max-microamp = <950000>; >>> + regulator-boot-on; >>> + }; >>> + }; >> >> I prefer to add dt data about max14577-muic on following: >> If extcon consumer driver need to use muic device, dts should provide following dt data. >> >> muic: max14577-muic { >> compatible = "maxim, max14577-muic"; >> }; >>> + }; >>> >> For example, >> If fuel-gauge want to use extcon subsystem, dt data of fuel-gauge add 'extcon' property as following: >> >> fuel-gague@ { >> compatible = "..."; >> extcon = <&muic>; >> } >> > Hi, > > The extcon driver itself does not use the binding. It is only needed for > consumers. Is it a common practice to add to documentation such notice > for consumers only? As you comment, It is only for extcon consumer. But extcon provider driver always is operated with extcon consumer driver. If dts file don't support upper dt data about extcon provider driver, all of extcon consumer drivers could not use extcon subsystem. I thinks it is common dt data for muic device. Thanks, Chanwoo Choi
On Wed, 2013-12-04 at 20:01 +0900, Chanwoo Choi wrote: > On 12/04/2013 07:56 PM, Krzysztof Kozlowski wrote: > > On Wed, 2013-12-04 at 19:50 +0900, Chanwoo Choi wrote: (...) > >> > >> I prefer to add dt data about max14577-muic on following: > >> If extcon consumer driver need to use muic device, dts should provide following dt data. > >> > >> muic: max14577-muic { > >> compatible = "maxim, max14577-muic"; > >> }; > >>> + }; > >>> > >> For example, > >> If fuel-gauge want to use extcon subsystem, dt data of fuel-gauge add 'extcon' property as following: > >> > >> fuel-gague@ { > >> compatible = "..."; > >> extcon = <&muic>; > >> } > >> > > Hi, > > > > The extcon driver itself does not use the binding. It is only needed for > > consumers. Is it a common practice to add to documentation such notice > > for consumers only? > > As you comment, It is only for extcon consumer. > But extcon provider driver always is operated with extcon consumer driver. > If dts file don't support upper dt data about extcon provider driver, > all of extcon consumer drivers could not use extcon subsystem. > > I thinks it is common dt data for muic device. OK, I'll add it. Best regards, Krzysztof
On 12/04/2013 08:03 PM, Krzysztof Kozlowski wrote: > On Wed, 2013-12-04 at 20:01 +0900, Chanwoo Choi wrote: >> On 12/04/2013 07:56 PM, Krzysztof Kozlowski wrote: >>> On Wed, 2013-12-04 at 19:50 +0900, Chanwoo Choi wrote: > (...) >>>> >>>> I prefer to add dt data about max14577-muic on following: >>>> If extcon consumer driver need to use muic device, dts should provide following dt data. >>>> >>>> muic: max14577-muic { >>>> compatible = "maxim, max14577-muic"; >>>> }; >>>>> + }; >>>>> >>>> For example, >>>> If fuel-gauge want to use extcon subsystem, dt data of fuel-gauge add 'extcon' property as following: >>>> >>>> fuel-gague@ { >>>> compatible = "..."; >>>> extcon = <&muic>; >>>> } >>>> >>> Hi, >>> >>> The extcon driver itself does not use the binding. It is only needed for >>> consumers. Is it a common practice to add to documentation such notice >>> for consumers only? >> >> As you comment, It is only for extcon consumer. >> But extcon provider driver always is operated with extcon consumer driver. >> If dts file don't support upper dt data about extcon provider driver, >> all of extcon consumer drivers could not use extcon subsystem. >> >> I thinks it is common dt data for muic device. > > OK, I'll add it. > Thanks, Best Regards, Chanwoo Choi
> 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 | 48 ++++++++++++++++++++ > 1 file changed, 48 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..f4fd16360b6b > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/max14577.txt > @@ -0,0 +1,48 @@ > +MAXIM MAX14577 multi-function device Why is Maxim capitalised in such a way? If you're going to use the term Multi-Function Device, please capitalise it. > +MAX14577 is a Multi-function device with Micro-USB Interface Circuit, Li+ Same here. > +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: > +- regulators : > + Required child node properties: > + - compatible : "maxim,max14577-regulator" > + > + Optional child nodes: > + Each child node representing a regulator, following standard regulator > + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". > + The SAFEOUT is a constant voltage regulator so there is no need s/constant/fixed > + to specify voltages for it. The whole of the "Optional nodes" explaination isn't as clear as it could otherwise be. > + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt > + > +Example: > + max14577@25 { > + compatible = "maxim,max14577"; > + reg = <0x25>; > + interrupt-parent = <&gpx1>; > + interrupts = <5 0>; I'm gussing this is IRQ5 and the 0 is flags? If so, please use #defines for the flags. > + regulators { > + compatible = "maxim,max14577-regulator"; > + > + safeout_reg: safeout@1 { The name of the node (not the label:safeout_reg) usually depicts the type of device, so 'regulator@0' and 'regulator@1'. However, I'm unsure how this works when these nodes are encapsulated by an overarching 'regulators' parent node. Perhaps Mark can answer this? > + regulator-compatible = "SAFEOUT"; > + regulator-name = "SAFEOUT"; > + }; > + charger_reg: charger@0 { > + regulator-compatible = "CHARGER"; > + regulator-name = "CHARGER"; > + regulator-min-microamp = <90000>; > + regulator-max-microamp = <950000>; > + regulator-boot-on; > + }; > + }; > + };
On Wed, Dec 04, 2013 at 11:20:39AM +0000, Lee Jones wrote: > > + regulators { > > + compatible = "maxim,max14577-regulator"; > > + > > + safeout_reg: safeout@1 { > The name of the node (not the label:safeout_reg) usually depicts the > type of device, so 'regulator@0' and 'regulator@1'. However, I'm > unsure how this works when these nodes are encapsulated by an > overarching 'regulators' parent node. > Perhaps Mark can answer this? The name there is meaningless, the DT convention is as you describe and applies to any node but equally well it's just a convention and I struggle to see the value it adds. Having something that humans find helpful is just as well.
On Wed, 2013-12-04 at 11:20 +0000, Lee Jones 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 | 48 ++++++++++++++++++++ > > 1 file changed, 48 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..f4fd16360b6b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/max14577.txt > > @@ -0,0 +1,48 @@ > > +MAXIM MAX14577 multi-function device > > Why is Maxim capitalised in such a way? > > If you're going to use the term Multi-Function Device, please > capitalise it. > > > +MAX14577 is a Multi-function device with Micro-USB Interface Circuit, Li+ > > Same here. I'll fix this. The MAXIM capitalization came from other drivers. > > +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: > > +- regulators : > > + Required child node properties: > > + - compatible : "maxim,max14577-regulator" > > + > > + Optional child nodes: > > + Each child node representing a regulator, following standard regulator > > + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". > > + The SAFEOUT is a constant voltage regulator so there is no need > > s/constant/fixed OK. > > + to specify voltages for it. > > The whole of the "Optional nodes" explaination isn't as clear as it > could otherwise be. > > > + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt > > + > > +Example: > > + max14577@25 { > > + compatible = "maxim,max14577"; > > + reg = <0x25>; > > + interrupt-parent = <&gpx1>; > > + interrupts = <5 0>; > > I'm gussing this is IRQ5 and the 0 is flags? If so, please use > #defines for the flags. This is a standard interrupt binding. The max14577 MFD core driver ignores the interrupt flags and always uses IRQF_TRIGGER_FALLING. I can fix this with new patch for drivers/mfd/max14577.c but still I don't see what should be added in documentation. Best regards, Krzysztof
> > > +MAXIM MAX14577 multi-function device > > > > Why is Maxim capitalised in such a way? > > > > If you're going to use the term Multi-Function Device, please > > capitalise it. > > > > > +MAX14577 is a Multi-function device with Micro-USB Interface Circuit, Li+ > > > > Same here. > > I'll fix this. The MAXIM capitalization came from other drivers. No need to repeat mistakes. I believe it should be Maxim. > > > +Example: > > > + max14577@25 { > > > + compatible = "maxim,max14577"; > > > + reg = <0x25>; > > > + interrupt-parent = <&gpx1>; > > > + interrupts = <5 0>; > > > > I'm gussing this is IRQ5 and the 0 is flags? If so, please use > > #defines for the flags. > > This is a standard interrupt binding. The max14577 MFD core driver > ignores the interrupt flags and always uses IRQF_TRIGGER_FALLING. I can > fix this with new patch for drivers/mfd/max14577.c but still I don't see > what should be added in documentation. s/0/IRQ_TYPE_NONE
diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt new file mode 100644 index 000000000000..f4fd16360b6b --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/max14577.txt @@ -0,0 +1,48 @@ +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: +- regulators : + Required child node properties: + - compatible : "maxim,max14577-regulator" + + Optional child nodes: + Each child node representing a regulator, following standard regulator + bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT". + The SAFEOUT is a constant voltage regulator so there is no need to specify + voltages for it. + + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt + +Example: + max14577@25 { + compatible = "maxim,max14577"; + reg = <0x25>; + interrupt-parent = <&gpx1>; + interrupts = <5 0>; + + regulators { + compatible = "maxim,max14577-regulator"; + + safeout_reg: safeout@1 { + regulator-compatible = "SAFEOUT"; + regulator-name = "SAFEOUT"; + }; + charger_reg: charger@0 { + regulator-compatible = "CHARGER"; + regulator-name = "CHARGER"; + regulator-min-microamp = <90000>; + regulator-max-microamp = <950000>; + regulator-boot-on; + }; + }; + };