2022-03-27 21:51:07

by Kuldeep Singh

[permalink] [raw]
Subject: [PATCH] dt-bindings: spi: qcom-qspi: Add minItems to interconnect-names

Qualcomm QSPI DT spec says interconnects has minimum length 1 and
maximum length 2. Same configuration will be applicable for
interconnect-names. Schema currently depicts interconnects length
correctly but not interconnect-names. It can have a single entry, which
is a valid case yet to be incorporated in the current configuration. The
schema tries to look for 2 names and fail for DTs with a single entry.
Thus, add minItems property to interconnect-names to fix it.

With the change applied, below interconnect-names values are possible:
['qspi-config'], ['qspi-config', 'qspi-memory']

Fixes: 8f9c291558ea ("dt-bindings: spi: Add interconnect binding for QSPI")
Signed-off-by: Kuldeep Singh <[email protected]>
---
Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml
index 055524fe8327..116f3746c1e6 100644
--- a/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml
+++ b/Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml
@@ -49,6 +49,7 @@ properties:
maxItems: 2

interconnect-names:
+ minItems: 1
items:
- const: qspi-config
- const: qspi-memory
--
2.25.1


2022-03-28 03:30:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: spi: qcom-qspi: Add minItems to interconnect-names

On 26/03/2022 22:21, Kuldeep Singh wrote:
> Qualcomm QSPI DT spec says interconnects has minimum length 1 and
> maximum length 2. Same configuration will be applicable for
> interconnect-names. Schema currently depicts interconnects length
> correctly but not interconnect-names. It can have a single entry, which
> is a valid case yet to be incorporated in the current configuration. The
> schema tries to look for 2 names and fail for DTs with a single entry.
> Thus, add minItems property to interconnect-names to fix it.

Thanks for the patch.

6 sentences which explaining the same, which could be written in one
sentence. In the same time you actually do not explain whether having
one interconnect is correct. You just say what DT spec says about
interconnects, but maybe that's wrong.

So either keep just one sentence or really explain why one or two are
allowed (based on datasheet for example).

>
> With the change applied, below interconnect-names values are possible:
> ['qspi-config'], ['qspi-config', 'qspi-memory']
>
> Fixes: 8f9c291558ea ("dt-bindings: spi: Add interconnect binding for QSPI")
> Signed-off-by: Kuldeep Singh <[email protected]>
> ---
> Documentation/devicetree/bindings/spi/qcom,spi-qcom-qspi.yaml | 1 +
> 1 file changed, 1 insertion(+)


Best regards,
Krzysztof

2022-03-28 22:59:50

by Kuldeep Singh

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: spi: qcom-qspi: Add minItems to interconnect-names

On Sun, Mar 27, 2022 at 09:21:34PM +0200, Krzysztof Kozlowski wrote:
> On 26/03/2022 22:21, Kuldeep Singh wrote:
> > Qualcomm QSPI DT spec says interconnects has minimum length 1 and
> > maximum length 2. Same configuration will be applicable for
> > interconnect-names. Schema currently depicts interconnects length
> > correctly but not interconnect-names. It can have a single entry, which
> > is a valid case yet to be incorporated in the current configuration. The
> > schema tries to look for 2 names and fail for DTs with a single entry.
> > Thus, add minItems property to interconnect-names to fix it.
>
> Thanks for the patch.
>
> 6 sentences which explaining the same, which could be written in one
> sentence. In the same time you actually do not explain whether having
> one interconnect is correct. You just say what DT spec says about
> interconnects, but maybe that's wrong.

Though commit desscription is subject to personal perspective, but I
also see some duplication here and will submit v2 with a shorter
description.

>
> So either keep just one sentence or really explain why one or two are
> allowed (based on datasheet for example).

Please see commit 8f9c291558ea, interconnect properties were added by
one of maintainers of qcom-qspi. And it's been sometime this change is
in-tree with no regressions introduced. So, I don't think we should
double check this change. My change simply define minItem constraint to
interconnect-names which is already existing on interconnects.

-Kuldeep