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@xxxxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
