On Fri, 18 Mar 2022, Vishal Verma wrote:
+/* Max possible _OSC capability DWORDS */
+#define OSC_CAPABILITY_DWORDS_MAX 5
+
/* Indexes into _OSC Capabilities Buffer (DWORDs 2 & 3 are device-specific) */
#define OSC_QUERY_DWORD 0 /* DWORD 1 */
#define OSC_SUPPORT_DWORD 1 /* DWORD 2 */
#define OSC_CONTROL_DWORD 2 /* DWORD 3 */
+#define OSC_CXL_SUPPORT_DWORD 3 /* DWORD 4 */
+#define OSC_CXL_CONTROL_DWORD 4 /* DWORD 5 */
Shouldn't all this be in patch 1/2 (and also as enum maybe)? Or at least
the define OSC_CAPABILITY_DWORDS_MAX instead of having to do:
-static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 support)
+static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask,
+ u32 support, u32 *cxl_mask,
+ u32 cxl_support)
{
u32 req = OSC_PCI_EXPRESS_CAPABILITY_CONTROL;
struct acpi_pci_root *root;
acpi_status status;
- u32 ctrl, capbuf[6];
+ u32 ctrl, cxl_ctrl = 0, capbuf[OSC_CAPABILITY_DWORDS_MAX];
... which btw why is capbuf 6 in the previous patch and now 5 in this one?
Sorry if I missed anything obvious here, just seems odd.
And also it's ugly to just add extra params to acpi_pci_osc_control_set()
and have callers do do:
+ status = acpi_pci_osc_control_set(handle, &control, support,
+ &cxl_control, cxl_support);
And this sort of thing happens all over the patch with struct acpi_pci_root.
So the whole handling of the _OSC state of support/control bits imo really
wants to be consolidated between CXL and non-CXL.
Thanks,
Davidlohr
