On 16/05/2023 12:27, Paulo Pavačić wrote: > From 5a202ed7c7aa3433e348c5fed176defab1af1fae Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Paulo=20Pava=C4=8Di=C4=87?= <paulo.pavacic@xxxxxxxxxxx> > Date: Tue, 16 May 2023 12:17:38 +0200 > Subject: [PATCH] dt-bindings: add panel-mipi-dsi-bringup DT schema > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit Your patch/email header looks corrupted. Please use standard tools like git format-patch or b4. Also: Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Due to above, limited review. Please fix everything and send v2 for full review. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. A nit, subject: drop second/last, redundant "DT schema". The "dt-bindings" prefix is already stating that these are bindings. > > Add dt-bindings documentation for panel-mipi-dsi-bringup which currently > supports fannal,C3004 panel. Also added fannal to vendor-prefixes. > > Signed-off-by: Paulo Pavačić <pavacic.p@xxxxxxxxx> > --- > .../display/panel/panel-mipi-dsi-bringup.yaml | 65 +++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > 2 files changed, 67 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-mipi-dsi-bringup.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-mipi-dsi-bringup.yaml > b/Documentation/devicetree/bindings/display/panel/panel-mipi-dsi-bringup.yaml > new file mode 100644 > index 000000000000..a867f86fa984 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-mipi-dsi-bringup.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/panel-mipi-dsi-bringup.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MIPI DSI Bringup driver Drop driver. Explain what is "bringup". We describe hardware, not driver. > + > +description: | > + This document defines device tree for the MIPI DSI Bringup driver. And all Drop redundant pieces like "This document ..." and any references to driver. > + the required parameters to get your panel to work. Driver was made to help > + enabling MIPI DSI panels, to get those first pixels drawn on to the screen. > + Since already you have to patch the driver with initialization sequences all > + the settings such as DSI lanes, video mode, dsi formats are set in the > + driver directly. SInce adding new panel can be overwhelming and to make Since? > + porting easier, you can search for word `INTERACTION` in the driver > + to check sections that you have to modify . Everything should be rephrased to describe hardware, not driver. > + > + > +maintainers: > + - | > + Paulo Pavačić > + <paulo.pavacic@xxxxxxxxxxx> <pavacic.p@xxxxxxxxx> <@ppavacic:matrix.org> That's not how we code it... Look at the files. > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > + const: fannal,C3004 OK, I am now confused. Bindings file name must match compatible. Why do you describe everything as some MIPI bringup driver and then add compatible for specific device? Also, only lowercase. > + > + reg: true > + reset-gpios: true > + > +required: > + - compatible > + - reg > + - reset-gpios > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + //example on IMX8MM where GPIO pin 9 is used as a reset pin Drop > + &mipi_dsi { This should be regular node. > + status = "okay"; Drop > + panel@0 { > + status = "okay"; Drop > + reg = <0>; > + compatible = "fannal,C3004"; Compatible is first, reg is second. > + pinctrl-0 = <&pinctrl_mipi_dsi_rst>; > + pinctrl-names = "default"; > + reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>; > + }; > + }; > + &iomuxc { > + pinctrl_mipi_dsi_rst: pinctrl_mipi_dsi_rst { Drop entire node. This is so far away from any acceptable DTS... please look at existing bindings. Best regards, Krzysztof
