Re: [PATCH v1 3/4] arm: dts: mt7623: add display related nodes

From: Ryder Lee
Date: Thu Sep 21 2017 - 05:50:37 EST


On Tue, 2017-09-19 at 23:26 +0800, CK Hu wrote:
> Hi, Ryder:
>
> Some comment inline.
>
> On Tue, 2017-09-19 at 14:27 +0800, Ryder Lee wrote:
> > This patch adds the device nodes for the display function block.
> > Also, we add some missing pin macros in mt7623-pinfunc.h.
> >
> > Signed-off-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> > CC: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > ---
> > arch/arm/boot/dts/mt7623.dtsi | 210 ++++++++++++++++++++++++++
> > arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 41 ++++-
> > include/dt-bindings/pinctrl/mt7623-pinfunc.h | 12 ++
> > 3 files changed, 261 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> > index 9ec3767..e11e5e7 100644
> > --- a/arch/arm/boot/dts/mt7623.dtsi
> > +++ b/arch/arm/boot/dts/mt7623.dtsi
> > @@ -20,6 +20,7 @@
> > #include <dt-bindings/power/mt2701-power.h>
> > #include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/phy/phy.h>
> > +#include <dt-bindings/memory/mt2701-larb-port.h>
> > #include <dt-bindings/reset/mt2701-resets.h>
> > #include <dt-bindings/thermal/thermal.h>
> > #include "skeleton64.dtsi"
> > @@ -28,6 +29,11 @@
> > compatible = "mediatek,mt7623";
> > interrupt-parent = <&sysirq>;
> >
> > + aliases {
> > + rdma0 = &rdma0;
> > + rdma1 = &rdma1;
>
> For display, are these two aliases enough?

Yes, this part is the same as mt2701.

https://patchwork.kernel.org/patch/9803813/

> > + };
> > +

...
> > + wdma@14009000 {
> > + compatible = "mediatek,mt7623-disp-wdma",
> > + "mediatek,mt2701-disp-wdma";
>
> There is neither "mediatek,mt7623-disp-wdma" nor
> "mediatek,mt2701-disp-wdma" in driver, do you really need this device
> node?

Okay, I will remove it.

> > + 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: bls@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@1400b000 {
>
> color: color@1400b000 {

Okay.
> > + 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>;
> > + };
> > +