On Sat, Jul 25, 2020 at 7:01 AM
<sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote:
>
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
>
> 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
