All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manivannan Sadhasivam <mani@kernel.org>
To: Niklas Cassel <cassel@kernel.org>
Cc: "Richard Zhu" <hongxing.zhu@nxp.com>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Fabio Estevam" <festevam@gmail.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Minghuan Lian" <minghuan.Lian@nxp.com>,
	"Mingkai Hu" <mingkai.hu@nxp.com>, "Roy Zang" <roy.zang@nxp.com>,
	"Srikanth Thokala" <srikanth.thokala@intel.com>,
	"Marek Vasut" <marek.vasut+renesas@gmail.com>,
	"Yoshihiro Shimoda" <yoshihiro.shimoda.uh@renesas.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Kunihiko Hayashi" <hayashi.kunihiko@socionext.com>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Jon Mason" <jdmason@kudzu.us>,
	"Dave Jiang" <dave.jiang@intel.com>,
	"Allen Hubbe" <allenbh@gmail.com>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Damien Le Moal" <dlemoal@kernel.org>,
	"Frank Li" <Frank.Li@nxp.com>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-renesas-soc@vger.kernel.org,
	linux-tegra@vger.kernel.org, ntb@lists.linux.dev
Subject: Re: [PATCH 1/2] PCI: endpoint: Clean up hardware description for BARs
Date: Fri, 16 Feb 2024 16:49:08 +0530	[thread overview]
Message-ID: <20240216111908.GC2559@thinkpad> (raw)
In-Reply-To: <20240210012634.600301-2-cassel@kernel.org>

On Sat, Feb 10, 2024 at 02:26:25AM +0100, Niklas Cassel wrote:
> The hardware description for BARs is scattered in many different variables
> in pci_epc_features. Some of these things are mutually exclusive, so it
> can create confusion over which variable that has precedence over another.
> 
> Improve the situation by creating a struct pci_epc_bar_desc, and a new
> enum pci_epc_bar_type, and convert the endpoint controller drivers to use
> this more well defined format.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c         |  3 +-
>  drivers/pci/controller/dwc/pci-keystone.c     | 12 +++----
>  .../pci/controller/dwc/pci-layerscape-ep.c    |  5 ++-
>  drivers/pci/controller/dwc/pcie-keembay.c     |  8 +++--
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   |  4 ++-
>  drivers/pci/controller/dwc/pcie-tegra194.c    | 10 ++++--
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c | 15 ++++++--
>  drivers/pci/controller/pcie-rcar-ep.c         | 14 +++++---
>  drivers/pci/endpoint/functions/pci-epf-ntb.c  |  4 +--
>  drivers/pci/endpoint/functions/pci-epf-test.c |  8 ++---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c |  2 +-
>  drivers/pci/endpoint/pci-epc-core.c           | 32 +++++++++--------
>  drivers/pci/endpoint/pci-epf-core.c           | 15 ++++----
>  include/linux/pci-epc.h                       | 34 +++++++++++++++----
>  14 files changed, 108 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index dc2c036ab28c..47a9a96484ed 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1081,7 +1081,8 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
>  	.align = SZ_64K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index c0c62533a3f1..b2b93b4fa82d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -924,12 +924,12 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = true,
> -	.reserved_bar = 1 << BAR_0 | 1 << BAR_1,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[2] = SZ_1M,
> -	.bar_fixed_size[3] = SZ_64K,
> -	.bar_fixed_size[4] = 256,
> -	.bar_fixed_size[5] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, },
> +	.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, },
> +	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
> +	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 2e398494e7c0..1f6ee1460ec2 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -250,7 +250,10 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = pcie->drvdata->dw_pcie_ops;
>  
> -	ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
> +	ls_epc->bar[BAR_2].only_64bit = true;
> +	ls_epc->bar[BAR_3].type = BAR_RESERVED;

BAR_3 and BAR_4 were not reserved previously.

> +	ls_epc->bar[BAR_4].only_64bit = true;
> +	ls_epc->bar[BAR_5].type = BAR_RESERVED;
>  	ls_epc->linkup_notifier = true;
>  
>  	pcie->pci = pci;
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 208d3b0ba196..5e8e54f597dd 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -312,8 +312,12 @@ static const struct pci_epc_features keembay_pcie_epc_features = {
>  	.linkup_notifier	= false,
>  	.msi_capable		= true,
>  	.msix_capable		= true,
> -	.reserved_bar		= BIT(BAR_1) | BIT(BAR_3) | BIT(BAR_5),
> -	.bar_fixed_64bit	= BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +	.bar[BAR_0]		= { .only_64bit = true, },
> +	.bar[BAR_1]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_2]		= { .only_64bit = true, },
> +	.bar[BAR_3]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_4]		= { .only_64bit = true, },
> +	.bar[BAR_5]		= { .type = BAR_RESERVED, },
>  	.align			= SZ_16K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index e9166619b1f9..0be760ed420b 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -383,7 +383,9 @@ static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7afa9e9aabe2..1f7b662cb8e1 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2007,9 +2007,13 @@ static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.core_init_notifier = true,
>  	.msi_capable = false,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_2 | 1 << BAR_3 | 1 << BAR_4 | 1 << BAR_5,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[0] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M,
> +			.only_64bit = true, },
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },

Same, BAR_1 was not reserved.

> +	.bar[BAR_2] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_4] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  };
>  
>  static const struct pci_epc_features*
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 3fced0d3e851..265f65fc673f 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -411,8 +411,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 16,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> -		.reserved_bar =  BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

> +		.bar[BAR_2] = { .only_64bit = true, },
> +		.bar[BAR_3] = { .type = BAR_RESERVED, },
> +		.bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, },
> +		.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	},
>  };
>  
> @@ -425,7 +429,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_nx1_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 12,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

- Mani

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

WARNING: multiple messages have this Message-ID (diff)
From: Manivannan Sadhasivam <mani@kernel.org>
To: Niklas Cassel <cassel@kernel.org>
Cc: "Krzysztof Wilczyński" <kw@linux.com>,
	"Kunihiko Hayashi" <hayashi.kunihiko@socionext.com>,
	linux-pci@vger.kernel.org,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Frank Li" <Frank.Li@nxp.com>,
	"Minghuan Lian" <minghuan.Lian@nxp.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Marek Vasut" <marek.vasut+renesas@gmail.com>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Dave Jiang" <dave.jiang@intel.com>,
	linux-tegra@vger.kernel.org,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Allen Hubbe" <allenbh@gmail.com>,
	"Richard Zhu" <hongxing.zhu@nxp.com>,
	"Srikanth Thokala" <srikanth.thokala@intel.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Damien Le Moal" <dlemoal@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Mingkai Hu" <mingkai.hu@nxp.com>,
	linux-arm-kernel@lists.infradead.org,
	"Roy Zang" <roy.zang@nxp.com>,
	linuxppc-dev@lists.ozlabs.org,
	"Yoshihiro Shimoda" <yoshihiro.shimoda.uh@renesas.com>,
	linux-renesas-soc@vger.kernel.org, ntb@lists.linux.dev,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Jon Mason" <jdmason@kudzu.us>, "Shawn Guo" <shawnguo@kernel.org>,
	"Lucas Stach" <l.stach@pengutronix.de>
Subject: Re: [PATCH 1/2] PCI: endpoint: Clean up hardware description for BARs
Date: Fri, 16 Feb 2024 16:49:08 +0530	[thread overview]
Message-ID: <20240216111908.GC2559@thinkpad> (raw)
In-Reply-To: <20240210012634.600301-2-cassel@kernel.org>

On Sat, Feb 10, 2024 at 02:26:25AM +0100, Niklas Cassel wrote:
> The hardware description for BARs is scattered in many different variables
> in pci_epc_features. Some of these things are mutually exclusive, so it
> can create confusion over which variable that has precedence over another.
> 
> Improve the situation by creating a struct pci_epc_bar_desc, and a new
> enum pci_epc_bar_type, and convert the endpoint controller drivers to use
> this more well defined format.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c         |  3 +-
>  drivers/pci/controller/dwc/pci-keystone.c     | 12 +++----
>  .../pci/controller/dwc/pci-layerscape-ep.c    |  5 ++-
>  drivers/pci/controller/dwc/pcie-keembay.c     |  8 +++--
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   |  4 ++-
>  drivers/pci/controller/dwc/pcie-tegra194.c    | 10 ++++--
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c | 15 ++++++--
>  drivers/pci/controller/pcie-rcar-ep.c         | 14 +++++---
>  drivers/pci/endpoint/functions/pci-epf-ntb.c  |  4 +--
>  drivers/pci/endpoint/functions/pci-epf-test.c |  8 ++---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c |  2 +-
>  drivers/pci/endpoint/pci-epc-core.c           | 32 +++++++++--------
>  drivers/pci/endpoint/pci-epf-core.c           | 15 ++++----
>  include/linux/pci-epc.h                       | 34 +++++++++++++++----
>  14 files changed, 108 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index dc2c036ab28c..47a9a96484ed 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1081,7 +1081,8 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
>  	.align = SZ_64K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index c0c62533a3f1..b2b93b4fa82d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -924,12 +924,12 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = true,
> -	.reserved_bar = 1 << BAR_0 | 1 << BAR_1,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[2] = SZ_1M,
> -	.bar_fixed_size[3] = SZ_64K,
> -	.bar_fixed_size[4] = 256,
> -	.bar_fixed_size[5] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, },
> +	.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, },
> +	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
> +	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 2e398494e7c0..1f6ee1460ec2 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -250,7 +250,10 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = pcie->drvdata->dw_pcie_ops;
>  
> -	ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
> +	ls_epc->bar[BAR_2].only_64bit = true;
> +	ls_epc->bar[BAR_3].type = BAR_RESERVED;

BAR_3 and BAR_4 were not reserved previously.

> +	ls_epc->bar[BAR_4].only_64bit = true;
> +	ls_epc->bar[BAR_5].type = BAR_RESERVED;
>  	ls_epc->linkup_notifier = true;
>  
>  	pcie->pci = pci;
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 208d3b0ba196..5e8e54f597dd 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -312,8 +312,12 @@ static const struct pci_epc_features keembay_pcie_epc_features = {
>  	.linkup_notifier	= false,
>  	.msi_capable		= true,
>  	.msix_capable		= true,
> -	.reserved_bar		= BIT(BAR_1) | BIT(BAR_3) | BIT(BAR_5),
> -	.bar_fixed_64bit	= BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +	.bar[BAR_0]		= { .only_64bit = true, },
> +	.bar[BAR_1]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_2]		= { .only_64bit = true, },
> +	.bar[BAR_3]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_4]		= { .only_64bit = true, },
> +	.bar[BAR_5]		= { .type = BAR_RESERVED, },
>  	.align			= SZ_16K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index e9166619b1f9..0be760ed420b 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -383,7 +383,9 @@ static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7afa9e9aabe2..1f7b662cb8e1 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2007,9 +2007,13 @@ static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.core_init_notifier = true,
>  	.msi_capable = false,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_2 | 1 << BAR_3 | 1 << BAR_4 | 1 << BAR_5,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[0] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M,
> +			.only_64bit = true, },
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },

Same, BAR_1 was not reserved.

> +	.bar[BAR_2] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_4] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  };
>  
>  static const struct pci_epc_features*
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 3fced0d3e851..265f65fc673f 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -411,8 +411,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 16,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> -		.reserved_bar =  BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

> +		.bar[BAR_2] = { .only_64bit = true, },
> +		.bar[BAR_3] = { .type = BAR_RESERVED, },
> +		.bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, },
> +		.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	},
>  };
>  
> @@ -425,7 +429,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_nx1_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 12,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

- Mani

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

WARNING: multiple messages have this Message-ID (diff)
From: Manivannan Sadhasivam <mani@kernel.org>
To: Niklas Cassel <cassel@kernel.org>
Cc: "Richard Zhu" <hongxing.zhu@nxp.com>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Fabio Estevam" <festevam@gmail.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	"Minghuan Lian" <minghuan.Lian@nxp.com>,
	"Mingkai Hu" <mingkai.hu@nxp.com>, "Roy Zang" <roy.zang@nxp.com>,
	"Srikanth Thokala" <srikanth.thokala@intel.com>,
	"Marek Vasut" <marek.vasut+renesas@gmail.com>,
	"Yoshihiro Shimoda" <yoshihiro.shimoda.uh@renesas.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Kunihiko Hayashi" <hayashi.kunihiko@socionext.com>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Jon Mason" <jdmason@kudzu.us>,
	"Dave Jiang" <dave.jiang@intel.com>,
	"Allen Hubbe" <allenbh@gmail.com>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Damien Le Moal" <dlemoal@kernel.org>,
	"Frank Li" <Frank.Li@nxp.com>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, linux-renesas-soc@vger.kernel.org,
	linux-tegra@vger.kernel.org, ntb@lists.linux.dev
Subject: Re: [PATCH 1/2] PCI: endpoint: Clean up hardware description for BARs
Date: Fri, 16 Feb 2024 16:49:08 +0530	[thread overview]
Message-ID: <20240216111908.GC2559@thinkpad> (raw)
In-Reply-To: <20240210012634.600301-2-cassel@kernel.org>

On Sat, Feb 10, 2024 at 02:26:25AM +0100, Niklas Cassel wrote:
> The hardware description for BARs is scattered in many different variables
> in pci_epc_features. Some of these things are mutually exclusive, so it
> can create confusion over which variable that has precedence over another.
> 
> Improve the situation by creating a struct pci_epc_bar_desc, and a new
> enum pci_epc_bar_type, and convert the endpoint controller drivers to use
> this more well defined format.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c         |  3 +-
>  drivers/pci/controller/dwc/pci-keystone.c     | 12 +++----
>  .../pci/controller/dwc/pci-layerscape-ep.c    |  5 ++-
>  drivers/pci/controller/dwc/pcie-keembay.c     |  8 +++--
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   |  4 ++-
>  drivers/pci/controller/dwc/pcie-tegra194.c    | 10 ++++--
>  drivers/pci/controller/dwc/pcie-uniphier-ep.c | 15 ++++++--
>  drivers/pci/controller/pcie-rcar-ep.c         | 14 +++++---
>  drivers/pci/endpoint/functions/pci-epf-ntb.c  |  4 +--
>  drivers/pci/endpoint/functions/pci-epf-test.c |  8 ++---
>  drivers/pci/endpoint/functions/pci-epf-vntb.c |  2 +-
>  drivers/pci/endpoint/pci-epc-core.c           | 32 +++++++++--------
>  drivers/pci/endpoint/pci-epf-core.c           | 15 ++++----
>  include/linux/pci-epc.h                       | 34 +++++++++++++++----
>  14 files changed, 108 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index dc2c036ab28c..47a9a96484ed 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1081,7 +1081,8 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
>  	.align = SZ_64K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index c0c62533a3f1..b2b93b4fa82d 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -924,12 +924,12 @@ static const struct pci_epc_features ks_pcie_am654_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = true,
> -	.reserved_bar = 1 << BAR_0 | 1 << BAR_1,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[2] = SZ_1M,
> -	.bar_fixed_size[3] = SZ_64K,
> -	.bar_fixed_size[4] = 256,
> -	.bar_fixed_size[5] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_RESERVED, .only_64bit = true, },
> +	.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, },
> +	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256, },
> +	.bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 2e398494e7c0..1f6ee1460ec2 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -250,7 +250,10 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
>  	pci->dev = dev;
>  	pci->ops = pcie->drvdata->dw_pcie_ops;
>  
> -	ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
> +	ls_epc->bar[BAR_2].only_64bit = true;
> +	ls_epc->bar[BAR_3].type = BAR_RESERVED;

BAR_3 and BAR_4 were not reserved previously.

> +	ls_epc->bar[BAR_4].only_64bit = true;
> +	ls_epc->bar[BAR_5].type = BAR_RESERVED;
>  	ls_epc->linkup_notifier = true;
>  
>  	pcie->pci = pci;
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 208d3b0ba196..5e8e54f597dd 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -312,8 +312,12 @@ static const struct pci_epc_features keembay_pcie_epc_features = {
>  	.linkup_notifier	= false,
>  	.msi_capable		= true,
>  	.msix_capable		= true,
> -	.reserved_bar		= BIT(BAR_1) | BIT(BAR_3) | BIT(BAR_5),
> -	.bar_fixed_64bit	= BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +	.bar[BAR_0]		= { .only_64bit = true, },
> +	.bar[BAR_1]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_2]		= { .only_64bit = true, },
> +	.bar[BAR_3]		= { .type = BAR_RESERVED, },
> +	.bar[BAR_4]		= { .only_64bit = true, },
> +	.bar[BAR_5]		= { .type = BAR_RESERVED, },
>  	.align			= SZ_16K,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index e9166619b1f9..0be760ed420b 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -383,7 +383,9 @@ static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
>  	.linkup_notifier = false,
>  	.msi_capable = true,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	.align = SZ_1M,
>  };
>  
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7afa9e9aabe2..1f7b662cb8e1 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2007,9 +2007,13 @@ static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.core_init_notifier = true,
>  	.msi_capable = false,
>  	.msix_capable = false,
> -	.reserved_bar = 1 << BAR_2 | 1 << BAR_3 | 1 << BAR_4 | 1 << BAR_5,
> -	.bar_fixed_64bit = 1 << BAR_0,
> -	.bar_fixed_size[0] = SZ_1M,
> +	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M,
> +			.only_64bit = true, },
> +	.bar[BAR_1] = { .type = BAR_RESERVED, },

Same, BAR_1 was not reserved.

> +	.bar[BAR_2] = { .type = BAR_RESERVED, },
> +	.bar[BAR_3] = { .type = BAR_RESERVED, },
> +	.bar[BAR_4] = { .type = BAR_RESERVED, },
> +	.bar[BAR_5] = { .type = BAR_RESERVED, },
>  };
>  
>  static const struct pci_epc_features*
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 3fced0d3e851..265f65fc673f 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -411,8 +411,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 16,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> -		.reserved_bar =  BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

> +		.bar[BAR_2] = { .only_64bit = true, },
> +		.bar[BAR_3] = { .type = BAR_RESERVED, },
> +		.bar[BAR_4] = { .type = BAR_RESERVED, .only_64bit = true, },
> +		.bar[BAR_5] = { .type = BAR_RESERVED, },
>  	},
>  };
>  
> @@ -425,7 +429,12 @@ static const struct uniphier_pcie_ep_soc_data uniphier_nx1_data = {
>  		.msi_capable = true,
>  		.msix_capable = false,
>  		.align = 1 << 12,
> -		.bar_fixed_64bit = BIT(BAR_0) | BIT(BAR_2) | BIT(BAR_4),
> +		.bar[BAR_0] = { .only_64bit = true, },
> +		.bar[BAR_1] = { .type = BAR_RESERVED, },

Same as above.

- Mani

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-02-16 11:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-10  1:26 [PATCH 0/2] PCI endpoint BAR hardware description cleanup Niklas Cassel
2024-02-10  1:26 ` Niklas Cassel
2024-02-10  1:26 ` Niklas Cassel
2024-02-10  1:26 ` [PATCH 1/2] PCI: endpoint: Clean up hardware description for BARs Niklas Cassel
2024-02-10  1:26   ` Niklas Cassel
2024-02-10  1:26   ` Niklas Cassel
2024-02-16 11:19   ` Manivannan Sadhasivam [this message]
2024-02-16 11:19     ` Manivannan Sadhasivam
2024-02-16 11:19     ` Manivannan Sadhasivam
2024-02-16 11:25     ` Manivannan Sadhasivam
2024-02-16 11:25       ` Manivannan Sadhasivam
2024-02-16 11:25       ` Manivannan Sadhasivam
2024-02-10  1:26 ` [PATCH 2/2] PCI: endpoint: Drop only_64bit on reserved BARs Niklas Cassel
2024-02-10  1:26   ` Niklas Cassel
2024-02-16 11:28   ` Manivannan Sadhasivam
2024-02-16 11:28     ` Manivannan Sadhasivam
2024-02-14  4:17 ` [PATCH 0/2] PCI endpoint BAR hardware description cleanup Kishon Vijay Abraham I
2024-02-14  4:17   ` Kishon Vijay Abraham I
2024-02-14  4:17   ` Kishon Vijay Abraham I
2024-02-14 10:38   ` Niklas Cassel
2024-02-14 10:38     ` Niklas Cassel
2024-02-14 10:38     ` Niklas Cassel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240216111908.GC2559@thinkpad \
    --to=mani@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=allenbh@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=cassel@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=dlemoal@kernel.org \
    --cc=festevam@gmail.com \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=hongxing.zhu@nxp.com \
    --cc=jdmason@kudzu.us \
    --cc=jonathanh@nvidia.com \
    --cc=kernel@pengutronix.de \
    --cc=kishon@kernel.org \
    --cc=kw@linux.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lpieralisi@kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=marek.vasut+renesas@gmail.com \
    --cc=mhiramat@kernel.org \
    --cc=minghuan.Lian@nxp.com \
    --cc=mingkai.hu@nxp.com \
    --cc=ntb@lists.linux.dev \
    --cc=robh@kernel.org \
    --cc=roy.zang@nxp.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=srikanth.thokala@intel.com \
    --cc=thierry.reding@gmail.com \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.