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