[PATCH v3 0/6] mfd: twl: system-power-controller

[PATCH v3 0/6] mfd: twl: system-power-controller

[Andreas Kemnade]

Add system-power-controller property in the bindings and
the corresponding implementation and use it where
appropriate.
Not all cases are hit yet, there has probably to be a
separate series after going through with a brush.

Changes in v3:
- twl-core: 
   - remove repetitive verbose error messages
   - placed constants at top part of function
   - minor cleanups

Changes in v2:
- add A-By
- fix compiler warning

Andreas Kemnade (6):
  dt-bindings: mfd: ti,twl: Document system-power-controller
  twl-core: add power off implementation for twl603x
  ARM: dts: omap-embt2ws: system-power-controller for bt200
  ARM: dts: omap4-panda-common: Enable powering off the device
  mfd: twl4030-power: accept standard property for power controller
  ARM: dts: omap: gta04: standardize system-power-controller

 .../devicetree/bindings/mfd/ti,twl.yaml       |  2 ++
 arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi    |  2 +-
 .../boot/dts/ti/omap/omap4-epson-embt2ws.dts  |  1 +
 .../boot/dts/ti/omap/omap4-panda-common.dtsi  |  1 +
 drivers/mfd/twl-core.c                        | 28 +++++++++++++++++++
 drivers/mfd/twl4030-power.c                   |  3 ++
 include/linux/mfd/twl.h                       |  1 +
 7 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.39.2

[PATCH v3 1/6] dt-bindings: mfd: ti,twl: Document system-power-controller

[Andreas Kemnade]

Add system-power-controller property because these chips
can power off the device.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/mfd/ti,twl.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
index c04d57ba22b49..52ed228fb1e7e 100644
--- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
@@ -34,6 +34,8 @@ properties:
 
   interrupt-controller: true
 
+  system-power-controller: true
+
   "#interrupt-cells":
     const: 1
 
-- 
2.39.2

[PATCH v3 2/6] twl-core: add power off implementation for twl603x

[Andreas Kemnade]

If the system-power-controller property is there, enable power off.
Implementation is based on a Linux v3.0 vendor kernel.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/mfd/twl-core.c  | 28 ++++++++++++++++++++++++++++
 include/linux/mfd/twl.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 6e384a79e3418..f3982d18008d1 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -124,6 +124,11 @@
 #define TWL6030_BASEADD_RSV		0x0000
 #define TWL6030_BASEADD_ZERO		0x0000
 
+/* some fields in TWL6030_PHOENIX_DEV_ON */
+#define TWL6030_APP_DEVOFF		BIT(0)
+#define TWL6030_CON_DEVOFF		BIT(1)
+#define TWL6030_MOD_DEVOFF		BIT(2)
+
 /* Few power values */
 #define R_CFG_BOOT			0x05
 
@@ -687,6 +692,20 @@ static void twl_remove(struct i2c_client *client)
 	twl_priv->ready = false;
 }
 
+static void twl6030_power_off(void)
+{
+	int err;
+	u8 val;
+
+	err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &val, TWL6030_PHOENIX_DEV_ON);
+	if (err)
+		return;
+
+	val |= TWL6030_APP_DEVOFF | TWL6030_CON_DEVOFF | TWL6030_MOD_DEVOFF;
+	twl_i2c_write_u8(TWL_MODULE_PM_MASTER, val, TWL6030_PHOENIX_DEV_ON);
+}
+
+
 static struct of_dev_auxdata twl_auxdata_lookup[] = {
 	OF_DEV_AUXDATA("ti,twl4030-gpio", 0, "twl4030-gpio", NULL),
 	{ /* sentinel */ },
@@ -852,6 +871,15 @@ twl_probe(struct i2c_client *client)
 			goto free;
 	}
 
+	if (twl_class_is_6030()) {
+		if (of_device_is_system_power_controller(node)) {
+			if (!pm_power_off)
+				pm_power_off = twl6030_power_off;
+			else
+				dev_warn(&client->dev, "Poweroff callback already assigned\n");
+		}
+	}
+
 	status = of_platform_populate(node, NULL, twl_auxdata_lookup,
 				      &client->dev);
 
diff --git a/include/linux/mfd/twl.h b/include/linux/mfd/twl.h
index c062d91a67d92..85dc406173dba 100644
--- a/include/linux/mfd/twl.h
+++ b/include/linux/mfd/twl.h
@@ -461,6 +461,7 @@ static inline int twl6030_mmc_card_detect(struct device *dev, int slot)
 
 #define TWL4030_PM_MASTER_GLOBAL_TST		0xb6
 
+#define TWL6030_PHOENIX_DEV_ON                  0x06
 /*----------------------------------------------------------------------*/
 
 /* Power bus message definitions */
-- 
2.39.2

[PATCH v3 3/6] ARM: dts: omap-embt2ws: system-power-controller for bt200

[Andreas Kemnade]

Configure the TWL6032 as system power controller to let the device
power off.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
index 4e267b259ebf0..bb2e9544723c3 100644
--- a/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
+++ b/arch/arm/boot/dts/ti/omap/omap4-epson-embt2ws.dts
@@ -215,6 +215,7 @@ twl: pmic@48 {
 		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; /* IRQ_SYS_1N cascaded to gic */
 		interrupt-controller;
 		#interrupt-cells = <1>;
+		system-power-controller;
 
 		rtc {
 			compatible = "ti,twl4030-rtc";
-- 
2.39.2

[PATCH v3 4/6] ARM: dts: omap4-panda-common: Enable powering off the device

[Andreas Kemnade]

As the TWL6030 chip is the main power controller here, declare
it as system-power-controller

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi b/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
index f528511c2537b..97706d6296a68 100644
--- a/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
@@ -408,6 +408,7 @@ twl: twl@48 {
 		reg = <0x48>;
 		/* IRQ# = 7 */
 		interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; /* IRQ_SYS_1N cascaded to gic */
+		system-power-controller;
 	};
 
 	twl6040: twl@4b {
-- 
2.39.2

[PATCH v3 5/6] mfd: twl4030-power: accept standard property for power controller

[Andreas Kemnade]

Instead of only accepting the ti specific properties accept also
the standard property. For uniformity, search in the parent node
for the tag. The code for powering of is also isolated from the
rest in this file. So it is a pure Linux design decision to put it
here.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/mfd/twl4030-power.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index e35b0f788c504..3ef892e63b88f 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -686,6 +686,9 @@ static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata,
 	if (of_property_read_bool(node, "ti,use_poweroff"))
 		return true;
 
+	if (of_device_is_system_power_controller(node->parent))
+		return true;
+
 	return false;
 }
 
-- 
2.39.2

[PATCH v3 6/6] ARM: dts: omap: gta04: standardize system-power-controller

[Andreas Kemnade]

Replace TI-specific property by generic one.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
cannot be applied independently of the other ones, so maybe simply delay
it.

 arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
index 3661340009e7a..5001c4ea35658 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
@@ -476,6 +476,7 @@ twl: twl@48 {
 		reg = <0x48>;
 		interrupts = <7>; /* SYS_NIRQ cascaded to intc */
 		interrupt-parent = <&intc>;
+		system-power-controller;
 
 		clocks = <&hfclk_26m>;
 		clock-names = "fck";
@@ -490,7 +491,6 @@ codec {
 
 		twl_power: power {
 			compatible = "ti,twl4030-power-idle";
-			ti,system-power-controller;
 		};
 	};
 };
-- 
2.39.2

Re: [PATCH v3 6/6] ARM: dts: omap: gta04: standardize system-power-controller

[Adam Ford]

On Sun, Dec 3, 2023 at 4:29 PM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> Replace TI-specific property by generic one.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> cannot be applied independently of the other ones, so maybe simply delay
> it.
>
>  arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> index 3661340009e7a..5001c4ea35658 100644
> --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> @@ -476,6 +476,7 @@ twl: twl@48 {
>                 reg = <0x48>;
>                 interrupts = <7>; /* SYS_NIRQ cascaded to intc */
>                 interrupt-parent = <&intc>;
> +               system-power-controller;

Could this go into the twl4030.dtsi file so we don't have every omap3
board with this part duplicating this line?

adam
>
>                 clocks = <&hfclk_26m>;
>                 clock-names = "fck";
> @@ -490,7 +491,6 @@ codec {
>
>                 twl_power: power {
>                         compatible = "ti,twl4030-power-idle";
> -                       ti,system-power-controller;
>                 };
>         };
>  };
> --
> 2.39.2
>
>

Re: [PATCH v3 6/6] ARM: dts: omap: gta04: standardize system-power-controller

[Andreas Kemnade]

On Sun, 3 Dec 2023 16:46:20 -0600
Adam Ford <aford173@gmail.com> wrote:

> On Sun, Dec 3, 2023 at 4:29 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > Replace TI-specific property by generic one.
> >
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > cannot be applied independently of the other ones, so maybe simply delay
> > it.
> >
> >  arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > index 3661340009e7a..5001c4ea35658 100644
> > --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > @@ -476,6 +476,7 @@ twl: twl@48 {
> >                 reg = <0x48>;
> >                 interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> >                 interrupt-parent = <&intc>;
> > +               system-power-controller;  
> 
> Could this go into the twl4030.dtsi file so we don't have every omap3
> board with this part duplicating this line?
> 
Well, that is a board-specific issue, so I do not think it belongs there,
although quite common to have the twl4030 as the system-power-controller.

Regards,
Andreas

Re: [PATCH v3 6/6] ARM: dts: omap: gta04: standardize system-power-controller

[Tony Lindgren]

* Adam Ford <aford173@gmail.com> [231203 22:46]:
> On Sun, Dec 3, 2023 at 4:29 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > Replace TI-specific property by generic one.
> >
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > cannot be applied independently of the other ones, so maybe simply delay
> > it.
> >
> >  arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > index 3661340009e7a..5001c4ea35658 100644
> > --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
> > @@ -476,6 +476,7 @@ twl: twl@48 {
> >                 reg = <0x48>;
> >                 interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> >                 interrupt-parent = <&intc>;
> > +               system-power-controller;
> 
> Could this go into the twl4030.dtsi file so we don't have every omap3
> board with this part duplicating this line?

Sounds good to me.

Regards,

Tony

Re: [PATCH v3 2/6] twl-core: add power off implementation for twl603x

[Lee Jones]

On Sun, 03 Dec 2023, Andreas Kemnade wrote:

> If the system-power-controller property is there, enable power off.
> Implementation is based on a Linux v3.0 vendor kernel.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  drivers/mfd/twl-core.c  | 28 ++++++++++++++++++++++++++++
>  include/linux/mfd/twl.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 6e384a79e3418..f3982d18008d1 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -124,6 +124,11 @@
>  #define TWL6030_BASEADD_RSV		0x0000
>  #define TWL6030_BASEADD_ZERO		0x0000
>  
> +/* some fields in TWL6030_PHOENIX_DEV_ON */

My preference is for proper grammar in comments please.

"Some"

What is TWL6030_PHOENIX_DEV_ON?  A register?

> +#define TWL6030_APP_DEVOFF		BIT(0)
> +#define TWL6030_CON_DEVOFF		BIT(1)
> +#define TWL6030_MOD_DEVOFF		BIT(2)
> +
>  /* Few power values */
>  #define R_CFG_BOOT			0x05
>  
> @@ -687,6 +692,20 @@ static void twl_remove(struct i2c_client *client)
>  	twl_priv->ready = false;
>  }
>  
> +static void twl6030_power_off(void)
> +{
> +	int err;
> +	u8 val;
> +
> +	err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &val, TWL6030_PHOENIX_DEV_ON);
> +	if (err)
> +		return;
> +
> +	val |= TWL6030_APP_DEVOFF | TWL6030_CON_DEVOFF | TWL6030_MOD_DEVOFF;
> +	twl_i2c_write_u8(TWL_MODULE_PM_MASTER, val, TWL6030_PHOENIX_DEV_ON);
> +}
> +
> +
>  static struct of_dev_auxdata twl_auxdata_lookup[] = {
>  	OF_DEV_AUXDATA("ti,twl4030-gpio", 0, "twl4030-gpio", NULL),
>  	{ /* sentinel */ },
> @@ -852,6 +871,15 @@ twl_probe(struct i2c_client *client)
>  			goto free;
>  	}
>  
> +	if (twl_class_is_6030()) {

Is this check required?

> +		if (of_device_is_system_power_controller(node)) {

Shouldn't this cover it?

> +			if (!pm_power_off)
> +				pm_power_off = twl6030_power_off;
> +			else
> +				dev_warn(&client->dev, "Poweroff callback already assigned\n");

Can this happen?  Why would anyone care if it did?

> +		}
> +	}
> +
>  	status = of_platform_populate(node, NULL, twl_auxdata_lookup,
>  				      &client->dev);
>  
> diff --git a/include/linux/mfd/twl.h b/include/linux/mfd/twl.h
> index c062d91a67d92..85dc406173dba 100644
> --- a/include/linux/mfd/twl.h
> +++ b/include/linux/mfd/twl.h
> @@ -461,6 +461,7 @@ static inline int twl6030_mmc_card_detect(struct device *dev, int slot)
>  
>  #define TWL4030_PM_MASTER_GLOBAL_TST		0xb6
>  
> +#define TWL6030_PHOENIX_DEV_ON                  0x06
>  /*----------------------------------------------------------------------*/
>  
>  /* Power bus message definitions */
> -- 
> 2.39.2
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v3 5/6] mfd: twl4030-power: accept standard property for power controller

[Lee Jones]

On Sun, 03 Dec 2023, Andreas Kemnade wrote:

> Instead of only accepting the ti specific properties accept also
> the standard property. For uniformity, search in the parent node

Search 'in' the parent node or 'from' the parent node?

Where is the property?

> for the tag. The code for powering of is also isolated from the

Should this be "off"?

> rest in this file. So it is a pure Linux design decision to put it
> here.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  drivers/mfd/twl4030-power.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
> index e35b0f788c504..3ef892e63b88f 100644
> --- a/drivers/mfd/twl4030-power.c
> +++ b/drivers/mfd/twl4030-power.c
> @@ -686,6 +686,9 @@ static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata,
>  	if (of_property_read_bool(node, "ti,use_poweroff"))
>  		return true;
>  
> +	if (of_device_is_system_power_controller(node->parent))
> +		return true;
> +
>  	return false;
>  }
>  
> -- 
> 2.39.2
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v3 2/6] twl-core: add power off implementation for twl603x

[Andreas Kemnade]

On Thu, 7 Dec 2023 17:11:55 +0000
Lee Jones <lee@kernel.org> wrote:

> On Sun, 03 Dec 2023, Andreas Kemnade wrote:
> 
> > If the system-power-controller property is there, enable power off.
> > Implementation is based on a Linux v3.0 vendor kernel.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  drivers/mfd/twl-core.c  | 28 ++++++++++++++++++++++++++++
> >  include/linux/mfd/twl.h |  1 +
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> > index 6e384a79e3418..f3982d18008d1 100644
> > --- a/drivers/mfd/twl-core.c
> > +++ b/drivers/mfd/twl-core.c
> > @@ -124,6 +124,11 @@
> >  #define TWL6030_BASEADD_RSV		0x0000
> >  #define TWL6030_BASEADD_ZERO		0x0000
> >  
> > +/* some fields in TWL6030_PHOENIX_DEV_ON */  
> 
> My preference is for proper grammar in comments please.
> 
> "Some"
> 
> What is TWL6030_PHOENIX_DEV_ON?  A register?
> 
yes, a register.

> > +#define TWL6030_APP_DEVOFF		BIT(0)
> > +#define TWL6030_CON_DEVOFF		BIT(1)
> > +#define TWL6030_MOD_DEVOFF		BIT(2)
> > +
> >  /* Few power values */
> >  #define R_CFG_BOOT			0x05
> >  
> > @@ -687,6 +692,20 @@ static void twl_remove(struct i2c_client *client)
> >  	twl_priv->ready = false;
> >  }
> >  
> > +static void twl6030_power_off(void)
> > +{
> > +	int err;
> > +	u8 val;
> > +
> > +	err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &val, TWL6030_PHOENIX_DEV_ON);
> > +	if (err)
> > +		return;
> > +
> > +	val |= TWL6030_APP_DEVOFF | TWL6030_CON_DEVOFF | TWL6030_MOD_DEVOFF;
> > +	twl_i2c_write_u8(TWL_MODULE_PM_MASTER, val, TWL6030_PHOENIX_DEV_ON);
> > +}
> > +
> > +
> >  static struct of_dev_auxdata twl_auxdata_lookup[] = {
> >  	OF_DEV_AUXDATA("ti,twl4030-gpio", 0, "twl4030-gpio", NULL),
> >  	{ /* sentinel */ },
> > @@ -852,6 +871,15 @@ twl_probe(struct i2c_client *client)
> >  			goto free;
> >  	}
> >  
> > +	if (twl_class_is_6030()) {  
> 
> Is this check required?
> 
Well, as we want to do something specific to 603x we need the check.

> > +		if (of_device_is_system_power_controller(node)) {  
> 
> Shouldn't this cover it?
> 
Well, in the twl4030 case there is another power off routine required.


> > +			if (!pm_power_off)
> > +				pm_power_off = twl6030_power_off;
> > +			else
> > +				dev_warn(&client->dev, "Poweroff callback already assigned\n");  
> 
> Can this happen?  Why would anyone care if it did?
> 
If system is correctly configured, then not. Well, I do not know about others
but I personally expect my devices to turn on after having them turned off
without significant battery loss and really turn off.
But if it is not the case, then having such warning messages would be an
early indication that something is really wrong.
I personally have been in a situation where two devices wanted to register
a power off handler. Order of device probing is random, so having such a 
message is a real good idea I think.

Regards,
Andreas

Re: [PATCH v3 5/6] mfd: twl4030-power: accept standard property for power controller

[Andreas Kemnade]

On Thu, 7 Dec 2023 17:13:41 +0000
Lee Jones <lee@kernel.org> wrote:

> On Sun, 03 Dec 2023, Andreas Kemnade wrote:
> 
> > Instead of only accepting the ti specific properties accept also
> > the standard property. For uniformity, search in the parent node  
> 
> Search 'in' the parent node or 'from' the parent node?
> 
> Where is the property?
> 
The idea was to have the device tree property at the same place for all
twl family devices. So no distinction for these devices needed
in the bindings. It is in the main node.

I guess today this twl4030-power subnode would not be accepted nowadays
and the compatible would be some kind of flag in the parent.

> > for the tag. The code for powering of is also isolated from the  
> 
> Should this be "off"?
> 
yes, of course.

Regards,
Andreas