From mboxrd@z Thu Jan 1 00:00:00 1970 From: scott.branden@broadcom.com (Scott Branden) Date: Mon, 11 Apr 2016 15:26:59 -0700 Subject: [PATCH 1/2] PCI: iproc: Support DT property for ignoring aborts when probing In-Reply-To: References: <1460238624-2086-1-git-send-email-zajec5@gmail.com> <79DC79A7-D4CE-4983-B1C5-CBD2E9CBBFB9@gmail.com> <4768648c-841b-490d-a752-f31cba545f74@broadcom.com> <570C1D47.7010102@gmail.com> Message-ID: <570C24B3.4080104@broadcom.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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" >>>> 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 >>>>> --- >>>> >>>>> +- 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