diff mbox

[v11,1/6] PCI: designware: move calculation of bus addresses to DRA7xx

Message ID 1444991021-109306-2-git-send-email-wangzhou1@hisilicon.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Zhou Wang Oct. 16, 2015, 10:23 a.m. UTC
From: gabriele paoloni <gabriele.paoloni@huawei.com>

Commit f4c55c5a3f7f ("PCI: designware: Program ATU with untranslated
address") added the calculation of PCI BUS addresses in designware,
storing them in new fields added in "struct pcie_port". This
calculation is done for every designware user even if is only
applicable to DRA7xx.
This patch moves the calculation of the bus addresses to the DRA7xx
driver and is needed to allow the rework of designware to use
the new DT parsing API.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
---
 drivers/pci/host/pci-dra7xx.c      | 13 +++++++++++++
 drivers/pci/host/pcie-designware.c | 15 ++++-----------
 2 files changed, 17 insertions(+), 11 deletions(-)

Comments

Bjorn Helgaas Oct. 21, 2015, 10:15 p.m. UTC | #1
Hi Zhou & Gabriele,

On Fri, Oct 16, 2015 at 06:23:36PM +0800, Zhou Wang wrote:
> From: gabriele paoloni <gabriele.paoloni@huawei.com>
> 
> Commit f4c55c5a3f7f ("PCI: designware: Program ATU with untranslated
> address") added the calculation of PCI BUS addresses in designware,
> storing them in new fields added in "struct pcie_port". This
> calculation is done for every designware user even if is only
> applicable to DRA7xx.
> This patch moves the calculation of the bus addresses to the DRA7xx
> driver and is needed to allow the rework of designware to use
> the new DT parsing API.
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
> ---
>  drivers/pci/host/pci-dra7xx.c      | 13 +++++++++++++
>  drivers/pci/host/pcie-designware.c | 15 ++++-----------
>  2 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 199e29a..ebdffa0 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -62,6 +62,7 @@
>  
>  #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
>  #define	LINK_UP						BIT(16)
> +#define	CPU_TO_BUS_ADDR					0x0FFFFFFF

"CPU_TO_BUS_ADDR" is a very generic name.  Since you do have DRA7XX in
other #defines and static symbols in this file, maybe it could be DRA7XX to
make it obvious that it only applies here?

>  
>  struct dra7xx_pcie {
>  	void __iomem		*base;
> @@ -151,6 +152,18 @@ static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
>  static void dra7xx_pcie_host_init(struct pcie_port *pp)
>  {
>  	dw_pcie_setup_rc(pp);
> +
> +	if (pp->io_mod_base)
> +		pp->io_mod_base &= CPU_TO_BUS_ADDR;

These are equivalent to

  pp->io_mod_base &= CPU_TO_BUS_ADDR;

(You don't need to test whether they're zero first.)

> +
> +	if (pp->mem_mod_base)
> +		pp->mem_mod_base &= CPU_TO_BUS_ADDR;
> +
> +	if (pp->cfg0_mod_base) {
> +		pp->cfg0_mod_base &= CPU_TO_BUS_ADDR;
> +		pp->cfg1_mod_base &= CPU_TO_BUS_ADDR;
> +	}
> +
>  	dra7xx_pcie_establish_link(pp);
>  	if (IS_ENABLED(CONFIG_PCI_MSI))
>  		dw_pcie_msi_init(pp);
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 52aa6e3..75338a6 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -365,14 +365,10 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	struct of_pci_range range;
>  	struct of_pci_range_parser parser;
>  	struct resource *cfg_res;
> -	u32 val, na, ns;
> +	u32 val, ns;
>  	const __be32 *addrp;
>  	int i, index, ret;
>  
> -	/* Find the address cell size and the number of cells in order to get
> -	 * the untranslated address.
> -	 */
> -	of_property_read_u32(np, "#address-cells", &na);
>  	ns = of_n_size_cells(np);
>  
>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> @@ -415,8 +411,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			pp->io_base = range.cpu_addr;
>  
>  			/* Find the untranslated IO space address */
> -			pp->io_mod_base = of_read_number(parser.range -
> -							 parser.np + na, ns);
> +			pp->io_mod_base = range.cpu_addr;

So apparently 

  "of_read_number() == range.cpu_addr & CPU_TO_BUS_ADDR" on DRA7xx
  "of_read_number() == range.cpu_addr" everywhere else?

Is that right?  Is that a valid assumption, i.e., are we assuming
anything about DTs in the field that we shouldn't?

>  		}
>  		if (restype == IORESOURCE_MEM) {
>  			of_pci_range_to_resource(&range, np, &pp->mem);
> @@ -425,8 +420,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			pp->mem_bus_addr = range.pci_addr;
>  
>  			/* Find the untranslated MEM space address */
> -			pp->mem_mod_base = of_read_number(parser.range -
> -							  parser.np + na, ns);
> +			pp->mem_mod_base = range.cpu_addr;
>  		}
>  		if (restype == 0) {
>  			of_pci_range_to_resource(&range, np, &pp->cfg);
> @@ -436,8 +430,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
>  
>  			/* Find the untranslated configuration space address */
> -			pp->cfg0_mod_base = of_read_number(parser.range -
> -							   parser.np + na, ns);
> +			pp->cfg0_mod_base = range.cpu_addr;
>  			pp->cfg1_mod_base = pp->cfg0_mod_base +
>  					    pp->cfg0_size;
>  		}
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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
Gabriele Paoloni Oct. 22, 2015, 7:21 a.m. UTC | #2
> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 21 October 2015 23:15
> To: Wangzhou (B)
> Cc: Bjorn Helgaas; jingoohan1@gmail.com; pratyush.anand@gmail.com; Arnd
> Bergmann; linux@arm.linux.org.uk; thomas.petazzoni@free-electrons.com;
> Gabriele Paoloni; lorenzo.pieralisi@arm.com; james.morse@arm.com;
> Liviu.Dudau@arm.com; jason@lakedaemon.net; robh@kernel.org;
> gabriel.fernandez@linaro.org; Minghuan.Lian@freescale.com; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; zhangjukuo;
> qiuzhenfa; liudongdong (C); qiujiang; xuwei (O); Liguozhu (Kenneth)
> Subject: Re: [PATCH v11 1/6] PCI: designware: move calculation of bus
> addresses to DRA7xx
> 
> Hi Zhou & Gabriele,

Hi Bjorn, thanks for reviewing

> 
> On Fri, Oct 16, 2015 at 06:23:36PM +0800, Zhou Wang wrote:
> > From: gabriele paoloni <gabriele.paoloni@huawei.com>
> >
> > Commit f4c55c5a3f7f ("PCI: designware: Program ATU with untranslated
> > address") added the calculation of PCI BUS addresses in designware,
> > storing them in new fields added in "struct pcie_port". This
> > calculation is done for every designware user even if is only
> > applicable to DRA7xx.
> > This patch moves the calculation of the bus addresses to the DRA7xx
> > driver and is needed to allow the rework of designware to use
> > the new DT parsing API.
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> > Acked-by: Pratyush Anand <pratyush.anand@gmail.com>
> > ---
> >  drivers/pci/host/pci-dra7xx.c      | 13 +++++++++++++
> >  drivers/pci/host/pcie-designware.c | 15 ++++-----------
> >  2 files changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> > index 199e29a..ebdffa0 100644
> > --- a/drivers/pci/host/pci-dra7xx.c
> > +++ b/drivers/pci/host/pci-dra7xx.c
> > @@ -62,6 +62,7 @@
> >
> >  #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
> >  #define	LINK_UP						BIT(16)
> > +#define	CPU_TO_BUS_ADDR					0x0FFFFFFF
> 
> "CPU_TO_BUS_ADDR" is a very generic name.  Since you do have DRA7XX in
> other #defines and static symbols in this file, maybe it could be DRA7XX to
> make it obvious that it only applies here?

Ok will change to DRA7XX_CPU_TO_BUS_ADRR in v12

> 
> >
> >  struct dra7xx_pcie {
> >  	void __iomem		*base;
> > @@ -151,6 +152,18 @@ static void dra7xx_pcie_enable_interrupts(struct
> pcie_port *pp)
> >  static void dra7xx_pcie_host_init(struct pcie_port *pp)
> >  {
> >  	dw_pcie_setup_rc(pp);
> > +
> > +	if (pp->io_mod_base)
> > +		pp->io_mod_base &= CPU_TO_BUS_ADDR;
> 
> These are equivalent to
> 
>   pp->io_mod_base &= CPU_TO_BUS_ADDR;
> 
> (You don't need to test whether they're zero first.)
> 

Yes agreed, will change in v12

> > +
> > +	if (pp->mem_mod_base)
> > +		pp->mem_mod_base &= CPU_TO_BUS_ADDR;
> > +
> > +	if (pp->cfg0_mod_base) {
> > +		pp->cfg0_mod_base &= CPU_TO_BUS_ADDR;
> > +		pp->cfg1_mod_base &= CPU_TO_BUS_ADDR;
> > +	}
> > +
> >  	dra7xx_pcie_establish_link(pp);
> >  	if (IS_ENABLED(CONFIG_PCI_MSI))
> >  		dw_pcie_msi_init(pp);
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> > index 52aa6e3..75338a6 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -365,14 +365,10 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >  	struct of_pci_range range;
> >  	struct of_pci_range_parser parser;
> >  	struct resource *cfg_res;
> > -	u32 val, na, ns;
> > +	u32 val, ns;
> >  	const __be32 *addrp;
> >  	int i, index, ret;
> >
> > -	/* Find the address cell size and the number of cells in order to get
> > -	 * the untranslated address.
> > -	 */
> > -	of_property_read_u32(np, "#address-cells", &na);
> >  	ns = of_n_size_cells(np);
> >
> >  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> > @@ -415,8 +411,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >  			pp->io_base = range.cpu_addr;
> >
> >  			/* Find the untranslated IO space address */
> > -			pp->io_mod_base = of_read_number(parser.range -
> > -							 parser.np + na, ns);
> > +			pp->io_mod_base = range.cpu_addr;
> 
> So apparently
> 
>   "of_read_number() == range.cpu_addr & CPU_TO_BUS_ADDR" on DRA7xx
>   "of_read_number() == range.cpu_addr" everywhere else?

Yes correct

> 
> Is that right?  Is that a valid assumption, i.e., are we assuming
> anything about DTs in the field that we shouldn't?

Before I wrote a patch that was generic to accommodate intermediate level
of translation between PCI_addr -> BUS_addr -> CPU_addr

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/360922.html

After discussion we agreed to solve it with a bitmask rather than adding
a field to of_pci_range to make it generic.
The bitmask only applies to DRA7xx

> 
> >  		}
> >  		if (restype == IORESOURCE_MEM) {
> >  			of_pci_range_to_resource(&range, np, &pp->mem);
> > @@ -425,8 +420,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >  			pp->mem_bus_addr = range.pci_addr;
> >
> >  			/* Find the untranslated MEM space address */
> > -			pp->mem_mod_base = of_read_number(parser.range -
> > -							  parser.np + na, ns);
> > +			pp->mem_mod_base = range.cpu_addr;
> >  		}
> >  		if (restype == 0) {
> >  			of_pci_range_to_resource(&range, np, &pp->cfg);
> > @@ -436,8 +430,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >  			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
> >
> >  			/* Find the untranslated configuration space address */
> > -			pp->cfg0_mod_base = of_read_number(parser.range -
> > -							   parser.np + na, ns);
> > +			pp->cfg0_mod_base = range.cpu_addr;
> >  			pp->cfg1_mod_base = pp->cfg0_mod_base +
> >  					    pp->cfg0_size;
> >  		}
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
--
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
Bjorn Helgaas Oct. 22, 2015, 4:35 p.m. UTC | #3
Hi Gabriele,

On Thu, Oct 22, 2015 at 07:21:41AM +0000, Gabriele Paoloni wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@kernel.org]

> > >  #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
> > >  #define	LINK_UP						BIT(16)
> > > +#define	CPU_TO_BUS_ADDR					0x0FFFFFFF
> > 
> > "CPU_TO_BUS_ADDR" is a very generic name.  Since you do have DRA7XX in
> > other #defines and static symbols in this file, maybe it could be DRA7XX to
> > make it obvious that it only applies here?
> 
> Ok will change to DRA7XX_CPU_TO_BUS_ADRR in v12
> ..

> > > +	if (pp->io_mod_base)
> > > +		pp->io_mod_base &= CPU_TO_BUS_ADDR;
> > 
> > These are equivalent to
> > 
> >   pp->io_mod_base &= CPU_TO_BUS_ADDR;
> > 
> > (You don't need to test whether they're zero first.)
> 
> Yes agreed, will change in v12
> ...

> > >  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> > > @@ -415,8 +411,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> > >  			pp->io_base = range.cpu_addr;
> > >
> > >  			/* Find the untranslated IO space address */
> > > -			pp->io_mod_base = of_read_number(parser.range -
> > > -							 parser.np + na, ns);
> > > +			pp->io_mod_base = range.cpu_addr;
> > 
> > So apparently
> > 
> >   "of_read_number() == range.cpu_addr & CPU_TO_BUS_ADDR" on DRA7xx
> >   "of_read_number() == range.cpu_addr" everywhere else?
> 
> Yes correct
> 
> > Is that right?  Is that a valid assumption, i.e., are we assuming
> > anything about DTs in the field that we shouldn't?
> 
> Before I wrote a patch that was generic to accommodate intermediate level
> of translation between PCI_addr -> BUS_addr -> CPU_addr
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/360922.html
> 
> After discussion we agreed to solve it with a bitmask rather than adding
> a field to of_pci_range to make it generic.
> The bitmask only applies to DRA7xx

I haven't gotten all the way through this series yet, but don't bother with
a v12 just for these minor changes.  I can easily fix them up when applying
it.

Bjorn
--
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
Gabriele Paoloni Oct. 22, 2015, 4:37 p.m. UTC | #4
> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Bjorn Helgaas
> Sent: Thursday, October 22, 2015 5:36 PM
> To: Gabriele Paoloni
> Cc: Wangzhou (B); Bjorn Helgaas; jingoohan1@gmail.com;
> pratyush.anand@gmail.com; Arnd Bergmann; linux@arm.linux.org.uk;
> thomas.petazzoni@free-electrons.com; lorenzo.pieralisi@arm.com;
> james.morse@arm.com; Liviu.Dudau@arm.com; jason@lakedaemon.net;
> robh@kernel.org; gabriel.fernandez@linaro.org;
> Minghuan.Lian@freescale.com; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; zhangjukuo; qiuzhenfa; liudongdong (C);
> qiujiang; xuwei (O); Liguozhu (Kenneth)
> Subject: Re: [PATCH v11 1/6] PCI: designware: move calculation of bus
> addresses to DRA7xx
> 
> Hi Gabriele,
> 
> On Thu, Oct 22, 2015 at 07:21:41AM +0000, Gabriele Paoloni wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> 
> > > >  #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
> > > >  #define	LINK_UP						BIT(16)
> > > > +#define	CPU_TO_BUS_ADDR					0x0FFFFFFF
> > >
> > > "CPU_TO_BUS_ADDR" is a very generic name.  Since you do have DRA7XX
> in
> > > other #defines and static symbols in this file, maybe it could be
> DRA7XX to
> > > make it obvious that it only applies here?
> >
> > Ok will change to DRA7XX_CPU_TO_BUS_ADRR in v12
> > ..
> 
> > > > +	if (pp->io_mod_base)
> > > > +		pp->io_mod_base &= CPU_TO_BUS_ADDR;
> > >
> > > These are equivalent to
> > >
> > >   pp->io_mod_base &= CPU_TO_BUS_ADDR;
> > >
> > > (You don't need to test whether they're zero first.)
> >
> > Yes agreed, will change in v12
> > ...
> 
> > > >  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "config");
> > > > @@ -415,8 +411,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> > > >  			pp->io_base = range.cpu_addr;
> > > >
> > > >  			/* Find the untranslated IO space address */
> > > > -			pp->io_mod_base = of_read_number(parser.range -
> > > > -							 parser.np + na, ns);
> > > > +			pp->io_mod_base = range.cpu_addr;
> > >
> > > So apparently
> > >
> > >   "of_read_number() == range.cpu_addr & CPU_TO_BUS_ADDR" on DRA7xx
> > >   "of_read_number() == range.cpu_addr" everywhere else?
> >
> > Yes correct
> >
> > > Is that right?  Is that a valid assumption, i.e., are we assuming
> > > anything about DTs in the field that we shouldn't?
> >
> > Before I wrote a patch that was generic to accommodate intermediate
> level
> > of translation between PCI_addr -> BUS_addr -> CPU_addr
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-
> July/360922.html
> >
> > After discussion we agreed to solve it with a bitmask rather than
> adding
> > a field to of_pci_range to make it generic.
> > The bitmask only applies to DRA7xx
> 
> I haven't gotten all the way through this series yet, but don't bother
> with
> a v12 just for these minor changes.  I can easily fix them up when
> applying
> it.

Great Many Thanks for this!

Gab

> 
> Bjorn
> --
> 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
--
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
Zhou Wang Oct. 26, 2015, 7:27 a.m. UTC | #5
On 2015/10/23 0:35, Bjorn Helgaas wrote:
> Hi Gabriele,
> 
> On Thu, Oct 22, 2015 at 07:21:41AM +0000, Gabriele Paoloni wrote:
>>> -----Original Message-----
>>> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> 
>>>>  #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
>>>>  #define	LINK_UP						BIT(16)
>>>> +#define	CPU_TO_BUS_ADDR					0x0FFFFFFF
>>>
>>> "CPU_TO_BUS_ADDR" is a very generic name.  Since you do have DRA7XX in
>>> other #defines and static symbols in this file, maybe it could be DRA7XX to
>>> make it obvious that it only applies here?
>>
>> Ok will change to DRA7XX_CPU_TO_BUS_ADRR in v12
>> ..
> 
>>>> +	if (pp->io_mod_base)
>>>> +		pp->io_mod_base &= CPU_TO_BUS_ADDR;
>>>
>>> These are equivalent to
>>>
>>>   pp->io_mod_base &= CPU_TO_BUS_ADDR;
>>>
>>> (You don't need to test whether they're zero first.)
>>
>> Yes agreed, will change in v12
>> ...
> 
>>>>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>>>> @@ -415,8 +411,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>>>  			pp->io_base = range.cpu_addr;
>>>>
>>>>  			/* Find the untranslated IO space address */
>>>> -			pp->io_mod_base = of_read_number(parser.range -
>>>> -							 parser.np + na, ns);
>>>> +			pp->io_mod_base = range.cpu_addr;
>>>
>>> So apparently
>>>
>>>   "of_read_number() == range.cpu_addr & CPU_TO_BUS_ADDR" on DRA7xx
>>>   "of_read_number() == range.cpu_addr" everywhere else?
>>
>> Yes correct
>>
>>> Is that right?  Is that a valid assumption, i.e., are we assuming
>>> anything about DTs in the field that we shouldn't?
>>
>> Before I wrote a patch that was generic to accommodate intermediate level
>> of translation between PCI_addr -> BUS_addr -> CPU_addr
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/360922.html
>>
>> After discussion we agreed to solve it with a bitmask rather than adding
>> a field to of_pci_range to make it generic.
>> The bitmask only applies to DRA7xx
> 
> I haven't gotten all the way through this series yet, but don't bother with
> a v12 just for these minor changes.  I can easily fix them up when applying
> it.
> 
> Bjorn
>

Hi Gabriele and Bjorn,

Sorry for late, I am preparing v12 patchset and I will fix above problems together
with other patches.

Many thanks,
Zhou

> .
> 


--
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
diff mbox

Patch

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 199e29a..ebdffa0 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -62,6 +62,7 @@ 
 
 #define	PCIECTRL_DRA7XX_CONF_PHY_CS			0x010C
 #define	LINK_UP						BIT(16)
+#define	CPU_TO_BUS_ADDR					0x0FFFFFFF
 
 struct dra7xx_pcie {
 	void __iomem		*base;
@@ -151,6 +152,18 @@  static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
 static void dra7xx_pcie_host_init(struct pcie_port *pp)
 {
 	dw_pcie_setup_rc(pp);
+
+	if (pp->io_mod_base)
+		pp->io_mod_base &= CPU_TO_BUS_ADDR;
+
+	if (pp->mem_mod_base)
+		pp->mem_mod_base &= CPU_TO_BUS_ADDR;
+
+	if (pp->cfg0_mod_base) {
+		pp->cfg0_mod_base &= CPU_TO_BUS_ADDR;
+		pp->cfg1_mod_base &= CPU_TO_BUS_ADDR;
+	}
+
 	dra7xx_pcie_establish_link(pp);
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		dw_pcie_msi_init(pp);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 52aa6e3..75338a6 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -365,14 +365,10 @@  int dw_pcie_host_init(struct pcie_port *pp)
 	struct of_pci_range range;
 	struct of_pci_range_parser parser;
 	struct resource *cfg_res;
-	u32 val, na, ns;
+	u32 val, ns;
 	const __be32 *addrp;
 	int i, index, ret;
 
-	/* Find the address cell size and the number of cells in order to get
-	 * the untranslated address.
-	 */
-	of_property_read_u32(np, "#address-cells", &na);
 	ns = of_n_size_cells(np);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -415,8 +411,7 @@  int dw_pcie_host_init(struct pcie_port *pp)
 			pp->io_base = range.cpu_addr;
 
 			/* Find the untranslated IO space address */
-			pp->io_mod_base = of_read_number(parser.range -
-							 parser.np + na, ns);
+			pp->io_mod_base = range.cpu_addr;
 		}
 		if (restype == IORESOURCE_MEM) {
 			of_pci_range_to_resource(&range, np, &pp->mem);
@@ -425,8 +420,7 @@  int dw_pcie_host_init(struct pcie_port *pp)
 			pp->mem_bus_addr = range.pci_addr;
 
 			/* Find the untranslated MEM space address */
-			pp->mem_mod_base = of_read_number(parser.range -
-							  parser.np + na, ns);
+			pp->mem_mod_base = range.cpu_addr;
 		}
 		if (restype == 0) {
 			of_pci_range_to_resource(&range, np, &pp->cfg);
@@ -436,8 +430,7 @@  int dw_pcie_host_init(struct pcie_port *pp)
 			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
 
 			/* Find the untranslated configuration space address */
-			pp->cfg0_mod_base = of_read_number(parser.range -
-							   parser.np + na, ns);
+			pp->cfg0_mod_base = range.cpu_addr;
 			pp->cfg1_mod_base = pp->cfg0_mod_base +
 					    pp->cfg0_size;
 		}