diff mbox

[3/4] fix module autoloading for ACPI enumerated devices

Message ID 1389689198-2641-4-git-send-email-rui.zhang@intel.com (mailing list archive)
State Accepted
Headers show

Commit Message

Zhang Rui Jan. 14, 2014, 8:46 a.m. UTC
ACPI enumerated devices has ACPI style _HID and _CID strings,
all of these strings can be used for both driver loading and matching.

Currently, in Platform, I2C and SPI bus, the ACPI style driver matching
is supported by invoking acpi_driver_match_device() in bus .match() callback.
But, the module autoloading is still broken.

For example, there is any ACPI device with _HID "INTABCD" that is
enumerated to platform bus, and we have a driver that can probe it.

The driver exports its module_alias as "acpi:INTABCD" use the following code
static const struct acpi_device_id xxx_acpi_match[] = {
        { "INTABCD", 0 },
        { }
};
MODULE_DEVICE_TABLE(acpi, xxx_acpi_match);

But, unfortunately, the device' modalias is shown as "platform:INTABCD:00",
please refer to modalias_show() and platform_uevent() in
drivers/base/platform.c.
This results in that the driver will not be loaded automatically when the
device node is created, because their modalias do not match.

This also applies to I2C and SPI bus.

With this patch, the device' modalias will be shown as "acpi:INTABCD" as well.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/base/platform.c |   12 +++++++++++-
 drivers/i2c/i2c-core.c  |   11 +++++++++++
 drivers/spi/spi.c       |   10 ++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

Comments

Mika Westerberg Jan. 15, 2014, 3:08 p.m. UTC | #1
On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 3a94b79..2f4aea2 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -677,7 +677,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
>  			     char *buf)
>  {
>  	struct platform_device	*pdev = to_platform_device(dev);
> -	int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> +	int len;
> +
> +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);

Here you have PAGE_SIZE -1...

> +	if (len != -ENODEV)
> +		return len;
> +
> +	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
>  
>  	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
>  }
> @@ -699,6 +705,10 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	if (rc != -ENODEV)
>  		return rc;
>  
> +	rc = acpi_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;
> +
>  	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
>  			pdev->name);
>  	return 0;
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index d74c0b3..c4c5588 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
>  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {
>  	struct i2c_client	*client = to_i2c_client(dev);
> +	int rc;
> +
> +	rc = acpi_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;
>  
>  	if (add_uevent_var(env, "MODALIAS=%s%s",
>  			   I2C_MODULE_PREFIX, client->name))
> @@ -409,6 +414,12 @@ static ssize_t
>  show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	int len;
> +
> +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);

and here

> +	if (len != -ENODEV)
> +		return len;
> +
>  	return sprintf(buf, "%s%s\n", I2C_MODULE_PREFIX, client->name);
>  }
>  
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 349ebba..ab70eda 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -58,6 +58,11 @@ static ssize_t
>  modalias_show(struct device *dev, struct device_attribute *a, char *buf)
>  {
>  	const struct spi_device	*spi = to_spi_device(dev);
> +	int len;
> +
> +	len = acpi_device_modalias(dev, buf, PAGE_SIZE);

but here it is PAGE_SIZE.

Perhaps it should be PAGE_SIZE in all sites?
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang Rui Jan. 16, 2014, 8:04 a.m. UTC | #2
On Wed, 2014-01-15 at 17:08 +0200, Mika Westerberg wrote:
> On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 3a94b79..2f4aea2 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -677,7 +677,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
> >  			     char *buf)
> >  {
> >  	struct platform_device	*pdev = to_platform_device(dev);
> > -	int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > +	int len;
> > +
> > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> 
> Here you have PAGE_SIZE -1...
> 
> > +	if (len != -ENODEV)
> > +		return len;
> > +
> > +	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> >  
> >  	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> >  }
> > @@ -699,6 +705,10 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> >  	if (rc != -ENODEV)
> >  		return rc;
> >  
> > +	rc = acpi_device_uevent_modalias(dev, env);
> > +	if (rc != -ENODEV)
> > +		return rc;
> > +
> >  	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> >  			pdev->name);
> >  	return 0;
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index d74c0b3..c4c5588 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
> >  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
> >  {
> >  	struct i2c_client	*client = to_i2c_client(dev);
> > +	int rc;
> > +
> > +	rc = acpi_device_uevent_modalias(dev, env);
> > +	if (rc != -ENODEV)
> > +		return rc;
> >  
> >  	if (add_uevent_var(env, "MODALIAS=%s%s",
> >  			   I2C_MODULE_PREFIX, client->name))
> > @@ -409,6 +414,12 @@ static ssize_t
> >  show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
> >  {
> >  	struct i2c_client *client = to_i2c_client(dev);
> > +	int len;
> > +
> > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> 
> and here
> 
> > +	if (len != -ENODEV)
> > +		return len;
> > +
> >  	return sprintf(buf, "%s%s\n", I2C_MODULE_PREFIX, client->name);
> >  }
> >  
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 349ebba..ab70eda 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -58,6 +58,11 @@ static ssize_t
> >  modalias_show(struct device *dev, struct device_attribute *a, char *buf)
> >  {
> >  	const struct spi_device	*spi = to_spi_device(dev);
> > +	int len;
> > +
> > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE);
> 
> but here it is PAGE_SIZE.
> 
good catch.

> Perhaps it should be PAGE_SIZE in all sites?

dev_attr_show() will give a warning message if modalias_show() returns
PAGE_SIZE, thus I'd prefer to use PAGE_SIZE - 1 for all sites.

thanks,
rui


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Jan. 16, 2014, 12:27 p.m. UTC | #3
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index d74c0b3..c4c5588 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
>  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {
>  	struct i2c_client	*client = to_i2c_client(dev);
> +	int rc;
> +
> +	rc = acpi_device_uevent_modalias(dev, env);
> +	if (rc != -ENODEV)
> +		return rc;
>  
>  	if (add_uevent_var(env, "MODALIAS=%s%s",
>  			   I2C_MODULE_PREFIX, client->name))

I wonder why we don't have/need that with CONFIG_OF? Because probably
nobody is using modules with i2c devices there?
Mark Brown Jan. 16, 2014, 12:28 p.m. UTC | #4
On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> ACPI enumerated devices has ACPI style _HID and _CID strings,
> all of these strings can be used for both driver loading and matching.
> 
> Currently, in Platform, I2C and SPI bus, the ACPI style driver matching
> is supported by invoking acpi_driver_match_device() in bus .match() callback.
> But, the module autoloading is still broken.

Acked-by: Mark Brown <broonie@linaro.org>

modulo the PAGE_SIZE stuff Mika noted - unless this changes radically
please just assume I'm OK with it.
Zhang Rui Jan. 16, 2014, 12:51 p.m. UTC | #5
On Thu, 2014-01-16 at 12:28 +0000, Mark Brown wrote:
> On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> > ACPI enumerated devices has ACPI style _HID and _CID strings,
> > all of these strings can be used for both driver loading and matching.
> > 
> > Currently, in Platform, I2C and SPI bus, the ACPI style driver matching
> > is supported by invoking acpi_driver_match_device() in bus .match() callback.
> > But, the module autoloading is still broken.
> 
> Acked-by: Mark Brown <broonie@linaro.org>
> 
> modulo the PAGE_SIZE stuff Mika noted - unless this changes radically
> please just assume I'm OK with it.

thanks.

-rui


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang Rui Jan. 16, 2014, 1:05 p.m. UTC | #6
On Thu, 2014-01-16 at 13:27 +0100, Wolfram Sang wrote:
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index d74c0b3..c4c5588 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
> >  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
> >  {
> >  	struct i2c_client	*client = to_i2c_client(dev);
> > +	int rc;
> > +
> > +	rc = acpi_device_uevent_modalias(dev, env);
> > +	if (rc != -ENODEV)
> > +		return rc;
> >  
> >  	if (add_uevent_var(env, "MODALIAS=%s%s",
> >  			   I2C_MODULE_PREFIX, client->name))
> 
> I wonder why we don't have/need that with CONFIG_OF? Because probably
> nobody is using modules with i2c devices there?
> 
This seems a gap to me but I'm not 100% sure.
I saw Grant Likely introduced the OF style MODALIAS to platform bus, and
OF style registration/binding to i2c bus, maybe he has an answer for
this.

thanks,
rui


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Jan. 16, 2014, 7:24 p.m. UTC | #7
On Thu, Jan 16, 2014 at 12:28:19PM +0000, Mark Brown wrote:
> On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> > ACPI enumerated devices has ACPI style _HID and _CID strings,
> > all of these strings can be used for both driver loading and matching.
> > 
> > Currently, in Platform, I2C and SPI bus, the ACPI style driver matching
> > is supported by invoking acpi_driver_match_device() in bus .match() callback.
> > But, the module autoloading is still broken.
> 
> Acked-by: Mark Brown <broonie@linaro.org>
> 
> modulo the PAGE_SIZE stuff Mika noted - unless this changes radically
> please just assume I'm OK with it.

Ditto

Acked-by: Wolfram Sang <wsa@the-dreams.de>
Mark Brown Jan. 16, 2014, 7:46 p.m. UTC | #8
On Thu, Jan 16, 2014 at 09:05:09PM +0800, Zhang Rui wrote:
> On Thu, 2014-01-16 at 13:27 +0100, Wolfram Sang wrote:

> > I wonder why we don't have/need that with CONFIG_OF? Because probably
> > nobody is using modules with i2c devices there?

> This seems a gap to me but I'm not 100% sure.
> I saw Grant Likely introduced the OF style MODALIAS to platform bus, and
> OF style registration/binding to i2c bus, maybe he has an answer for
> this.

This is needed for ACPI because we rewrite the device names to give us
stable names.  With OF for I2C and SPI nothing bus specific is done that 
affects this stuff so the default behaviour works.
Rafael J. Wysocki Jan. 17, 2014, 1:28 a.m. UTC | #9
On Thursday, January 16, 2014 04:04:35 PM Zhang Rui wrote:
> On Wed, 2014-01-15 at 17:08 +0200, Mika Westerberg wrote:
> > On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > index 3a94b79..2f4aea2 100644
> > > --- a/drivers/base/platform.c
> > > +++ b/drivers/base/platform.c
> > > @@ -677,7 +677,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
> > >  			     char *buf)
> > >  {
> > >  	struct platform_device	*pdev = to_platform_device(dev);
> > > -	int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > > +	int len;
> > > +
> > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> > 
> > Here you have PAGE_SIZE -1...
> > 
> > > +	if (len != -ENODEV)
> > > +		return len;
> > > +
> > > +	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > >  
> > >  	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> > >  }
> > > @@ -699,6 +705,10 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> > >  	if (rc != -ENODEV)
> > >  		return rc;
> > >  
> > > +	rc = acpi_device_uevent_modalias(dev, env);
> > > +	if (rc != -ENODEV)
> > > +		return rc;
> > > +
> > >  	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > >  			pdev->name);
> > >  	return 0;
> > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > index d74c0b3..c4c5588 100644
> > > --- a/drivers/i2c/i2c-core.c
> > > +++ b/drivers/i2c/i2c-core.c
> > > @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
> > >  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
> > >  {
> > >  	struct i2c_client	*client = to_i2c_client(dev);
> > > +	int rc;
> > > +
> > > +	rc = acpi_device_uevent_modalias(dev, env);
> > > +	if (rc != -ENODEV)
> > > +		return rc;
> > >  
> > >  	if (add_uevent_var(env, "MODALIAS=%s%s",
> > >  			   I2C_MODULE_PREFIX, client->name))
> > > @@ -409,6 +414,12 @@ static ssize_t
> > >  show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
> > >  {
> > >  	struct i2c_client *client = to_i2c_client(dev);
> > > +	int len;
> > > +
> > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> > 
> > and here
> > 
> > > +	if (len != -ENODEV)
> > > +		return len;
> > > +
> > >  	return sprintf(buf, "%s%s\n", I2C_MODULE_PREFIX, client->name);
> > >  }
> > >  
> > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > > index 349ebba..ab70eda 100644
> > > --- a/drivers/spi/spi.c
> > > +++ b/drivers/spi/spi.c
> > > @@ -58,6 +58,11 @@ static ssize_t
> > >  modalias_show(struct device *dev, struct device_attribute *a, char *buf)
> > >  {
> > >  	const struct spi_device	*spi = to_spi_device(dev);
> > > +	int len;
> > > +
> > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE);
> > 
> > but here it is PAGE_SIZE.
> > 
> good catch.
> 
> > Perhaps it should be PAGE_SIZE in all sites?
> 
> dev_attr_show() will give a warning message if modalias_show() returns
> PAGE_SIZE, thus I'd prefer to use PAGE_SIZE - 1 for all sites.

So I changed the PAGE_SIZE to PAGE_SIZE - 1 in the last instance and queued
up the whole series for 3.14 in linux-pm.git/linux-next.  Please have a look
at that and let me know if there's anything wrong with it.

Thanks!
Zhang Rui Jan. 17, 2014, 1:56 a.m. UTC | #10
On Fri, 2014-01-17 at 02:28 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 16, 2014 04:04:35 PM Zhang Rui wrote:
> > On Wed, 2014-01-15 at 17:08 +0200, Mika Westerberg wrote:
> > > On Tue, Jan 14, 2014 at 04:46:37PM +0800, Zhang Rui wrote:
> > > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > > > index 3a94b79..2f4aea2 100644
> > > > --- a/drivers/base/platform.c
> > > > +++ b/drivers/base/platform.c
> > > > @@ -677,7 +677,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
> > > >  			     char *buf)
> > > >  {
> > > >  	struct platform_device	*pdev = to_platform_device(dev);
> > > > -	int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > > > +	int len;
> > > > +
> > > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> > > 
> > > Here you have PAGE_SIZE -1...
> > > 
> > > > +	if (len != -ENODEV)
> > > > +		return len;
> > > > +
> > > > +	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > > >  
> > > >  	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> > > >  }
> > > > @@ -699,6 +705,10 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
> > > >  	if (rc != -ENODEV)
> > > >  		return rc;
> > > >  
> > > > +	rc = acpi_device_uevent_modalias(dev, env);
> > > > +	if (rc != -ENODEV)
> > > > +		return rc;
> > > > +
> > > >  	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > > >  			pdev->name);
> > > >  	return 0;
> > > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > > > index d74c0b3..c4c5588 100644
> > > > --- a/drivers/i2c/i2c-core.c
> > > > +++ b/drivers/i2c/i2c-core.c
> > > > @@ -104,6 +104,11 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv)
> > > >  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
> > > >  {
> > > >  	struct i2c_client	*client = to_i2c_client(dev);
> > > > +	int rc;
> > > > +
> > > > +	rc = acpi_device_uevent_modalias(dev, env);
> > > > +	if (rc != -ENODEV)
> > > > +		return rc;
> > > >  
> > > >  	if (add_uevent_var(env, "MODALIAS=%s%s",
> > > >  			   I2C_MODULE_PREFIX, client->name))
> > > > @@ -409,6 +414,12 @@ static ssize_t
> > > >  show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
> > > >  {
> > > >  	struct i2c_client *client = to_i2c_client(dev);
> > > > +	int len;
> > > > +
> > > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
> > > 
> > > and here
> > > 
> > > > +	if (len != -ENODEV)
> > > > +		return len;
> > > > +
> > > >  	return sprintf(buf, "%s%s\n", I2C_MODULE_PREFIX, client->name);
> > > >  }
> > > >  
> > > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > > > index 349ebba..ab70eda 100644
> > > > --- a/drivers/spi/spi.c
> > > > +++ b/drivers/spi/spi.c
> > > > @@ -58,6 +58,11 @@ static ssize_t
> > > >  modalias_show(struct device *dev, struct device_attribute *a, char *buf)
> > > >  {
> > > >  	const struct spi_device	*spi = to_spi_device(dev);
> > > > +	int len;
> > > > +
> > > > +	len = acpi_device_modalias(dev, buf, PAGE_SIZE);
> > > 
> > > but here it is PAGE_SIZE.
> > > 
> > good catch.
> > 
> > > Perhaps it should be PAGE_SIZE in all sites?
> > 
> > dev_attr_show() will give a warning message if modalias_show() returns
> > PAGE_SIZE, thus I'd prefer to use PAGE_SIZE - 1 for all sites.
> 
> So I changed the PAGE_SIZE to PAGE_SIZE - 1 in the last instance and queued
> up the whole series for 3.14 in linux-pm.git/linux-next.  Please have a look
> at that and let me know if there's anything wrong with it.
> 
the change looks okay to me.

thanks,
rui
> Thanks!
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Nikula Jan. 17, 2014, 7:37 a.m. UTC | #11
On 01/16/2014 09:46 PM, Mark Brown wrote:
> On Thu, Jan 16, 2014 at 09:05:09PM +0800, Zhang Rui wrote:
>> On Thu, 2014-01-16 at 13:27 +0100, Wolfram Sang wrote:
>> This seems a gap to me but I'm not 100% sure.
>> I saw Grant Likely introduced the OF style MODALIAS to platform bus, and
>> OF style registration/binding to i2c bus, maybe he has an answer for
>> this.
> This is needed for ACPI because we rewrite the device names to give us
> stable names.  With OF for I2C and SPI nothing bus specific is done that
> affects this stuff so the default behaviour works.
Sidenote: actually this modalias/module loading issue is different and 
not related to stable ACPI i2c/spi slave device names.
Mark Brown Jan. 17, 2014, 3:57 p.m. UTC | #12
On Fri, Jan 17, 2014 at 09:37:56AM +0200, Jarkko Nikula wrote:
> On 01/16/2014 09:46 PM, Mark Brown wrote:

> >This is needed for ACPI because we rewrite the device names to give us
> >stable names.  With OF for I2C and SPI nothing bus specific is done that
> >affects this stuff so the default behaviour works.

> Sidenote: actually this modalias/module loading issue is different
> and not related to stable ACPI i2c/spi slave device names.

Oh, I'd been under the impression that it was the rewrite that was
triggering this?
Jarkko Nikula Jan. 20, 2014, 7:10 a.m. UTC | #13
On 01/17/2014 05:57 PM, Mark Brown wrote:
> On Fri, Jan 17, 2014 at 09:37:56AM +0200, Jarkko Nikula wrote:
>> Sidenote: actually this modalias/module loading issue is different
>> and not related to stable ACPI i2c/spi slave device names.
> Oh, I'd been under the impression that it was the rewrite that was
> triggering this?
IIRC issue has been there since when ACPI slave device support was added.

I have a partial fix for it in cf9eb39c6f7b ("spi: Fix modalias for ACPI 
enumerated SPI devices") and when doing similar change for i2c Rui 
pointed me that he has a better fix that takes care of _CID string and 
platform code too.
diff mbox

Patch

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 3a94b79..2f4aea2 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -677,7 +677,13 @@  static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 			     char *buf)
 {
 	struct platform_device	*pdev = to_platform_device(dev);
-	int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
+	if (len != -ENODEV)
+		return len;
+
+	len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
 
 	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
 }
@@ -699,6 +705,10 @@  static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 	if (rc != -ENODEV)
 		return rc;
 
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
+
 	add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
 			pdev->name);
 	return 0;
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d74c0b3..c4c5588 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -104,6 +104,11 @@  static int i2c_device_match(struct device *dev, struct device_driver *drv)
 static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
+	int rc;
+
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
 
 	if (add_uevent_var(env, "MODALIAS=%s%s",
 			   I2C_MODULE_PREFIX, client->name))
@@ -409,6 +414,12 @@  static ssize_t
 show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE -1);
+	if (len != -ENODEV)
+		return len;
+
 	return sprintf(buf, "%s%s\n", I2C_MODULE_PREFIX, client->name);
 }
 
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 349ebba..ab70eda 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -58,6 +58,11 @@  static ssize_t
 modalias_show(struct device *dev, struct device_attribute *a, char *buf)
 {
 	const struct spi_device	*spi = to_spi_device(dev);
+	int len;
+
+	len = acpi_device_modalias(dev, buf, PAGE_SIZE);
+	if (len != -ENODEV)
+		return len;
 
 	return sprintf(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias);
 }
@@ -114,6 +119,11 @@  static int spi_match_device(struct device *dev, struct device_driver *drv)
 static int spi_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	const struct spi_device		*spi = to_spi_device(dev);
+	int rc;
+
+	rc = acpi_device_uevent_modalias(dev, env);
+	if (rc != -ENODEV)
+		return rc;
 
 	add_uevent_var(env, "MODALIAS=%s%s", SPI_MODULE_PREFIX, spi->modalias);
 	return 0;