* [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one [not found] <20200306130341.9585-1-Sergey.Semin@baikalelectronics.ru> @ 2020-03-06 13:03 ` Sergey.Semin 2020-03-06 19:56 ` Sebastian Reichel ` (2 more replies) 2020-03-06 13:03 ` [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings Sergey.Semin 2020-03-06 13:03 ` [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support Sergey.Semin 2 siblings, 3 replies; 16+ messages in thread From: Sergey.Semin @ 2020-03-06 13:03 UTC (permalink / raw) To: Sebastian Reichel, Rob Herring, Mark Rutland Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one 2020-03-06 13:03 ` [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one Sergey.Semin @ 2020-03-06 19:56 ` Sebastian Reichel [not found] ` <20200306200551.49C47803087C@mail.baikalelectronics.ru> 2020-03-12 21:12 ` Rob Herring 2 siblings, 0 replies; 16+ messages in thread From: Sebastian Reichel @ 2020-03-06 19:56 UTC (permalink / raw) To: Sergey.Semin Cc: Rob Herring, Mark Rutland, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20200306200551.49C47803087C@mail.baikalelectronics.ru>]
* Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one [not found] ` <20200306200551.49C47803087C@mail.baikalelectronics.ru> @ 2020-03-11 20:47 ` Sergey Semin 0 siblings, 0 replies; 16+ messages in thread From: Sergey Semin @ 2020-03-11 20:47 UTC (permalink / raw) To: Sebastian Reichel Cc: Rob Herring, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one 2020-03-06 13:03 ` [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one Sergey.Semin 2020-03-06 19:56 ` Sebastian Reichel [not found] ` <20200306200551.49C47803087C@mail.baikalelectronics.ru> @ 2020-03-12 21:12 ` Rob Herring 2 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2020-03-12 21:12 UTC (permalink / raw) To: Sergey.Semin Cc: Sebastian Reichel, Mark Rutland, Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings [not found] <20200306130341.9585-1-Sergey.Semin@baikalelectronics.ru> 2020-03-06 13:03 ` [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one Sergey.Semin @ 2020-03-06 13:03 ` Sergey.Semin 2020-03-06 19:57 ` Sebastian Reichel 2020-03-12 21:14 ` Rob Herring 2020-03-06 13:03 ` [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support Sergey.Semin 2 siblings, 2 replies; 16+ messages in thread From: Sergey.Semin @ 2020-03-06 13:03 UTC (permalink / raw) To: Sebastian Reichel, Rob Herring, Mark Rutland Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-06 13:03 ` [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings Sergey.Semin @ 2020-03-06 19:57 ` Sebastian Reichel 2020-03-12 21:14 ` Rob Herring 1 sibling, 0 replies; 16+ messages in thread From: Sebastian Reichel @ 2020-03-06 19:57 UTC (permalink / raw) To: Sergey.Semin Cc: Rob Herring, Mark Rutland, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-06 13:03 ` [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings Sergey.Semin 2020-03-06 19:57 ` Sebastian Reichel @ 2020-03-12 21:14 ` Rob Herring 2020-03-13 13:02 ` Sergey Semin 1 sibling, 1 reply; 16+ messages in thread From: Rob Herring @ 2020-03-12 21:14 UTC (permalink / raw) To: Sergey.Semin Cc: Sebastian Reichel, Mark Rutland, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-12 21:14 ` Rob Herring @ 2020-03-13 13:02 ` Sergey Semin 2020-03-14 18:04 ` Sebastian Reichel 2020-03-18 23:14 ` Rob Herring 0 siblings, 2 replies; 16+ messages in thread From: Sergey Semin @ 2020-03-13 13:02 UTC (permalink / raw) To: Rob Herring Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-13 13:02 ` Sergey Semin @ 2020-03-14 18:04 ` Sebastian Reichel 2020-03-18 23:14 ` Rob Herring 1 sibling, 0 replies; 16+ messages in thread From: Sebastian Reichel @ 2020-03-14 18:04 UTC (permalink / raw) To: Sergey Semin Cc: Rob Herring, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-13 13:02 ` Sergey Semin 2020-03-14 18:04 ` Sebastian Reichel @ 2020-03-18 23:14 ` Rob Herring 2020-03-31 19:50 ` Sergey Semin 1 sibling, 1 reply; 16+ messages in thread From: Rob Herring @ 2020-03-18 23:14 UTC (permalink / raw) To: Sergey Semin Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, open list:THERMAL, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-18 23:14 ` Rob Herring @ 2020-03-31 19:50 ` Sergey Semin 2020-04-16 19:56 ` Sergey Semin 0 siblings, 1 reply; 16+ messages in thread From: Sergey Semin @ 2020-03-31 19:50 UTC (permalink / raw) To: Rob Herring Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, open list:THERMAL, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-03-31 19:50 ` Sergey Semin @ 2020-04-16 19:56 ` Sergey Semin 2020-04-16 21:28 ` Rob Herring 0 siblings, 1 reply; 16+ messages in thread From: Sergey Semin @ 2020-04-16 19:56 UTC (permalink / raw) To: Rob Herring Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, open list:THERMAL, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-04-16 19:56 ` Sergey Semin @ 2020-04-16 21:28 ` Rob Herring 2020-04-17 7:45 ` Sergey Semin 0 siblings, 1 reply; 16+ messages in thread From: Rob Herring @ 2020-04-16 21:28 UTC (permalink / raw) To: Sergey Semin Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, open list:THERMAL, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings 2020-04-16 21:28 ` Rob Herring @ 2020-04-17 7:45 ` Sergey Semin 0 siblings, 0 replies; 16+ messages in thread From: Sergey Semin @ 2020-04-17 7:45 UTC (permalink / raw) To: Rob Herring Cc: Sebastian Reichel, Mark Rutland, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, open list:THERMAL, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support [not found] <20200306130341.9585-1-Sergey.Semin@baikalelectronics.ru> 2020-03-06 13:03 ` [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one Sergey.Semin 2020-03-06 13:03 ` [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings Sergey.Semin @ 2020-03-06 13:03 ` Sergey.Semin 2020-03-06 19:57 ` Sebastian Reichel 2 siblings, 1 reply; 16+ messages in thread From: Sergey.Semin @ 2020-03-06 13:03 UTC (permalink / raw) To: Sebastian Reichel Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, linux-kernel From: Serge Semin <Sergey.Semin@baikalelectronics.ru> 'Reboot-mode'-type of devices are supposed to work in conjunction with 'reboot'-type devices. In particular Baikal-T1 SoC provides a special CCU_WDT_RCR register, which is preserved during any type of the CPU reset (standard and caused by a watchdog one). Since both of them are responsible for the system-wide operation and related with each other it would be better to place them at the same place in the dt hierarchy. In particular the best location would be the dt root node. Currently 'syscon-reboot' device node can be placed anywhere in a dtb as long as the corresponding device is created at the system boot-up procedure. While according to the corresponding bindings file 'system-boot-mode' should be represented as a sub-node of a "syscon", "simple-mfd" node. This isn't always suitable especially when the reboot-preserved register is provided by some device, which we don't want to declared as MFD. In this case it would be good to have the 'syscon-reboot-mode' node accepting the 'regmap' property with a phandle reference to the 'syscon' dt-node, in the same way the 'syscon-reboot' driver does. This is what this patch provides - it makes the driver to handle the optional 'regmap' property. In case if one isn't provided the previously implemented scheme is expected to be found in dtb. Moreover seeing current dt-interface implementation of the 'syscon-reboot', 'syscon-poweroff' and 'syscon-reboot-mode' drivers, they look more or less similar. All of them handle 'offset' and 'mask' dt-properties. While 'value' property is only acceptable by the 'syscon-reboot' and 'syscon-poweroff' driver, the 'mode-*' properties of 'syscon-reboot-mode' serve to the similar purpose. The only strong difference between them is the ability to get the syscon regmap from the 'regmap' property. By having this patch merged we'll have that difference eliminated, so the interfaces would look unified. 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> --- drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c index e0772c9f70f7..f8f8218ae3ee 100644 --- a/drivers/power/reset/syscon-reboot-mode.c +++ b/drivers/power/reset/syscon-reboot-mode.c @@ -40,6 +40,7 @@ static int syscon_reboot_mode_probe(struct platform_device *pdev) { int ret; struct syscon_reboot_mode *syscon_rbm; + struct regmap *map; syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL); if (!syscon_rbm) @@ -49,9 +50,13 @@ static int syscon_reboot_mode_probe(struct platform_device *pdev) syscon_rbm->reboot.write = syscon_reboot_mode_write; syscon_rbm->mask = 0xffffffff; - syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node); - if (IS_ERR(syscon_rbm->map)) - return PTR_ERR(syscon_rbm->map); + map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "regmap"); + if (IS_ERR(map)) { + map = syscon_node_to_regmap(pdev->dev.parent->of_node); + if (IS_ERR(map)) + return PTR_ERR(map); + } + syscon_rbm->map = map; if (of_property_read_u32(pdev->dev.of_node, "offset", &syscon_rbm->offset)) -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support 2020-03-06 13:03 ` [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support Sergey.Semin @ 2020-03-06 19:57 ` Sebastian Reichel 0 siblings, 0 replies; 16+ messages in thread From: Sebastian Reichel @ 2020-03-06 19:57 UTC (permalink / raw) To: Sergey.Semin Cc: Serge Semin, Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle, linux-pm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 3852 bytes --] Hi, On Fri, Mar 06, 2020 at 04:03:41PM +0300, Sergey.Semin@baikalelectronics.ru wrote: > From: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > 'Reboot-mode'-type of devices are supposed to work in conjunction with > 'reboot'-type devices. In particular Baikal-T1 SoC provides a special > CCU_WDT_RCR register, which is preserved during any type of the CPU > reset (standard and caused by a watchdog one). Since both of them are > responsible for the system-wide operation and related with each other > it would be better to place them at the same place in the dt hierarchy. > In particular the best location would be the dt root node. Currently > 'syscon-reboot' device node can be placed anywhere in a dtb as long as > the corresponding device is created at the system boot-up procedure. > While according to the corresponding bindings file 'system-boot-mode' > should be represented as a sub-node of a "syscon", "simple-mfd" node. > This isn't always suitable especially when the reboot-preserved > register is provided by some device, which we don't want to declared > as MFD. In this case it would be good to have the 'syscon-reboot-mode' > node accepting the 'regmap' property with a phandle reference to the > 'syscon' dt-node, in the same way the 'syscon-reboot' driver does. > This is what this patch provides - it makes the driver to handle the > optional 'regmap' property. In case if one isn't provided the > previously implemented scheme is expected to be found in dtb. > > Moreover seeing current dt-interface implementation of the > 'syscon-reboot', 'syscon-poweroff' and 'syscon-reboot-mode' drivers, > they look more or less similar. All of them handle 'offset' and > 'mask' dt-properties. While 'value' property is only acceptable > by the 'syscon-reboot' and 'syscon-poweroff' driver, the 'mode-*' > properties of 'syscon-reboot-mode' serve to the similar purpose. > The only strong difference between them is the ability to get the > syscon regmap from the 'regmap' property. By having this patch merged > we'll have that difference eliminated, so the interfaces would look > unified. > > 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 > drivers/power/reset/syscon-reboot-mode.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c > index e0772c9f70f7..f8f8218ae3ee 100644 > --- a/drivers/power/reset/syscon-reboot-mode.c > +++ b/drivers/power/reset/syscon-reboot-mode.c > @@ -40,6 +40,7 @@ static int syscon_reboot_mode_probe(struct platform_device *pdev) > { > int ret; > struct syscon_reboot_mode *syscon_rbm; > + struct regmap *map; > > syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL); > if (!syscon_rbm) > @@ -49,9 +50,13 @@ static int syscon_reboot_mode_probe(struct platform_device *pdev) > syscon_rbm->reboot.write = syscon_reboot_mode_write; > syscon_rbm->mask = 0xffffffff; > > - syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node); > - if (IS_ERR(syscon_rbm->map)) > - return PTR_ERR(syscon_rbm->map); > + map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "regmap"); > + if (IS_ERR(map)) { > + map = syscon_node_to_regmap(pdev->dev.parent->of_node); > + if (IS_ERR(map)) > + return PTR_ERR(map); > + } > + syscon_rbm->map = map; > > if (of_property_read_u32(pdev->dev.of_node, "offset", > &syscon_rbm->offset)) > -- > 2.25.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-04-17 7:45 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200306130341.9585-1-Sergey.Semin@baikalelectronics.ru> 2020-03-06 13:03 ` [PATCH 2/4] dt-bindings: power: reset: Replace SYSCON reboot-mode legacy bindings with YAML-based one Sergey.Semin 2020-03-06 19:56 ` Sebastian Reichel [not found] ` <20200306200551.49C47803087C@mail.baikalelectronics.ru> 2020-03-11 20:47 ` Sergey Semin 2020-03-12 21:12 ` Rob Herring 2020-03-06 13:03 ` [PATCH 3/4] dt-bindings: power: reset: Add regmap support to the SYSCON reboot-mode bindings Sergey.Semin 2020-03-06 19:57 ` Sebastian Reichel 2020-03-12 21:14 ` Rob Herring 2020-03-13 13:02 ` Sergey Semin 2020-03-14 18:04 ` Sebastian Reichel 2020-03-18 23:14 ` Rob Herring 2020-03-31 19:50 ` Sergey Semin 2020-04-16 19:56 ` Sergey Semin 2020-04-16 21:28 ` Rob Herring 2020-04-17 7:45 ` Sergey Semin 2020-03-06 13:03 ` [PATCH 4/4] power: reset: syscon-reboot-mode: Add regmap dts-property support Sergey.Semin 2020-03-06 19:57 ` Sebastian Reichel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).