Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
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
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
Hi Ryan, How about to use "anbernic,rg35xx-panel" ? It's not generic though, some other drivers use similar strings already. Regards, kikuchan.
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
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
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
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 :) signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
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 = < 8 9 GPIO_ACTIVE_HIGH>; // PI9
>> +mosi-gpios = < 8 10 GPIO_ACTIVE_HIGH>; // PI10
>> +cs-gpios = < 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 = < 8 14 GPIO_ACTIVE_LOW>; // PI14
>> +
>> +backlight = <>;
>> +power-supply = <_lcd>;
>> +pinctrl-0 = <_rgb888_pins>;
>> +pinctrl-names = "default";
>> +
>> +port {
>> +panel_in_rgb: endpoint {
>
> Neither is this label afaict.
>
> Thanks,
> Conor.
Regards,
Ryan
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
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
> ---
> .../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 0..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
> +
> +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
> +
> +spi_lcd: spi {
> +compatible = "spi-gpio";
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +sck-gpios = < 8 9 GPIO_ACTIVE_HIGH>; // PI9
> +mosi-gpios = < 8 10 GPIO_ACTIVE_HIGH>; // PI10
> +cs-gpios = < 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 = < 8 14 GPIO_ACTIVE_LOW>; // PI14
> +
> +backlight = <>;
> +power-supply = <_lcd>;
> +pinctrl-0 = <_rgb888_pins>;
> +pinctrl-names = "default";
> +
> +port {
> + panel_in_rgb: endpoint {
Neither is this label afaict.
Thanks,
Conor.
> +remote-endpoint = <_lcd0_out_lcd>;
> +};
> +};
> +};
> +};
> --
> 2.45.1
>
signature.asc
Description: PGP signature
Re: [PATCH 1/2] dt-bindings: display: panel: Add WL-355608-A8 panel
On Fri, 24 May 2024 22:33:13 +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 > --- > .../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 > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/display/panel/wl-355608-a8.example.dtb: /example-0/spi/panel@0: failed to match any schema with compatible: ['wl_355608_a8'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240524103506.187277-2-r...@testtoast.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
