Re: [PATCH v8 2/5] PCI: Add Loongson PCI Controller support

[Bjorn Helgaas]

On Sat, May 09, 2020 at 01:28:24AM +0800, Jiaxun Yang wrote:
> ä 2020å5æ9æ GMT+08:00 äå1:17:30, Bjorn Helgaas <helgaas@xxxxxxxxxx> åå:
> >On Fri, May 08, 2020 at 07:34:02PM +0800, Jiaxun Yang wrote:
> >> This controller can be found on Loongson-2K SoC, Loongson-3
> >> systems with RS780E/LS7A PCH.
> >> 
> >> The RS780E part of code was previously located at
> >> arch/mips/pci/ops-loongson3.c and now it can use generic PCI
> >> driver implementation.
> >> 
> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
> >> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> >
> >> +static void system_bus_quirk(struct pci_dev *pdev)
> >> +{
> >> +	u16 tmp;
> >> +
> >> +	/* 
> >> +	 * System buses on Loongson system contain garbage in BARs
> >> +	 * but their decoding need to be enabled to ensure devices
> >> +	 * under system buses are reachable. In most cases it should
> >> +	 * be done by the firmware.
> >
> >This isn't a very satisfying explanation because devices that have
> >decoding enabled can interfere with other devices in the system, and I
> >can't tell whether that's a problem here.
> >
> >What happens when you turn on MEM/IO decoding below?  Does the device
> >decode any address space?  How do we know what it is?  Is it related
> >to the BAR contents?
> >
> >I'm a little dubious about the need for the PCI_COMMAND write because
> >the previous version didn't do it (since it incorrectly wrote to
> >PCI_STATUS), and I assume that version worked.
> 
> Sorry, but that's all I can tell from the chips manual as I'm not a
> employee of the vendor.
> 
> My assumption is these BAR contains the address of those system
> components that already configured by firmware and we shouldn't
> touch it.

What are the values in the BARs?  Do they look like addresses?  Would
they make sense in the iomem_resource, i.e., in /proc/iomem?

If the BARs contain addresses of devices, the kernel needs to avoid
that address space when allocating space for other devices.  But
without probing the BARs, the kernel has no idea what the *size* of
the regions is.  Does the manual say anything about that?

Can you tell if there's anything at all Linux could do with these
devices?  If they don't have BARs and they don't document any
device-specific registers in config space, there's not much value in
treating them like PCI devices at all.

I wonder if we should just make a way to completely ignore them, e.g.,
a way for pci_setup_device() to return an error.   I guess we *could*
do that already by making your early fixup set dev->hdr_type to 0xff
or something.

> In fact according to my tests if we let Kernel probe these BAR then
> the system will hang immediately.
> 
> Chip manual suggested OS to ensure decoding is enabled so I'm doing
> like this. But without this system can also work.
> 
> Do you think I should drop it until figure out what was it actually
> doing?