diff mbox series

[v2,1/9] dt-bindings: arm: mstar: Add binding details for mstar,pmsleep

Message ID 20200728100321.1691745-2-daniel@0x0f.com (mailing list archive)
State Mainlined
Commit 33cabc0bc679022160c48f96e2d358e666710f58
Headers show
Series ARM: mstar: DT filling out | expand

Commit Message

Daniel Palmer July 28, 2020, 10:03 a.m. UTC
This adds a YAML description of the pmsleep node used by
MStar/SigmaStar Armv7 SoCs.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 .../bindings/arm/mstar/mstar,pmsleep.yaml     | 43 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml

Comments

Rob Herring (Arm) July 28, 2020, 7:16 p.m. UTC | #1
On Tue, 28 Jul 2020 19:03:13 +0900, Daniel Palmer wrote:
> This adds a YAML description of the pmsleep node used by
> MStar/SigmaStar Armv7 SoCs.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  .../bindings/arm/mstar/mstar,pmsleep.yaml     | 43 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

Error: Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.example.dts:21.23-24 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:315: recipe for target 'Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1347: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1337730

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.
Rob Herring (Arm) July 28, 2020, 7:18 p.m. UTC | #2
On Tue, Jul 28, 2020 at 07:03:13PM +0900, Daniel Palmer wrote:
> This adds a YAML description of the pmsleep node used by
> MStar/SigmaStar Armv7 SoCs.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  .../bindings/arm/mstar/mstar,pmsleep.yaml     | 43 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml b/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
> new file mode 100644
> index 000000000000..ef78097a7087
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 thingy.jp.
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/mstar/mstar,pmsleep.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: MStar/SigmaStar Armv7 SoC pmsleep register region
> +
> +maintainers:
> +  - Daniel Palmer <daniel@thingy.jp>
> +
> +description: |
> +  MStar/Sigmastar's Armv7 SoCs contain a region of registers that are
> +  in the always on domain that the vendor code calls the "pmsleep" area.
> +
> +  This area contains registers and bits for a broad range of functionality
> +  ranging from registers that control going into deep sleep to bits that
> +  turn things like the internal temperature sensor on and off.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +         - enum:
> +             - mstar,pmsleep

Needs to be SoC specific. Random collections of bits are never 
'standard' from one SoC to the next.

If your never going to have child nodes, then you can just add the 
compatible to syscon.yaml.

> +         - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pmsleep: pmsleep@1c00 {
> +        compatible = "mstar,pmsleep", "syscon";
> +        reg = <0x0x1c00 0x100>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 991814ea6f76..432fcc867ed6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2140,6 +2140,7 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained
>  W:	http://linux-chenxing.org/
>  F:	Documentation/devicetree/bindings/arm/mstar.yaml
> +F:	Documentation/devicetree/bindings/arm/mstar/*
>  F:	arch/arm/boot/dts/infinity*.dtsi
>  F:	arch/arm/boot/dts/mercury*.dtsi
>  F:	arch/arm/boot/dts/mstar-v7.dtsi
> -- 
> 2.27.0
>
Daniel Palmer July 29, 2020, 9:13 a.m. UTC | #3
Hi Rob,

On Wed, 29 Jul 2020 at 04:18, Rob Herring <robh@kernel.org> wrote:

> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +         - enum:
> > +             - mstar,pmsleep
>
> Needs to be SoC specific. Random collections of bits are never
> 'standard' from one SoC to the next.

I don't have a manual for any of the chips so I can't say for sure but
so far all of the chips in this group (ARMv7 based MStar/Sigmastar)
has the same layout for the registers i.e. the reset register,
the resume address registers are at the same place for all of them.

Does calling it "mstar,pmsleepv7" make more sense? I'm not sure what
to call it really.

> If your never going to have child nodes, then you can just add the
> compatible to syscon.yaml.

Considering the above would it make sense to drop the specific
compatible string for now and just leave it as syscon until there is a reason
to make it more specific?

Thanks,

Daniel
Arnd Bergmann July 29, 2020, 12:13 p.m. UTC | #4
On Wed, Jul 29, 2020 at 11:14 AM Daniel Palmer <daniel@0x0f.com> wrote:
> On Wed, 29 Jul 2020 at 04:18, Rob Herring <robh@kernel.org> wrote:
>
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +         - enum:
> > > +             - mstar,pmsleep
> >
> > Needs to be SoC specific. Random collections of bits are never
> > 'standard' from one SoC to the next.
>
> I don't have a manual for any of the chips so I can't say for sure but
> so far all of the chips in this group (ARMv7 based MStar/Sigmastar)
> has the same layout for the registers i.e. the reset register,
> the resume address registers are at the same place for all of them.
>
> Does calling it "mstar,pmsleepv7" make more sense? I'm not sure what
> to call it really.

Use the name of the oldest chip you know that supports it in there,
such as "mstar,msc313-pmsleep" if this one is specific to msc313.

       Arnd
Daniel Palmer July 29, 2020, 12:34 p.m. UTC | #5
Hi Arnd,

On Wed, 29 Jul 2020 at 21:14, Arnd Bergmann <arnd@arndb.de> wrote:
> > Does calling it "mstar,pmsleepv7" make more sense? I'm not sure what
> > to call it really.
>
> Use the name of the oldest chip you know that supports it in there,
> such as "mstar,msc313-pmsleep" if this one is specific to msc313.

That makes sense. I think the original patch got merged to soc/arm/newsoc.
Should I recreate the series or create a new patch to do the corrections?

Slightly off topic but I'm working on the series for the interrupt controller
and I've just renamed it from mstar,msc313e-intc to mstar,v7intc.
I originally called it msc313e because I only knew of that chip but the
same controller is present at the same place in all of the chips so far.
I guess I should probably rename it to mstar,msc313-intc to keep with
the first chip it appeared in pattern?

Thanks,

Daniel
Rob Herring (Arm) July 29, 2020, 2:11 p.m. UTC | #6
On Wed, Jul 29, 2020 at 6:34 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> Hi Arnd,
>
> On Wed, 29 Jul 2020 at 21:14, Arnd Bergmann <arnd@arndb.de> wrote:
> > > Does calling it "mstar,pmsleepv7" make more sense? I'm not sure what
> > > to call it really.
> >
> > Use the name of the oldest chip you know that supports it in there,
> > such as "mstar,msc313-pmsleep" if this one is specific to msc313.
>
> That makes sense. I think the original patch got merged to soc/arm/newsoc.
> Should I recreate the series or create a new patch to do the corrections?
>
> Slightly off topic but I'm working on the series for the interrupt controller
> and I've just renamed it from mstar,msc313e-intc to mstar,v7intc.
> I originally called it msc313e because I only knew of that chip but the
> same controller is present at the same place in all of the chips so far.
> I guess I should probably rename it to mstar,msc313-intc to keep with
> the first chip it appeared in pattern?

Yes.

Rob
Arnd Bergmann July 29, 2020, 2:17 p.m. UTC | #7
On Wed, Jul 29, 2020 at 2:34 PM Daniel Palmer <daniel@0x0f.com> wrote:
>
> On Wed, 29 Jul 2020 at 21:14, Arnd Bergmann <arnd@arndb.de> wrote:
> > > Does calling it "mstar,pmsleepv7" make more sense? I'm not sure what
> > > to call it really.
> >
> > Use the name of the oldest chip you know that supports it in there,
> > such as "mstar,msc313-pmsleep" if this one is specific to msc313.
>
> That makes sense. I think the original patch got merged to soc/arm/newsoc.
> Should I recreate the series or create a new patch to do the corrections?

Please send an incremental patch.

> Slightly off topic but I'm working on the series for the interrupt controller
> and I've just renamed it from mstar,msc313e-intc to mstar,v7intc.
> I originally called it msc313e because I only knew of that chip but the
> same controller is present at the same place in all of the chips so far.
> I guess I should probably rename it to mstar,msc313-intc to keep with
> the first chip it appeared in pattern?

Yes, correct. If you have multiple chips using this controller, use the
name of the oldest chip as the generic identifier and then add a more
specific one for each the later chips that also use it, so the driver is
able to tell the difference if it ever needs to, something like:

(on msc313)
compatible = "mstar,msc313-intc";

(on msc314)
compatible = "mstar,msc314-intc", "mstar,msc313-intc";

(on msc315)
compatible = "mstar,msc315-intc", "mstar,msc313-intc";

   Arnd
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml b/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
new file mode 100644
index 000000000000..ef78097a7087
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mstar/mstar,pmsleep.yaml
@@ -0,0 +1,43 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 thingy.jp.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/arm/mstar/mstar,pmsleep.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MStar/SigmaStar Armv7 SoC pmsleep register region
+
+maintainers:
+  - Daniel Palmer <daniel@thingy.jp>
+
+description: |
+  MStar/Sigmastar's Armv7 SoCs contain a region of registers that are
+  in the always on domain that the vendor code calls the "pmsleep" area.
+
+  This area contains registers and bits for a broad range of functionality
+  ranging from registers that control going into deep sleep to bits that
+  turn things like the internal temperature sensor on and off.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+         - enum:
+             - mstar,pmsleep
+         - const: syscon
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    pmsleep: pmsleep@1c00 {
+        compatible = "mstar,pmsleep", "syscon";
+        reg = <0x0x1c00 0x100>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 991814ea6f76..432fcc867ed6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2140,6 +2140,7 @@  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 W:	http://linux-chenxing.org/
 F:	Documentation/devicetree/bindings/arm/mstar.yaml
+F:	Documentation/devicetree/bindings/arm/mstar/*
 F:	arch/arm/boot/dts/infinity*.dtsi
 F:	arch/arm/boot/dts/mercury*.dtsi
 F:	arch/arm/boot/dts/mstar-v7.dtsi