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 <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Bard Liao <[email protected]>
---
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);
--
2.43.0
On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart
<[email protected]> 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 <[email protected]>
> Reviewed-by: Péter Ujfalusi <[email protected]>
> Reviewed-by: Bard Liao <[email protected]>
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 <[email protected]>
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 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 <[email protected]>
> 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 6/7/24 20:51, Rafael J. Wysocki wrote:
> On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart
> <[email protected]> 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 <[email protected]>
>> Reviewed-by: Péter Ujfalusi <[email protected]>
>> Reviewed-by: Bard Liao <[email protected]>
>
> 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 <[email protected]>
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 7, 2024 at 10:33 PM Pierre-Louis Bossart
<[email protected]> wrote:
>
>
>
> On 6/7/24 20:51, Rafael J. Wysocki wrote:
> > On Tue, May 28, 2024 at 9:29 PM Pierre-Louis Bossart
> > <[email protected]> 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 <[email protected]>
> >> Reviewed-by: Péter Ujfalusi <[email protected]>
> >> Reviewed-by: Bard Liao <[email protected]>
> >
> > 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 <[email protected]>
>
> 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.