Re: [02/12,v3] pci: fsl: add structure fsl_pci
On Mon, 2014-01-06 at 14:10 +0800, Lian Minghuan-b31939 wrote:
On 01/04/2014 06:19 AM, Scott Wood wrote:
I don't like the extent to which this duplicates (not moves) PPC's struct
pci_controller. Also this leaves some fields like indirect_type
unexplained (PPC_INDIRECT_TYPE_xxx is only in the PPC header).
Does the arch-independent part of the driver really need all this? Given
how closely this tracks the PPC code, how would this work on ARM?
[Minghuan] I added the duplicate fields because PPC's struct
pci_controller need them.
I think a better approach would be to create a cleanly architected
arch-independent driver. Share what you reasonably can with the current
fsl_pci.c, but not to the extent of propagating PPCisms that don't match
up with what we ultimately want to see in generic code, or copying
things that ought to be controller-independent infrastructure into
controller-specific code.
See these threads:
http://www.spinics.net/lists/linux-pci/msg25769.html
https://lkml.org/lkml/2013/5/4/103
The following is for ARM, I will submit them after verification:
[snip]
+static int fsl_pcie_register(struct fsl_pcie *pcie)
+{
+pcie-controller = fsl_hw_pcie.nr_controllers;
+fsl_hw_pcie.nr_controllers = 1;
+fsl_hw_pcie.private_data = (void **)pcie;
I believe this should be:
fsl_hw_pcie.private_data = pcie;
+pci_common_init(fsl_hw_pcie);
+pci_assign_unassigned_resources();
+#ifdef CONFIG_PCI_DOMAINS
+fsl_hw_pcie.domain++;
+#endif
What serializes that non-atomic increment?
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [02/12,v3] pci: fsl: add structure fsl_pci
On 01/04/2014 06:19 AM, Scott Wood wrote:
On Wed, Oct 23, 2013 at 06:41:24PM +0800, Minghuan Lian wrote:
PowerPC uses structure pci_controller to describe PCI controller,
but ARM uses structure pci_sys_data. In order to support PowerPC
and ARM simultaneously, the patch adds a structure fsl_pci that
contains most of the members of the pci_controller and pci_sys_data.
Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
be implemented in architecture-specific PCI controller driver to
convert pci_controller or pci_sys_data to fsl_pci.
Signed-off-by: Minghuan Lian minghuan.l...@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/
include/linux/fsl/pci-common.h | 41 +
1 file changed, 41 insertions(+)
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index 5e4f683..e56a040 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -102,5 +102,46 @@ struct ccsr_pci {
};
+/*
+ * Structure of a PCI controller (host bridge)
+ */
+struct fsl_pci {
+ struct list_head node;
+ bool is_pcie;
+ struct device_node *dn;
+ struct device *dev;
+
+ int first_busno;
+ int last_busno;
+ int self_busno;
+ struct resource busn;
+
+ struct pci_ops *ops;
+ struct ccsr_pci __iomem *regs;
+
+ u32 indirect_type;
+
+ struct resource io_resource;
+ resource_size_t io_base_phys;
+ resource_size_t pci_io_size;
+
+ struct resource mem_resources[3];
+ resource_size_t mem_offset[3];
+
+ int global_number; /* PCI domain number */
+
+ resource_size_t dma_window_base_cur;
+ resource_size_t dma_window_size;
+
+ void *sys;
+};
I don't like the extent to which this duplicates (not moves) PPC's struct
pci_controller. Also this leaves some fields like indirect_type
unexplained (PPC_INDIRECT_TYPE_xxx is only in the PPC header).
Does the arch-independent part of the driver really need all this? Given
how closely this tracks the PPC code, how would this work on ARM?
[Minghuan] I added the duplicate fields because PPC's struct
pci_controller need them.
The common PCI driver gets the related information and pass to PowerPC
driver.
And I do hope PowerPC driver to parse dts or access controller to get
the information again.
please see the following code for PowerPC:
int fsl_arch_pci_sys_register(struct fsl_pci *pci)
+{
+struct pci_controller *hose;
+hose = pcibios_alloc_controller(pci-dn);
+
+hose-private_data = pci;
+hose-parent = pci-dev;
+hose-first_busno = pci-first_busno;
+hose-last_busno = pci-last_busno;
+hose-ops = pci-ops;
+
+hose-io_base_virt = ioremap(pci-io_base_phys +
pci-io_resource.start,
+ pci-pci_io_size);
+hose-pci_io_size = pci-io_resource.start + pci-pci_io_size;
+hose-io_base_phys = pci-io_base_phys;
+hose-io_resource = pci-io_resource;
+
+memcpy(hose-mem_offset, pci-mem_offset, sizeof(hose-mem_offset));
+memcpy(hose-mem_resources, pci-mem_resources,
+sizeof(hose-mem_resources));
+hose-dma_window_base_cur = pci-dma_window_base_cur;
+hose-dma_window_size = pci-dma_window_size;
+pci-sys = hose;
+
+return 0;
+}
The following is for ARM, I will submit them after verification:
+
+static inline struct fsl_pcie *sys_to_pcie(struct pci_sys_data *sys)
+{
+return sys-private_data;
+}
+
+static int fsl_pcie_setup(int nr, struct pci_sys_data *sys)
+{
+struct fsl_pcie *pcie;
+
+pcie = sys_to_pcie(sys);
+
+if (!pcie)
+return 0;
+
+pcie-sys = sys;
+
+sys-io_offset = pcie-io_base_phys;
+pci_ioremap_io(sys-io_offset, pcie-io_resource.start);
+pci_add_resource_offset(sys-resources, pcie-io_resource,
+sys-io_offset);
+
+sys-mem_offset = pcie-mem_offset;
+pci_add_resource_offset(sys-resources, pcie-mem_resource,
+sys-mem_offset);
+
+return 1;
+}
+
+static struct pci_bus *
+fsl_pcie_scan_bus(int nr, struct pci_sys_data *sys)
+{
+struct pci_bus *bus;
+struct fsl_pcie *pcie = sys_to_pcie(sys);
+
+bus = pci_create_root_bus(pcie-dev, sys-busnr,
+ pcie-ops, sys, sys-resources);
+if (!bus)
+return NULL;
+
+pci_scan_child_bus(bus);
+
+return bus;
+}
+
+static int fsl_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+struct of_irq oirq;
+int ret;
+
+ret = of_irq_map_pci(dev, oirq);
+if (ret)
+return ret;
+
+return irq_create_of_mapping(oirq.controller, oirq.specifier,
+ oirq.size);
+}
+
+static struct hw_pci fsl_hw_pcie = {
+.ops= fsl_indirect_pci_ops;
+.setup= fsl_pcie_setup,
+.scan= fsl_pcie_scan_bus,
+.map_irq=
Re: [02/12,v3] pci: fsl: add structure fsl_pci
On Wed, Oct 23, 2013 at 06:41:24PM +0800, Minghuan Lian wrote:
PowerPC uses structure pci_controller to describe PCI controller,
but ARM uses structure pci_sys_data. In order to support PowerPC
and ARM simultaneously, the patch adds a structure fsl_pci that
contains most of the members of the pci_controller and pci_sys_data.
Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
be implemented in architecture-specific PCI controller driver to
convert pci_controller or pci_sys_data to fsl_pci.
Signed-off-by: Minghuan Lian minghuan.l...@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/
include/linux/fsl/pci-common.h | 41 +
1 file changed, 41 insertions(+)
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index 5e4f683..e56a040 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -102,5 +102,46 @@ struct ccsr_pci {
};
+/*
+ * Structure of a PCI controller (host bridge)
+ */
+struct fsl_pci {
+ struct list_head node;
+ bool is_pcie;
+ struct device_node *dn;
+ struct device *dev;
+
+ int first_busno;
+ int last_busno;
+ int self_busno;
+ struct resource busn;
+
+ struct pci_ops *ops;
+ struct ccsr_pci __iomem *regs;
+
+ u32 indirect_type;
+
+ struct resource io_resource;
+ resource_size_t io_base_phys;
+ resource_size_t pci_io_size;
+
+ struct resource mem_resources[3];
+ resource_size_t mem_offset[3];
+
+ int global_number; /* PCI domain number */
+
+ resource_size_t dma_window_base_cur;
+ resource_size_t dma_window_size;
+
+ void *sys;
+};
I don't like the extent to which this duplicates (not moves) PPC's struct
pci_controller. Also this leaves some fields like indirect_type
unexplained (PPC_INDIRECT_TYPE_xxx is only in the PPC header).
Does the arch-independent part of the driver really need all this? Given
how closely this tracks the PPC code, how would this work on ARM?
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci
On Fri, 2013-10-25 at 13:58 +0800, Lian Minghuan-b31939 wrote: Hi Kumar, please see my comment inline. On 10/24/2013 12:11 PM, Kumar Gala wrote: On Oct 23, 2013, at 5:41 AM, Minghuan Lian wrote: PowerPC uses structure pci_controller to describe PCI controller, but ARM uses structure pci_sys_data. In order to support PowerPC and ARM simultaneously, the patch adds a structure fsl_pci that contains most of the members of the pci_controller and pci_sys_data. Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should be implemented in architecture-specific PCI controller driver to convert pci_controller or pci_sys_data to fsl_pci. Signed-off-by: Minghuan Lian minghuan.l...@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/ include/linux/fsl/pci-common.h | 41 + 1 file changed, 41 insertions(+) NAK. We discussed this some at the ARM Summit this week and the feeling is we need to move to a common interface between the various ARCHs. [Minghuan] Do you mean we will use the common interface instead of arch/powerpc/kernel/pci-common.c... and arch/arm/kernel/bios32.c? Who will do the code movement and when will the work be completed? The patches just move the common functions of FSL PCI controller operation which can be re-used by PowerPC and ARM. LS1 is coming, I worry about not having enough time to wait for the move is completed. I agree -- it can take quite a while to get from the feeling is we need to move to a common interface to actually having something we can use. If and when this unification is achieved, we can drop this extra layer. It's a better interim solution than just duplicating the entire driver and letting them drift apart. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci
Hi Kumar,
please see my comment inline.
On 10/24/2013 12:11 PM, Kumar Gala wrote:
On Oct 23, 2013, at 5:41 AM, Minghuan Lian wrote:
PowerPC uses structure pci_controller to describe PCI controller,
but ARM uses structure pci_sys_data. In order to support PowerPC
and ARM simultaneously, the patch adds a structure fsl_pci that
contains most of the members of the pci_controller and pci_sys_data.
Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
be implemented in architecture-specific PCI controller driver to
convert pci_controller or pci_sys_data to fsl_pci.
Signed-off-by: Minghuan Lian minghuan.l...@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/
include/linux/fsl/pci-common.h | 41 +
1 file changed, 41 insertions(+)
NAK.
We discussed this some at the ARM Summit this week and the feeling is we need
to move to a common interface between the various ARCHs.
[Minghuan] Do you mean we will use the common interface instead of
arch/powerpc/kernel/pci-common.c...
and arch/arm/kernel/bios32.c? Who will do the code movement and when
will the work be completed? The patches just move the common functions
of FSL PCI controller operation which can be re-used by PowerPC and ARM.
LS1 is coming, I worry about not having enough time to wait for the move
is completed.
- k
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index 5e4f683..e56a040 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -102,5 +102,46 @@ struct ccsr_pci {
};
+/*
+ * Structure of a PCI controller (host bridge)
+ */
+struct fsl_pci {
+ struct list_head node;
+ bool is_pcie;
+ struct device_node *dn;
+ struct device *dev;
+
+ int first_busno;
+ int last_busno;
+ int self_busno;
+ struct resource busn;
+
+ struct pci_ops *ops;
+ struct ccsr_pci __iomem *regs;
+
+ u32 indirect_type;
+
+ struct resource io_resource;
+ resource_size_t io_base_phys;
+ resource_size_t pci_io_size;
+
+ struct resource mem_resources[3];
+ resource_size_t mem_offset[3];
+
+ int global_number; /* PCI domain number */
+
+ resource_size_t dma_window_base_cur;
+ resource_size_t dma_window_size;
+
+ void *sys;
+};
+
+/*
+ * Convert architecture specific pci controller structure to fsl_pci
+ * PowerPC uses structure pci_controller and ARM uses structure pci_sys_data
+ * to describe pci controller.
+ */
+extern struct fsl_pci *fsl_arch_sys_to_pci(void *sys);
+
#endif /* __PCI_COMMON_H */
#endif /* __KERNEL__ */
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 02/12][v3] pci: fsl: add structure fsl_pci
PowerPC uses structure pci_controller to describe PCI controller,
but ARM uses structure pci_sys_data. In order to support PowerPC
and ARM simultaneously, the patch adds a structure fsl_pci that
contains most of the members of the pci_controller and pci_sys_data.
Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
be implemented in architecture-specific PCI controller driver to
convert pci_controller or pci_sys_data to fsl_pci.
Signed-off-by: Minghuan Lian minghuan.l...@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/
include/linux/fsl/pci-common.h | 41 +
1 file changed, 41 insertions(+)
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index 5e4f683..e56a040 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -102,5 +102,46 @@ struct ccsr_pci {
};
+/*
+ * Structure of a PCI controller (host bridge)
+ */
+struct fsl_pci {
+ struct list_head node;
+ bool is_pcie;
+ struct device_node *dn;
+ struct device *dev;
+
+ int first_busno;
+ int last_busno;
+ int self_busno;
+ struct resource busn;
+
+ struct pci_ops *ops;
+ struct ccsr_pci __iomem *regs;
+
+ u32 indirect_type;
+
+ struct resource io_resource;
+ resource_size_t io_base_phys;
+ resource_size_t pci_io_size;
+
+ struct resource mem_resources[3];
+ resource_size_t mem_offset[3];
+
+ int global_number; /* PCI domain number */
+
+ resource_size_t dma_window_base_cur;
+ resource_size_t dma_window_size;
+
+ void *sys;
+};
+
+/*
+ * Convert architecture specific pci controller structure to fsl_pci
+ * PowerPC uses structure pci_controller and ARM uses structure pci_sys_data
+ * to describe pci controller.
+ */
+extern struct fsl_pci *fsl_arch_sys_to_pci(void *sys);
+
#endif /* __PCI_COMMON_H */
#endif /* __KERNEL__ */
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci
On Oct 23, 2013, at 5:41 AM, Minghuan Lian wrote:
PowerPC uses structure pci_controller to describe PCI controller,
but ARM uses structure pci_sys_data. In order to support PowerPC
and ARM simultaneously, the patch adds a structure fsl_pci that
contains most of the members of the pci_controller and pci_sys_data.
Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
be implemented in architecture-specific PCI controller driver to
convert pci_controller or pci_sys_data to fsl_pci.
Signed-off-by: Minghuan Lian minghuan.l...@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/
include/linux/fsl/pci-common.h | 41 +
1 file changed, 41 insertions(+)
NAK.
We discussed this some at the ARM Summit this week and the feeling is we need
to move to a common interface between the various ARCHs.
- k
diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
index 5e4f683..e56a040 100644
--- a/include/linux/fsl/pci-common.h
+++ b/include/linux/fsl/pci-common.h
@@ -102,5 +102,46 @@ struct ccsr_pci {
};
+/*
+ * Structure of a PCI controller (host bridge)
+ */
+struct fsl_pci {
+ struct list_head node;
+ bool is_pcie;
+ struct device_node *dn;
+ struct device *dev;
+
+ int first_busno;
+ int last_busno;
+ int self_busno;
+ struct resource busn;
+
+ struct pci_ops *ops;
+ struct ccsr_pci __iomem *regs;
+
+ u32 indirect_type;
+
+ struct resource io_resource;
+ resource_size_t io_base_phys;
+ resource_size_t pci_io_size;
+
+ struct resource mem_resources[3];
+ resource_size_t mem_offset[3];
+
+ int global_number; /* PCI domain number */
+
+ resource_size_t dma_window_base_cur;
+ resource_size_t dma_window_size;
+
+ void *sys;
+};
+
+/*
+ * Convert architecture specific pci controller structure to fsl_pci
+ * PowerPC uses structure pci_controller and ARM uses structure pci_sys_data
+ * to describe pci controller.
+ */
+extern struct fsl_pci *fsl_arch_sys_to_pci(void *sys);
+
#endif /* __PCI_COMMON_H */
#endif /* __KERNEL__ */
--
1.8.1.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci
-Original Message- From: Kumar Gala [mailto:ga...@kernel.crashing.org] Sent: Wednesday, October 23, 2013 11:11 PM To: Lian Minghuan-B31939 Cc: linuxppc-dev@lists.ozlabs.org list; Arnd Bergmann; Zang Roy-R61911; Wood Scott-B07421; Bjorn Helgaas; linux-...@vger.kernel.org; linux-arm- ker...@lists.infradead.org Subject: Re: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci On Oct 23, 2013, at 5:41 AM, Minghuan Lian wrote: PowerPC uses structure pci_controller to describe PCI controller, but ARM uses structure pci_sys_data. In order to support PowerPC and ARM simultaneously, the patch adds a structure fsl_pci that contains most of the members of the pci_controller and pci_sys_data. Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should be implemented in architecture-specific PCI controller driver to convert pci_controller or pci_sys_data to fsl_pci. Signed-off-by: Minghuan Lian minghuan.l...@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/ include/linux/fsl/pci-common.h | 41 + 1 file changed, 41 insertions(+) NAK. We discussed this some at the ARM Summit this week and the feeling is we need to move to a common interface between the various ARCHs. Can you share more about the *common interface*? Any plan to implement the * common interface*? Thanks. Roy ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
