[v3,12/26] PCI: keystone: Prevent ARM32 specific code to be compiled for ARM64

Message ID 20190325093947.32633-13-kishon@ti.com
State Accepted
Commit f316a2b53cd7f37963ae20ec7072eb27a349a4ce
Headers show
Series
  • Untitled series #19461
Related show

Commit Message

Kishon Vijay Abraham I March 25, 2019, 9:39 a.m.
hook_fault_code is an ARM32 specific API for hooking into data abort.
Since pci-keystone.c will be used for AM65X platforms which is an
ARM64 platform, allow hook_fault_code to be compiled only for ARM32.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.17.1

Comments

Kishon Vijay Abraham I April 12, 2019, 11:02 a.m. | #1
Hi Russell,

On 12/04/19 3:01 PM, Russell King - ARM Linux admin wrote:
> On Fri, Apr 12, 2019 at 02:20:06PM +0530, Kishon Vijay Abraham I wrote:

>> Hi Lorenzo,

>>

>> On 11/04/19 8:33 PM, Lorenzo Pieralisi wrote:

>>> On Mon, Mar 25, 2019 at 03:09:33PM +0530, Kishon Vijay Abraham I wrote:

>>>> hook_fault_code is an ARM32 specific API for hooking into data abort.

>>>> Since pci-keystone.c will be used for AM65X platforms which is an

>>>> ARM64 platform,

>>>

>>> Hi Kishon,

>>>

>>> How is the problem plugged by the fault hook fixed on ARM64 ?

>>

>> At least in AM654 platform, I don't see a bus error when PCIe device is not

>> connected but returns 0xffffffff. So there is no necessary for hook_fault_code

>> in AM654 platform.

> 

> ... which needs to be explained somewhere, either in the commit

> message or comments in the file, so that years later when someone

> has to look at this code, they know why it is the way that it is.

> 

> When writing commits, please think about whether there is enough

> information to understand the change in ten years time.


sure, I'll fix this one.

Lorenzo, let me know if you want me to resend with updated commit log.

Thanks
Kishon

> 

>>

>> Thanks

>> Kishon

>>

>>>

>>> Thanks,

>>> Lorenzo

>>>

>>>> allow hook_fault_code to be compiled only for ARM32.

>>>>

>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>>>> ---

>>>>  drivers/pci/controller/dwc/pci-keystone.c | 4 ++++

>>>>  1 file changed, 4 insertions(+)

>>>>

>>>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c

>>>> index dfe54553d832..93296d434f40 100644

>>>> --- a/drivers/pci/controller/dwc/pci-keystone.c

>>>> +++ b/drivers/pci/controller/dwc/pci-keystone.c

>>>> @@ -710,6 +710,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)

>>>>  	return ret;

>>>>  }

>>>>  

>>>> +#ifdef CONFIG_ARM

>>>>  /*

>>>>   * When a PCI device does not exist during config cycles, keystone host gets a

>>>>   * bus error instead of returning 0xffffffff. This handler always returns 0

>>>> @@ -729,6 +730,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,

>>>>  

>>>>  	return 0;

>>>>  }

>>>> +#endif

>>>>  

>>>>  static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)

>>>>  {

>>>> @@ -778,12 +780,14 @@ static int __init ks_pcie_host_init(struct pcie_port *pp)

>>>>  	if (ret < 0)

>>>>  		return ret;

>>>>  

>>>> +#ifdef CONFIG_ARM

>>>>  	/*

>>>>  	 * PCIe access errors that result into OCP errors are caught by ARM as

>>>>  	 * "External aborts"

>>>>  	 */

>>>>  	hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,

>>>>  			"Asynchronous external abort");

>>>> +#endif

>>>>  

>>>>  	ks_pcie_start_link(pci);

>>>>  	dw_pcie_wait_for_link(pci);

>>>> -- 

>>>> 2.17.1

>>>>

>>

>> _______________________________________________

>> linux-arm-kernel mailing list

>> linux-arm-kernel@lists.infradead.org

>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

>>

>
Kishon Vijay Abraham I April 12, 2019, 11:29 a.m. | #2
Hi Lorenzo,

On 12/04/19 4:41 PM, Lorenzo Pieralisi wrote:
> On Fri, Apr 12, 2019 at 02:20:06PM +0530, Kishon Vijay Abraham I wrote:

>> Hi Lorenzo,

>>

>> On 11/04/19 8:33 PM, Lorenzo Pieralisi wrote:

>>> On Mon, Mar 25, 2019 at 03:09:33PM +0530, Kishon Vijay Abraham I wrote:

>>>> hook_fault_code is an ARM32 specific API for hooking into data abort.

>>>> Since pci-keystone.c will be used for AM65X platforms which is an

>>>> ARM64 platform,

>>>

>>> Hi Kishon,

>>>

>>> How is the problem plugged by the fault hook fixed on ARM64 ?

>>

>> At least in AM654 platform, I don't see a bus error when PCIe device

>> is not connected but returns 0xffffffff. So there is no necessary for

>> hook_fault_code in AM654 platform.

> 

> That can't have much to do with ARM32<->ARM64, it is rather a platform

> integration issue AFAICS. Russell has a point, this has to be documented

> I can do it for you but I need additional information.


Right now only ARM32 exports hook_fault_code which was used by K2G (ARM32
platform) and since AM654 (ARM64 platform) uses the same driver, it'll result
in compilation error. Also AM654 doesn't require hook_fault_code (or something
similar) because it doesn't throw bus error on accessing PCIe address space
when PCI device is not connected).

Are you looking for some other information?

Thanks
Kishon
> 

> Thanks,

> Lorenzo

> 

>> Thanks

>> Kishon

>>

>>>

>>> Thanks,

>>> Lorenzo

>>>

>>>> allow hook_fault_code to be compiled only for ARM32.

>>>>

>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>>>> ---

>>>>  drivers/pci/controller/dwc/pci-keystone.c | 4 ++++

>>>>  1 file changed, 4 insertions(+)

>>>>

>>>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c

>>>> index dfe54553d832..93296d434f40 100644

>>>> --- a/drivers/pci/controller/dwc/pci-keystone.c

>>>> +++ b/drivers/pci/controller/dwc/pci-keystone.c

>>>> @@ -710,6 +710,7 @@ static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)

>>>>  	return ret;

>>>>  }

>>>>  

>>>> +#ifdef CONFIG_ARM

>>>>  /*

>>>>   * When a PCI device does not exist during config cycles, keystone host gets a

>>>>   * bus error instead of returning 0xffffffff. This handler always returns 0

>>>> @@ -729,6 +730,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,

>>>>  

>>>>  	return 0;

>>>>  }

>>>> +#endif

>>>>  

>>>>  static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)

>>>>  {

>>>> @@ -778,12 +780,14 @@ static int __init ks_pcie_host_init(struct pcie_port *pp)

>>>>  	if (ret < 0)

>>>>  		return ret;

>>>>  

>>>> +#ifdef CONFIG_ARM

>>>>  	/*

>>>>  	 * PCIe access errors that result into OCP errors are caught by ARM as

>>>>  	 * "External aborts"

>>>>  	 */

>>>>  	hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,

>>>>  			"Asynchronous external abort");

>>>> +#endif

>>>>  

>>>>  	ks_pcie_start_link(pci);

>>>>  	dw_pcie_wait_for_link(pci);

>>>> -- 

>>>> 2.17.1

>>>>

Patch

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index dfe54553d832..93296d434f40 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -710,6 +710,7 @@  static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
 	return ret;
 }
 
+#ifdef CONFIG_ARM
 /*
  * When a PCI device does not exist during config cycles, keystone host gets a
  * bus error instead of returning 0xffffffff. This handler always returns 0
@@ -729,6 +730,7 @@  static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
 
 	return 0;
 }
+#endif
 
 static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 {
@@ -778,12 +780,14 @@  static int __init ks_pcie_host_init(struct pcie_port *pp)
 	if (ret < 0)
 		return ret;
 
+#ifdef CONFIG_ARM
 	/*
 	 * PCIe access errors that result into OCP errors are caught by ARM as
 	 * "External aborts"
 	 */
 	hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,
 			"Asynchronous external abort");
+#endif
 
 	ks_pcie_start_link(pci);
 	dw_pcie_wait_for_link(pci);