Message ID | 1389620911-3890-1-git-send-email-rui.zhang@intel.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
On Mon, Jan 13, 2014 at 09:48:31PM +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. > But currently, in Platform, I2C and SPI bus, only the ACPI style > driver matching is supported by invoking acpi_driver_match_device() > in bus .match() callback. I don't understand what this means, sorry. As far as I can tell "ACPI style _HID and _CID strings" are something different to "ACPI style driver matching" but what that actually means is not at all clear to me so I don't know what problem this is intended to fix. Please also always remember to CC maintainers on patches. > 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); > } What does this do and why can't acpi_driver_match_device() handle this like it does for other ACPI devices? We don't need to add such code for other firmware interfaces...
On Mon, 2014-01-13 at 17:35 +0000, Mark Brown wrote: > On Mon, Jan 13, 2014 at 09:48:31PM +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. > > > But currently, in Platform, I2C and SPI bus, only the ACPI style > > driver matching is supported by invoking acpi_driver_match_device() > > in bus .match() callback. > > I don't understand what this means, sorry. sorry, I should be clearer about this. > As far as I can tell "ACPI > style _HID and _CID strings" are something different to "ACPI style > driver matching" but what that actually means is not at all clear to me > so I don't know what problem this is intended to fix. > I gave a more detailed description about the problem in the changelog of patch 2/4. Take the following piece of code for example, static const struct acpi_device_id xxx_acpi_match[] = { { "INTABCD", 0 }, { } }; MODULE_DEVICE_TABLE(acpi, xxx_acpi_match); this can be seen in a platform/I2C/SPI driver that supports an ACPI enumerated device, right? If this piece of code is used in a platform driver for an ACPI enumerated platform device, the platform DRIVER module_alias is "acpi:INTABCD", but the uevent attribute of its platform device node is "platform:INTABCD:00" (PREFIX:platform_device->name). If this piece of code is used in an I2C driver for an ACPI enumerated i2c device, the i2c driver module_alias is "acpi:INTABCD", but the uevent of its i2c device node is "i2c:INTABCD:00" (PREFIX:i2c_client->name). If this piece of code is used in an *SPI* driver for an ACPI enumerated spi device, the spi driver module_alias is "acpi:INTABCD", but the uevent of its spi device node is "spi:INTABCD" (PREFIX:spi_device->modalias). thus when the device node is created, the driver will not be loaded automatically because their modalias do not match. > Please also always remember to CC maintainers on patches. > okay, will resend the patches later. thanks, rui > > 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); > > } > > What does this do and why can't acpi_driver_match_device() handle this > like it does for other ACPI devices? We don't need to add such code for > other firmware interfaces... -- 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
On Tue, Jan 14, 2014 at 04:00:17PM +0800, Zhang Rui wrote: > On Mon, 2014-01-13 at 17:35 +0000, Mark Brown wrote: > > On Mon, Jan 13, 2014 at 09:48:31PM +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. > If this piece of code is used in an *SPI* driver for an ACPI enumerated > spi device, the spi driver module_alias is "acpi:INTABCD", but > the uevent of its spi device node is > "spi:INTABCD" (PREFIX:spi_device->modalias). OK that makes sense, but what does this have to do with the _HID and _CID methods? Surely we're just replacing spi: with acpi: in the uevent?
On Tue, 2014-01-14 at 14:41 +0000, Mark Brown wrote: > On Tue, Jan 14, 2014 at 04:00:17PM +0800, Zhang Rui wrote: > > On Mon, 2014-01-13 at 17:35 +0000, Mark Brown wrote: > > > On Mon, Jan 13, 2014 at 09:48:31PM +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. > > > If this piece of code is used in an *SPI* driver for an ACPI enumerated > > spi device, the spi driver module_alias is "acpi:INTABCD", but > > the uevent of its spi device node is > > "spi:INTABCD" (PREFIX:spi_device->modalias). > > OK that makes sense, but what does this have to do with the _HID and > _CID methods? If an ACPI enumerated SPI device has a _HID and a _CID, both of them need to be exposed in 'uevent', so that a driver that matches _CID can also have a chance to be probed. This can not be done by the current code, thus we need special handling of ACPI enumerated SPI devices. > Surely we're just replacing spi: with acpi: in the uevent? yes. 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
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;
ACPI enumerated devices has ACPI style _HID and _CID strings, all of these strings can be used for both driver loading and matching. But currently, in Platform, I2C and SPI bus, only the ACPI style driver matching is supported by invoking acpi_driver_match_device() in bus .match() callback. This patch fixes module autoloading on those buses for ACPI enumerated devices. 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(-)