| Message ID | 20220411211243.11121-3-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm/solomon: Add SSD130x OLED displays SPI support | expand |
Hi Javier, On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > The Solomon SSD130x OLED displays can either have an I2C or SPI interface, > add to the schema the properties and examples for OLED devices under SPI. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > Acked-by: Mark Brown <broonie@kernel.org> > --- > > Changes in v2: > - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven). Thanks for the update! > --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > @@ -39,9 +39,14 @@ properties: > reset-gpios: > maxItems: 1 > > + dc-gpios: > + maxItems: 1 > + Perhaps add a description, and clarify this is for SPI only? Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hello Geert, On 4/12/22 09:16, Geert Uytterhoeven wrote: > Hi Javier, > > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas > <javierm@redhat.com> wrote: >> The Solomon SSD130x OLED displays can either have an I2C or SPI interface, >> add to the schema the properties and examples for OLED devices under SPI. >> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >> Acked-by: Mark Brown <broonie@kernel.org> >> --- >> >> Changes in v2: >> - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven). > > Thanks for the update! > You are welcome and thanks for your prompt review and feedback! >> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml >> @@ -39,9 +39,14 @@ properties: >> reset-gpios: >> maxItems: 1 >> >> + dc-gpios: >> + maxItems: 1 >> + > > Perhaps add a description, and clarify this is for SPI only? > I wondered how to make it required for SPI but couldn't find another binding that did the same and I'm not that familiar with DT schemas to figure it out. Before, when I had compatible strings just for SPI I could do the following: - if: properties: compatible: contains: enum: - sinowealth,sh1106-spi - solomon,ssd1305-spi - solomon,ssd1306-spi - solomon,ssd1307-spi - solomon,ssd1309-spi then: required: - spi-max-frequency - dc-gpios but now that we are using the same compatible strings for I2C and SPI, the compatible string can't be used anymore as an indication to make required. Do you have any hints here on how I should enforce this in the schema ? Or if you think that a comment is enough, then I will add it in v3. > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > Thanks!
Hi Javier, On Tue, Apr 12, 2022 at 10:01 AM Javier Martinez Canillas <javierm@redhat.com> wrote: > On 4/12/22 09:16, Geert Uytterhoeven wrote: > > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas > > <javierm@redhat.com> wrote: > >> The Solomon SSD130x OLED displays can either have an I2C or SPI interface, > >> add to the schema the properties and examples for OLED devices under SPI. > >> > >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > >> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml > >> @@ -39,9 +39,14 @@ properties: > >> reset-gpios: > >> maxItems: 1 > >> > >> + dc-gpios: > >> + maxItems: 1 > >> + > > > > Perhaps add a description, and clarify this is for SPI only? > > I wondered how to make it required for SPI but couldn't find another binding > that did the same and I'm not that familiar with DT schemas to figure it out. > > Before, when I had compatible strings just for SPI I could do the following: > > - if: > properties: > compatible: > contains: > enum: > - sinowealth,sh1106-spi > - solomon,ssd1305-spi > - solomon,ssd1306-spi > - solomon,ssd1307-spi > - solomon,ssd1309-spi > then: > required: > - spi-max-frequency > - dc-gpios > > but now that we are using the same compatible strings for I2C and SPI, the > compatible string can't be used anymore as an indication to make required. > > Do you have any hints here on how I should enforce this in the schema ? > > Or if you think that a comment is enough, then I will add it in v3. I don't know how to make it required for SPI, if possible at all. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 4/12/22 10:07, Geert Uytterhoeven wrote: > Hi Javier, [snip] >> >> Do you have any hints here on how I should enforce this in the schema ? >> >> Or if you think that a comment is enough, then I will add it in v3. > > I don't know how to make it required for SPI, if possible at all. > I see. Let's go with a comment then and we can later enforce it, if someone knows if is possible / how to do it.
On Tue, Apr 12, 2022 at 10:12:32AM +0200, Javier Martinez Canillas wrote: > On 4/12/22 10:07, Geert Uytterhoeven wrote: > > Hi Javier, > > [snip] > > >> > >> Do you have any hints here on how I should enforce this in the schema ? > >> > >> Or if you think that a comment is enough, then I will add it in v3. > > > > I don't know how to make it required for SPI, if possible at all. > > > > I see. Let's go with a comment then and we can later enforce it, if someone > knows if is possible / how to do it. It's not possible unless we annotate child nodes with something from the parent. '$nodename' is the one and only example of that. Really, this falls below the bar of what I'm concerned about checking. Ask me again when the conversions are all done (only ~2K to go :( ). Rob
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml index 6b9d0c72739a..b58d2d7bd8c1 100644 --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml @@ -39,9 +39,14 @@ properties: reset-gpios: maxItems: 1 + dc-gpios: + maxItems: 1 + vbat-supply: description: The supply for VBAT + spi-max-frequency: true + solomon,height: $ref: /schemas/types.yaml#/definitions/uint32 default: 16 @@ -160,6 +165,7 @@ allOf: compatible: contains: const: solomon,ssd1305 + then: properties: solomon,dclk-div: @@ -213,14 +219,14 @@ examples: #address-cells = <1>; #size-cells = <0>; - ssd1307: oled@3c { + ssd1307_i2c: oled@3c { compatible = "solomon,ssd1307"; reg = <0x3c>; pwms = <&pwm 4 3000>; reset-gpios = <&gpio2 7>; }; - ssd1306: oled@3d { + ssd1306_i2c: oled@3d { compatible = "solomon,ssd1306"; reg = <0x3c>; pwms = <&pwm 4 3000>; @@ -231,3 +237,30 @@ examples: solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>; }; }; + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + ssd1307_spi: oled@0 { + compatible = "solomon,ssd1307"; + reg = <0x0>; + pwms = <&pwm 4 3000>; + reset-gpios = <&gpio2 7>; + dc-gpios = <&gpio2 8>; + spi-max-frequency = <10000000>; + }; + + ssd1306_spi: oled@1 { + compatible = "solomon,ssd1306"; + reg = <0x1>; + pwms = <&pwm 4 3000>; + reset-gpios = <&gpio2 7>; + dc-gpios = <&gpio2 8>; + spi-max-frequency = <10000000>; + solomon,com-lrremap; + solomon,com-invdir; + solomon,com-offset = <32>; + solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>; + }; + };