[PATCH 0/3] Drop useless compatibles from the SCM driver

[PATCH 0/3] Drop useless compatibles from the SCM driver

[Konrad Dybcio]

The compatibles, apart from some ancient ones kept for backwards compat
due to no generic fallback, are largely useless and we can easily remove
them. This series attempts to do that with hopefully no harm.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (3):
      firmware: qcom_scm: Always try to consume all three clocks
      firmware: qcom_scm: Always return devm_clk_get_optional errors
      firmware: qcom_scm: Drop useless compatibles

 drivers/firmware/qcom_scm.c | 87 +++++++++------------------------------------
 1 file changed, 16 insertions(+), 71 deletions(-)
---
base-commit: c87d46a9e8ebd2f2c3960927b1d21687096d1109
change-id: 20230623-topic-scm_cleanup-c4620461126a

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>

[PATCH 1/3] firmware: qcom_scm: Always try to consume all three clocks

[Konrad Dybcio]

The code for handling more than 1 clock is a bit messy and requires
one to add new, SoC-specific compatibles if one wants to attach a clock.

Switch devm_clk_get to devm_clk_get_optional to prevent throwing it
from throwing errors when the clock is absent and defer checking the
clock requirements to dt schema.

This lets us get rid of compatibles that aren't necessary for backwards
compatibility *and* will hopefully prevent the addition of meaningless
new compatibles.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/firmware/qcom_scm.c | 70 +++++++++------------------------------------
 1 file changed, 13 insertions(+), 57 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index fde33acd46b7..dfa298347041 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -26,10 +26,6 @@
 static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
 module_param(download_mode, bool, 0);
 
-#define SCM_HAS_CORE_CLK	BIT(0)
-#define SCM_HAS_IFACE_CLK	BIT(1)
-#define SCM_HAS_BUS_CLK		BIT(2)
-
 struct qcom_scm {
 	struct device *dev;
 	struct clk *core_clk;
@@ -1425,43 +1421,22 @@ static int qcom_scm_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(scm->path),
 				     "failed to acquire interconnect path\n");
 
-	scm->core_clk = devm_clk_get(&pdev->dev, "core");
+	scm->core_clk = devm_clk_get_optional(&pdev->dev, "core");
 	if (IS_ERR(scm->core_clk)) {
 		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
 			return PTR_ERR(scm->core_clk);
-
-		if (clks & SCM_HAS_CORE_CLK) {
-			dev_err(&pdev->dev, "failed to acquire core clk\n");
-			return PTR_ERR(scm->core_clk);
-		}
-
-		scm->core_clk = NULL;
 	}
 
-	scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
+	scm->iface_clk = devm_clk_get_optional(&pdev->dev, "iface");
 	if (IS_ERR(scm->iface_clk)) {
 		if (PTR_ERR(scm->iface_clk) == -EPROBE_DEFER)
 			return PTR_ERR(scm->iface_clk);
-
-		if (clks & SCM_HAS_IFACE_CLK) {
-			dev_err(&pdev->dev, "failed to acquire iface clk\n");
-			return PTR_ERR(scm->iface_clk);
-		}
-
-		scm->iface_clk = NULL;
 	}
 
-	scm->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	scm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus");
 	if (IS_ERR(scm->bus_clk)) {
 		if (PTR_ERR(scm->bus_clk) == -EPROBE_DEFER)
 			return PTR_ERR(scm->bus_clk);
-
-		if (clks & SCM_HAS_BUS_CLK) {
-			dev_err(&pdev->dev, "failed to acquire bus clk\n");
-			return PTR_ERR(scm->bus_clk);
-		}
-
-		scm->bus_clk = NULL;
 	}
 
 	scm->reset.ops = &qcom_scm_pas_reset_ops;
@@ -1512,38 +1487,19 @@ static void qcom_scm_shutdown(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_scm_dt_match[] = {
-	{ .compatible = "qcom,scm-apq8064",
-	  /* FIXME: This should have .data = (void *) SCM_HAS_CORE_CLK */
-	},
-	{ .compatible = "qcom,scm-apq8084", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK)
-	},
+	{ .compatible = "qcom,scm-apq8064" },
+	{ .compatible = "qcom,scm-apq8084" },
 	{ .compatible = "qcom,scm-ipq4019" },
-	{ .compatible = "qcom,scm-mdm9607", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK) },
-	{ .compatible = "qcom,scm-msm8660", .data = (void *) SCM_HAS_CORE_CLK },
-	{ .compatible = "qcom,scm-msm8960", .data = (void *) SCM_HAS_CORE_CLK },
-	{ .compatible = "qcom,scm-msm8916", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK)
-	},
-	{ .compatible = "qcom,scm-msm8953", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK)
-	},
-	{ .compatible = "qcom,scm-msm8974", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK)
-	},
-	{ .compatible = "qcom,scm-msm8976", .data = (void *)(SCM_HAS_CORE_CLK |
-							     SCM_HAS_IFACE_CLK |
-							     SCM_HAS_BUS_CLK)
-	},
+	{ .compatible = "qcom,scm-mdm9607" },
+	{ .compatible = "qcom,scm-msm8660" },
+	{ .compatible = "qcom,scm-msm8960" },
+	{ .compatible = "qcom,scm-msm8916" },
+	{ .compatible = "qcom,scm-msm8953" },
+	{ .compatible = "qcom,scm-msm8974" },
+	{ .compatible = "qcom,scm-msm8976" },
 	{ .compatible = "qcom,scm-msm8994" },
 	{ .compatible = "qcom,scm-msm8996" },
-	{ .compatible = "qcom,scm-sm6375", .data = (void *)SCM_HAS_CORE_CLK },
+	{ .compatible = "qcom,scm-sm6375" },
 	{ .compatible = "qcom,scm" },
 	{}
 };

-- 
2.41.0

[PATCH 2/3] firmware: qcom_scm: Always return devm_clk_get_optional errors

[Konrad Dybcio]

If devm_clk_get_optional throws an error, something is really wrong.
It may be a probe deferral, or it may be a problem with the clock
provider.

Regardless of what it may be, it should definitely not be ignored.
Stop doing that.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/firmware/qcom_scm.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index dfa298347041..d7fc972b20ab 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1422,22 +1422,16 @@ static int qcom_scm_probe(struct platform_device *pdev)
 				     "failed to acquire interconnect path\n");
 
 	scm->core_clk = devm_clk_get_optional(&pdev->dev, "core");
-	if (IS_ERR(scm->core_clk)) {
-		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
-			return PTR_ERR(scm->core_clk);
-	}
+	if (IS_ERR(scm->core_clk))
+		return PTR_ERR(scm->core_clk);
 
 	scm->iface_clk = devm_clk_get_optional(&pdev->dev, "iface");
-	if (IS_ERR(scm->iface_clk)) {
-		if (PTR_ERR(scm->iface_clk) == -EPROBE_DEFER)
-			return PTR_ERR(scm->iface_clk);
-	}
+	if (IS_ERR(scm->iface_clk))
+		return PTR_ERR(scm->iface_clk);
 
 	scm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus");
-	if (IS_ERR(scm->bus_clk)) {
-		if (PTR_ERR(scm->bus_clk) == -EPROBE_DEFER)
-			return PTR_ERR(scm->bus_clk);
-	}
+	if (IS_ERR(scm->bus_clk))
+		return PTR_ERR(scm->bus_clk);
 
 	scm->reset.ops = &qcom_scm_pas_reset_ops;
 	scm->reset.nr_resets = 1;

-- 
2.41.0

[PATCH 3/3] firmware: qcom_scm: Drop useless compatibles

[Konrad Dybcio]

There are three categories of compatibles within the driver:

1. Ones which were introduced without a qcom,scm fallback
2. Ones which were introduced with a qcom,scm fallback
3. Ones which were defined but never used

Keep 1 for backwards compatibility and axe 2 & 3.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/firmware/qcom_scm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d7fc972b20ab..ff0e8911499c 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1481,20 +1481,15 @@ static void qcom_scm_shutdown(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_scm_dt_match[] = {
+	{ .compatible = "qcom,scm" },
+
+	/* Legacy entries kept for backwards compatibility */
 	{ .compatible = "qcom,scm-apq8064" },
 	{ .compatible = "qcom,scm-apq8084" },
 	{ .compatible = "qcom,scm-ipq4019" },
-	{ .compatible = "qcom,scm-mdm9607" },
-	{ .compatible = "qcom,scm-msm8660" },
-	{ .compatible = "qcom,scm-msm8960" },
-	{ .compatible = "qcom,scm-msm8916" },
 	{ .compatible = "qcom,scm-msm8953" },
 	{ .compatible = "qcom,scm-msm8974" },
-	{ .compatible = "qcom,scm-msm8976" },
-	{ .compatible = "qcom,scm-msm8994" },
 	{ .compatible = "qcom,scm-msm8996" },
-	{ .compatible = "qcom,scm-sm6375" },
-	{ .compatible = "qcom,scm" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);

-- 
2.41.0

Re: [PATCH 1/3] firmware: qcom_scm: Always try to consume all three clocks

[kernel test robot]

Hi Konrad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on c87d46a9e8ebd2f2c3960927b1d21687096d1109]

url:    https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/firmware-qcom_scm-Always-try-to-consume-all-three-clocks/20230624-055215
base:   c87d46a9e8ebd2f2c3960927b1d21687096d1109
patch link:    https://lore.kernel.org/r/20230623-topic-scm_cleanup-v1-1-383089eae98d%40linaro.org
patch subject: [PATCH 1/3] firmware: qcom_scm: Always try to consume all three clocks
config: arm64-randconfig-r023-20230622 (https://download.01.org/0day-ci/archive/20230624/202306240841.iVZhYL4l-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230624/202306240841.iVZhYL4l-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306240841.iVZhYL4l-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/firmware/qcom_scm.c: In function 'qcom_scm_probe':
>> drivers/firmware/qcom_scm.c:1404:23: warning: variable 'clks' set but not used [-Wunused-but-set-variable]
    1404 |         unsigned long clks;
         |                       ^~~~


vim +/clks +1404 drivers/firmware/qcom_scm.c

6bf32599223634 Guru Das Srinagesh 2023-01-13  1400  
d0f6fa7ba2d624 Andy Gross         2016-06-03  1401  static int qcom_scm_probe(struct platform_device *pdev)
d0f6fa7ba2d624 Andy Gross         2016-06-03  1402  {
d0f6fa7ba2d624 Andy Gross         2016-06-03  1403  	struct qcom_scm *scm;
ab0822d57d8ccd Sarangdhar Joshi   2016-11-15 @1404  	unsigned long clks;
6bf32599223634 Guru Das Srinagesh 2023-01-13  1405  	int irq, ret;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1406  
d0f6fa7ba2d624 Andy Gross         2016-06-03  1407  	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
d0f6fa7ba2d624 Andy Gross         2016-06-03  1408  	if (!scm)
d0f6fa7ba2d624 Andy Gross         2016-06-03  1409  		return -ENOMEM;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1410  
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1411  	ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1412  	if (ret < 0)
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1413  		return ret;
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1414  
65b7ebda502861 Sibi Sankar        2022-05-23  1415  	mutex_init(&scm->scm_bw_lock);
65b7ebda502861 Sibi Sankar        2022-05-23  1416  
ab0822d57d8ccd Sarangdhar Joshi   2016-11-15  1417  	clks = (unsigned long)of_device_get_match_data(&pdev->dev);
60cd420c91e28c Bjorn Andersson    2018-08-29  1418  
65b7ebda502861 Sibi Sankar        2022-05-23  1419  	scm->path = devm_of_icc_get(&pdev->dev, NULL);
65b7ebda502861 Sibi Sankar        2022-05-23  1420  	if (IS_ERR(scm->path))
65b7ebda502861 Sibi Sankar        2022-05-23  1421  		return dev_err_probe(&pdev->dev, PTR_ERR(scm->path),
65b7ebda502861 Sibi Sankar        2022-05-23  1422  				     "failed to acquire interconnect path\n");
65b7ebda502861 Sibi Sankar        2022-05-23  1423  
06987a4b9a3c31 Konrad Dybcio      2023-06-23  1424  	scm->core_clk = devm_clk_get_optional(&pdev->dev, "core");
d0f6fa7ba2d624 Andy Gross         2016-06-03  1425  	if (IS_ERR(scm->core_clk)) {
60cd420c91e28c Bjorn Andersson    2018-08-29  1426  		if (PTR_ERR(scm->core_clk) == -EPROBE_DEFER)
60cd420c91e28c Bjorn Andersson    2018-08-29  1427  			return PTR_ERR(scm->core_clk);
ab0822d57d8ccd Sarangdhar Joshi   2016-11-15  1428  	}
d0f6fa7ba2d624 Andy Gross         2016-06-03  1429  
06987a4b9a3c31 Konrad Dybcio      2023-06-23  1430  	scm->iface_clk = devm_clk_get_optional(&pdev->dev, "iface");
d0f6fa7ba2d624 Andy Gross         2016-06-03  1431  	if (IS_ERR(scm->iface_clk)) {
60cd420c91e28c Bjorn Andersson    2018-08-29  1432  		if (PTR_ERR(scm->iface_clk) == -EPROBE_DEFER)
60cd420c91e28c Bjorn Andersson    2018-08-29  1433  			return PTR_ERR(scm->iface_clk);
d0f6fa7ba2d624 Andy Gross         2016-06-03  1434  	}
60cd420c91e28c Bjorn Andersson    2018-08-29  1435  
06987a4b9a3c31 Konrad Dybcio      2023-06-23  1436  	scm->bus_clk = devm_clk_get_optional(&pdev->dev, "bus");
d0f6fa7ba2d624 Andy Gross         2016-06-03  1437  	if (IS_ERR(scm->bus_clk)) {
60cd420c91e28c Bjorn Andersson    2018-08-29  1438  		if (PTR_ERR(scm->bus_clk) == -EPROBE_DEFER)
60cd420c91e28c Bjorn Andersson    2018-08-29  1439  			return PTR_ERR(scm->bus_clk);
d0f6fa7ba2d624 Andy Gross         2016-06-03  1440  	}
d0f6fa7ba2d624 Andy Gross         2016-06-03  1441  
dd4fe5b292226f Bjorn Andersson    2016-06-17  1442  	scm->reset.ops = &qcom_scm_pas_reset_ops;
dd4fe5b292226f Bjorn Andersson    2016-06-17  1443  	scm->reset.nr_resets = 1;
dd4fe5b292226f Bjorn Andersson    2016-06-17  1444  	scm->reset.of_node = pdev->dev.of_node;
bd4760ca031567 Wei Yongjun        2016-08-28  1445  	ret = devm_reset_controller_register(&pdev->dev, &scm->reset);
bd4760ca031567 Wei Yongjun        2016-08-28  1446  	if (ret)
bd4760ca031567 Wei Yongjun        2016-08-28  1447  		return ret;
dd4fe5b292226f Bjorn Andersson    2016-06-17  1448  
d0f6fa7ba2d624 Andy Gross         2016-06-03  1449  	/* vote for max clk rate for highest performance */
d0f6fa7ba2d624 Andy Gross         2016-06-03  1450  	ret = clk_set_rate(scm->core_clk, INT_MAX);
d0f6fa7ba2d624 Andy Gross         2016-06-03  1451  	if (ret)
d0f6fa7ba2d624 Andy Gross         2016-06-03  1452  		return ret;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1453  
d0f6fa7ba2d624 Andy Gross         2016-06-03  1454  	__scm = scm;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1455  	__scm->dev = &pdev->dev;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1456  
6bf32599223634 Guru Das Srinagesh 2023-01-13  1457  	init_completion(&__scm->waitq_comp);
6bf32599223634 Guru Das Srinagesh 2023-01-13  1458  
f3d0fbad6765da Johan Hovold       2023-03-09  1459  	irq = platform_get_irq_optional(pdev, 0);
6bf32599223634 Guru Das Srinagesh 2023-01-13  1460  	if (irq < 0) {
6bf32599223634 Guru Das Srinagesh 2023-01-13  1461  		if (irq != -ENXIO)
6bf32599223634 Guru Das Srinagesh 2023-01-13  1462  			return irq;
6bf32599223634 Guru Das Srinagesh 2023-01-13  1463  	} else {
6bf32599223634 Guru Das Srinagesh 2023-01-13  1464  		ret = devm_request_threaded_irq(__scm->dev, irq, NULL, qcom_scm_irq_handler,
6bf32599223634 Guru Das Srinagesh 2023-01-13  1465  						IRQF_ONESHOT, "qcom-scm", __scm);
6bf32599223634 Guru Das Srinagesh 2023-01-13  1466  		if (ret < 0)
6bf32599223634 Guru Das Srinagesh 2023-01-13  1467  			return dev_err_probe(scm->dev, ret, "Failed to request qcom-scm irq\n");
6bf32599223634 Guru Das Srinagesh 2023-01-13  1468  	}
6bf32599223634 Guru Das Srinagesh 2023-01-13  1469  
f6ea568f0ddcdf Stephen Boyd       2021-02-23  1470  	__get_convention();
6b1751a86ce2eb Kumar Gala         2016-06-03  1471  
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1472  	/*
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1473  	 * If requested enable "download mode", from this point on warmboot
c19698a9e41bd6 Jiang Jian         2022-06-21  1474  	 * will cause the boot stages to enter download mode, unless
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1475  	 * disabled below by a clean shutdown/reboot.
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1476  	 */
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1477  	if (download_mode)
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1478  		qcom_scm_set_download_mode(true);
8c1b7dc9ba2294 Bjorn Andersson    2017-08-14  1479  
d0f6fa7ba2d624 Andy Gross         2016-06-03  1480  	return 0;
d0f6fa7ba2d624 Andy Gross         2016-06-03  1481  }
d0f6fa7ba2d624 Andy Gross         2016-06-03  1482  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki