Patchwork [03/12,v3] pci: fsl: add PCI indirect access support

login
register
mail settings
Submitter Minghuan Lian
Date Oct. 23, 2013, 10:41 a.m.
Message ID <1382524894-15164-3-git-send-email-Minghuan.Lian@freescale.com>
Download mbox | patch
Permalink /patch/285634/
State Superseded
Delegated to: Scott Wood
Headers show

Comments

Minghuan Lian - Oct. 23, 2013, 10:41 a.m.
The patch adds PCI indirect read/write functions. The main code
is ported from arch/powerpc/sysdev/indirect_pci.c. We use general
IO API iowrite32be/ioread32be instead of out_be32/in_be32, and
use structure fsl_Pci instead of PowerPC's pci_controller.
The patch also provides fsl_pcie_check_link() to check PCI link.
The weak function fsl_arch_pci_exclude_device() is provided to
call ppc_md.pci_exclude_device() for PowerPC architecture.

Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
---
change log:
v1-v3:
Derived from http://patchwork.ozlabs.org/patch/278965/

Based on upstream master.
Based on the discussion of RFC version here
http://patchwork.ozlabs.org/patch/274487/

 drivers/pci/host/pci-fsl-common.c | 169 ++++++++++++++++++++++++++++++++------
 include/linux/fsl/pci-common.h    |   6 ++
 2 files changed, 151 insertions(+), 24 deletions(-)
Scott Wood - Jan. 3, 2014, 10:33 p.m.
On Wed, Oct 23, 2013 at 06:41:25PM +0800, Minghuan Lian wrote:
> The patch adds PCI indirect read/write functions. The main code
> is ported from arch/powerpc/sysdev/indirect_pci.c. We use general
> IO API iowrite32be/ioread32be instead of out_be32/in_be32, and
> use structure fsl_Pci instead of PowerPC's pci_controller.
> The patch also provides fsl_pcie_check_link() to check PCI link.
> The weak function fsl_arch_pci_exclude_device() is provided to
> call ppc_md.pci_exclude_device() for PowerPC architecture.
> 
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> 
> ---
> change log:
> v1-v3:
> Derived from http://patchwork.ozlabs.org/patch/278965/
> 
> Based on upstream master.
> Based on the discussion of RFC version here
> http://patchwork.ozlabs.org/patch/274487/
> 
>  drivers/pci/host/pci-fsl-common.c | 169 ++++++++++++++++++++++++++++++++------
>  include/linux/fsl/pci-common.h    |   6 ++
>  2 files changed, 151 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-fsl-common.c b/drivers/pci/host/pci-fsl-common.c
> index 69d338b..8bc9a64 100644
> --- a/drivers/pci/host/pci-fsl-common.c
> +++ b/drivers/pci/host/pci-fsl-common.c
> @@ -35,52 +35,173 @@
>  #include <sysdev/fsl_soc.h>
>  #include <sysdev/fsl_pci.h>
>  
> -static int fsl_pcie_check_link(struct pci_controller *hose)
> +/* Indirect type */
> +#define INDIRECT_TYPE_EXT_REG			0x00000002
> +#define INDIRECT_TYPE_SURPRESS_PRIMARY_BUS	0x00000004
> +#define INDIRECT_TYPE_NO_PCIE_LINK		0x00000008
> +#define INDIRECT_TYPE_BIG_ENDIAN		0x00000010
> +#define INDIRECT_TYPE_FSL_CFG_REG_LINK		0x00000040

Why are these here rather than in the header, given that you have
indirect_type in the struct in the header?

> +int __weak fsl_arch_pci_exclude_device(struct fsl_pci *pci, u8 bus, u8 devfn)
> +{
> +	return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int fsl_pci_read_config(struct fsl_pci *pci, int bus, int devfn,
> +				int offset, int len, u32 *val)
> +{
> +	u32 bus_no, reg, data;
> +
> +	if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
> +		if (bus != pci->first_busno)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +		if (devfn != 0)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +	}

A lot of this seems duplicated from arch/powerpc/sysdev/indirect_pci.c.

How generally applicable is that file to non-PPC implementations?  At a
minimum I see a similar file in arch/microblaze.  It should probably
eventually be moved to common code, rather than duplicated again.  A
prerequisite for that would be making common the dependencies it has on
the rest of what is currently arch PCI infrastructure; until then, it's
probably better to just have the common fsl-pci code know how to
interface with the appropriate PPC/ARM code rather than trying to copy
the infrastructure as well.

-Scott
Lian Minghaun-b31939 - Jan. 6, 2014, 5:36 a.m.
HI Scott,

please see my comments inline.

On 01/04/2014 06:33 AM, Scott Wood wrote:
> On Wed, Oct 23, 2013 at 06:41:25PM +0800, Minghuan Lian wrote:
>> The patch adds PCI indirect read/write functions. The main code
>> is ported from arch/powerpc/sysdev/indirect_pci.c. We use general
>> IO API iowrite32be/ioread32be instead of out_be32/in_be32, and
>> use structure fsl_Pci instead of PowerPC's pci_controller.
>> The patch also provides fsl_pcie_check_link() to check PCI link.
>> The weak function fsl_arch_pci_exclude_device() is provided to
>> call ppc_md.pci_exclude_device() for PowerPC architecture.
>>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>>
>> ---
>> change log:
>> v1-v3:
>> Derived from http://patchwork.ozlabs.org/patch/278965/
>>
>> Based on upstream master.
>> Based on the discussion of RFC version here
>> http://patchwork.ozlabs.org/patch/274487/
>>
>>   drivers/pci/host/pci-fsl-common.c | 169 ++++++++++++++++++++++++++++++++------
>>   include/linux/fsl/pci-common.h    |   6 ++
>>   2 files changed, 151 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-fsl-common.c b/drivers/pci/host/pci-fsl-common.c
>> index 69d338b..8bc9a64 100644
>> --- a/drivers/pci/host/pci-fsl-common.c
>> +++ b/drivers/pci/host/pci-fsl-common.c
>> @@ -35,52 +35,173 @@
>>   #include <sysdev/fsl_soc.h>
>>   #include <sysdev/fsl_pci.h>
>>   
>> -static int fsl_pcie_check_link(struct pci_controller *hose)
>> +/* Indirect type */
>> +#define INDIRECT_TYPE_EXT_REG			0x00000002
>> +#define INDIRECT_TYPE_SURPRESS_PRIMARY_BUS	0x00000004
>> +#define INDIRECT_TYPE_NO_PCIE_LINK		0x00000008
>> +#define INDIRECT_TYPE_BIG_ENDIAN		0x00000010
>> +#define INDIRECT_TYPE_FSL_CFG_REG_LINK		0x00000040
> Why are these here rather than in the header, given that you have
> indirect_type in the struct in the header?
[Minghuan] It's better to define the type in the header file. I will fix it.
>
>> +int __weak fsl_arch_pci_exclude_device(struct fsl_pci *pci, u8 bus, u8 devfn)
>> +{
>> +	return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int fsl_pci_read_config(struct fsl_pci *pci, int bus, int devfn,
>> +				int offset, int len, u32 *val)
>> +{
>> +	u32 bus_no, reg, data;
>> +
>> +	if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
>> +		if (bus != pci->first_busno)
>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>> +		if (devfn != 0)
>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>> +	}
> A lot of this seems duplicated from arch/powerpc/sysdev/indirect_pci.c.
>
> How generally applicable is that file to non-PPC implementations?  At a
> minimum I see a similar file in arch/microblaze.  It should probably
> eventually be moved to common code, rather than duplicated again.  A
> prerequisite for that would be making common the dependencies it has on
> the rest of what is currently arch PCI infrastructure; until then, it's
> probably better to just have the common fsl-pci code know how to
> interface with the appropriate PPC/ARM code rather than trying to copy
> the infrastructure as well.
[Minghuan] Yes, This is a duplicate except it uses struct fsl_pci. But 
it is hard to be move to common code.
because every indirect read/write functions use different PCI controller 
structure which is very basic structure and ARM has no this structure.
If we can not establish a unified pci controller structure, we can only 
abstract out a simple structure which includes indirect access related 
fields,
and need a callback function to get the pointer like this: 
((powerpc/microblaze/mips/ pci_controller 
*)(pci_bus->sysdata))->indirect_struct.
Should we provide the common code for indirect access API or wait for 
the common PCI controller structure?
> -Scott
Scott Wood - Jan. 7, 2014, 7:13 a.m.
On Mon, 2014-01-06 at 13:36 +0800, Lian Minghuan-b31939 wrote:
> HI Scott,
> 
> please see my comments inline.
> 
> On 01/04/2014 06:33 AM, Scott Wood wrote:
> > A lot of this seems duplicated from arch/powerpc/sysdev/indirect_pci.c.
> >
> > How generally applicable is that file to non-PPC implementations?  At a
> > minimum I see a similar file in arch/microblaze.  It should probably
> > eventually be moved to common code, rather than duplicated again.  A
> > prerequisite for that would be making common the dependencies it has on
> > the rest of what is currently arch PCI infrastructure; until then, it's
> > probably better to just have the common fsl-pci code know how to
> > interface with the appropriate PPC/ARM code rather than trying to copy
> > the infrastructure as well.
> [Minghuan] Yes, This is a duplicate except it uses struct fsl_pci. But 
> it is hard to be move to common code.
> because every indirect read/write functions use different PCI controller 
> structure which is very basic structure and ARM has no this structure.
> If we can not establish a unified pci controller structure, we can only 
> abstract out a simple structure which includes indirect access related 
> fields,
> and need a callback function to get the pointer like this: 
> ((powerpc/microblaze/mips/ pci_controller 
> *)(pci_bus->sysdata))->indirect_struct.
> Should we provide the common code for indirect access API or wait for 
> the common PCI controller structure?

Either work with the PCI maintainer to come up with a common structure,
or leave the code where it is and call into it.

-Scott

Patch

diff --git a/drivers/pci/host/pci-fsl-common.c b/drivers/pci/host/pci-fsl-common.c
index 69d338b..8bc9a64 100644
--- a/drivers/pci/host/pci-fsl-common.c
+++ b/drivers/pci/host/pci-fsl-common.c
@@ -35,52 +35,173 @@ 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 
-static int fsl_pcie_check_link(struct pci_controller *hose)
+/* Indirect type */
+#define INDIRECT_TYPE_EXT_REG			0x00000002
+#define INDIRECT_TYPE_SURPRESS_PRIMARY_BUS	0x00000004
+#define INDIRECT_TYPE_NO_PCIE_LINK		0x00000008
+#define INDIRECT_TYPE_BIG_ENDIAN		0x00000010
+#define INDIRECT_TYPE_FSL_CFG_REG_LINK		0x00000040
+
+int __weak fsl_arch_pci_exclude_device(struct fsl_pci *pci, u8 bus, u8 devfn)
+{
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int fsl_pci_read_config(struct fsl_pci *pci, int bus, int devfn,
+				int offset, int len, u32 *val)
+{
+	u32 bus_no, reg, data;
+
+	if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
+		if (bus != pci->first_busno)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		if (devfn != 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	if (fsl_arch_pci_exclude_device(pci, bus, devfn))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	bus_no = (bus == pci->first_busno) ? pci->self_busno : bus;
+
+	if (pci->indirect_type & INDIRECT_TYPE_EXT_REG)
+		reg = ((offset & 0xf00) << 16) | (offset & 0xfc);
+	else
+		reg = offset & 0xfc;
+
+	if (pci->indirect_type & INDIRECT_TYPE_BIG_ENDIAN)
+		iowrite32be(0x80000000 | (bus_no << 16) | (devfn << 8) | reg,
+			    &pci->regs->config_addr);
+	else
+		iowrite32(0x80000000 | (bus_no << 16) | (devfn << 8) | reg,
+			  &pci->regs->config_addr);
+
+	/*
+	 * Note: the caller has already checked that offset is
+	 * suitably aligned and that len is 1, 2 or 4.
+	 */
+	data = ioread32(&pci->regs->config_data);
+	switch (len) {
+	case 1:
+		*val = (data >> (8 * (offset & 3))) & 0xff;
+		break;
+	case 2:
+		*val = (data >> (8 * (offset & 3))) & 0xffff;
+		break;
+	default:
+		*val = data;
+		break;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int fsl_pci_write_config(struct fsl_pci *pci, int bus, int devfn,
+				 int offset, int len, u32 val)
+{
+	void __iomem *cfg_data;
+	u32 bus_no, reg;
+
+	if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
+		if (bus != pci->first_busno)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		if (devfn != 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	if (fsl_arch_pci_exclude_device(pci, bus, devfn))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	bus_no = (bus == pci->first_busno) ?
+			pci->self_busno : bus;
+
+	if (pci->indirect_type & INDIRECT_TYPE_EXT_REG)
+		reg = ((offset & 0xf00) << 16) | (offset & 0xfc);
+	else
+		reg = offset & 0xfc;
+
+	if (pci->indirect_type & INDIRECT_TYPE_BIG_ENDIAN)
+		iowrite32be(0x80000000 | (bus_no << 16) | (devfn << 8) | reg,
+			    &pci->regs->config_addr);
+	else
+		iowrite32(0x80000000 | (bus_no << 16) | (devfn << 8) | reg,
+			  &pci->regs->config_addr);
+
+	/* suppress setting of PCI_PRIMARY_BUS */
+	if (pci->indirect_type & INDIRECT_TYPE_SURPRESS_PRIMARY_BUS)
+		if ((offset == PCI_PRIMARY_BUS) &&
+		    (bus == pci->first_busno))
+			val &= 0xffffff00;
+
+	/*
+	 * Note: the caller has already checked that offset is
+	 * suitably aligned and that len is 1, 2 or 4.
+	 */
+	cfg_data = ((void *) &(pci->regs->config_data)) + (offset & 3);
+	switch (len) {
+	case 1:
+		iowrite8(val, cfg_data);
+		break;
+	case 2:
+		iowrite16(val, cfg_data);
+		break;
+	default:
+		iowrite32(val, cfg_data);
+		break;
+	}
+	return PCIBIOS_SUCCESSFUL;
+}
+
+bool fsl_pci_check_link(struct fsl_pci *pci)
 {
 	u32 val = 0;
 
-	if (hose->indirect_type & PPC_INDIRECT_TYPE_FSL_CFG_REG_LINK) {
-		if (hose->ops->read == fsl_indirect_read_config) {
-			struct pci_bus bus;
-			bus.number = hose->first_busno;
-			bus.sysdata = hose;
-			bus.ops = hose->ops;
-			indirect_read_config(&bus, 0, PCIE_LTSSM, 4, &val);
-		} else
-			early_read_config_dword(hose, 0, 0, PCIE_LTSSM, &val);
+	if (pci->indirect_type & INDIRECT_TYPE_FSL_CFG_REG_LINK) {
+		fsl_pci_read_config(pci, 0, 0, PCIE_LTSSM, 4, &val);
 		if (val < PCIE_LTSSM_L0)
-			return 1;
+			return false;
 	} else {
-		struct ccsr_pci __iomem *pci = hose->private_data;
 		/* for PCIe IP rev 3.0 or greater use CSR0 for link state */
-		val = (in_be32(&pci->pex_csr0) & PEX_CSR0_LTSSM_MASK)
+		val = (ioread32be(&pci->regs->pex_csr0) & PEX_CSR0_LTSSM_MASK)
 				>> PEX_CSR0_LTSSM_SHIFT;
 		if (val != PEX_CSR0_LTSSM_L0)
-			return 1;
+			return false;
 	}
 
-	return 0;
+	return true;
 }
 
 static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
 				    int offset, int len, u32 *val)
 {
-	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct fsl_pci *pci = fsl_arch_sys_to_pci(bus->sysdata);
+
+	if (!pci)
+		return PCIBIOS_DEVICE_NOT_FOUND;
 
-	if (fsl_pcie_check_link(hose))
-		hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+	if (fsl_pci_check_link(pci))
+		pci->indirect_type &= ~INDIRECT_TYPE_NO_PCIE_LINK;
 	else
-		hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		pci->indirect_type |= INDIRECT_TYPE_NO_PCIE_LINK;
 
-	return indirect_read_config(bus, devfn, offset, len, val);
+	return fsl_pci_read_config(pci, bus->number, devfn, offset, len, val);
 }
 
-#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
-
-static struct pci_ops fsl_indirect_pcie_ops =
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned int devfn,
+				     int offset, int len, u32 val)
 {
+	struct fsl_pci *pci = fsl_arch_sys_to_pci(bus->sysdata);
+
+	if (!pci)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	return fsl_pci_write_config(pci, bus->number, devfn,
+				     offset, len, val);
+}
+
+static struct pci_ops fsl_indirect_pci_ops = {
 	.read = fsl_indirect_read_config,
-	.write = indirect_write_config,
+	.write = fsl_indirect_write_config,
 };
 
 static int setup_one_atmu(struct ccsr_pci __iomem *pci,
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index e56a040..7df4355 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -143,5 +143,11 @@  struct fsl_pci {
  */
 extern struct fsl_pci *fsl_arch_sys_to_pci(void *sys);
 
+/* Return link status true -> link, false -> no link */
+bool fsl_pci_check_link(struct fsl_pci *pci);
+
+/* To avoid touching specified devices */
+int fsl_arch_pci_exclude_device(struct fsl_pci *pci, u8 bus, u8 devfn);
+
 #endif /* __PCI_COMMON_H */
 #endif /* __KERNEL__ */