| Message ID | 20241211-fix-ipu-v4-2-4102e97aceb6@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ipu6: get rid of all the IS_ENABLED(CONFIG_ACPI) | expand |
On Wed, Dec 11, 2024 at 5:07 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > Provide an implementation of acpi_get_physical_device_location that can > be used when CONFIG_ACPI is not set. > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > include/acpi/acpi_bus.h | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 2b09e513ecf3..b312a72f48ad 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -43,9 +43,6 @@ acpi_status > acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, > struct acpi_buffer *status_buf); > > -acpi_status > -acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > - > bool acpi_has_method(acpi_handle handle, char *name); > acpi_status acpi_execute_simple_method(acpi_handle handle, char *method, > u64 arg); > @@ -60,6 +57,9 @@ bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs); > union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, > u64 rev, u64 func, union acpi_object *argv4); > #ifdef CONFIG_ACPI > +acpi_status > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > + > static inline union acpi_object * > acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev, > u64 func, union acpi_object *argv4, > @@ -1003,6 +1003,12 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } > > static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } > > +static inline acpi_status > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) > +{ > + return AE_ERROR; > +} I overlooked this before, sorry. It generally is not OK to use acpi_status and/or AE_ error codes without CONFIG_ACPI and they really only should be used in drivers/acpi/ (and not everywhere in there for that matter). So acpi_get_physical_device_location() needs to be redefined to return something different from acpi_status (preferably bool) in order to be used in !CONFIG_ACPI code. > + > #define for_each_acpi_dev_match(adev, hid, uid, hrv) \ > for (adev = NULL; false && (hid) && (uid) && (hrv); ) > > > --
Hi Rafael On Mon, 16 Dec 2024 at 21:17, Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Dec 11, 2024 at 5:07 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > Provide an implementation of acpi_get_physical_device_location that can > > be used when CONFIG_ACPI is not set. > > > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Acked-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > include/acpi/acpi_bus.h | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 2b09e513ecf3..b312a72f48ad 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -43,9 +43,6 @@ acpi_status > > acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, > > struct acpi_buffer *status_buf); > > > > -acpi_status > > -acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > > - > > bool acpi_has_method(acpi_handle handle, char *name); > > acpi_status acpi_execute_simple_method(acpi_handle handle, char *method, > > u64 arg); > > @@ -60,6 +57,9 @@ bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs); > > union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, > > u64 rev, u64 func, union acpi_object *argv4); > > #ifdef CONFIG_ACPI > > +acpi_status > > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > > + > > static inline union acpi_object * > > acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev, > > u64 func, union acpi_object *argv4, > > @@ -1003,6 +1003,12 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } > > > > static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } > > > > +static inline acpi_status > > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) > > +{ > > + return AE_ERROR; > > +} > > I overlooked this before, sorry. > > It generally is not OK to use acpi_status and/or AE_ error codes > without CONFIG_ACPI and they really only should be used in > drivers/acpi/ (and not everywhere in there for that matter). > > So acpi_get_physical_device_location() needs to be redefined to return > something different from acpi_status (preferably bool) in order to be > used in !CONFIG_ACPI code. Shall I redefine it to bool acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld)/ For both the ACPI and !ACPI cases? or just for the !ACPI? Regards! > > > + > > #define for_each_acpi_dev_match(adev, hid, uid, hrv) \ > > for (adev = NULL; false && (hid) && (uid) && (hrv); ) > > > > > > --
On Mon, Dec 16, 2024 at 9:42 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > Hi Rafael > > On Mon, 16 Dec 2024 at 21:17, Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Wed, Dec 11, 2024 at 5:07 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > > > Provide an implementation of acpi_get_physical_device_location that can > > > be used when CONFIG_ACPI is not set. > > > > > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Acked-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > --- > > > include/acpi/acpi_bus.h | 12 +++++++++--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > > index 2b09e513ecf3..b312a72f48ad 100644 > > > --- a/include/acpi/acpi_bus.h > > > +++ b/include/acpi/acpi_bus.h > > > @@ -43,9 +43,6 @@ acpi_status > > > acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, > > > struct acpi_buffer *status_buf); > > > > > > -acpi_status > > > -acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > > > - > > > bool acpi_has_method(acpi_handle handle, char *name); > > > acpi_status acpi_execute_simple_method(acpi_handle handle, char *method, > > > u64 arg); > > > @@ -60,6 +57,9 @@ bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs); > > > union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, > > > u64 rev, u64 func, union acpi_object *argv4); > > > #ifdef CONFIG_ACPI > > > +acpi_status > > > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); > > > + > > > static inline union acpi_object * > > > acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev, > > > u64 func, union acpi_object *argv4, > > > @@ -1003,6 +1003,12 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } > > > > > > static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } > > > > > > +static inline acpi_status > > > +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) > > > +{ > > > + return AE_ERROR; > > > +} > > > > I overlooked this before, sorry. > > > > It generally is not OK to use acpi_status and/or AE_ error codes > > without CONFIG_ACPI and they really only should be used in > > drivers/acpi/ (and not everywhere in there for that matter). > > > > So acpi_get_physical_device_location() needs to be redefined to return > > something different from acpi_status (preferably bool) in order to be > > used in !CONFIG_ACPI code. > > Shall I redefine it to > bool acpi_get_physical_device_location(acpi_handle handle, struct > acpi_pld_info **pld)/ > > For both the ACPI and !ACPI cases? or just for the !ACPI? For both cases, please.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 2b09e513ecf3..b312a72f48ad 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -43,9 +43,6 @@ acpi_status acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, struct acpi_buffer *status_buf); -acpi_status -acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); - bool acpi_has_method(acpi_handle handle, char *name); acpi_status acpi_execute_simple_method(acpi_handle handle, char *method, u64 arg); @@ -60,6 +57,9 @@ bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs); union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 func, union acpi_object *argv4); #ifdef CONFIG_ACPI +acpi_status +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld); + static inline union acpi_object * acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev, u64 func, union acpi_object *argv4, @@ -1003,6 +1003,12 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } +static inline acpi_status +acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) +{ + return AE_ERROR; +} + #define for_each_acpi_dev_match(adev, hid, uid, hrv) \ for (adev = NULL; false && (hid) && (uid) && (hrv); )