linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/7] minor PCI endpoint cleanups
@ 2024-03-20 11:31 Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 1/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop Niklas Cassel
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, Shawn Lin,
	Heiko Stuebner, Kishon Vijay Abraham I
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci,
	linux-rockchip, linux-arm-kernel

Hello all,

This series used to be called:
"PCI: endpoint: set prefetchable bit for 64-bit BARs"

However, since after discussions with Arnd and Mani, that patch has been
dropped, however, the other cleanups are still worth including IMO, thus
the series has been renamed.


Changes since v3:
-Picked up tags from Mani.
-Fixed minor comments from Mani.
-Dropped patch [PATCH v3 1/9] PCI: endpoint: pci-epf-test: Fix incorrect
 loop increment.
-Dropped patch [PATCH v3 9/9] PCI: endpoint: Set prefetch when allocating
 memory for 64-bit BARs.
-Reordered some of the patches to have a more logical ordering.


Niklas Cassel (7):
  PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop
  PCI: endpoint: Allocate a 64-bit BAR if that is the only option
  PCI: endpoint: pci-epf-test: Remove superfluous code
  PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop
  PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind()
  PCI: cadence: Set a 64-bit BAR if requested
  PCI: rockchip-ep: Set a 64-bit BAR if requested

 .../pci/controller/cadence/pcie-cadence-ep.c  |  5 +-
 drivers/pci/controller/pcie-rockchip-ep.c     |  2 +-
 drivers/pci/endpoint/functions/pci-epf-test.c | 62 +++++--------------
 drivers/pci/endpoint/pci-epf-core.c           |  9 ++-
 4 files changed, 23 insertions(+), 55 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 2/7] PCI: endpoint: Allocate a 64-bit BAR if that is the only option Niklas Cassel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

Make pci-epf-test use pci_epc_get_next_free_bar() just like pci-epf-ntb.c
and pci-epf-vntb.c.

Using pci_epc_get_next_free_bar() also makes it more obvious that
pci-epf-test does no special configuration at all.

(The only configuration pci-epf-test does is setting
PCI_BASE_ADDRESS_MEM_TYPE_64 if epc_features has marked the specific BAR
as only_64bit. pci_epc_get_next_free_bar() already takes only_64bit into
account when looping.)

This way, the code is more consistent between EPF drivers, and pci-epf-test
does not need to explicitly check if the BAR is reserved, or if the index
belongs to a BAR succeeding a 64-bit only BAR.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index cd4ffb39dcdc..16dfd61cd9fb 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -817,14 +817,13 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	struct device *dev = &epf->dev;
-	struct pci_epf_bar *epf_bar;
 	size_t msix_table_size = 0;
 	size_t test_reg_bar_size;
 	size_t pba_size = 0;
 	bool msix_capable;
 	void *base;
-	int bar, add;
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	enum pci_barno bar;
 	const struct pci_epc_features *epc_features;
 	size_t test_reg_size;
 
@@ -849,16 +848,14 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	}
 	epf_test->reg[test_reg_bar] = base;
 
-	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
-		epf_bar = &epf->bar[bar];
-		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
+	for (bar = BAR_0; bar < PCI_STD_NUM_BARS; bar++) {
+		bar = pci_epc_get_next_free_bar(epc_features, bar);
+		if (bar == NO_BAR)
+			break;
 
 		if (bar == test_reg_bar)
 			continue;
 
-		if (epc_features->bar[bar].type == BAR_RESERVED)
-			continue;
-
 		base = pci_epf_alloc_space(epf, bar_size[bar], bar,
 					   epc_features, PRIMARY_INTERFACE);
 		if (!base)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/7] PCI: endpoint: Allocate a 64-bit BAR if that is the only option
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 1/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 3/7] PCI: endpoint: pci-epf-test: Remove superfluous code Niklas Cassel
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

pci_epf_alloc_space() already sets the 64-bit flag if the BAR size is
larger than 2GB, even if the caller did not explicitly request a 64-bit
BAR.

Thus, let pci_epf_alloc_space() also set the 64-bit flag if the hardware
description says that the specific BAR can only be 64-bit.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/endpoint/pci-epf-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 0a28a0b0911b..323f2a60ab16 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -255,6 +255,8 @@ EXPORT_SYMBOL_GPL(pci_epf_free_space);
  * @type: Identifies if the allocation is for primary EPC or secondary EPC
  *
  * Invoke to allocate memory for the PCI EPF register space.
+ * Flag PCI_BASE_ADDRESS_MEM_TYPE_64 will automatically get set if the BAR
+ * can only be a 64-bit BAR, or if the requested size is larger than 2 GB.
  */
 void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 			  const struct pci_epc_features *epc_features,
@@ -304,9 +306,10 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
 	epf_bar[bar].addr = space;
 	epf_bar[bar].size = size;
 	epf_bar[bar].barno = bar;
-	epf_bar[bar].flags |= upper_32_bits(size) ?
-				PCI_BASE_ADDRESS_MEM_TYPE_64 :
-				PCI_BASE_ADDRESS_MEM_TYPE_32;
+	if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+	else
+		epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
 
 	return space;
 }
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/7] PCI: endpoint: pci-epf-test: Remove superfluous code
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 1/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 2/7] PCI: endpoint: Allocate a 64-bit BAR if that is the only option Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop Niklas Cassel
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

The only reason why we call pci_epf_configure_bar() is to set
PCI_BASE_ADDRESS_MEM_TYPE_64 in case the hardware requires it.

However, this flag is now automatically set when allocating a BAR that
can only be a 64-bit BAR, so we can drop pci_epf_configure_bar()
completely.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 16dfd61cd9fb..0a83a1901bb7 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -867,19 +867,6 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	return 0;
 }
 
-static void pci_epf_configure_bar(struct pci_epf *epf,
-				  const struct pci_epc_features *epc_features)
-{
-	struct pci_epf_bar *epf_bar;
-	int i;
-
-	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
-		epf_bar = &epf->bar[i];
-		if (epc_features->bar[i].only_64bit)
-			epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
-	}
-}
-
 static int pci_epf_test_bind(struct pci_epf *epf)
 {
 	int ret;
@@ -904,7 +891,6 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 	test_reg_bar = pci_epc_get_first_free_bar(epc_features);
 	if (test_reg_bar < 0)
 		return -EINVAL;
-	pci_epf_configure_bar(epf, epc_features);
 
 	epf_test->test_reg_bar = test_reg_bar;
 	epf_test->epc_features = epc_features;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
                   ` (2 preceding siblings ...)
  2024-03-20 11:31 ` [PATCH v4 3/7] PCI: endpoint: pci-epf-test: Remove superfluous code Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-22 10:31   ` Manivannan Sadhasivam
  2024-03-20 11:31 ` [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind() Niklas Cassel
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

Simplify the loop in pci_epf_test_set_bar().
If we allocated memory for the BAR, we need to call set_bar() for that
BAR, if we did not allocated memory for that BAR, we need to skip.
It is as simple as that. This also matches the logic in
pci_epf_test_unbind().

A 64-bit BAR will still only be one allocation, with the BAR succeeding
the 64-bit BAR being null.

While at it, remove the misleading comment.
A EPC .set_bar() callback should never change the epf_bar->flags.
(E.g. to set a 64-bit BAR if we requested a 32-bit BAR.)

A .set_bar() callback should do what we request it to do.
If it can't satisfy the request, it should return an error.

If platform has a specific requirement, e.g. that a certain BAR has to
be a 64-bit BAR, then it should specify that by setting the .only_64bit
flag for that specific BAR in epc_features->bar[], such that
pci_epf_alloc_space() will return a epf_bar with the 64-bit flag set.
(Such that .set_bar() will receive a request to set a 64-bit BAR.)

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 21 ++++---------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 0a83a1901bb7..faf347216b6b 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -709,31 +709,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 
 static int pci_epf_test_set_bar(struct pci_epf *epf)
 {
-	int bar, add;
-	int ret;
-	struct pci_epf_bar *epf_bar;
+	int bar, ret;
 	struct pci_epc *epc = epf->epc;
 	struct device *dev = &epf->dev;
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
-	const struct pci_epc_features *epc_features;
 
-	epc_features = epf_test->epc_features;
-
-	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
-		epf_bar = &epf->bar[bar];
-		/*
-		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
-		 * if the specific implementation required a 64-bit BAR,
-		 * even if we only requested a 32-bit BAR.
-		 */
-		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
-
-		if (epc_features->bar[bar].type == BAR_RESERVED)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
+		if (!epf_test->reg[bar])
 			continue;
 
 		ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no,
-				      epf_bar);
+				      &epf->bar[bar]);
 		if (ret) {
 			pci_epf_free_space(epf, epf_test->reg[bar], bar,
 					   PRIMARY_INTERFACE);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind()
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
                   ` (3 preceding siblings ...)
  2024-03-20 11:31 ` [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-22 10:31   ` Manivannan Sadhasivam
  2024-03-20 11:31 ` [PATCH v4 6/7] PCI: cadence: Set a 64-bit BAR if requested Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 7/7] PCI: rockchip-ep: " Niklas Cassel
  6 siblings, 1 reply; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

Clean up pci_epf_test_unbind() by using a continue if we did not allocate
memory for the BAR index. This reduces the indentation level by one.

This makes pci_epf_test_unbind() more similar to pci_epf_test_set_bar().

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index faf347216b6b..d244a5083d04 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -690,20 +690,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	struct pci_epc *epc = epf->epc;
-	struct pci_epf_bar *epf_bar;
 	int bar;
 
 	cancel_delayed_work(&epf_test->cmd_handler);
 	pci_epf_test_clean_dma_chan(epf_test);
 	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
-		epf_bar = &epf->bar[bar];
+		if (!epf_test->reg[bar])
+			continue;
 
-		if (epf_test->reg[bar]) {
-			pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
-					  epf_bar);
-			pci_epf_free_space(epf, epf_test->reg[bar], bar,
-					   PRIMARY_INTERFACE);
-		}
+		pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
+				  &epf->bar[bar]);
+		pci_epf_free_space(epf, epf_test->reg[bar], bar,
+				   PRIMARY_INTERFACE);
 	}
 }
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 6/7] PCI: cadence: Set a 64-bit BAR if requested
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
                   ` (4 preceding siblings ...)
  2024-03-20 11:31 ` [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind() Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  2024-03-20 11:31 ` [PATCH v4 7/7] PCI: rockchip-ep: " Niklas Cassel
  6 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci

Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB
is invalid if 64-bit flag is not set") it has been impossible to get the
.set_bar() callback with a BAR size > 2 GB (yes, 2GB!), if the BAR was
also not requested to be configured as a 64-bit BAR.

Thus, forcing setting the 64-bit flag for BARs larger than 2 GB in the
lower level driver is dead code and can be removed.

It is however possible that an EPF driver configures a BAR as 64-bit,
even if the requested size is < 4 GB.

Respect the requested BAR configuration, just like how it is already
repected with regards to the prefetchable bit.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 2d0a8d78bffb..de10e5edd1b0 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -99,14 +99,11 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn,
 		ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS;
 	} else {
 		bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH);
-		bool is_64bits = sz > SZ_2G;
+		bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64);
 
 		if (is_64bits && (bar & 1))
 			return -EINVAL;
 
-		if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64))
-			epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
-
 		if (is_64bits && is_prefetch)
 			ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS;
 		else if (is_prefetch)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 7/7] PCI: rockchip-ep: Set a 64-bit BAR if requested
  2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
                   ` (5 preceding siblings ...)
  2024-03-20 11:31 ` [PATCH v4 6/7] PCI: cadence: Set a 64-bit BAR if requested Niklas Cassel
@ 2024-03-20 11:31 ` Niklas Cassel
  6 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2024-03-20 11:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Shawn Lin, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Heiko Stuebner
  Cc: Shradha Todi, Damien Le Moal, Niklas Cassel, linux-pci,
	linux-rockchip, linux-arm-kernel

Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB
is invalid if 64-bit flag is not set") it has been impossible to get the
.set_bar() callback with a BAR size > 2 GB (yes, 2GB!), if the BAR was
also not requested to be configured as a 64-bit BAR.

It is however possible that an EPF driver configures a BAR as 64-bit,
even if the requested size is < 4 GB.

Respect the requested BAR configuration, just like how it is already
repected with regards to the prefetchable bit.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/pcie-rockchip-ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index c9046e97a1d2..57472cf48997 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -153,7 +153,7 @@ static int rockchip_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn,
 		ctrl = ROCKCHIP_PCIE_CORE_BAR_CFG_CTRL_IO_32BITS;
 	} else {
 		bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH);
-		bool is_64bits = sz > SZ_2G;
+		bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64);
 
 		if (is_64bits && (bar & 1))
 			return -EINVAL;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop
  2024-03-20 11:31 ` [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop Niklas Cassel
@ 2024-03-22 10:31   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2024-03-22 10:31 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Shradha Todi,
	Damien Le Moal, linux-pci

On Wed, Mar 20, 2024 at 12:31:51PM +0100, Niklas Cassel wrote:
> Simplify the loop in pci_epf_test_set_bar().
> If we allocated memory for the BAR, we need to call set_bar() for that
> BAR, if we did not allocated memory for that BAR, we need to skip.
> It is as simple as that. This also matches the logic in
> pci_epf_test_unbind().
> 
> A 64-bit BAR will still only be one allocation, with the BAR succeeding
> the 64-bit BAR being null.
> 
> While at it, remove the misleading comment.
> A EPC .set_bar() callback should never change the epf_bar->flags.
> (E.g. to set a 64-bit BAR if we requested a 32-bit BAR.)
> 
> A .set_bar() callback should do what we request it to do.
> If it can't satisfy the request, it should return an error.
> 
> If platform has a specific requirement, e.g. that a certain BAR has to
> be a 64-bit BAR, then it should specify that by setting the .only_64bit
> flag for that specific BAR in epc_features->bar[], such that
> pci_epf_alloc_space() will return a epf_bar with the 64-bit flag set.
> (Such that .set_bar() will receive a request to set a 64-bit BAR.)
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 21 ++++---------------
>  1 file changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 0a83a1901bb7..faf347216b6b 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -709,31 +709,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>  
>  static int pci_epf_test_set_bar(struct pci_epf *epf)
>  {
> -	int bar, add;
> -	int ret;
> -	struct pci_epf_bar *epf_bar;
> +	int bar, ret;
>  	struct pci_epc *epc = epf->epc;
>  	struct device *dev = &epf->dev;
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>  	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> -	const struct pci_epc_features *epc_features;
>  
> -	epc_features = epf_test->epc_features;
> -
> -	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
> -		epf_bar = &epf->bar[bar];
> -		/*
> -		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
> -		 * if the specific implementation required a 64-bit BAR,
> -		 * even if we only requested a 32-bit BAR.
> -		 */
> -		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
> -
> -		if (epc_features->bar[bar].type == BAR_RESERVED)
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> +		if (!epf_test->reg[bar])
>  			continue;
>  
>  		ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no,
> -				      epf_bar);
> +				      &epf->bar[bar]);
>  		if (ret) {
>  			pci_epf_free_space(epf, epf_test->reg[bar], bar,
>  					   PRIMARY_INTERFACE);
> -- 
> 2.44.0
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind()
  2024-03-20 11:31 ` [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind() Niklas Cassel
@ 2024-03-22 10:31   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2024-03-22 10:31 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Kishon Vijay Abraham I, Bjorn Helgaas, Shradha Todi,
	Damien Le Moal, linux-pci

On Wed, Mar 20, 2024 at 12:31:52PM +0100, Niklas Cassel wrote:
> Clean up pci_epf_test_unbind() by using a continue if we did not allocate
> memory for the BAR index. This reduces the indentation level by one.
> 
> This makes pci_epf_test_unbind() more similar to pci_epf_test_set_bar().
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index faf347216b6b..d244a5083d04 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -690,20 +690,18 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>  {
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>  	struct pci_epc *epc = epf->epc;
> -	struct pci_epf_bar *epf_bar;
>  	int bar;
>  
>  	cancel_delayed_work(&epf_test->cmd_handler);
>  	pci_epf_test_clean_dma_chan(epf_test);
>  	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> -		epf_bar = &epf->bar[bar];
> +		if (!epf_test->reg[bar])
> +			continue;
>  
> -		if (epf_test->reg[bar]) {
> -			pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
> -					  epf_bar);
> -			pci_epf_free_space(epf, epf_test->reg[bar], bar,
> -					   PRIMARY_INTERFACE);
> -		}
> +		pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no,
> +				  &epf->bar[bar]);
> +		pci_epf_free_space(epf, epf_test->reg[bar], bar,
> +				   PRIMARY_INTERFACE);
>  	}
>  }
>  
> -- 
> 2.44.0
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-03-22 10:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20 11:31 [PATCH v4 0/7] minor PCI endpoint cleanups Niklas Cassel
2024-03-20 11:31 ` [PATCH v4 1/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_alloc_space() loop Niklas Cassel
2024-03-20 11:31 ` [PATCH v4 2/7] PCI: endpoint: Allocate a 64-bit BAR if that is the only option Niklas Cassel
2024-03-20 11:31 ` [PATCH v4 3/7] PCI: endpoint: pci-epf-test: Remove superfluous code Niklas Cassel
2024-03-20 11:31 ` [PATCH v4 4/7] PCI: endpoint: pci-epf-test: Simplify pci_epf_test_set_bar() loop Niklas Cassel
2024-03-22 10:31   ` Manivannan Sadhasivam
2024-03-20 11:31 ` [PATCH v4 5/7] PCI: endpoint: pci-epf-test: Clean up pci_epf_test_unbind() Niklas Cassel
2024-03-22 10:31   ` Manivannan Sadhasivam
2024-03-20 11:31 ` [PATCH v4 6/7] PCI: cadence: Set a 64-bit BAR if requested Niklas Cassel
2024-03-20 11:31 ` [PATCH v4 7/7] PCI: rockchip-ep: " Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).