[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200422152956.GE28105@pendragon.ideasonboard.com>
Date: Wed, 22 Apr 2020 18:29:56 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Adrian Ratiu <adrian.ratiu@...labora.com>
Cc: linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
linux-rockchip@...ts.infradead.org,
Jernej Skrabec <jernej.skrabec@...l.net>,
Heiko Stuebner <heiko@...ech.de>,
Jonas Karlman <jonas@...boo.se>, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org,
Andrzej Hajda <a.hajda@...sung.com>, kernel@...labora.com,
linux-stm32@...md-mailman.stormreply.com, linux-imx@....com,
Rob Herring <robh@...nel.org>,
Neil Armstrong <narmstrong@...libre.com>,
Fabio Estevam <festevam@...il.com>,
Adrian Pop <pop.adrian61@...il.com>,
Arnaud Ferraris <arnaud.ferraris@...labora.com>,
Sjoerd Simons <sjoerd.simons@...labora.com>,
Martyn Welch <martyn.welch@...labora.com>
Subject: Re: [PATCH v7 5/8] dt-bindings: display: add i.MX6 MIPI DSI host
controller doc
Hi Adrian,
On Wed, Apr 22, 2020 at 01:15:41PM +0300, Adrian Ratiu wrote:
> On Wed, 22 Apr 2020, Laurent Pinchart wrote:
> > On Wed, Apr 22, 2020 at 03:58:33AM +0300, Laurent Pinchart wrote:
> >> On Tue, Apr 21, 2020 at 07:16:07PM +0300, Adrian Ratiu wrote:
> >>> This provides an example DT binding for the MIPI DSI host
> >>> controller present on the i.MX6 SoC based on Synopsis
> >>> DesignWare v1.01 IP. Cc: Rob Herring <robh@...nel.org> Cc:
> >>> Neil Armstrong <narmstrong@...libre.com> Cc: Fabio Estevam
> >>> <festevam@...il.com> Cc: Laurent Pinchart
> >>> <laurent.pinchart@...asonboard.com> Cc:
> >>> devicetree@...r.kernel.org Tested-by: Adrian Pop
> >>> <pop.adrian61@...il.com> Tested-by: Arnaud Ferraris
> >>> <arnaud.ferraris@...labora.com> Signed-off-by: Sjoerd Simons
> >>> <sjoerd.simons@...labora.com> Signed-off-by: Martyn Welch
> >>> <martyn.welch@...labora.com> Signed-off-by: Adrian Ratiu
> >>> <adrian.ratiu@...labora.com> --- Changes since v6:
> >>> - Added ref to the newly created snps,dw-mipi-dsi.yaml
> >>> (Laurent) - Moved *-cells properties outside
> >>> patternProperties (Laurent) - Removed the panel port
> >>> documentation (Laurent) - Wrapped lines at 80 chars, typo
> >>> fixes, sort includes (Laurent)
> >>> Changes since v5:
> >>> - Fixed missing reg warning (Fabio) - Updated dt-schema and
> >>> fixed warnings (Rob)
> >>> Changes since v4:
> >>> - Fixed yaml binding to pass `make dt_binding_check
> >>> dtbs_check` and addressed received binding feedback (Rob)
> >>> Changes since v3:
> >>> - Added commit message (Neil) - Converted to yaml format
> >>> (Neil) - Minor dt node + driver fixes (Rob) - Added small
> >>> panel example to the host controller binding
> >>> Changes since v2:
> >>> - Fixed commit tags (Emil)
> >>> ---
> >>> .../display/imx/fsl,mipi-dsi-imx6.yaml | 135
> >>> ++++++++++++++++++ 1 file changed, 135 insertions(+) create
> >>> mode 100644
> >>> Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> >>> b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> >>> new file mode 100644 index 0000000000000..b73e3ae33a852 ---
> >>> /dev/null +++
> >>> b/Documentation/devicetree/bindings/display/imx/fsl,mipi-dsi-imx6.yaml
> >>> @@ -0,0 +1,135 @@ +# SPDX-License-Identifier: (GPL-2.0-only
> >>> OR BSD-2-Clause) +%YAML 1.2 +--- +$id:
> >>> http://devicetree.org/schemas/display/imx/fsl,mipi-dsi-imx6.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# +
> >>> +title: Freescale i.MX6 DW MIPI DSI Host Controller +
> >>> +maintainers: + - Adrian Ratiu <adrian.ratiu@...labora.com>
> >>> + +description: | + The i.MX6 DSI host controller is a
> >>> Synopsys DesignWare MIPI DSI v1.01 + IP block with a
> >>> companion PHY IP.
> >
> > I forgot to mention, if there's a companion PHY, shouldn't it be
> > referenced from the DT bindings ?
>
> I don't think so, that description was copied verbatim from the
> imx6 ref manual IIRC, the physical layer is the same for MIPI DSI
> which does TX as for MIPI CSI which does RX, but looking at the
> ref manual and how drivers are written I don't think it's
> necessary.
Does that mean that the PHY is controlled through the registers
specified by the reg property ? If so then this is fine.
> This might change if we wanted to unify the DSI and CSI drivers a
> bit, but considering the scope already associated with this patch
> series I'm a bit afraid to open a subject like that =)
That's understandable :-)
> >>> +
> >>> + These DT bindings follow the Synopsys DW MIPI DSI bindings defined in
> >>> + Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt with
> >>> + the following device-specific properties.
> >>> +
> >>> +allOf:
> >>> + - $ref: ../bridge/snps,dw-mipi-dsi.yaml#
> >>> +
> >>> +properties:
> >>> + '#address-cells':
> >>> + const: 1
> >>> +
> >>> + '#size-cells':
> >>> + const: 0
> >>> +
> >>> + compatible:
> >>> + items:
> >>> + - const: fsl,imx6q-mipi-dsi
> >>> + - const: snps,dw-mipi-dsi
> >>> +
> >>> + reg:
> >>> + maxItems: 1
> >>> +
> >>> + interrupts:
> >>> + maxItems: 1
> >>> +
> >>> + clocks:
> >>> + items:
> >>> + - description: Module Clock
> >>> + - description: DSI bus clock
> >>> +
> >>> + clock-names:
> >>> + items:
> >>> + - const: ref
> >>> + - const: pclk
> >>> +
> >>> + fsl,gpr:
> >>> + description:
> >>> + Phandle to the iomuxc-gpr region containing the multiplexer ctrl register.
> >>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>> +
> >>> + ports:
> >>> + type: object
> >>> + description: |
> >>> + A node containing DSI input & output port nodes with endpoint
> >>> + definitions as documented in
> >>> + Documentation/devicetree/bindings/media/video-interfaces.txt
> >>> + Documentation/devicetree/bindings/graph.txt
> >>> + properties:
> >>> + port@0:
> >>> + type: object
> >>> + description:
> >>> + DSI input port node, connected to the ltdc rgb output port.
> >>> +
> >>> + port@1:
> >>> + type: object
> >>> + description:
> >>> + RGB output port node, connected to a panel or a bridge input port.
> >>
> >> Isn't it the other way around, doesn't the bridge take RGB input and
> >> output DSI ? And to be precise, it's not about RGB, but about the input
> >> being parallel interface (DSI will also carry RGB).
> >>
> >> I would add
> >>
> >> required:
> >> - port@0
> >> - port@1
> >>
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +patternProperties:
> >>> + "^panel@[0-3]$":
> >>> + type: object
> >>> +
> >>> +required:
> >>> + - "#address-cells"
> >>> + - "#size-cells"
> >>> + - compatible
> >>> + - reg
> >>> + - interrupts
> >>> + - clocks
> >>> + - clock-names
> >>> + - ports
> >>> +
> >>> +examples:
> >>> + - |+
> >>> + #include <dt-bindings/clock/imx6qdl-clock.h>
> >>> + #include <dt-bindings/gpio/gpio.h>
> >>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>> +
> >>> + dsi: dsi@...0000 {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> + compatible = "fsl,imx6q-mipi-dsi", "snps,dw-mipi-dsi";
> >>> + reg = <0x021e0000 0x4000>;
> >>> + interrupts = <0 102 IRQ_TYPE_LEVEL_HIGH>;
> >>> + fsl,gpr = <&gpr>;
> >>> + clocks = <&clks IMX6QDL_CLK_MIPI_CORE_CFG>,
> >>> + <&clks IMX6QDL_CLK_MIPI_IPG>;
> >>> + clock-names = "ref", "pclk";
> >>> +
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>
> >> port@0 is missing.
> >>
> >>> + port@1 {
> >>> + reg = <1>;
> >>> + dsi_out: endpoint {
> >>> + remote-endpoint = <&panel_in>;
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> + panel@0 {
> >>> + compatible = "sharp,ls032b3sx01";
> >>> + reg = <0>;
> >>> + reset-gpios = <&gpio6 8 GPIO_ACTIVE_LOW>;
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> + port@0 {
> >>> + reg = <0>;
> >>> + panel_in: endpoint {
> >>> + remote-endpoint = <&dsi_out>;
> >>> + };
> >>> + };
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> +...
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists