[PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing

[Scott Branden]

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 at 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 at 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