[PATCH 1/4] drm/exynos: rename compatible strings for hdmi subsystem
Rahul Sharma
r.sh.open at gmail.com
Wed Jun 19 03:26:19 PDT 2013
On Wed, Jun 19, 2013 at 3:20 PM, Tomasz Figa <t.figa at samsung.com> wrote:
> Hi Rahul,
>
> On Wednesday 19 of June 2013 15:02:59 Rahul Sharma wrote:
>> Hi All,
>>
>> On Wed, Jun 19, 2013 at 1:57 PM, Inki Dae <inki.dae at samsung.com> wrote:
>> >> -----Original Message-----
>> >> From: dri-devel-bounces+inki.dae=samsung.com at lists.freedesktop.org
>> >> [mailto:dri-devel-bounces+inki.dae=samsung.com at lists.freedesktop.org] On
>> >> Behalf Of Lucas Stach
>> >> Sent: Wednesday, June 19, 2013 4:59 PM
>> >> To: Tomasz Figa
>> >> Cc: kgene.kim at samsung.com; devicetree-discuss at lists.ozlabs.org;
>> >> sw0312.kim at samsung.com; joshi at samsung.com;
>> >
>> > dri-devel at lists.freedesktop.org;
>> >
>> >> linux-samsung-soc at vger.kernel.org; rob.herring at calxeda.com;
>> >> s.nawrocki at samsung.com; grant.likely at linaro.org; Rahul Sharma
>> >> Subject: Re: [PATCH 1/4] drm/exynos: rename compatible strings for hdmi
>> >> subsystem
>> >>
>> >> 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 at 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).
>>
>> @Tomasz, I understand your point but how is it possible to change
>> compatible types in driver as well as in dtbs by not breaking either of them
>> other then putting changes in a single patch.
>
> It's very easy. (Let's forget about the fact that DT bindings are an ABI
> temporarily) You can simply add new compatible values to the driver in first
> patch, then modify dts files in second one and then remove old compatibles
> values from the driver in third patch. (Now we remember about DT being ABI
> again.)
>
>> I ensured that hdmi stuff is
>> intact with whole series merged in either tree (drm or arch). Please
>> suggest a better way.
>>
>> The Only existing user is Exynos5250, which is modified in the same patch
>> set.
>
> This is not true. You have modified only the existing _in_ _tree_ users.
>
> Keep in mind that device tree is not a part of the kernel. It is currently
> located in the same tree, but it is _not_ a part of the kernel.
>
> Now think about existing boards (like exynos5250-snow) that have a dtb built
> from older dts sources stored in their flash memory. You need to maintain
> compatibilty with this old dtb in new kernels as well.
>
>> >> > 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.
>>
>> I agree, above is a decent approach, but in this case we have only one
>> user for this compatible type including in flight patches which I have
>> modified along.
>
> This only user is already present in 3.10-rc6. This means that we now must
> maintain compatibility with it.
>
>> If it seems better to keep old compatible type (deprecated), it is fine
>> with me.
>
> Yes, this is an acceptable solution.
>
>> > That's a good idea. We really need to mitigate such misleading somehow or
>> > other.
>>
>> Please sugggest me how to proceed.
>
> I think it's enough said now.
>
> Best regards,
> Tomasz
Fine. It seems if I change this patch to add new compatible strings,
above concerns
will be addressed. I will post the v2.
regards,
Rahul Sharma.
>
>> regards,
>> Rahul Sharma.
>>
>> > Thanks,
>> > Inki Dae
>> >
>> >> 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
>> >> |
>> >>
>> >> _______________________________________________
>> >> dri-devel mailing list
>> >> dri-devel at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
>> in the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the dri-devel
mailing list