| Message ID | 20201007012438.27970-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mxsfb: Allow overriding bus width | expand |
On 10/7/20 3:24 AM, Laurent Pinchart wrote: [...] > +properties: > + compatible: > + enum: > + - fsl,imx23-lcdif > + - fsl,imx28-lcdif > + - fsl,imx6sx-lcdif > + - fsl,imx8mq-lcdif There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, so the DT must specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4). Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or dropped from the YAML file or neither ? [...]
On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote: > On 10/7/20 3:24 AM, Laurent Pinchart wrote: > [...] > > +properties: > > + compatible: > > + enum: > > + - fsl,imx23-lcdif > > + - fsl,imx28-lcdif > > + - fsl,imx6sx-lcdif > > + - fsl,imx8mq-lcdif > > There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, > so the DT must specify compatible = "fsl,imx8mq-lcdif", > "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4). > > Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or > dropped from the YAML file or neither ? Neither. As far as we know the block is compatible, so the DT should claim that it's compatible to "fsl,imx28-lcdif". However we don't know if there are any surprises, so we add the SoC specific compatible to be able to change the driver matching later without changing the DT if the need arises. For the DT validation to pass the SoC specific compatible needs to be documented, even if it currently unused by the driver. Regards, Lucas
On 10/7/20 10:43 AM, Lucas Stach wrote: > On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote: >> On 10/7/20 3:24 AM, Laurent Pinchart wrote: >> [...] >>> +properties: >>> + compatible: >>> + enum: >>> + - fsl,imx23-lcdif >>> + - fsl,imx28-lcdif >>> + - fsl,imx6sx-lcdif >>> + - fsl,imx8mq-lcdif >> >> There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, >> so the DT must specify compatible = "fsl,imx8mq-lcdif", >> "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4). >> >> Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or >> dropped from the YAML file or neither ? > > Neither. As far as we know the block is compatible, so the DT should > claim that it's compatible to "fsl,imx28-lcdif". However we don't know > if there are any surprises, so we add the SoC specific compatible to be > able to change the driver matching later without changing the DT if the > need arises. For the DT validation to pass the SoC specific compatible > needs to be documented, even if it currently unused by the driver. What in that binding says you should specify compatible = "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ?
Hi Marek, On Wed, Oct 07, 2020 at 10:55:24AM +0200, Marek Vasut wrote: > On 10/7/20 10:43 AM, Lucas Stach wrote: > > On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote: > >> On 10/7/20 3:24 AM, Laurent Pinchart wrote: > >> [...] > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - fsl,imx23-lcdif > >>> + - fsl,imx28-lcdif > >>> + - fsl,imx6sx-lcdif > >>> + - fsl,imx8mq-lcdif > >> > >> There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, > >> so the DT must specify compatible = "fsl,imx8mq-lcdif", > >> "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4). > >> > >> Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or > >> dropped from the YAML file or neither ? > > > > Neither. As far as we know the block is compatible, so the DT should > > claim that it's compatible to "fsl,imx28-lcdif". However we don't know > > if there are any surprises, so we add the SoC specific compatible to be > > able to change the driver matching later without changing the DT if the > > need arises. For the DT validation to pass the SoC specific compatible > > needs to be documented, even if it currently unused by the driver. > > What in that binding says you should specify compatible = > "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", > "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ? Nothing, until the next patch :-) This patch only handles the YAML conversion but doesn't fix issues.
On 10/7/20 3:33 PM, Laurent Pinchart wrote: > Hi Marek, > > On Wed, Oct 07, 2020 at 10:55:24AM +0200, Marek Vasut wrote: >> On 10/7/20 10:43 AM, Lucas Stach wrote: >>> On Mi, 2020-10-07 at 10:32 +0200, Marek Vasut wrote: >>>> On 10/7/20 3:24 AM, Laurent Pinchart wrote: >>>> [...] >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - fsl,imx23-lcdif >>>>> + - fsl,imx28-lcdif >>>>> + - fsl,imx6sx-lcdif >>>>> + - fsl,imx8mq-lcdif >>>> >>>> There is no fsl,imx8mq-lcdif in drivers/gpu/drm/mxsfb/mxsfb_drv.c, >>>> so the DT must specify compatible = "fsl,imx8mq-lcdif", >>>> "fsl,imx28-lcdif" (since imx28 is the oldest SoC with LCDIF V4). >>>> >>>> Should the compatible be added to drivers/gpu/drm/mxsfb/mxsfb_drv.c or >>>> dropped from the YAML file or neither ? >>> >>> Neither. As far as we know the block is compatible, so the DT should >>> claim that it's compatible to "fsl,imx28-lcdif". However we don't know >>> if there are any surprises, so we add the SoC specific compatible to be >>> able to change the driver matching later without changing the DT if the >>> need arises. For the DT validation to pass the SoC specific compatible >>> needs to be documented, even if it currently unused by the driver. >> >> What in that binding says you should specify compatible = >> "fsl,imx8mq-lcdif", "fsl,imx28-lcdif"; and not e.g. "fsl,imx8mq-lcdif", >> "fsl,imx23-lcdif" or simply "fsl,imx8mq-lcdif" ? > > Nothing, until the next patch :-) This patch only handles the YAML > conversion but doesn't fix issues. Good, thanks !
On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote: > Convert the mxsfb binding to YAML. The deprecated binding is dropped, as > neither the DT sources nor the driver support it anymore. The converted > binding is named fsl,lcdif.yaml to match the usual bindings naming > scheme. > > The compatible strings are messy, and DT sources use different kinds of > combination of documented and undocumented values. Keep it simple for > now, and update the example to make it valid. Aligning the binding with > the existing DT sources will be performed separately. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > -- > Changes since v1: > > - Drop unneeded quotes in string > - Replace minItems with maxItems in conditional check > - Add blank line before ... > - Squash the rename in this commit > --- > .../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ > .../devicetree/bindings/display/mxsfb.txt | 87 ------------- > MAINTAINERS | 2 +- > 3 files changed, 117 insertions(+), 88 deletions(-) > create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml > delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > new file mode 100644 > index 000000000000..063bb8c58114 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > @@ -0,0 +1,116 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale/NXP i.MX LCD Interface (LCDIF) > + > +maintainers: > + - Marek Vasut <marex@denx.de> > + - Stefan Agner <stefan@agner.ch> > + > +description: | > + (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs. > + > +properties: > + compatible: > + enum: > + - fsl,imx23-lcdif > + - fsl,imx28-lcdif > + - fsl,imx6sx-lcdif > + - fsl,imx8mq-lcdif > + > + reg: > + maxItems: 1 > + > + clocks: > + items: > + - description: Pixel clock > + - description: Bus clock > + - description: Display AXI clock > + minItems: 1 > + > + clock-names: > + items: > + - const: pix > + - const: axi > + - const: disp_axi > + minItems: 1 > + > + interrupts: > + maxItems: 1 > + > + port: > + description: The LCDIF output port > + type: object > + > + properties: > + endpoint: What happened on the graph binding schema work? I started a meta-schema for it BTW. You can drop all the endpoint parts. With that, Reviewed-by: Rob Herring <robh@kernel.org> > + type: object > + > + properties: > + remote-endpoint: > + $ref: /schemas/types.yaml#/definitions/phandle > + > + required: > + - remote-endpoint > + > + additionalProperties: false > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - clocks > + - interrupts > + - port > + > +additionalProperties: false > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: fsl,imx6sx-lcdif > + then: > + properties: > + clocks: > + minItems: 2 > + maxItems: 3 > + clock-names: > + minItems: 2 > + maxItems: 3 > + required: > + - clock-names > + else: > + properties: > + clocks: > + maxItems: 1 > + clock-names: > + maxItems: 1 > + > +examples: > + - | > + #include <dt-bindings/clock/imx6sx-clock.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + display-controller@2220000 { > + compatible = "fsl,imx6sx-lcdif"; > + reg = <0x02220000 0x4000>; > + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > + <&clks IMX6SX_CLK_LCDIF_APB>, > + <&clks IMX6SX_CLK_DISPLAY_AXI>; > + clock-names = "pix", "axi", "disp_axi"; > + > + port { > + endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + > +... > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt > deleted file mode 100644 > index c985871c46b3..000000000000 > --- a/Documentation/devicetree/bindings/display/mxsfb.txt > +++ /dev/null > @@ -1,87 +0,0 @@ > -* Freescale MXS LCD Interface (LCDIF) > - > -New bindings: > -============= > -Required properties: > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > - Should be "fsl,imx28-lcdif" for i.MX28. > - Should be "fsl,imx6sx-lcdif" for i.MX6SX. > - Should be "fsl,imx8mq-lcdif" for i.MX8MQ. > -- reg: Address and length of the register set for LCDIF > -- interrupts: Should contain LCDIF interrupt > -- clocks: A list of phandle + clock-specifier pairs, one for each > - entry in 'clock-names'. > -- clock-names: A list of clock names. For MXSFB it should contain: > - - "pix" for the LCDIF block clock > - - (MX6SX-only) "axi", "disp_axi" for the bus interface clock > - > -Required sub-nodes: > - - port: The connection to an encoder chip. > - > -Example: > - > - lcdif1: display-controller@2220000 { > - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; > - reg = <0x02220000 0x4000>; > - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > - <&clks IMX6SX_CLK_LCDIF_APB>, > - <&clks IMX6SX_CLK_DISPLAY_AXI>; > - clock-names = "pix", "axi", "disp_axi"; > - > - port { > - parallel_out: endpoint { > - remote-endpoint = <&panel_in_parallel>; > - }; > - }; > - }; > - > -Deprecated bindings: > -==================== > -Required properties: > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > - Should be "fsl,imx28-lcdif" for i.MX28. > -- reg: Address and length of the register set for LCDIF > -- interrupts: Should contain LCDIF interrupts > -- display: phandle to display node (see below for details) > - > -* display node > - > -Required properties: > -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. > -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>. > - > -Required sub-node: > -- display-timings: Refer to binding doc display-timing.txt for details. > - > -Examples: > - > -lcdif@80030000 { > - compatible = "fsl,imx28-lcdif"; > - reg = <0x80030000 2000>; > - interrupts = <38 86>; > - > - display: display { > - bits-per-pixel = <32>; > - bus-width = <24>; > - > - display-timings { > - native-mode = <&timing0>; > - timing0: timing0 { > - clock-frequency = <33500000>; > - hactive = <800>; > - vactive = <480>; > - hfront-porch = <164>; > - hback-porch = <89>; > - hsync-len = <10>; > - vback-porch = <23>; > - vfront-porch = <10>; > - vsync-len = <10>; > - hsync-active = <0>; > - vsync-active = <0>; > - de-active = <1>; > - pixelclk-active = <0>; > - }; > - }; > - }; > -}; > diff --git a/MAINTAINERS b/MAINTAINERS > index f0dd1f01703a..87e20680c104 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11910,7 +11910,7 @@ M: Stefan Agner <stefan@agner.ch> > L: dri-devel@lists.freedesktop.org > S: Supported > T: git git://anongit.freedesktop.org/drm/drm-misc > -F: Documentation/devicetree/bindings/display/mxsfb.txt > +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml > F: drivers/gpu/drm/mxsfb/ > > MYLEX DAC960 PCI RAID Controller > -- > Regards, > > Laurent Pinchart >
On Wed, Oct 07, 2020 at 11:00:20AM -0500, Rob Herring wrote: > On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote: > > Convert the mxsfb binding to YAML. The deprecated binding is dropped, as > > neither the DT sources nor the driver support it anymore. The converted > > binding is named fsl,lcdif.yaml to match the usual bindings naming > > scheme. > > > > The compatible strings are messy, and DT sources use different kinds of > > combination of documented and undocumented values. Keep it simple for > > now, and update the example to make it valid. Aligning the binding with > > the existing DT sources will be performed separately. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > -- > > Changes since v1: > > > > - Drop unneeded quotes in string > > - Replace minItems with maxItems in conditional check > > - Add blank line before ... > > - Squash the rename in this commit > > --- > > .../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ > > .../devicetree/bindings/display/mxsfb.txt | 87 ------------- > > MAINTAINERS | 2 +- > > 3 files changed, 117 insertions(+), 88 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt > > > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > new file mode 100644 > > index 000000000000..063bb8c58114 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > @@ -0,0 +1,116 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale/NXP i.MX LCD Interface (LCDIF) > > + > > +maintainers: > > + - Marek Vasut <marex@denx.de> > > + - Stefan Agner <stefan@agner.ch> > > + > > +description: | > > + (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs. > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx23-lcdif > > + - fsl,imx28-lcdif > > + - fsl,imx6sx-lcdif > > + - fsl,imx8mq-lcdif > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Pixel clock > > + - description: Bus clock > > + - description: Display AXI clock > > + minItems: 1 > > + > > + clock-names: > > + items: > > + - const: pix > > + - const: axi > > + - const: disp_axi > > + minItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + port: > > + description: The LCDIF output port > > + type: object > > + > > + properties: > > + endpoint: > > What happened on the graph binding schema work? I started a meta-schema > for it BTW. > > You can drop all the endpoint parts. With that, NM, I see in patch 3 you need it. > > Reviewed-by: Rob Herring <robh@kernel.org> > > > + type: object > > + > > + properties: > > + remote-endpoint: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + > > + required: > > + - remote-endpoint > > + > > + additionalProperties: false > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - interrupts > > + - port > > + > > +additionalProperties: false > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx6sx-lcdif > > + then: > > + properties: > > + clocks: > > + minItems: 2 > > + maxItems: 3 > > + clock-names: > > + minItems: 2 > > + maxItems: 3 > > + required: > > + - clock-names > > + else: > > + properties: > > + clocks: > > + maxItems: 1 > > + clock-names: > > + maxItems: 1 > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/imx6sx-clock.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + display-controller@2220000 { > > + compatible = "fsl,imx6sx-lcdif"; > > + reg = <0x02220000 0x4000>; > > + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > > + <&clks IMX6SX_CLK_LCDIF_APB>, > > + <&clks IMX6SX_CLK_DISPLAY_AXI>; > > + clock-names = "pix", "axi", "disp_axi"; > > + > > + port { > > + endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > + }; > > + > > +... > > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt > > deleted file mode 100644 > > index c985871c46b3..000000000000 > > --- a/Documentation/devicetree/bindings/display/mxsfb.txt > > +++ /dev/null > > @@ -1,87 +0,0 @@ > > -* Freescale MXS LCD Interface (LCDIF) > > - > > -New bindings: > > -============= > > -Required properties: > > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > > - Should be "fsl,imx28-lcdif" for i.MX28. > > - Should be "fsl,imx6sx-lcdif" for i.MX6SX. > > - Should be "fsl,imx8mq-lcdif" for i.MX8MQ. > > -- reg: Address and length of the register set for LCDIF > > -- interrupts: Should contain LCDIF interrupt > > -- clocks: A list of phandle + clock-specifier pairs, one for each > > - entry in 'clock-names'. > > -- clock-names: A list of clock names. For MXSFB it should contain: > > - - "pix" for the LCDIF block clock > > - - (MX6SX-only) "axi", "disp_axi" for the bus interface clock > > - > > -Required sub-nodes: > > - - port: The connection to an encoder chip. > > - > > -Example: > > - > > - lcdif1: display-controller@2220000 { > > - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; > > - reg = <0x02220000 0x4000>; > > - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > > - clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > > - <&clks IMX6SX_CLK_LCDIF_APB>, > > - <&clks IMX6SX_CLK_DISPLAY_AXI>; > > - clock-names = "pix", "axi", "disp_axi"; > > - > > - port { > > - parallel_out: endpoint { > > - remote-endpoint = <&panel_in_parallel>; > > - }; > > - }; > > - }; > > - > > -Deprecated bindings: > > -==================== > > -Required properties: > > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > > - Should be "fsl,imx28-lcdif" for i.MX28. > > -- reg: Address and length of the register set for LCDIF > > -- interrupts: Should contain LCDIF interrupts > > -- display: phandle to display node (see below for details) > > - > > -* display node > > - > > -Required properties: > > -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. > > -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>. > > - > > -Required sub-node: > > -- display-timings: Refer to binding doc display-timing.txt for details. > > - > > -Examples: > > - > > -lcdif@80030000 { > > - compatible = "fsl,imx28-lcdif"; > > - reg = <0x80030000 2000>; > > - interrupts = <38 86>; > > - > > - display: display { > > - bits-per-pixel = <32>; > > - bus-width = <24>; > > - > > - display-timings { > > - native-mode = <&timing0>; > > - timing0: timing0 { > > - clock-frequency = <33500000>; > > - hactive = <800>; > > - vactive = <480>; > > - hfront-porch = <164>; > > - hback-porch = <89>; > > - hsync-len = <10>; > > - vback-porch = <23>; > > - vfront-porch = <10>; > > - vsync-len = <10>; > > - hsync-active = <0>; > > - vsync-active = <0>; > > - de-active = <1>; > > - pixelclk-active = <0>; > > - }; > > - }; > > - }; > > -}; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f0dd1f01703a..87e20680c104 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -11910,7 +11910,7 @@ M: Stefan Agner <stefan@agner.ch> > > L: dri-devel@lists.freedesktop.org > > S: Supported > > T: git git://anongit.freedesktop.org/drm/drm-misc > > -F: Documentation/devicetree/bindings/display/mxsfb.txt > > +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > F: drivers/gpu/drm/mxsfb/ > > > > MYLEX DAC960 PCI RAID Controller > > -- > > Regards, > > > > Laurent Pinchart > >
Hi Rob, On Wed, Oct 07, 2020 at 11:00:20AM -0500, Rob Herring wrote: > On Wed, Oct 07, 2020 at 04:24:32AM +0300, Laurent Pinchart wrote: > > Convert the mxsfb binding to YAML. The deprecated binding is dropped, as > > neither the DT sources nor the driver support it anymore. The converted > > binding is named fsl,lcdif.yaml to match the usual bindings naming > > scheme. > > > > The compatible strings are messy, and DT sources use different kinds of > > combination of documented and undocumented values. Keep it simple for > > now, and update the example to make it valid. Aligning the binding with > > the existing DT sources will be performed separately. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > -- > > Changes since v1: > > > > - Drop unneeded quotes in string > > - Replace minItems with maxItems in conditional check > > - Add blank line before ... > > - Squash the rename in this commit > > --- > > .../bindings/display/fsl,lcdif.yaml | 116 ++++++++++++++++++ > > .../devicetree/bindings/display/mxsfb.txt | 87 ------------- > > MAINTAINERS | 2 +- > > 3 files changed, 117 insertions(+), 88 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > delete mode 100644 Documentation/devicetree/bindings/display/mxsfb.txt > > > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > new file mode 100644 > > index 000000000000..063bb8c58114 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > @@ -0,0 +1,116 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale/NXP i.MX LCD Interface (LCDIF) > > + > > +maintainers: > > + - Marek Vasut <marex@denx.de> > > + - Stefan Agner <stefan@agner.ch> > > + > > +description: | > > + (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs. > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx23-lcdif > > + - fsl,imx28-lcdif > > + - fsl,imx6sx-lcdif > > + - fsl,imx8mq-lcdif > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Pixel clock > > + - description: Bus clock > > + - description: Display AXI clock > > + minItems: 1 > > + > > + clock-names: > > + items: > > + - const: pix > > + - const: axi > > + - const: disp_axi > > + minItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + port: > > + description: The LCDIF output port > > + type: object > > + > > + properties: > > + endpoint: > > What happened on the graph binding schema work? Still on my todo list, I hope to switch back to that task in the not too distant future. > I started a meta-schema for it BTW. Nice :-) Is it available in a public tree ? > You can drop all the endpoint parts. With that, > > Reviewed-by: Rob Herring <robh@kernel.org> > > > + type: object > > + > > + properties: > > + remote-endpoint: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + > > + required: > > + - remote-endpoint > > + > > + additionalProperties: false > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - interrupts > > + - port > > + > > +additionalProperties: false > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx6sx-lcdif > > + then: > > + properties: > > + clocks: > > + minItems: 2 > > + maxItems: 3 > > + clock-names: > > + minItems: 2 > > + maxItems: 3 > > + required: > > + - clock-names > > + else: > > + properties: > > + clocks: > > + maxItems: 1 > > + clock-names: > > + maxItems: 1 > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/imx6sx-clock.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + display-controller@2220000 { > > + compatible = "fsl,imx6sx-lcdif"; > > + reg = <0x02220000 0x4000>; > > + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > > + <&clks IMX6SX_CLK_LCDIF_APB>, > > + <&clks IMX6SX_CLK_DISPLAY_AXI>; > > + clock-names = "pix", "axi", "disp_axi"; > > + > > + port { > > + endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > + }; > > + > > +... > > diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt > > deleted file mode 100644 > > index c985871c46b3..000000000000 > > --- a/Documentation/devicetree/bindings/display/mxsfb.txt > > +++ /dev/null > > @@ -1,87 +0,0 @@ > > -* Freescale MXS LCD Interface (LCDIF) > > - > > -New bindings: > > -============= > > -Required properties: > > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > > - Should be "fsl,imx28-lcdif" for i.MX28. > > - Should be "fsl,imx6sx-lcdif" for i.MX6SX. > > - Should be "fsl,imx8mq-lcdif" for i.MX8MQ. > > -- reg: Address and length of the register set for LCDIF > > -- interrupts: Should contain LCDIF interrupt > > -- clocks: A list of phandle + clock-specifier pairs, one for each > > - entry in 'clock-names'. > > -- clock-names: A list of clock names. For MXSFB it should contain: > > - - "pix" for the LCDIF block clock > > - - (MX6SX-only) "axi", "disp_axi" for the bus interface clock > > - > > -Required sub-nodes: > > - - port: The connection to an encoder chip. > > - > > -Example: > > - > > - lcdif1: display-controller@2220000 { > > - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; > > - reg = <0x02220000 0x4000>; > > - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > > - clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, > > - <&clks IMX6SX_CLK_LCDIF_APB>, > > - <&clks IMX6SX_CLK_DISPLAY_AXI>; > > - clock-names = "pix", "axi", "disp_axi"; > > - > > - port { > > - parallel_out: endpoint { > > - remote-endpoint = <&panel_in_parallel>; > > - }; > > - }; > > - }; > > - > > -Deprecated bindings: > > -==================== > > -Required properties: > > -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. > > - Should be "fsl,imx28-lcdif" for i.MX28. > > -- reg: Address and length of the register set for LCDIF > > -- interrupts: Should contain LCDIF interrupts > > -- display: phandle to display node (see below for details) > > - > > -* display node > > - > > -Required properties: > > -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. > > -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>. > > - > > -Required sub-node: > > -- display-timings: Refer to binding doc display-timing.txt for details. > > - > > -Examples: > > - > > -lcdif@80030000 { > > - compatible = "fsl,imx28-lcdif"; > > - reg = <0x80030000 2000>; > > - interrupts = <38 86>; > > - > > - display: display { > > - bits-per-pixel = <32>; > > - bus-width = <24>; > > - > > - display-timings { > > - native-mode = <&timing0>; > > - timing0: timing0 { > > - clock-frequency = <33500000>; > > - hactive = <800>; > > - vactive = <480>; > > - hfront-porch = <164>; > > - hback-porch = <89>; > > - hsync-len = <10>; > > - vback-porch = <23>; > > - vfront-porch = <10>; > > - vsync-len = <10>; > > - hsync-active = <0>; > > - vsync-active = <0>; > > - de-active = <1>; > > - pixelclk-active = <0>; > > - }; > > - }; > > - }; > > -}; > > diff --git a/MAINTAINERS b/MAINTAINERS > > index f0dd1f01703a..87e20680c104 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -11910,7 +11910,7 @@ M: Stefan Agner <stefan@agner.ch> > > L: dri-devel@lists.freedesktop.org > > S: Supported > > T: git git://anongit.freedesktop.org/drm/drm-misc > > -F: Documentation/devicetree/bindings/display/mxsfb.txt > > +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml > > F: drivers/gpu/drm/mxsfb/ > > > > MYLEX DAC960 PCI RAID Controller
hi Laurent,
Do you mind me adding a DT property in the old format now? If so, I'd
appreciated an ack in this thread:
https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sigxcpu.org/
If it causes extra work for you and want to resend your series soon, I'll
gladly delay them for later.
thanks,
martin
Hi Martin, On Fri, Jan 15, 2021 at 08:59:18AM +0100, Martin Kepplinger wrote: > hi Laurent, > > Do you mind me adding a DT property in the old format now? If so, I'd > appreciated an ack in this thread: > https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sigxcpu.org/ > > If it causes extra work for you and want to resend your series soon, I'll > gladly delay them for later. I think the conversion ot YAML is ready. I've split it from the rest of my series, and posted a v3, asking Rob to merge it. Would you mind rebasing the addition of the new properties on top ?
Am 15. Jänner 2021 23:25:14 MEZ schrieb Laurent Pinchart <laurent.pinchart@ideasonboard.com>: >Hi Martin, > >On Fri, Jan 15, 2021 at 08:59:18AM +0100, Martin Kepplinger wrote: >> hi Laurent, >> >> Do you mind me adding a DT property in the old format now? If so, I'd >> appreciated an ack in this thread: >> >https://lore.kernel.org/linux-arm-kernel/20201201134638.GA305734@bogon.m.sigxcpu.org/ >> >> If it causes extra work for you and want to resend your series soon, >I'll >> gladly delay them for later. > >I think the conversion ot YAML is ready. I've split it from the rest of >my series, and posted a v3, asking Rob to merge it. Would you mind >rebasing the addition of the new properties on top ? Hi Laurent, thanks for the timely answer. sounds good; I'll rebase. martin
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml new file mode 100644 index 000000000000..063bb8c58114 --- /dev/null +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/fsl,lcdif.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale/NXP i.MX LCD Interface (LCDIF) + +maintainers: + - Marek Vasut <marex@denx.de> + - Stefan Agner <stefan@agner.ch> + +description: | + (e)LCDIF display controller found in the Freescale/NXP i.MX SoCs. + +properties: + compatible: + enum: + - fsl,imx23-lcdif + - fsl,imx28-lcdif + - fsl,imx6sx-lcdif + - fsl,imx8mq-lcdif + + reg: + maxItems: 1 + + clocks: + items: + - description: Pixel clock + - description: Bus clock + - description: Display AXI clock + minItems: 1 + + clock-names: + items: + - const: pix + - const: axi + - const: disp_axi + minItems: 1 + + interrupts: + maxItems: 1 + + port: + description: The LCDIF output port + type: object + + properties: + endpoint: + type: object + + properties: + remote-endpoint: + $ref: /schemas/types.yaml#/definitions/phandle + + required: + - remote-endpoint + + additionalProperties: false + + additionalProperties: false + +required: + - compatible + - reg + - clocks + - interrupts + - port + +additionalProperties: false + +allOf: + - if: + properties: + compatible: + contains: + const: fsl,imx6sx-lcdif + then: + properties: + clocks: + minItems: 2 + maxItems: 3 + clock-names: + minItems: 2 + maxItems: 3 + required: + - clock-names + else: + properties: + clocks: + maxItems: 1 + clock-names: + maxItems: 1 + +examples: + - | + #include <dt-bindings/clock/imx6sx-clock.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + display-controller@2220000 { + compatible = "fsl,imx6sx-lcdif"; + reg = <0x02220000 0x4000>; + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, + <&clks IMX6SX_CLK_LCDIF_APB>, + <&clks IMX6SX_CLK_DISPLAY_AXI>; + clock-names = "pix", "axi", "disp_axi"; + + port { + endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + +... diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt deleted file mode 100644 index c985871c46b3..000000000000 --- a/Documentation/devicetree/bindings/display/mxsfb.txt +++ /dev/null @@ -1,87 +0,0 @@ -* Freescale MXS LCD Interface (LCDIF) - -New bindings: -============= -Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. - Should be "fsl,imx28-lcdif" for i.MX28. - Should be "fsl,imx6sx-lcdif" for i.MX6SX. - Should be "fsl,imx8mq-lcdif" for i.MX8MQ. -- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupt -- clocks: A list of phandle + clock-specifier pairs, one for each - entry in 'clock-names'. -- clock-names: A list of clock names. For MXSFB it should contain: - - "pix" for the LCDIF block clock - - (MX6SX-only) "axi", "disp_axi" for the bus interface clock - -Required sub-nodes: - - port: The connection to an encoder chip. - -Example: - - lcdif1: display-controller@2220000 { - compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; - reg = <0x02220000 0x4000>; - interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>, - <&clks IMX6SX_CLK_LCDIF_APB>, - <&clks IMX6SX_CLK_DISPLAY_AXI>; - clock-names = "pix", "axi", "disp_axi"; - - port { - parallel_out: endpoint { - remote-endpoint = <&panel_in_parallel>; - }; - }; - }; - -Deprecated bindings: -==================== -Required properties: -- compatible: Should be "fsl,imx23-lcdif" for i.MX23. - Should be "fsl,imx28-lcdif" for i.MX28. -- reg: Address and length of the register set for LCDIF -- interrupts: Should contain LCDIF interrupts -- display: phandle to display node (see below for details) - -* display node - -Required properties: -- bits-per-pixel: <16> for RGB565, <32> for RGB888/666. -- bus-width: number of data lines. Could be <8>, <16>, <18> or <24>. - -Required sub-node: -- display-timings: Refer to binding doc display-timing.txt for details. - -Examples: - -lcdif@80030000 { - compatible = "fsl,imx28-lcdif"; - reg = <0x80030000 2000>; - interrupts = <38 86>; - - display: display { - bits-per-pixel = <32>; - bus-width = <24>; - - display-timings { - native-mode = <&timing0>; - timing0: timing0 { - clock-frequency = <33500000>; - hactive = <800>; - vactive = <480>; - hfront-porch = <164>; - hback-porch = <89>; - hsync-len = <10>; - vback-porch = <23>; - vfront-porch = <10>; - vsync-len = <10>; - hsync-active = <0>; - vsync-active = <0>; - de-active = <1>; - pixelclk-active = <0>; - }; - }; - }; -}; diff --git a/MAINTAINERS b/MAINTAINERS index f0dd1f01703a..87e20680c104 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11910,7 +11910,7 @@ M: Stefan Agner <stefan@agner.ch> L: dri-devel@lists.freedesktop.org S: Supported T: git git://anongit.freedesktop.org/drm/drm-misc -F: Documentation/devicetree/bindings/display/mxsfb.txt +F: Documentation/devicetree/bindings/display/fsl,lcdif.yaml F: drivers/gpu/drm/mxsfb/ MYLEX DAC960 PCI RAID Controller