[PATCH 0/3] ACPI/ALSA/soundwire: add acpi_get_local_u64_address() helper
The acpi_get_local_address() helper assumes a 32-bit ADR is used. If we want to use this helper for SoundWire/SDCA ASoC codecs, we need an extension where the native 64-bits are used. This patchset suggests a new helper, acpi_get_local_address() may be renamed if desired in a folow-up patch.
The path of least resistance would be to merge this patchset in the ASoC tree, since I have additional changes for ASoC/SDCA (SoundWire Device Class) that depend on the new helper.
Pierre-Louis Bossart (3): ACPI: utils: introduce acpi_get_local_u64_address() soundwire: slave: simplify code with acpi_get_local_u64_address() ALSA: hda: intel-sdw-acpi: use acpi_get_local_u64_address()
drivers/acpi/utils.c | 22 ++++++++++++++++------ drivers/soundwire/slave.c | 13 ++++--------- include/linux/acpi.h | 1 + sound/hda/intel-sdw-acpi.c | 6 +++--- 4 files changed, 24 insertions(+), 18 deletions(-)
The ACPI _ADR is a 64-bit value. We changed the definitions in commit ca6f998cf9a2 ("ACPI: bus: change _ADR representation to 64 bits") but some helpers still assume the value is a 32-bit value.
This patch adds a new helper to extract the full 64-bits. The existing 32-bit helper is kept for backwards-compatibility and cases where the _ADR is known to fit in a 32-bit value.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/acpi/utils.c | 22 ++++++++++++++++------ include/linux/acpi.h | 1 + 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 202234ba54bd..ae9384282273 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -277,15 +277,25 @@ acpi_evaluate_integer(acpi_handle handle,
EXPORT_SYMBOL(acpi_evaluate_integer);
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr) +{ + acpi_status status; + + status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, addr); + if (ACPI_FAILURE(status)) + return -ENODATA; + return 0; +} +EXPORT_SYMBOL(acpi_get_local_u64_address); + int acpi_get_local_address(acpi_handle handle, u32 *addr) { - unsigned long long adr; - acpi_status status; - - status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr); - if (ACPI_FAILURE(status)) - return -ENODATA; + u64 adr; + int ret;
+ ret = acpi_get_local_u64_address(handle, &adr); + if (ret < 0) + return ret; *addr = (u32)adr; return 0; } diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 28c3fb2bef0d..65e7177bcb02 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -761,6 +761,7 @@ static inline u64 acpi_arch_get_root_pointer(void) } #endif
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr); int acpi_get_local_address(acpi_handle handle, u32 *addr); const char *acpi_get_subsystem_id(acpi_handle handle);
On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
The ACPI _ADR is a 64-bit value. We changed the definitions in commit ca6f998cf9a2 ("ACPI: bus: change _ADR representation to 64 bits") but some helpers still assume the value is a 32-bit value.
This patch adds a new helper to extract the full 64-bits. The existing 32-bit helper is kept for backwards-compatibility and cases where the _ADR is known to fit in a 32-bit value.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com
Do you want me to apply this or do you want me to route it along with the rest of the series?
In the latter case feel free to add
Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
to it.
Thanks!
drivers/acpi/utils.c | 22 ++++++++++++++++------ include/linux/acpi.h | 1 + 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 202234ba54bd..ae9384282273 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -277,15 +277,25 @@ acpi_evaluate_integer(acpi_handle handle,
EXPORT_SYMBOL(acpi_evaluate_integer);
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr) +{
acpi_status status;status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, addr);if (ACPI_FAILURE(status))return -ENODATA;return 0;+} +EXPORT_SYMBOL(acpi_get_local_u64_address);
I'd prefer EXPORT_SYMBOL_GPL() here unless you absolutely cannot live with it.
int acpi_get_local_address(acpi_handle handle, u32 *addr) {
unsigned long long adr;acpi_status status;status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr);if (ACPI_FAILURE(status))return -ENODATA;
u64 adr;int ret;ret = acpi_get_local_u64_address(handle, &adr);if (ret < 0)return ret; *addr = (u32)adr; return 0;} diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 28c3fb2bef0d..65e7177bcb02 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -761,6 +761,7 @@ static inline u64 acpi_arch_get_root_pointer(void) } #endif
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr); int acpi_get_local_address(acpi_handle handle, u32 *addr); const char *acpi_get_subsystem_id(acpi_handle handle);
--
On 6/7/24 20:51, Rafael J. Wysocki wrote:
On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
The ACPI _ADR is a 64-bit value. We changed the definitions in commit ca6f998cf9a2 ("ACPI: bus: change _ADR representation to 64 bits") but some helpers still assume the value is a 32-bit value.
This patch adds a new helper to extract the full 64-bits. The existing 32-bit helper is kept for backwards-compatibility and cases where the _ADR is known to fit in a 32-bit value.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com
Do you want me to apply this or do you want me to route it along with the rest of the series?
In the latter case feel free to add
Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
Thanks Rafael. I think it's easier if Mark Brown takes the series in ASoC, I have additional ASoC patches that use the u64 helper.
Mark?
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr) +{
acpi_status status;status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, addr);if (ACPI_FAILURE(status))return -ENODATA;return 0;+} +EXPORT_SYMBOL(acpi_get_local_u64_address);
I'd prefer EXPORT_SYMBOL_GPL() here unless you absolutely cannot live with it.
I don't mind, but the existing helper was using EXPORT_SYMBOL so I just copied. It'd be odd to have two helpers that only differ by the argument size use a different EXPORT_ macro, no? Not to mention that the get_local address uses EXPORT_SYMBOL but would become a wrapper for an EXPORT_SYMBOL_GPL. That gives me a headache...
This was the original code:
int acpi_get_local_address(acpi_handle handle, u32 *addr) { unsigned long long adr; acpi_status status;
status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr); if (ACPI_FAILURE(status)) return -ENODATA;
*addr = (u32)adr; return 0; } EXPORT_SYMBOL(acpi_get_local_address);
On Fri, Jun 07, 2024 at 10:33:00PM +0200, Pierre-Louis Bossart wrote:
On 6/7/24 20:51, Rafael J. Wysocki wrote:
Do you want me to apply this or do you want me to route it along with the rest of the series?
In the latter case feel free to add
Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
Thanks Rafael. I think it's easier if Mark Brown takes the series in ASoC, I have additional ASoC patches that use the u64 helper.
Mark?
Sure, no problem taking it via ASoC.
On Fri, Jun 7, 2024 at 10:33 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 6/7/24 20:51, Rafael J. Wysocki wrote:
On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
The ACPI _ADR is a 64-bit value. We changed the definitions in commit ca6f998cf9a2 ("ACPI: bus: change _ADR representation to 64 bits") but some helpers still assume the value is a 32-bit value.
This patch adds a new helper to extract the full 64-bits. The existing 32-bit helper is kept for backwards-compatibility and cases where the _ADR is known to fit in a 32-bit value.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com
Do you want me to apply this or do you want me to route it along with the rest of the series?
In the latter case feel free to add
Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com
Thanks Rafael. I think it's easier if Mark Brown takes the series in ASoC, I have additional ASoC patches that use the u64 helper.
Mark?
+int acpi_get_local_u64_address(acpi_handle handle, u64 *addr) +{
acpi_status status;status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, addr);if (ACPI_FAILURE(status))return -ENODATA;return 0;+} +EXPORT_SYMBOL(acpi_get_local_u64_address);
I'd prefer EXPORT_SYMBOL_GPL() here unless you absolutely cannot live with it.
I don't mind, but the existing helper was using EXPORT_SYMBOL so I just copied. It'd be odd to have two helpers that only differ by the argument size use a different EXPORT_ macro, no? Not to mention that the get_local address uses EXPORT_SYMBOL but would become a wrapper for an EXPORT_SYMBOL_GPL. That gives me a headache...
OK, fair enough.
Now we have a helper so there's no need to open-code.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com --- drivers/soundwire/slave.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c index 9963b92eb505..f1a4df6cfebd 100644 --- a/drivers/soundwire/slave.c +++ b/drivers/soundwire/slave.c @@ -97,18 +97,13 @@ static bool find_slave(struct sdw_bus *bus, struct acpi_device *adev, struct sdw_slave_id *id) { - u64 addr; unsigned int link_id; - acpi_status status; + u64 addr; + int ret;
- status = acpi_evaluate_integer(adev->handle, - METHOD_NAME__ADR, NULL, &addr); - - if (ACPI_FAILURE(status)) { - dev_err(bus->dev, "_ADR resolution failed: %x\n", - status); + ret = acpi_get_local_u64_address(adev->handle, &addr); + if (ret < 0) return false; - }
if (bus->ops->override_adr) addr = bus->ops->override_adr(bus, addr);
On 28-05-24, 14:29, Pierre-Louis Bossart wrote:
Now we have a helper so there's no need to open-code.
Acked-by: Vinod Koul vkoul@kernel.org
Now we have a helper so there's no need to open-code.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com --- sound/hda/intel-sdw-acpi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/hda/intel-sdw-acpi.c b/sound/hda/intel-sdw-acpi.c index d7417a40392b..f3b2a610df23 100644 --- a/sound/hda/intel-sdw-acpi.c +++ b/sound/hda/intel-sdw-acpi.c @@ -125,11 +125,11 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level, void *cdata, void **return_value) { struct sdw_intel_acpi_info *info = cdata; - acpi_status status; u64 adr; + int ret;
- status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr); - if (ACPI_FAILURE(status)) + ret = acpi_get_local_u64_address(handle, &adr); + if (ret < 0) return AE_OK; /* keep going */
if (!acpi_fetch_acpi_dev(handle)) {
On Tue, 28 May 2024 21:29:32 +0200, Pierre-Louis Bossart wrote:
The acpi_get_local_address() helper assumes a 32-bit ADR is used. If we want to use this helper for SoundWire/SDCA ASoC codecs, we need an extension where the native 64-bits are used. This patchset suggests a new helper, acpi_get_local_address() may be renamed if desired in a folow-up patch.
The path of least resistance would be to merge this patchset in the ASoC tree, since I have additional changes for ASoC/SDCA (SoundWire Device Class) that depend on the new helper.
Pierre-Louis Bossart (3): ACPI: utils: introduce acpi_get_local_u64_address() soundwire: slave: simplify code with acpi_get_local_u64_address() ALSA: hda: intel-sdw-acpi: use acpi_get_local_u64_address()
Reviewed-by: Takashi Iwai tiwai@suse.de
thanks,
Takashi
On Tue, May 28, 2024 at 02:29:32PM -0500, Pierre-Louis Bossart wrote:
The acpi_get_local_address() helper assumes a 32-bit ADR is used. If we want to use this helper for SoundWire/SDCA ASoC codecs, we need an extension where the native 64-bits are used. This patchset suggests a new helper, acpi_get_local_address() may be renamed if desired in a folow-up patch.
The path of least resistance would be to merge this patchset in the ASoC tree, since I have additional changes for ASoC/SDCA (SoundWire Device Class) that depend on the new helper.
Perhaps we may rename existing one to _u32 or so. Anyway, for this series: Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
On Tue, 28 May 2024 14:29:32 -0500, Pierre-Louis Bossart wrote:
The acpi_get_local_address() helper assumes a 32-bit ADR is used. If we want to use this helper for SoundWire/SDCA ASoC codecs, we need an extension where the native 64-bits are used. This patchset suggests a new helper, acpi_get_local_address() may be renamed if desired in a folow-up patch.
The path of least resistance would be to merge this patchset in the ASoC tree, since I have additional changes for ASoC/SDCA (SoundWire Device Class) that depend on the new helper.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ACPI: utils: introduce acpi_get_local_u64_address() commit: 0b7e448119428e1dcb854abb5855f66966fb82dc [2/3] soundwire: slave: simplify code with acpi_get_local_u64_address() commit: b6212f9bf489daf9716aed0e8c4dc6a807ce839f [3/3] ALSA: hda: intel-sdw-acpi: use acpi_get_local_u64_address() commit: 9b7dc68eeba04d20f4a1733e791bc71355423612
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (6)
-
Andy Shevchenko -
Mark Brown -
Pierre-Louis Bossart -
Rafael J. Wysocki -
Takashi Iwai -
Vinod Koul