Re: [PATCH v8 08/13] arm64: dts: mt8173: Add display subsystem related nodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Philipp,

Two more comments below...

On Tue, Feb 2, 2016 at 4:10 PM, Daniel Kurtz <djkurtz@xxxxxxxxxxxx> wrote:
> On Tue, Jan 5, 2016 at 1:36 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
>> From: CK Hu <ck.hu@xxxxxxxxxxxx>
>>
>> This patch adds the device nodes for the DISP function blocks
>> comprising the display subsystem.
>>
>> Signed-off-by: CK Hu <ck.hu@xxxxxxxxxxxx>
>> Signed-off-by: Cawa Cheng <cawa.cheng@xxxxxxxxxxxx>
>> Signed-off-by: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
>> Signed-off-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
>> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>

>> +
>> +               dsi0: dsi@1401b000 {
>> +                       compatible = "mediatek,mt8173-dsi";
>> +                       reg = <0 0x1401b000 0 0x1000>;
>> +                       interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_LOW>;
>> +                       power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> +                       clocks = <&mmsys CLK_MM_DSI0_ENGINE>,
>> +                                <&mmsys CLK_MM_DSI0_DIGITAL>,
>> +                                <&mipi_tx0>;
>> +                       clock-names = "engine", "digital", "hs";
>> +                       phys = <&mipi_tx0>;
>> +                       phy-names = "dphy";

I think it might work better if this was also default
status="disabled", and require the board-specific .dts to enable the
dsi*.
This would be useful, for example, boards that use only the MIPI/DSI
or only HDMI.
IMHO, it is more clear to have such boards explicit mark the supported
ports as 'status="okay"' in their .dts, rather than having to mark all
of the unused ones as disabled.


>> +               };
>> +
>> +               dsi1: dsi@1401c000 {
>> +                       compatible = "mediatek,mt8173-dsi";
>> +                       reg = <0 0x1401c000 0 0x1000>;
>> +                       interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>;
>> +                       power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> +                       clocks = <&mmsys CLK_MM_DSI1_ENGINE>,
>> +                                <&mmsys CLK_MM_DSI1_DIGITAL>,
>> +                                <&mipi_tx1>;
>> +                       clock-names = "engine", "digital", "hs";
>> +                       phy = <&mipi_tx1>;
>> +                       phy-names = "dphy";
>> +                       status = "disabled";
>> +               };
>> +
>> +               dpi0: dpi@1401d000 {
>> +                       compatible = "mediatek,mt8173-dpi";
>> +                       reg = <0 0x1401d000 0 0x1000>;
>> +                       interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
>> +                       power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> +                       clocks = <&mmsys CLK_MM_DPI_PIXEL>,
>> +                                <&mmsys CLK_MM_DPI_ENGINE>,
>> +                                <&apmixedsys CLK_APMIXED_TVDPLL>;
>> +                       clock-names = "pixel", "engine", "pll";
>> +
>> +                       port {
>> +                               dpi0_out: endpoint {
>> +                                       remote-endpoint = <&hdmi0_in>;
>
> nit: At this point in the patch series, you haven't defined hdmi0_in yet.
> Move this port to "Add HDMI related nodes".

And, let's mark dpi0 as status="disabled"; by default, and require an
enabling in the board specific .dts.

>
>> +                               };
>> +                       };
>>                 };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux