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

[Jiaxun Yang <jiaxun.yang@flygoat.com>]

于 2020年5月13日 GMT+08:00 上午2:06:02, Bjorn Helgaas <helgaas@kernel.org> 写到:
>On Tue, May 12, 2020 at 03:43:56PM +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@flygoat.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>
>> +static void system_bus_quirk(struct pci_dev *pdev)
>> +{
>> +	u16 tmp;
>> +
>> +	/* 
>> +	 * These devices are not sharing resouces with rest of devices
>> +	 * on host bus and firmware will ensure their BARs are placed
>> +	 * in safe ranges. Also there might be some config registers
>> +	 * in their config space so kernel shouldn't ignore them.
>
>"Firmware ensuring BARs are placed in 'safe' ranges" is not a
>sufficient answer.  As I said before, Linux needs to know both the
>ADDRESS and the SIZE of whatever non-standard BARs these are.
>Otherwise, we're liable to assign that space to a different device.

The address assigned to these devices will never be a part of resources
belongs to the host bridge. That's enforced by hardware and firmware,
so address conflict would never happen.

I'm doing like this to ensure kernel will discover this device but do nothing
about assignment of resources to it.

>
>If you have to hard-code the size, so be it.  That would mean the
>hardware is completely broken, but at least we could make Linux deal
>with it.  If the hardware consumes address space we don't know about,
>we can't deal with that.
>
>> +	 */
>> +	pdev->mmio_always_on = 1;
>> +	pdev->non_compliant_bars = 1;
>> +}
>
>> +void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devfn,
>> +			       int where)
>> +{
>> +	unsigned char busnum = bus->number;
>> +	struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
>> +	struct loongson_pci *priv =  pci_host_bridge_priv(bridge);
>> +
>> +	/*
>> +	 * Do not read more than one device on the bus other than
>> +	 * the host bus 0.
>> +	 */
>
>If the hardware is restricted such that the root bus number must be
>zero, pleae say that explicitly here.  Otherwise, it just looks like
>a bug.
>
>> +	if (priv->flags & FLAG_DEV_FIX && busnum != 0 &&
>> +		PCI_SLOT(devfn) > 0)
>> +		return NULL;
>> +
>> +	/* CFG0 can only access standard space */
>> +	if (where < PCI_CFG_SPACE_SIZE && priv->cfg0_base)
>> +		return cfg0_map(priv, busnum, devfn, where);
>> +
>> +	/* CFG1 can access extended space */
>> +	if (where < PCI_CFG_SPACE_EXP_SIZE && priv->cfg1_base)
>> +		return cfg1_map(priv, busnum, devfn, where);
>> +
>> +	return NULL;
>> +}

-- 
Jiaxun Yang