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 <[email protected]>
---
Changes in v2:
- Fix GCC Wunused-but-set-variable in patch 1 (thanks intel robot)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
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 | 90 ++++++++-------------------------------------
1 file changed, 16 insertions(+), 74 deletions(-)
---
base-commit: 8d2be868b42c08290509c60515865f4de24ea704
change-id: 20230623-topic-scm_cleanup-c4620461126a
Best regards,
--
Konrad Dybcio <[email protected]>
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 <[email protected]>
---
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 237d05d6208b..b8398002205d 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1419,22 +1419,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
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 <[email protected]>
---
drivers/firmware/qcom_scm.c | 73 ++++++++-------------------------------------
1 file changed, 13 insertions(+), 60 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index fde33acd46b7..237d05d6208b 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;
@@ -1405,7 +1401,6 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
static int qcom_scm_probe(struct platform_device *pdev)
{
struct qcom_scm *scm;
- unsigned long clks;
int irq, ret;
scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
@@ -1418,50 +1413,27 @@ static int qcom_scm_probe(struct platform_device *pdev)
mutex_init(&scm->scm_bw_lock);
- clks = (unsigned long)of_device_get_match_data(&pdev->dev);
-
scm->path = devm_of_icc_get(&pdev->dev, NULL);
if (IS_ERR(scm->path))
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 +1484,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
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 <[email protected]>
---
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 b8398002205d..b6055b1c18d8 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1478,20 +1478,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
On Sat, 24 Jun 2023 14:23:44 +0200, Konrad Dybcio wrote:
> 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.
>
>
Applied, thanks!
[1/3] firmware: qcom_scm: Always try to consume all three clocks
commit: f14459df6721cd23feb4618e8bbe3778d0d0a552
[2/3] firmware: qcom_scm: Always return devm_clk_get_optional errors
commit: f8e27cdc8833dee56f43e767dd16c97488d3af11
[3/3] firmware: qcom_scm: Drop useless compatibles
commit: b1d48e61d8068304fda215880ca021c22b00a19a
Best regards,
--
Bjorn Andersson <[email protected]>