[PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported

[Pali Rohár]

On Tuesday 30 November 2021 07:09:36 Stefan Roese wrote:
> On 11/25/21 11:32, Pali Rohár wrote:
> > If U-Boot does not have any I/O resource for assignment then disable I/O
> > forwarding in PCI bridge autoconfiguration code. Default initial state of
> > PCI bridge IO registers is unspecified, therefore they can be in enabled if
> > U-Boot does not touch them.
> > 
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > 
> > ---
> >   drivers/pci/pci_auto.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> > index 7e6ee54be087..6e5ed194f247 100644
> > --- a/drivers/pci/pci_auto.c
> > +++ b/drivers/pci/pci_auto.c
> > @@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
> >   				      (pci_io->bus_lower & 0xffff0000) >> 16);
> >   		cmdstat |= PCI_COMMAND_IO;
> > +	} else {
> > +		/* Disable I/O if unsupported */
> > +		dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32);
> > +		dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32);
> 
> Does it perhaps make sense to add / use a macro for this 0xf0 above?

For setting range bits we already have macro PCI_IO_RANGE_MASK.

So for setting all base range bits to one (which is this patch is
suppose to do) I can write something like this: (~0 & PCI_IO_RANGE_MASK)
or (0xff & PCI_IO_RANGE_MASK). (write_config8 denotes that we use 8bit
numbers...)

I/O forwarding is disabled when base address is higher than limit
address and it is common to choose base address as the highest possible
(hence all ones) and limit address to lowest possible (hence all zeros).

So question is, do we need macro which evaluates to number with all
zeros and another macro which evaluates to number with all ones?

I'm not sure. For me is "0xf0 | io_32" more readable as "the verbose
usage with macro" "(0xff & PCI_IO_RANGE_MASK) | io_32".

But this is all just about "naming conventions" and "coding style". If
you think that for consistency is better to create macro or use another
construction, please let me know other ideas. I would follow any style
which is expected here...

> Other than this:
> 
> Reviewed-by: Stefan Roese <sr at denx.de>
> 
> Thanks,
> Stefan
> 
> 
> > +		if (io_32 == PCI_IO_RANGE_TYPE_32) {
> > +			dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0);
> > +			dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0);
> > +		}
> >   	}
> >   	/* Enable memory and I/O accesses, enable bus master */
> > 
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de