<div dir="auto"><div><div style="font-size:12.8px" dir="auto">Hi Sam,</div><div style="font-size:12.8px" dir="auto"><br></div><div style="font-size:12.8px" dir="auto">Thanks for your suggestions.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Sam Ravnborg <<a href="mailto:sam@ravnborg.org" target="_blank" rel="noreferrer">sam@ravnborg.org</a>> 于 2022年7月10日周日 上午4:47写道：<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Molly,<br>
<br>
thanks for the quick response to the review comments.<br>
<br>
On Sat, Jul 09, 2022 at 10:11:35PM +0800, MollySophia wrote:<br>
> Add documentation for "novatek,nt35596s" panel.<br>
> <br>
> Signed-off-by: MollySophia <<a href="mailto:mollysophia379@gmail.com" rel="noreferrer noreferrer" target="_blank">mollysophia379@gmail.com</a>><br>
The s-o-b needs your real name - guess the above is a concatenation of<br>
first name and surname.<br>
<br>
The binding included in this patch fails the check:<br>
$ make DT_CHECKER_FLAGS=-m dt_binding_check<br>
<br>
You may need to run:<br>
$ pip3 install dtschema --upgrade<br>
<br>
Or you may have to install some dependencies first.<br>
The problem is that the patch is missing a "reset-gpios: true"<br>
<br>
On top of this I looked at the binding - and the description<br>
this is copied from is almost identical.<br>
So another approach would be to extend the existing binding like<br>
in the following.<br>
<br>
And this also gives a good hint that maybe this can be embedded in<br>
the existing driver - and there is no need for a new driver.<br>
Could you try to give this a spin and get back on this.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div><div dir="auto"><div style="font-size:12.8px" dir="auto"><div>That's reasonable. Actually, this driver was modified from novatek,nt35596s, with different panel initialization commands, and it seems easy to be embedded in</div>the existing driver. However, I wonder what the driver file name would be...? "panel-novatek-nt35596s-nt36672a.c" or something else?</div><div style="color:rgb(136,136,136);font-size:12.8px" dir="auto"><div><br></div></div><div style="font-size:12.8px" dir="auto"><div style="color:rgb(136,136,136)"> Molly</div><div style="color:rgb(136,136,136)" dir="auto"><br></div></div></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Sorry for not seeing this in the first place.<br>
<br>
Sam<br>
<br>
diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml<br>
index 41ee3157a1cd..913bb81ae93d 100644<br>
--- a/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml<br>
+++ b/Documentation/devicetree/bindings/display/panel/novatek,nt36672a.yaml<br>
@@ -20,14 +20,20 @@ allOf:<br>
<br>
properties:<br>
compatible:<br>
- items:<br>
- - enum:<br>
- - tianma,fhd-video<br>
- - const: novatek,nt36672a<br>
+ oneOf:<br>
+ - items:<br>
+ - enum:<br>
+ - tianma,fhd-video<br>
+ - const: novatek,nt36672a<br>
+<br>
+ - items:<br>
+ - enum:<br>
+ - jdi,fhd-nt35596s<br>
+ - const: novatek,nt35596s<br>
+<br>
description: This indicates the panel manufacturer of the panel that is<br>
- in turn using the NT36672A panel driver. This compatible string<br>
- determines how the NT36672A panel driver is configured for the indicated<br>
- panel. The novatek,nt36672a compatible shall always be provided as a fallback.<br>
+ in turn using the NT36672A or the NT35596S panel driver. This compatible string<br>
+ determines how the panel driver is configured for the indicated panel.<br>
<br>
reset-gpios:<br>
maxItems: 1<br>
@@ -85,4 +91,27 @@ examples:<br>
};<br>
};<br>
<br>
+ dsi1 {<br>
+ #address-cells = <1>;<br>
+ #size-cells = <0>;<br>
+<br>
+ panel@0 {<br>
+ compatible = "jdi,fhd-nt35596s", "novatek,nt35596s";<br>
+ reg = <0>;<br>
+ vddi0-supply = <&vreg_l14a_1p88>;<br>
+ vddpos-supply = <&lab>;<br>
+ vddneg-supply = <&ibb>;<br>
+<br>
+ backlight = <&pmi8998_wled>;<br>
+ reset-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;<br>
+<br>
+ port {<br>
+ jdi_nt35596s_in_1: endpoint {<br>
+ remote-endpoint = <&dsi1_out>;<br>
+ };<br>
+ };<br>
+ };<br>
+ };<br>
+<br>
+<br>
...<br>
<br>
> ---<br>
> .../display/panel/novatek,nt35596s.yaml | 83 +++++++++++++++++++<br>
> 1 file changed, 83 insertions(+)<br>
> create mode 100644 Documentation/devicetree/bindings/display/panel/novatek,nt35596s.yaml<br>
> <br>
> diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt35596s.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt35596s.yaml<br>
> new file mode 100644<br>
> index 000000000000..f724f101a6fd<br>
> --- /dev/null<br>
> +++ b/Documentation/devicetree/bindings/display/panel/novatek,nt35596s.yaml<br>
> @@ -0,0 +1,83 @@<br>
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause<br>
> +%YAML 1.2<br>
> +---<br>
> +$id: <a href="http://devicetree.org/schemas/display/panel/novatek,nt35596s.yaml#" rel="noreferrer noreferrer noreferrer" target="_blank">http://devicetree.org/schemas/display/panel/novatek,nt35596s.yaml#</a><br>
> +$schema: <a href="http://devicetree.org/meta-schemas/core.yaml#" rel="noreferrer noreferrer noreferrer" target="_blank">http://devicetree.org/meta-schemas/core.yaml#</a><br>
> +<br>
> +title: Novatek NT35596S based DSI display Panels<br>
> +<br>
> +maintainers:<br>
> + - Molly Sophia <<a href="mailto:mollysophia379@gmail.com" rel="noreferrer noreferrer" target="_blank">mollysophia379@gmail.com</a>><br>
> +<br>
> +description: |<br>
> + The nt35596s IC from Novatek is a generic DSI Panel IC used to drive dsi<br>
> + panels.<br>
> + Right now, support is added only for a JDI FHD+ LCD display panel with a<br>
> + resolution of 1080x2160. It is a video mode DSI panel.<br>
> +<br>
> +allOf:<br>
> + - $ref: panel-common.yaml#<br>
> +<br>
> +properties:<br>
> + compatible:<br>
> + items:<br>
> + - enum:<br>
> + - jdi,fhd-nt35596s<br>
> + - const: novatek,nt35596s<br>
> + description: This indicates the panel manufacturer of the panel that is<br>
> + in turn using the NT35596S panel driver. This compatible string<br>
> + determines how the NT35596S panel driver is configured for the indicated<br>
> + panel. The novatek,nt35596s compatible shall always be provided as a fallback.<br>
> +<br>
> + vddi0-supply:<br>
> + description: regulator that provides the supply voltage<br>
> + Power IC supply<br>
> +<br>
> + vddpos-supply:<br>
> + description: positive boost supply regulator<br>
> +<br>
> + vddneg-supply:<br>
> + description: negative boost supply regulator<br>
> +<br>
> + reg: true<br>
> + port: true<br>
> + backlight: true<br>
> +<br>
> +required:<br>
> + - compatible<br>
> + - reg<br>
> + - vddi0-supply<br>
> + - vddpos-supply<br>
> + - vddneg-supply<br>
> + - reset-gpios<br>
> + - port<br>
> +<br>
> +additionalProperties: false<br>
> +<br>
> +examples:<br>
> + - |<br>
> + #include <dt-bindings/gpio/gpio.h><br>
> +<br>
> + dsi {<br>
> + #address-cells = <1>;<br>
> + #size-cells = <0>;<br>
> +<br>
> + panel@0 {<br>
> + compatible = "jdi,fhd-nt35596s", "novatek,nt35596s";<br>
> + reg = <0>;<br>
> + vddi0-supply = <&vreg_l14a_1p88>;<br>
> + vddpos-supply = <&lab>;<br>
> + vddneg-supply = <&ibb>;<br>
> +<br>
> + backlight = <&pmi8998_wled>;<br>
> + reset-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;<br>
> +<br>
> + port {<br>
> + jdi_nt35596s_in_0: endpoint {<br>
> + remote-endpoint = <&dsi0_out>;<br>
> + };<br>
> + };<br>
> + };<br>
> + };<br>
> +<br>
> +...<br>
> -- <br>
> 2.37.0<br>
</blockquote></div></div></div>