On Wed, Jan 12, 2022 at 04:21:28PM -0800, Lucas De Marchi wrote:
> On Wed, Jan 12, 2022 at 06:08:05PM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 12, 2022 at 03:30:43PM -0800, Lucas De Marchi wrote:
> > > The flags are only used to mark a quirk to be called once and nothing
> > > else. Also, that logic may not be appropriate if the quirk wants to
> > > do additional filtering and set quirk as applied by itself.
> > >
> > > So replace the uses of QFLAG_APPLY_ONCE with static local variables in
> > > the few quirks that use this logic and remove all the flags logic.
> > >
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > > Reviewed-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >
> > Only occurred to me now, but another, less intrusive approach would be
> > to just remove QFLAG_APPLY_ONCE from intel_graphics_quirks() and do
> > its bookkeeping internally, e.g.,
>
> that is actually what I suggested after your comment in v2: this would
> be the first patch with "minimal fix". But then to keep it consistent
> with the other calls to follow up with additional patches on top
> converting them as well. Maybe what I wrote wasn't clear in the
> direction? Copying it here:
>
> 1) add the static local only to intel graphics quirk and remove the
> flag from this item
> 2 and 3) add the static local to other functions and remove the flag
> from those items
> 4) remove the flag from the table, the defines and its usage.
> 5) fix the coding style (to be clear, it's already wrong, not
> something wrong introduced here... maybe could be squashed in (4)?)
Oh, sorry, I guess I just skimmed over that without really
comprehending it.
Although the patch below is basically just 1 from above and doesn't
require any changes to the other functions or the flags themselves
(2-4 above).
> > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> > index 391a4e2b8604..7b655004e5fd 100644
> > --- a/arch/x86/kernel/early-quirks.c
> > +++ b/arch/x86/kernel/early-quirks.c
> > @@ -587,10 +587,14 @@ intel_graphics_stolen(int num, int slot, int func,
> >
> > static void __init intel_graphics_quirks(int num, int slot, int func)
> > {
> > + static bool stolen __initdata = false;
> > const struct intel_early_ops *early_ops;
> > u16 device;
> > int i;
> >
> > + if (stolen)
> > + return;
> > +
> > device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
> >
> > for (i = 0; i < ARRAY_SIZE(intel_early_ids); i++) {
> > @@ -602,6 +606,7 @@ static void __init intel_graphics_quirks(int num, int slot, int func)
> > early_ops = (typeof(early_ops))driver_data;
> >
> > intel_graphics_stolen(num, slot, func, early_ops);
> > + stolen = true;
> >
> > return;
> > }
> > @@ -703,7 +708,7 @@ static struct chipset early_qrk[] __initdata = {
> > { PCI_VENDOR_ID_INTEL, 0x3406, PCI_CLASS_BRIDGE_HOST,
> > PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
> > { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
> > - QFLAG_APPLY_ONCE, intel_graphics_quirks },
> > + 0, intel_graphics_quirks },
> > /*
> > * HPET on the current version of the Baytrail platform has accuracy
> > * problems: it will halt in deep idle state - so we disable it.
