diff mbox series

[v2,2/6] dt-bindings: soc: actions: Add Actions Semi Owl socinfo binding

Message ID 15da0257b10aa62bfb7046437915d05a614c01ee.1617110420.git.cristian.ciocaltea@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add support for Actions Semi Owl socinfo | expand

Commit Message

Cristian Ciocaltea March 30, 2021, 1:48 p.m. UTC 
Add devicetree binding for the Actions Semi Owl socinfo driver.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 .../bindings/soc/actions/owl-socinfo.yaml     | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml

Comments

Rob Herring (Arm) April 1, 2021, 5:08 p.m. UTC | #1 
On Tue, Mar 30, 2021 at 04:48:17PM +0300, Cristian Ciocaltea wrote:
> Add devicetree binding for the Actions Semi Owl socinfo driver.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> ---
>  .../bindings/soc/actions/owl-socinfo.yaml     | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> new file mode 100644
> index 000000000000..01e4a8b4f5ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Actions Semi Owl SoC info module
> +
> +maintainers:
> +  - Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> +
> +description: |
> +  Actions Semi Owl SoC info module provides access to various information
> +  about the S500, S700 and S900 SoC variants, such as serial number or id.
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - actions,s500-soc
> +          - actions,s700-soc
> +          - actions,s900-soc
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - actions,s500-soc
> +          - actions,s700-soc
> +          - actions,s900-soc
> +      - const: simple-bus
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    / {
> +        compatible = "roseapplepi,roseapplepi", "actions,s500";
> +        model = "Roseapple Pi";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        soc {
> +            compatible = "actions,s500-soc", "simple-bus";

What's the actual h/w for this bus? Still looks like abuse of DT to 
create your virtual soc_info driver.

> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges;
> +        };
> +    };
> +
> +...
> -- 
> 2.31.1
>
Cristian Ciocaltea April 1, 2021, 5:57 p.m. UTC | #2 
On Thu, Apr 01, 2021 at 12:08:18PM -0500, Rob Herring wrote:
> On Tue, Mar 30, 2021 at 04:48:17PM +0300, Cristian Ciocaltea wrote:
> > Add devicetree binding for the Actions Semi Owl socinfo driver.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > ---
> >  .../bindings/soc/actions/owl-socinfo.yaml     | 57 +++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > new file mode 100644
> > index 000000000000..01e4a8b4f5ac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Actions Semi Owl SoC info module
> > +
> > +maintainers:
> > +  - Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > +
> > +description: |
> > +  Actions Semi Owl SoC info module provides access to various information
> > +  about the S500, S700 and S900 SoC variants, such as serial number or id.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - actions,s500-soc
> > +          - actions,s700-soc
> > +          - actions,s900-soc
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - actions,s500-soc
> > +          - actions,s700-soc
> > +          - actions,s900-soc
> > +      - const: simple-bus
> > +
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    / {
> > +        compatible = "roseapplepi,roseapplepi", "actions,s500";
> > +        model = "Roseapple Pi";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        soc {
> > +            compatible = "actions,s500-soc", "simple-bus";
> 
> What's the actual h/w for this bus? Still looks like abuse of DT to 
> create your virtual soc_info driver.

Right, there is no bus involved in accessing soc info, but I assumed
the already existing soc node in common DTS is a good candidate for
binding the driver. (e.g. arch/arm/boot/dts/owl-s500.dtsi)

Should I, instead, create a dedicated sub-node?

Thanks,
Cristi

> 
> > +            #address-cells = <1>;
> > +            #size-cells = <1>;
> > +            ranges;
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.31.1
> >
Manivannan Sadhasivam April 2, 2021, 6:04 p.m. UTC | #3 
On Tue, Mar 30, 2021 at 04:48:17PM +0300, Cristian Ciocaltea wrote:
> Add devicetree binding for the Actions Semi Owl socinfo driver.
> 

Devicetree binding shouldn't be added for a driver instead for an IP or hw.

> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> ---
>  .../bindings/soc/actions/owl-socinfo.yaml     | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> new file mode 100644
> index 000000000000..01e4a8b4f5ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Actions Semi Owl SoC info module
> +
> +maintainers:
> +  - Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> +
> +description: |
> +  Actions Semi Owl SoC info module provides access to various information
> +  about the S500, S700 and S900 SoC variants, such as serial number or id.
> +

S700/S900 are not yet confirmed, so please avoid them.

> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - actions,s500-soc
> +          - actions,s700-soc
> +          - actions,s900-soc
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - actions,s500-soc
> +          - actions,s700-soc
> +          - actions,s900-soc
> +      - const: simple-bus
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    / {
> +        compatible = "roseapplepi,roseapplepi", "actions,s500";
> +        model = "Roseapple Pi";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        soc {
> +            compatible = "actions,s500-soc", "simple-bus";

No. This shouldn't fall under /soc. I think you should added a separate
compatible for the reserved memory itself and add a corresponding socinfo
driver under drivers/soc.

This way it is obvious that the information is contained in a memory region and
a driver is used for parsing that.

Thanks,
Mani

> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges;
> +        };
> +    };
> +
> +...
> -- 
> 2.31.1
>
Cristian Ciocaltea April 2, 2021, 7:25 p.m. UTC | #4 
On Fri, Apr 02, 2021 at 11:34:07PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Mar 30, 2021 at 04:48:17PM +0300, Cristian Ciocaltea wrote:
> > Add devicetree binding for the Actions Semi Owl socinfo driver.
> > 
> 
> Devicetree binding shouldn't be added for a driver instead for an IP or hw.
> 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > ---
> >  .../bindings/soc/actions/owl-socinfo.yaml     | 57 +++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > new file mode 100644
> > index 000000000000..01e4a8b4f5ac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Actions Semi Owl SoC info module
> > +
> > +maintainers:
> > +  - Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> > +
> > +description: |
> > +  Actions Semi Owl SoC info module provides access to various information
> > +  about the S500, S700 and S900 SoC variants, such as serial number or id.
> > +
> 
> S700/S900 are not yet confirmed, so please avoid them.
> 
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - actions,s500-soc
> > +          - actions,s700-soc
> > +          - actions,s900-soc
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - actions,s500-soc
> > +          - actions,s700-soc
> > +          - actions,s900-soc
> > +      - const: simple-bus
> > +
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    / {
> > +        compatible = "roseapplepi,roseapplepi", "actions,s500";
> > +        model = "Roseapple Pi";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        soc {
> > +            compatible = "actions,s500-soc", "simple-bus";
> 
> No. This shouldn't fall under /soc. I think you should added a separate
> compatible for the reserved memory itself and add a corresponding socinfo
> driver under drivers/soc.
> 
> This way it is obvious that the information is contained in a memory region and
> a driver is used for parsing that.

I avoided on purpose to bind the driver on the reserved memory node
in order to be able to handle also the S700 and S900 SoCs, for which
we currently provide (only) the following information: machine, family,
soc_id.

I assumed the serial_number is not mandatory and we can use the driver
as it is for all SoC variants. I don't know how the serial number for
S700 and S900 could be read, but I think it is very likely they require
a different access method.

Hence my intention was to keep the driver generic, not coupled strictly
with the serial number information.

Regards,
Cristi

> Thanks,
> Mani
> 
> > +            #address-cells = <1>;
> > +            #size-cells = <1>;
> > +            ranges;
> > +        };
> > +    };
> > +
> > +...
> > -- 
> > 2.31.1
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
new file mode 100644
index 000000000000..01e4a8b4f5ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/actions/owl-socinfo.yaml
@@ -0,0 +1,57 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/actions/owl-socinfo.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoC info module
+
+maintainers:
+  - Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
+
+description: |
+  Actions Semi Owl SoC info module provides access to various information
+  about the S500, S700 and S900 SoC variants, such as serial number or id.
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - actions,s500-soc
+          - actions,s700-soc
+          - actions,s900-soc
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - actions,s500-soc
+          - actions,s700-soc
+          - actions,s900-soc
+      - const: simple-bus
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    / {
+        compatible = "roseapplepi,roseapplepi", "actions,s500";
+        model = "Roseapple Pi";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        soc {
+            compatible = "actions,s500-soc", "simple-bus";
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges;
+        };
+    };
+
+...