Patchwork [02/12,v3] pci: fsl: add structure fsl_pci

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

Comments

Minghuan Lian - Oct. 23, 2013, 10:41 a.m.
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.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/

 include/linux/fsl/pci-common.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
Kumar Gala - Oct. 24, 2013, 4:11 a.m.
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.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/
> 
> 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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zang Roy-R61911 - Oct. 24, 2013, 4:15 a.m.
> -----Original Message-----
> From: Kumar Gala [mailto:galak@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-pci@vger.kernel.org; linux-arm-
> kernel@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.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/
> >
> > 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


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lian Minghaun-b31939 - Oct. 25, 2013, 5:58 a.m.
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.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/
>>
>> 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
>>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Oct. 28, 2013, 6:22 p.m.
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.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/
> >>
> >> 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



--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Jan. 3, 2014, 10:19 p.m.
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.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/
> 
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lian Minghaun-b31939 - Jan. 6, 2014, 6:10 a.m.
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.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/
>>
>>   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    = fsl_pcie_map_irq,
+};

+static struct pci_bus *
+fake_pci_bus(struct fsl_pcie *pcie, int busnr)
+{
+    static struct pci_bus bus;
+    static struct pci_sys_data sys;
+
+    bus.number = busnr;
+    bus.sysdata = &sys;
+    sys.private_data = pcie;
+    bus.ops = pcie->ops;
+    return &bus;
+}
+
+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;
+
+    pci_common_init(&fsl_hw_pcie);
+    pci_assign_unassigned_resources();
+#ifdef CONFIG_PCI_DOMAINS
+    fsl_hw_pcie.domain++;
+#endif
+}



>
> -Scott


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Jan. 7, 2014, 8:33 a.m.
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


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zang Roy-R61911 - Jan. 22, 2014, 11:38 p.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, January 03, 2014 4:19 PM
> To: Lian Minghuan-B31939
> Cc: linuxppc-dev@lists.ozlabs.org; linux-pci@vger.kernel.org; Zang Roy-
> R61911; Bjorn Helgaas
> Subject: 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.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/
> >
> >  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).
INDIRECT type is for configuration space access, I do not think it is ppc header specific.
It is good to put it to freescale pci common code.
Roy
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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__ */