2022-07-13 05:44:21

by Dongjin Yang

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: mfd: Add bindings for Samsung SysMgr

Add an devicetree binding for Samsung system manager.
This driver is used for SoCs produced by Samsung Foundry to provide
Samsung sysmgr API. The read/write request of sysmgr is delivered to
Samsung secure monitor call.

Signed-off-by: Dongjin Yang <[email protected]>
---
.../devicetree/bindings/mfd/samsung,sys-mgr.yaml | 42 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml

diff --git a/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
new file mode 100644
index 000000000000..83b9d73a5420
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/samsung,sys-mgr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung System Manager
+
+maintainers:
+ - Jesper Nilsson <[email protected]>
+
+description: |
+ The file documents device tree bindings of Samsung system manager.
+
+properties:
+ compatible:
+ enum:
+ - samsung,sys-mgr
+ - samsung,sys-mgr-smccc
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+
+ syscon_imem@10020000 {
+ compatible = "samsung,sys-mgr";
+ reg = <0x0 0x10020000 0x0 0x1000>;
+ };
+
+ syscon_fsys: syscon@16c20000 {
+ compatible = "samsung,sys-mgr-smccc";
+ reg = <0x0 0x16c20000 0x0 0x1000>;
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d173043ffb46..55cb8901ccdc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1863,6 +1863,7 @@ M: Lars Persson <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/firmware/samsung,smccc-svc.yaml
+F: Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
F: Documentation/devicetree/bindings/pinctrl/axis,artpec6-pinctrl.txt
F: arch/arm/boot/dts/artpec6*
F: arch/arm/mach-artpec
--
2.9.5


2022-07-13 07:50:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: mfd: Add bindings for Samsung SysMgr

On 13/07/2022 06:56, Dongjin Yang wrote:
> Add an devicetree binding for Samsung system manager.
> This driver is used for SoCs produced by Samsung Foundry to provide
> Samsung sysmgr API. The read/write request of sysmgr is delivered to
> Samsung secure monitor call.

Nope. Second patch with the same - you need to describe the hardware or
underlying firmware, not the driver. "This driver" is not acceptable for
bindings.

>
> Signed-off-by: Dongjin Yang <[email protected]>
> ---
> .../devicetree/bindings/mfd/samsung,sys-mgr.yaml | 42 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
> new file mode 100644
> index 000000000000..83b9d73a5420
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/samsung,sys-mgr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung System Manager
> +
> +maintainers:
> + - Jesper Nilsson <[email protected]>

How is this related to Axis platforms?

> +
> +description: |
> + The file documents device tree bindings of Samsung system manager.

Useless description. Describe the hardw

> +
> +properties:
> + compatible:
> + enum:
> + - samsung,sys-mgr
> + - samsung,sys-mgr-smccc

Sorry, this does not look like hardware. It could be description of
underlying firmware, but you would need to justify it. Look at Qualcomm
SCM for example. Or ARM SCMI, SCPI.


Best regards,
Krzysztof

2022-07-13 14:49:53

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: mfd: Add bindings for Samsung SysMgr

On Wed, 13 Jul 2022 13:56:28 +0900, Dongjin Yang wrote:
> Add an devicetree binding for Samsung system manager.
> This driver is used for SoCs produced by Samsung Foundry to provide
> Samsung sysmgr API. The read/write request of sysmgr is delivered to
> Samsung secure monitor call.
>
> Signed-off-by: Dongjin Yang <[email protected]>
> ---
> .../devicetree/bindings/mfd/samsung,sys-mgr.yaml | 42 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/mfd/samsung,sys-mgr.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.example.dtb: syscon_imem@10020000: reg: [[0, 268566528], [0, 4096]] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.example.dtb: syscon@16c20000: reg: [[0, 381812736], [0, 4096]] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml

doc reference errors (make refcheckdocs):

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

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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

pip3 install dtschema --upgrade

Please check and re-submit.

2022-07-26 04:48:02

by Dongjin Yang

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: mfd: Add bindings for Samsung SysMgr

On 14/07/2022 04:28, Krzysztof Kozlowski wrote:
> On 13/07/2022 06:56, Dongjin Yang wrote:
> > Add an devicetree binding for Samsung system manager.
> > This driver is used for SoCs produced by Samsung Foundry to provide
> > Samsung sysmgr API. The read/write request of sysmgr is delivered to
> > Samsung secure monitor call.
>
> Nope. Second patch with the same - you need to describe the hardware or
> underlying firmware, not the driver. "This driver" is not acceptable for
> bindings.
>

Yes, I will describe the hardware and underlying firmware.

> > 
> > Signed-off-by: Dongjin Yang <[email protected]>
> > ---
> >  .../devicetree/bindings/mfd/samsung,sys-mgr.yaml   | 42 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 43 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
> > new file mode 100644
> > index 000000000000..83b9d73a5420
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/samsung,sys-mgr.yaml
> > @@ -0,0 +1,42 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: https://protect2.fireeye.com/v1/url?k=4e490fd5-2fc21afa-4e48849a-74fe485cbfe7-58c60ac4380837bf&q=1&e=f2eeb6c4-6c9d-40cd-9437-933a16218ac5&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Ffirmware%2Fsamsung%2Csys-mgr.yaml%23
> > +$schema: https://protect2.fireeye.com/v1/url?k=1c5b5e89-7dd04ba6-1c5ad5c6-74fe485cbfe7-8170c2ecb94e64e4&q=1&e=f2eeb6c4-6c9d-40cd-9437-933a16218ac5&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Samsung System Manager
> > +
> > +maintainers:
> > +  - Jesper Nilsson <[email protected]>
>
> How is this related to Axis platforms?
>

This is for Artpec8 SoC.

> > +
> > +description: |
> > +  The file documents device tree bindings of Samsung system manager.
>
> Useless description. Describe the hardw
>

Yes, I will describe hardware (Artpec8).

> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - samsung,sys-mgr
> > +      - samsung,sys-mgr-smccc
>
> Sorry, this does not look like hardware. It could be description of
> underlying firmware, but you would need to justify it. Look at Qualcomm
> SCM for example. Or ARM SCMI, SCPI.
>

Yes, Should it be like the below?

properties:
compatible:
enum:
- axis,artpec8-sys-mgr
- axis,artpec8-sys-mgr-smccc

>
> Best regards,
> Krzysztof