[PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Lorenzo Pieralisi]

The number of windows allocated for the host bridge depends on the
bus resource. Instead of first allocating the windows and then
limit the bus resource, this patch reshuffles the code so that if any
limitation is applied to the bus resource it is taken into account in
the windows allocation.

Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/pci/host/pci-host-generic.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-hos=
t-generic.c
index 3d2076f..1e1a80f 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
ci *pci)
 =09=09return err;
 =09}
=20
-=09pci->cfg.win =3D devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
-=09=09=09=09    sizeof(*pci->cfg.win), GFP_KERNEL);
-=09if (!pci->cfg.win)
-=09=09return -ENOMEM;
-
 =09/* Limit the bus-range to fit within reg */
 =09bus_max =3D pci->cfg.bus_range.start +
 =09=09  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
 =09pci->cfg.bus_range.end =3D min_t(resource_size_t, pci->cfg.bus_range.en=
d,
 =09=09=09=09       bus_max);
=20
+=09pci->cfg.win =3D devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
+=09=09=09=09    sizeof(*pci->cfg.win), GFP_KERNEL);
+=09if (!pci->cfg.win)
+=09=09return -ENOMEM;
+
 =09/* Map our Configuration Space windows */
 =09if (!devm_request_mem_region(dev, pci->cfg.res.start,
 =09=09=09=09     resource_size(&pci->cfg.res),
--=20
2.1.2

[PATCH RFC 2/2] drivers: pci: convert generic host controller to DT resource parsing API

[Lorenzo Pieralisi]

In order to consolidate DT configuration for PCI host controllers in the
kernel, a new API (ie of_pci_get_host_bridge_resources()) was developed
to allow parsing and assigning IO/BUS/MEM resources from DT, removing
duplicated code present in the majority of pci host driver implementations.

This patch converts the existing PCI generic host controller driver to
the new API. Most of the code parsing ranges and creating resources is
now delegated to of_pci_get_host_bridge_resources() API.

The PCI host controller code carries out resources filtering on the
resulting resource list and maps IO space by using the newly introduced
pci_ioremap_iospace() API.

New code supports only one IO resource per generic host controller, which
should cater for all existing host controller configurations.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 drivers/pci/host/pci-host-generic.c | 120 ++++++++------------------------=
----
 1 file changed, 27 insertions(+), 93 deletions(-)

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-hos=
t-generic.c
index 1e1a80f..1895907 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -32,7 +32,7 @@ struct gen_pci_cfg_bus_ops {
=20
 struct gen_pci_cfg_windows {
 =09struct resource=09=09=09=09res;
-=09struct resource=09=09=09=09bus_range;
+=09struct resource=09=09=09=09*bus_range;
 =09void __iomem=09=09=09=09**win;
=20
 =09const struct gen_pci_cfg_bus_ops=09*ops;
@@ -50,7 +50,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_b=
us *bus,
 {
 =09struct pci_sys_data *sys =3D bus->sysdata;
 =09struct gen_pci *pci =3D sys->private_data;
-=09resource_size_t idx =3D bus->number - pci->cfg.bus_range.start;
+=09resource_size_t idx =3D bus->number - pci->cfg.bus_range->start;
=20
 =09return pci->cfg.win[idx] + ((devfn << 8) | where);
 }
@@ -66,7 +66,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_=
bus *bus,
 {
 =09struct pci_sys_data *sys =3D bus->sysdata;
 =09struct gen_pci *pci =3D sys->private_data;
-=09resource_size_t idx =3D bus->number - pci->cfg.bus_range.start;
+=09resource_size_t idx =3D bus->number - pci->cfg.bus_range->start;
=20
 =09return pci->cfg.win[idx] + ((devfn << 12) | where);
 }
@@ -138,106 +138,50 @@ static const struct of_device_id gen_pci_of_match[] =
=3D {
 };
 MODULE_DEVICE_TABLE(of, gen_pci_of_match);
=20
-static int gen_pci_calc_io_offset(struct device *dev,
-=09=09=09=09  struct of_pci_range *range,
-=09=09=09=09  struct resource *res,
-=09=09=09=09  resource_size_t *offset)
-{
-=09static atomic_t wins =3D ATOMIC_INIT(0);
-=09int err, idx, max_win;
-=09unsigned int window;
-
-=09if (!PAGE_ALIGNED(range->cpu_addr))
-=09=09return -EINVAL;
-
-=09max_win =3D (IO_SPACE_LIMIT + 1) / SZ_64K;
-=09idx =3D atomic_inc_return(&wins);
-=09if (idx > max_win)
-=09=09return -ENOSPC;
-
-=09window =3D (idx - 1) * SZ_64K;
-=09err =3D pci_ioremap_io(window, range->cpu_addr);
-=09if (err)
-=09=09return err;
-
-=09of_pci_range_to_resource(range, dev->of_node, res);
-=09res->start =3D window;
-=09res->end =3D res->start + range->size - 1;
-=09*offset =3D window - range->pci_addr;
-=09return 0;
-}
-
-static int gen_pci_calc_mem_offset(struct device *dev,
-=09=09=09=09   struct of_pci_range *range,
-=09=09=09=09   struct resource *res,
-=09=09=09=09   resource_size_t *offset)
-{
-=09of_pci_range_to_resource(range, dev->of_node, res);
-=09*offset =3D range->cpu_addr - range->pci_addr;
-=09return 0;
-}
-
 static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
 {
-=09struct pci_host_bridge_window *win;
-
-=09list_for_each_entry(win, &pci->resources, list)
-=09=09release_resource(win->res);
-
 =09pci_free_resource_list(&pci->resources);
 }
=20
 static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
 {
-=09struct of_pci_range range;
-=09struct of_pci_range_parser parser;
 =09int err, res_valid =3D 0;
 =09struct device *dev =3D pci->host.dev.parent;
 =09struct device_node *np =3D dev->of_node;
+=09resource_size_t iobase;
+=09struct pci_host_bridge_window *win;
=20
-=09if (of_pci_range_parser_init(&parser, np)) {
-=09=09dev_err(dev, "missing \"ranges\" property\n");
-=09=09return -EINVAL;
-=09}
+=09err =3D of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
+=09=09=09=09=09       &iobase);
+=09if (err)
+=09=09return err;
=20
-=09for_each_of_pci_range(&parser, &range) {
-=09=09struct resource *parent, *res;
-=09=09resource_size_t offset;
-=09=09u32 restype =3D range.flags & IORESOURCE_TYPE_BITS;
+=09list_for_each_entry(win, &pci->resources, list) {
+=09=09struct resource *parent, *res =3D win->res;
=20
-=09=09res =3D devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
-=09=09if (!res) {
-=09=09=09err =3D -ENOMEM;
-=09=09=09goto out_release_res;
-=09=09}
-
-=09=09switch (restype) {
+=09=09switch (resource_type(res)) {
 =09=09case IORESOURCE_IO:
 =09=09=09parent =3D &ioport_resource;
-=09=09=09err =3D gen_pci_calc_io_offset(dev, &range, res, &offset);
+=09=09=09err =3D pci_remap_iospace(res, iobase);
+=09=09=09if (err) {
+=09=09=09=09dev_warn(dev, "error %d: failed to map resource %pR\n",
+=09=09=09=09=09 err, res);
+=09=09=09=09continue;
+=09=09=09}
 =09=09=09break;
 =09=09case IORESOURCE_MEM:
 =09=09=09parent =3D &iomem_resource;
-=09=09=09err =3D gen_pci_calc_mem_offset(dev, &range, res, &offset);
-=09=09=09res_valid |=3D !(res->flags & IORESOURCE_PREFETCH || err);
+=09=09=09res_valid |=3D !(res->flags & IORESOURCE_PREFETCH);
 =09=09=09break;
+=09=09case IORESOURCE_BUS:
+=09=09=09pci->cfg.bus_range =3D res;
 =09=09default:
-=09=09=09err =3D -EINVAL;
 =09=09=09continue;
 =09=09}
=20
-=09=09if (err) {
-=09=09=09dev_warn(dev,
-=09=09=09=09 "error %d: failed to add resource [type 0x%x, %lld bytes]\n",
-=09=09=09=09 err, restype, range.size);
-=09=09=09continue;
-=09=09}
-
-=09=09err =3D request_resource(parent, res);
+=09=09err =3D devm_request_resource(dev, parent, res);
 =09=09if (err)
 =09=09=09goto out_release_res;
-
-=09=09pci_add_resource_offset(&pci->resources, res, offset);
 =09}
=20
 =09if (!res_valid) {
@@ -262,14 +206,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pc=
i *pci)
 =09struct device *dev =3D pci->host.dev.parent;
 =09struct device_node *np =3D dev->of_node;
=20
-=09if (of_pci_parse_bus_range(np, &pci->cfg.bus_range))
-=09=09pci->cfg.bus_range =3D (struct resource) {
-=09=09=09.name=09=3D np->name,
-=09=09=09.start=09=3D 0,
-=09=09=09.end=09=3D 0xff,
-=09=09=09.flags=09=3D IORESOURCE_BUS,
-=09=09};
-
 =09err =3D of_address_to_resource(np, 0, &pci->cfg.res);
 =09if (err) {
 =09=09dev_err(dev, "missing \"reg\" property\n");
@@ -277,12 +213,12 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
ci *pci)
 =09}
=20
 =09/* Limit the bus-range to fit within reg */
-=09bus_max =3D pci->cfg.bus_range.start +
+=09bus_max =3D pci->cfg.bus_range->start +
 =09=09  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
-=09pci->cfg.bus_range.end =3D min_t(resource_size_t, pci->cfg.bus_range.en=
d,
-=09=09=09=09       bus_max);
+=09pci->cfg.bus_range->end =3D min_t(resource_size_t,
+=09=09=09=09=09pci->cfg.bus_range->end, bus_max);
=20
-=09pci->cfg.win =3D devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
+=09pci->cfg.win =3D devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
 =09=09=09=09    sizeof(*pci->cfg.win), GFP_KERNEL);
 =09if (!pci->cfg.win)
 =09=09return -ENOMEM;
@@ -293,7 +229,7 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci=
 *pci)
 =09=09=09=09     "Configuration Space"))
 =09=09return -ENOMEM;
=20
-=09bus_range =3D &pci->cfg.bus_range;
+=09bus_range =3D pci->cfg.bus_range;
 =09for (busn =3D bus_range->start; busn <=3D bus_range->end; ++busn) {
 =09=09u32 idx =3D busn - bus_range->start;
 =09=09u32 sz =3D 1 << pci->cfg.ops->bus_shift;
@@ -305,8 +241,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci=
 *pci)
 =09=09=09return -ENOMEM;
 =09}
=20
-=09/* Register bus resource */
-=09pci_add_resource(&pci->resources, bus_range);
 =09return 0;
 }
=20
--=20
2.1.2

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Bjorn Helgaas]

On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> The number of windows allocated for the host bridge depends on the
> bus resource. Instead of first allocating the windows and then
> limit the bus resource, this patch reshuffles the code so that if any
> limitation is applied to the bus resource it is taken into account in
> the windows allocation.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

Hi Lorenzo,

I can *read* your patches just fine, but when I save them using mutt to
apply them, they look like this:

  --- a/drivers/pci/host/pci-host-generic.c
  +++ b/drivers/pci/host/pci-host-generic.c
  @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
  ci *pci)
   =09=09return err;
    =09}
    =20
    -=09pci->cfg.win =3D devm_kcalloc(dev,
    resource_size(&pci->cfg.bus_range),

I can work around this by downloading them another way, but it is a
bit of a hassle.  I *think* this is because your email has

  Content-Type: text/plain; charset=WINDOWS-1252

which apparently confuses mutt.  Any idea why this is?  Anybody know how I
can fix mutt to deal with this?

Bjorn

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Lorenzo Pieralisi]

On Thu, Oct 23, 2014 at 11:27:07PM +0100, Bjorn Helgaas wrote:
> On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> > The number of windows allocated for the host bridge depends on the
> > bus resource. Instead of first allocating the windows and then
> > limit the bus resource, this patch reshuffles the code so that if any
> > limitation is applied to the bus resource it is taken into account in
> > the windows allocation.
> > 
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> Hi Lorenzo,
> 
> I can *read* your patches just fine, but when I save them using mutt to
> apply them, they look like this:
> 
>   --- a/drivers/pci/host/pci-host-generic.c
>   +++ b/drivers/pci/host/pci-host-generic.c
>   @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
>   ci *pci)
>    =09=09return err;
>     =09}
>     =20
>     -=09pci->cfg.win =3D devm_kcalloc(dev,
>     resource_size(&pci->cfg.bus_range),
> 
> I can work around this by downloading them another way, but it is a
> bit of a hassle.  I *think* this is because your email has
> 
>   Content-Type: text/plain; charset=WINDOWS-1252

Yes, it is the SMTP server messing about with patches, I will have to switch
to another one, sorry.

I pushed a branch on my arm git:

git://linux-arm.org/linux-2.6-lp.git pci-host-generic

I can also resend the patches, whatever it is best for you.

Thanks,
Lorenzo

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Liviu Dudau]

On Thu, Oct 23, 2014 at 11:27:07PM +0100, Bjorn Helgaas wrote:
> On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> > The number of windows allocated for the host bridge depends on the
> > bus resource. Instead of first allocating the windows and then
> > limit the bus resource, this patch reshuffles the code so that if any
> > limitation is applied to the bus resource it is taken into account in
> > the windows allocation.
> > 
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> Hi Lorenzo,
> 
> I can *read* your patches just fine, but when I save them using mutt to
> apply them, they look like this:
> 
>   --- a/drivers/pci/host/pci-host-generic.c
>   +++ b/drivers/pci/host/pci-host-generic.c
>   @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
>   ci *pci)
>    =09=09return err;
>     =09}
>     =20
>     -=09pci->cfg.win =3D devm_kcalloc(dev,
>     resource_size(&pci->cfg.bus_range),
> 
> I can work around this by downloading them another way, but it is a
> bit of a hassle.  I *think* this is because your email has
> 
>   Content-Type: text/plain; charset=WINDOWS-1252
> 
> which apparently confuses mutt.  Any idea why this is?  Anybody know how I
> can fix mutt to deal with this?

I use decode-save (<Esc>s on my setup) to save the attachment(s). Sometimes
it works well enough, although more often than not tabs get lost.

Best regards,
Liviu

> 
> Bjorn
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Arnd Bergmann]

On Friday 24 October 2014 10:04:09 Liviu Dudau wrote:
> On Thu, Oct 23, 2014 at 11:27:07PM +0100, Bjorn Helgaas wrote:
> > On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> > > The number of windows allocated for the host bridge depends on the
> > > bus resource. Instead of first allocating the windows and then
> > > limit the bus resource, this patch reshuffles the code so that if any
> > > limitation is applied to the bus resource it is taken into account in
> > > the windows allocation.
> > > 
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > 
> > Hi Lorenzo,
> > 
> > I can *read* your patches just fine, but when I save them using mutt to
> > apply them, they look like this:
> > 
> >   --- a/drivers/pci/host/pci-host-generic.c
> >   +++ b/drivers/pci/host/pci-host-generic.c
> >   @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_p=
> >   ci *pci)
> >    =09=09return err;
> >     =09}
> >     =20
> >     -=09pci->cfg.win =3D devm_kcalloc(dev,
> >     resource_size(&pci->cfg.bus_range),
> > 
> > I can work around this by downloading them another way, but it is a
> > bit of a hassle.  I *think* this is because your email has
> > 
> >   Content-Type: text/plain; charset=WINDOWS-1252
> > 
> > which apparently confuses mutt.  Any idea why this is?  Anybody know how I
> > can fix mutt to deal with this?
> 
> I use decode-save (<Esc>s on my setup) to save the attachment(s). Sometimes
> it works well enough, although more often than not tabs get lost.

I just tried applying the patches locally here. The patch looks the same way
for me, but 'git am' actually works while 'patch' does not.

Maybe Bjorn is using an outdated git version. While the patch clearly shouldn't
have been sent that way, I think git has been changed now to work around that.
I'm using a git-2.1 snapshot here.

	Arnd

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Will Deacon]

On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> The number of windows allocated for the host bridge depends on the
> bus resource. Instead of first allocating the windows and then
> limit the bus resource, this patch reshuffles the code so that if any
> limitation is applied to the bus resource it is taken into account in
> the windows allocation.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  drivers/pci/host/pci-host-generic.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 3d2076f..1e1a80f 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  		return err;
>  	}
>  
> -	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> -				    sizeof(*pci->cfg.win), GFP_KERNEL);
> -	if (!pci->cfg.win)
> -		return -ENOMEM;
> -
>  	/* Limit the bus-range to fit within reg */
>  	bus_max = pci->cfg.bus_range.start +
>  		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
>  	pci->cfg.bus_range.end = min_t(resource_size_t, pci->cfg.bus_range.end,
>  				       bus_max);
>  
> +	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> +				    sizeof(*pci->cfg.win), GFP_KERNEL);
> +	if (!pci->cfg.win)
> +		return -ENOMEM;
> +
>  	/* Map our Configuration Space windows */
>  	if (!devm_request_mem_region(dev, pci->cfg.res.start,
>  				     resource_size(&pci->cfg.res),
> -- 
> 2.1.2
> 

Re: [PATCH RFC 2/2] drivers: pci: convert generic host controller to DT resource parsing API

[Will Deacon]

On Thu, Oct 23, 2014 at 04:23:07PM +0100, Lorenzo Pieralisi wrote:
> In order to consolidate DT configuration for PCI host controllers in the
> kernel, a new API (ie of_pci_get_host_bridge_resources()) was developed
> to allow parsing and assigning IO/BUS/MEM resources from DT, removing
> duplicated code present in the majority of pci host driver implementations.
> 
> This patch converts the existing PCI generic host controller driver to
> the new API. Most of the code parsing ranges and creating resources is
> now delegated to of_pci_get_host_bridge_resources() API.
> 
> The PCI host controller code carries out resources filtering on the
> resulting resource list and maps IO space by using the newly introduced
> pci_ioremap_iospace() API.
> 
> New code supports only one IO resource per generic host controller, which
> should cater for all existing host controller configurations.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  drivers/pci/host/pci-host-generic.c | 120 ++++++++----------------------------
>  1 file changed, 27 insertions(+), 93 deletions(-)

Looks fine to me -- I assume you tested this under a 32-bit kernel?

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 1e1a80f..1895907 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -32,7 +32,7 @@ struct gen_pci_cfg_bus_ops {
>  
>  struct gen_pci_cfg_windows {
>  	struct resource				res;
> -	struct resource				bus_range;
> +	struct resource				*bus_range;
>  	void __iomem				**win;
>  
>  	const struct gen_pci_cfg_bus_ops	*ops;
> @@ -50,7 +50,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>  {
>  	struct pci_sys_data *sys = bus->sysdata;
>  	struct gen_pci *pci = sys->private_data;
> -	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 8) | where);
>  }
> @@ -66,7 +66,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
>  {
>  	struct pci_sys_data *sys = bus->sysdata;
>  	struct gen_pci *pci = sys->private_data;
> -	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
> +	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 12) | where);
>  }
> @@ -138,106 +138,50 @@ static const struct of_device_id gen_pci_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, gen_pci_of_match);
>  
> -static int gen_pci_calc_io_offset(struct device *dev,
> -				  struct of_pci_range *range,
> -				  struct resource *res,
> -				  resource_size_t *offset)
> -{
> -	static atomic_t wins = ATOMIC_INIT(0);
> -	int err, idx, max_win;
> -	unsigned int window;
> -
> -	if (!PAGE_ALIGNED(range->cpu_addr))
> -		return -EINVAL;
> -
> -	max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
> -	idx = atomic_inc_return(&wins);
> -	if (idx > max_win)
> -		return -ENOSPC;
> -
> -	window = (idx - 1) * SZ_64K;
> -	err = pci_ioremap_io(window, range->cpu_addr);
> -	if (err)
> -		return err;
> -
> -	of_pci_range_to_resource(range, dev->of_node, res);
> -	res->start = window;
> -	res->end = res->start + range->size - 1;
> -	*offset = window - range->pci_addr;
> -	return 0;
> -}
> -
> -static int gen_pci_calc_mem_offset(struct device *dev,
> -				   struct of_pci_range *range,
> -				   struct resource *res,
> -				   resource_size_t *offset)
> -{
> -	of_pci_range_to_resource(range, dev->of_node, res);
> -	*offset = range->cpu_addr - range->pci_addr;
> -	return 0;
> -}
> -
>  static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
>  {
> -	struct pci_host_bridge_window *win;
> -
> -	list_for_each_entry(win, &pci->resources, list)
> -		release_resource(win->res);
> -
>  	pci_free_resource_list(&pci->resources);
>  }
>  
>  static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
>  {
> -	struct of_pci_range range;
> -	struct of_pci_range_parser parser;
>  	int err, res_valid = 0;
>  	struct device *dev = pci->host.dev.parent;
>  	struct device_node *np = dev->of_node;
> +	resource_size_t iobase;
> +	struct pci_host_bridge_window *win;
>  
> -	if (of_pci_range_parser_init(&parser, np)) {
> -		dev_err(dev, "missing \"ranges\" property\n");
> -		return -EINVAL;
> -	}
> +	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
> +					       &iobase);
> +	if (err)
> +		return err;
>  
> -	for_each_of_pci_range(&parser, &range) {
> -		struct resource *parent, *res;
> -		resource_size_t offset;
> -		u32 restype = range.flags & IORESOURCE_TYPE_BITS;
> +	list_for_each_entry(win, &pci->resources, list) {
> +		struct resource *parent, *res = win->res;
>  
> -		res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
> -		if (!res) {
> -			err = -ENOMEM;
> -			goto out_release_res;
> -		}
> -
> -		switch (restype) {
> +		switch (resource_type(res)) {
>  		case IORESOURCE_IO:
>  			parent = &ioport_resource;
> -			err = gen_pci_calc_io_offset(dev, &range, res, &offset);
> +			err = pci_remap_iospace(res, iobase);
> +			if (err) {
> +				dev_warn(dev, "error %d: failed to map resource %pR\n",
> +					 err, res);
> +				continue;
> +			}
>  			break;
>  		case IORESOURCE_MEM:
>  			parent = &iomem_resource;
> -			err = gen_pci_calc_mem_offset(dev, &range, res, &offset);
> -			res_valid |= !(res->flags & IORESOURCE_PREFETCH || err);
> +			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
>  			break;
> +		case IORESOURCE_BUS:
> +			pci->cfg.bus_range = res;
>  		default:
> -			err = -EINVAL;
>  			continue;
>  		}
>  
> -		if (err) {
> -			dev_warn(dev,
> -				 "error %d: failed to add resource [type 0x%x, %lld bytes]\n",
> -				 err, restype, range.size);
> -			continue;
> -		}
> -
> -		err = request_resource(parent, res);
> +		err = devm_request_resource(dev, parent, res);
>  		if (err)
>  			goto out_release_res;
> -
> -		pci_add_resource_offset(&pci->resources, res, offset);
>  	}
>  
>  	if (!res_valid) {
> @@ -262,14 +206,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	struct device *dev = pci->host.dev.parent;
>  	struct device_node *np = dev->of_node;
>  
> -	if (of_pci_parse_bus_range(np, &pci->cfg.bus_range))
> -		pci->cfg.bus_range = (struct resource) {
> -			.name	= np->name,
> -			.start	= 0,
> -			.end	= 0xff,
> -			.flags	= IORESOURCE_BUS,
> -		};
> -
>  	err = of_address_to_resource(np, 0, &pci->cfg.res);
>  	if (err) {
>  		dev_err(dev, "missing \"reg\" property\n");
> @@ -277,12 +213,12 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	}
>  
>  	/* Limit the bus-range to fit within reg */
> -	bus_max = pci->cfg.bus_range.start +
> +	bus_max = pci->cfg.bus_range->start +
>  		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
> -	pci->cfg.bus_range.end = min_t(resource_size_t, pci->cfg.bus_range.end,
> -				       bus_max);
> +	pci->cfg.bus_range->end = min_t(resource_size_t,
> +					pci->cfg.bus_range->end, bus_max);
>  
> -	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> +	pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
>  				    sizeof(*pci->cfg.win), GFP_KERNEL);
>  	if (!pci->cfg.win)
>  		return -ENOMEM;
> @@ -293,7 +229,7 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  				     "Configuration Space"))
>  		return -ENOMEM;
>  
> -	bus_range = &pci->cfg.bus_range;
> +	bus_range = pci->cfg.bus_range;
>  	for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
>  		u32 idx = busn - bus_range->start;
>  		u32 sz = 1 << pci->cfg.ops->bus_shift;
> @@ -305,8 +241,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  			return -ENOMEM;
>  	}
>  
> -	/* Register bus resource */
> -	pci_add_resource(&pci->resources, bus_range);
>  	return 0;
>  }
>  
> -- 
> 2.1.2
> 

Re: [PATCH RFC 2/2] drivers: pci: convert generic host controller to DT resource parsing API

[Lorenzo Pieralisi]

On Mon, Oct 27, 2014 at 12:03:35PM +0000, Will Deacon wrote:
> On Thu, Oct 23, 2014 at 04:23:07PM +0100, Lorenzo Pieralisi wrote:
> > In order to consolidate DT configuration for PCI host controllers in the
> > kernel, a new API (ie of_pci_get_host_bridge_resources()) was developed
> > to allow parsing and assigning IO/BUS/MEM resources from DT, removing
> > duplicated code present in the majority of pci host driver implementations.
> > 
> > This patch converts the existing PCI generic host controller driver to
> > the new API. Most of the code parsing ranges and creating resources is
> > now delegated to of_pci_get_host_bridge_resources() API.
> > 
> > The PCI host controller code carries out resources filtering on the
> > resulting resource list and maps IO space by using the newly introduced
> > pci_ioremap_iospace() API.
> > 
> > New code supports only one IO resource per generic host controller, which
> > should cater for all existing host controller configurations.
> > 
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  drivers/pci/host/pci-host-generic.c | 120 ++++++++----------------------------
> >  1 file changed, 27 insertions(+), 93 deletions(-)
> 
> Looks fine to me -- I assume you tested this under a 32-bit kernel?

Yes, through kvmtools, 32-bit guest on 32/64 bits hosts. I will keep on
testing it before asking to merge it in -next. I would be grateful if the
parsing logic could be tested on arm64 too (Suravee, did you give them
a try ?) and collect the resulting tested-by.

>   Acked-by: Will Deacon <will.deacon@arm.com>

Thank you !
Lorenzo

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Bjorn Helgaas]

On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> The number of windows allocated for the host bridge depends on the
> bus resource. Instead of first allocating the windows and then
> limit the bus resource, this patch reshuffles the code so that if any
> limitation is applied to the bus resource it is taken into account in
> the windows allocation.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

I applied these with Will's acks to pci/host-generic for v3.19, thanks!

In my mind, posting patches to the list is implicitly a request for
comments, so I usually don't pay too much attention to things explicitly
marked "RFC" because I figure that's a hint that "this is work-in-progress,
please comment if you see anything obviously broken."

Bjorn

> ---
>  drivers/pci/host/pci-host-generic.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 3d2076f..1e1a80f 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  		return err;
>  	}
>  
> -	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> -				    sizeof(*pci->cfg.win), GFP_KERNEL);
> -	if (!pci->cfg.win)
> -		return -ENOMEM;
> -
>  	/* Limit the bus-range to fit within reg */
>  	bus_max = pci->cfg.bus_range.start +
>  		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
>  	pci->cfg.bus_range.end = min_t(resource_size_t, pci->cfg.bus_range.end,
>  				       bus_max);
>  
> +	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> +				    sizeof(*pci->cfg.win), GFP_KERNEL);
> +	if (!pci->cfg.win)
> +		return -ENOMEM;
> +
>  	/* Map our Configuration Space windows */
>  	if (!devm_request_mem_region(dev, pci->cfg.res.start,
>  				     resource_size(&pci->cfg.res),
> -- 
> 2.1.2
> 
> 
> --
> 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/

Re: [PATCH RFC 1/2] drivers: pci: fix window allocation order wrt bus_range filtering

[Lorenzo Pieralisi]

On Thu, Nov 06, 2014 at 12:05:36AM +0000, Bjorn Helgaas wrote:
> On Thu, Oct 23, 2014 at 04:23:06PM +0100, Lorenzo Pieralisi wrote:
> > The number of windows allocated for the host bridge depends on the
> > bus resource. Instead of first allocating the windows and then
> > limit the bus resource, this patch reshuffles the code so that if any
> > limitation is applied to the bus resource it is taken into account in
> > the windows allocation.
> > 
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> 
> I applied these with Will's acks to pci/host-generic for v3.19, thanks!

Thank you !

> In my mind, posting patches to the list is implicitly a request for
> comments, so I usually don't pay too much attention to things explicitly
> marked "RFC" because I figure that's a hint that "this is work-in-progress,
> please comment if you see anything obviously broken."

I decided to give it an RFC tag because it is a rework of existing code
on top of the new range parsing API and also new iospace remap API, I
just wanted to be cautious.

Thank you,
Lorenzo

> 
> Bjorn
> 
> > ---
> >  drivers/pci/host/pci-host-generic.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> > index 3d2076f..1e1a80f 100644
> > --- a/drivers/pci/host/pci-host-generic.c
> > +++ b/drivers/pci/host/pci-host-generic.c
> > @@ -276,17 +276,17 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
> >  		return err;
> >  	}
> >  
> > -	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> > -				    sizeof(*pci->cfg.win), GFP_KERNEL);
> > -	if (!pci->cfg.win)
> > -		return -ENOMEM;
> > -
> >  	/* Limit the bus-range to fit within reg */
> >  	bus_max = pci->cfg.bus_range.start +
> >  		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
> >  	pci->cfg.bus_range.end = min_t(resource_size_t, pci->cfg.bus_range.end,
> >  				       bus_max);
> >  
> > +	pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range),
> > +				    sizeof(*pci->cfg.win), GFP_KERNEL);
> > +	if (!pci->cfg.win)
> > +		return -ENOMEM;
> > +
> >  	/* Map our Configuration Space windows */
> >  	if (!devm_request_mem_region(dev, pci->cfg.res.start,
> >  				     resource_size(&pci->cfg.res),
> > -- 
> > 2.1.2
> > 
> > 
> > --
> > 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/
>