Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for twl6030-power

Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for twl6030-power

[Mithil]

Subject: Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for
twl6030-power

On Tue, Aug 23, 2022 at 12:54 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Aug 20, 2022 at 12:46:55PM +0530, Mithil Bavishi wrote:
> > Adds documentation for the twl6030 power driver.
> >
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
> > ---
> >  .../bindings/power/reset/twl6030-power.txt    | 31 +++++++++++++++++++
>
> New bindings must be DT schema format.
>
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/twl6030-power.txt b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> > new file mode 100644
> > index 000000000..946bb3d9f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> > @@ -0,0 +1,31 @@
> > +Texas Instruments TWL family (twl6030) reset and power management module
> > +
> > +For now, the binding only supports the complete shutdown of the system after
> > +poweroff.
> > +
> > +Required properties:
> > +- compatible : must be
> > +       "ti,twl6030-power"
> > +
> > +Optional properties:
> > +
> > +- ti,system-power-controller: This indicates that TWL6030 is the
>
> We have a generic property for this.
>

What is property is that? And how would it get implemented here?

> > +  power supply master of the system. With this flag, the chip will
> > +  initiate an ACTIVE-to-OFF or SLEEP-to-OFF transition when the
> > +  system poweroffs.
> > +
> > +Example:
> > +&i2c1 {
> > +       clock-frequency = <2600000>;
> > +
> > +       twl: twl@48 {
> > +               reg = <0x48>;
> > +               interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> > +               interrupt-parent = <&intc>;
> > +
> > +               twl_power: power {
> > +                       compatible = "ti,twl6030-power";
> > +                       ti,system-power-controller;
>
> Why do you need a child node here? There aren't any resources for the
> sub-block.
>

Just an example and how it is used on a device as well, is it fine if
just the block is as-is?

Mithil

Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for twl6030-power

[Krzysztof Kozlowski]

On 14/06/2023 11:13, Mithil wrote:
> Subject: Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for
> twl6030-power
> 
> On Tue, Aug 23, 2022 at 12:54 AM Rob Herring <robh@kernel.org> wrote:
>>
>> On Sat, Aug 20, 2022 at 12:46:55PM +0530, Mithil Bavishi wrote:
>>> Adds documentation for the twl6030 power driver.
>>>
>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>> Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
>>> ---
>>>  .../bindings/power/reset/twl6030-power.txt    | 31 +++++++++++++++++++
>>
>> New bindings must be DT schema format.
>>
>>>  1 file changed, 31 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/power/reset/twl6030-power.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/reset/twl6030-power.txt b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
>>> new file mode 100644
>>> index 000000000..946bb3d9f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
>>> @@ -0,0 +1,31 @@
>>> +Texas Instruments TWL family (twl6030) reset and power management module
>>> +
>>> +For now, the binding only supports the complete shutdown of the system after
>>> +poweroff.
>>> +
>>> +Required properties:
>>> +- compatible : must be
>>> +       "ti,twl6030-power"
>>> +
>>> +Optional properties:
>>> +
>>> +- ti,system-power-controller: This indicates that TWL6030 is the
>>
>> We have a generic property for this.
>>
> 
> What is property is that? And how would it get implemented here?

Easy to guess...

git grep system-power-controller



> 
>>> +  power supply master of the system. With this flag, the chip will
>>> +  initiate an ACTIVE-to-OFF or SLEEP-to-OFF transition when the
>>> +  system poweroffs.
>>> +
>>> +Example:
>>> +&i2c1 {
>>> +       clock-frequency = <2600000>;
>>> +
>>> +       twl: twl@48 {
>>> +               reg = <0x48>;
>>> +               interrupts = <7>; /* SYS_NIRQ cascaded to intc */
>>> +               interrupt-parent = <&intc>;
>>> +
>>> +               twl_power: power {
>>> +                       compatible = "ti,twl6030-power";
>>> +                       ti,system-power-controller;
>>
>> Why do you need a child node here? There aren't any resources for the
>> sub-block.
>>
> 
> Just an example and how it is used on a device as well, is it fine if
> just the block is as-is?

The question is not about example. Question was why do you need child
node at all. Children without resources are usually useless.

Best regards,
Krzysztof

[PATCH 00/10] Add TWL6030 power off and powerbutton support

[Mithil Bavishi]

Hello!

This patchset is based off on an earlier series of patches submitted
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1110053.html
It adds support for power off to the TWL6030 chipset, adds power button
support to the TWL6030 chipset, moves power driver to the correct
location, clean up in code from the original patch.

Thanks, Mithil

Mithil Bavishi (6):
  power: reset: Move TWL4030 power driver from mfd
  dt-bindings: power: reset: Move twl4030-power from mfd to power/reset
  dt-bindings: input: twl-pwrbutton: Add support for twl6030-pwrbutton
  dt-bindings: power: reset: Add bindings for twl6030-power
  input: misc: Rename twl4030_pwrbutton to twl_pwrbutton
  dt-bindings: input: Rename twl4030-pwrbutton to twl-pwrbutton

Paul Kocialkowski (4):
  power: reset: Add TWL6030 power driver, with minimal support for power
    off
  ARM: OMAP2+: Only select TWL4030_POWER for OMAP3
  ARM: OMAP2+: Select TWL6030_POWER for OMAP4
  input: misc: Add TWL6030 power button support to twl-pwrbutton

 ...wl4030-pwrbutton.txt => twl-pwrbutton.txt} |  5 +-
 .../{mfd => power/reset}/twl4030-power.txt    |  0
 .../bindings/power/reset/twl6030-power.txt    | 31 +++++++
 arch/arm/boot/dts/twl6030.dtsi                |  5 +
 arch/arm/configs/omap2plus_defconfig          |  2 +-
 arch/arm/mach-omap2/Kconfig                   |  3 +-
 drivers/input/misc/Kconfig                    |  8 +-
 drivers/input/misc/Makefile                   |  2 +-
 .../{twl4030-pwrbutton.c => twl-pwrbutton.c}  | 75 ++++++++++-----
 drivers/mfd/Kconfig                           | 13 ---
 drivers/mfd/Makefile                          |  1 -
 drivers/power/reset/Kconfig                   | 22 +++++
 drivers/power/reset/Makefile                  |  2 +
 drivers/{mfd => power/reset}/twl4030-power.c  |  0
 drivers/power/reset/twl6030-power.c           | 93 +++++++++++++++++++
 include/linux/mfd/twl.h                       |  1 +
 16 files changed, 217 insertions(+), 46 deletions(-)
 rename Documentation/devicetree/bindings/input/{twl4030-pwrbutton.txt => twl-pwrbutton.txt} (72%)
 rename Documentation/devicetree/bindings/{mfd => power/reset}/twl4030-power.txt (100%)
 create mode 100644 Documentation/devicetree/bindings/power/reset/twl6030-power.txt
 rename drivers/input/misc/{twl4030-pwrbutton.c => twl-pwrbutton.c} (55%)
 rename drivers/{mfd => power/reset}/twl4030-power.c (100%)
 create mode 100644 drivers/power/reset/twl6030-power.c

-- 
2.25.1

[PATCH 05/10] dt-bindings: power: reset: Add bindings for twl6030-power

[Mithil Bavishi]

Adds documentation for the twl6030 power driver.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
---
 .../bindings/power/reset/twl6030-power.txt    | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/twl6030-power.txt

diff --git a/Documentation/devicetree/bindings/power/reset/twl6030-power.txt b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
new file mode 100644
index 000000000..946bb3d9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
@@ -0,0 +1,31 @@
+Texas Instruments TWL family (twl6030) reset and power management module
+
+For now, the binding only supports the complete shutdown of the system after
+poweroff.
+
+Required properties:
+- compatible : must be
+       "ti,twl6030-power"
+
+Optional properties:
+
+- ti,system-power-controller: This indicates that TWL6030 is the
+  power supply master of the system. With this flag, the chip will
+  initiate an ACTIVE-to-OFF or SLEEP-to-OFF transition when the
+  system poweroffs.
+
+Example:
+&i2c1 {
+       clock-frequency = <2600000>;
+
+       twl: twl@48 {
+               reg = <0x48>;
+               interrupts = <7>; /* SYS_NIRQ cascaded to intc */
+               interrupt-parent = <&intc>;
+
+               twl_power: power {
+                       compatible = "ti,twl6030-power";
+                       ti,system-power-controller;
+               };
+       };
+};
-- 
2.25.1

Re: [PATCH 05/10] dt-bindings: power: reset: Add bindings for twl6030-power

[Rob Herring]

On Sat, Aug 20, 2022 at 12:46:55PM +0530, Mithil Bavishi wrote:
> Adds documentation for the twl6030 power driver.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> Signed-off-by: Mithil Bavishi <bavishimithil@gmail.com>
> ---
>  .../bindings/power/reset/twl6030-power.txt    | 31 +++++++++++++++++++

New bindings must be DT schema format.

>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/twl6030-power.txt b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> new file mode 100644
> index 000000000..946bb3d9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/twl6030-power.txt
> @@ -0,0 +1,31 @@
> +Texas Instruments TWL family (twl6030) reset and power management module
> +
> +For now, the binding only supports the complete shutdown of the system after
> +poweroff.
> +
> +Required properties:
> +- compatible : must be
> +       "ti,twl6030-power"
> +
> +Optional properties:
> +
> +- ti,system-power-controller: This indicates that TWL6030 is the

We have a generic property for this.

> +  power supply master of the system. With this flag, the chip will
> +  initiate an ACTIVE-to-OFF or SLEEP-to-OFF transition when the
> +  system poweroffs.
> +
> +Example:
> +&i2c1 {
> +       clock-frequency = <2600000>;
> +
> +       twl: twl@48 {
> +               reg = <0x48>;
> +               interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> +               interrupt-parent = <&intc>;
> +
> +               twl_power: power {
> +                       compatible = "ti,twl6030-power";
> +                       ti,system-power-controller;

Why do you need a child node here? There aren't any resources for the 
sub-block.

Rob