linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
@ 2012-09-19  7:23 Minghuan Lian
  2012-09-19 14:22 ` Kumar Gala
  0 siblings, 1 reply; 4+ messages in thread
From: Minghuan Lian @ 2012-09-19  7:23 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Minghuan Lian

The original code uses 'Programming Interface' field to judge if PCIE is
EP or RC mode, however, some latest silicons do not support this functionality.
According to PCIE specification, 'Header Type' offset 0x0e is used to
indicate header type, so change code to use 'Header Type' field to
judge PCIE mode. Because FSL PCI controller does not support 'Header Type',
patch still uses 'Programming Interface' to identify PCI mode.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 arch/powerpc/sysdev/fsl_pci.c |   38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index c37f461..43d30df 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
 
 static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
 {
-	u8 progif;
+	u8 hdr_type;
 
 	/* if we aren't a PCIe don't bother */
 	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
 		return;
 
 	/* if we aren't in host mode don't bother */
-	pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
-	if (progif & 0x1)
+	pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
+	if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
 		return;
 
 	dev->class = PCI_CLASS_BRIDGE_PCI << 8;
@@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 	struct pci_controller *hose;
 	struct resource rsrc;
 	const int *bus_range;
-	u8 progif;
+	u8 hdr_type, progif;
 
 	if (!of_device_is_available(dev)) {
 		pr_warning("%s: disabled\n", dev->full_name);
@@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
 		PPC_INDIRECT_TYPE_BIG_ENDIAN);
 
-	early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
-	if ((progif & 1) == 1) {
-		/* unmap cfg_data & cfg_addr separately if not on same page */
-		if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
-		    ((unsigned long)hose->cfg_addr & PAGE_MASK))
-			iounmap(hose->cfg_data);
-		iounmap(hose->cfg_addr);
-		pcibios_free_controller(hose);
-		return -ENODEV;
-	}
-
 	setup_pci_cmd(hose);
 
 	/* check PCI express link status */
 	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
+		/* For PCIE read HEADER_TYPE to identify controler mode */
+		early_read_config_byte(hose, 0, 0, PCI_HEADER_TYPE, &hdr_type);
+		if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
+			goto no_bridge;
+
 		hose->indirect_type |= PPC_INDIRECT_TYPE_EXT_REG |
 			PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS;
 		if (fsl_pcie_check_link(hose))
 			hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+	} else {
+		/* For PCI read PROG to identify controller mode */
+		early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
+		if ((progif & 1) == 1)
+			goto no_bridge;
 	}
 
 	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
@@ -494,6 +493,15 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 	setup_pci_atmu(hose, &rsrc);
 
 	return 0;
+
+no_bridge:
+	/* unmap cfg_data & cfg_addr separately if not on same page */
+	if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
+	    ((unsigned long)hose->cfg_addr & PAGE_MASK))
+		iounmap(hose->cfg_data);
+	iounmap(hose->cfg_addr);
+	pcibios_free_controller(hose);
+	return -ENODEV;
 }
 #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx */
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
  2012-09-19  7:23 [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode Minghuan Lian
@ 2012-09-19 14:22 ` Kumar Gala
  2012-09-21  3:37   ` Lian Minghaun-b31939
  0 siblings, 1 reply; 4+ messages in thread
From: Kumar Gala @ 2012-09-19 14:22 UTC (permalink / raw)
  To: Minghuan Lian; +Cc: linuxppc-dev


On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:

> The original code uses 'Programming Interface' field to judge if PCIE =
is
> EP or RC mode, however, some latest silicons do not support this =
functionality.
> According to PCIE specification, 'Header Type' offset 0x0e is used to
> indicate header type, so change code to use 'Header Type' field to
> judge PCIE mode. Because FSL PCI controller does not support 'Header =
Type',
> patch still uses 'Programming Interface' to identify PCI mode.
>=20
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> arch/powerpc/sysdev/fsl_pci.c |   38 =
+++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>=20
> diff --git a/arch/powerpc/sysdev/fsl_pci.c =
b/arch/powerpc/sysdev/fsl_pci.c
> index c37f461..43d30df 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>=20
> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
> {
> -	u8 progif;
> +	u8 hdr_type;
>=20
> 	/* if we aren't a PCIe don't bother */
> 	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
> 		return;
>=20
> 	/* if we aren't in host mode don't bother */
> -	pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
> -	if (progif & 0x1)
> +	pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
> +	if ((hdr_type & 0x7f) !=3D PCI_HEADER_TYPE_BRIDGE)
> 		return;
>=20
> 	dev->class =3D PCI_CLASS_BRIDGE_PCI << 8;
> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node *dev, =
int is_primary)
> 	struct pci_controller *hose;
> 	struct resource rsrc;
> 	const int *bus_range;
> -	u8 progif;
> +	u8 hdr_type, progif;
>=20
> 	if (!of_device_is_available(dev)) {
> 		pr_warning("%s: disabled\n", dev->full_name);
> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
> 	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
> 		PPC_INDIRECT_TYPE_BIG_ENDIAN);
>=20
> -	early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
> -	if ((progif & 1) =3D=3D 1) {
> -		/* unmap cfg_data & cfg_addr separately if not on same =
page */
> -		if (((unsigned long)hose->cfg_data & PAGE_MASK) !=3D
> -		    ((unsigned long)hose->cfg_addr & PAGE_MASK))
> -			iounmap(hose->cfg_data);
> -		iounmap(hose->cfg_addr);
> -		pcibios_free_controller(hose);
> -		return -ENODEV;
> -	}
> -
> 	setup_pci_cmd(hose);

I think we should be doing the check before we call setup_pci_cmd().  =
The old code didn't touch the controller registers if we where and =
end-point.  We should maintain that.

>=20
> 	/* check PCI express link status */
> 	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
> +		/* For PCIE read HEADER_TYPE to identify controler mode =
*/
> +		early_read_config_byte(hose, 0, 0, PCI_HEADER_TYPE, =
&hdr_type);
> +		if ((hdr_type & 0x7f) !=3D PCI_HEADER_TYPE_BRIDGE)
> +			goto no_bridge;
> +
> 		hose->indirect_type |=3D PPC_INDIRECT_TYPE_EXT_REG |
> 			PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS;
> 		if (fsl_pcie_check_link(hose))
> 			hose->indirect_type |=3D =
PPC_INDIRECT_TYPE_NO_PCIE_LINK;
> +	} else {
> +		/* For PCI read PROG to identify controller mode */
> +		early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, =
&progif);
> +		if ((progif & 1) =3D=3D 1)
> +			goto no_bridge;
> 	}
>=20
> 	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
> @@ -494,6 +493,15 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
> 	setup_pci_atmu(hose, &rsrc);
>=20
> 	return 0;
> +
> +no_bridge:
> +	/* unmap cfg_data & cfg_addr separately if not on same page */
> +	if (((unsigned long)hose->cfg_data & PAGE_MASK) !=3D
> +	    ((unsigned long)hose->cfg_addr & PAGE_MASK))
> +		iounmap(hose->cfg_data);
> +	iounmap(hose->cfg_addr);
> +	pcibios_free_controller(hose);
> +	return -ENODEV;
> }
> #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx */
>=20
> --=20
> 1.7.9.5
>=20

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
  2012-09-19 14:22 ` Kumar Gala
@ 2012-09-21  3:37   ` Lian Minghaun-b31939
  2012-09-21 13:06     ` Kumar Gala
  0 siblings, 1 reply; 4+ messages in thread
From: Lian Minghaun-b31939 @ 2012-09-21  3:37 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Minghuan Lian, linuxppc-dev

Hi Kumar,

please see my comments inline.


On 09/19/2012 10:22 PM, Kumar Gala wrote:
> On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:
>
>> The original code uses 'Programming Interface' field to judge if PCIE is
>> EP or RC mode, however, some latest silicons do not support this functionality.
>> According to PCIE specification, 'Header Type' offset 0x0e is used to
>> indicate header type, so change code to use 'Header Type' field to
>> judge PCIE mode. Because FSL PCI controller does not support 'Header Type',
>> patch still uses 'Programming Interface' to identify PCI mode.
>>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
>> ---
>> arch/powerpc/sysdev/fsl_pci.c |   38 +++++++++++++++++++++++---------------
>> 1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>> index c37f461..43d30df 100644
>> --- a/arch/powerpc/sysdev/fsl_pci.c
>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>>
>> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
>> {
>> -	u8 progif;
>> +	u8 hdr_type;
>>
>> 	/* if we aren't a PCIe don't bother */
>> 	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>> 		return;
>>
>> 	/* if we aren't in host mode don't bother */
>> -	pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
>> -	if (progif & 0x1)
>> +	pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
>> +	if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
>> 		return;
>>
>> 	dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> 	struct pci_controller *hose;
>> 	struct resource rsrc;
>> 	const int *bus_range;
>> -	u8 progif;
>> +	u8 hdr_type, progif;
>>
>> 	if (!of_device_is_available(dev)) {
>> 		pr_warning("%s: disabled\n", dev->full_name);
>> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> 	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
>> 		PPC_INDIRECT_TYPE_BIG_ENDIAN);
>>
>> -	early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>> -	if ((progif & 1) == 1) {
>> -		/* unmap cfg_data & cfg_addr separately if not on same page */
>> -		if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
>> -		    ((unsigned long)hose->cfg_addr & PAGE_MASK))
>> -			iounmap(hose->cfg_data);
>> -		iounmap(hose->cfg_addr);
>> -		pcibios_free_controller(hose);
>> -		return -ENODEV;
>> -	}
>> -
>> 	setup_pci_cmd(hose);
> I think we should be doing the check before we call setup_pci_cmd().  The old code didn't touch the controller registers if we where and end-point.  We should maintain that.
[Minghuan] Thanks for you pointing this.
I want to move setup_pci_cmd like this:

pr_debug(" ->Hose at 0x%p, cfg_addr=0x%p,cfg_data=0x%p\n",
         hose, hose->cfg_addr, hose->cfg_data);

+    setup_pci_cmd(hose);

     /* Interpret the "ranges" property */
     /* This also maps the I/O region and sets isa_io/mem_base */
     pci_process_bridge_OF_ranges(hose, dev, is_primary);

This movement will cause fsl_pcie_check_link() calling before 
setup_pci_cmd().
Is this ok?
If not, I will call setup_pci_cmd() for PCI and PCIE respectively.

>> 	/* check PCI express link status */
>> 	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
>> +		/* For PCIE read HEADER_TYPE to identify controler mode */
>> +		early_read_config_byte(hose, 0, 0, PCI_HEADER_TYPE, &hdr_type);
>> +		if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
>> +			goto no_bridge;
>> +
>> 		hose->indirect_type |= PPC_INDIRECT_TYPE_EXT_REG |
>> 			PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS;
>> 		if (fsl_pcie_check_link(hose))
>> 			hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
>> +	} else {
>> +		/* For PCI read PROG to identify controller mode */
>> +		early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>> +		if ((progif & 1) == 1)
>> +			goto no_bridge;
>> 	}
>>
>> 	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
>> @@ -494,6 +493,15 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
>> 	setup_pci_atmu(hose, &rsrc);
>>
>> 	return 0;
>> +
>> +no_bridge:
>> +	/* unmap cfg_data & cfg_addr separately if not on same page */
>> +	if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
>> +	    ((unsigned long)hose->cfg_addr & PAGE_MASK))
>> +		iounmap(hose->cfg_data);
>> +	iounmap(hose->cfg_addr);
>> +	pcibios_free_controller(hose);
>> +	return -ENODEV;
>> }
>> #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx */
>>
>> -- 
>> 1.7.9.5
>>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode
  2012-09-21  3:37   ` Lian Minghaun-b31939
@ 2012-09-21 13:06     ` Kumar Gala
  0 siblings, 0 replies; 4+ messages in thread
From: Kumar Gala @ 2012-09-21 13:06 UTC (permalink / raw)
  To: Lian Minghaun-b31939; +Cc: Minghuan Lian, linuxppc-dev


On Sep 20, 2012, at 10:37 PM, Lian Minghaun-b31939 wrote:

> Hi Kumar,
>=20
> please see my comments inline.
>=20
>=20
> On 09/19/2012 10:22 PM, Kumar Gala wrote:
>> On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:
>>=20
>>> The original code uses 'Programming Interface' field to judge if =
PCIE is
>>> EP or RC mode, however, some latest silicons do not support this =
functionality.
>>> According to PCIE specification, 'Header Type' offset 0x0e is used =
to
>>> indicate header type, so change code to use 'Header Type' field to
>>> judge PCIE mode. Because FSL PCI controller does not support 'Header =
Type',
>>> patch still uses 'Programming Interface' to identify PCI mode.
>>>=20
>>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>>> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
>>> ---
>>> arch/powerpc/sysdev/fsl_pci.c |   38 =
+++++++++++++++++++++++---------------
>>> 1 file changed, 23 insertions(+), 15 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/sysdev/fsl_pci.c =
b/arch/powerpc/sysdev/fsl_pci.c
>>> index c37f461..43d30df 100644
>>> --- a/arch/powerpc/sysdev/fsl_pci.c
>>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>>> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>>>=20
>>> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
>>> {
>>> -	u8 progif;
>>> +	u8 hdr_type;
>>>=20
>>> 	/* if we aren't a PCIe don't bother */
>>> 	if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>>> 		return;
>>>=20
>>> 	/* if we aren't in host mode don't bother */
>>> -	pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
>>> -	if (progif & 0x1)
>>> +	pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
>>> +	if ((hdr_type & 0x7f) !=3D PCI_HEADER_TYPE_BRIDGE)
>>> 		return;
>>>=20
>>> 	dev->class =3D PCI_CLASS_BRIDGE_PCI << 8;
>>> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
>>> 	struct pci_controller *hose;
>>> 	struct resource rsrc;
>>> 	const int *bus_range;
>>> -	u8 progif;
>>> +	u8 hdr_type, progif;
>>>=20
>>> 	if (!of_device_is_available(dev)) {
>>> 		pr_warning("%s: disabled\n", dev->full_name);
>>> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node =
*dev, int is_primary)
>>> 	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
>>> 		PPC_INDIRECT_TYPE_BIG_ENDIAN);
>>>=20
>>> -	early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>>> -	if ((progif & 1) =3D=3D 1) {
>>> -		/* unmap cfg_data & cfg_addr separately if not on same =
page */
>>> -		if (((unsigned long)hose->cfg_data & PAGE_MASK) !=3D
>>> -		    ((unsigned long)hose->cfg_addr & PAGE_MASK))
>>> -			iounmap(hose->cfg_data);
>>> -		iounmap(hose->cfg_addr);
>>> -		pcibios_free_controller(hose);
>>> -		return -ENODEV;
>>> -	}
>>> -
>>> 	setup_pci_cmd(hose);
>> I think we should be doing the check before we call setup_pci_cmd().  =
The old code didn't touch the controller registers if we where and =
end-point.  We should maintain that.
> [Minghuan] Thanks for you pointing this.
> I want to move setup_pci_cmd like this:
>=20
> pr_debug(" ->Hose at 0x%p, cfg_addr=3D0x%p,cfg_data=3D0x%p\n",
>        hose, hose->cfg_addr, hose->cfg_data);
>=20
> +    setup_pci_cmd(hose);
>=20
>    /* Interpret the "ranges" property */
>    /* This also maps the I/O region and sets isa_io/mem_base */
>    pci_process_bridge_OF_ranges(hose, dev, is_primary);
>=20
> This movement will cause fsl_pcie_check_link() calling before =
setup_pci_cmd().
> Is this ok?

I think so, as its how the code is today:

setup_pci_cmd()
..
if (pcie)
	fsl_pcie_check_link()


> If not, I will call setup_pci_cmd() for PCI and PCIE respectively.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-09-21 13:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19  7:23 [PATCH] powerpc/fsl-pci: use 'Header Type' to identify PCIE mode Minghuan Lian
2012-09-19 14:22 ` Kumar Gala
2012-09-21  3:37   ` Lian Minghaun-b31939
2012-09-21 13:06     ` Kumar Gala

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).