On Wed, Jun 19, 2013 at 3:20 PM, Tomasz Figa <t.figa@xxxxxxxxxxx> 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@xxxxxxxxxxx> wrote: >> >> -----Original Message----- >> >> From: dri-devel-bounces+inki.dae=samsung.com@xxxxxxxxxxxxxxxxxxxxx >> >> [mailto:dri-devel-bounces+inki.dae=samsung.com@xxxxxxxxxxxxxxxxxxxxx] On >> >> Behalf Of Lucas Stach >> >> Sent: Wednesday, June 19, 2013 4:59 PM >> >> To: Tomasz Figa >> >> Cc: kgene.kim@xxxxxxxxxxx; devicetree-discuss@xxxxxxxxxxxxxxxx; >> >> sw0312.kim@xxxxxxxxxxx; joshi@xxxxxxxxxxx; >> > >> > dri-devel@xxxxxxxxxxxxxxxxxxxxx; >> > >> >> linux-samsung-soc@xxxxxxxxxxxxxxx; rob.herring@xxxxxxxxxxx; >> >> s.nawrocki@xxxxxxxxxxx; grant.likely@xxxxxxxxxx; 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@xxxxxxxxxxx> >> >> > > --- >> >> > > >> >> > > 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@xxxxxxxxxxxxxxxxxxxxx >> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> > >> > _______________________________________________ >> > dri-devel mailing list >> > dri-devel@xxxxxxxxxxxxxxxxxxxxx >> > 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@xxxxxxxxxxxxxxx >> 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@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |