lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150925142944.GB22463@ulmo>
Date:	Fri, 25 Sep 2015 16:29:44 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 1/6] driver-core: platform: Provide helpers for
 multi-driver modules

On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@...dia.com>
> 
> 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.
> 
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Thierry Reding <treding@...dia.com>
> ---
>  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/driver-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 *))
>  
> +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 *drivers,
> +					 unsigned int count);
> +
> +If one of the drivers fails to register, all drivers registered up to that
> +point will be unregistered in reverse order.
> +
>  
>  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);
>  
> +/**
> + * 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 register a
> + * driver, all previously registered drivers will be unregistered. Callers of
> + * this API should use platform_unregister_drivers() to unregister drivers 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 = 0; i < count; i++) {
> +		pr_debug("registering platform driver %ps\n", drivers[i]);
> +
> +		err = 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 system is
>   *     fully running:  "modprobe $MODALIAS"
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.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_bundle(
>  	struct resource *res, unsigned int n_res,
>  	const void *data, size_t size, struct module *module);
>  
> +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;
> -- 
> 2.5.0
> 

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ