All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ray Jui <rjui@broadcom.com>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: Scott Branden <sbranden@broadcom.com>,
	Jon Mason <jonmason@broadcom.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Linux PCI <linux-pci@vger.kernel.org>,
	Hante Meuleman <meuleman@broadcom.com>
Subject: Re: iProc bus scanning regression (after "PCI: iproc: Add PAXC interface support")
Date: Wed, 20 Jan 2016 09:55:06 -0800	[thread overview]
Message-ID: <569FC9FA.4090607@broadcom.com> (raw)
In-Reply-To: <CACna6rxyMoarw=Yq93gijXeXx5bm35hzAs-+9w3SaXg7e1thtQ@mail.gmail.com>

Hi Rafal,

On 1/20/2016 8:43 AM, Rafał Miłecki wrote:
> On 20 January 2016 at 09:13, Ray Jui <rjui@broadcom.com> wrote:
>> On 1/19/2016 11:02 PM, Rafał Miłecki wrote:
>>>
>>> On 20 January 2016 at 07:53, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>>
>>>> In OpenWrt trunk code we use iProc driver for PCIe controllers on bcma
>>>> bus (PCIE_IPROC_BCMA). Right now we use 4.1 kernel but we backported
>>>> all iProc changes. Unfortunately backporting set queued for 4.5 broke
>>>> bus scanning on some routers.
>>>>
>>>> Most BCM4708 / BCM4709 chipsets have 3 PCIe controllers. All known
>>>> routers use only first 2 of them. Even if router has 3 PCIe cards, the
>>>> last two cards are connected to the 2nd PCIe controller.
>>>>
>>>> So that PAXC patch broke support for routers with 3 cards, 2 of them
>>>> at the 2nd controller. It doesn't affect routers with just 2 cards.
>>>> This problem was tracked down in:
>>>> https://dev.openwrt.org/ticket/21393
>>>>
>>>> I'm attaching two OpenWrt boot logs.
>>>> 1) r48381
>>>> It contains all backported iProc changes. The log looks "nice", but
>>>> only one card (0000:01:00.0) was detected.
>>>> 2) r48382
>>>> It contains "Revert "PCI: iproc: Add PAXC interface support"" patch.
>>>> It contains many "[Firmware Bug]: reg 0x??: invalid BAR (can't size)"
>>>> messages but there are all 3 cards detected: 0000:01:00.0,
>>>> 0001:03:00.0 and 0001:04:00.0 (see brcmfmac messages).
>>>>
>>>> Can you take a look at this problem, please?
>>>
>>>
>>> linux-arm-kernel@ stopped my e-mail due to its size. Sending
>>> compressed attachments.
>>>
>>
>> I'm a bit confused by these logs that you provided. Based on the log, there
>> seems to be 3 PCIe root complexes populated on the platform, domain
>> 0000:00:00.0, 0001:00:00:0, 0002:00:00:0 are the 3 root complexes. Root
>> complex 0002 seems to have nothing connected since it does not detect any
>> EP.
>>
>> Is the first card installed on root complex 0 (domain 0000), but the second
>> card and 3rd card are both installed on root complex 1 (domain 0001) on
>> different slots or something?
>>
>> I suspect a potential issue is in this function:
>>
>> 173 static inline bool iproc_pcie_device_is_valid(struct iproc_pcie *pcie,
>> 174                                               unsigned int slot,
>> 175                                               unsigned int fn)
>> 176 {
>> 177         if (slot > 0)
>> 178                 return false;
>> 179
>> 180         /* PAXC can only support limited number of functions */
>> 181         if (pcie->type == IPROC_PCIE_PAXC && fn >= MAX_NUM_PAXC_PF)
>> 182                 return false;
>> 183
>> 184         return true;
>> 185 }
>>
>> Compared to the original code, it's different. Can you try passing bus
>> number into the function as another argument and do the following check on
>> line 177:
>>
>> if (busnum == 0 && slot > 0)
>>      return false;
>>
>> instead of
>>
>> if (slot > 0)
>>      return false;
>
> This did work, thanks! I can see again all these scary
> [Firmware Bug]: reg 0x??: invalid BAR (can't size)
> messages but also all 3 cards were detected!
>
> Will you submit a patch for this, please?
>

Thanks for helping to test.

Let me run a few tests on my platforms today and after that I'll submit 
a fix.

Thanks,

Ray

WARNING: multiple messages have this Message-ID (diff)
From: rjui@broadcom.com (Ray Jui)
To: linux-arm-kernel@lists.infradead.org
Subject: iProc bus scanning regression (after "PCI: iproc: Add PAXC interface support")
Date: Wed, 20 Jan 2016 09:55:06 -0800	[thread overview]
Message-ID: <569FC9FA.4090607@broadcom.com> (raw)
In-Reply-To: <CACna6rxyMoarw=Yq93gijXeXx5bm35hzAs-+9w3SaXg7e1thtQ@mail.gmail.com>

Hi Rafal,

On 1/20/2016 8:43 AM, Rafa? Mi?ecki wrote:
> On 20 January 2016 at 09:13, Ray Jui <rjui@broadcom.com> wrote:
>> On 1/19/2016 11:02 PM, Rafa? Mi?ecki wrote:
>>>
>>> On 20 January 2016 at 07:53, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>>>
>>>> In OpenWrt trunk code we use iProc driver for PCIe controllers on bcma
>>>> bus (PCIE_IPROC_BCMA). Right now we use 4.1 kernel but we backported
>>>> all iProc changes. Unfortunately backporting set queued for 4.5 broke
>>>> bus scanning on some routers.
>>>>
>>>> Most BCM4708 / BCM4709 chipsets have 3 PCIe controllers. All known
>>>> routers use only first 2 of them. Even if router has 3 PCIe cards, the
>>>> last two cards are connected to the 2nd PCIe controller.
>>>>
>>>> So that PAXC patch broke support for routers with 3 cards, 2 of them
>>>> at the 2nd controller. It doesn't affect routers with just 2 cards.
>>>> This problem was tracked down in:
>>>> https://dev.openwrt.org/ticket/21393
>>>>
>>>> I'm attaching two OpenWrt boot logs.
>>>> 1) r48381
>>>> It contains all backported iProc changes. The log looks "nice", but
>>>> only one card (0000:01:00.0) was detected.
>>>> 2) r48382
>>>> It contains "Revert "PCI: iproc: Add PAXC interface support"" patch.
>>>> It contains many "[Firmware Bug]: reg 0x??: invalid BAR (can't size)"
>>>> messages but there are all 3 cards detected: 0000:01:00.0,
>>>> 0001:03:00.0 and 0001:04:00.0 (see brcmfmac messages).
>>>>
>>>> Can you take a look at this problem, please?
>>>
>>>
>>> linux-arm-kernel@ stopped my e-mail due to its size. Sending
>>> compressed attachments.
>>>
>>
>> I'm a bit confused by these logs that you provided. Based on the log, there
>> seems to be 3 PCIe root complexes populated on the platform, domain
>> 0000:00:00.0, 0001:00:00:0, 0002:00:00:0 are the 3 root complexes. Root
>> complex 0002 seems to have nothing connected since it does not detect any
>> EP.
>>
>> Is the first card installed on root complex 0 (domain 0000), but the second
>> card and 3rd card are both installed on root complex 1 (domain 0001) on
>> different slots or something?
>>
>> I suspect a potential issue is in this function:
>>
>> 173 static inline bool iproc_pcie_device_is_valid(struct iproc_pcie *pcie,
>> 174                                               unsigned int slot,
>> 175                                               unsigned int fn)
>> 176 {
>> 177         if (slot > 0)
>> 178                 return false;
>> 179
>> 180         /* PAXC can only support limited number of functions */
>> 181         if (pcie->type == IPROC_PCIE_PAXC && fn >= MAX_NUM_PAXC_PF)
>> 182                 return false;
>> 183
>> 184         return true;
>> 185 }
>>
>> Compared to the original code, it's different. Can you try passing bus
>> number into the function as another argument and do the following check on
>> line 177:
>>
>> if (busnum == 0 && slot > 0)
>>      return false;
>>
>> instead of
>>
>> if (slot > 0)
>>      return false;
>
> This did work, thanks! I can see again all these scary
> [Firmware Bug]: reg 0x??: invalid BAR (can't size)
> messages but also all 3 cards were detected!
>
> Will you submit a patch for this, please?
>

Thanks for helping to test.

Let me run a few tests on my platforms today and after that I'll submit 
a fix.

Thanks,

Ray

  parent reply	other threads:[~2016-01-20 17:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CACna6rx2t7Zg_xSVvYqZTW2dYEgRugaorV58Zy1YZ4Wy=+L4fQ@mail.gmail.com>
2016-01-20  7:02 ` iProc bus scanning regression (after "PCI: iproc: Add PAXC interface support") Rafał Miłecki
2016-01-20  7:02   ` Rafał Miłecki
2016-01-20  8:13   ` Ray Jui
2016-01-20  8:13     ` Ray Jui
2016-01-20  9:04     ` Rafał Miłecki
2016-01-20  9:04       ` Rafał Miłecki
     [not found]     ` <CACna6rxyMoarw=Yq93gijXeXx5bm35hzAs-+9w3SaXg7e1thtQ@mail.gmail.com>
2016-01-20 17:55       ` Ray Jui [this message]
2016-01-20 17:55         ` Ray Jui
2016-01-22 21:31         ` Hauke Mehrtens
2016-01-22 21:31           ` Hauke Mehrtens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=569FC9FA.4090607@broadcom.com \
    --to=rjui@broadcom.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=hauke@hauke-m.de \
    --cc=jonmason@broadcom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=meuleman@broadcom.com \
    --cc=sbranden@broadcom.com \
    --cc=zajec5@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.