diff mbox

[RFC,1/6] Introduce acpi_match_device_id().

Message ID 1348817910.10877.321.camel@rui.sh.intel.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Zhang, Rui Sept. 28, 2012, 7:38 a.m. UTC 
From 72df5d1f51fb27a4ba7f70a3b07df759d32b8288 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Thu, 27 Sep 2012 15:11:55 +0800
Subject: [RFC PATCH 1/6] Introduce acpi_match_device_id().

This API is used to check if a device id string is compatible
with an ACPI device,
either PNP id exported via _HID or compatible ids exported
via _CID control method.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/scan.c     |   22 ++++++++++++++++++++++
 include/acpi/acpi_bus.h |    6 ++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

Comments

Mika Westerberg Sept. 28, 2012, 2:13 p.m. UTC | #1 
On Fri, Sep 28, 2012 at 03:38:30PM +0800, Zhang Rui wrote:
> >From 72df5d1f51fb27a4ba7f70a3b07df759d32b8288 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Thu, 27 Sep 2012 15:11:55 +0800
> Subject: [RFC PATCH 1/6] Introduce acpi_match_device_id().
> 
> This API is used to check if a device id string is compatible
> with an ACPI device,
> either PNP id exported via _HID or compatible ids exported
> via _CID control method.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/scan.c     |   22 ++++++++++++++++++++++
>  include/acpi/acpi_bus.h |    6 ++++++
>  2 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..936a7c9 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -312,6 +312,28 @@ int acpi_match_device_ids(struct acpi_device *device,
>  }
>  EXPORT_SYMBOL(acpi_match_device_ids);
>  
> +int acpi_match_device_id(const struct device *dev, const char *id)

Would be good idea to implement this in terms of of_match_device() so that
it returns pointer to the matched id. This way drivers can get the
->driver_data pretty easily if needed.

> +{
> +	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);

If the device is not bound to an ACPI handle this will return NULL. And I
don't see you doing that in this series meaning that..

> +	struct acpi_device *device;
> +	struct acpi_hardware_id *hwid;
> +	acpi_status status;
> +
> +	if (!handle || !id)
> +		return -ENODEV;

..you always return -ENODEV here, right?

> +
> +	status = acpi_bus_get_device(handle, &device);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	list_for_each_entry(hwid, &device->pnp.ids, list)
> +		if (!strcmp(id, hwid->id))
> +			return 0;
> +
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL(acpi_match_device_id);
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Rui Sept. 29, 2012, 1:31 p.m. UTC | #2 
> -----Original Message-----
> From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com]
> Sent: Friday, September 28, 2012 10:14 PM
> To: Zhang, Rui
> Cc: LKML; linux-pm; linux-i2c; linux-acpi@vger.kernel.org; Len, Brown;
> Rafael J. Wysocki; Grant Likely; Dirk Brandewie
> Subject: Re: [RFC PATCH 1/6] Introduce acpi_match_device_id().
> Importance: High
> 
> On Fri, Sep 28, 2012 at 03:38:30PM +0800, Zhang Rui wrote:
> > >From 72df5d1f51fb27a4ba7f70a3b07df759d32b8288 Mon Sep 17 00:00:00
> > >2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Thu, 27 Sep 2012 15:11:55 +0800
> > Subject: [RFC PATCH 1/6] Introduce acpi_match_device_id().
> >
> > This API is used to check if a device id string is compatible with an
> > ACPI device, either PNP id exported via _HID or compatible ids
> > exported via _CID control method.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/scan.c     |   22 ++++++++++++++++++++++
> >  include/acpi/acpi_bus.h |    6 ++++++
> >  2 files changed, 28 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index
> > d1ecca2..936a7c9 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -312,6 +312,28 @@ int acpi_match_device_ids(struct acpi_device
> > *device,  }  EXPORT_SYMBOL(acpi_match_device_ids);
> >
> > +int acpi_match_device_id(const struct device *dev, const char *id)
> 
> Would be good idea to implement this in terms of of_match_device() so
> that it returns pointer to the matched id. This way drivers can get the
> ->driver_data pretty easily if needed.
>
Good idea.
Will do that in v2.
 
> > +{
> > +	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> 
> If the device is not bound to an ACPI handle this will return NULL. And
> I don't see you doing that in this series meaning that..
> 


You're right, I should set pdev->archdata.acpi_handle to the I2C controller in i2c_root.c.

Thanks,
rui
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Oct. 1, 2012, 6:37 a.m. UTC | #3 
On Sat, Sep 29, 2012 at 01:31:52PM +0000, Zhang, Rui wrote:
>  
> > > +{
> > > +	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> > 
> > If the device is not bound to an ACPI handle this will return NULL. And
> > I don't see you doing that in this series meaning that..
> > 
> 
> 
> You're right, I should set pdev->archdata.acpi_handle to the I2C
> controller in i2c_root.c.

There already is an API for that - check drivers/acpi/glue.c.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Rui Oct. 1, 2012, 1:56 p.m. UTC | #4 
> -----Original Message-----
> From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com]
> Sent: Monday, October 01, 2012 2:38 PM
> To: Zhang, Rui
> Cc: LKML; linux-pm; linux-i2c; linux-acpi@vger.kernel.org; Len, Brown;
> Rafael J. Wysocki; Grant Likely; Dirk Brandewie
> Subject: Re: [RFC PATCH 1/6] Introduce acpi_match_device_id().
> Importance: High
> 
> On Sat, Sep 29, 2012 at 01:31:52PM +0000, Zhang, Rui wrote:
> >
> > > > +{
> > > > +	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> > >
> > > If the device is not bound to an ACPI handle this will return NULL.
> > > And I don't see you doing that in this series meaning that..
> > >
> >
> >
> > You're right, I should set pdev->archdata.acpi_handle to the I2C
> > controller in i2c_root.c.
> 
> There already is an API for that - check drivers/acpi/glue.c.

Do you mean acpi_bind?
acpi_bind_one will bind the physical device node to the ACPI device,
But for the i2c controller ACPI device, the physical node is the I2C
adapter in the I2C bus.
If we introduce I2C bus ACPI binding, which is what we're doing now,
the i2c_adapter->dev.archdata.acpi_handle will be set to the ACPI
i2c controller handle by acpi_bind_one.

Thanks,
rui
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Oct. 2, 2012, 6:10 a.m. UTC | #5 
On Mon, Oct 01, 2012 at 01:56:17PM +0000, Zhang, Rui wrote:
> 
> 
> > -----Original Message-----
> > From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com]
> > Sent: Monday, October 01, 2012 2:38 PM
> > To: Zhang, Rui
> > Cc: LKML; linux-pm; linux-i2c; linux-acpi@vger.kernel.org; Len, Brown;
> > Rafael J. Wysocki; Grant Likely; Dirk Brandewie
> > Subject: Re: [RFC PATCH 1/6] Introduce acpi_match_device_id().
> > Importance: High
> > 
> > On Sat, Sep 29, 2012 at 01:31:52PM +0000, Zhang, Rui wrote:
> > >
> > > > > +{
> > > > > +	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> > > >
> > > > If the device is not bound to an ACPI handle this will return NULL.
> > > > And I don't see you doing that in this series meaning that..
> > > >
> > >
> > >
> > > You're right, I should set pdev->archdata.acpi_handle to the I2C
> > > controller in i2c_root.c.
> > 
> > There already is an API for that - check drivers/acpi/glue.c.
> 
> Do you mean acpi_bind?

No, I mean register_acpi_bus_type(). You can see an example of this looking
PCI, USB, .., drivers.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..936a7c9 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -312,6 +312,28 @@  int acpi_match_device_ids(struct acpi_device *device,
 }
 EXPORT_SYMBOL(acpi_match_device_ids);
 
+int acpi_match_device_id(const struct device *dev, const char *id)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+	struct acpi_device *device;
+	struct acpi_hardware_id *hwid;
+	acpi_status status;
+
+	if (!handle || !id)
+		return -ENODEV;
+
+	status = acpi_bus_get_device(handle, &device);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	list_for_each_entry(hwid, &device->pnp.ids, list)
+		if (!strcmp(id, hwid->id))
+			return 0;
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL(acpi_match_device_id);
+
 static void acpi_free_ids(struct acpi_device *device)
 {
 	struct acpi_hardware_id *id, *tmp;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bde976e..8b5b124 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -378,6 +378,7 @@  int acpi_bus_start(struct acpi_device *device);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
 int acpi_match_device_ids(struct acpi_device *device,
 			  const struct acpi_device_id *ids);
+int acpi_match_device_id(const struct device *, const char *);
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
@@ -448,6 +449,11 @@  static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int acpi_match_device_id(const struct device *device,
+					const char *name)
+{
+	return -ENODEV;
+}
 
 #endif				/* CONFIG_ACPI */