Re: [REGRESSION] "bind" a device to a driver doesn't not workanymore

[Dmitry Torokhov]

On Mon, Oct 12, 2009 at 08:17:41PM -0700, Dmitry Torokhov wrote:
> On Mon, Oct 12, 2009 at 11:54:17AM -0700, Greg KH wrote:
> > On Mon, Oct 12, 2009 at 11:33:13AM -0700, Dmitry Torokhov wrote:
> > > On Mon, Oct 12, 2009 at 10:35:51AM -0700, Greg KH wrote:
> > > > On Mon, Oct 12, 2009 at 08:48:46AM -0700, Dmitry Torokhov wrote:
> > > > > > So at least, unbind should fail as well as bind.
> > > > > >
> > > > > 
> > > > > That would be Greg's domain s it is driver core decision whether to
> > > > > allow unbinding platform devices registered with
> > > > > platform_driver_probe().
> > > > 
> > > > No, I do not see why that should not be allowed.
> > > > 
> > > 
> > > Because once you did unbind the device you are stuck (unless the driver
> > > is compiled as a module, but then you could just unload the module
> > > instead of unbinding). Disallowing unbind would allow discarding not
> > > only __devinit but __devexit sections when driver is built in which
> > > would make ebedded people happy[^Hier].
> > 
> > Yeah, good point.
> > 
> > Does anyone do bind/unbind with platform devices today?
> > 
> > If not, a patch changing this would be welcome.
> > 
> 
> How about this one?
> 

*ping* We need to resolve this, otherwise everyone who likes to rebind
i8042 in couse of STR/STD will be in trouble.

> -- 
> Dmitry
> 
> 
> Driver core: allow certain drivers prohibit bind/unbind via sysfs
> 
> From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> 
> Platform drivers registered via platform_driver_probe() can be bound
> to devices only once, upon registration, because discard their probe()
> routines to save memory. Unbinding the driver through sysfs 'unbind'
> leaves the device stranded and confuses users so let's not create
> bind and unbind attributes for such drivers.
> 
> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx>
> ---
> 
>  drivers/base/bus.c      |   17 +++++++++++------
>  drivers/base/platform.c |    6 +++++-
>  include/linux/device.h  |    4 +++-
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 973bf2a..63c143e 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -689,15 +689,19 @@ int bus_add_driver(struct device_driver *drv)
>  		printk(KERN_ERR "%s: driver_add_attrs(%s) failed\n",
>  			__func__, drv->name);
>  	}
> -	error = add_bind_files(drv);
> -	if (error) {
> -		/* Ditto */
> -		printk(KERN_ERR "%s: add_bind_files(%s) failed\n",
> -			__func__, drv->name);
> +
> +	if (!drv->suppress_bind_attrs) {
> +		error = add_bind_files(drv);
> +		if (error) {
> +			/* Ditto */
> +			printk(KERN_ERR "%s: add_bind_files(%s) failed\n",
> +				__func__, drv->name);
> +		}
>  	}
>  
>  	kobject_uevent(&priv->kobj, KOBJ_ADD);
>  	return 0;
> +
>  out_unregister:
>  	kfree(drv->p);
>  	drv->p = NULL;
> @@ -720,7 +724,8 @@ void bus_remove_driver(struct device_driver *drv)
>  	if (!drv->bus)
>  		return;
>  
> -	remove_bind_files(drv);
> +	if (!drv->suppress_bind_attrs)
> +		remove_bind_files(drv);
>  	driver_remove_attrs(drv->bus, drv);
>  	driver_remove_file(drv, &driver_attr_uevent);
>  	klist_remove(&drv->p->knode_bus);
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index ed156a1..4fa954b 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -521,11 +521,15 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
>  {
>  	int retval, code;
>  
> +	/* make sure driver won't have bind/unbind attributes */
> +	drv->driver.suppress_bind_attrs = true;
> +
>  	/* temporary section violation during probe() */
>  	drv->probe = probe;
>  	retval = code = platform_driver_register(drv);
>  
> -	/* Fixup that section violation, being paranoid about code scanning
> +	/*
> +	 * Fixup that section violation, being paranoid about code scanning
>  	 * the list of drivers in order to probe new devices.  Check to see
>  	 * if the probe was successful, and make sure any forced probes of
>  	 * new devices fail.
> diff --git a/include/linux/device.h b/include/linux/device.h
> index aca31bf..2ea3e49 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -124,7 +124,9 @@ struct device_driver {
>  	struct bus_type		*bus;
>  
>  	struct module		*owner;
> -	const char 		*mod_name;	/* used for built-in modules */
> +	const char		*mod_name;	/* used for built-in modules */
> +
> +	bool suppress_bind_attrs;	/* disables bind/unbind via sysfs */
>  
>  	int (*probe) (struct device *dev);
>  	int (*remove) (struct device *dev);

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