diff mbox series

[v2,7/8] PCI: qcom: Clean up IP configurations

Message ID 20220714071348.6792-8-johan+linaro@kernel.org (mailing list archive)
State Accepted
Delegated to: Lorenzo Pieralisi
Headers show
Series PCI: qcom: Add support for SC8280XP and SA8540P | expand

Commit Message

Johan Hovold July 14, 2022, 7:13 a.m. UTC 
The various IP versions have different configurations that are encoded
in separate sets of operation callbacks. Currently, there is no need for
also maintaining corresponding sets of data parameters, but it is
conceivable that these may again be found useful (e.g. to implement
minor variations of the operation callbacks).

Rename the default configuration structures after the IP version they
apply to so that they can more easily be reused by different SoCs.

Note that SoC specific configurations can be added later if need arises
(e.g. cfg_sc8280xp).

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 89 +++++++++-----------------
 1 file changed, 29 insertions(+), 60 deletions(-)

Comments

Brian Masney July 14, 2022, 2:42 p.m. UTC | #1 
On Thu, Jul 14, 2022 at 09:13:47AM +0200, Johan Hovold wrote:
> The various IP versions have different configurations that are encoded
> in separate sets of operation callbacks. Currently, there is no need for
> also maintaining corresponding sets of data parameters, but it is
> conceivable that these may again be found useful (e.g. to implement
> minor variations of the operation callbacks).
> 
> Rename the default configuration structures after the IP version they
> apply to so that they can more easily be reused by different SoCs.
> 
> Note that SoC specific configurations can be added later if need arises
> (e.g. cfg_sc8280xp).
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Brian Masney <bmasney@redhat.com>
Dmitry Baryshkov July 18, 2022, 10:39 a.m. UTC | #2 
On 14/07/2022 10:13, Johan Hovold wrote:
> The various IP versions have different configurations that are encoded
> in separate sets of operation callbacks. Currently, there is no need for
> also maintaining corresponding sets of data parameters, but it is
> conceivable that these may again be found useful (e.g. to implement
> minor variations of the operation callbacks).
> 
> Rename the default configuration structures after the IP version they
> apply to so that they can more easily be reused by different SoCs.
> 
> Note that SoC specific configurations can be added later if need arises
> (e.g. cfg_sc8280xp).
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>


If we have nothing left in the qcom_pcie_cfg other than the .ops, what 
about dropping the qcom_pcie_cfg completely and using the qcom_pcie_ops 
as match data?

This patch is nevertheless:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 89 +++++++++-----------------
>   1 file changed, 29 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 1339f05bee65..8dddb72f8647 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1606,66 +1606,35 @@ static const struct qcom_pcie_ops ops_2_9_0 = {
>   	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
>   };
>   
> -static const struct qcom_pcie_cfg apq8084_cfg = {
> +static const struct qcom_pcie_cfg cfg_1_0_0 = {
>   	.ops = &ops_1_0_0,
>   };
>   
> -static const struct qcom_pcie_cfg ipq8064_cfg = {
> +static const struct qcom_pcie_cfg cfg_1_9_0 = {
> +	.ops = &ops_1_9_0,
> +};
> +
> +static const struct qcom_pcie_cfg cfg_2_1_0 = {
>   	.ops = &ops_2_1_0,
>   };
>   
> -static const struct qcom_pcie_cfg msm8996_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_3_2 = {
>   	.ops = &ops_2_3_2,
>   };
>   
> -static const struct qcom_pcie_cfg ipq8074_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_3_3 = {
>   	.ops = &ops_2_3_3,
>   };
>   
> -static const struct qcom_pcie_cfg ipq4019_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_4_0 = {
>   	.ops = &ops_2_4_0,
>   };
>   
> -static const struct qcom_pcie_cfg sa8540p_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc8280xp_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sdm845_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_7_0 = {
>   	.ops = &ops_2_7_0,
>   };
>   
> -static const struct qcom_pcie_cfg sm8150_cfg = {
> -	/* sm8150 has qcom IP rev 1.5.0. However 1.5.0 ops are same as
> -	 * 1.9.0, so reuse the same.
> -	 */
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8250_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc7280_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc8180x_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg ipq6018_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_9_0 = {
>   	.ops = &ops_2_9_0,
>   };
>   
> @@ -1780,24 +1749,24 @@ static int qcom_pcie_remove(struct platform_device *pdev)
>   }
>   
>   static const struct of_device_id qcom_pcie_match[] = {
> -	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
> -	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
> -	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
> -	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
> -	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
> -	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
> -	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
> -	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
> -	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
> -	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
> -	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
> -	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
> -	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
> -	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
> -	{ .compatible = "qcom,pcie-ipq6018", .data = &ipq6018_cfg },
> +	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
> +	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> +	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> +	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
> +	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> +	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
> +	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-ipq6018", .data = &cfg_2_9_0 },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, qcom_pcie_match);
Johan Hovold July 18, 2022, noon UTC | #3 
On Mon, Jul 18, 2022 at 01:39:32PM +0300, Dmitry Baryshkov wrote:
> On 14/07/2022 10:13, Johan Hovold wrote:
> > The various IP versions have different configurations that are encoded
> > in separate sets of operation callbacks. Currently, there is no need for
> > also maintaining corresponding sets of data parameters, but it is
> > conceivable that these may again be found useful (e.g. to implement
> > minor variations of the operation callbacks).
> > 
> > Rename the default configuration structures after the IP version they
> > apply to so that they can more easily be reused by different SoCs.
> > 
> > Note that SoC specific configurations can be added later if need arises
> > (e.g. cfg_sc8280xp).
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> 
> If we have nothing left in the qcom_pcie_cfg other than the .ops, what 
> about dropping the qcom_pcie_cfg completely and using the qcom_pcie_ops 
> as match data?

As I mention above I decided to keep the config structures as they can
be used to implement minor variations of the ops.
 
> This patch is nevertheless:
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Thanks for reviewing.

Johan
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 1339f05bee65..8dddb72f8647 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1606,66 +1606,35 @@  static const struct qcom_pcie_ops ops_2_9_0 = {
 	.ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
 };
 
-static const struct qcom_pcie_cfg apq8084_cfg = {
+static const struct qcom_pcie_cfg cfg_1_0_0 = {
 	.ops = &ops_1_0_0,
 };
 
-static const struct qcom_pcie_cfg ipq8064_cfg = {
+static const struct qcom_pcie_cfg cfg_1_9_0 = {
+	.ops = &ops_1_9_0,
+};
+
+static const struct qcom_pcie_cfg cfg_2_1_0 = {
 	.ops = &ops_2_1_0,
 };
 
-static const struct qcom_pcie_cfg msm8996_cfg = {
+static const struct qcom_pcie_cfg cfg_2_3_2 = {
 	.ops = &ops_2_3_2,
 };
 
-static const struct qcom_pcie_cfg ipq8074_cfg = {
+static const struct qcom_pcie_cfg cfg_2_3_3 = {
 	.ops = &ops_2_3_3,
 };
 
-static const struct qcom_pcie_cfg ipq4019_cfg = {
+static const struct qcom_pcie_cfg cfg_2_4_0 = {
 	.ops = &ops_2_4_0,
 };
 
-static const struct qcom_pcie_cfg sa8540p_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc8280xp_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sdm845_cfg = {
+static const struct qcom_pcie_cfg cfg_2_7_0 = {
 	.ops = &ops_2_7_0,
 };
 
-static const struct qcom_pcie_cfg sm8150_cfg = {
-	/* sm8150 has qcom IP rev 1.5.0. However 1.5.0 ops are same as
-	 * 1.9.0, so reuse the same.
-	 */
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8250_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc7280_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc8180x_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg ipq6018_cfg = {
+static const struct qcom_pcie_cfg cfg_2_9_0 = {
 	.ops = &ops_2_9_0,
 };
 
@@ -1780,24 +1749,24 @@  static int qcom_pcie_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_pcie_match[] = {
-	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
-	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
-	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
-	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
-	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
-	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
-	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
-	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
-	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
-	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
-	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
-	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
-	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
-	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
-	{ .compatible = "qcom,pcie-ipq6018", .data = &ipq6018_cfg },
+	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
+	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
+	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
+	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
+	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
+	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
+	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-ipq6018", .data = &cfg_2_9_0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_pcie_match);