[PATCH 1/4] drm/exynos: rename compatible strings for hdmi subsystem
Tomasz Figa
t.figa at samsung.com
Wed Jun 19 02:50:07 PDT 2013
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
> 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
More information about the dri-devel
mailing list