Hi Molly, On Fri, Jul 08, 2022 at 10:18:24AM +0800, MollySophia wrote: > Add documentation for "novatek,nt35596s" panel. As a general note, we cannot apply a driver without the binding. So this patch should be the first in the series. Not a big deal, but it makes it easier on the committer later. A few comments below. Sam > > Signed-off-by: MollySophia <mollysophia379@xxxxxxxxx> > --- > .../display/panel/novatek,nt35596s.yaml | 88 +++++++++++++++++++ > 1 file changed, 88 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..937b194a6f18 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/novatek,nt35596s.yaml > @@ -0,0 +1,88 @@ > +# 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 <mollysophia379@xxxxxxxxx> > + > +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. > + > + reset-gpios: > + maxItems: 1 > + description: phandle of gpio for reset line - This should be 8mA, gpio > + can be configured using mux, pinctrl, pinctrl-names (active high) reset-gpios is part of panel-common and there is no need to describe it here. The description, which looks like a copy from another binding, does not really tell anything that is specific for this HW. So replace it with a reset-gpios: true later in the binding would be better. > + > + vddi0-supply: > + description: phandle of the regulator that provides the supply voltage > + Power IC supply Please drop the "phandle of the" part. This is implicit and does not provide any extra info on the actual HW. This comments applies for all *-supply. > + > + vddpos-supply: > + description: phandle of the positive boost supply regulator > + > + vddneg-supply: > + description: phandle of the negative boost supply regulator > + > + reg: true > + port: true > + backlight: true > + > +required: > + - compatible > + - reg > + - vddi0-supply > + - vddpos-supply > + - vddneg-supply > + - reset-gpios > + - port > + > +unevaluatedProperties: false Use: additionalProperties: false This will catch if the DT file contains any properties that is not mentioned here. The use of unevaluatedProperties in the file you copied from looks wrong. > + > +examples: > + - |+ No need for the + sign here. > + #include <dt-bindings/gpio/gpio.h> > + > + dsi0 { 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
