[PATCH v3 5/5] arm: dts: mt7623: add display subsystem related device nodes

Chun-Kuang Hu chunkuang.hu at kernel.org
Tue Aug 4 12:01:29 EDT 2020


Hi, Frank:

Frank Wunderlich <frank-w at public-files.de> 於 2020年8月4日 週二 下午11:23寫道:
>
> Hi
>
> Except mmsys (i added in Patch #1) all mt7623-compatibles are not defined in code and fallback (mt2701-x/mt8173-x) is used. If i add it in dt-binding, it should be added in code too, right? or should i remove mt7623 compatibles and only add documentation for new mmsys?

You should add mt7623 compatibles in binding document and device node
(like mmsys node). The driver is optional to have it. If driver does
not implement mt7623, it would fallback to mt2701.

Regards,
Chun-Kuang.

>
> regards Frank
>
>
> > Gesendet: Dienstag, 04. August 2020 um 17:00 Uhr
> > Von: "Chun-Kuang Hu" <chunkuang.hu at kernel.org>
> > An: "Frank Wunderlich" <linux at fw-web.de>
> > > +       mipi_tx0: mipi-dphy at 10010000 {
> > > +               compatible = "mediatek,mt7623-mipi-tx",
> >
> > In mediatek,dsi.txt [1], "mediatek,mt7623-mipi-tx" is undefined.
> >
> > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdsi.txt
>
> in drivers/gpu/drm/mediatek/mtk_mipi_tx.c only the compatible for mt2701 is defined (which is fallback in dts). So should i remove mt7623 compatible here and in the other occurences?
>
> >
> > > +                            "mediatek,mt2701-mipi-tx";
>
> > > +
> > > +       cec: cec at 10012000 {
> > > +               compatible = "mediatek,mt7623-cec",
> >
> > Please explicitly define "mediatek,mt7623-cec" in mediatek,hdmi.txt [2].
> >
> > [2] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Chdmi.txt
> >
> > > +                            "mediatek,mt8173-cec";
>
> same here...only mt8173-cec is defined in drivers/gpu/drm/mediatek/mtk_cec.c
>
> > >         cir: cir at 10013000 {
> > >                 compatible = "mediatek,mt7623-cir";
> > >                 reg = <0 0x10013000 0 0x1000>;
> > > @@ -369,6 +393,18 @@ apmixedsys: syscon at 10209000 {
> > >                 #clock-cells = <1>;
> > >         };
> > >
> > > +       hdmi_phy: phy at 10209100 {
> > > +               compatible = "mediatek,mt7623-hdmi-phy",
> >
> > Ditto.
> >
> > > +                            "mediatek,mt2701-hdmi-phy";
> same as above (./drivers/gpu/drm/mediatek/mtk_hdmi_phy.c)
>
> > > +       hdmiddc0: i2c at 11013000 {
> > > +               compatible = "mediatek,mt7623-hdmi-ddc",
> >
> > Ditto.
> >
> > > +                            "mediatek,mt8173-hdmi-ddc";
> > > +               interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > > +               reg = <0 0x11013000 0 0x1C>;
> > > +               clocks = <&pericfg CLK_PERI_I2C3>;
> > > +               clock-names = "ddc-i2c";
> > > +               status = "disabled";
> > > +       };
> > > +
> > >         nor_flash: spi at 11014000 {
> > >                 compatible = "mediatek,mt7623-nor",
> > >                              "mediatek,mt8173-nor";
> > > @@ -766,6 +812,84 @@ mmsys: syscon at 14000000 {
> > >                 #clock-cells = <1>;
> > >         };
> > >
> > > +       display_components: dispsys at 14000000 {
> > > +               compatible = "mediatek,mt7623-mmsys",
> > > +                            "mediatek,mt2701-mmsys";
> >
> > In mediatek,mmsys.txt [3], this should be:
> >
> > mmsys: syscon at 14000000 {
> >         compatible = "mediatek,mt7623-mmsys", "mediatek,mt2701-mmsys", "syscon"
> >
> > [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/mediatek/mediatek%2Cmmsys.txt
>
> as i added this in driver, i add this to documentation (and maybe remove the fallback because it results in wrong routing?)
>
> >
> > > +               reg = <0 0x14000000 0 0x1000>;
> > > +               power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > > +       };
> > > +
> > > +       ovl at 14007000 {
> > > +               compatible = "mediatek,mt7623-disp-ovl",
> >
> > This is not defined in mediatek,disp.txt [4].
> >
> > [4] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdisp.txt
> also not defined in drivers/gpu/drm/mediatek/mtk_drm_drv.c so again fallback used
>
> >
> > > +                            "mediatek,mt2701-disp-ovl";
>
> > > +               reg = <0 0x14007000 0 0x1000>;
> > > +               interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > > +               clocks = <&mmsys CLK_MM_DISP_OVL>;
> > > +               iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > > +               mediatek,larb = <&larb0>;
> > > +       };
> > > +
> > > +       rdma0: rdma at 14008000 {
> > > +               compatible = "mediatek,mt7623-disp-rdma",
> > > +                            "mediatek,mt2701-disp-rdma";
> > > +               reg = <0 0x14008000 0 0x1000>;
> > > +               interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> > > +               clocks = <&mmsys CLK_MM_DISP_RDMA>;
> > > +               iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> > > +               mediatek,larb = <&larb0>;
> > > +       };
> > > +
> > > +       wdma at 14009000 {
> > > +               compatible = "mediatek,mt7623-disp-wdma",
> > > +                            "mediatek,mt2701-disp-wdma";
> > > +               reg = <0 0x14009000 0 0x1000>;
> > > +               interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> > > +               clocks = <&mmsys CLK_MM_DISP_WDMA>;
> > > +               iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> > > +               mediatek,larb = <&larb0>;
> > > +       };
> > > +
> > > +       bls: pwm at 1400a000 {
> > > +               compatible = "mediatek,mt7623-disp-pwm",
> > > +                            "mediatek,mt2701-disp-pwm";
> > > +               reg = <0 0x1400a000 0 0x1000>;
> > > +               #pwm-cells = <2>;
> > > +               clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> > > +                        <&mmsys CLK_MM_DISP_BLS>;
> > > +               clock-names = "main", "mm";
> > > +               status = "disabled";
> > > +       };
> > > +
> > > +       color at 1400b000 {
> > > +               compatible = "mediatek,mt7623-disp-color",
> > > +                            "mediatek,mt2701-disp-color";
> > > +               reg = <0 0x1400b000 0 0x1000>;
> > > +               interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> > > +               clocks = <&mmsys CLK_MM_DISP_COLOR>;
> > > +       };
> > > +
> > > +       dsi: dsi at 1400c000 {
> > > +               compatible = "mediatek,mt7623-dsi",
> >
> > This is not defined in mediatek,dsi.txt [1].
> >
> > > +                            "mediatek,mt2701-dsi";
> also fallback used (drivers/gpu/drm/mediatek/mtk_drm_drv.c)
>
>
> > > +       dpi0: dpi at 14014000 {
> > > +               compatible = "mediatek,mt7623-dpi",
> >
> > This is not defined in mediatek,dpi.txt [5].
> >
> > [5] https://www.kernel.org/doc/Documentation/devicetree/bindings/display/mediatek/mediatek%2Cdpi.txt
> >
> > > +                            "mediatek,mt2701-dpi";
>



More information about the linux-arm-kernel mailing list