Message ID | 20210625121045.81711-1-paul@crapouillou.net |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [1/2] dt-bindings: display/panel: Add Innolux EJ030NA | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/dt-meta-schema | success | |
robh/dtbs-check | success |
Hi Sam, thanks for the review. Le sam., juil. 10 2021 at 08:14:43 +0200, Sam Ravnborg <sam@ravnborg.org> a écrit : > Hi Paul, > > On Fri, Jun 25, 2021 at 01:10:44PM +0100, Paul Cercueil wrote: >> Add binding for the Innolux EJ030NA panel, which is a 320x480 3.0" >> 4:3 >> 24-bit TFT LCD panel with non-square pixels and a delta-RGB 8-bit >> interface. >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> .../display/panel/innolux,ej030na.yaml | 62 >> +++++++++++++++++++ >> 1 file changed, 62 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml >> >> diff --git >> a/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml >> b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml >> new file mode 100644 >> index 000000000000..cda36c04e85c >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml >> @@ -0,0 +1,62 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: >> http://devicetree.org/schemas/display/panel/innolux,ej030na.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Innolux EJ030NA 3.0" (320x480 pixels) 24-bit TFT LCD panel >> + >> +description: | >> + The panel must obey the rules for a SPI slave device as >> specified in >> + spi/spi-controller.yaml >> + >> +maintainers: >> + - Paul Cercueil <paul@crapouillou.net> >> + >> +allOf: >> + - $ref: panel-common.yaml# >> + >> +properties: >> + compatible: >> + const: innolux,ej030na >> + >> + backlight: true >> + port: true >> + power-supply: true >> + reg: true >> + reset-gpios: true >> + >> +required: >> + - compatible >> + - reg >> + - power-supply >> + - reset-gpios >> + >> +unevaluatedProperties: false > I had expected: > additionalProperties: false > > With this fixed: > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> I am not sure; the doc states that this (additionalProperties: false) "can't be used in case where another schema is referenced", which is the case here, as we include "panel-common.yaml". Cheers, -Paul >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + panel@0 { >> + compatible = "innolux,ej030na"; >> + reg = <0>; >> + >> + spi-max-frequency = <10000000>; >> + >> + reset-gpios = <&gpe 4 GPIO_ACTIVE_LOW>; >> + power-supply = <&lcd_power>; >> + >> + backlight = <&backlight>; >> + >> + port { >> + panel_input: endpoint { >> + remote-endpoint = <&panel_output>; >> + }; >> + }; >> + }; >> + }; >> -- >> 2.30.2
[...] >> I am not sure; the doc states that this (additionalProperties: >> false) "can't >> be used in case where another schema is referenced", which is the >> case here, >> as we include "panel-common.yaml". > > This DT schema already list all relevant properties like: > > backlight: true > > So "additionalProperties: false" tells that no other properties are > allowed other than the listed properties. > > To my best understanding unevaluatedProperties: false is less strict > and > should be used if one does not list all possilbe properties. > This could be the case for a panel haging below a SPI controller as in > this case. So in other words giving this some extra thought I think > unevaluatedProperties: false is OK here. A panel below a SPI controller would have all its SPI-specific properties covered by spi-controller.yaml, I believe? So maybe "additionalProperties: false" would work? In any case, if I use "additionalProperties: false", "make dt_binding_check" complains that my example's "spi-max-frequency" property is not covered. So maybe you are right. > So my r-b is ok if you keep it as it. > > PS. Where do you guys hang out with the downfall of freenode - > somewhere > on oftc? We moved to #opendingux on Libera. Cheers, -Paul
On Sat, Jul 10, 2021 at 11:21:56AM +0100, Paul Cercueil wrote: > > [...] > > > > I am not sure; the doc states that this (additionalProperties: > > > false) "can't > > > be used in case where another schema is referenced", which is the > > > case here, > > > as we include "panel-common.yaml". > > > > This DT schema already list all relevant properties like: > > > > backlight: true > > > > So "additionalProperties: false" tells that no other properties are > > allowed other than the listed properties. > > > > To my best understanding unevaluatedProperties: false is less strict and > > should be used if one does not list all possilbe properties. Right. There's some value of listing which common properties you are using as well. > > This could be the case for a panel haging below a SPI controller as in > > this case. So in other words giving this some extra thought I think > > unevaluatedProperties: false is OK here. > > A panel below a SPI controller would have all its SPI-specific properties > covered by spi-controller.yaml, I believe? So maybe "additionalProperties: > false" would work? No. Because spi-controller.yaml is evaluated on the SPI host node and this one is evaluated on the SPI slave. There's some work to address this, but it means every SPI slave will have to reference a SPI device schema. The bigger issue here is SPI controller specific device properties. So for this case, we'll have to use unevaluatedProperties. > > In any case, if I use "additionalProperties: false", "make dt_binding_check" > complains that my example's "spi-max-frequency" property is not covered. So > maybe you are right. > > > So my r-b is ok if you keep it as it. > > > > PS. Where do you guys hang out with the downfall of freenode - somewhere > > on oftc? > > We moved to #opendingux on Libera. > > Cheers, > -Paul > > >
On Fri, 25 Jun 2021 13:10:44 +0100, Paul Cercueil wrote: > Add binding for the Innolux EJ030NA panel, which is a 320x480 3.0" 4:3 > 24-bit TFT LCD panel with non-square pixels and a delta-RGB 8-bit > interface. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > .../display/panel/innolux,ej030na.yaml | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Rob, Le mer., juil. 14 2021 at 14:30:13 -0600, Rob Herring <robh@kernel.org> a écrit : > On Sat, Jul 10, 2021 at 11:21:56AM +0100, Paul Cercueil wrote: >> >> [...] >> >> > > I am not sure; the doc states that this (additionalProperties: >> > > false) "can't >> > > be used in case where another schema is referenced", which is >> the >> > > case here, >> > > as we include "panel-common.yaml". >> > >> > This DT schema already list all relevant properties like: >> > >> > backlight: true >> > >> > So "additionalProperties: false" tells that no other properties >> are >> > allowed other than the listed properties. >> > >> > To my best understanding unevaluatedProperties: false is less >> strict and >> > should be used if one does not list all possilbe properties. > > Right. There's some value of listing which common properties you are > using as well. > >> > This could be the case for a panel haging below a SPI controller >> as in >> > this case. So in other words giving this some extra thought I >> think >> > unevaluatedProperties: false is OK here. >> >> A panel below a SPI controller would have all its SPI-specific >> properties >> covered by spi-controller.yaml, I believe? So maybe >> "additionalProperties: >> false" would work? > > No. Because spi-controller.yaml is evaluated on the SPI host node and > this one is evaluated on the SPI slave. There's some work to address > this, but it means every SPI slave will have to reference a SPI device > schema. The bigger issue here is SPI controller specific device > properties. So for this case, we'll have to use unevaluatedProperties. Thank you for the clarification. Cheers, -Paul >> >> In any case, if I use "additionalProperties: false", "make >> dt_binding_check" >> complains that my example's "spi-max-frequency" property is not >> covered. So >> maybe you are right. >> >> > So my r-b is ok if you keep it as it. >> > >> > PS. Where do you guys hang out with the downfall of freenode - >> somewhere >> > on oftc? >> >> We moved to #opendingux on Libera. >> >> Cheers, >> -Paul >> >> >>
Hi Sam, Le sam., juil. 10 2021 at 08:18:44 +0200, Sam Ravnborg <sam@ravnborg.org> a écrit : > Hi Paul, Christophe, > > On Fri, Jun 25, 2021 at 01:10:45PM +0100, Paul Cercueil wrote: >> From: Christophe Branchereau <cbranchereau@gmail.com> >> >> Add support for the Innolux/Chimei EJ030NA 3.0" >> 320x480 TFT panel. >> >> This panel can be found in the LDKs, RS97 V2.1 and RG300 (non IPS) >> handheld gaming consoles. >> >> While being 320x480, it is actually a horizontal 4:3 >> panel with non-square pixels in delta arrangement. >> >> Signed-off-by: Christophe Branchereau <cbranchereau@gmail.com> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> > > The drivers looks good. > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Pushed to drm-misc-next. Thanks for the review! Cheers, -Paul
diff --git a/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml new file mode 100644 index 000000000000..cda36c04e85c --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/innolux,ej030na.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Innolux EJ030NA 3.0" (320x480 pixels) 24-bit TFT LCD panel + +description: | + The panel must obey the rules for a SPI slave device as specified in + spi/spi-controller.yaml + +maintainers: + - Paul Cercueil <paul@crapouillou.net> + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + const: innolux,ej030na + + backlight: true + port: true + power-supply: true + reg: true + reset-gpios: true + +required: + - compatible + - reg + - power-supply + - reset-gpios + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + panel@0 { + compatible = "innolux,ej030na"; + reg = <0>; + + spi-max-frequency = <10000000>; + + reset-gpios = <&gpe 4 GPIO_ACTIVE_LOW>; + power-supply = <&lcd_power>; + + backlight = <&backlight>; + + port { + panel_input: endpoint { + remote-endpoint = <&panel_output>; + }; + }; + }; + };
Add binding for the Innolux EJ030NA panel, which is a 320x480 3.0" 4:3 24-bit TFT LCD panel with non-square pixels and a delta-RGB 8-bit interface. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- .../display/panel/innolux,ej030na.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml