| Message ID | 1446161922-8755-2-git-send-email-afd@ti.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 29 Oct 2015, Andrew F. Davis wrote: > The TPS65912 PMIC contains several regulators and a GPIO controller. > Add bindings for the TPS65912 PMIC. > > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ Why have you dropped Linus' Review-by? > Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++ > .../bindings/regulator/tps65912-regulator.txt | 28 ++++++++++++ > 3 files changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt > create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt > create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt > new file mode 100644 > index 0000000..0c5c05c4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt > @@ -0,0 +1,16 @@ > +* TPS65912 GPIO Controller bindings > + > +Required properties: > + - compatible : Should be "ti,tps65912-gpio". > + - gpio-controller : Marks the device node as a GPIO Controller. > + - #gpio-cells : Should be two. The first cell is the pin number and You should add a (See: ../gpio/gpio.txt) here I think. > + the second cell is used to specify flags. > + See include/dt-bindings/gpio/gpio.h for possible values. This is a Linuxisum and shouldn't really live in here. I think it would be better to document them in ../gpio/gpio.txt and reference that instead. > +Example: > + > + gpio4: tps65912_gpio { Device names don't belong in node names. Just "gpio" will do. > + compatible = "ti,tps65912-gpio"; > + gpio-controller; > + #gpio-cells = <2>; > + }; > diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt > new file mode 100644 > index 0000000..da5804a > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/tps65912.txt > @@ -0,0 +1,51 @@ > +* TPS65912 Power Management Integrated Circuit bindings > + > +Required properties: > + - compatible : Should be "ti,tps65912". > + - reg : Slave address or chip select number (I2C / SPI). > + - interrupt-parent : The parent interrupt controller. > + - 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 Nit: We *normally* treat these as bullet-points and not place full-stops on them: $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l 5227 $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l 486 ... and I *personally* think it's easier to read the document if it's formatted as follows: - compatible : Should be "ti,tps65912". - reg : Slave address or chip select number (I2C / SPI). - interrupt-parent : The parent interrupt controller. - 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 ... I hope you agree. [...]
On 10/30/2015 12:10 PM, Lee Jones wrote: > On Thu, 29 Oct 2015, Andrew F. Davis wrote: > >> The TPS65912 PMIC contains several regulators and a GPIO controller. >> Add bindings for the TPS65912 PMIC. >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> --- >> .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ > > Why have you dropped Linus' Review-by? > Strange, I thought I made a change to this. Well this brings up a question, how much change can we have before we are supposed to drop Reviewed/Acked-by? >> Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++ >> .../bindings/regulator/tps65912-regulator.txt | 28 ++++++++++++ >> 3 files changed, 95 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt >> create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt >> create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt >> >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt >> new file mode 100644 >> index 0000000..0c5c05c4 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt >> @@ -0,0 +1,16 @@ >> +* TPS65912 GPIO Controller bindings >> + >> +Required properties: >> + - compatible : Should be "ti,tps65912-gpio". >> + - gpio-controller : Marks the device node as a GPIO Controller. >> + - #gpio-cells : Should be two. The first cell is the pin number and > > You should add a (See: ../gpio/gpio.txt) here I think. > ACK >> + the second cell is used to specify flags. >> + See include/dt-bindings/gpio/gpio.h for possible values. > > This is a Linuxisum and shouldn't really live in here. > > I think it would be better to document them in ../gpio/gpio.txt and > reference that instead. > Looks like that is already in ../gpio/gpio.txt:57 >> +Example: >> + >> + gpio4: tps65912_gpio { > > Device names don't belong in node names. > > Just "gpio" will do. > ACK >> + compatible = "ti,tps65912-gpio"; >> + gpio-controller; >> + #gpio-cells = <2>; >> + }; >> diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt >> new file mode 100644 >> index 0000000..da5804a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/tps65912.txt >> @@ -0,0 +1,51 @@ >> +* TPS65912 Power Management Integrated Circuit bindings >> + >> +Required properties: >> + - compatible : Should be "ti,tps65912". >> + - reg : Slave address or chip select number (I2C / SPI). >> + - interrupt-parent : The parent interrupt controller. >> + - 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 > > Nit: We *normally* treat these as bullet-points and not place > full-stops on them: > > $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l > 5227 > $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l > 486 > What about for multi-sentence descriptions, we need the middle full-stops, then to not have one on the end seems kinda odd looking. > ... and I *personally* think it's easier to read the document if it's > formatted as follows: > > - compatible : Should be "ti,tps65912". > - reg : Slave address or chip select number (I2C / SPI). > - interrupt-parent : The parent interrupt controller. > - 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 > > ... I hope you agree. > Yeah, I did that with the last driver, not sure why I forgot to fix that here, I'll fix this. > [...] > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 30 Oct 2015, Andrew F. Davis wrote: > On 10/30/2015 12:10 PM, Lee Jones wrote: > >On Thu, 29 Oct 2015, Andrew F. Davis wrote: > > > >>The TPS65912 PMIC contains several regulators and a GPIO controller. > >>Add bindings for the TPS65912 PMIC. > >> > >>Signed-off-by: Andrew F. Davis <afd@ti.com> > >>--- > >> .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ > > > >Why have you dropped Linus' Review-by? > > > > Strange, I thought I made a change to this. Well this brings up a question, > how much change can we have before we are supposed to drop Reviewed/Acked-by? Common sense call I'm afraid. ;) [...] > >>+ the second cell is used to specify flags. > >>+ See include/dt-bindings/gpio/gpio.h for possible values. > > > >This is a Linuxisum and shouldn't really live in here. > > > >I think it would be better to document them in ../gpio/gpio.txt and > >reference that instead. > > > > Looks like that is already in ../gpio/gpio.txt:57 There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example. However GPIO_ACTIVE_LOW is missing. I think both could do with documenting properly, then you can refer to them from here. [...] > >>+Required properties: > >>+ - compatible : Should be "ti,tps65912". > >>+ - reg : Slave address or chip select number (I2C / SPI). > >>+ - interrupt-parent : The parent interrupt controller. > >>+ - 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 > > > >Nit: We *normally* treat these as bullet-points and not place > >full-stops on them: > > > >$ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l > >5227 > >$ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l > >486 > > > > What about for multi-sentence descriptions, we need the middle full-stops, then to not > have one on the end seems kinda odd looking. That's the way I usually do it -- doesn't look too bad. ;) [...]
On 10/30/2015 02:06 PM, Lee Jones wrote: > On Fri, 30 Oct 2015, Andrew F. Davis wrote: > >> On 10/30/2015 12:10 PM, Lee Jones wrote: >>> On Thu, 29 Oct 2015, Andrew F. Davis wrote: >>> >>>> The TPS65912 PMIC contains several regulators and a GPIO controller. >>>> Add bindings for the TPS65912 PMIC. >>>> >>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>> --- >>>> .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ >>> >>> Why have you dropped Linus' Review-by? >>> >> >> Strange, I thought I made a change to this. Well this brings up a question, >> how much change can we have before we are supposed to drop Reviewed/Acked-by? > > Common sense call I'm afraid. ;) > > [...] > >>>> + the second cell is used to specify flags. >>>> + See include/dt-bindings/gpio/gpio.h for possible values. >>> >>> This is a Linuxisum and shouldn't really live in here. >>> >>> I think it would be better to document them in ../gpio/gpio.txt and >>> reference that instead. >>> >> >> Looks like that is already in ../gpio/gpio.txt:57 > > There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example. > However GPIO_ACTIVE_LOW is missing. I think both could do with > documenting properly, then you can refer to them from here. > I mean the lines above the example, they say to use the macros defined in include/dt-bindings/gpio/gpio.h whenever possible, that's really all I would say. > [...] > >>>> +Required properties: >>>> + - compatible : Should be "ti,tps65912". >>>> + - reg : Slave address or chip select number (I2C / SPI). >>>> + - interrupt-parent : The parent interrupt controller. >>>> + - 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 >>> >>> Nit: We *normally* treat these as bullet-points and not place >>> full-stops on them: >>> >>> $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l >>> 5227 >>> $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l >>> 486 >>> >> >> What about for multi-sentence descriptions, we need the middle full-stops, then to not >> have one on the end seems kinda odd looking. > > That's the way I usually do it -- doesn't look too bad. ;) > > [...] > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 30 Oct 2015, Andrew F. Davis wrote: > On 10/30/2015 02:06 PM, Lee Jones wrote: > >On Fri, 30 Oct 2015, Andrew F. Davis wrote: > > > >>On 10/30/2015 12:10 PM, Lee Jones wrote: > >>>On Thu, 29 Oct 2015, Andrew F. Davis wrote: > >>> > >>>>The TPS65912 PMIC contains several regulators and a GPIO controller. > >>>>Add bindings for the TPS65912 PMIC. > >>>> > >>>>Signed-off-by: Andrew F. Davis <afd@ti.com> > >>>>--- > >>>> .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ > >>> > >>>Why have you dropped Linus' Review-by? > >>> > >> > >>Strange, I thought I made a change to this. Well this brings up a question, > >>how much change can we have before we are supposed to drop Reviewed/Acked-by? > > > >Common sense call I'm afraid. ;) > > > >[...] > > > >>>>+ the second cell is used to specify flags. > >>>>+ See include/dt-bindings/gpio/gpio.h for possible values. > >>> > >>>This is a Linuxisum and shouldn't really live in here. > >>> > >>>I think it would be better to document them in ../gpio/gpio.txt and > >>>reference that instead. > >>> > >> > >>Looks like that is already in ../gpio/gpio.txt:57 > > > >There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example. > >However GPIO_ACTIVE_LOW is missing. I think both could do with > >documenting properly, then you can refer to them from here. > > > > I mean the lines above the example, they say to use the macros > defined in include/dt-bindings/gpio/gpio.h whenever possible, > that's really all I would say. Let's at least attempt to stick to the rules. Please adapt your formatting to <dt-bindings/gpio/gpio.h> in order to eradicate any Linuxiness. > >[...] > > > >>>>+Required properties: > >>>>+ - compatible : Should be "ti,tps65912". > >>>>+ - reg : Slave address or chip select number (I2C / SPI). > >>>>+ - interrupt-parent : The parent interrupt controller. > >>>>+ - 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 > >>> > >>>Nit: We *normally* treat these as bullet-points and not place > >>>full-stops on them: > >>> > >>>$ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l > >>>5227 > >>>$ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l > >>>486 > >>> > >> > >>What about for multi-sentence descriptions, we need the middle full-stops, then to not > >>have one on the end seems kinda odd looking. > > > >That's the way I usually do it -- doesn't look too bad. ;) > > > >[...] > >
On 11/02/2015 03:17 AM, Lee Jones wrote: > On Fri, 30 Oct 2015, Andrew F. Davis wrote: > >> On 10/30/2015 02:06 PM, Lee Jones wrote: >>> On Fri, 30 Oct 2015, Andrew F. Davis wrote: >>> >>>> On 10/30/2015 12:10 PM, Lee Jones wrote: >>>>> On Thu, 29 Oct 2015, Andrew F. Davis wrote: >>>>> >>>>>> The TPS65912 PMIC contains several regulators and a GPIO controller. >>>>>> Add bindings for the TPS65912 PMIC. >>>>>> >>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>>>> --- >>>>>> .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ >>>>> >>>>> Why have you dropped Linus' Review-by? >>>>> >>>> >>>> Strange, I thought I made a change to this. Well this brings up a question, >>>> how much change can we have before we are supposed to drop Reviewed/Acked-by? >>> >>> Common sense call I'm afraid. ;) >>> >>> [...] >>> >>>>>> + the second cell is used to specify flags. >>>>>> + See include/dt-bindings/gpio/gpio.h for possible values. >>>>> >>>>> This is a Linuxisum and shouldn't really live in here. >>>>> >>>>> I think it would be better to document them in ../gpio/gpio.txt and >>>>> reference that instead. >>>>> >>>> >>>> Looks like that is already in ../gpio/gpio.txt:57 >>> >>> There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example. >>> However GPIO_ACTIVE_LOW is missing. I think both could do with >>> documenting properly, then you can refer to them from here. >>> >> >> I mean the lines above the example, they say to use the macros >> defined in include/dt-bindings/gpio/gpio.h whenever possible, >> that's really all I would say. > > Let's at least attempt to stick to the rules. > > Please adapt your formatting to <dt-bindings/gpio/gpio.h> in order to > eradicate any Linuxiness. > Works for me, although I pushed v6 a couple days ago with a different fix, maybe that will work too? >>> [...] >>> >>>>>> +Required properties: >>>>>> + - compatible : Should be "ti,tps65912". >>>>>> + - reg : Slave address or chip select number (I2C / SPI). >>>>>> + - interrupt-parent : The parent interrupt controller. >>>>>> + - 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 >>>>> >>>>> Nit: We *normally* treat these as bullet-points and not place >>>>> full-stops on them: >>>>> >>>>> $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l >>>>> 5227 >>>>> $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l >>>>> 486 >>>>> >>>> >>>> What about for multi-sentence descriptions, we need the middle full-stops, then to not >>>> have one on the end seems kinda odd looking. >>> >>> That's the way I usually do it -- doesn't look too bad. ;) >>> >>> [...] >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt new file mode 100644 index 0000000..0c5c05c4 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt @@ -0,0 +1,16 @@ +* TPS65912 GPIO Controller bindings + +Required properties: + - compatible : Should be "ti,tps65912-gpio". + - gpio-controller : Marks the device node as a GPIO Controller. + - #gpio-cells : Should be two. The first cell is the pin number and + the second cell is used to specify flags. + See include/dt-bindings/gpio/gpio.h for possible values. + +Example: + + gpio4: tps65912_gpio { + compatible = "ti,tps65912-gpio"; + gpio-controller; + #gpio-cells = <2>; + }; diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt new file mode 100644 index 0000000..da5804a --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/tps65912.txt @@ -0,0 +1,51 @@ +* TPS65912 Power Management Integrated Circuit bindings + +Required properties: + - compatible : Should be "ti,tps65912". + - reg : Slave address or chip select number (I2C / SPI). + - interrupt-parent : The parent interrupt controller. + - 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 + +Additional nodes defined in: + - Regulators: ../regulator/tps65912-regulator.txt + - GPIO: ../gpio/gpio-tps65912.txt. + +Example: + + pmic: tps65912@2d { + compatible = "ti,tps65912"; + reg = <0x2d>; + interrupt-parent = <&gpio1>; + interrupts = <28 IRQ_TYPE_LEVEL_LOW>; + interrupt-controller; + #interrupt-cells = <2>; + + regulators { + compatible = "ti,tps65912-regulator"; + + dcdc1 { + regulator-name = "vdd_core"; + regulator-min-microvolt = <912000>; + regulator-max-microvolt = <1144000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo1 { + regulator-name = "ldo1"; + regulator-min-microvolt = <1900000>; + regulator-max-microvolt = <1900000>; + }; + }; + + gpio4: tps65912_gpio { + compatible = "ti,tps65912-gpio"; + gpio-controller; + #gpio-cells = <2>; + }; + }; diff --git a/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt b/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt new file mode 100644 index 0000000..4cd05fa --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt @@ -0,0 +1,28 @@ +* TPS65912 regulator bindings + +Required properties: + - compatible: "ti,tps65912-regulator" + - list of regulators provided by this controller, must be named + after their hardware counterparts: dcdc[1-4] and ldo[1-10] + +Each regulator is defined using the standard binding for regulators. + +Example: + + tps65912_regulator { + compatible = "ti,tps65912-regulator"; + + dcdc1 { + regulator-name = "vdd_core"; + regulator-min-microvolt = <912000>; + regulator-max-microvolt = <1144000>; + regulator-boot-on; + regulator-always-on; + }; + + ldo1 { + regulator-name = "ldo1"; + regulator-min-microvolt = <1900000>; + regulator-max-microvolt = <1900000>; + }; + };
The TPS65912 PMIC contains several regulators and a GPIO controller. Add bindings for the TPS65912 PMIC. Signed-off-by: Andrew F. Davis <afd@ti.com> --- .../devicetree/bindings/gpio/gpio-tps65912.txt | 16 +++++++ Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++ .../bindings/regulator/tps65912-regulator.txt | 28 ++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt