| Message ID | 20240528192936.16180-2-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [1/3] ACPI: utils: introduce acpi_get_local_u64_address() | expand |
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.
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);
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(-)