linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: scott.branden@broadcom.com (Scott Branden)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: BCM5301X: Add back handler ignoring external imprecise aborts
Date: Mon, 31 Oct 2016 15:05:35 -0700	[thread overview]
Message-ID: <3847db9f-a8e1-b7fe-c2fa-19ea893bae5f@broadcom.com> (raw)
In-Reply-To: <18db27c7-1055-ff3c-0b0b-eeeaf5f3f5e0@gmail.com>



On 16-10-31 02:56 PM, Florian Fainelli wrote:
> On 10/31/2016 02:46 PM, Scott Branden wrote:
>>
>>
>> On 16-10-31 02:01 PM, Florian Fainelli wrote:
>>> On 10/31/2016 01:59 PM, Hauke Mehrtens wrote:
>>>>
>>>>
>>>> On 10/31/2016 07:08 PM, Scott Branden wrote:
>>>>> Hi Rafal,
>>>>>
>>>>>
>>>>> On 16-10-29 04:12 AM, Rafa? Mi?ecki wrote:
>>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>>
>>>>>> Since early BCM5301X days we got abort handler that was removed by
>>>>>> commit 937b12306ea79 ("ARM: BCM5301X: remove workaround imprecise
>>>>>> abort
>>>>>> fault handler"). It assumed we need to deal only with pending aborts
>>>>>> left by the bootloader. Unfortunately this isn't true for BCM5301X.
>>>>>>
>>>>>> When probing PCI config space (device enumeration) it is expected to
>>>>>> have master aborts on the PCI bus. Most bridges don't forward (or they
>>>>>> allow disabling it) these errors onto the AXI/AMBA bus but not the
>>>>>> Northstar (BCM5301X) one.
>>>>> Should we only add this workaround code if CONFIG_PCI is on then?
>>>>
>>>> I think all the supported northstar devices have a PCIe controller. We
>>>> could add such a CONFIG_PCI check, but I do not see a big advantage.
>>>
>>> Actually, I do see a couple disadvantages if we gate this with
>>> CONFIG_PCI: if this problem shows up irrespective of your kernel
>>> configuration, you want the error handler to clear it, not rely on
>>> CONFIG_PCI to be enabled for the error to go away and also, without an
>>> additional ifdef, additional compiler coverage.
>>>
>> A problem with reintroducing this change is that all imprecise data
>> aborts are ignored, not just PCI.  So if you don't actually use PCI in
>> your system and want to debug other aborts you are unable to.  I don't
>> know if we care about such situation.
>
> Considering that any abort is pretty much fatal, the options are:
>
> - update the freaking bootloader to a version where there are no such
> aborts generated, not an option on consumer devices, unclear which
> version (if any) fixes that
>
> - fixups the aborts externally, via a boot wrapper, which is going to
> take some time to develop, causes additional burden on the distributors
> to provide instructions/build images on how to do it
I think the abort is already fixed in the kernel now on boot so option 1 
and 2 were not needed - and abort handler was removed as detailed by 
Rafal in the commit.  Only outstanding issue is now this new PCI 
enumeration issue.
>
> - fixups the aborts in the kernel, irrespective of where they come from,
> simple and easy
>
> - fixups the aborts in the kernel, look where they come from, by using
> some bit of magic, looking at PCIe registers and whatnot (provided that
> is even possible), not too hard, but can take a while, and is error prone
Option 4 sounds like the proper solution - check the range the abort is 
due to the PCI device enumeration and only ignore those aborts.
>
> I can certainly advocate that option 3 is the one that gives a working
> device, and this is what matters from a distribution perspective like
> LEDE/OpenWrt.
>
Since I don't use BCM5301x option 3 is fine by me.

  parent reply	other threads:[~2016-10-31 22:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-29 11:12 [PATCH] ARM: BCM5301X: Add back handler ignoring external imprecise aborts Rafał Miłecki
2016-10-31 18:08 ` Scott Branden
2016-10-31 20:59   ` Hauke Mehrtens
2016-10-31 21:01     ` Florian Fainelli
2016-10-31 21:46       ` Scott Branden
2016-10-31 21:56         ` Florian Fainelli
2016-10-31 22:04           ` Rafał Miłecki
2016-10-31 22:05           ` Scott Branden [this message]
2016-10-31 22:16             ` Rafał Miłecki
2016-10-31 22:28               ` Ray Jui

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=3847db9f-a8e1-b7fe-c2fa-19ea893bae5f@broadcom.com \
    --to=scott.branden@broadcom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).