diff mbox series

[RFC,1/2] spi: dt-bindings: add Siflower Quad SPI controller

Message ID 20240329015147.1481349-1-dqfext@gmail.com
State Superseded
Headers show
Series [RFC,1/2] spi: dt-bindings: add Siflower Quad SPI controller | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Qingfang Deng March 29, 2024, 1:51 a.m. UTC
From: Qingfang Deng <qingfang.deng@siflower.com.cn>

Add YAML devicetree bindings for Siflower Quad SPI controller.

Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
---
 .../bindings/spi/siflower,qspi.yaml           | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/siflower,qspi.yaml

Comments

Rob Herring (Arm) March 29, 2024, 3:27 a.m. UTC | #1
On Fri, 29 Mar 2024 09:51:46 +0800, Qingfang Deng wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add YAML devicetree bindings for Siflower Quad SPI controller.
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> ---
>  .../bindings/spi/siflower,qspi.yaml           | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/siflower,qspi.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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/siflower,qspi.example.dtb: spi@c200000: reg: [[0, 203423744], [0, 4096]] is too long
	from schema $id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/siflower,qspi.example.dtb: spi@c200000: Unevaluated properties are not allowed ('reg' was unexpected)
	from schema $id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240329015147.1481349-1-dqfext@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski March 30, 2024, 5:42 p.m. UTC | #2
On 29/03/2024 02:51, Qingfang Deng wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add YAML devicetree bindings for Siflower Quad SPI controller.

Describe the hardware. What is this Siflower?
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> ---
>  .../bindings/spi/siflower,qspi.yaml           | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/siflower,qspi.yaml b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> new file mode 100644
> index 000000000000..c2dbe82affc2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Siflower Quad Serial Peripheral Interface (QSPI)
> +
> +maintainers:
> +  - Qingfang Deng <qingfang.deng@siflower.com.cn>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: siflower,qspi

Except that this was not tested, aren't you adding it for some SoC? If
so, then you miss here SoC part.

Best regards,
Krzysztof
Qingfang Deng April 1, 2024, 3:36 a.m. UTC | #3
Hi Krzysztof,

On Sun, Mar 31, 2024 at 1:42 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/03/2024 02:51, Qingfang Deng wrote:
> > Add YAML devicetree bindings for Siflower Quad SPI controller.
>
> Describe the hardware. What is this Siflower?

It's a new RISC-V SoC which hasn't been upstreamed yet.

> > +properties:
> > +  compatible:
> > +    const: siflower,qspi
>
> Except that this was not tested, aren't you adding it for some SoC? If
> so, then you miss here SoC part.

I should add the "siflower" prefix to
Documentation/devicetree/bindings/vendor-prefixes.yaml, right?

Regards,
Qingfang
Krzysztof Kozlowski April 1, 2024, 9:53 a.m. UTC | #4
On 01/04/2024 05:36, Qingfang Deng wrote:
> Hi Krzysztof,
> 
> On Sun, Mar 31, 2024 at 1:42 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 29/03/2024 02:51, Qingfang Deng wrote:
>>> Add YAML devicetree bindings for Siflower Quad SPI controller.
>>
>> Describe the hardware. What is this Siflower?
> 
> It's a new RISC-V SoC which hasn't been upstreamed yet.
> 
>>> +properties:
>>> +  compatible:
>>> +    const: siflower,qspi
>>
>> Except that this was not tested, aren't you adding it for some SoC? If
>> so, then you miss here SoC part.
> 
> I should add the "siflower" prefix to
> Documentation/devicetree/bindings/vendor-prefixes.yaml, right?

Isn't it already there? Then obvious you must, but that was not the
point. Please read writing-bindings document. Compatibles should be SoC
specific, not generic. "qspi" is generic.

Best regards,
Krzysztof
Mark Brown April 2, 2024, 12:22 p.m. UTC | #5
On Sat, Mar 30, 2024 at 06:42:11PM +0100, Krzysztof Kozlowski wrote:
> On 29/03/2024 02:51, Qingfang Deng wrote:

> > Add YAML devicetree bindings for Siflower Quad SPI controller.

> Describe the hardware. What is this Siflower?

That seems like a perfectly adequate description - ${VENDOR} ${FUNCTION}
is normal enough and Quad SPI is a well known standard.  We don't need a
marketing spiel for whatever IP version is currently supported.
Krzysztof Kozlowski April 2, 2024, 1:12 p.m. UTC | #6
On 02/04/2024 14:22, Mark Brown wrote:
> On Sat, Mar 30, 2024 at 06:42:11PM +0100, Krzysztof Kozlowski wrote:
>> On 29/03/2024 02:51, Qingfang Deng wrote:
> 
>>> Add YAML devicetree bindings for Siflower Quad SPI controller.
> 
>> Describe the hardware. What is this Siflower?
> 
> That seems like a perfectly adequate description - ${VENDOR} ${FUNCTION}
> is normal enough and Quad SPI is a well known standard.  We don't need a
> marketing spiel for whatever IP version is currently supported.

What we are missing here is the final product, so for example the SoC.
Is the company making exactly one and only one Quad SPI? I provided more
explanation what is missing further in the quoted email and in follow up
email/discussion.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/siflower,qspi.yaml b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
new file mode 100644
index 000000000000..c2dbe82affc2
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
@@ -0,0 +1,54 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Siflower Quad Serial Peripheral Interface (QSPI)
+
+maintainers:
+  - Qingfang Deng <qingfang.deng@siflower.com.cn>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    const: siflower,qspi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi@c200000 {
+      compatible = "siflower,qspi";
+      reg = <0 0xc200000 0 0x1000>;
+      clocks = <&apb_clk>;
+      interrupts = <39>;
+      pinctrl-names = "default";
+      pinctrl-0 = <&spi0_pins>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };