From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v3 13/32] drm/exynos: hdmi: remove the i2c drivers and use devtree Date: Fri, 29 Nov 2013 11:24:03 +0100 Message-ID: <20131129102402.GF22771@ulmo.nvidia.com> References: <1383063198-10526-1-git-send-email-seanpaul@chromium.org> <3586522.SB7l1OESEl@flatron> <20131111084426.GA3884@ulmo.nvidia.com> <20408902.mr0KEZob0h@amdc1227> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0780945945==" Return-path: Received: from mail-bk0-f42.google.com (mail-bk0-f42.google.com [209.85.214.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 97D94FAAEA for ; Fri, 29 Nov 2013 02:24:51 -0800 (PST) Received: by mail-bk0-f42.google.com with SMTP id w11so4283144bkz.29 for ; Fri, 29 Nov 2013 02:24:50 -0800 (PST) In-Reply-To: <20408902.mr0KEZob0h@amdc1227> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Tomasz Figa Cc: marcheu@chromium.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0780945945== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="65ImJOski3p8EhYV" Content-Disposition: inline --65ImJOski3p8EhYV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 28, 2013 at 02:30:24PM +0100, Tomasz Figa wrote: > On Monday 11 of November 2013 09:44:27 Thierry Reding wrote: > > On Sun, Nov 10, 2013 at 09:46:02PM +0100, Tomasz Figa wrote: > > [...] > > > On Tuesday 29 of October 2013 12:12:59 Sean Paul wrote: > > [...] > > > [snip] > > > > @@ -1957,21 +1943,30 @@ static int hdmi_probe(struct platform_devic= e *pdev) > > > > } > > > > =20 > > > > /* DDC i2c driver */ > > > > - if (i2c_add_driver(&ddc_driver)) { > > > > - DRM_ERROR("failed to register ddc i2c driver\n"); > > > > - return -ENOENT; > > > > + ddc_node =3D of_find_node_by_name(NULL, "hdmiddc"); > > >=20 > > > This is wrong. You shall not reference a device tree node by its name, > > > except some very specific well-defined cases, such as cpus or memory > > > nodes. > > >=20 > > > A solution closest to yours, but correct, would be to use the same ma= tch > > > table as in the I2C driver you are removing and call > > > of_find_matching_node(). > >=20 > > Isn't the correct solution to use a phandle? That might need the binding > > to change in a backwards incompatible way. >=20 > Yes, phandle is an even better option as it can point you precisely to the > node you are interested in, but this will be incompatible, meaning that > you would have to support both variants anyway. Oh come on. If a phandle is the right way to do it, then we should just do it. Will it really be so difficult to carry code for both variants? If nothing else it will at least set a good example and reduce the risk of people doing the same mistakes over and over again. Adding the right binding also gives you a way to start deprecating the wrong one and eventually remove it. The longer you wait, the more people will start to use the existing, broken binding and removing it will only become more difficult over time. > > Then again, if something as > > simple as specifying a DDC I2C bus causes the binding to change in a > > backwards incompatible way then it can't have been a very good binding > > in the first place, right? +1 for unstable DT bindings... >=20 > Well, some of already existing bindings should have been definitely marked > unstable, as they haven't been thought and reviewed well enough, if at all > (especially reviewed, as we only started seriously reviewing DT bindings > not so long ago). >=20 > Honestly, I'm not quite sure about this binding in particular, especially > how much it would be a problem if we broke compatibility. I mean, how much > tied to old DTBs are existing boards using this binding. The affected > boards are: > - exynos5250-snow, > - exynos5250-arndale, > - exynos5250-smdk5250, > - exynos5420-smdk5420. > The last three are most likely to be used only with DTB appended, so > I don't think that anyone would complain. However I'm not sure about the > first one, which is supposed to be a Chromebook if I'm not mistaken. Well, if it's a Chromebook it likely doesn't ship with a completely mainline kernel. That frees it from the stability requirements, doesn't it? Thierry --65ImJOski3p8EhYV Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJSmGtCAAoJEN0jrNd/PrOhndsQALp45FAGH1gA4OEwDYL6ZOGf s8V2eMdAZ2bWsmq0LNmyo+b7msFtO/MpOslQISAgHB083Died8roI1D1W4Qe+GRL sqRG/PT0FRo18jjzoctK6g+nfl59kuzAmdTLTUNI9TkRCH4uEytZwBuGdpKBnh/q 8IOJWIvi3dgf+CMHD+kex0HfqlDL65ud6miwWcaLuYA7CRQ/zLSUmPsJAP7qX7in KHXgumOLW9FuBlyTY38bGCzTNCxr5dD2F9RTzu2HgWdI9f592Lm18Jo1P03s5n8H XWOy7+u9RsPaJEOlml5Gr2XOWICI7K9FFCUC1lXyeelKwfoBXeSSV9+Vta4i/Qnh SxhhUvBfAJmnaM13JOYIL9Hknsp1rpUgDT4vc5CuXjzEqN32pTAgSeebU5iOPRwx 8eLl/3fEcBbxJ9NteJtWwVRpTSaTCO0Jd4ViQNOdefUuEgJiWeNKFrtO5UYAAJWn 52EDKLUWhvGiiDNGgKy6MixpKt95GaUgkmuMehrOjXbB8g+mQlKuPoUm8s9PYoOp r8WO5BvuvX52nJwO1GWBvbTu/2CzTnVES817rAkYG7zDIUVVjU5vWVk/Okcp1Bw/ 8uFF38lv1VoAqbpS+2JiWrc1W/658oP+GcQCmeJv98NDTqAfOU7napjikkhRTUhY evB7FVV9hhR5TA6x6kVz =gVmt -----END PGP SIGNATURE----- --65ImJOski3p8EhYV-- --===============0780945945== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0780945945==--