From: scott.branden@broadcom•com (Scott Branden)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing
Date: Mon, 11 Apr 2016 15:26:59 -0700 [thread overview]
Message-ID: <570C24B3.4080104@broadcom.com> (raw)
In-Reply-To: <ccabf53c-c035-be4b-a016-389bb7531557@broadcom.com>
One Comment below
On 16-04-11 03:24 PM, Ray Jui wrote:
>
>
> On 4/11/2016 2:55 PM, Florian Fainelli wrote:
>> On 11/04/16 13:06, Ray Jui wrote:
>>>
>>>
>>> On 4/10/2016 6:43 PM, Florian Fainelli wrote:
>>>> On April 9, 2016 2:50:23 PM PDT, "Rafa? Mi?ecki" <zajec5@gmail•com>
>>>> wrote:
>>>>> Some devices (e.g. Northstar ones) may have bridges that forward
>>>>> harmless errors to the ARM core. In such case we need an option to
>>>>> add a handler ignoring them.
>>>>>
>>>>> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail•com>
>>>>> ---
>>>>
>>>>> +- brcm,pcie-hook-abort-handler: During PCI bus probing (device
>>>>> enumeration)
>>>>> + there can be errors that are expected and harmless. Unfortunately
>>>>> some bridges
>>>>> + can't be configured to ignore them and they forward them to the ARM
>>>>> core
>>>>> + triggering die().
>>>>> + This property should be set in such case, it will make driver add
>>>>> its own
>>>>> + handler ignoring such errors.
>>>>
>>>> The property is named after the function that allows you to catch
>>>> abort handlers, whereas you should be describing the HW here.
>>>> Something like brcm,bridge-error-forward or a better name even would
>>>> be preferable.
>>>>
>>>>> +#ifdef CONFIG_ARM
>>>>> +static int iproc_pcie_abort_handler(unsigned long addr, unsigned int
>>>>> fsr,
>>>>> + struct pt_regs *regs)
>>>>> +{
>>>>> + if (fsr == 0x1406)
>>>>> + return 0;
>>>>> +
>>>>> + return 1;
>>>>
>>>> As you later noted this prevents this driver from being a module now.
>>>> Since the expectation is that either a fixed bootloader or a platform
>>>> should enot produce these data aborts, or allow them to be ignored,
>>>> why not just put this code back where it belongs in the machine
>>>> specific file which kills many birds with the same stone:
>>>>
>>>
>>> I assume the module compile issue can be simply fixed by exporting
>>> symbol of "hook_fault_code"?
>>
>> I do not think it is desireable for this symbol to be exported in the
>> first place, also last I looked, this was a one time registration thing,
>> you cannot undo the hook you installed, but everything can be fixed.
>>
>
> Okay.
>
>>>
>>> I believe ARM64 based NS2 that uses the same iProc PCIe driver might
>>> also need something like this (I'm still waiting for Jon Mason to give
>>> me some more information on the NS2 errors that he saw, which is likely
>>> related to this). I assume there will be something similar to the ARM
>>> specific "hook_fault_code" for ARM64, but then ARM64 does not have any
>>> "mach" specific directory. Where can this type of hook be installed for
>>> ARM64 based platforms if not in the PCIe driver?
>>
>> OK, that is a fair point, then maybe have a two stage process, where we
>> make sure that the part that installs the hook is always available and
>> built-into the kernel, but let the iProc PCIe driver remain a module?
>>
>
> I guess we are sort of stuck on this. If "hook_fault_code" is not
> supposed to be used by any driver compiled as module like you described,
> then yes, I agree, I don't see how we can leave this in the iProc PCIe
> driver.
>
Why does thie PCI driver need to be compiled as module though? Why
can't we limit it to being linked in the kernel?
Regards,
Scott
next prev parent reply other threads:[~2016-04-11 22:26 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-09 21:50 [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing Rafał Miłecki
2016-04-09 21:50 ` [PATCH 2/2] PCI: iproc: Enable hooking abort handler on devices with bcma Rafał Miłecki
2016-04-10 2:59 ` [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing kbuild test robot
2016-04-10 10:54 ` Rafał Miłecki
2016-04-11 1:43 ` Florian Fainelli
2016-04-11 20:06 ` Ray Jui
2016-04-11 21:55 ` Florian Fainelli
2016-04-11 22:24 ` Ray Jui
2016-04-11 22:26 ` Scott Branden [this message]
2016-04-11 22:34 ` Ray Jui
2016-04-11 22:41 ` Florian Fainelli
2016-04-11 22:51 ` Ray Jui
2016-04-11 22:51 ` Florian Fainelli
2016-04-17 15:54 ` Rafał Miłecki
2016-04-17 14:02 ` Arnd Bergmann
2016-04-18 17:47 ` Ray Jui
2016-04-20 18:18 ` Ray Jui
2016-10-28 15:31 ` Rafał Miłecki
2016-10-28 16:58 ` Ray Jui
2016-10-28 17:04 ` Florian Fainelli
2016-10-29 6:14 ` Rafał Miłecki
2016-04-11 8:57 ` Mark Rutland
2016-04-17 15:43 ` Rafał Miłecki
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=570C24B3.4080104@broadcom.com \
--to=scott.branden@broadcom$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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