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 Daniel,

Am Mittwoch, den 03.02.2016, 00:24 +0800 schrieb Daniel Kurtz:
> 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:
[...]
> >> +
> >> +               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.

I'm ok with that, the same should be done for the phys then. To enable
MIPI DSI output on dsi0, the board DT would have to:

	&mipi_tx0 {
		status = "okay";
	};
	&dsi0 {
		status = "okay";
		/* output port here */
	};

To enable HDMI output via dpi0:

	&cec {
		status = "okay";
	};
	&hdmi_phy {
		status = "okay";
	};
	&dpi0 {
		status = "okay";
	};
	&hdmi0 {
		status = "okay";
		/* output port here */
	};

[...]
> >> +               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.

Will do.

regards
Philipp


--
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