Re: [PATCH v8 2/5] ACPI/PCI: Ignore _OSC negotiation result if pcie_ports_native is set.

Andy Shevchenko
2020-07-25T10:36:02Z

On Sat, Jul 25, 2020 at 7:01 AM
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> pcie_ports_native is set only if user requests native handling

the user

> of PCIe capabilities via pcie_port_setup command line option.
> User input takes precedence over _OSC based control negotiation
> result. So consider the _OSC negotiated result only if
> pcie_ports_native is unset.
>
> Also, since struct pci_host_bridge ->native_* members caches the
> ownership status of various PCIe capabilities, use them instead
> of distributed checks for pcie_ports_native.

...

> +static char *get_osc_desc(u32 bit)
> +{

> + int i = 0;

Unneeded assignment.

> + for (i = 0; i < ARRAY_SIZE(pci_osc_control_bit); i++)
> + if (bit == pci_osc_control_bit[i].bit)
> + return pci_osc_control_bit[i].desc;
> +
> + return NULL;
> +}

...

> host_bridge = to_pci_host_bridge(bus->bridge);
> - if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
> - host_bridge->native_pcie_hotplug = 0;
> + if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) {
> + if (!pcie_ports_native)
> + host_bridge->native_pcie_hotplug = 0;
> + else

> + dev_warn(&bus->dev, "OS overrides %s firmware control",

pci_warn() ?

> + get_osc_desc(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL));
> + }
> +

> if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
> host_bridge->native_shpc_hotplug = 0;

Group them in the same way you did in the previous patch.

> - if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL))
> - host_bridge->native_aer = 0;
> - if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL))
> - host_bridge->native_pme = 0;
> - if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL))
> - host_bridge->native_ltr = 0;
> - if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> - host_bridge->native_dpc = 0;
> +
> + if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL)) {

> + if (!pcie_ports_native)
> + host_bridge->native_aer = 0;
> + else
> + dev_warn(&bus->dev, "OS overrides %s firmware control",
> + get_osc_desc(OSC_PCI_EXPRESS_AER_CONTROL));

Looks like a lot of duplication here. Perhaps split out a helper ?

> + }

--
With Best Regards,
Andy Shevchenko