[alsa-devel] [PATCH v2 0/3] ACPI, ASoC, gpio: Introduce and use acpi_dev_get_dev_name()
It appears that at least one current user (patch 2) and upcoming one (patch 3) need to get device name by ACPI HID.
Here we introduce acpi_dev_get_dev_name() based on code done for acpi_dev_present() and reuse it where appropriate.
The series has been tested on Intel Edison with ACPI enabled U-Boot, while patch 2 wasn't tested anyhow (Pierre, can you help with it?).
Since patch 1 and cross subsystem nature of the series I think the best way is to push this via Rafael's linux-pm tree.
In worst case, if, Rafael, you have no objection, push at least first patch for this cycle to allow utilization in the future.
Andy Shevchenko (3): ACPI / utils: Introduce acpi_dev_get_dev_name() ASoC: Intel - Convert users to use acpi_dev_get_dev_name() gpio: merrifield: Add support of ACPI enabled platforms
drivers/acpi/utils.c | 40 +++++++++++++++++++++++++++------ drivers/gpio/gpio-merrifield.c | 11 ++++++++- include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 6 +++++ include/sound/soc-acpi.h | 7 ------ sound/soc/intel/boards/bytcht_da7213.c | 2 +- sound/soc/intel/boards/bytcr_rt5640.c | 2 +- sound/soc/intel/boards/bytcr_rt5651.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5645.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +- sound/soc/soc-acpi.c | 33 --------------------------- 11 files changed, 55 insertions(+), 53 deletions(-)
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/acpi/utils.c | 40 +++++++++++++++++++++++++++++++++------- include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 6 ++++++ 3 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 9d49a1acebe3..1da9e986d510 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -737,16 +737,17 @@ bool acpi_dev_found(const char *hid) } EXPORT_SYMBOL(acpi_dev_found);
-struct acpi_dev_present_info { +struct acpi_dev_match_info { + const char *dev_name; struct acpi_device_id hid[2]; const char *uid; s64 hrv; };
-static int acpi_dev_present_cb(struct device *dev, void *data) +static int acpi_dev_match_cb(struct device *dev, void *data) { struct acpi_device *adev = to_acpi_device(dev); - struct acpi_dev_present_info *match = data; + struct acpi_dev_match_info *match = data; unsigned long long hrv; acpi_status status;
@@ -757,6 +758,8 @@ static int acpi_dev_present_cb(struct device *dev, void *data) strcmp(adev->pnp.unique_id, match->uid))) return 0;
+ match->dev_name = acpi_dev_name(adev); + if (match->hrv == -1) return 1;
@@ -789,20 +792,43 @@ static int acpi_dev_present_cb(struct device *dev, void *data) */ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) { - struct acpi_dev_present_info match = {}; + struct acpi_dev_match_info match = {}; struct device *dev;
strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); match.uid = uid; match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match, - acpi_dev_present_cb); - + dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb); return !!dev; } EXPORT_SYMBOL(acpi_dev_present);
+/** + * acpi_dev_get_dev_name - Return device name of first match of ACPI device + * @hid: Hardware ID of the device. + * @uid: Unique ID of the device, pass NULL to not check _UID + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV + * + * Return device name if a matching device was present + * at the moment of invocation, or NULL otherwise. + * + * See additional information in acpi_dev_present() as well. + */ +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{ + struct acpi_dev_match_info match = {}; + struct device *dev; + + strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); + match.uid = uid; + match.hrv = hrv; + + dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb); + return dev ? match.dev_name : NULL; +} +EXPORT_SYMBOL(acpi_dev_get_dev_name); + /* * acpi_backlight= handling, this is done here rather then in video_detect.c * because __setup cannot be used in modules. diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 79287629c888..8883f5ebb6ce 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -90,6 +90,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
bool acpi_dev_found(const char *hid); bool acpi_dev_present(const char *hid, const char *uid, s64 hrv); +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv);
#ifdef CONFIG_ACPI
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1922063f6894..d6576eec45d1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -644,6 +644,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) return false; }
+static inline +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{ + return NULL; +} + static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return false;
On 1/4/18 10:47 AM, Andy Shevchenko wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
I would have done this differently. You have two routines (acpi_dev_present and acpi_dev_get_dev_name) which do the same thing except for what they return (bool and const char *) respectively.
This could be factored with
static struct device *dev _acpi_dev_present(const char *hid, const char *uid, s64 hrv) {
struct acpi_dev_present_info match = {}; struct device *dev;
strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); match.uid = uid; match.hrv = hrv;
dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_present_cb); return dev; }
bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) { return !!_acpi_dev_present(const char *hid, const char *uid, s64 hrv); }
const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) { struct device *dev;
dev = _acpi_dev_present(const char *hid, const char *uid, s64 hrv);
return dev ? match.dev_name : NULL; }
That said you are a much better programmer than me so the other code is fine with me...
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
drivers/acpi/utils.c | 40 +++++++++++++++++++++++++++++++++------- include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 6 ++++++ 3 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 9d49a1acebe3..1da9e986d510 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -737,16 +737,17 @@ bool acpi_dev_found(const char *hid) } EXPORT_SYMBOL(acpi_dev_found);
-struct acpi_dev_present_info { +struct acpi_dev_match_info {
- const char *dev_name; struct acpi_device_id hid[2]; const char *uid; s64 hrv; };
-static int acpi_dev_present_cb(struct device *dev, void *data) +static int acpi_dev_match_cb(struct device *dev, void *data) { struct acpi_device *adev = to_acpi_device(dev);
- struct acpi_dev_present_info *match = data;
- struct acpi_dev_match_info *match = data; unsigned long long hrv; acpi_status status;
@@ -757,6 +758,8 @@ static int acpi_dev_present_cb(struct device *dev, void *data) strcmp(adev->pnp.unique_id, match->uid))) return 0;
- match->dev_name = acpi_dev_name(adev);
- if (match->hrv == -1) return 1;
@@ -789,20 +792,43 @@ static int acpi_dev_present_cb(struct device *dev, void *data) */ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) {
- struct acpi_dev_present_info match = {};
struct acpi_dev_match_info match = {}; struct device *dev;
strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); match.uid = uid; match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match,
acpi_dev_present_cb);
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb); return !!dev; } EXPORT_SYMBOL(acpi_dev_present);
+/**
- acpi_dev_get_dev_name - Return device name of first match of ACPI device
- @hid: Hardware ID of the device.
- @uid: Unique ID of the device, pass NULL to not check _UID
- @hrv: Hardware Revision of the device, pass -1 to not check _HRV
- Return device name if a matching device was present
- at the moment of invocation, or NULL otherwise.
- See additional information in acpi_dev_present() as well.
- */
+const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{
- struct acpi_dev_match_info match = {};
- struct device *dev;
- strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id));
- match.uid = uid;
- match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
- return dev ? match.dev_name : NULL;
+} +EXPORT_SYMBOL(acpi_dev_get_dev_name);
- /*
- acpi_backlight= handling, this is done here rather then in video_detect.c
- because __setup cannot be used in modules.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 79287629c888..8883f5ebb6ce 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -90,6 +90,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
bool acpi_dev_found(const char *hid); bool acpi_dev_present(const char *hid, const char *uid, s64 hrv); +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv);
#ifdef CONFIG_ACPI
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1922063f6894..d6576eec45d1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -644,6 +644,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) return false; }
+static inline +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{
- return NULL;
+}
- static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return false;
On Thu, 2018-01-04 at 11:31 -0600, Pierre-Louis Bossart wrote:
On 1/4/18 10:47 AM, Andy Shevchenko wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
I would have done this differently. You have two routines (acpi_dev_present and acpi_dev_get_dev_name) which do the same thing except for what they return (bool and const char *) respectively.
I have thought about it :-)
I decide that for two current users there is a little benefit over effort and readability.
See also below.
This could be factored with
static struct device *dev _acpi_dev_present(const char *hid, const char *uid, s64 hrv) {
struct acpi_dev_present_info match = {}; struct device *dev;
strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); match.uid = uid; match.hrv = hrv;
dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_present_cb);
return dev; }
bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) { return !!_acpi_dev_present(const char *hid, const char *uid, s64 hrv); }
const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) { struct device *dev;
dev = _acpi_dev_present(const char *hid, const char *uid, s64 hrv);
return dev ? match.dev_name : NULL;
And where is the match declaration?
Yes, needs to be duplicated. That's why I stopped going this direction.
It still might make sense when 3+ variant will appear.
}
That said you are a much better programmer than me so the other code is fine with me...
Thanks.
Would be nice to get your Tested-by at some point.
On 01/04/2018 10:47 AM, Andy Shevchenko wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
This works fine on a Dell 5585 where the default codec dai name needs to be updated based on the actual HID information, so Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
the next patch does not apply directly however, and needs additional changes for the ES8316 machine driver. see the changes here: https://github.com/plbossart/sound/tree/topic/bytcht-acpi-fixes
It's probably best to let this patch go through the acpi tree, and the next one through Mark's tree once the first is merged and all the other Kconfig/acpi stuff is also in -next?
drivers/acpi/utils.c | 40 +++++++++++++++++++++++++++++++++------- include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 6 ++++++ 3 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 9d49a1acebe3..1da9e986d510 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -737,16 +737,17 @@ bool acpi_dev_found(const char *hid) } EXPORT_SYMBOL(acpi_dev_found);
-struct acpi_dev_present_info { +struct acpi_dev_match_info {
- const char *dev_name; struct acpi_device_id hid[2]; const char *uid; s64 hrv; };
-static int acpi_dev_present_cb(struct device *dev, void *data) +static int acpi_dev_match_cb(struct device *dev, void *data) { struct acpi_device *adev = to_acpi_device(dev);
- struct acpi_dev_present_info *match = data;
- struct acpi_dev_match_info *match = data; unsigned long long hrv; acpi_status status;
@@ -757,6 +758,8 @@ static int acpi_dev_present_cb(struct device *dev, void *data) strcmp(adev->pnp.unique_id, match->uid))) return 0;
- match->dev_name = acpi_dev_name(adev);
- if (match->hrv == -1) return 1;
@@ -789,20 +792,43 @@ static int acpi_dev_present_cb(struct device *dev, void *data) */ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) {
- struct acpi_dev_present_info match = {};
struct acpi_dev_match_info match = {}; struct device *dev;
strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); match.uid = uid; match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match,
acpi_dev_present_cb);
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb); return !!dev; } EXPORT_SYMBOL(acpi_dev_present);
+/**
- acpi_dev_get_dev_name - Return device name of first match of ACPI device
- @hid: Hardware ID of the device.
- @uid: Unique ID of the device, pass NULL to not check _UID
- @hrv: Hardware Revision of the device, pass -1 to not check _HRV
- Return device name if a matching device was present
- at the moment of invocation, or NULL otherwise.
- See additional information in acpi_dev_present() as well.
- */
+const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{
- struct acpi_dev_match_info match = {};
- struct device *dev;
- strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id));
- match.uid = uid;
- match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
- return dev ? match.dev_name : NULL;
+} +EXPORT_SYMBOL(acpi_dev_get_dev_name);
- /*
- acpi_backlight= handling, this is done here rather then in video_detect.c
- because __setup cannot be used in modules.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 79287629c888..8883f5ebb6ce 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -90,6 +90,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
bool acpi_dev_found(const char *hid); bool acpi_dev_present(const char *hid, const char *uid, s64 hrv); +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv);
#ifdef CONFIG_ACPI
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1922063f6894..d6576eec45d1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -644,6 +644,12 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) return false; }
+static inline +const char *acpi_dev_get_dev_name(const char *hid, const char *uid, s64 hrv) +{
- return NULL;
+}
- static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return false;
On Thu, Jan 04, 2018 at 06:47:56PM -0600, Pierre-Louis Bossart wrote:
one through Mark's tree once the first is merged and all the other Kconfig/acpi stuff is also in -next?
If we could get this in during the merge window that'd be ideal...
On Thu, 2018-01-04 at 18:47 -0600, Pierre-Louis Bossart wrote:
On 01/04/2018 10:47 AM, Andy Shevchenko wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
This works fine on a Dell 5585 where the default codec dai name needs to be updated based on the actual HID information, so Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Thanks!
Is this only for patch 1, or for both 1 and 2 ?
the next patch does not apply directly however, and needs additional changes for the ES8316 machine driver. see the changes here: https://github.com/plbossart/sound/tree/topic/bytcht-acpi-fixes
You mean because of this https://github.com/plbossart/sound/commit/2416827fa6d221b27edd6397f17daa a1f3cd5fb0 ?
I.e. I need to rebase on top of your series.
It's probably best to let this patch go through the acpi tree, and the next one through Mark's tree once the first is merged and all the other Kconfig/acpi stuff is also in -next?
It wouldn't prevent to push patch 3 as well via linux-pm at the same time as patch 1.
Linus, do you have any objection on that?
On 1/5/18 6:43 AM, Andy Shevchenko wrote:
On Thu, 2018-01-04 at 18:47 -0600, Pierre-Louis Bossart wrote:
On 01/04/2018 10:47 AM, Andy Shevchenko wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not. To make life easier for such users introduce acpi_dev_get_dev_name() helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()in the utils.c file.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
This works fine on a Dell 5585 where the default codec dai name needs to be updated based on the actual HID information, so Tested-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
Thanks!
Is this only for patch 1, or for both 1 and 2 ?
the next patch does not apply directly however, and needs additional changes for the ES8316 machine driver. see the changes here: https://github.com/plbossart/sound/tree/topic/bytcht-acpi-fixes
You mean because of this https://github.com/plbossart/sound/commit/2416827fa6d221b27edd6397f17daa a1f3cd5fb0 ?
No, not only, there is also a conflict in soc-acpi.c
I.e. I need to rebase on top of your series.
The code I use contains all the Kconfig cleanups (v3 sent yesterday), so there is another dependency here.
It's probably best to let this patch go through the acpi tree, and the next one through Mark's tree once the first is merged and all the other Kconfig/acpi stuff is also in -next?
It wouldn't prevent to push patch 3 as well via linux-pm at the same time as patch 1.
Linus, do you have any objection on that?
On Thu, Jan 4, 2018 at 5:47 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not.
I would give an example here.
To make life easier for such users introduce acpi_dev_get_dev_name()
What about acpi_dev_get_match_name() or just acpi_dev_match_name()?
helper based on code for acpi_dev_present().
To be more consistent with the purpose rename
struct acpi_dev_present_info -> struct acpi_dev_match_info acpi_dev_present_cb() -> acpi_dev_match_cb()
in the utils.c file.
OK
Thanks, Rafael
On Fri, 2018-01-05 at 13:06 +0100, Rafael J. Wysocki wrote:
On Thu, Jan 4, 2018 at 5:47 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not.
I would give an example here.
You mean to mention what is in patch 2? I can do that.
"Subset of Intel ASoC drivers is an existing user of this API when they need to find an actual instance of the codec device based on its ACPI HID."
To make life easier for such users introduce acpi_dev_get_dev_name()
What about acpi_dev_get_match_name() or just acpi_dev_match_name()?
It's "get", not "match".
So, acpi_dev_get_name() then?
On Fri, Jan 5, 2018 at 1:22 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, 2018-01-05 at 13:06 +0100, Rafael J. Wysocki wrote:
On Thu, Jan 4, 2018 at 5:47 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
Sometimes the user want to have device name of the match rather than just checking if device present or not.
I would give an example here.
You mean to mention what is in patch 2? I can do that.
"Subset of Intel ASoC drivers is an existing user of this API when they need to find an actual instance of the codec device based on its ACPI HID."
And why do they need the name (and not something else)?
To make life easier for such users introduce acpi_dev_get_dev_name()
What about acpi_dev_get_match_name() or just acpi_dev_match_name()?
It's "get", not "match".
OK
So, acpi_dev_get_name() then?
It would be somewhat clearer to call it acpi_dev_get_first_match_name() IMO.
Otherwise it may not be clear what name this is going to return.
On Fri, 2018-01-05 at 13:34 +0100, Rafael J. Wysocki wrote:
On Fri, Jan 5, 2018 at 1:22 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, 2018-01-05 at 13:06 +0100, Rafael J. Wysocki wrote:
On Thu, Jan 4, 2018 at 5:47 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
"Subset of Intel ASoC drivers is an existing user of this API when they need to find an actual instance of the codec device based on its ACPI HID."
And why do they need the name (and not something else)?
I don't know the guts of Intel ASoC implementation, so, can't answer this without investigation.
Pierre, can you elaborate this piece?
So, acpi_dev_get_name() then?
It would be somewhat clearer to call it acpi_dev_get_first_match_name() IMO.
Otherwise it may not be clear what name this is going to return.
Works for me.
On 1/5/18 6:39 AM, Andy Shevchenko wrote:
On Fri, 2018-01-05 at 13:34 +0100, Rafael J. Wysocki wrote:
On Fri, Jan 5, 2018 at 1:22 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Fri, 2018-01-05 at 13:06 +0100, Rafael J. Wysocki wrote:
On Thu, Jan 4, 2018 at 5:47 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
"Subset of Intel ASoC drivers is an existing user of this API when they need to find an actual instance of the codec device based on its ACPI HID."
And why do they need the name (and not something else)?
I don't know the guts of Intel ASoC implementation, so, can't answer this without investigation.
Pierre, can you elaborate this piece?
Because the ASoC DAI structures use strings to represent the codec name. See e.g.
http://elixir.free-electrons.com/linux/latest/source/sound/soc/intel/boards/... http://elixir.free-electrons.com/linux/latest/source/sound/soc/intel/boards/...
In most cases a hard-coded name is fine, except when the BIOS uses a different HID or the ACPI subsystem changes the :00 ending to :01
So what the code does is that you look for a set of known HIDs, create the machine driver and pass the HID as pdata, then in the machine driver once you query the string that lets ASoC do the rest of the card creation.
You can argue that ASoC should not use strings, but that's not really a debate at this point: all I am trying to do is to make devices work when their BIOS is slightly different from the norm.
So, acpi_dev_get_name() then?
It would be somewhat clearer to call it acpi_dev_get_first_match_name() IMO.
Otherwise it may not be clear what name this is going to return.
Works for me.
Instead of home grown snd_soc_acpi_find_name_from_hid() use acpi_dev_get_dev_name().
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- include/sound/soc-acpi.h | 7 ------- sound/soc/intel/boards/bytcht_da7213.c | 2 +- sound/soc/intel/boards/bytcr_rt5640.c | 2 +- sound/soc/intel/boards/bytcr_rt5651.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5645.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +- sound/soc/soc-acpi.c | 33 --------------------------------- 7 files changed, 5 insertions(+), 45 deletions(-)
diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h index a7d8d335b043..a71a935b5bc8 100644 --- a/include/sound/soc-acpi.h +++ b/include/sound/soc-acpi.h @@ -27,16 +27,9 @@ struct snd_soc_acpi_package_context { };
#if IS_ENABLED(CONFIG_ACPI) -/* translation fron HID to I2C name, needed for DAI codec_name */ -const char *snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN]); bool snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN], struct snd_soc_acpi_package_context *ctx); #else -static inline const char * -snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN]) -{ - return NULL; -} static inline bool snd_soc_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN], struct snd_soc_acpi_package_context *ctx) diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c index c4d82ad41bd7..828f839d0701 100644 --- a/sound/soc/intel/boards/bytcht_da7213.c +++ b/sound/soc/intel/boards/bytcht_da7213.c @@ -243,7 +243,7 @@ static int bytcht_da7213_probe(struct platform_device *pdev) }
/* fixup codec name based on HID */ - i2c_name = snd_soc_acpi_find_name_from_hid(mach->id); + i2c_name = acpi_dev_get_dev_name(mach->id, NULL, -1); if (i2c_name) { snprintf(codec_name, sizeof(codec_name), "%s%s", "i2c-", i2c_name); diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index f2c0fc415e52..849344baf233 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -762,7 +762,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) }
/* fixup codec name based on HID */ - i2c_name = snd_soc_acpi_find_name_from_hid(mach->id); + i2c_name = acpi_dev_get_dev_name(mach->id, NULL, -1); if (i2c_name) { snprintf(byt_rt5640_codec_name, sizeof(byt_rt5640_codec_name), "%s%s", "i2c-", i2c_name); diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 488ec48f296a..87bab84c13a7 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -511,7 +511,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) }
/* fixup codec name based on HID */ - i2c_name = snd_soc_acpi_find_name_from_hid(mach->id); + i2c_name = acpi_dev_get_dev_name(mach->id, NULL, -1); if (i2c_name) { snprintf(byt_rt5651_codec_name, sizeof(byt_rt5651_codec_name), "%s%s", "i2c-", i2c_name); diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index f898ee140cdc..9443b4d4f8b0 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -573,7 +573,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) }
/* fixup codec name based on HID */ - i2c_name = snd_soc_acpi_find_name_from_hid(mach->id); + i2c_name = acpi_dev_get_dev_name(mach->id, NULL, -1); if (i2c_name) { snprintf(cht_rt5645_codec_name, sizeof(cht_rt5645_codec_name), "%s%s", "i2c-", i2c_name); diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index f8f21eee9b2d..029670048c17 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -396,7 +396,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
/* fixup codec name based on HID */ if (mach) { - i2c_name = snd_soc_acpi_find_name_from_hid(mach->id); + i2c_name = acpi_dev_get_dev_name(mach->id, NULL, -1); if (i2c_name) { snprintf(drv->codec_name, sizeof(drv->codec_name), "i2c-%s", i2c_name); diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c index f21df28bc28e..ed2e7f1a77ea 100644 --- a/sound/soc/soc-acpi.c +++ b/sound/soc/soc-acpi.c @@ -16,39 +16,6 @@
#include <sound/soc-acpi.h>
-static acpi_status snd_soc_acpi_find_name(acpi_handle handle, u32 level, - void *context, void **ret) -{ - struct acpi_device *adev; - const char *name = NULL; - - if (acpi_bus_get_device(handle, &adev)) - return AE_OK; - - if (adev->status.present && adev->status.functional) { - name = acpi_dev_name(adev); - *(const char **)ret = name; - return AE_CTRL_TERMINATE; - } - - return AE_OK; -} - -const char *snd_soc_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN]) -{ - const char *name = NULL; - acpi_status status; - - status = acpi_get_devices(hid, snd_soc_acpi_find_name, NULL, - (void **)&name); - - if (ACPI_FAILURE(status) || name[0] == '\0') - return NULL; - - return name; -} -EXPORT_SYMBOL_GPL(snd_soc_acpi_find_name_from_hid); - static acpi_status snd_soc_acpi_mach_match(acpi_handle handle, u32 level, void *context, void **ret) {
The driver needs the pin control device name for ACPI.
We are looking through ACPI namespace and return first found device based on ACPI HID for Intel Merrifield FLIS.
Cc: Mika Westerberg mika.westerberg@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/gpio/gpio-merrifield.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c index dd67a31ac337..68320d57d5f0 100644 --- a/drivers/gpio/gpio-merrifield.c +++ b/drivers/gpio/gpio-merrifield.c @@ -9,6 +9,7 @@ * published by the Free Software Foundation. */
+#include <linux/acpi.h> #include <linux/bitops.h> #include <linux/gpio/driver.h> #include <linux/init.h> @@ -380,9 +381,16 @@ static void mrfld_irq_init_hw(struct mrfld_gpio *priv) } }
+static const char *mrfld_gpio_get_pinctrl_dev_name(const char *fallback) +{ + const char *dev_name = acpi_dev_get_dev_name("INTC1002", NULL, -1); + return dev_name ? dev_name : fallback; +} + static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id) { const struct mrfld_gpio_pinrange *range; + const char *pinctrl_dev_name; struct mrfld_gpio *priv; u32 gpio_base, irq_base; void __iomem *base; @@ -439,10 +447,11 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id return retval; }
+ pinctrl_dev_name = mrfld_gpio_get_pinctrl_dev_name("pinctrl-merrifield"); for (i = 0; i < ARRAY_SIZE(mrfld_gpio_ranges); i++) { range = &mrfld_gpio_ranges[i]; retval = gpiochip_add_pin_range(&priv->chip, - "pinctrl-merrifield", + pinctrl_dev_name, range->gpio_base, range->pin_base, range->npins);
On 1/4/18 10:47 AM, Andy Shevchenko wrote:
It appears that at least one current user (patch 2) and upcoming one (patch 3) need to get device name by ACPI HID.
Here we introduce acpi_dev_get_dev_name() based on code done for acpi_dev_present() and reuse it where appropriate.
The series has been tested on Intel Edison with ACPI enabled U-Boot, while patch 2 wasn't tested anyhow (Pierre, can you help with it?).
Yes I'll try to test later this week. I just have to figure out which of my devices needs this (been a while since I worked on this).
Since patch 1 and cross subsystem nature of the series I think the best way is to push this via Rafael's linux-pm tree.
In worst case, if, Rafael, you have no objection, push at least first patch for this cycle to allow utilization in the future.
Andy Shevchenko (3): ACPI / utils: Introduce acpi_dev_get_dev_name() ASoC: Intel - Convert users to use acpi_dev_get_dev_name() gpio: merrifield: Add support of ACPI enabled platforms
drivers/acpi/utils.c | 40 +++++++++++++++++++++++++++------ drivers/gpio/gpio-merrifield.c | 11 ++++++++- include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 6 +++++ include/sound/soc-acpi.h | 7 ------ sound/soc/intel/boards/bytcht_da7213.c | 2 +- sound/soc/intel/boards/bytcr_rt5640.c | 2 +- sound/soc/intel/boards/bytcr_rt5651.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5645.c | 2 +- sound/soc/intel/boards/cht_bsw_rt5672.c | 2 +- sound/soc/soc-acpi.c | 33 --------------------------- 11 files changed, 55 insertions(+), 53 deletions(-)
participants (4)
-
Andy Shevchenko -
Mark Brown -
Pierre-Louis Bossart -
Rafael J. Wysocki