From: Sam Ravnborg <sam@ravnborg.org> To: "H. Nikolaus Schaller" <hns@goldelico.com> Cc: Paul Cercueil <paul@crapouillou.net>, Paul Boddie <paul@boddie.org.uk>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Ralf Baechle <ralf@linux-mips.org>, Paul Burton <paulburton@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, Andi Kleen <ak@linux.intel.com>, Krzysztof Kozlowski <krzk@kernel.org>, Geert Uytterhoeven <geert+renesas@glider.be>, "Eric W. Biederman" <ebiederm@xmission.com>, Miquel Raynal <miquel.raynal@bootlin.com>, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, Kees Cook <keescook@chromium.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mips@vger.kernel.org, linux-gpio@vger.kernel.org, mips-creator-ci20-dev@googlegroups.com, letux-kernel@openphoenux.org Subject: Re: [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml Date: Thu, 9 Apr 2020 14:20:54 +0200 [thread overview] Message-ID: <20200409122054.GA5683@ravnborg.org> (raw) In-Reply-To: <F4F5D267-A538-444B-9DCA-EA87AC798FB3@goldelico.com> Hi Nikolaus. > >> + > >> +examples: > >> + - | > >> + #include <dt-bindings/clock/jz4725b-cgu.h> > >> + > >> + panel { > >> + compatible = "sharp,ls020b1dd01d"; > >> + > >> + backlight = <&backlight>; > >> + power-supply = <&vcc>; > >> + > >> + port { > >> + panel_input: endpoint { > >> + remote-endpoint = <&panel_output>; > >> + }; > >> + }; > >> + }; > > The panel part is not needed - better to drop it. > > Well, it is needed to fulfill the remote-endpoint below. Examples may have phandle that are not defined. So the example will work fine without it. See other similar examples. > > > > > > >> + > >> + lcd: lcd-controller@13050000 { > >> + compatible = "ingenic,jz4725b-lcd"; > >> + reg = <0x13050000 0x1000>; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + clocks = <&cgu JZ4725B_CLK_LCD>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + port { > >> + panel_output: endpoint { > >> + remote-endpoint = <&panel_input>; > >> + }; > >> + }; > >> + }; > > We know this example will not pass the check, as there is only > > one clock specified. > > I suggest to drop this example. > > If it later turns out that jz4725b only have one clock, > > Paul already reported that it only wants to see one clock. > > > then the binding > > needs to be updated. > > Yes, I have that on my to-do list to update the binding to reflect > this minItems/maxItems thing but I am not yet sure about how > to handle the clock-names in that case. I.e. make "lcd" optional > and enforce "lcd_pclk" only. Look forward to next version. > > > But the best guess is that the example is wrong. > > > > The example below for jz4780-lcd cover all relevant parts - so > > just keep it as the only example. > > > >> + > >> + - | > >> + #include <dt-bindings/clock/jz4780-cgu.h> > >> + > >> + lcdc0: lcdc0@13050000 { > > Name this lcdc > > And drop "lcdc0@13050000" as this is not relevant for this example. > > > > Remember - the examples exist to explain the binding. They are > > just examples. > > > >> + compatible = "ingenic,jz4780-lcd"; > >> + reg = <0x13050000 0x1800>; > >> + > >> + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + jz4780_lcd_out: port { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + jz4780_out_hdmi: endpoint@0 { > >> + reg = <0>; > >> + remote-endpoint = <&hdmi_in_lcd>; > >> + }; > >> + }; > >> + }; > >> + > > > > And drop this as it does not add anything extra. > > Well, it demonstrates how to add a second lcdc which is disabled. The purpose of the example is to show an example of the binding specified in this file. Adding a second disabled lcdc is a general thing, and not something we want in all the individual examples. Also the actual content, for example register values can be random as they are not part of the binding. This is not a documentation of the HW but a binding example. Sam > > Showing that it is possible to do so is IMHO the most important > part of the example because it is not at all obvious. > > I have also added both SoC to show how differently they can > and should be. > > >> + lcdc1: lcdc1@130a0000 { > >> + compatible = "ingenic,jz4780-lcd"; > >> + reg = <0x130a0000 0x1800>; > >> + > >> + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD1PIXCLK>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + status = "disabled"; > >> + }; > > > > Sam > > BR and thanks, > Nikolaus >
WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org> To: "H. Nikolaus Schaller" <hns@goldelico.com> Cc: Mark Rutland <mark.rutland@arm.com>, Paul Boddie <paul@boddie.org.uk>, Geert Uytterhoeven <geert+renesas@glider.be>, David Airlie <airlied@linux.ie>, dri-devel@lists.freedesktop.org, linux-mips@vger.kernel.org, Paul Cercueil <paul@crapouillou.net>, Miquel Raynal <miquel.raynal@bootlin.com>, Andi Kleen <ak@linux.intel.com>, Paul Burton <paulburton@kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>, mips-creator-ci20-dev@googlegroups.com, devicetree@vger.kernel.org, Kees Cook <keescook@chromium.org>, linux-gpio@vger.kernel.org, Rob Herring <robh+dt@kernel.org>, letux-kernel@openphoenux.org, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>, "Eric W. Biederman" <ebiederm@xmission.com> Subject: Re: [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml Date: Thu, 9 Apr 2020 14:20:54 +0200 [thread overview] Message-ID: <20200409122054.GA5683@ravnborg.org> (raw) In-Reply-To: <F4F5D267-A538-444B-9DCA-EA87AC798FB3@goldelico.com> Hi Nikolaus. > >> + > >> +examples: > >> + - | > >> + #include <dt-bindings/clock/jz4725b-cgu.h> > >> + > >> + panel { > >> + compatible = "sharp,ls020b1dd01d"; > >> + > >> + backlight = <&backlight>; > >> + power-supply = <&vcc>; > >> + > >> + port { > >> + panel_input: endpoint { > >> + remote-endpoint = <&panel_output>; > >> + }; > >> + }; > >> + }; > > The panel part is not needed - better to drop it. > > Well, it is needed to fulfill the remote-endpoint below. Examples may have phandle that are not defined. So the example will work fine without it. See other similar examples. > > > > > > >> + > >> + lcd: lcd-controller@13050000 { > >> + compatible = "ingenic,jz4725b-lcd"; > >> + reg = <0x13050000 0x1000>; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + clocks = <&cgu JZ4725B_CLK_LCD>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + port { > >> + panel_output: endpoint { > >> + remote-endpoint = <&panel_input>; > >> + }; > >> + }; > >> + }; > > We know this example will not pass the check, as there is only > > one clock specified. > > I suggest to drop this example. > > If it later turns out that jz4725b only have one clock, > > Paul already reported that it only wants to see one clock. > > > then the binding > > needs to be updated. > > Yes, I have that on my to-do list to update the binding to reflect > this minItems/maxItems thing but I am not yet sure about how > to handle the clock-names in that case. I.e. make "lcd" optional > and enforce "lcd_pclk" only. Look forward to next version. > > > But the best guess is that the example is wrong. > > > > The example below for jz4780-lcd cover all relevant parts - so > > just keep it as the only example. > > > >> + > >> + - | > >> + #include <dt-bindings/clock/jz4780-cgu.h> > >> + > >> + lcdc0: lcdc0@13050000 { > > Name this lcdc > > And drop "lcdc0@13050000" as this is not relevant for this example. > > > > Remember - the examples exist to explain the binding. They are > > just examples. > > > >> + compatible = "ingenic,jz4780-lcd"; > >> + reg = <0x13050000 0x1800>; > >> + > >> + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + jz4780_lcd_out: port { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + jz4780_out_hdmi: endpoint@0 { > >> + reg = <0>; > >> + remote-endpoint = <&hdmi_in_lcd>; > >> + }; > >> + }; > >> + }; > >> + > > > > And drop this as it does not add anything extra. > > Well, it demonstrates how to add a second lcdc which is disabled. The purpose of the example is to show an example of the binding specified in this file. Adding a second disabled lcdc is a general thing, and not something we want in all the individual examples. Also the actual content, for example register values can be random as they are not part of the binding. This is not a documentation of the HW but a binding example. Sam > > Showing that it is possible to do so is IMHO the most important > part of the example because it is not at all obvious. > > I have also added both SoC to show how differently they can > and should be. > > >> + lcdc1: lcdc1@130a0000 { > >> + compatible = "ingenic,jz4780-lcd"; > >> + reg = <0x130a0000 0x1800>; > >> + > >> + clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD1PIXCLK>; > >> + clock-names = "lcd", "lcd_pclk"; > >> + > >> + interrupt-parent = <&intc>; > >> + interrupts = <31>; > >> + > >> + status = "disabled"; > >> + }; > > > > Sam > > BR and thanks, > Nikolaus > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-04-09 12:21 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-29 17:35 [RFC v3 0/8] MIPS: CI20: add HDMI out support H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 1/8] dt-bindings: display: convert ingenic,lcd.txt to ingenic,lcd.yaml H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml H. Nikolaus Schaller 2020-03-30 15:42 ` [RFC v3 1/8] dt-bindings: display: convert ingenic,lcd.txt to ingenic,lcd.yaml Rob Herring 2020-03-30 15:42 ` Rob Herring 2020-03-30 16:33 ` H. Nikolaus Schaller 2020-03-30 16:33 ` [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml H. Nikolaus Schaller [not found] ` <PGM08Q.HSFQKBTIA4ZJ1@crapouillou.net> [not found] ` <9F25715B-3C06-4302-9393-3440E9D54470@goldelico.com> [not found] ` <9LN08Q.J1LY4QL3MG9B3@crapouillou.net> 2020-03-30 17:18 ` [RFC v3 1/8] dt-bindings: display: convert ingenic,lcd.txt to ingenic,lcd.yaml H. Nikolaus Schaller 2020-04-09 7:25 ` [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml Sam Ravnborg 2020-04-09 7:25 ` Sam Ravnborg 2020-04-09 7:35 ` H. Nikolaus Schaller 2020-04-09 12:20 ` Sam Ravnborg [this message] 2020-04-09 12:20 ` Sam Ravnborg 2020-04-09 12:41 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 2/8] drm: ingenic-drm: add MODULE_DEVICE_TABLE H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-05-04 1:32 ` Paul Cercueil 2020-05-04 1:32 ` Paul Cercueil 2020-03-29 17:35 ` [RFC v3 3/8] drm: ingenic-drm: add support for ingenic,jz4780-lcd H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 4/8] dt-bindings: display: add ingenic-jz4780-hdmi DT Schema H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-04-09 7:12 ` Sam Ravnborg 2020-04-09 7:12 ` Sam Ravnborg 2020-03-29 17:35 ` [RFC v3 5/8] drm: ingenic: add jz4780 Synopsys HDMI driver H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 6/8] MIPS: DTS: jz4780: account for Synopsys HDMI driver and LCD controller H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 7/8] MIPS: DTS: CI20: add HDMI setup H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller 2020-03-29 17:35 ` [RFC v3 8/8] MIPS: CI20: defconfig: configure for DRM_DW_HDMI_JZ4780 H. Nikolaus Schaller 2020-03-29 17:35 ` H. Nikolaus Schaller
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200409122054.GA5683@ravnborg.org \ --to=sam@ravnborg.org \ --cc=airlied@linux.ie \ --cc=ak@linux.intel.com \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=ebiederm@xmission.com \ --cc=geert+renesas@glider.be \ --cc=hns@goldelico.com \ --cc=keescook@chromium.org \ --cc=krzk@kernel.org \ --cc=letux-kernel@openphoenux.org \ --cc=linus.walleij@linaro.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mips-creator-ci20-dev@googlegroups.com \ --cc=miquel.raynal@bootlin.com \ --cc=paul@boddie.org.uk \ --cc=paul@crapouillou.net \ --cc=paulburton@kernel.org \ --cc=ralf@linux-mips.org \ --cc=robh+dt@kernel.org \ --cc=tsbogend@alpha.franken.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.