[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87368vjxw9.fsf@collabora.com>
Date: Wed, 22 Apr 2020 19:19:50 +0300
From: Adrian Ratiu <adrian.ratiu@...labora.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
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
On Wed, 22 Apr 2020, Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
> 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.
>
Yes that is correct, there is just a single set of conf registers
specified via reg.
>> 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