| Message ID | 20230403124458.198631-1-festevam@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] dt-bindings: bridge: Convert Samsung MIPI DSIM bridge to yaml | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 278 lines checked |
| robh/patch-applied | fail | build log |
On 03/04/2023 14:44, Fabio Estevam wrote: > From: Jagan Teki <jagan@amarulasolutions.com> > > Samsung MIPI DSIM bridge can be found on Exynos and NXP's > i.MX8M Mini/Nano/Plus SoCs. > > Convert exynos_dsim.txt to yaml. > > Used the example node from latest Exynos SoC instead of > the one used in legacy exynos_dsim.txt. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Changes since v1: > - Added samsung,mipi-dsim.yaml entry to MAINTAINERS file (Jagan) > - Added Marek Szyprowski entry to the samsung,mipi-dsim.yaml maintainers section (Jagan) > - Mention that i.MX8M Plus is also supported (Marek) > - Remove endpoint@0 description as it only has one endpoint (Marek) Where is the changelog from original submission? How your v1 differs form it? Or did you just ignore all the feedback? > > .../display/bridge/samsung,mipi-dsim.yaml | 271 ++++++++++++++++++ > .../bindings/display/exynos/exynos_dsim.txt | 92 ------ > MAINTAINERS | 1 + > 3 files changed, 272 insertions(+), 92 deletions(-) > create mode 100644 Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > delete mode 100644 Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt > > diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > new file mode 100644 > index 000000000000..2698752dc6ed > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > @@ -0,0 +1,271 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/samsung,mipi-dsim.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Samsung MIPI DSIM bridge controller > + > +maintainers: > + - Inki Dae <inki.dae@samsung.com> > + - Jagan Teki <jagan@amarulasolutions.com> > + > +description: | > + Samsung MIPI DSIM bridge controller can be found it on Exynos > + and i.MX8M Mini/Nano/Plus SoC's. > + > +properties: > + compatible: > + enum: > + - samsung,exynos3250-mipi-dsi > + - samsung,exynos4210-mipi-dsi > + - samsung,exynos5410-mipi-dsi > + - samsung,exynos5422-mipi-dsi > + - samsung,exynos5433-mipi-dsi > + - fsl,imx8mm-mipi-dsim > + - fsl,imx8mp-mipi-dsim > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + clocks: > + minItems: 2 > + maxItems: 5 > + > + clock-names: > + minItems: 2 > + maxItems: 5 > + > + phys: > + maxItems: 1 > + description: phandle to the phy module representing the DPHY OK, so you did ignore the feedback. NAK, go through the feedback and implement it. Best regards, Krzysztof
Hi Krzysztof, On 03/04/2023 09:49, Krzysztof Kozlowski wrote: >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> Signed-off-by: Fabio Estevam <festevam@denx.de> >> --- >> Changes since v1: >> - Added samsung,mipi-dsim.yaml entry to MAINTAINERS file (Jagan) >> - Added Marek Szyprowski entry to the samsung,mipi-dsim.yaml >> maintainers section (Jagan) >> - Mention that i.MX8M Plus is also supported (Marek) >> - Remove endpoint@0 description as it only has one endpoint (Marek) > > Where is the changelog from original submission? How your v1 differs > form it? Or did you just ignore all the feedback? I'm sorry, but it was not my intention to ignore any feedback. Which feedback are you referring to specifically? Some more context: last week I sent a patch adding a new property for exynos_dsim.txt and you asked me to convert it to yaml first: https://lore.kernel.org/all/ff66c8b9-c7f7-1eb2-c730-4812b7ff6824@linaro.org/#t Jagan pointed out an earlier submission he did in 2021: https://lore.kernel.org/all/20210704090230.26489-9-jagan@amarulasolutions.com/ That was my starting point. >> + phys: >> + maxItems: 1 >> + description: phandle to the phy module representing the DPHY > > OK, so you did ignore the feedback. Not intentionally. > NAK, go through the feedback and implement it. Just found this feedback from Rob about Jagan's initial submission: https://lore.kernel.org/all/20210712151322.GA1931925@robh.at.kernel.org/ I can send a new version that takes Rob's feedback into account. Were there any further versions/feedback that were submitted? Can't find them on lore. In another reply, you mention that this should be v13. I could not find previous versions of the yaml submission. Please advise. Thanks, Fabio Estevam
On 03/04/2023 15:25, Fabio Estevam wrote: > Hi Krzysztof, > > On 03/04/2023 09:49, Krzysztof Kozlowski wrote: > >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >>> Signed-off-by: Fabio Estevam <festevam@denx.de> >>> --- >>> Changes since v1: >>> - Added samsung,mipi-dsim.yaml entry to MAINTAINERS file (Jagan) >>> - Added Marek Szyprowski entry to the samsung,mipi-dsim.yaml >>> maintainers section (Jagan) >>> - Mention that i.MX8M Plus is also supported (Marek) >>> - Remove endpoint@0 description as it only has one endpoint (Marek) >> >> Where is the changelog from original submission? How your v1 differs >> form it? Or did you just ignore all the feedback? > > I'm sorry, but it was not my intention to ignore any feedback. > > Which feedback are you referring to specifically? > > Some more context: last week I sent a patch adding a new property > for exynos_dsim.txt and you asked me to convert it to yaml first: > > https://lore.kernel.org/all/ff66c8b9-c7f7-1eb2-c730-4812b7ff6824@linaro.org/#t > > Jagan pointed out an earlier submission he did in 2021: > > https://lore.kernel.org/all/20210704090230.26489-9-jagan@amarulasolutions.com/ > > That was my starting point. I think I saw v11 but maybe not. I cannot find it now. But anyway if this was the newest submission, it already got feedback which we do not need to repeat... Best regards, Krzysztof
On Mon, Apr 3, 2023 at 7:13 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 03/04/2023 15:25, Fabio Estevam wrote: > > Hi Krzysztof, > > > > On 03/04/2023 09:49, Krzysztof Kozlowski wrote: > > > >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > >>> Signed-off-by: Fabio Estevam <festevam@denx.de> > >>> --- > >>> Changes since v1: > >>> - Added samsung,mipi-dsim.yaml entry to MAINTAINERS file (Jagan) > >>> - Added Marek Szyprowski entry to the samsung,mipi-dsim.yaml > >>> maintainers section (Jagan) > >>> - Mention that i.MX8M Plus is also supported (Marek) > >>> - Remove endpoint@0 description as it only has one endpoint (Marek) > >> > >> Where is the changelog from original submission? How your v1 differs > >> form it? Or did you just ignore all the feedback? > > > > I'm sorry, but it was not my intention to ignore any feedback. > > > > Which feedback are you referring to specifically? > > > > Some more context: last week I sent a patch adding a new property > > for exynos_dsim.txt and you asked me to convert it to yaml first: > > > > https://lore.kernel.org/all/ff66c8b9-c7f7-1eb2-c730-4812b7ff6824@linaro.org/#t > > > > Jagan pointed out an earlier submission he did in 2021: > > > > https://lore.kernel.org/all/20210704090230.26489-9-jagan@amarulasolutions.com/ > > > > That was my starting point. > > I think I saw v11 but maybe not. I cannot find it now. But anyway if > this was the newest submission, it already got feedback which we do not > need to repeat... Are you referring to v11 based on DSIM series versioning? If so, the DSIM series from v1 onwards never had this patch of converting .txt to .yaml that series instead added the new dt-bindings on top of existing .txt. The actual conversion patch from .txt to .yaml was initially sent as RFC, so Fabio's versioning seems correct to me. Thanks, Jagan.
diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml new file mode 100644 index 000000000000..2698752dc6ed --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml @@ -0,0 +1,271 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/samsung,mipi-dsim.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung MIPI DSIM bridge controller + +maintainers: + - Inki Dae <inki.dae@samsung.com> + - Jagan Teki <jagan@amarulasolutions.com> + +description: | + Samsung MIPI DSIM bridge controller can be found it on Exynos + and i.MX8M Mini/Nano/Plus SoC's. + +properties: + compatible: + enum: + - samsung,exynos3250-mipi-dsi + - samsung,exynos4210-mipi-dsi + - samsung,exynos5410-mipi-dsi + - samsung,exynos5422-mipi-dsi + - samsung,exynos5433-mipi-dsi + - fsl,imx8mm-mipi-dsim + - fsl,imx8mp-mipi-dsim + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + clocks: + minItems: 2 + maxItems: 5 + + clock-names: + minItems: 2 + maxItems: 5 + + phys: + maxItems: 1 + description: phandle to the phy module representing the DPHY + + phy-names: + items: + - const: dsim + + samsung,phy-type: + $ref: /schemas/types.yaml#/definitions/uint32 + description: phandle to the samsung phy-type + + power-domains: + description: phandle to the associated power domain + maxItems: 1 + + samsung,power-domain: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the associated samsung power domain + + vddcore-supply: + description: MIPI DSIM Core voltage supply (e.g. 1.1V) + + vddio-supply: + description: MIPI DSIM I/O and PLL voltage supply (e.g. 1.8V) + + samsung,burst-clock-frequency: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + DSIM high speed burst mode frequency. + + samsung,esc-clock-frequency: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + DSIM escape mode frequency. + + samsung,pll-clock-frequency: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + DSIM oscillator clock frequency. + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/$defs/port-base + description: + Input port node to receive pixel data from the + display controller. Exactly one endpoint must be + specified. + + unevaluatedProperties: false + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + DSI output port node to the panel or the next bridge + in the chain + +required: + - '#address-cells' + - '#size-cells' + - clock-names + - clocks + - compatible + - interrupts + - phy-names + - phys + - reg + - samsung,burst-clock-frequency + - samsung,esc-clock-frequency + - samsung,pll-clock-frequency + +allOf: + - $ref: ../dsi-controller.yaml# + - if: + properties: + compatible: + contains: + const: samsung,exynos5433-mipi-dsi + + then: + properties: + clocks: + minItems: 5 + + clock-names: + items: + - const: bus_clk + - const: phyclk_mipidphy0_bitclkdiv8 + - const: phyclk_mipidphy0_rxclkesc0 + - const: sclk_rgb_vclk_to_dsim0 + - const: sclk_mipi + + ports: + required: + - port@0 + + required: + - ports + - vddcore-supply + - vddio-supply + + - if: + properties: + compatible: + contains: + const: samsung,exynos5410-mipi-dsi + + then: + properties: + clocks: + minItems: 2 + + clock-names: + items: + - const: bus_clk + - const: pll_clk + + required: + - vddcore-supply + - vddio-supply + + - if: + properties: + compatible: + contains: + const: samsung,exynos4210-mipi-dsi + + then: + properties: + clocks: + minItems: 2 + + clock-names: + items: + - const: bus_clk + - const: sclk_mipi + + required: + - vddcore-supply + - vddio-supply + + - if: + properties: + compatible: + contains: + const: samsung,exynos3250-mipi-dsi + + then: + properties: + clocks: + minItems: 2 + + clock-names: + items: + - const: bus_clk + - const: pll_clk + + required: + - vddcore-supply + - vddio-supply + - samsung,phy-type + +additionalProperties: + type: object + +examples: + - | + #include <dt-bindings/clock/exynos5433.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + dsi@13900000 { + compatible = "samsung,exynos5433-mipi-dsi"; + reg = <0x13900000 0xC0>; + interrupts = <GIC_SPI 205 IRQ_TYPE_LEVEL_HIGH>; + phys = <&mipi_phy 1>; + phy-names = "dsim"; + clocks = <&cmu_disp CLK_PCLK_DSIM0>, + <&cmu_disp CLK_PHYCLK_MIPIDPHY0_BITCLKDIV8>, + <&cmu_disp CLK_PHYCLK_MIPIDPHY0_RXCLKESC0>, + <&cmu_disp CLK_SCLK_RGB_VCLK_TO_DSIM0>, + <&cmu_disp CLK_SCLK_DSIM0>; + clock-names = "bus_clk", + "phyclk_mipidphy0_bitclkdiv8", + "phyclk_mipidphy0_rxclkesc0", + "sclk_rgb_vclk_to_dsim0", + "sclk_mipi"; + power-domains = <&pd_disp>; + vddcore-supply = <&ldo6_reg>; + vddio-supply = <&ldo7_reg>; + samsung,burst-clock-frequency = <512000000>; + samsung,esc-clock-frequency = <16000000>; + samsung,pll-clock-frequency = <24000000>; + pinctrl-names = "default"; + pinctrl-0 = <&te_irq>; + status = "disabled"; + #address-cells = <1>; + #size-cells = <0>; + + panel@0 { + compatible = "samsung,s6e3ha2"; + reg = <0>; + vdd3-supply = <&ldo27_reg>; + vci-supply = <&ldo28_reg>; + reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>; + enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + dsi_to_mic: endpoint { + remote-endpoint = <&mic_to_dsi>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt deleted file mode 100644 index 2a5f0889ec32..000000000000 --- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt +++ /dev/null @@ -1,92 +0,0 @@ -Exynos MIPI DSI Master - -Required properties: - - compatible: value should be one of the following - "samsung,exynos3250-mipi-dsi" /* for Exynos3250/3472 SoCs */ - "samsung,exynos4210-mipi-dsi" /* for Exynos4 SoCs */ - "samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs */ - "samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */ - "samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */ - "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini/Nano SoCs */ - "fsl,imx8mp-mipi-dsim" /* for i.MX8M Plus SoCs */ - - reg: physical base address and length of the registers set for the device - - interrupts: should contain DSI interrupt - - clocks: list of clock specifiers, must contain an entry for each required - entry in clock-names - - clock-names: should include "bus_clk"and "sclk_mipi" entries - the use of "pll_clk" is deprecated - - phys: list of phy specifiers, must contain an entry for each required - entry in phy-names - - phy-names: should include "dsim" entry - - vddcore-supply: MIPI DSIM Core voltage supply (e.g. 1.1V) - - vddio-supply: MIPI DSIM I/O and PLL voltage supply (e.g. 1.8V) - - samsung,pll-clock-frequency: specifies frequency of the oscillator clock - - #address-cells, #size-cells: should be set respectively to <1> and <0> - according to DSI host bindings (see MIPI DSI bindings [1]) - - samsung,burst-clock-frequency: specifies DSI frequency in high-speed burst - mode - - samsung,esc-clock-frequency: specifies DSI frequency in escape mode - -Optional properties: - - power-domains: a phandle to DSIM power domain node - -Child nodes: - Should contain DSI peripheral nodes (see MIPI DSI bindings [1]). - -Video interfaces: - Device node can contain following video interface port nodes according to [2]: - 0: RGB input, - 1: DSI output - -[1]: Documentation/devicetree/bindings/display/mipi-dsi-bus.txt -[2]: Documentation/devicetree/bindings/media/video-interfaces.txt - -Example: - - dsi@11c80000 { - compatible = "samsung,exynos4210-mipi-dsi"; - reg = <0x11C80000 0x10000>; - interrupts = <0 79 0>; - clocks = <&clock 286>, <&clock 143>; - clock-names = "bus_clk", "sclk_mipi"; - phys = <&mipi_phy 1>; - phy-names = "dsim"; - vddcore-supply = <&vusb_reg>; - vddio-supply = <&vmipi_reg>; - power-domains = <&pd_lcd0>; - #address-cells = <1>; - #size-cells = <0>; - samsung,pll-clock-frequency = <24000000>; - - panel@1 { - reg = <0>; - ... - port { - panel_ep: endpoint { - remote-endpoint = <&dsi_ep>; - }; - }; - }; - - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; - decon_to_mic: endpoint { - remote-endpoint = <&mic_to_decon>; - }; - }; - - port@1 { - reg = <1>; - dsi_ep: endpoint { - reg = <0>; - samsung,burst-clock-frequency = <500000000>; - samsung,esc-clock-frequency = <20000000>; - remote-endpoint = <&panel_ep>; - }; - }; - }; - }; diff --git a/MAINTAINERS b/MAINTAINERS index 4f57deee1a08..aca7027dc464 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6640,6 +6640,7 @@ M: Jagan Teki <jagan@amarulasolutions.com> M: Marek Szyprowski <m.szyprowski@samsung.com> S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml F: drivers/gpu/drm/bridge/samsung-dsim.c F: include/drm/bridge/samsung-dsim.h