Hi Zhou & Gabriele,

On Fri, Oct 16, 2015 at 06:23:36PM +0800, Zhou Wang wrote:
> From: gabriele paoloni <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: Zhou Wang <[email protected]>
> Acked-by: Pratyush Anand <[email protected]>
> ---
>  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 [email protected]
> 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-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to