Re: [PATCH 1/4] drm/exynos: rename compatible strings for hdmi subsystem

[Lucas Stach <l.stach@pengutronix.de>]

Am Mittwoch, den 19.06.2013, 09:52 +0200 schrieb Tomasz Figa:
> Hi Rahul,
> 
> On Tuesday 18 of June 2013 18:19:35 Rahul Sharma wrote:
> > This patch renames the combatible strings for hdmi, mixer, ddc
> > and hdmiphy. It follows the convention of using compatible string
> > which represent the SoC in which the IP was added for the first
> > time.
> > 
> > Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/video/exynos_hdmi.txt    |    6
> > ++++-- Documentation/devicetree/bindings/video/exynos_hdmiddc.txt |   
> > 4 ++-- Documentation/devicetree/bindings/video/exynos_hdmiphy.txt |   
> > 6 ++++-- Documentation/devicetree/bindings/video/exynos_mixer.txt   |  
> >  7 +++++-- drivers/gpu/drm/exynos/exynos_ddc.c                        |
> >    2 +- drivers/gpu/drm/exynos/exynos_hdmi.c                       |   
> > 2 +- drivers/gpu/drm/exynos/exynos_hdmiphy.c                    |    4
> > +++- drivers/gpu/drm/exynos/exynos_mixer.c                      |   12
> > ++++++------ 8 files changed, 26 insertions(+), 17 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> > b/Documentation/devicetree/bindings/video/exynos_hdmi.txt index
> > 589edee..2ac01ca 100644
> > --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> > +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
> > @@ -1,7 +1,9 @@
> >  Device-Tree bindings for drm hdmi driver
> > 
> >  Required properties:
> > -- compatible: value should be "samsung,exynos5-hdmi".
> > +- compatible: value should be one among the following:
> > +	1) "samsung,exynos4210-hdmi"
> > +	2) "samsung,exynos4212-hdmi"
> >  - reg: physical base address of the hdmi and length of memory mapped
> >  	region.
> >  - interrupts: interrupt number to the cpu.
> > @@ -15,7 +17,7 @@ Required properties:
> >  Example:
> > 
> >  	hdmi {
> > -		compatible = "samsung,exynos5-hdmi";
> > +		compatible = "samsung,exynos4212-hdmi";
> 
> Sorry, but it's a NAK from me.
> 
> DeviceTree bindings are considered an ABI. This is to allow older dtbs to 
> work with new kernels.
> 
> If you just change the binding this way, you break all the existing users 
> of this compatible value.
> 
> In addition you are doing it in a way that breaks bisection:
>  - patch 1/4 breaks existing in-tree users of current compatible values,
>  - after patch 2 and 3 it is still broken,
>  - and eventually all in-tree users are fixed by patch 4 (but you can't 
> fix out-of-tree users).
> 
> Please do it without changing existing compatible values. Even if they are 
> misleading, this is all can be described in the documentation - just list 
> SoCs that can be used with each compatible value there.
> 

Or you could just introduce the new compatible value and make all
in-tree users use this, but keep the old values around and still accept
them in the drivers. This way you get the goodness of the cleaner new
symbols without breaking existing users. Just mark the old values as
deprecated in the documentation, so no new devicetree usees them.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |