Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1259645

Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

Show all headers | View raw

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.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
  [PATCH v5 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
  [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
    Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC Lee Jones <lee.jones@linaro.org> - 2015-10-30 18:20 +0100
      Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 19:10 +0100
        Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC Lee Jones <lee.jones@linaro.org> - 2015-10-30 20:10 +0100
          Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 20:30 +0100
            Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC Lee Jones <lee.jones@linaro.org> - 2015-11-02 10:20 +0100
              Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-11-02 16:20 +0100

csiph-web