[PATCH 1/4] dt-bindings: syscon: Add syscon endian properties support

[PATCH 1/4] dt-bindings: syscon: Add syscon endian properties support

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

In accordance with the syscon-driver (drivers/mfd/syscon.c) the syscon
dts-nodes may accept endian properties of the boolean type: little-endian,
big-endian, native-endian. Lets make sure that syscon bindings json-schema
also supports them.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 39375e4313d2..9ee404991533 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -61,6 +61,11 @@ properties:
     description:
       Reference to a phandle of a hardware spinlock provider node.
 
+patternProperties:
+  "^(big|little|native)-endian$":
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Bytes order of the system controller memory space.
+
 required:
   - compatible
   - reg
@@ -81,4 +86,13 @@ examples:
         hwlocks = <&hwlock1 1>;
     };
 
+  - |
+    cpu_ctl: cpu@1F04D02C {
+      compatible = "syscon";
+      reg = <0x1F04D02C 0x004>;
+
+      little-endian;
+      reg-io-width = <4>;
+    };
+
 ...
-- 
2.25.1

Re: [PATCH 1/4] dt-bindings: syscon: Add syscon endian properties support

[Rob Herring]

On Fri, Mar 06, 2020 at 04:03:38PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> In accordance with the syscon-driver (drivers/mfd/syscon.c) the syscon
> dts-nodes may accept endian properties of the boolean type: little-endian,
> big-endian, native-endian. Lets make sure that syscon bindings json-schema
> also supports them.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 39375e4313d2..9ee404991533 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -61,6 +61,11 @@ properties:
>      description:
>        Reference to a phandle of a hardware spinlock provider node.
>  
> +patternProperties:
> +  "^(big|little|native)-endian$":
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Bytes order of the system controller memory space.

Common properties should have a type definition in a common schema. For 
this one, I'd like it in the core schema in dtschema. 

I'd expect for any specific 'syscon', either none or only a subset of 
these are valid, so I don't think this should be added here.

Rob

Re: [PATCH 1/4] dt-bindings: syscon: Add syscon endian properties support

[Sergey Semin]

On Thu, Mar 12, 2020 at 04:11:02PM -0500, Rob Herring wrote:
> On Fri, Mar 06, 2020 at 04:03:38PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > In accordance with the syscon-driver (drivers/mfd/syscon.c) the syscon
> > dts-nodes may accept endian properties of the boolean type: little-endian,
> > big-endian, native-endian. Lets make sure that syscon bindings json-schema
> > also supports them.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  Documentation/devicetree/bindings/mfd/syscon.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > index 39375e4313d2..9ee404991533 100644
> > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > @@ -61,6 +61,11 @@ properties:
> >      description:
> >        Reference to a phandle of a hardware spinlock provider node.
> >  
> > +patternProperties:
> > +  "^(big|little|native)-endian$":
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: Bytes order of the system controller memory space.
> 
> Common properties should have a type definition in a common schema. For 
> this one, I'd like it in the core schema in dtschema. 

So what do you suggest then? Will you move this file to the dt-scheme/schemas
in your repo? What shall I do with this patch, just drop? Or would you like me
to fork your dt-schema repo, add dt-schemas/schemas/mfd/syscon.yaml file similar
to this one with "*-endian" property supported, then pull-request or
send a patch with the alteration back to your repo?

> 
> I'd expect for any specific 'syscon', either none or only a subset of 
> these are valid, so I don't think this should be added here.

AFAIU mfd/syscon.yaml describes a generic syscon compatible with generic
driver drivers/mfd/syscon.c, which may have any of these properties
declared in its dt-node. We can't predict which one because, well, it's
generic. At the same time, yes, only a subset of these properties can be
supported by a specific system controller, which one can be determined
by the controller specific dt schema. So if we left the property here in
the generic syscon.yaml, then the controller dt-schema would have had a
pattern like:

>
> allOf:
>   - $ref: ../../mfd/syscon.yaml#
>
> properties:
>   little-endian: true
>
> additionalProperties: false
>

as I did for soc/baikal-t1/be,bt1-l2-ctl.yaml. See the patch: "dt-bindings:
Add Baikal-T1 L2-cache Control Block dts bindings file" in the corresponding
patchset in your email Inbox.

Regards,
-Sergey

> 
> Rob

[PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces SYSCON reboot-mode
legacy bare text bindings with YAML file. As before the bindings file
states that the corresponding dts node is supposed to be compatible
"syscon-reboot-mode" device and necessarily have an offset property
to determine which register from the regmap is supposed to keep the
mode on reboot.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../power/reset/syscon-reboot-mode.txt        | 35 ------------
 .../power/reset/syscon-reboot-mode.yaml       | 55 +++++++++++++++++++
 2 files changed, 55 insertions(+), 35 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
 create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
deleted file mode 100644
index f7ce1d8af04a..000000000000
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-SYSCON reboot mode driver
-
-This driver gets reboot mode magic value form reboot-mode driver
-and stores it in a SYSCON mapped register. Then the bootloader
-can read it and take different action according to the magic
-value stored.
-
-This DT node should be represented as a sub-node of a "syscon", "simple-mfd"
-node.
-
-Required properties:
-- compatible: should be "syscon-reboot-mode"
-- offset: offset in the register map for the storage register (in bytes)
-
-Optional property:
-- mask: bits mask of the bits in the register to store the reboot mode magic value,
-  default set to 0xffffffff if missing.
-
-The rest of the properties should follow the generic reboot-mode description
-found in reboot-mode.txt
-
-Example:
-	pmu: pmu@20004000 {
-		compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
-		reg = <0x20004000 0x100>;
-
-		reboot-mode {
-			compatible = "syscon-reboot-mode";
-			offset = <0x40>;
-			mode-normal = <BOOT_NORMAL>;
-			mode-recovery = <BOOT_RECOVERY>;
-			mode-bootloader = <BOOT_FASTBOOT>;
-			mode-loader = <BOOT_BL_DOWNLOAD>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
new file mode 100644
index 000000000000..e09bb07b1abb
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/reset/syscon-reboot-mode.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic SYSCON reboot mode driver
+
+maintainers:
+  - Sebastian Reichel <sre@kernel.org>
+
+description: |
+  This driver gets reboot mode magic value from reboot-mode driver
+  and stores it in a SYSCON mapped register. Then the bootloader
+  can read it and take different action according to the magic
+  value stored. The SYSCON mapped register is retrieved from the
+  parental dt-node plus the offset. So the SYSCON reboot-mode node
+  should be represented as a sub-node of a "syscon", "simple-mfd" node.
+
+properties:
+  compatible:
+    const: syscon-reboot-mode
+
+  mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Update only the register bits defined by the mask (32 bit).
+
+  offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Offset in the register map for the mode register (in bytes).
+
+patternProperties:
+  "^mode-.+":
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Vendor-specific mode value written to the mode register.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - offset
+
+examples:
+  - |
+    #include <dt-bindings/soc/rockchip,boot-mode.h>
+
+    reboot-mode {
+      compatible = "syscon-reboot-mode";
+      offset = <0x40>;
+      mode-normal = <BOOT_NORMAL>;
+      mode-recovery = <BOOT_RECOVERY>;
+      mode-bootloader = <BOOT_FASTBOOT>;
+      mode-loader = <BOOT_BL_DOWNLOAD>;
+    };
+...
-- 
2.25.1

Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5100 bytes --]

Hi,

On Fri, Mar 06, 2020 at 04:03:39PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces SYSCON reboot-mode
> legacy bare text bindings with YAML file. As before the bindings file
> states that the corresponding dts node is supposed to be compatible
> "syscon-reboot-mode" device and necessarily have an offset property
> to determine which register from the regmap is supposed to keep the
> mode on reboot.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---

I'm missing patch 1 and would like an Acked-by from Rob Herring, so
for now:

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  .../power/reset/syscon-reboot-mode.txt        | 35 ------------
>  .../power/reset/syscon-reboot-mode.yaml       | 55 +++++++++++++++++++
>  2 files changed, 55 insertions(+), 35 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> deleted file mode 100644
> index f7ce1d8af04a..000000000000
> --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -SYSCON reboot mode driver
> -
> -This driver gets reboot mode magic value form reboot-mode driver
> -and stores it in a SYSCON mapped register. Then the bootloader
> -can read it and take different action according to the magic
> -value stored.
> -
> -This DT node should be represented as a sub-node of a "syscon", "simple-mfd"
> -node.
> -
> -Required properties:
> -- compatible: should be "syscon-reboot-mode"
> -- offset: offset in the register map for the storage register (in bytes)
> -
> -Optional property:
> -- mask: bits mask of the bits in the register to store the reboot mode magic value,
> -  default set to 0xffffffff if missing.
> -
> -The rest of the properties should follow the generic reboot-mode description
> -found in reboot-mode.txt
> -
> -Example:
> -	pmu: pmu@20004000 {
> -		compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
> -		reg = <0x20004000 0x100>;
> -
> -		reboot-mode {
> -			compatible = "syscon-reboot-mode";
> -			offset = <0x40>;
> -			mode-normal = <BOOT_NORMAL>;
> -			mode-recovery = <BOOT_RECOVERY>;
> -			mode-bootloader = <BOOT_FASTBOOT>;
> -			mode-loader = <BOOT_BL_DOWNLOAD>;
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> new file mode 100644
> index 000000000000..e09bb07b1abb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/syscon-reboot-mode.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic SYSCON reboot mode driver
> +
> +maintainers:
> +  - Sebastian Reichel <sre@kernel.org>
> +
> +description: |
> +  This driver gets reboot mode magic value from reboot-mode driver
> +  and stores it in a SYSCON mapped register. Then the bootloader
> +  can read it and take different action according to the magic
> +  value stored. The SYSCON mapped register is retrieved from the
> +  parental dt-node plus the offset. So the SYSCON reboot-mode node
> +  should be represented as a sub-node of a "syscon", "simple-mfd" node.
> +
> +properties:
> +  compatible:
> +    const: syscon-reboot-mode
> +
> +  mask:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Update only the register bits defined by the mask (32 bit).
> +
> +  offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Offset in the register map for the mode register (in bytes).
> +
> +patternProperties:
> +  "^mode-.+":
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Vendor-specific mode value written to the mode register.
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - offset
> +
> +examples:
> +  - |
> +    #include <dt-bindings/soc/rockchip,boot-mode.h>
> +
> +    reboot-mode {
> +      compatible = "syscon-reboot-mode";
> +      offset = <0x40>;
> +      mode-normal = <BOOT_NORMAL>;
> +      mode-recovery = <BOOT_RECOVERY>;
> +      mode-bootloader = <BOOT_FASTBOOT>;
> +      mode-loader = <BOOT_BL_DOWNLOAD>;
> +    };
> +...
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one

[Sergey Semin]

On Fri, Mar 06, 2020 at 08:56:38PM +0100, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Mar 06, 2020 at 04:03:39PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Modern device tree bindings are supposed to be created as YAML-files
> > in accordance with dt-schema. This commit replaces SYSCON reboot-mode
> > legacy bare text bindings with YAML file. As before the bindings file
> > states that the corresponding dts node is supposed to be compatible
> > "syscon-reboot-mode" device and necessarily have an offset property
> > to determine which register from the regmap is supposed to keep the
> > mode on reboot.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> 
> I'm missing patch 1 and would like an Acked-by from Rob Herring, so
> for now:
> 
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 

Great. Thanks. I'll resend the patchset very soon. You aren't in the
first patch Cc because it doesn't concern power/reset subsystem, but
mfd/syscon. That's why my submission script didn't add you to the list.
Sorry about that. I'll send a v2 copy to you.

Regards,
-Sergey

> -- Sebastian
> 
> >  .../power/reset/syscon-reboot-mode.txt        | 35 ------------
> >  .../power/reset/syscon-reboot-mode.yaml       | 55 +++++++++++++++++++
> >  2 files changed, 55 insertions(+), 35 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> >  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> > deleted file mode 100644
> > index f7ce1d8af04a..000000000000
> > --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
> > +++ /dev/null
> > @@ -1,35 +0,0 @@
> > -SYSCON reboot mode driver
> > -
> > -This driver gets reboot mode magic value form reboot-mode driver
> > -and stores it in a SYSCON mapped register. Then the bootloader
> > -can read it and take different action according to the magic
> > -value stored.
> > -
> > -This DT node should be represented as a sub-node of a "syscon", "simple-mfd"
> > -node.
> > -
> > -Required properties:
> > -- compatible: should be "syscon-reboot-mode"
> > -- offset: offset in the register map for the storage register (in bytes)
> > -
> > -Optional property:
> > -- mask: bits mask of the bits in the register to store the reboot mode magic value,
> > -  default set to 0xffffffff if missing.
> > -
> > -The rest of the properties should follow the generic reboot-mode description
> > -found in reboot-mode.txt
> > -
> > -Example:
> > -	pmu: pmu@20004000 {
> > -		compatible = "rockchip,rk3066-pmu", "syscon", "simple-mfd";
> > -		reg = <0x20004000 0x100>;
> > -
> > -		reboot-mode {
> > -			compatible = "syscon-reboot-mode";
> > -			offset = <0x40>;
> > -			mode-normal = <BOOT_NORMAL>;
> > -			mode-recovery = <BOOT_RECOVERY>;
> > -			mode-bootloader = <BOOT_FASTBOOT>;
> > -			mode-loader = <BOOT_BL_DOWNLOAD>;
> > -		};
> > -	};
> > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > new file mode 100644
> > index 000000000000..e09bb07b1abb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/reset/syscon-reboot-mode.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic SYSCON reboot mode driver
> > +
> > +maintainers:
> > +  - Sebastian Reichel <sre@kernel.org>
> > +
> > +description: |
> > +  This driver gets reboot mode magic value from reboot-mode driver
> > +  and stores it in a SYSCON mapped register. Then the bootloader
> > +  can read it and take different action according to the magic
> > +  value stored. The SYSCON mapped register is retrieved from the
> > +  parental dt-node plus the offset. So the SYSCON reboot-mode node
> > +  should be represented as a sub-node of a "syscon", "simple-mfd" node.
> > +
> > +properties:
> > +  compatible:
> > +    const: syscon-reboot-mode
> > +
> > +  mask:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Update only the register bits defined by the mask (32 bit).
> > +
> > +  offset:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Offset in the register map for the mode register (in bytes).
> > +
> > +patternProperties:
> > +  "^mode-.+":
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Vendor-specific mode value written to the mode register.
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - offset
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/soc/rockchip,boot-mode.h>
> > +
> > +    reboot-mode {
> > +      compatible = "syscon-reboot-mode";
> > +      offset = <0x40>;
> > +      mode-normal = <BOOT_NORMAL>;
> > +      mode-recovery = <BOOT_RECOVERY>;
> > +      mode-bootloader = <BOOT_FASTBOOT>;
> > +      mode-loader = <BOOT_BL_DOWNLOAD>;
> > +    };
> > +...
> > -- 
> > 2.25.1
> > 

Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one

[Rob Herring]

On Fri, 6 Mar 2020 16:03:39 +0300, <Sergey.Semin@baikalelectronics.ru> wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces SYSCON reboot-mode
> legacy bare text bindings with YAML file. As before the bindings file
> states that the corresponding dts node is supposed to be compatible
> "syscon-reboot-mode" device and necessarily have an offset property
> to determine which register from the regmap is supposed to keep the
> mode on reboot.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  .../power/reset/syscon-reboot-mode.txt        | 35 ------------
>  .../power/reset/syscon-reboot-mode.yaml       | 55 +++++++++++++++++++
>  2 files changed, 55 insertions(+), 35 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.txt
>  create mode 100644 Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Optional regmap property will be used to refer to a syscon-controller
having a reboot tolerant register mapped.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../bindings/power/reset/syscon-reboot-mode.yaml         | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
index e09bb07b1abb..f47bf52ad983 100644
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
@@ -13,9 +13,8 @@ description: |
   This driver gets reboot mode magic value from reboot-mode driver
   and stores it in a SYSCON mapped register. Then the bootloader
   can read it and take different action according to the magic
-  value stored. The SYSCON mapped register is retrieved from the
-  parental dt-node plus the offset. So the SYSCON reboot-mode node
-  should be represented as a sub-node of a "syscon", "simple-mfd" node.
+  value stored. The SYSCON mapped register is retrieved either from
+  the parental dt-node or from a regmap phandle plus the offset.
 
 properties:
   compatible:
@@ -29,6 +28,10 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: Offset in the register map for the mode register (in bytes).
 
+  regmap:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Phandle to the register map node.
+
 patternProperties:
   "^mode-.+":
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.25.1

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

Hi,

On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Optional regmap property will be used to refer to a syscon-controller
> having a reboot tolerant register mapped.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  .../bindings/power/reset/syscon-reboot-mode.yaml         | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> index e09bb07b1abb..f47bf52ad983 100644
> --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> @@ -13,9 +13,8 @@ description: |
>    This driver gets reboot mode magic value from reboot-mode driver
>    and stores it in a SYSCON mapped register. Then the bootloader
>    can read it and take different action according to the magic
> -  value stored. The SYSCON mapped register is retrieved from the
> -  parental dt-node plus the offset. So the SYSCON reboot-mode node
> -  should be represented as a sub-node of a "syscon", "simple-mfd" node.
> +  value stored. The SYSCON mapped register is retrieved either from
> +  the parental dt-node or from a regmap phandle plus the offset.
>  
>  properties:
>    compatible:
> @@ -29,6 +28,10 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: Offset in the register map for the mode register (in bytes).
>  
> +  regmap:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the register map node.
> +
>  patternProperties:
>    "^mode-.+":
>      $ref: /schemas/types.yaml#/definitions/uint32
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Rob Herring]

On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Optional regmap property will be used to refer to a syscon-controller
> having a reboot tolerant register mapped.

NAK. It should simply be a child node of the 'syscon-controller'.

> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  .../bindings/power/reset/syscon-reboot-mode.yaml         | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> index e09bb07b1abb..f47bf52ad983 100644
> --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> @@ -13,9 +13,8 @@ description: |
>    This driver gets reboot mode magic value from reboot-mode driver
>    and stores it in a SYSCON mapped register. Then the bootloader
>    can read it and take different action according to the magic
> -  value stored. The SYSCON mapped register is retrieved from the
> -  parental dt-node plus the offset. So the SYSCON reboot-mode node
> -  should be represented as a sub-node of a "syscon", "simple-mfd" node.
> +  value stored. The SYSCON mapped register is retrieved either from
> +  the parental dt-node or from a regmap phandle plus the offset.
>  
>  properties:
>    compatible:
> @@ -29,6 +28,10 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: Offset in the register map for the mode register (in bytes).
>  
> +  regmap:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the register map node.
> +
>  patternProperties:
>    "^mode-.+":
>      $ref: /schemas/types.yaml#/definitions/uint32
> -- 
> 2.25.1
> 

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sergey Semin]

On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Optional regmap property will be used to refer to a syscon-controller
> > having a reboot tolerant register mapped.
> 
> NAK. It should simply be a child node of the 'syscon-controller'.

Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
So the code change will be merged while the doc-part won't? Lets discuss then
to settle the issue.

Why 'syscon-reboot' can be out of syscon-controller node, while
'syscon-reboot-mode' can't? They both belong to the same usecase: save
cause id and reboot. So having similar properties-set and declaring their
nodes someplace nearby is natural. According to the driver 'syscon-reboot'
can't lack the regmap property because it's mandatory, while here you refuse
to have even optional support. Additionally in most of the cases the
'syscon-reboot' nodes aren't declared as a child of a system controller
node. Why 'syscon-reboot-mode' can't work in a similar way?

Regards,
-Sergey

> 
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  .../bindings/power/reset/syscon-reboot-mode.yaml         | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > index e09bb07b1abb..f47bf52ad983 100644
> > --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
> > @@ -13,9 +13,8 @@ description: |
> >    This driver gets reboot mode magic value from reboot-mode driver
> >    and stores it in a SYSCON mapped register. Then the bootloader
> >    can read it and take different action according to the magic
> > -  value stored. The SYSCON mapped register is retrieved from the
> > -  parental dt-node plus the offset. So the SYSCON reboot-mode node
> > -  should be represented as a sub-node of a "syscon", "simple-mfd" node.
> > +  value stored. The SYSCON mapped register is retrieved either from
> > +  the parental dt-node or from a regmap phandle plus the offset.
> >  
> >  properties:
> >    compatible:
> > @@ -29,6 +28,10 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/uint32
> >      description: Offset in the register map for the mode register (in bytes).
> >  
> > +  regmap:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Phandle to the register map node.
> > +
> >  patternProperties:
> >    "^mode-.+":
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > -- 
> > 2.25.1
> > 

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

Hi,

On Fri, Mar 13, 2020 at 04:02:31PM +0300, Sergey Semin wrote:
> On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Optional regmap property will be used to refer to a syscon-controller
> > > having a reboot tolerant register mapped.
> > 
> > NAK. It should simply be a child node of the 'syscon-controller'.
> 
> Hm, It's dilemma. The driver maintainer said ack, while you
> disagree. So the code change will be merged while the doc-part
> won't?

FWIW I do not merge with bindings being NAK'd by Rob.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Rob Herring]

On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > Optional regmap property will be used to refer to a syscon-controller
> > > having a reboot tolerant register mapped.
> >
> > NAK. It should simply be a child node of the 'syscon-controller'.
>
> Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
> So the code change will be merged while the doc-part won't? Lets discuss then
> to settle the issue.
>
> Why 'syscon-reboot' can be out of syscon-controller node, while
> 'syscon-reboot-mode' can't?

Look at the history and you will see one was reviewed by DT
maintainers and one wasn't.

> They both belong to the same usecase: save
> cause id and reboot. So having similar properties-set and declaring their
> nodes someplace nearby is natural.

Which is what I'm asking for. Where else in the tree does it make
sense to locate the 'syscon-reboot-mode' node? Locate nodes where they
logically belong.

> According to the driver 'syscon-reboot'
> can't lack the regmap property because it's mandatory, while here you refuse
> to have even optional support. Additionally in most of the cases the
> 'syscon-reboot' nodes aren't declared as a child of a system controller
> node. Why 'syscon-reboot-mode' can't work in a similar way?

There's plenty of bad or "don't follow current best practice" examples
in the tree for all sorts of things. That is not a reason for doing
something in a new binding or adding to an existing one.

Rob

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sergey Semin]

On Wed, Mar 18, 2020 at 05:14:25PM -0600, Rob Herring wrote:
> On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > >
> > > > Optional regmap property will be used to refer to a syscon-controller
> > > > having a reboot tolerant register mapped.
> > >
> > > NAK. It should simply be a child node of the 'syscon-controller'.
> >
> > Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
> > So the code change will be merged while the doc-part won't? Lets discuss then
> > to settle the issue.
> >
> > Why 'syscon-reboot' can be out of syscon-controller node, while
> > 'syscon-reboot-mode' can't?
> 
> Look at the history and you will see one was reviewed by DT
> maintainers and one wasn't.
> 
> > They both belong to the same usecase: save
> > cause id and reboot. So having similar properties-set and declaring their
> > nodes someplace nearby is natural.
> 
> Which is what I'm asking for. Where else in the tree does it make
> sense to locate the 'syscon-reboot-mode' node? Locate nodes where they
> logically belong.
> 
> > According to the driver 'syscon-reboot'
> > can't lack the regmap property because it's mandatory, while here you refuse
> > to have even optional support. Additionally in most of the cases the
> > 'syscon-reboot' nodes aren't declared as a child of a system controller
> > node. Why 'syscon-reboot-mode' can't work in a similar way?
> 
> There's plenty of bad or "don't follow current best practice" examples
> in the tree for all sorts of things. That is not a reason for doing
> something in a new binding or adding to an existing one.
> 
> Rob

Alright. I see your point. What about I'd provide a sort of opposite
implementation? I could make the "regmap"-phandle reference being optional
in the !"syscon-reboot"! driver instead of adding the regmap-property
support to the "syscon-reboot-mode" driver. So if regmap property isn't
defined in the "syscon-reboot"-compatible node, the driver will try to
get a syscon regmap from the parental node as it's done in the
"syscon-reboot-mode" driver.

Seeing you think that regmap-property-based design is a bad practice in
this case, I also could mark the property as deprecated in the "syscon-reboot"
dt schema and print a warning from the "syscon-reboot" driver if one is defined.

What do you think?

Regards,
-Sergey

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sergey Semin]

Rob,
Any comment on my suggestion below?

Regards,
-Sergey

On Tue, Mar 31, 2020 at 10:50:53PM +0300, Sergey Semin wrote:
> On Wed, Mar 18, 2020 at 05:14:25PM -0600, Rob Herring wrote:
> > On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > > > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > >
> > > > > Optional regmap property will be used to refer to a syscon-controller
> > > > > having a reboot tolerant register mapped.
> > > >
> > > > NAK. It should simply be a child node of the 'syscon-controller'.
> > >
> > > Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
> > > So the code change will be merged while the doc-part won't? Lets discuss then
> > > to settle the issue.
> > >
> > > Why 'syscon-reboot' can be out of syscon-controller node, while
> > > 'syscon-reboot-mode' can't?
> > 
> > Look at the history and you will see one was reviewed by DT
> > maintainers and one wasn't.
> > 
> > > They both belong to the same usecase: save
> > > cause id and reboot. So having similar properties-set and declaring their
> > > nodes someplace nearby is natural.
> > 
> > Which is what I'm asking for. Where else in the tree does it make
> > sense to locate the 'syscon-reboot-mode' node? Locate nodes where they
> > logically belong.
> > 
> > > According to the driver 'syscon-reboot'
> > > can't lack the regmap property because it's mandatory, while here you refuse
> > > to have even optional support. Additionally in most of the cases the
> > > 'syscon-reboot' nodes aren't declared as a child of a system controller
> > > node. Why 'syscon-reboot-mode' can't work in a similar way?
> > 
> > There's plenty of bad or "don't follow current best practice" examples
> > in the tree for all sorts of things. That is not a reason for doing
> > something in a new binding or adding to an existing one.
> > 
> > Rob
> 
> Alright. I see your point. What about I'd provide a sort of opposite
> implementation? I could make the "regmap"-phandle reference being optional
> in the !"syscon-reboot"! driver instead of adding the regmap-property
> support to the "syscon-reboot-mode" driver. So if regmap property isn't
> defined in the "syscon-reboot"-compatible node, the driver will try to
> get a syscon regmap from the parental node as it's done in the
> "syscon-reboot-mode" driver.
> 
> Seeing you think that regmap-property-based design is a bad practice in
> this case, I also could mark the property as deprecated in the "syscon-reboot"
> dt schema and print a warning from the "syscon-reboot" driver if one is defined.
> 
> What do you think?
> 
> Regards,
> -Sergey

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Rob Herring]

On Thu, Apr 16, 2020 at 10:56:20PM +0300, Sergey Semin wrote:
> Rob,
> Any comment on my suggestion below?
> 
> Regards,
> -Sergey
> 
> On Tue, Mar 31, 2020 at 10:50:53PM +0300, Sergey Semin wrote:
> > On Wed, Mar 18, 2020 at 05:14:25PM -0600, Rob Herring wrote:
> > > On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin
> > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >
> > > > On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > > > > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > > > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > > >
> > > > > > Optional regmap property will be used to refer to a syscon-controller
> > > > > > having a reboot tolerant register mapped.
> > > > >
> > > > > NAK. It should simply be a child node of the 'syscon-controller'.
> > > >
> > > > Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
> > > > So the code change will be merged while the doc-part won't? Lets discuss then
> > > > to settle the issue.
> > > >
> > > > Why 'syscon-reboot' can be out of syscon-controller node, while
> > > > 'syscon-reboot-mode' can't?
> > > 
> > > Look at the history and you will see one was reviewed by DT
> > > maintainers and one wasn't.
> > > 
> > > > They both belong to the same usecase: save
> > > > cause id and reboot. So having similar properties-set and declaring their
> > > > nodes someplace nearby is natural.
> > > 
> > > Which is what I'm asking for. Where else in the tree does it make
> > > sense to locate the 'syscon-reboot-mode' node? Locate nodes where they
> > > logically belong.
> > > 
> > > > According to the driver 'syscon-reboot'
> > > > can't lack the regmap property because it's mandatory, while here you refuse
> > > > to have even optional support. Additionally in most of the cases the
> > > > 'syscon-reboot' nodes aren't declared as a child of a system controller
> > > > node. Why 'syscon-reboot-mode' can't work in a similar way?
> > > 
> > > There's plenty of bad or "don't follow current best practice" examples
> > > in the tree for all sorts of things. That is not a reason for doing
> > > something in a new binding or adding to an existing one.
> > > 
> > > Rob
> > 
> > Alright. I see your point. What about I'd provide a sort of opposite
> > implementation? I could make the "regmap"-phandle reference being optional
> > in the !"syscon-reboot"! driver instead of adding the regmap-property
> > support to the "syscon-reboot-mode" driver. So if regmap property isn't
> > defined in the "syscon-reboot"-compatible node, the driver will try to
> > get a syscon regmap from the parental node as it's done in the
> > "syscon-reboot-mode" driver.

That seems fine.

> > Seeing you think that regmap-property-based design is a bad practice in
> > this case, I also could mark the property as deprecated in the "syscon-reboot"
> > dt schema and print a warning from the "syscon-reboot" driver if one is defined.

Depends on how many platforms will start getting warnings. I think just 
marking deprecated is enough.

Rob

Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings

[Sergey Semin]

On Thu, Apr 16, 2020 at 04:28:42PM -0500, Rob Herring wrote:
> On Thu, Apr 16, 2020 at 10:56:20PM +0300, Sergey Semin wrote:
> > Rob,
> > Any comment on my suggestion below?
> > 
> > Regards,
> > -Sergey
> > 
> > On Tue, Mar 31, 2020 at 10:50:53PM +0300, Sergey Semin wrote:
> > > On Wed, Mar 18, 2020 at 05:14:25PM -0600, Rob Herring wrote:
> > > > On Fri, Mar 13, 2020 at 7:03 AM Sergey Semin
> > > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > > >
> > > > > On Thu, Mar 12, 2020 at 04:14:38PM -0500, Rob Herring wrote:
> > > > > > On Fri, Mar 06, 2020 at 04:03:40PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > > > > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > > > >
> > > > > > > Optional regmap property will be used to refer to a syscon-controller
> > > > > > > having a reboot tolerant register mapped.
> > > > > >
> > > > > > NAK. It should simply be a child node of the 'syscon-controller'.
> > > > >
> > > > > Hm, It's dilemma. The driver maintainer said ack, while you disagree.)
> > > > > So the code change will be merged while the doc-part won't? Lets discuss then
> > > > > to settle the issue.
> > > > >
> > > > > Why 'syscon-reboot' can be out of syscon-controller node, while
> > > > > 'syscon-reboot-mode' can't?
> > > > 
> > > > Look at the history and you will see one was reviewed by DT
> > > > maintainers and one wasn't.
> > > > 
> > > > > They both belong to the same usecase: save
> > > > > cause id and reboot. So having similar properties-set and declaring their
> > > > > nodes someplace nearby is natural.
> > > > 
> > > > Which is what I'm asking for. Where else in the tree does it make
> > > > sense to locate the 'syscon-reboot-mode' node? Locate nodes where they
> > > > logically belong.
> > > > 
> > > > > According to the driver 'syscon-reboot'
> > > > > can't lack the regmap property because it's mandatory, while here you refuse
> > > > > to have even optional support. Additionally in most of the cases the
> > > > > 'syscon-reboot' nodes aren't declared as a child of a system controller
> > > > > node. Why 'syscon-reboot-mode' can't work in a similar way?
> > > > 
> > > > There's plenty of bad or "don't follow current best practice" examples
> > > > in the tree for all sorts of things. That is not a reason for doing
> > > > something in a new binding or adding to an existing one.
> > > > 
> > > > Rob
> > > 
> > > Alright. I see your point. What about I'd provide a sort of opposite
> > > implementation? I could make the "regmap"-phandle reference being optional
> > > in the !"syscon-reboot"! driver instead of adding the regmap-property
> > > support to the "syscon-reboot-mode" driver. So if regmap property isn't
> > > defined in the "syscon-reboot"-compatible node, the driver will try to
> > > get a syscon regmap from the parental node as it's done in the
> > > "syscon-reboot-mode" driver.
> 
> That seems fine.
> 
> > > Seeing you think that regmap-property-based design is a bad practice in
> > > this case, I also could mark the property as deprecated in the "syscon-reboot"
> > > dt schema and print a warning from the "syscon-reboot" driver if one is defined.
> 
> Depends on how many platforms will start getting warnings. I think just 
> marking deprecated is enough.

Ok. Thanks. I'll do this in v2.

Regards,
-Sergey

> 
> Rob