On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> In V1, discussed with Doug and Linus [1], we need break out as separate
> driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
> and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
> controller, they have some common CMDS. So add new documentation for
> this panels.
It'd be good to note in the commit message that the 3v3 supply is not
present on these panels, given it was present in the other binding and
not here.
> [1]: https://lore.kernel.org/all/CACRpkdbzYZAS0=zBQJUC4CB2wj4s1h6n6aSAZQvdMV95r3zRUw@mail.gmail.com
>
> Signed-off-by: Cong Yang <[email protected]>
> ---
> Chage since V3:
>
> - Update commit message.
>
> V2: https://lore.kernel.org/all/20240422090310.3311429-2-yangcong5@huaqin.corp-partner.google.com
>
> ---
> .../display/panel/boe,tv101wum-nl6.yaml | 2 -
> .../bindings/display/panel/himax,hx83102.yaml | 73 +++++++++++++++++++
> 2 files changed, 73 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> index 906ef62709b8..53fb35f5c9de 100644
> --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> @@ -32,8 +32,6 @@ properties:
> - innolux,hj110iz-01a
> # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
> - starry,2081101qfh032011-53g
> - # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> - - starry,himax83102-j02
> # STARRY ili9882t 10.51" WUXGA TFT LCD panel
> - starry,ili9882t
>
> diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> new file mode 100644
> index 000000000000..2e0cd6998ba8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
Filename matching a compatible please. What you've done here makes it
seem like there's a fallback compatible missing, given this looks like
the LCD panel controller and the starry compatible below is an LCD panel.
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax HX83102 MIPI-DSI LCD panel controller
> +
> +maintainers:
> + - Cong Yang <[email protected]>
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> + - starry,himax83102-j02
> +
> + reg:
> + description: the virtual channel number of a DSI peripheral
> +
> + enable-gpios:
> + description: a GPIO spec for the enable pin
> +
> + pp1800-supply:
> + description: core voltage supply
> +
> + avdd-supply:
> + description: phandle of the regulator that provides positive voltage
> +
> + avee-supply:
> + description: phandle of the regulator that provides negative voltage
> +
> + backlight:
> + description: phandle of the backlight device attached to the panel
I'm not sure why this was given a description when port or rotation
was not.
Otherwise, this looks fine to me.
Cheers,
Conor.
> +
> + port: true
> + rotation: true
> +
> +required:
> + - compatible
> + - reg
> + - enable-gpios
> + - pp1800-supply
> + - avdd-supply
> + - avee-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + dsi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + panel@0 {
> + compatible = "starry,himax83102-j02";
> + reg = <0>;
> + enable-gpios = <&pio 45 0>;
> + avdd-supply = <&ppvarn_lcd>;
> + avee-supply = <&ppvarp_lcd>;
> + pp1800-supply = <&pp1800_lcd>;
> + backlight = <&backlight_lcd0>;
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&dsi_out>;
> + };
> + };
> + };
> + };
> +
> +...
> --
> 2.25.1
>
Hi,
Thanks for review.
Conor Dooley <[email protected]> 于2024年4月25日周四 00:55写道：
>
> On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> > In V1, discussed with Doug and Linus [1], we need break out as separate
> > driver for the himax83102-j02 controller. Beacuse "starry,himax83102-j02"
> > and in this series "BOE nv110wum-l60" "IVO t109nw41" panels use same
> > controller, they have some common CMDS. So add new documentation for
> > this panels.
>
> It'd be good to note in the commit message that the 3v3 supply is not
> present on these panels, given it was present in the other binding and
> not here.
Got it, fix in V4,thanks.
>
> > [1]: https://lore.kernel.org/all/CACRpkdbzYZAS0=zBQJUC4CB2wj4s1h6n6aSAZQvdMV95r3zRUw@mail.gmail.com
> >
> > Signed-off-by: Cong Yang <[email protected]>
> > ---
> > Chage since V3:
> >
> > - Update commit message.
> >
> > V2: https://lore.kernel.org/all/20240422090310.3311429-2-yangcong5@huaqin.corp-partner.google.com
> >
> > ---
> > .../display/panel/boe,tv101wum-nl6.yaml | 2 -
> > .../bindings/display/panel/himax,hx83102.yaml | 73 +++++++++++++++++++
> > 2 files changed, 73 insertions(+), 2 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > index 906ef62709b8..53fb35f5c9de 100644
> > --- a/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/boe,tv101wum-nl6.yaml
> > @@ -32,8 +32,6 @@ properties:
> > - innolux,hj110iz-01a
> > # STARRY 2081101QFH032011-53G 10.1" WUXGA TFT LCD panel
> > - starry,2081101qfh032011-53g
> > - # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > - - starry,himax83102-j02
> > # STARRY ili9882t 10.51" WUXGA TFT LCD panel
> > - starry,ili9882t
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> > new file mode 100644
> > index 000000000000..2e0cd6998ba8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
>
> Filename matching a compatible please. What you've done here makes it
> seem like there's a fallback compatible missing, given this looks like
> the LCD panel controller and the starry compatible below is an LCD panel.
So change the filename to starry,himax83102-j02.yaml?
>
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Himax HX83102 MIPI-DSI LCD panel controller
> > +
> > +maintainers:
> > + - Cong Yang <[email protected]>
> > +
> > +allOf:
> > + - $ref: panel-common.yaml#
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > + - starry,himax83102-j02
> > +
> > + reg:
> > + description: the virtual channel number of a DSI peripheral
> > +
> > + enable-gpios:
> > + description: a GPIO spec for the enable pin
> > +
> > + pp1800-supply:
> > + description: core voltage supply
> > +
> > + avdd-supply:
> > + description: phandle of the regulator that provides positive voltage
> > +
> > + avee-supply:
> > + description: phandle of the regulator that provides negative voltage
> > +
> > + backlight:
> > + description: phandle of the backlight device attached to the panel
>
> I'm not sure why this was given a description when port or rotation
> was not.
So change it to backlight: true ?
Thanks.
>
> Otherwise, this looks fine to me.
>
> Cheers,
> Conor.
>
> > +
> > + port: true
> > + rotation: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - enable-gpios
> > + - pp1800-supply
> > + - avdd-supply
> > + - avee-supply
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + dsi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + panel@0 {
> > + compatible = "starry,himax83102-j02";
> > + reg = <0>;
> > + enable-gpios = <&pio 45 0>;
> > + avdd-supply = <&ppvarn_lcd>;
> > + avee-supply = <&ppvarp_lcd>;
> > + pp1800-supply = <&pp1800_lcd>;
> > + backlight = <&backlight_lcd0>;
> > + port {
> > + panel_in: endpoint {
> > + remote-endpoint = <&dsi_out>;
> > + };
> > + };
> > + };
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
On Thu, Apr 25, 2024 at 02:03:24PM +0800, cong yang wrote:
> Conor Dooley <[email protected]> 于2024年4月25日周四 00:55写道：
> > On Wed, Apr 24, 2024 at 10:30:04AM +0800, Cong Yang wrote:
> > > +++ b/Documentation/devicetree/bindings/display/panel/himax,hx83102.yaml
> >
> > Filename matching a compatible please. What you've done here makes it
> > seem like there's a fallback compatible missing, given this looks like
> > the LCD panel controller and the starry compatible below is an LCD panel.
>
> So change the filename to starry,himax83102-j02.yaml?
IDK chief, are you missing a fallback or not?
>
> >
> > > @@ -0,0 +1,73 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/panel/himax,hx83102.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Himax HX83102 MIPI-DSI LCD panel controller
Because the title here makes it seem like there should be.
> > > +maintainers:
> > > + - Cong Yang <[email protected]>
> > > +
> > > +allOf:
> > > + - $ref: panel-common.yaml#
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + # STARRY himax83102-j02 10.51" WUXGA TFT LCD panel
> > > + - starry,himax83102-j02
> > > +
> > > + reg:
> > > + description: the virtual channel number of a DSI peripheral
> > > +
> > > + enable-gpios:
> > > + description: a GPIO spec for the enable pin
> > > +
> > > + pp1800-supply:
> > > + description: core voltage supply
> > > +
> > > + avdd-supply:
> > > + description: phandle of the regulator that provides positive voltage
> > > +
> > > + avee-supply:
> > > + description: phandle of the regulator that provides negative voltage
> > > +
> > > + backlight:
> > > + description: phandle of the backlight device attached to the panel
> >
> > I'm not sure why this was given a description when port or rotation
> > was not.
>
> So change it to backlight: true ?
Sure? It is just a repeat of something already described in
panel-common.