From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932275AbbFQSSJ (ORCPT ); Wed, 17 Jun 2015 14:18:09 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:36289 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756675AbbFQSSC (ORCPT ); Wed, 17 Jun 2015 14:18:02 -0400 Date: Wed, 17 Jun 2015 19:13:59 +0100 From: Mark Brown To: Tomeu Vizoso Cc: linux-arm-kernel@lists.infradead.org, Alexander Holler , Alexandre Courbot , Andrzej Hajda , Arnd Bergmann , Dmitry Torokhov , Grant Likely , Greg Kroah-Hartman , Ian Campbell , Javier Martinez Canillas , Krzysztof Kozlowski , Kumar Gala , Len Brown , Linus Walleij , linux-kernel@vger.kernel.org, Lv Zheng , Mark Rutland , Pawel Moll , "Rafael J. Wysocki" , Robert Moore , Rob Herring , Russell King , Stephen Warren , Terje =?iso-8859-1?Q?Bergstr=F6m?= , Thierry Reding Message-ID: <20150617181359.GA14071@sirena.org.uk> References: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> <1434548543-22949-14-git-send-email-tomeu.vizoso@collabora.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="+9Nh279w3tr8TrRk" Content-Disposition: inline In-Reply-To: <1434548543-22949-14-git-send-email-tomeu.vizoso@collabora.com> X-Cookie: The end of labor is to gain leisure. User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 94.175.94.161 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [PATCH 13/13] driver-core: probe dependencies before probing X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --+9Nh279w3tr8TrRk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 17, 2015 at 03:42:23PM +0200, Tomeu Vizoso wrote: > Before actually probing a device, find out what dependencies it has and > do our best to ensure that they are available at this point. > This is accomplished by finding out what platform devices need to be > probed so the dependencies are available. =2E..and then trying to probe them first. > If any dependencies are still unavailable after that (most probably a > missing driver or an error in the HW description from the firmware), we > print a nice error message so that people don't have to add a zillion of > printks to find out why a device asked for its probe to be deferred. So, I think I like this approach though I've not done a full pass through and I'm not sure how expensive it gets (there's definitely room for optimisation as the patch notes). I'm not 100% sure I see what prints this error message you're referring to (I'm just seeing debug prints). > +static struct fwnode_handle *get_enclosing_platform_dev( > + struct fwnode_handle *fwnode) Only platform devices? > +static void check_dependencies_per_class(struct class *class, void *data) > +{ > + struct fwnode_handle *fwnode =3D data; > + struct list_head *deps; > + struct fwnode_dependency *dep, *tmp; > + > + if (!class->get_dependencies) > + return; > + > + deps =3D class->get_dependencies(fwnode); > + if (!deps) > + return; > + > + list_for_each_entry_safe(dep, tmp, deps, dependency) { > + if (!check_dependency(dep->fwnode)) > + pr_debug("Dependency '%s' not available\n", > + fwnode_get_name(dep->fwnode)); > + > + list_del(&dep->dependency); > + kfree(dep); > + } > + > + kfree(deps); OK, so the caller is responsible for freeing everything and the class must allocate - this definitely suggests that=20 I'm not sure there's any benefit in having deps be dynamically allocated here, just put it on the stack and iterate through the list - the iteration is going to be cheap if we get nothing back (probably the common case) and probably cheaper than the alloc/free. One thing here is that I was under the impression classes were supposed to be going away... --+9Nh279w3tr8TrRk Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJVgbjmAAoJECTWi3JdVIfQNq0H/2Llc2XIq+zFOEPaj5bKRh6i TNqomynh3aP4Gsx9uXFC+9Mxi9se/TjdxiEcY5LiwxXJ2xOF/eYumzbCZT23P0Va t1bxg6VShqLHCqouRrGHPgA/ePjQ//06dYALeNk/OPAxEL2/dyOvAUKUwG26Pb+T Tgf4EsVxeeNEEm9jy2vf8Fg1Syg8jkJrf+rQL1bO5kheQ3ReDlI1x9PgOW155M4o hH/T8KSjpn4SJTcMmRj0sRh1vHRx5BnJ+CcMWyTfcH9RHWM6KC/LxWFo3nHoPc8T rm7apJ80vp7H6aCzadM3xvi2tH0S36pKNiz/DPlNcbujUrJGISw8Ntl9Ohit+IA= =EPHw -----END PGP SIGNATURE----- --+9Nh279w3tr8TrRk-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: broonie@kernel.org (Mark Brown) Date: Wed, 17 Jun 2015 19:13:59 +0100 Subject: [PATCH 13/13] driver-core: probe dependencies before probing In-Reply-To: <1434548543-22949-14-git-send-email-tomeu.vizoso@collabora.com> References: <1434548543-22949-1-git-send-email-tomeu.vizoso@collabora.com> <1434548543-22949-14-git-send-email-tomeu.vizoso@collabora.com> Message-ID: <20150617181359.GA14071@sirena.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Jun 17, 2015 at 03:42:23PM +0200, Tomeu Vizoso wrote: > Before actually probing a device, find out what dependencies it has and > do our best to ensure that they are available at this point. > This is accomplished by finding out what platform devices need to be > probed so the dependencies are available. ...and then trying to probe them first. > If any dependencies are still unavailable after that (most probably a > missing driver or an error in the HW description from the firmware), we > print a nice error message so that people don't have to add a zillion of > printks to find out why a device asked for its probe to be deferred. So, I think I like this approach though I've not done a full pass through and I'm not sure how expensive it gets (there's definitely room for optimisation as the patch notes). I'm not 100% sure I see what prints this error message you're referring to (I'm just seeing debug prints). > +static struct fwnode_handle *get_enclosing_platform_dev( > + struct fwnode_handle *fwnode) Only platform devices? > +static void check_dependencies_per_class(struct class *class, void *data) > +{ > + struct fwnode_handle *fwnode = data; > + struct list_head *deps; > + struct fwnode_dependency *dep, *tmp; > + > + if (!class->get_dependencies) > + return; > + > + deps = class->get_dependencies(fwnode); > + if (!deps) > + return; > + > + list_for_each_entry_safe(dep, tmp, deps, dependency) { > + if (!check_dependency(dep->fwnode)) > + pr_debug("Dependency '%s' not available\n", > + fwnode_get_name(dep->fwnode)); > + > + list_del(&dep->dependency); > + kfree(dep); > + } > + > + kfree(deps); OK, so the caller is responsible for freeing everything and the class must allocate - this definitely suggests that I'm not sure there's any benefit in having deps be dynamically allocated here, just put it on the stack and iterate through the list - the iteration is going to be cheap if we get nothing back (probably the common case) and probably cheaper than the alloc/free. One thing here is that I was under the impression classes were supposed to be going away... -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: Digital signature URL: