devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Inki Dae <inki.dae@samsung.com>
To: 'Lucas Stach' <l.stach@pengutronix.de>,
	'Tomasz Figa' <tomasz.figa@gmail.com>
Cc: kgene.kim@samsung.com, devicetree-discuss@lists.ozlabs.org,
	sw0312.kim@samsung.com, joshi@samsung.com,
	dri-devel@lists.freedesktop.org,
	linux-samsung-soc@vger.kernel.org, rob.herring@calxeda.com,
	s.nawrocki@samsung.com, grant.likely@linaro.org,
	'Rahul Sharma' <rahul.sharma@samsung.com>
Subject: RE: [PATCH 1/4] drm/exynos: rename compatible strings for hdmi subsystem
Date: Wed, 19 Jun 2013 17:27:08 +0900	[thread overview]
Message-ID: <00a001ce6cc6$ca1807e0$5e4817a0$%dae@samsung.com> (raw)
In-Reply-To: <1371628750.4230.3.camel@weser.hi.pengutronix.de>



> -----Original Message-----
> From: dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org
> [mailto:dri-devel-bounces+inki.dae=samsung.com@lists.freedesktop.org] On
> Behalf Of Lucas Stach
> Sent: Wednesday, June 19, 2013 4:59 PM
> To: Tomasz Figa
> Cc: kgene.kim@samsung.com; devicetree-discuss@lists.ozlabs.org;
> sw0312.kim@samsung.com; joshi@samsung.com;
dri-devel@lists.freedesktop.org;
> linux-samsung-soc@vger.kernel.org; rob.herring@calxeda.com;
> s.nawrocki@samsung.com; grant.likely@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@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.
> 

That's a good idea. We really need to mitigate such misleading somehow or
other.

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2013-06-19  8:27 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-18 12:49 [PATCH 0/4] exynos5420/hdmi: add support for hdmi subsystem Rahul Sharma
2013-06-18 12:49 ` [PATCH 1/4] drm/exynos: rename compatible strings " Rahul Sharma
2013-06-19  5:43   ` 김승우
2013-06-19  7:05   ` Inki Dae
2013-06-19  7:36   ` Inki Dae
     [not found]   ` <1371559778-9359-2-git-send-email-rahul.sharma-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-06-19  7:52     ` Tomasz Figa
2013-06-19  7:59       ` Lucas Stach
2013-06-19  8:27         ` Inki Dae [this message]
2013-06-19  9:32           ` Rahul Sharma
2013-06-19  9:50             ` Tomasz Figa
2013-06-19 10:26               ` Rahul Sharma
2013-06-18 12:49 ` [PATCH 2/4] drm/exynos: add support for exynos5420 mixer Rahul Sharma
2013-06-19  5:24   ` 김승우
2013-06-19  5:32     ` Rahul Sharma
2013-06-18 12:49 ` [PATCH 3/4] drm/exynos: fix interlace resolutions for exynos5420 Rahul Sharma
2013-06-19  5:34   ` 김승우
2013-06-19  5:59     ` Inki Dae
2013-06-18 12:49 ` [PATCH 4/4] ARM/dts: change compatible strings for hdmi subsystem Rahul Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='00a001ce6cc6$ca1807e0$5e4817a0$%dae@samsung.com' \
    --to=inki.dae@samsung.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=grant.likely@linaro.org \
    --cc=joshi@samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=rahul.sharma@samsung.com \
    --cc=rob.herring@calxeda.com \
    --cc=s.nawrocki@samsung.com \
    --cc=sw0312.kim@samsung.com \
    --cc=tomasz.figa@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).