diff mbox

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

Message ID 1414077787-20633-1-git-send-email-lorenzo.pieralisi@arm.com
State Accepted
Commit a5525b240368cf496e669703a5fe169febbc72ef
Headers show

Commit Message

Lorenzo Pieralisi Oct. 23, 2014, 3:23 p.m. UTC
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(-)

Comments

Bjorn Helgaas Oct. 23, 2014, 10:27 p.m. UTC | #1
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
--
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/
Lorenzo Pieralisi Oct. 24, 2014, 8:53 a.m. UTC | #2
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

--
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/
Liviu Dudau Oct. 24, 2014, 9:04 a.m. UTC | #3
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
>
Arnd Bergmann Oct. 24, 2014, 10:55 a.m. UTC | #4
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
--
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/
Will Deacon Oct. 27, 2014, 12:01 p.m. UTC | #5
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
> 
--
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/
Bjorn Helgaas Nov. 6, 2014, 12:05 a.m. UTC | #6
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/
--
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/
Lorenzo Pieralisi Nov. 6, 2014, 9:58 a.m. UTC | #7
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/
> 
--
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/
diff mbox

Patch

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),