lkml.org 
[lkml]   [2016]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] PCI: iproc: Fix BCMA PCIe bus scanning regression
From
Date
Hi Bjorn,

On 1/26/2016 10:22 AM, Bjorn Helgaas wrote:
> Hi Ray,
>
> On Wed, Jan 20, 2016 at 02:55:10PM -0800, Ray Jui wrote:
>> Commit 943ebae781f5 ("PCI: iproc: Add PAXC interface support") causes
>> regression on EP device detection on BCMA based platforms. This patch
>> fixes the issue by allowing multiple devices to be configured on the
>> same bus, for all PAXB based child buses
>>
>> Reported-by: Rafal Milecki <zajec5@gmail.com>
>> Fixes: 943ebae781f5 ("PCI: iproc: Add PAXC interface support")
>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>> ---
>> drivers/pci/host/pcie-iproc.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
>> index 5816bce..4627561 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -171,10 +171,11 @@ static inline void iproc_pcie_ob_write(struct iproc_pcie *pcie,
>> }
>>
>> static inline bool iproc_pcie_device_is_valid(struct iproc_pcie *pcie,
>> + unsigned int busnum,
>> unsigned int slot,
>> unsigned int fn)
>> {
>> - if (slot > 0)
>> + if ((pcie->type == IPROC_PCIE_PAXC || busnum == 0) && slot > 0)
>> return false;
>>
>> /* PAXC can only support limited number of functions */
>
> I don't understand this. Here's the whole function (with this patch
> applied):
>
> static inline bool iproc_pcie_device_is_valid(struct iproc_pcie *pcie,
> unsigned int busnum,
> unsigned int slot,
> unsigned int fn)
> {
> if ((pcie->type == IPROC_PCIE_PAXC || busnum == 0) && slot > 0)
> return false;
>
> /* PAXC can only support limited number of functions */
> if (pcie->type == IPROC_PCIE_PAXC && fn >= MAX_NUM_PAXC_PF)
> return false;
>
> return true;
> }
>
> This says:
>
> - On bus 00, device 0 is the only valid device. That seems
> plausible because the devices on bus 00 are probably built-in to
> the SoC.
>
> - On PAXC-based systems, device 0 is the only valid device on *any*
> bus. Is that really true? If there's any way to add a plug-in
> card, this seems overly restrictive.

Yah, PAXC is connected with one internal device within the SoC. There's
no connection brought out of the chip.

>
> PCIe devices are generally all device 0, but this would mean you
> cannot plug in a PCIe-to-PCI bridge leading to a PCI device with a
> non-zero device number.
>
> I think it also means you could not plug in a PCIe device with ARI
> enabled, because I think we store the upper 5 bits of the 8-bit
> ARI function number in the PCI_SLOT bits.
>
> - On PAXC-based systems, only functions 0, 1, 2, and 3 are valid
> anywhere in the hierarchy. I think this again restricts what what
> cards can be plugged in.

Yes, the internal device connected to PAXC supports 4 physical functions.

>
> If iProc only supports devices built directly into the SoC, maybe
> these constraints are valid. But if it supports any plugin or
> external devices, they don't seem to make sense.

Correct. PAXC only connects to one built-in device, while PAXB can
support external EP devices.

>
> Also, is it the case that an iProc root bus is always bus number zero?
> That's certainly not the case for many other host controllers, but
> maybe you only have one possible host controller per system and the
> base number is not programmable.

An iProc based SoC can potentially have multiple root complexes, with
each of them resides on separate PCIe domain (and always on bus 0). I
think this is similar to how Exynos PCIe host controller is modeled.

>
> Bjorn
>

Thanks,

Ray

\
 
 \ /
  Last update: 2016-01-26 20:21    [W:3.084 / U:1.416 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site