diff mbox series

[v2] dt-bindings: trivial-devices: Document spi-cpha and spi-cpol

Message ID 20240830171849.3750165-1-festevam@gmail.com
State New
Headers show
Series [v2] dt-bindings: trivial-devices: Document spi-cpha and spi-cpol | expand

Commit Message

Fabio Estevam Aug. 30, 2024, 5:18 p.m. UTC
There may be cases where a trivial-device needs to describe
the SPI clock polarity and phase via spi-cpol and spi-cpha
properties.

Document these properties to fix the following dt-schema warnings:

rv1108-elgin-r1.dtb: display@0: 'spi-cpha', 'spi-cpol' do not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Changes since v1:
- Add spi-cpha and spi-cpol to trivial-devices.yaml. (Conor)

 Documentation/devicetree/bindings/trivial-devices.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Krzysztof Kozlowski Aug. 31, 2024, 6:23 a.m. UTC | #1
On Fri, Aug 30, 2024 at 02:18:49PM -0300, Fabio Estevam wrote:
> There may be cases where a trivial-device needs to describe
> the SPI clock polarity and phase via spi-cpol and spi-cpha
> properties.
> 
> Document these properties to fix the following dt-schema warnings:
> 
> rv1108-elgin-r1.dtb: display@0: 'spi-cpha', 'spi-cpol' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Changes since v1:
> - Add spi-cpha and spi-cpol to trivial-devices.yaml. (Conor)
> 

No, this does not look correct. Why suddenly all devices get CPHA/CPOL?
This is supposed to be only for devices REALLY needing it (as discussed
with patch moving it out of spi-peripheral-props, did anything change
here?).

>  Documentation/devicetree/bindings/trivial-devices.yaml | 4 ++++
>  1 file changed, 4 insertions(+)

Best regards,
Krzysztof
Fabio Estevam Aug. 31, 2024, 7:58 p.m. UTC | #2
Hi Krzysztof,

On Sat, Aug 31, 2024 at 3:23 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> No, this does not look correct. Why suddenly all devices get CPHA/CPOL?
> This is supposed to be only for devices REALLY needing it (as discussed
> with patch moving it out of spi-peripheral-props, did anything change
> here?).

I tried like to apply spi-cpha and spi-cpol only to elgin,jg10309-01:

--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -419,6 +419,17 @@ properties:
           - vicor,pli1209bc
             # Winbond/Nuvoton H/W Monitor
           - winbond,w83793
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - elgin,jg10309-01
+    then:
+      properties:
+        spi-cpha: true
+        spi-cpol: true

 required:
   - compatible

but that did not help:

$ make CHECK_DTBS=y rockchip/rv1108-elgin-r1.dtb -j12

  DTC [C] arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dtb
/home/fabio/linux-next/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dtb:
display@0: 'spi-cpha', 'spi-cpol' do not match any of the regexes:
'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#

I would appreciate a suggestion on how to fix the warning.

Thanks
Krzysztof Kozlowski Sept. 1, 2024, 10:24 a.m. UTC | #3
On 31/08/2024 21:58, Fabio Estevam wrote:
> Hi Krzysztof,
> 
> On Sat, Aug 31, 2024 at 3:23 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> No, this does not look correct. Why suddenly all devices get CPHA/CPOL?
>> This is supposed to be only for devices REALLY needing it (as discussed
>> with patch moving it out of spi-peripheral-props, did anything change
>> here?).
> 
> I tried like to apply spi-cpha and spi-cpol only to elgin,jg10309-01:
> 

I think the device should be moved out of trivial devices to its own
schema. However wait for feedback from Rob, because he proposed this
patch here.

Best regards,
Krzysztof
Rob Herring Sept. 10, 2024, 3:02 p.m. UTC | #4
On Sun, Sep 1, 2024 at 5:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 31/08/2024 21:58, Fabio Estevam wrote:
> > Hi Krzysztof,
> >
> > On Sat, Aug 31, 2024 at 3:23 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> No, this does not look correct. Why suddenly all devices get CPHA/CPOL?
> >> This is supposed to be only for devices REALLY needing it (as discussed
> >> with patch moving it out of spi-peripheral-props, did anything change
> >> here?).
> >
> > I tried like to apply spi-cpha and spi-cpol only to elgin,jg10309-01:
> >
>
> I think the device should be moved out of trivial devices to its own
> schema. However wait for feedback from Rob, because he proposed this
> patch here.

Okay, let's do a separate schema.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 75a5fad08c44..f7c11eb6e5fd 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -24,6 +24,10 @@  properties:
   interrupts:
     maxItems: 1
 
+  spi-cpha: true
+
+  spi-cpol: true
+
   spi-max-frequency: true
 
   compatible: