From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756750AbbIYO3u (ORCPT ); Fri, 25 Sep 2015 10:29:50 -0400 Received: from mail-wi0-f169.google.com ([209.85.212.169]:35481 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756295AbbIYO3t (ORCPT ); Fri, 25 Sep 2015 10:29:49 -0400 Date: Fri, 25 Sep 2015 16:29:44 +0200 From: Thierry Reding To: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules Message-ID: <20150925142944.GB22463@ulmo> References: <1443114161-7965-1-git-send-email-thierry.reding@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Fba/0zbH8Xs+Fj9o" Content-Disposition: inline In-Reply-To: <1443114161-7965-1-git-send-email-thierry.reding@gmail.com> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --Fba/0zbH8Xs+Fj9o Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote: > From: Thierry Reding >=20 > Some modules register several sub-drivers. Provide a helper that makes > it easy to register and unregister a list of sub-drivers, as well as > unwind properly on error. >=20 > Cc: Greg Kroah-Hartman > Signed-off-by: Thierry Reding > --- > Documentation/driver-model/platform.txt | 11 ++++++ > drivers/base/platform.c | 60 +++++++++++++++++++++++++++= ++++++ > include/linux/platform_device.h | 5 +++ > 3 files changed, 76 insertions(+) Hi Greg, In addition to patches 2-6 I have about two dozen patches across various subsystems that make use of this. I didn't want to spam everyone with all of them before you've given an indication about what you think about this patch. The diffstat of the conversions I did is this: drivers/crypto/n2_core.c | 17 +++++++---------- drivers/edac/mpc85xx_edac.c | 16 ++++++++-------- drivers/edac/mv64x60_edac.c | 39 +++++++++++-------------= --------------- drivers/gpio/gpio-mpc5200.c | 17 +++++++---------- drivers/gpu/drm/armada/armada_drv.c | 16 +++++++--------- drivers/gpu/drm/exynos/exynos_drm_drv.c | 42 ++++++++----------------= ------------------ drivers/gpu/drm/omapdrm/omap_drv.c | 24 +++++++----------------- drivers/gpu/host1x/dev.c | 17 +++++++---------- drivers/input/misc/sparcspkr.c | 18 +++++++----------- drivers/iommu/msm_iommu_dev.c | 25 +++++++------------------ drivers/leds/leds-sunfire.c | 23 +++++++---------------- drivers/mfd/sta2x11-mfd.c | 36 ++++++++++--------------= ------------ drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++------- drivers/net/ethernet/broadcom/bcm63xx_enet.c | 28 ++++++++----------------= ---- drivers/net/ethernet/freescale/fec_mpc52xx.c | 22 +++++++++------------- drivers/net/ethernet/marvell/mv643xx_eth.c | 19 +++++++------------ drivers/pinctrl/pinctrl-adi2.c | 24 ++++++++---------------- drivers/pinctrl/pinctrl-at91.c | 14 +++++++------- drivers/regulator/lp8788-ldo.c | 16 +++++++--------- drivers/regulator/wm831x-dcdc.c | 31 +++++++++---------------= ------- drivers/regulator/wm831x-ldo.c | 27 ++++++++----------------= --- drivers/tty/serial/mpsc.c | 19 ++++++++----------- drivers/usb/gadget/udc/dummy_hcd.c | 17 ++++++++--------- drivers/video/fbdev/s3c2410fb.c | 15 +++++++-------- 24 files changed, 186 insertions(+), 350 deletions(-) That's not too thrilling but in many cases this fixes a potential bug if a driver fails to register and the code doesn't properly unregister any previously registered drivers. Anyway, if you think this is worth merging, how would you like to go about it? Do you want Acked-bys on all the patches and merge them through your tree? Perhaps the easiest would be to just merge this patch and then take the other patches through the maintainer trees after the core patch has landed? Thierry > diff --git a/Documentation/driver-model/platform.txt b/Documentation/driv= er-model/platform.txt > index 07795ec51cde..e80468738ba9 100644 > --- a/Documentation/driver-model/platform.txt > +++ b/Documentation/driver-model/platform.txt > @@ -63,6 +63,17 @@ runtime memory footprint: > int platform_driver_probe(struct platform_driver *drv, > int (*probe)(struct platform_device *)) > =20 > +Kernel modules can be composed of several platform drivers. The platform= core > +provides helpers to register and unregister an array of drivers: > + > + int platform_register_drivers(struct platform_driver * const *drivers, > + unsigned int count); > + void platform_unregister_drivers(struct platform_driver * const *driver= s, > + unsigned int count); > + > +If one of the drivers fails to register, all drivers registered up to th= at > +point will be unregistered in reverse order. > + > =20 > Device Enumeration > ~~~~~~~~~~~~~~~~~~ > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index f80aaaf9f610..b7d7987fda97 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -711,6 +711,66 @@ err_out: > } > EXPORT_SYMBOL_GPL(__platform_create_bundle); > =20 > +/** > + * platform_register_drivers - register an array of platform drivers > + * @drivers: an array of drivers to register > + * @count: the number of drivers to register > + * > + * Registers platform drivers specified by an array. On failure to regis= ter a > + * driver, all previously registered drivers will be unregistered. Calle= rs of > + * this API should use platform_unregister_drivers() to unregister drive= rs in > + * the reverse order. > + * > + * Returns: 0 on success or a negative error code on failure. > + */ > +int platform_register_drivers(struct platform_driver * const *drivers, > + unsigned int count) > +{ > + unsigned int i; > + int err; > + > + for (i =3D 0; i < count; i++) { > + pr_debug("registering platform driver %ps\n", drivers[i]); > + > + err =3D platform_driver_register(drivers[i]); > + if (err < 0) { > + pr_err("failed to register platform driver %ps: %d\n", > + drivers[i], err); > + goto error; > + } > + } > + > + return 0; > + > +error: > + while (i--) { > + pr_debug("unregistering platform driver %ps\n", drivers[i]); > + platform_driver_unregister(drivers[i]); > + } > + > + return err; > +} > +EXPORT_SYMBOL_GPL(platform_register_drivers); > + > +/** > + * platform_unregister_drivers - unregister an array of platform drivers > + * @drivers: an array of drivers to unregister > + * @count: the number of drivers to unregister > + * > + * Unegisters platform drivers specified by an array. This is typically = used > + * to complement an earlier call to platform_register_drivers(). Drivers= are > + * unregistered in the reverse order in which they were registered. > + */ > +void platform_unregister_drivers(struct platform_driver * const *drivers, > + unsigned int count) > +{ > + while (count--) { > + pr_debug("unregistering platform driver %ps\n", drivers[count]); > + platform_driver_unregister(drivers[count]); > + } > +} > +EXPORT_SYMBOL_GPL(platform_unregister_drivers); > + > /* modalias support enables more hands-off userspace setup: > * (a) environment variable lets new-style hotplug events work once syst= em is > * fully running: "modprobe $MODALIAS" > diff --git a/include/linux/platform_device.h b/include/linux/platform_dev= ice.h > index bba08f44cc97..0c9f16bfdd99 100644 > --- a/include/linux/platform_device.h > +++ b/include/linux/platform_device.h > @@ -270,6 +270,11 @@ extern struct platform_device *__platform_create_bun= dle( > struct resource *res, unsigned int n_res, > const void *data, size_t size, struct module *module); > =20 > +int platform_register_drivers(struct platform_driver * const *drivers, > + unsigned int count); > +void platform_unregister_drivers(struct platform_driver * const *drivers, > + unsigned int count); > + > /* early platform driver interface */ > struct early_platform_driver { > const char *class_str; > --=20 > 2.5.0 >=20 --Fba/0zbH8Xs+Fj9o Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWBVpYAAoJEN0jrNd/PrOhR74P/AqAFXFvmhgZuY1b7E9qy/B9 u2s+TYlJcnPf640S1bRIbh4VdYS9Cf04qBWOnQyWkc/IxSKOFZ58uDB1UdOQnOUi 4siYRdXd6EYMuwDtLkdkwA54tUfZ17oa6+CZ9Wqc4q620ZSddwaxKGvTHg7GkKmD aP7DDYn249W4nKnnZ6PHF7Vtg7gWVfBIWv0nTDaGAYsG5Lz+cifsLY15s9ye/ZOo 9DXerXIilzQ6NMJw9rBMSJSdGGJysiB0Le9Qw24svwJM0M7UsBZ+RVigB7aRwKpQ 5Ze1DF+uXoO0PFaGN/C1GWTZvsLuZywRx+99kaulXjr1RYlLG5u5uzgVQcEFNDk/ BkKw1mzmaAWa6NiqyYKXwxY59iJnHui25MaKch2DxqjBS4GEtlNF0lQyGllvCvMW AqzASw9d+GxHr4usf8KfZE5llB1vUiIpNj28Ev1UggZtle5Zzn563h1WBNfZfcWZ 8rD98WZyW3WcsdVEelPexp6yJbIxdzud26N19L2ljlxUA+VUKX+nSYR2xVqeNcIh 9jZSWuiMgQek2NpRekAjKlavclwLEeEKojB7bhY4ZKrkXggXAbPtIPLcPQGL2eu2 JCqoqDlRZRpLEDlbK1B4JKx3+J4nFTDQwiXdGThcK9lGf8BC2kClhH8ZmwFfGgtb cj3JjubShWB78o/4/dcV =qZpx -----END PGP SIGNATURE----- --Fba/0zbH8Xs+Fj9o--