diff mbox series

[2/2] PCI: endpoint: Drop only_64bit on reserved BARs

Message ID 20240210012634.600301-3-cassel@kernel.org (mailing list archive)
State Superseded
Delegated to: Manivannan Sadhasivam
Headers show
Series PCI endpoint BAR hardware description cleanup | expand

Commit Message

Niklas Cassel Feb. 10, 2024, 1:26 a.m. UTC
The definition of a reserved BAR is that EPF drivers should not touch
them.

The definition of only_64bit is that the EPF driver must configure this
BAR as 64-bit. (An EPF driver is not allowed to choose if this BAR should
be configured as 32-bit or 64-bit.)

Thus, it does not make sense to put only_64bit of a BAR that EPF drivers
are not allow to touch.

Drop the only_64bit property from hardware descriptions that are of type
reserved BAR.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pci-keystone.c     | 2 +-
 drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
 drivers/pci/endpoint/pci-epc-core.c           | 7 -------
 include/linux/pci-epc.h                       | 4 ++++
 4 files changed, 6 insertions(+), 9 deletions(-)

Comments

Manivannan Sadhasivam Feb. 16, 2024, 11:28 a.m. UTC | #1
On Sat, Feb 10, 2024 at 02:26:26AM +0100, Niklas Cassel wrote:
> The definition of a reserved BAR is that EPF drivers should not touch
> them.
> 
> The definition of only_64bit is that the EPF driver must configure this
> BAR as 64-bit. (An EPF driver is not allowed to choose if this BAR should
> be configured as 32-bit or 64-bit.)
> 
> Thus, it does not make sense to put only_64bit of a BAR that EPF drivers
> are not allow to touch.
> 
> Drop the only_64bit property from hardware descriptions that are of type
> reserved BAR.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

One nitpick below. With that addressed,

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

> ---
>  drivers/pci/controller/dwc/pci-keystone.c     | 2 +-
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
>  drivers/pci/endpoint/pci-epc-core.c           | 7 -------
>  include/linux/pci-epc.h                       | 4 ++++
>  4 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index b2b93b4fa82d..844de4418724 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -924,7 +924,7 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = true,
> -	.bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, },
> +	.bar[BAR_0] = { .type = BAR_RESERVED, },
>  	.bar[BAR_1] = { .type = BAR_RESERVED, },
>  	.bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
>  	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 265f65fc673f..639bc2e12476 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -415,7 +415,7 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>  		.bar[BAR_1] = { .type = BAR_RESERVED, },
>  		.bar[BAR_2] = { .only_64bit = true, },
>  		.bar[BAR_3] = { .type = BAR_RESERVED, },
> -		.bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, },
> +		.bar[BAR_4] = { .type = BAR_RESERVED, },
>  		.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	},
>  };
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 7fe8f4336765..da3fc0795b0b 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -120,13 +120,6 @@ enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
>  		/* If the BAR is not reserved, return it. */
>  		if (epc_features->bar[i].type != BAR_RESERVED)
>  			return i;
> -
> -		/*
> -		 * If the BAR is reserved, and marked as 64-bit only, then the
> -		 * succeeding BAR is also reserved.
> -		 */
> -		if (epc_features->bar[i].only_64bit)
> -			i++;
>  	}
>  
>  	return NO_BAR;
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 4ccb4f4f3883..bb9c4dfcea93 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -164,6 +164,10 @@ enum pci_epc_bar_type {
>   *		should be configured as 32-bit or 64-bit, the EPF driver must
>   *		configure this BAR as 64-bit. Additionally, the BAR succeeding
>   *		this BAR must be set to type BAR_RESERVED.
> + *
> + *		only_64bit should not be set on a BAR of type BAR_RESERVED.
> + *		(If BARx is a 64-bit BAR that an EPF driver is not allowed to
> + *		touch, then you must set both BARx and BARx+1 as BAR_RESERVED.)

"then both BARx and BARx+1 must be set to type BAR_RESERVED."

- Mani
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index b2b93b4fa82d..844de4418724 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -924,7 +924,7 @@  static const struct pci_epc_features ks_pcie_am654_epc_features = {
 	.linkup_notifier = false,
 	.msi_capable = true,
 	.msix_capable = true,
-	.bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, },
+	.bar[BAR_0] = { .type = BAR_RESERVED, },
 	.bar[BAR_1] = { .type = BAR_RESERVED, },
 	.bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
 	.bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
index 265f65fc673f..639bc2e12476 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
@@ -415,7 +415,7 @@  static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
 		.bar[BAR_1] = { .type = BAR_RESERVED, },
 		.bar[BAR_2] = { .only_64bit = true, },
 		.bar[BAR_3] = { .type = BAR_RESERVED, },
-		.bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, },
+		.bar[BAR_4] = { .type = BAR_RESERVED, },
 		.bar[BAR_5] = { .type = BAR_RESERVED, },
 	},
 };
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 7fe8f4336765..da3fc0795b0b 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -120,13 +120,6 @@  enum pci_barno pci_epc_get_next_free_bar(const struct pci_epc_features
 		/* If the BAR is not reserved, return it. */
 		if (epc_features->bar[i].type != BAR_RESERVED)
 			return i;
-
-		/*
-		 * If the BAR is reserved, and marked as 64-bit only, then the
-		 * succeeding BAR is also reserved.
-		 */
-		if (epc_features->bar[i].only_64bit)
-			i++;
 	}
 
 	return NO_BAR;
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 4ccb4f4f3883..bb9c4dfcea93 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -164,6 +164,10 @@  enum pci_epc_bar_type {
  *		should be configured as 32-bit or 64-bit, the EPF driver must
  *		configure this BAR as 64-bit. Additionally, the BAR succeeding
  *		this BAR must be set to type BAR_RESERVED.
+ *
+ *		only_64bit should not be set on a BAR of type BAR_RESERVED.
+ *		(If BARx is a 64-bit BAR that an EPF driver is not allowed to
+ *		touch, then you must set both BARx and BARx+1 as BAR_RESERVED.)
  */
 struct pci_epc_bar_desc {
 	enum pci_epc_bar_type type;