diff mbox series

[v5,1/3] PCI/ACPI: add a helper for retrieving _OSC Control DWORDs

Message ID 20220406023746.2807328-2-vishal.l.verma@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series PCI/ACPI: add support for CXL _OSC | expand

Commit Message

Verma, Vishal L April 6, 2022, 2:37 a.m. UTC
During _OSC negotiation, when the 'Control' DWORD is needed from the
result buffer after running _OSC, a couple of places performed manual
pointer arithmetic to offset into the right spot in the raw buffer.
Add a acpi_osc_ctx_get_pci_control() helper to use the #define'd
DWORD offsets to fetch the DWORDs needed from @acpi_osc_context, and
replace the above instances of the open-coded arithmetic.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 include/linux/acpi.h    | 13 +++++++++++++
 drivers/acpi/bus.c      |  2 +-
 drivers/acpi/pci_root.c |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

Comments

Adam Manzanares April 8, 2022, 8:14 p.m. UTC | #1
On Tue, Apr 05, 2022 at 08:37:44PM -0600, Vishal Verma wrote:
> During _OSC negotiation, when the 'Control' DWORD is needed from the
> result buffer after running _OSC, a couple of places performed manual
> pointer arithmetic to offset into the right spot in the raw buffer.
> Add a acpi_osc_ctx_get_pci_control() helper to use the #define'd
> DWORD offsets to fetch the DWORDs needed from @acpi_osc_context, and
> replace the above instances of the open-coded arithmetic.
> 
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  include/linux/acpi.h    | 13 +++++++++++++
>  drivers/acpi/bus.c      |  2 +-
>  drivers/acpi/pci_root.c |  2 +-
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6274758648e3..ff4d1fdd9009 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -607,6 +607,13 @@ extern u32 osc_sb_native_usb4_control;
>  #define OSC_PCI_EXPRESS_LTR_CONTROL		0x00000020
>  #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
>  
> +static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> +	u32 *ret = context->ret.pointer;
> +
> +	return ret[OSC_CONTROL_DWORD];
> +}
> +
>  #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
>  #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
>  #define ACPI_GSB_ACCESS_ATTRIB_BYTE		0x00000006
> @@ -1003,6 +1010,12 @@ static inline int acpi_register_wakeup_handler(int wake_irq,
>  static inline void acpi_unregister_wakeup_handler(
>  	bool (*wakeup)(void *context), void *context) { }
>  
> +struct acpi_osc_context;
> +static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> +	return 0;
> +}
> +
>  #endif	/* !CONFIG_ACPI */
>  
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 07f604832fd6..4992e05cf429 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -405,7 +405,7 @@ static void acpi_bus_osc_negotiate_usb_control(void)
>  	}
>  
>  	osc_sb_native_usb4_control =
> -		control & ((u32 *)context.ret.pointer)[OSC_CONTROL_DWORD];
> +		control &  acpi_osc_ctx_get_pci_control(&context);
>  
>  	acpi_bus_decode_usb_osc("USB4 _OSC: OS supports", control);
>  	acpi_bus_decode_usb_osc("USB4 _OSC: OS controls",
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index b76db99cced3..40a74ff3fa02 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -185,7 +185,7 @@ static acpi_status acpi_pci_run_osc(acpi_handle handle,
>  
>  	status = acpi_run_osc(handle, &context);
>  	if (ACPI_SUCCESS(status)) {
> -		*retval = *((u32 *)(context.ret.pointer + 8));
> +		*retval = acpi_osc_ctx_get_pci_control(&context);
>  		kfree(context.ret.pointer);
>  	}
>  	return status;
> -- 
> 2.35.1
> 

Looks good.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Rafael J. Wysocki April 12, 2022, 7:23 p.m. UTC | #2
On Wed, Apr 6, 2022 at 4:38 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> During _OSC negotiation, when the 'Control' DWORD is needed from the
> result buffer after running _OSC, a couple of places performed manual
> pointer arithmetic to offset into the right spot in the raw buffer.
> Add a acpi_osc_ctx_get_pci_control() helper to use the #define'd
> DWORD offsets to fetch the DWORDs needed from @acpi_osc_context, and
> replace the above instances of the open-coded arithmetic.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Suggested-by: Davidlohr Bueso <dave@stgolabs.net>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Please feel free to add

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to this patch.

Or I can take this series unless Bjorn wants to do that.

Bjorn, please let me know what you prefer here.

> ---
>  include/linux/acpi.h    | 13 +++++++++++++
>  drivers/acpi/bus.c      |  2 +-
>  drivers/acpi/pci_root.c |  2 +-
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6274758648e3..ff4d1fdd9009 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -607,6 +607,13 @@ extern u32 osc_sb_native_usb4_control;
>  #define OSC_PCI_EXPRESS_LTR_CONTROL            0x00000020
>  #define OSC_PCI_EXPRESS_DPC_CONTROL            0x00000080
>
> +static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> +       u32 *ret = context->ret.pointer;
> +
> +       return ret[OSC_CONTROL_DWORD];
> +}
> +
>  #define ACPI_GSB_ACCESS_ATTRIB_QUICK           0x00000002
>  #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
>  #define ACPI_GSB_ACCESS_ATTRIB_BYTE            0x00000006
> @@ -1003,6 +1010,12 @@ static inline int acpi_register_wakeup_handler(int wake_irq,
>  static inline void acpi_unregister_wakeup_handler(
>         bool (*wakeup)(void *context), void *context) { }
>
> +struct acpi_osc_context;
> +static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
> +{
> +       return 0;
> +}
> +
>  #endif /* !CONFIG_ACPI */
>
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 07f604832fd6..4992e05cf429 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -405,7 +405,7 @@ static void acpi_bus_osc_negotiate_usb_control(void)
>         }
>
>         osc_sb_native_usb4_control =
> -               control & ((u32 *)context.ret.pointer)[OSC_CONTROL_DWORD];
> +               control &  acpi_osc_ctx_get_pci_control(&context);
>
>         acpi_bus_decode_usb_osc("USB4 _OSC: OS supports", control);
>         acpi_bus_decode_usb_osc("USB4 _OSC: OS controls",
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index b76db99cced3..40a74ff3fa02 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -185,7 +185,7 @@ static acpi_status acpi_pci_run_osc(acpi_handle handle,
>
>         status = acpi_run_osc(handle, &context);
>         if (ACPI_SUCCESS(status)) {
> -               *retval = *((u32 *)(context.ret.pointer + 8));
> +               *retval = acpi_osc_ctx_get_pci_control(&context);
>                 kfree(context.ret.pointer);
>         }
>         return status;
> --
> 2.35.1
>
diff mbox series

Patch

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6274758648e3..ff4d1fdd9009 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -607,6 +607,13 @@  extern u32 osc_sb_native_usb4_control;
 #define OSC_PCI_EXPRESS_LTR_CONTROL		0x00000020
 #define OSC_PCI_EXPRESS_DPC_CONTROL		0x00000080
 
+static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
+{
+	u32 *ret = context->ret.pointer;
+
+	return ret[OSC_CONTROL_DWORD];
+}
+
 #define ACPI_GSB_ACCESS_ATTRIB_QUICK		0x00000002
 #define ACPI_GSB_ACCESS_ATTRIB_SEND_RCV         0x00000004
 #define ACPI_GSB_ACCESS_ATTRIB_BYTE		0x00000006
@@ -1003,6 +1010,12 @@  static inline int acpi_register_wakeup_handler(int wake_irq,
 static inline void acpi_unregister_wakeup_handler(
 	bool (*wakeup)(void *context), void *context) { }
 
+struct acpi_osc_context;
+static inline u32 acpi_osc_ctx_get_pci_control(struct acpi_osc_context *context)
+{
+	return 0;
+}
+
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 07f604832fd6..4992e05cf429 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -405,7 +405,7 @@  static void acpi_bus_osc_negotiate_usb_control(void)
 	}
 
 	osc_sb_native_usb4_control =
-		control & ((u32 *)context.ret.pointer)[OSC_CONTROL_DWORD];
+		control &  acpi_osc_ctx_get_pci_control(&context);
 
 	acpi_bus_decode_usb_osc("USB4 _OSC: OS supports", control);
 	acpi_bus_decode_usb_osc("USB4 _OSC: OS controls",
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index b76db99cced3..40a74ff3fa02 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -185,7 +185,7 @@  static acpi_status acpi_pci_run_osc(acpi_handle handle,
 
 	status = acpi_run_osc(handle, &context);
 	if (ACPI_SUCCESS(status)) {
-		*retval = *((u32 *)(context.ret.pointer + 8));
+		*retval = acpi_osc_ctx_get_pci_control(&context);
 		kfree(context.ret.pointer);
 	}
 	return status;