Re: [PATCH v2 4/4] video: mmp: add device tree support

On 04/03/14 13:28, Zhou Zhu wrote:

Hi, Tomi and Mark,

On Mon, Feb 17, 2014 at 10:37 PM, Mark Rutland [off-list ref] wrote:

quoted

On Tue, Jan 14, 2014 at 11:16:24AM +0000, Zhou Zhu wrote:

quoted

add device tree support for mmp fb/controller
the description of DT config is at
Documentation/devicetree/bindings/fb/mmp-disp.txt

Signed-off-by: Zhou Zhu <redacted>
---
 Documentation/devicetree/bindings/fb/mmp-disp.txt |   60 ++++++++
 drivers/video/mmp/fb/mmpfb.c                      |   73 ++++++----
 drivers/video/mmp/hw/mmp_ctrl.c                   |  160 ++++++++++++++++-----
 3 files changed, 235 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt

Thank you very much for your review! I am trying to update the code
according to your comments.

We have reviewed the dts and removed many software settings and
not-used settings - for example, we unpacked path-config/link-config
and removed some configures that we will never change.
Also we removed fb settings which is considered as software.

Would you please give us some feedbacks if we adjust our dts into follow style?
As there might be big changes on the code structures, I would update
the code after this dts layout is considered as "right".

mmp-disp@d420b000 {
    compatible = "marvell,mmp-disp";
    reg = <0xd420b000 0x1fc>;
    interrupts = <0 41 0x4>;

    internal-connections {
        pipe1: pn-path {
        input = "panel-graphic"; //panel-graphic is the overlay name in spec.
        output = &parallel;
        }
    }
    ports{
        parallel: parallel {
            marvell,rbswap;
            marvell,spi;
        }
    }
    status = "okay";
}

panel-xxx {
    properties;
    connection = &parallel;
}

I would recommend using the same style that is used in the OMAP DSS, imx
drm and exynos patches that are introducing DT support. They use ports
and endpoints as defined in:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/media/video-interfaces.txt

with the addition of a simplified endpoint format, where the 'port' node
is not required.

To give an idea what it could look like, I've modified your example
above. It's just a rough example, you should study and think how it best
fits for mmp.

You could use the same format for the internal connections also, but I
didn't touch that. I think internal connections can as well be
configured separately, in a custom format, because there's no need for
external components to connect to the internal links.

mmp-disp@d420b000 {
    compatible = "marvell,mmp-disp";
    reg = <0xd420b000 0x1fc>;
    interrupts = <0 41 0x4>;

    internal-connections {
        pipe1: pn-path {
        input = "panel-graphic";
        output = &disp_out;
        }
    }

	disp_out: endpoint {
		remote-endpoint = <&panel_in>;
		marvell,rbswap;
		marvell,spi;
	};

    status = "okay";
}

panel-xxx {
	properties;

	panel_in: endpoint {
		remote-endpoint = <&disp_out>;
	};
};

 Tomi