Re: [PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default

["Maciej W. Rozycki" <macro@orcam.me.uk>]

On Tue, 30 Aug 2022, Stefan Roese wrote:

> > I think I wrote it. One issue is that it is increasing size of SPL image
> > and we really should not include into SPL things which are not required
> > for all target platforms. Lot of boards have size constrained memory
> > requirements and unnecessary features should not be automatically
> > enabled.
> 
> I have to agree with Pali here. We need to be careful with size increase
> in the SPL images, as some of the build targets are very limited here.
> So making this workaround configurable is definitely a good idea.

 I object to having just a single workaround configurable as I expect it 
to be beyond the capability of people on average to get right.  You simply 
start getting lost at one point, just as I am with all the random options 
the Linux kernel has nowadays.  And the dozens added with every release.

 I agree on the SPL side and on a global option.  I think it only makes 
sense to have individual workarounds selectable for onboard devices with 
hardware that has no external PCI/e connectivity.  Those should not be 
exposed to the user and instead implicitly selected by the scriptery based 
on the base machine chosen.

> The question remains, at least for me, if the Kconfig option should be
> enable per default or not. For SPL my suggestions is to disable is per
> default because of the size remarks above. For U-Boot proper I'm not so
> sure. Please see below...

 FWIW Linux has a global PCI_QUIRKS option available in the expert mode 
only that lets one disable all workarounds:

config PCI_QUIRKS
	default y
	bool "Enable PCI quirk workarounds" if EXPERT
	help
	  This enables workarounds for various PCI chipset bugs/quirks.
	  Disable this only if your target machine is unaffected by PCI
	  quirks.

for U-Boot I could envisage something like:

config PCI_QUIRKS
	bool "Enable PCI quirk workarounds" if EXPERT
	default y if !SPL
	[...]

Leaving just a single workaround out, especially for option card devices 
is asking for people getting it wrong.

 If there are individual workarounds required by hardware with no external 
PCI/e connectivity, then we could have say:

config PCI_QUIRKS
	bool "Enable PCI quirk workarounds" if EXPERT
	default y if !SPL
	select SPECIFIC_QUIRK_FOR_BADLY_BROKEN_BOARD

config BADLY_BROKEN_BOARD
	bool "Enable support for Badly Broken Board"
	select SPECIFIC_QUIRK_FOR_BADLY_BROKEN_BOARD

config SPECIFIC_QUIRK_FOR_BADLY_BROKEN_BOARD
	bool

and then wire the specific quirk to SPECIFIC_QUIRK_FOR_BADLY_BROKEN_BOARD 
rather than PCI_QUIRKS in the Makefile system.

 Thank you for your input.

  Maciej