* [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
@ 2018-03-08 13:33 ` Niklas Cassel
2018-03-16 18:02 ` Lorenzo Pieralisi
2018-03-21 5:29 ` Kishon Vijay Abraham I
2018-03-08 13:33 ` [PATCH v4 2/5] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly Niklas Cassel
` (4 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Niklas Cassel @ 2018-03-08 13:33 UTC (permalink / raw)
To: kishon, Lorenzo Pieralisi, Bjorn Helgaas, Sekhar Nori,
John Keeping, Shawn Lin, Niklas Cassel, Cyrille Pitchen
Cc: Niklas Cassel, linux-pci, linux-kernel
If a BAR supports 64-bit width or not depends on the hardware,
and should thus not depend on sizeof(dma_addr_t).
Since this driver is generic, default to always using BAR width
of 32-bits. 64-bit BARs can easily be tested by replacing
PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
in bar_flags.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
Note to Lorenzo/Bjorn:
It is not trivial to convert the bar_size + bar_flags +
struct pci_epf->bar member array to an array of struct resources,
since we need to be able to store the addresses returned
by dma_alloc_coherent(), which is of type dma_addr_t.
struct resource uses resource_size_t, which is defined as phys_addr_t.
E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 800da09d9005..7c70433b11a7 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -71,6 +71,14 @@ struct pci_epf_test_data {
};
static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
+static int bar_flags[] = {
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
+ PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
+};
static int pci_epf_test_copy(struct pci_epf_test *epf_test)
{
@@ -358,7 +366,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
static int pci_epf_test_set_bar(struct pci_epf *epf)
{
- int flags;
int bar;
int ret;
struct pci_epf_bar *epf_bar;
@@ -367,15 +374,11 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
enum pci_barno test_reg_bar = epf_test->test_reg_bar;
- flags = PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32;
- if (sizeof(dma_addr_t) == 0x8)
- flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
-
for (bar = BAR_0; bar <= BAR_5; bar++) {
epf_bar = &epf->bar[bar];
ret = pci_epc_set_bar(epc, epf->func_no, bar,
epf_bar->phys_addr,
- epf_bar->size, flags);
+ epf_bar->size, bar_flags[bar]);
if (ret) {
pci_epf_free_space(epf, epf_test->reg[bar], bar);
dev_err(dev, "failed to set BAR%d\n", bar);
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
2018-03-08 13:33 ` [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t Niklas Cassel
@ 2018-03-16 18:02 ` Lorenzo Pieralisi
2018-03-17 0:55 ` Niklas Cassel
2018-03-21 5:29 ` Kishon Vijay Abraham I
1 sibling, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-16 18:02 UTC (permalink / raw)
To: Niklas Cassel
Cc: kishon, Bjorn Helgaas, Sekhar Nori, John Keeping, Shawn Lin,
Niklas Cassel, Cyrille Pitchen, linux-pci, linux-kernel
On Thu, Mar 08, 2018 at 02:33:26PM +0100, Niklas Cassel wrote:
> If a BAR supports 64-bit width or not depends on the hardware,
> and should thus not depend on sizeof(dma_addr_t).
>
> Since this driver is generic, default to always using BAR width
> of 32-bits. 64-bit BARs can easily be tested by replacing
> PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> in bar_flags.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> Note to Lorenzo/Bjorn:
> It is not trivial to convert the bar_size + bar_flags +
> struct pci_epf->bar member array to an array of struct resources,
> since we need to be able to store the addresses returned
> by dma_alloc_coherent(), which is of type dma_addr_t.
> struct resource uses resource_size_t, which is defined as phys_addr_t.
> E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
>
> drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 800da09d9005..7c70433b11a7 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> };
>
> static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> +static int bar_flags[] = {
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> +};
Niklas,
I think you are almost there, I have one question though to address
that can even simplify the patchset.
If, according to your own commit logs (and my reading of the code), the
Cadence driver makes a decision on the BAR size just by checking the
corresponding region size (I would be happy to hear the reason
underpinning that choice, BTW), why can't we do the same for DWC (ie to
let the DWC driver decides whether a BAR should be 64 or 32 bits ?)
This would mean that in this patch we would not bother about the BAR
32/64 size flag at all.
Thoughts ?
Lorenzo
>
> static int pci_epf_test_copy(struct pci_epf_test *epf_test)
> {
> @@ -358,7 +366,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> static int pci_epf_test_set_bar(struct pci_epf *epf)
> {
> - int flags;
> int bar;
> int ret;
> struct pci_epf_bar *epf_bar;
> @@ -367,15 +374,11 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> enum pci_barno test_reg_bar = epf_test->test_reg_bar;
>
> - flags = PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32;
> - if (sizeof(dma_addr_t) == 0x8)
> - flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> -
> for (bar = BAR_0; bar <= BAR_5; bar++) {
> epf_bar = &epf->bar[bar];
> ret = pci_epc_set_bar(epc, epf->func_no, bar,
> epf_bar->phys_addr,
> - epf_bar->size, flags);
> + epf_bar->size, bar_flags[bar]);
> if (ret) {
> pci_epf_free_space(epf, epf_test->reg[bar], bar);
> dev_err(dev, "failed to set BAR%d\n", bar);
> --
> 2.14.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
2018-03-16 18:02 ` Lorenzo Pieralisi
@ 2018-03-17 0:55 ` Niklas Cassel
2018-03-19 12:36 ` Lorenzo Pieralisi
0 siblings, 1 reply; 20+ messages in thread
From: Niklas Cassel @ 2018-03-17 0:55 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: kishon, Bjorn Helgaas, Sekhar Nori, John Keeping, Shawn Lin,
Cyrille Pitchen, linux-pci, linux-kernel
On Fri, Mar 16, 2018 at 06:02:20PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Mar 08, 2018 at 02:33:26PM +0100, Niklas Cassel wrote:
> > If a BAR supports 64-bit width or not depends on the hardware,
> > and should thus not depend on sizeof(dma_addr_t).
> >
> > Since this driver is generic, default to always using BAR width
> > of 32-bits. 64-bit BARs can easily be tested by replacing
> > PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> > in bar_flags.
> >
> > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > ---
> > Note to Lorenzo/Bjorn:
> > It is not trivial to convert the bar_size + bar_flags +
> > struct pci_epf->bar member array to an array of struct resources,
> > since we need to be able to store the addresses returned
> > by dma_alloc_coherent(), which is of type dma_addr_t.
> > struct resource uses resource_size_t, which is defined as phys_addr_t.
> > E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
> >
> > drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 800da09d9005..7c70433b11a7 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> > };
> >
> > static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> > +static int bar_flags[] = {
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> > +};
>
> Niklas,
>
> I think you are almost there, I have one question though to address
> that can even simplify the patchset.
>
> If, according to your own commit logs (and my reading of the code), the
> Cadence driver makes a decision on the BAR size just by checking the
> corresponding region size (I would be happy to hear the reason
> underpinning that choice, BTW), why can't we do the same for DWC (ie to
> let the DWC driver decides whether a BAR should be 64 or 32 bits ?)
We could, but I think that would be a mistake.
The API that the user/"endpoint function" has available to configure
the BARs:
pci_epc_set_bar()
If the user, for some reason, wants to configure a BAR with a
64-bit width, even though the BAR size is less than 4 GB,
I think that the API should allow that.
This would not be possible if pci_epc_set_bar() would start to
ignore PCI_BASE_ADDRESS_MEM_TYPE_64 from the flags parameter
(and BAR width thus only being controlled by the size parameter).
int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, enum pci_barno bar,
dma_addr_t bar_phys, size_t size, int flags);
Best regards,
Niklas
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
2018-03-17 0:55 ` Niklas Cassel
@ 2018-03-19 12:36 ` Lorenzo Pieralisi
0 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-19 12:36 UTC (permalink / raw)
To: Niklas Cassel
Cc: kishon, Bjorn Helgaas, Sekhar Nori, John Keeping, Shawn Lin,
Cyrille Pitchen, linux-pci, linux-kernel
On Sat, Mar 17, 2018 at 01:55:13AM +0100, Niklas Cassel wrote:
> On Fri, Mar 16, 2018 at 06:02:20PM +0000, Lorenzo Pieralisi wrote:
> > On Thu, Mar 08, 2018 at 02:33:26PM +0100, Niklas Cassel wrote:
> > > If a BAR supports 64-bit width or not depends on the hardware,
> > > and should thus not depend on sizeof(dma_addr_t).
> > >
> > > Since this driver is generic, default to always using BAR width
> > > of 32-bits. 64-bit BARs can easily be tested by replacing
> > > PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> > > in bar_flags.
> > >
> > > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > > ---
> > > Note to Lorenzo/Bjorn:
> > > It is not trivial to convert the bar_size + bar_flags +
> > > struct pci_epf->bar member array to an array of struct resources,
> > > since we need to be able to store the addresses returned
> > > by dma_alloc_coherent(), which is of type dma_addr_t.
> > > struct resource uses resource_size_t, which is defined as phys_addr_t.
> > > E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
> > >
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------
> > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index 800da09d9005..7c70433b11a7 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> > > };
> > >
> > > static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> > > +static int bar_flags[] = {
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > > + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> > > +};
> >
> > Niklas,
> >
> > I think you are almost there, I have one question though to address
> > that can even simplify the patchset.
> >
> > If, according to your own commit logs (and my reading of the code), the
> > Cadence driver makes a decision on the BAR size just by checking the
> > corresponding region size (I would be happy to hear the reason
> > underpinning that choice, BTW), why can't we do the same for DWC (ie to
> > let the DWC driver decides whether a BAR should be 64 or 32 bits ?)
>
> We could, but I think that would be a mistake.
>
> The API that the user/"endpoint function" has available to configure
> the BARs:
> pci_epc_set_bar()
>
> If the user, for some reason, wants to configure a BAR with a
> 64-bit width, even though the BAR size is less than 4 GB,
> I think that the API should allow that.
Yes, that's a good point, thanks a lot for the explanation.
Lorenzo
> This would not be possible if pci_epc_set_bar() would start to
> ignore PCI_BASE_ADDRESS_MEM_TYPE_64 from the flags parameter
> (and BAR width thus only being controlled by the size parameter).
>
> int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, enum pci_barno bar,
> dma_addr_t bar_phys, size_t size, int flags);
>
>
>
> Best regards,
> Niklas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
2018-03-08 13:33 ` [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t Niklas Cassel
2018-03-16 18:02 ` Lorenzo Pieralisi
@ 2018-03-21 5:29 ` Kishon Vijay Abraham I
1 sibling, 0 replies; 20+ messages in thread
From: Kishon Vijay Abraham I @ 2018-03-21 5:29 UTC (permalink / raw)
To: Niklas Cassel, Lorenzo Pieralisi, Bjorn Helgaas, Sekhar Nori,
John Keeping, Shawn Lin, Niklas Cassel, Cyrille Pitchen
Cc: linux-pci, linux-kernel
On Thursday 08 March 2018 07:03 PM, Niklas Cassel wrote:
> If a BAR supports 64-bit width or not depends on the hardware,
> and should thus not depend on sizeof(dma_addr_t).
>
> Since this driver is generic, default to always using BAR width
> of 32-bits. 64-bit BARs can easily be tested by replacing
> PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> in bar_flags.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> Note to Lorenzo/Bjorn:
> It is not trivial to convert the bar_size + bar_flags +
> struct pci_epf->bar member array to an array of struct resources,
> since we need to be able to store the addresses returned
> by dma_alloc_coherent(), which is of type dma_addr_t.
> struct resource uses resource_size_t, which is defined as phys_addr_t.
> E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
>
> drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 800da09d9005..7c70433b11a7 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> };
>
> static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> +static int bar_flags[] = {
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> +};
>
> static int pci_epf_test_copy(struct pci_epf_test *epf_test)
> {
> @@ -358,7 +366,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> static int pci_epf_test_set_bar(struct pci_epf *epf)
> {
> - int flags;
> int bar;
> int ret;
> struct pci_epf_bar *epf_bar;
> @@ -367,15 +374,11 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> enum pci_barno test_reg_bar = epf_test->test_reg_bar;
>
> - flags = PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32;
> - if (sizeof(dma_addr_t) == 0x8)
> - flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> -
> for (bar = BAR_0; bar <= BAR_5; bar++) {
> epf_bar = &epf->bar[bar];
> ret = pci_epc_set_bar(epc, epf->func_no, bar,
> epf_bar->phys_addr,
> - epf_bar->size, flags);
> + epf_bar->size, bar_flags[bar]);
> if (ret) {
> pci_epf_free_space(epf, epf_test->reg[bar], bar);
> dev_err(dev, "failed to set BAR%d\n", bar);
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 2/5] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
2018-03-08 13:33 ` [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t Niklas Cassel
@ 2018-03-08 13:33 ` Niklas Cassel
2018-03-21 5:49 ` Kishon Vijay Abraham I
2018-03-08 13:33 ` [PATCH v4 3/5] PCI: designware-ep: Make dw_pcie_ep_reset_bar() " Niklas Cassel
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Niklas Cassel @ 2018-03-08 13:33 UTC (permalink / raw)
To: kishon, Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas
Cc: Niklas Cassel, linux-pci, linux-kernel
Since a 64-bit BAR consists of a BAR pair, we need to write to both
BARs in the BAR pair to setup the BAR properly.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/pci/dwc/pcie-designware-ep.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 9236b998327f..946bbdf53c4d 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -136,8 +136,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
return ret;
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writel_dbi2(pci, reg, size - 1);
- dw_pcie_writel_dbi(pci, reg, flags);
+ if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+ dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
+ dw_pcie_writel_dbi(pci, reg, flags);
+ dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
+ dw_pcie_writel_dbi(pci, reg + 4, 0);
+ } else {
+ dw_pcie_writel_dbi2(pci, reg, size - 1);
+ dw_pcie_writel_dbi(pci, reg, flags);
+ }
dw_pcie_dbi_ro_wr_dis(pci);
return 0;
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 2/5] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly
2018-03-08 13:33 ` [PATCH v4 2/5] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly Niklas Cassel
@ 2018-03-21 5:49 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 20+ messages in thread
From: Kishon Vijay Abraham I @ 2018-03-21 5:49 UTC (permalink / raw)
To: Niklas Cassel, Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas
Cc: Niklas Cassel, linux-pci, linux-kernel
Hi Niklas,
On Thursday 08 March 2018 07:03 PM, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to setup the BAR properly.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> drivers/pci/dwc/pcie-designware-ep.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index 9236b998327f..946bbdf53c4d 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -136,8 +136,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> return ret;
>
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg, size - 1);
> - dw_pcie_writel_dbi(pci, reg, flags);
> + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg + 4, 0);
I think we should check in pci_epc_set_bar to make sure BAR_5 cannot have
PCI_BASE_ADDRESS_MEM_TYPE_64 flag set as this might lead undesired result.
Thanks
Kishon
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 3/5] PCI: designware-ep: Make dw_pcie_ep_reset_bar() handle 64-bit BARs properly
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
2018-03-08 13:33 ` [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t Niklas Cassel
2018-03-08 13:33 ` [PATCH v4 2/5] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly Niklas Cassel
@ 2018-03-08 13:33 ` Niklas Cassel
2018-03-21 5:54 ` Kishon Vijay Abraham I
2018-03-08 13:33 ` [PATCH v4 4/5] PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was set-up Niklas Cassel
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Niklas Cassel @ 2018-03-08 13:33 UTC (permalink / raw)
To: kishon, Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas
Cc: Niklas Cassel, linux-pci, linux-kernel
Since a 64-bit BAR consists of a BAR pair, we need to write to both
BARs in the BAR pair to clear the BAR properly.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/pci/dwc/pcie-designware-ep.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 946bbdf53c4d..b20b2651caf9 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -22,11 +22,18 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
{
u32 reg;
+ u32 val;
reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+ val = dw_pcie_readl_dbi(pci, reg);
dw_pcie_dbi_ro_wr_en(pci);
dw_pcie_writel_dbi2(pci, reg, 0x0);
dw_pcie_writel_dbi(pci, reg, 0x0);
+ if (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
+ (val & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+ dw_pcie_writel_dbi2(pci, reg + 4, 0x0);
+ dw_pcie_writel_dbi(pci, reg + 4, 0x0);
+ }
dw_pcie_dbi_ro_wr_dis(pci);
}
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/5] PCI: designware-ep: Make dw_pcie_ep_reset_bar() handle 64-bit BARs properly
2018-03-08 13:33 ` [PATCH v4 3/5] PCI: designware-ep: Make dw_pcie_ep_reset_bar() " Niklas Cassel
@ 2018-03-21 5:54 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 20+ messages in thread
From: Kishon Vijay Abraham I @ 2018-03-21 5:54 UTC (permalink / raw)
To: Niklas Cassel, Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas
Cc: Niklas Cassel, linux-pci, linux-kernel
Hi Niklas,
On Thursday 08 March 2018 07:03 PM, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to clear the BAR properly.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> drivers/pci/dwc/pcie-designware-ep.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index 946bbdf53c4d..b20b2651caf9 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -22,11 +22,18 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> {
> u32 reg;
> + u32 val;
>
> reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> + val = dw_pcie_readl_dbi(pci, reg);
> dw_pcie_dbi_ro_wr_en(pci);
> dw_pcie_writel_dbi2(pci, reg, 0x0);
> dw_pcie_writel_dbi(pci, reg, 0x0);
> + if (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
> + (val & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> + dw_pcie_writel_dbi2(pci, reg + 4, 0x0);
> + dw_pcie_writel_dbi(pci, reg + 4, 0x0);
> + }
same comment as previous patch apply here too.
Thanks
Kishon
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 4/5] PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was set-up
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
` (2 preceding siblings ...)
2018-03-08 13:33 ` [PATCH v4 3/5] PCI: designware-ep: Make dw_pcie_ep_reset_bar() " Niklas Cassel
@ 2018-03-08 13:33 ` Niklas Cassel
2018-03-21 6:06 ` Kishon Vijay Abraham I
2018-03-08 13:33 ` [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly Niklas Cassel
2018-03-19 16:52 ` [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Lorenzo Pieralisi
5 siblings, 1 reply; 20+ messages in thread
From: Niklas Cassel @ 2018-03-08 13:33 UTC (permalink / raw)
To: kishon, Alan Douglas, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Lorenzo Pieralisi, Sekhar Nori, Shawn Lin, Niklas Cassel,
Cyrille Pitchen
Cc: Niklas Cassel, linux-pci, linux-kernel
In order to properly handle 64-bit BARs, we need to know what BAR width
that was actually set-up by specific pci_epc_set_bar() implementations.
This is done so that we can know if we need to skip a BAR,
since a 64-bit BAR consists of a BAR pair.
It is important to know the BAR width that was actually set-up,
since some drivers, like the Cadence EP controller, does not
simply look at PCI_BASE_ADDRESS_MEM_TYPE_64, as it configures
all BARs larger than 2G as 64-bit.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
drivers/pci/dwc/pcie-designware-ep.c | 4 ++--
drivers/pci/endpoint/functions/pci-epf-test.c | 7 ++++++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/cadence/pcie-cadence-ep.c b/drivers/pci/cadence/pcie-cadence-ep.c
index 3c3a97743453..0e4cc4cca56d 100644
--- a/drivers/pci/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/cadence/pcie-cadence-ep.c
@@ -135,7 +135,7 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, enum pci_barno bar,
CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, ctrl));
cdns_pcie_writel(pcie, reg, cfg);
- return 0;
+ return is_64bits ? 1 : 0;
}
static void cdns_pcie_ep_clear_bar(struct pci_epc *epc, u8 fn,
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index b20b2651caf9..f3c19f8ff8e5 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -139,7 +139,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
as_type = DW_PCIE_AS_IO;
ret = dw_pcie_ep_inbound_atu(ep, bar, bar_phys, as_type);
- if (ret)
+ if (ret < 0)
return ret;
dw_pcie_dbi_ro_wr_en(pci);
@@ -154,7 +154,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
}
dw_pcie_dbi_ro_wr_dis(pci);
- return 0;
+ return (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 1 : 0;
}
static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr,
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 7c70433b11a7..09878e011284 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -379,12 +379,17 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
ret = pci_epc_set_bar(epc, epf->func_no, bar,
epf_bar->phys_addr,
epf_bar->size, bar_flags[bar]);
- if (ret) {
+ if (ret < 0) {
pci_epf_free_space(epf, epf_test->reg[bar], bar);
dev_err(dev, "failed to set BAR%d\n", bar);
if (bar == test_reg_bar)
return ret;
}
+ /*
+ * pci_epc_set_bar() returns 1 if a 64-bit BAR was set-up,
+ * or 0 if a 32-bit BAR was set-up.
+ */
+ bar += ret;
}
return 0;
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 4/5] PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was set-up
2018-03-08 13:33 ` [PATCH v4 4/5] PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was set-up Niklas Cassel
@ 2018-03-21 6:06 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 20+ messages in thread
From: Kishon Vijay Abraham I @ 2018-03-21 6:06 UTC (permalink / raw)
To: Niklas Cassel, Alan Douglas, Bjorn Helgaas, Jingoo Han,
Joao Pinto, Lorenzo Pieralisi, Sekhar Nori, Shawn Lin,
Niklas Cassel, Cyrille Pitchen
Cc: linux-pci, linux-kernel
Hi Niklas,
On Thursday 08 March 2018 07:03 PM, Niklas Cassel wrote:
> In order to properly handle 64-bit BARs, we need to know what BAR width
> that was actually set-up by specific pci_epc_set_bar() implementations.
>
> This is done so that we can know if we need to skip a BAR,
> since a 64-bit BAR consists of a BAR pair.
>
> It is important to know the BAR width that was actually set-up,
> since some drivers, like the Cadence EP controller, does not
> simply look at PCI_BASE_ADDRESS_MEM_TYPE_64, as it configures
> all BARs larger than 2G as 64-bit.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
> drivers/pci/dwc/pcie-designware-ep.c | 4 ++--
> drivers/pci/endpoint/functions/pci-epf-test.c | 7 ++++++-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/cadence/pcie-cadence-ep.c b/drivers/pci/cadence/pcie-cadence-ep.c
> index 3c3a97743453..0e4cc4cca56d 100644
> --- a/drivers/pci/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/cadence/pcie-cadence-ep.c
> @@ -135,7 +135,7 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, enum pci_barno bar,
> CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, ctrl));
> cdns_pcie_writel(pcie, reg, cfg);
>
> - return 0;
> + return is_64bits ? 1 : 0;
> }
>
> static void cdns_pcie_ep_clear_bar(struct pci_epc *epc, u8 fn,
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index b20b2651caf9..f3c19f8ff8e5 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -139,7 +139,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> as_type = DW_PCIE_AS_IO;
>
> ret = dw_pcie_ep_inbound_atu(ep, bar, bar_phys, as_type);
> - if (ret)
> + if (ret < 0)
> return ret;
>
> dw_pcie_dbi_ro_wr_en(pci);
> @@ -154,7 +154,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> }
> dw_pcie_dbi_ro_wr_dis(pci);
>
> - return 0;
> + return (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 1 : 0;
> }
>
> static int dw_pcie_find_index(struct dw_pcie_ep *ep, phys_addr_t addr,
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 7c70433b11a7..09878e011284 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -379,12 +379,17 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> ret = pci_epc_set_bar(epc, epf->func_no, bar,
> epf_bar->phys_addr,
> epf_bar->size, bar_flags[bar]);
> - if (ret) {
> + if (ret < 0) {
> pci_epf_free_space(epf, epf_test->reg[bar], bar);
> dev_err(dev, "failed to set BAR%d\n", bar);
> if (bar == test_reg_bar)
> return ret;
> }
> + /*
> + * pci_epc_set_bar() returns 1 if a 64-bit BAR was set-up,
> + * or 0 if a 32-bit BAR was set-up.
> + */
> + bar += ret;
I'd prefer having a new argument bool *is_64bit in set_bar rather than having
to work with return value of set_bar.
Even otherwise, can't we set PCI_BASE_ADDRESS_MEM_TYPE_64 based on size in
pci-epf-test itself?
Thanks
Kishon
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
` (3 preceding siblings ...)
2018-03-08 13:33 ` [PATCH v4 4/5] PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was set-up Niklas Cassel
@ 2018-03-08 13:33 ` Niklas Cassel
2018-03-15 13:22 ` Greg Kroah-Hartman
` (2 more replies)
2018-03-19 16:52 ` [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Lorenzo Pieralisi
5 siblings, 3 replies; 20+ messages in thread
From: Niklas Cassel @ 2018-03-08 13:33 UTC (permalink / raw)
To: kishon, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman
Cc: Niklas Cassel, linux-pci, linux-kernel
A 64-bit BAR consists of a BAR pair, where the second BAR has the
upper bits, so we cannot simply call pci_ioremap_bar() on every single
BAR index.
The second BAR in a BAR pair will not have the IORESOURCE_MEM resource
flag set. Only call ioremap on BARs that have the IORESOURCE_MEM
resource flag set.
pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR1
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR3
pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
pci-endpoint-test 0000:01:00.0: failed to read BAR5
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/misc/pci_endpoint_test.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 320276f42653..fe8897e64635 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -534,12 +534,14 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
}
for (bar = BAR_0; bar <= BAR_5; bar++) {
- base = pci_ioremap_bar(pdev, bar);
- if (!base) {
- dev_err(dev, "failed to read BAR%d\n", bar);
- WARN_ON(bar == test_reg_bar);
+ if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
+ base = pci_ioremap_bar(pdev, bar);
+ if (!base) {
+ dev_err(dev, "failed to read BAR%d\n", bar);
+ WARN_ON(bar == test_reg_bar);
+ }
+ test->bar[bar] = base;
}
- test->bar[bar] = base;
}
test->base = test->bar[test_reg_bar];
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-08 13:33 ` [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly Niklas Cassel
@ 2018-03-15 13:22 ` Greg Kroah-Hartman
2018-03-16 12:55 ` Niklas Cassel
2018-03-21 3:59 ` Niklas Cassel
2018-03-21 5:55 ` Kishon Vijay Abraham I
2 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-15 13:22 UTC (permalink / raw)
To: Niklas Cassel
Cc: kishon, Lorenzo Pieralisi, Arnd Bergmann, Niklas Cassel,
linux-pci, linux-kernel
On Thu, Mar 08, 2018 at 02:33:30PM +0100, Niklas Cassel wrote:
> A 64-bit BAR consists of a BAR pair, where the second BAR has the
> upper bits, so we cannot simply call pci_ioremap_bar() on every single
> BAR index.
>
> The second BAR in a BAR pair will not have the IORESOURCE_MEM resource
> flag set. Only call ioremap on BARs that have the IORESOURCE_MEM
> resource flag set.
>
> pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR1
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR3
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR5
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
Where are the 4 other patches in this series? I can't just only apply
the last one, right?
Please be careful when sending patches to properly notify everyone...
this is now dropped from my patch queue.
greg k-h
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-15 13:22 ` Greg Kroah-Hartman
@ 2018-03-16 12:55 ` Niklas Cassel
0 siblings, 0 replies; 20+ messages in thread
From: Niklas Cassel @ 2018-03-16 12:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: kishon, Lorenzo Pieralisi, Arnd Bergmann, linux-pci, linux-kernel, joe
On Thu, Mar 15, 2018 at 02:22:13PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 08, 2018 at 02:33:30PM +0100, Niklas Cassel wrote:
> > A 64-bit BAR consists of a BAR pair, where the second BAR has the
> > upper bits, so we cannot simply call pci_ioremap_bar() on every single
> > BAR index.
> >
> > The second BAR in a BAR pair will not have the IORESOURCE_MEM resource
> > flag set. Only call ioremap on BARs that have the IORESOURCE_MEM
> > resource flag set.
> >
> > pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
> > pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
> > pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
> > pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
> > pci-endpoint-test 0000:01:00.0: failed to read BAR1
> > pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
> > pci-endpoint-test 0000:01:00.0: failed to read BAR3
> > pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
> > pci-endpoint-test 0000:01:00.0: failed to read BAR5
> >
> > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > ---
> > drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
>
> Where are the 4 other patches in this series? I can't just only apply
> the last one, right?
http://patchwork.ozlabs.org/project/linux-pci/list/?series=32658
I'm expecting all patches to go via Lorenzo's pci tree.
>
> Please be careful when sending patches to properly notify everyone...
I'm using scripts from Joe Perches that run get_maintainer.pl,
these scripts are then used as --to-cmd and --cc-cmd for git send-email:
https://lkml.org/lkml/2016/9/14/482
(I did have to modify them so they handle rerolled patch series.)
BTW. Joe, do you have updated versions of your scripts?
Greg, I'm assuming that you wouldn't want to be cc'd on all patches in the
series, since all except the last one are pci related.
However, perhaps it would be nice to add all those who are cc'd in the
individual patches, as cc to the cover letter, that way it might be easier
to figure out what tree will most likely merge a certain patch series.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-08 13:33 ` [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly Niklas Cassel
2018-03-15 13:22 ` Greg Kroah-Hartman
@ 2018-03-21 3:59 ` Niklas Cassel
2018-03-21 9:22 ` Greg Kroah-Hartman
2018-03-21 5:55 ` Kishon Vijay Abraham I
2 siblings, 1 reply; 20+ messages in thread
From: Niklas Cassel @ 2018-03-21 3:59 UTC (permalink / raw)
To: kishon, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman
Cc: linux-pci, linux-kernel
Hello Greg,
Lorenzo is fine with this series
( https://marc.info/?l=linux-kernel&m=152147837619191&w=2 )
However, he wants your ack on this patch before merging.
Could you please have a look at this patch?
Kind regards,
Niklas
On Thu, Mar 08, 2018 at 02:33:30PM +0100, Niklas Cassel wrote:
> A 64-bit BAR consists of a BAR pair, where the second BAR has the
> upper bits, so we cannot simply call pci_ioremap_bar() on every single
> BAR index.
>
> The second BAR in a BAR pair will not have the IORESOURCE_MEM resource
> flag set. Only call ioremap on BARs that have the IORESOURCE_MEM
> resource flag set.
>
> pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR1
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR3
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR5
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
> drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 320276f42653..fe8897e64635 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -534,12 +534,14 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
> }
>
> for (bar = BAR_0; bar <= BAR_5; bar++) {
> - base = pci_ioremap_bar(pdev, bar);
> - if (!base) {
> - dev_err(dev, "failed to read BAR%d\n", bar);
> - WARN_ON(bar == test_reg_bar);
> + if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> + base = pci_ioremap_bar(pdev, bar);
> + if (!base) {
> + dev_err(dev, "failed to read BAR%d\n", bar);
> + WARN_ON(bar == test_reg_bar);
> + }
> + test->bar[bar] = base;
> }
> - test->bar[bar] = base;
> }
>
> test->base = test->bar[test_reg_bar];
> --
> 2.14.2
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-21 3:59 ` Niklas Cassel
@ 2018-03-21 9:22 ` Greg Kroah-Hartman
0 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-21 9:22 UTC (permalink / raw)
To: Niklas Cassel
Cc: kishon, Lorenzo Pieralisi, Arnd Bergmann, linux-pci, linux-kernel
On Wed, Mar 21, 2018 at 04:59:31AM +0100, Niklas Cassel wrote:
> Hello Greg,
>
> Lorenzo is fine with this series
> ( https://marc.info/?l=linux-kernel&m=152147837619191&w=2 )
>
> However, he wants your ack on this patch before merging.
No need for my ack, if you all want to maintain this driver, it's fine
with me, go ahead and merge what you want :)
greg k-h
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly
2018-03-08 13:33 ` [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly Niklas Cassel
2018-03-15 13:22 ` Greg Kroah-Hartman
2018-03-21 3:59 ` Niklas Cassel
@ 2018-03-21 5:55 ` Kishon Vijay Abraham I
2 siblings, 0 replies; 20+ messages in thread
From: Kishon Vijay Abraham I @ 2018-03-21 5:55 UTC (permalink / raw)
To: Niklas Cassel, Lorenzo Pieralisi, Arnd Bergmann, Greg Kroah-Hartman
Cc: Niklas Cassel, linux-pci, linux-kernel
On Thursday 08 March 2018 07:03 PM, Niklas Cassel wrote:
> A 64-bit BAR consists of a BAR pair, where the second BAR has the
> upper bits, so we cannot simply call pci_ioremap_bar() on every single
> BAR index.
>
> The second BAR in a BAR pair will not have the IORESOURCE_MEM resource
> flag set. Only call ioremap on BARs that have the IORESOURCE_MEM
> resource flag set.
>
> pci 0000:01:00.0: BAR 4: assigned [mem 0xc0300000-0xc031ffff 64bit]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xc0320000-0xc03203ff 64bit]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xc0320400-0xc03204ff 64bit]
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 1: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR1
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 3: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR3
> pci-endpoint-test 0000:01:00.0: can't ioremap BAR 5: [??? 0x00000000 flags 0x0]
> pci-endpoint-test 0000:01:00.0: failed to read BAR5
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 320276f42653..fe8897e64635 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -534,12 +534,14 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
> }
>
> for (bar = BAR_0; bar <= BAR_5; bar++) {
> - base = pci_ioremap_bar(pdev, bar);
> - if (!base) {
> - dev_err(dev, "failed to read BAR%d\n", bar);
> - WARN_ON(bar == test_reg_bar);
> + if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> + base = pci_ioremap_bar(pdev, bar);
> + if (!base) {
> + dev_err(dev, "failed to read BAR%d\n", bar);
> + WARN_ON(bar == test_reg_bar);
> + }
> + test->bar[bar] = base;
> }
> - test->bar[bar] = base;
> }
>
> test->base = test->bar[test_reg_bar];
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes
2018-03-08 13:33 [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Niklas Cassel
` (4 preceding siblings ...)
2018-03-08 13:33 ` [PATCH v4 5/5] misc: pci_endpoint_test: Handle 64-bit BARs properly Niklas Cassel
@ 2018-03-19 16:52 ` Lorenzo Pieralisi
2018-03-21 4:10 ` Niklas Cassel
5 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-19 16:52 UTC (permalink / raw)
To: Niklas Cassel; +Cc: kishon, linux-pci, Niklas Cassel, linux-kernel
On Thu, Mar 08, 2018 at 02:33:25PM +0100, Niklas Cassel wrote:
> PCI endpoint fixes to improve the way 64-bit BARs are handled.
>
>
> There are still future improvements that could be made:
>
> pci-epf-test.c always allocates space for
> 6 BARs, even when using 64-bit BARs (which
> really only requires us to allocate 3 BARs).
>
> pcitest.sh will print "NOT OKAY" for BAR1,
> BAR3, and BAR5 when using 64-bit BARs.
> This could probably be improved to say
> something like "N/A (64-bit BAR)".
>
> Niklas Cassel (5):
> PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
> PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs
> properly
> PCI: designware-ep: Make dw_pcie_ep_reset_bar() handle 64-bit BARs
> properly
> PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was
> set-up
> misc: pci_endpoint_test: Handle 64-bit BARs properly
>
> drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
> drivers/pci/dwc/pcie-designware-ep.c | 22 ++++++++++++++++++----
> drivers/pci/endpoint/functions/pci-epf-test.c | 22 +++++++++++++++-------
> 4 files changed, 41 insertions(+), 17 deletions(-)
Niklas,
I am fine with the series but I'd need Kishon ACKs (and Greg's for the
last patch), I am ready to queue them then.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes
2018-03-19 16:52 ` [PATCH v4 0/5] PCI endpoint 64-bit BAR fixes Lorenzo Pieralisi
@ 2018-03-21 4:10 ` Niklas Cassel
0 siblings, 0 replies; 20+ messages in thread
From: Niklas Cassel @ 2018-03-21 4:10 UTC (permalink / raw)
To: kishon; +Cc: linux-pci, linux-kernel, lorenzo.pieralisi
On Mon, Mar 19, 2018 at 04:52:34PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Mar 08, 2018 at 02:33:25PM +0100, Niklas Cassel wrote:
> > PCI endpoint fixes to improve the way 64-bit BARs are handled.
> >
> >
> > There are still future improvements that could be made:
> >
> > pci-epf-test.c always allocates space for
> > 6 BARs, even when using 64-bit BARs (which
> > really only requires us to allocate 3 BARs).
> >
> > pcitest.sh will print "NOT OKAY" for BAR1,
> > BAR3, and BAR5 when using 64-bit BARs.
> > This could probably be improved to say
> > something like "N/A (64-bit BAR)".
> >
> > Niklas Cassel (5):
> > PCI: endpoint: BAR width should not depend on sizeof dma_addr_t
> > PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs
> > properly
> > PCI: designware-ep: Make dw_pcie_ep_reset_bar() handle 64-bit BARs
> > properly
> > PCI: endpoint: Make pci_epc_set_bar() return the BAR width that was
> > set-up
> > misc: pci_endpoint_test: Handle 64-bit BARs properly
> >
> > drivers/misc/pci_endpoint_test.c | 12 +++++++-----
> > drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
> > drivers/pci/dwc/pcie-designware-ep.c | 22 ++++++++++++++++++----
> > drivers/pci/endpoint/functions/pci-epf-test.c | 22 +++++++++++++++-------
> > 4 files changed, 41 insertions(+), 17 deletions(-)
>
> Niklas,
>
> I am fine with the series but I'd need Kishon ACKs (and Greg's for the
> last patch), I am ready to queue them then.
Hello Kishon,
could you please have a look at this series?
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 20+ messages in thread