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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3258 bytes --]

On 04/03/14 13:28, Zhou Zhu wrote:
> Hi, Tomi and Mark,
> 
> On Mon, Feb 17, 2014 at 10:37 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> On Tue, Jan 14, 2014 at 11:16:24AM +0000, Zhou Zhu wrote:
>>> 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 <zzhu3@marvell.com>
>>> ---
>>>  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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]