Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1232265 > unrolled thread

[PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules Thierry Reding <thierry.reding@gmail.com> - 2015-09-24 19:10 +0200
    Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules Jani Nikula <jani.nikula@linux.intel.com> - 2015-09-25 12:30 +0200
      Re: [PATCH 1/6] driver-core: platform: Provide helpers for  multi-driver modules Thierry Reding <thierry.reding@gmail.com> - 2015-09-25 17:20 +0200
        Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules Jani Nikula <jani.nikula@linux.intel.com> - 2015-09-28 08:40 +0200
    Re: [PATCH 1/6] driver-core: platform: Provide helpers for  multi-driver modules Thierry Reding <thierry.reding@gmail.com> - 2015-09-25 16:30 +0200
      Re: [PATCH 1/6] driver-core: platform: Provide helpers for  multi-driver modules Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-09-26 03:00 +0200
    [PATCH v2] driver-core: platform: Provide helpers for multi-driver modules Thierry Reding <thierry.reding@gmail.com> - 2015-09-25 17:30 +0200
      Re: [PATCH v2] driver-core: platform: Provide helpers for  multi-driver modules Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-09-28 08:50 +0200
    Re: [PATCH 1/6] driver-core: platform: Provide helpers for  multi-driver modules Thierry Reding <thierry.reding@gmail.com> - 2015-09-25 17:30 +0200
    Re: [PATCH 1/6] driver-core: platform: Provide helpers for  multi-driver modules Daniel Vetter <daniel@ffwll.ch> - 2015-09-28 11:10 +0200
      Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules Eric Anholt <eric@anholt.net> - 2015-09-28 21:20 +0200

#1232265 — [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 Documentation/driver-model/platform.txt | 11 ++++++
 drivers/base/platform.c                 | 60 +++++++++++++++++++++++++++++++++
 include/linux/platform_device.h         |  5 +++
 3 files changed, 76 insertions(+)

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1232706

On Thu, 24 Sep 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  Documentation/driver-model/platform.txt | 11 ++++++
>  drivers/base/platform.c                 | 60 +++++++++++++++++++++++++++++++++
>  include/linux/platform_device.h         |  5 +++
>  3 files changed, 76 insertions(+)
>
> 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]);
> +	}

This will call platform_driver_unregister() on the driver that failed,
but not the first driver.

You should probably make i an int, and use while (--i >= 0).

BR,
Jani.

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

-- 
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1232884 — Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 25, 2015 at 01:27:28PM +0300, Jani Nikula wrote:
> On Thu, 24 Sep 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
[...]
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
[...]
> > +/**
> > + * 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]);
> > +	}
> 
> This will call platform_driver_unregister() on the driver that failed,
> but not the first driver.
> 
> You should probably make i an int, and use while (--i >= 0).

Actually it won't. I was especially careful and even tested this with
one driver by instrumenting platform_driver_register() to return failure
at various points in the sequence.

This works fine.

Thierry

[toc] | [prev] | [next] | [standalone]

#1233879

On Fri, 25 Sep 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Fri, Sep 25, 2015 at 01:27:28PM +0300, Jani Nikula wrote:
>> On Thu, 24 Sep 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> [...]
>> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> [...]
>> > +/**
>> > + * 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]);
>> > +	}
>> 
>> This will call platform_driver_unregister() on the driver that failed,
>> but not the first driver.
>> 
>> You should probably make i an int, and use while (--i >= 0).
>
> Actually it won't. I was especially careful and even tested this with
> one driver by instrumenting platform_driver_register() to return failure
> at various points in the sequence.
>
> This works fine.

You are right, of course. What was I thinking. My kingdom for an excuse!

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1232847 — Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.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
> 

[toc] | [prev] | [next] | [standalone]

#1233120 — Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

On Fri, Sep 25, 2015 at 04:29:44PM +0200, Thierry Reding wrote:
> On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> > Signed-off-by: Thierry Reding <treding@nvidia.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?

The last thing is the easiest, but you have to wait a release cycle for
that.  Or I can take the whole series if you want to, just cc: the
proper subsystem maintainers when you send them.  It's up to you.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1232885 — [PATCH v2] driver-core: platform: Provide helpers for multi-driver modules

From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- properly pass around the owner module

 Documentation/driver-model/platform.txt | 14 ++++++++
 drivers/base/platform.c                 | 61 +++++++++++++++++++++++++++++++++
 include/linux/platform_device.h         |  8 +++++
 3 files changed, 83 insertions(+)

diff --git a/Documentation/driver-model/platform.txt b/Documentation/driver-model/platform.txt
index 07795ec51cde..e456696cfef2 100644
--- a/Documentation/driver-model/platform.txt
+++ b/Documentation/driver-model/platform.txt
@@ -63,6 +63,20 @@ 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, struct module *owner);
+	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. Note that there is a convenience
+macro that passes THIS_MODULE as owner parameter:
+
+	#define platform_register_driver(drivers, count)
+
 
 Device Enumeration
 ~~~~~~~~~~~~~~~~~~
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f80aaaf9f610..68c58c43e45a 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -711,6 +711,67 @@ 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
+ * @owner: module owning the drivers
+ *
+ * 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, struct module *owner)
+{
+	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], owner);
+		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..dc777be5f2e1 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -270,6 +270,14 @@ 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, struct module *owner);
+void platform_unregister_drivers(struct platform_driver * const *drivers,
+				 unsigned int count);
+
+#define platform_register_drivers(drivers, count) \
+	__platform_register_drivers(drivers, count, THIS_MODULE)
+
 /* early platform driver interface */
 struct early_platform_driver {
 	const char *class_str;
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1233884 — Re: [PATCH v2] driver-core: platform: Provide helpers for multi-driver modules

On Fri, Sep 25, 2015 at 6:29 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index f80aaaf9f610..68c58c43e45a 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -711,6 +711,67 @@ 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
> + * @owner: module owning the drivers
> + *
> + * 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, struct module *owner)
> +{
> +       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], owner);
> +               if (err < 0) {
> +                       pr_err("failed to register platform driver %ps: %d\n",
> +                              drivers[i], err);

Would
platform_unregister_drivers(drivers, i);
work?

> +                       goto error;
> +               }
> +       }
> +
> +       return 0;
> +
> +error:
> +       while (i--) {

I think Jani was confused since often idiom is 'while (--i >=0)' is used.

> +               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);

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1232887 — Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 24, 2015 at 07:02:36PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  Documentation/driver-model/platform.txt | 11 ++++++
>  drivers/base/platform.c                 | 60 +++++++++++++++++++++++++++++++++
>  include/linux/platform_device.h         |  5 +++
>  3 files changed, 76 insertions(+)
> 
> 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]);

I notice that this is actually doing the wrong thing because the
platform drivers will end up with their .owner field set to NULL because
this file is always built-in. I've fixed it up by passing in a struct
module *owner into __platform_register_drivers() and pass it on to the
__platform_driver_register() function instead.

Thierry

[toc] | [prev] | [next] | [standalone]

#1233956 — Re: [PATCH 1/6] driver-core: platform: Provide helpers for multi-driver modules

On Thu, Sep 24, 2015 at 7:02 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

I raised this already on irc but let's do it here too for the record:
Eric Anholt has a very similar thing (but in drm only) with the
addition of also integrating with the component framework:

http://lists.freedesktop.org/archives/dri-devel/2015-September/090449.html

Having something that works for everyone (so includes msm and can be
used for vc4 too) would be great. Adding Eric.
-Daniel

> ---
>  Documentation/driver-model/platform.txt | 11 ++++++
>  drivers/base/platform.c                 | 60 +++++++++++++++++++++++++++++++++
>  include/linux/platform_device.h         |  5 +++
>  3 files changed, 76 insertions(+)
>
> 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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234351

[Multipart message — attachments visible in raw view] — view raw

Daniel Vetter <daniel@ffwll.ch> writes:

> On Thu, Sep 24, 2015 at 7:02 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> From: Thierry Reding <treding@nvidia.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@linuxfoundation.org>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> I raised this already on irc but let's do it here too for the record:
> Eric Anholt has a very similar thing (but in drm only) with the
> addition of also integrating with the component framework:
>
> http://lists.freedesktop.org/archives/dri-devel/2015-September/090449.html
>
> Having something that works for everyone (so includes msm and can be
> used for vc4 too) would be great. Adding Eric.

I'm not sure if I should be providing a Reviewed-by here, but I like
this patch.  It obsoletes part of my patch series, and I look forward to
using it in my driver.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web