diff mbox series

[v2,08/15] PCI: endpoint: Fix pci_epf_alloc_space to set correct MEM TYPE flags

Message ID 20190114111513.21618-9-kishon@ti.com
State New
Headers show
Series None | expand

Commit Message

Kishon Vijay Abraham I Jan. 14, 2019, 11:15 a.m. UTC 
pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
Base Address Register irrespective of the size. Fix it here to indicate
64-bit BAR if the size is > 2GB.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

---
 drivers/pci/endpoint/pci-epf-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.17.1

Comments

Lorenzo Pieralisi Feb. 11, 2019, 5:37 p.m. UTC | #1 
On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit

> Base Address Register irrespective of the size. Fix it here to indicate

> 64-bit BAR if the size is > 2GB.

> 

> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> ---

>  drivers/pci/endpoint/pci-epf-core.c | 4 +++-

>  1 file changed, 3 insertions(+), 1 deletion(-)


This looks like a fix and should me marked as such. Does it work
as as standalone patch if it gets backported ?

Lorenzo

> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c

> index 825fa24427a3..8bfdcd291196 100644

> --- a/drivers/pci/endpoint/pci-epf-core.c

> +++ b/drivers/pci/endpoint/pci-epf-core.c

> @@ -131,7 +131,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar)

>  	epf->bar[bar].phys_addr = phys_addr;

>  	epf->bar[bar].size = size;

>  	epf->bar[bar].barno = bar;

> -	epf->bar[bar].flags = PCI_BASE_ADDRESS_SPACE_MEMORY;

> +	epf->bar[bar].flags |= upper_32_bits(size) ?

> +				PCI_BASE_ADDRESS_MEM_TYPE_64 :

> +				PCI_BASE_ADDRESS_MEM_TYPE_32;

>  

>  	return space;

>  }

> -- 

> 2.17.1

>
Kishon Vijay Abraham I Feb. 13, 2019, 1:47 p.m. UTC | #2 
Hi Lorenzo,

On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:
> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:

>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit

>> Base Address Register irrespective of the size. Fix it here to indicate

>> 64-bit BAR if the size is > 2GB.

>>

>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>> ---

>>  drivers/pci/endpoint/pci-epf-core.c | 4 +++-

>>  1 file changed, 3 insertions(+), 1 deletion(-)

> 

> This looks like a fix and should me marked as such. Does it work

> as as standalone patch if it gets backported ?


Yeah, it should work. But the current users doesn't allocate > 2GB and some
EPC drivers configure their registers based on size. So nothing is broken
without this patch as such.

Thanks
Kishon
Lorenzo Pieralisi Feb. 14, 2019, 4:29 p.m. UTC | #3 
On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,

> 

> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:

> > On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:

> >> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit

> >> Base Address Register irrespective of the size. Fix it here to indicate

> >> 64-bit BAR if the size is > 2GB.

> >>

> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> >> ---

> >>  drivers/pci/endpoint/pci-epf-core.c | 4 +++-

> >>  1 file changed, 3 insertions(+), 1 deletion(-)

> > 

> > This looks like a fix and should me marked as such. Does it work

> > as as standalone patch if it gets backported ?

> 

> Yeah, it should work. But the current users doesn't allocate > 2GB and some

> EPC drivers configure their registers based on size. So nothing is broken

> without this patch as such.


I suspect you mean 4GB (here and the commit log), right ? I am checking
the commit logs, aiming at merging the patches.

Lorenzo
Kishon Vijay Abraham I Feb. 15, 2019, 6:19 a.m. UTC | #4 
Hi Lorenzo,

On 14/02/19 9:59 PM, Lorenzo Pieralisi wrote:
> On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:

>> Hi Lorenzo,

>>

>> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:

>>> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:

>>>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit

>>>> Base Address Register irrespective of the size. Fix it here to indicate

>>>> 64-bit BAR if the size is > 2GB.

>>>>

>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

>>>> ---

>>>>  drivers/pci/endpoint/pci-epf-core.c | 4 +++-

>>>>  1 file changed, 3 insertions(+), 1 deletion(-)

>>>

>>> This looks like a fix and should me marked as such. Does it work

>>> as as standalone patch if it gets backported ?

>>

>> Yeah, it should work. But the current users doesn't allocate > 2GB and some

>> EPC drivers configure their registers based on size. So nothing is broken

>> without this patch as such.

> 

> I suspect you mean 4GB (here and the commit log), right ? I am checking

> the commit logs, aiming at merging the patches.


A 32bit BAR register can support a 'size' of only up to 2GB. Though it can hold
a memory address of up to 4GB.

This is also mentioned in the PCI Local Bus Specification.
"A 32-bit register can be implemented to support a single memory size that is a
power of 2 from 16 bytes to 2 GB"

Thanks
Kishon
Lorenzo Pieralisi Feb. 15, 2019, 9:52 a.m. UTC | #5 
On Fri, Feb 15, 2019 at 11:49:12AM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,

> 

> On 14/02/19 9:59 PM, Lorenzo Pieralisi wrote:

> > On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:

> >> Hi Lorenzo,

> >>

> >> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:

> >>> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:

> >>>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit

> >>>> Base Address Register irrespective of the size. Fix it here to indicate

> >>>> 64-bit BAR if the size is > 2GB.

> >>>>

> >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

> >>>> ---

> >>>>  drivers/pci/endpoint/pci-epf-core.c | 4 +++-

> >>>>  1 file changed, 3 insertions(+), 1 deletion(-)

> >>>

> >>> This looks like a fix and should me marked as such. Does it work

> >>> as as standalone patch if it gets backported ?

> >>

> >> Yeah, it should work. But the current users doesn't allocate > 2GB and some

> >> EPC drivers configure their registers based on size. So nothing is broken

> >> without this patch as such.

> > 

> > I suspect you mean 4GB (here and the commit log), right ? I am checking

> > the commit logs, aiming at merging the patches.

> 

> A 32bit BAR register can support a 'size' of only up to 2GB. Though it

> can hold a memory address of up to 4GB.

> 

> This is also mentioned in the PCI Local Bus Specification.  "A 32-bit

> register can be implemented to support a single memory size that is a

> power of 2 from 16 bytes to 2 GB"


Very true - sorry for the noise.

Lorenz,o
diff mbox series

Patch

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 825fa24427a3..8bfdcd291196 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -131,7 +131,9 @@  void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar)
 	epf->bar[bar].phys_addr = phys_addr;
 	epf->bar[bar].size = size;
 	epf->bar[bar].barno = bar;
-	epf->bar[bar].flags = PCI_BASE_ADDRESS_SPACE_MEMORY;
+	epf->bar[bar].flags |= upper_32_bits(size) ?
+				PCI_BASE_ADDRESS_MEM_TYPE_64 :
+				PCI_BASE_ADDRESS_MEM_TYPE_32;
 
 	return space;
 }