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 (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: endpoint: Cleanup EPC features | 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(-)

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;
 }