| Message ID | 20240524103506.187277-2-ryan@testtoast.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add WL-355608-A8 panel | expand |
On Fri, May 24, 2024 at 10:33:13PM +1200, Ryan Walklin wrote: > The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown > OEM, used in a number of handheld gaming devices made by Anbernic. > > Add a device tree binding for the panel. > > Signed-off-by: Ryan Walklin <ryan@testtoast.com> > --- > .../bindings/display/panel/wl-355608-a8.yaml | 68 +++++++++++++++++++ > 1 file changed, 68 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml > new file mode 100644 > index 000000000..af12303e2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml > @@ -0,0 +1,68 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel > + > +maintainers: > + - Ryan Walklin <ryan@testtoast.com> > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +properties: > + compatible: > + const: wl-355608-a8 You're missing a vendor prefix here. And when you add it, update the filename to match. > + > + reg: > + maxItems: 1 > + > + spi-3wire: true > + > +required: > + - compatible > + - reg > + - port > + - power-supply > + - reset-gpios > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + spi_lcd: spi { > + compatible = "spi-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + sck-gpios = <&pio 8 9 GPIO_ACTIVE_HIGH>; // PI9 > + mosi-gpios = <&pio 8 10 GPIO_ACTIVE_HIGH>; // PI10 > + cs-gpios = <&pio 8 8 GPIO_ACTIVE_HIGH>; // PI8 > + num-chipselects = <1>; All of this is not needed in the example, all you need to have here is: spi { #address-cells = <1>; #size-cells = <0>; > + > + panel: panel@0 { This "panel" label is not used, you should drop it. > + compatible = "wl_355608_a8"; This doesn't match what you documented, be sure to run dt_binding_check. > + reg = <0>; > + > + spi-3wire; > + spi-max-frequency = <3125000>; > + > + reset-gpios = <&pio 8 14 GPIO_ACTIVE_LOW>; // PI14 > + > + backlight = <&backlight>; > + power-supply = <®_lcd>; > + pinctrl-0 = <&lcd0_rgb888_pins>; > + pinctrl-names = "default"; > + > + port { > + panel_in_rgb: endpoint { Neither is this label afaict. Thanks, Conor. > + remote-endpoint = <&tcon_lcd0_out_lcd>; > + }; > + }; > + }; > + }; > -- > 2.45.1 >
On Sat, 25 May 2024, at 7:10 AM, Conor Dooley wrote: Thanks for the review! >> + >> +properties: >> + compatible: >> + const: wl-355608-a8 > > You're missing a vendor prefix here. And when you add it, update the > filename to match. Thanks, I don't actually know the vendor, would it be acceptable to just use "wl"? >> + >> + sck-gpios = <&pio 8 9 GPIO_ACTIVE_HIGH>; // PI9 >> + mosi-gpios = <&pio 8 10 GPIO_ACTIVE_HIGH>; // PI10 >> + cs-gpios = <&pio 8 8 GPIO_ACTIVE_HIGH>; // PI8 >> + num-chipselects = <1>; > > All of this is not needed in the example, all you need to have here is: > > spi { > #address-cells = <1>; > #size-cells = <0>; > Thanks, will clean it up. >> + >> + panel: panel@0 { > > This "panel" label is not used, you should drop it. > Noted, ta. >> + compatible = "wl_355608_a8"; > > This doesn't match what you documented, be sure to run dt_binding_check. Thanks, changed underscore to dash mid-patch and neglected to fix all the examples (and the subsequent code patch it seems. Will correct. Is there a preference one way or another? > >> + reg = <0>; >> + >> + spi-3wire; >> + spi-max-frequency = <3125000>; >> + >> + reset-gpios = <&pio 8 14 GPIO_ACTIVE_LOW>; // PI14 >> + >> + backlight = <&backlight>; >> + power-supply = <®_lcd>; >> + pinctrl-0 = <&lcd0_rgb888_pins>; >> + pinctrl-names = "default"; >> + >> + port { >> + panel_in_rgb: endpoint { > > Neither is this label afaict. > > Thanks, > Conor. Regards, Ryan
On Sat, May 25, 2024 at 09:26:48AM +1200, Ryan Walklin wrote: > On Sat, 25 May 2024, at 7:10 AM, Conor Dooley wrote: > > Thanks for the review! > > >> + > >> +properties: > >> + compatible: > >> + const: wl-355608-a8 > > > > You're missing a vendor prefix here. And when you add it, update the > > filename to match. > > Thanks, I don't actually know the vendor, would it be acceptable to just use "wl"? You mean, "wl,355608-a8"? I did a wee bit of googling of the thing, and yeah, there's nothing that a surface level search turns up for it - other than they appeared to have a logo with a W in a circle... I think if we genuinely do not know what the vendor is then we just don't have a prefix. > >> + compatible = "wl_355608_a8"; > > > > This doesn't match what you documented, be sure to run dt_binding_check. > > Thanks, changed underscore to dash mid-patch and neglected to fix all > the examples (and the subsequent code patch it seems. Will correct. > Is there a preference one way or another? Not _s :)
On Sun, 26 May 2024, at 3:22 AM, Conor Dooley wrote: >> >> Thanks, I don't actually know the vendor, would it be acceptable to just use "wl"? > > You mean, "wl,355608-a8"? I did a wee bit of googling of the thing, and > yeah, there's nothing that a surface level search turns up for it - > other than they appeared to have a logo with a W in a circle... > I think if we genuinely do not know what the vendor is then we just > don't have a prefix. I was going to go with "wl,wl-355608-a8" as the whole string seems to be the product/serial code, but happy to just not have the vendor prefix as per my V1 if that's acceptable, seems pretty obscure as you've found. > >> >> + compatible = "wl_355608_a8"; > Not _s :) Noted, ta. Regards, Ryan
Hi Ryan, How about to use "anbernic,rg35xx-panel" ? It's not generic though, some other drivers use similar strings already. Regards, kikuchan.
On Sun, 26 May 2024, at 10:49 AM, きくちゃんさん wrote: > Hi Ryan, > > How about to use "anbernic,rg35xx-panel" ? > It's not generic though, some other drivers use similar strings already. Could do, although I think it is used for more than one of the Anbernic devices, so "anbernic,wl-355608-a8" might be best. Happy to go with whatever approach is preferred. > > Regards, > kikuchan. Regards, Ryan
diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml new file mode 100644 index 000000000..af12303e2 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel + +maintainers: + - Ryan Walklin <ryan@testtoast.com> + +allOf: + - $ref: panel-common.yaml# + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + const: wl-355608-a8 + + reg: + maxItems: 1 + + spi-3wire: true + +required: + - compatible + - reg + - port + - power-supply + - reset-gpios + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + spi_lcd: spi { + compatible = "spi-gpio"; + #address-cells = <1>; + #size-cells = <0>; + + sck-gpios = <&pio 8 9 GPIO_ACTIVE_HIGH>; // PI9 + mosi-gpios = <&pio 8 10 GPIO_ACTIVE_HIGH>; // PI10 + cs-gpios = <&pio 8 8 GPIO_ACTIVE_HIGH>; // PI8 + num-chipselects = <1>; + + panel: panel@0 { + compatible = "wl_355608_a8"; + reg = <0>; + + spi-3wire; + spi-max-frequency = <3125000>; + + reset-gpios = <&pio 8 14 GPIO_ACTIVE_LOW>; // PI14 + + backlight = <&backlight>; + power-supply = <®_lcd>; + pinctrl-0 = <&lcd0_rgb888_pins>; + pinctrl-names = "default"; + + port { + panel_in_rgb: endpoint { + remote-endpoint = <&tcon_lcd0_out_lcd>; + }; + }; + }; + };
The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown OEM, used in a number of handheld gaming devices made by Anbernic. Add a device tree binding for the panel. Signed-off-by: Ryan Walklin <ryan@testtoast.com> --- .../bindings/display/panel/wl-355608-a8.yaml | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml