Re: [PATCH v2 09/18] pci: Add config for Enhanced Allocation

[Andrew Scull]

On Thu, 14 Apr 2022 at 08:58, Andrew Scull <asc...@google.com> wrote:
>
> On Thu, 14 Apr 2022 at 01:42, Bin Meng <bmeng...@gmail.com> wrote:
> >
> > On Thu, Apr 14, 2022 at 12:30 AM Andrew Scull <asc...@google.com> wrote:
> > >
> > > > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > > > > index 47cd074aa1..fd2203420c 100644
> > > > > --- a/drivers/pci/Kconfig
> > > > > +++ b/drivers/pci/Kconfig
> > > > > @@ -67,6 +67,13 @@ config PCI_SRIOV
> > > > >           if available on a PCI Physical Function device and probe for
> > > > >           applicable drivers.
> > > > >
> > > > > +config PCI_ENHANCED_ALLOCATION
> > > > > +       bool "Enable support for Enhanced Allocation of resources"
> > > > > +       default y
> > > > > +       help
> > > > > +         Enable support for Enhanced Allocation which can be used by 
> > > > > supported
> > > > > +         devices in place of traditional BARS for allocation of 
> > > > > resources.
> > > > > +
> > > >
> > > > Why do we need a config option for EA as it can be figured out in the 
> > > > run time?
> > >
> > > I wanted to have the option to disable the logic for handling EA at
> > > runtime. A glance at the code suggested there's a whole other rabbit
> > > hole of missing safety checks that I've opted to leave for another
> > > day.
> >
> > We cannot have a single global option to work for one device using EA
> > but another device using BAR.
>
> Sorry, I don't understand what you mean. This config will just mean
> the u-boot doesn't look for, or handle, the EA capability; it disables
> u-boot's support for this feature. If a system relies on EA, they'll
> need to have it enabled which is the default state. If a system
> doesn't then it can be turned off and the suspect logic disabled.
>
> Could you help me understand your concern?

A neater version of the last hunk is below. But if you still don't
think it belongs upstream, I'll remove it from the series to unblock
the rest.

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 8bbeb62f2e..c4fe4e481a 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -645,7 +645,11 @@ int dm_pci_hose_probe_bus(struct udevice *bus)
                return log_msg_ret("probe", -EINVAL);
        }

-       ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA);
+       if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION))
+               ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA);
+       else
+               ea_pos = 0;
+
        if (ea_pos) {
                dm_pci_read_config8(bus, ea_pos + sizeof(u32) + sizeof(u8),
                                    &reg);
@@ -1606,7 +1610,11 @@ void *dm_pci_map_bar(struct udevice *dev, int
bar, unsigned long flags)
         * Incase of virtual functions, pdata will help read VF BEI
         * and EA entry size.
         */
-       ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA);
+       if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION))
+               ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA);
+       else
+               ea_off = 0;
+
        if (ea_off)
                return dm_pci_map_ea_bar(udev, bar, ea_off, pdata);