Booting a recent kernel on a rk3399-based system (nanopc-t4),
equipped with a recent u-boot and ATF results in an Oops due
to a NULL pointer dereference.
This turns out to be due to the rk3399-dmc driver looking for
an *undocumented* property (rockchip,pmu), and happily using
a NULL pointer when the property isn't there.
Instead, make most of what was brought in with 9173c5ceb035
("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters
to TF-A.") conditioned on finding this property in the device-tree,
preventing the driver from exploding.
Cc: [email protected]
Fixes: 9173c5ceb035 ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.")
Signed-off-by: Marc Zyngier <[email protected]>
---
* From v2:
- Trimmed down commit message
- Cc stable
drivers/devfreq/rk3399_dmc.c | 42 ++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 24f04f78285b..027769e39f9b 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -95,18 +95,20 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
mutex_lock(&dmcfreq->lock);
- if (target_rate >= dmcfreq->odt_dis_freq)
- odt_enable = true;
-
- /*
- * This makes a SMC call to the TF-A to set the DDR PD (power-down)
- * timings and to enable or disable the ODT (on-die termination)
- * resistors.
- */
- arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
- dmcfreq->odt_pd_arg1,
- ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
- odt_enable, 0, 0, 0, &res);
+ if (dmcfreq->regmap_pmu) {
+ if (target_rate >= dmcfreq->odt_dis_freq)
+ odt_enable = true;
+
+ /*
+ * This makes a SMC call to the TF-A to set the DDR PD
+ * (power-down) timings and to enable or disable the
+ * ODT (on-die termination) resistors.
+ */
+ arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
+ dmcfreq->odt_pd_arg1,
+ ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
+ odt_enable, 0, 0, 0, &res);
+ }
/*
* If frequency scaling from low to high, adjust voltage first.
@@ -371,13 +373,14 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
}
node = of_parse_phandle(np, "rockchip,pmu", 0);
- if (node) {
- data->regmap_pmu = syscon_node_to_regmap(node);
- of_node_put(node);
- if (IS_ERR(data->regmap_pmu)) {
- ret = PTR_ERR(data->regmap_pmu);
- goto err_edev;
- }
+ if (!node)
+ goto no_pmu;
+
+ data->regmap_pmu = syscon_node_to_regmap(node);
+ of_node_put(node);
+ if (IS_ERR(data->regmap_pmu)) {
+ ret = PTR_ERR(data->regmap_pmu);
+ goto err_edev;
}
regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
@@ -399,6 +402,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
goto err_edev;
};
+no_pmu:
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
ROCKCHIP_SIP_CONFIG_DRAM_INIT,
0, 0, 0, 0, &res);
--
2.27.0
Hi Marc,
On 6/30/20 7:05 PM, Marc Zyngier wrote:
> Booting a recent kernel on a rk3399-based system (nanopc-t4),
> equipped with a recent u-boot and ATF results in an Oops due
> to a NULL pointer dereference.
>
> This turns out to be due to the rk3399-dmc driver looking for
> an *undocumented* property (rockchip,pmu), and happily using
> a NULL pointer when the property isn't there.
>
> Instead, make most of what was brought in with 9173c5ceb035
> ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters
> to TF-A.") conditioned on finding this property in the device-tree,
> preventing the driver from exploding.
>
> Cc: [email protected]
> Fixes: 9173c5ceb035 ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.")
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> * From v2:
> - Trimmed down commit message
> - Cc stable
>
> drivers/devfreq/rk3399_dmc.c | 42 ++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index 24f04f78285b..027769e39f9b 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -95,18 +95,20 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>
> mutex_lock(&dmcfreq->lock);
>
> - if (target_rate >= dmcfreq->odt_dis_freq)
> - odt_enable = true;
> -
> - /*
> - * This makes a SMC call to the TF-A to set the DDR PD (power-down)
> - * timings and to enable or disable the ODT (on-die termination)
> - * resistors.
> - */
> - arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
> - dmcfreq->odt_pd_arg1,
> - ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
> - odt_enable, 0, 0, 0, &res);
> + if (dmcfreq->regmap_pmu) {
> + if (target_rate >= dmcfreq->odt_dis_freq)
> + odt_enable = true;
> +
> + /*
> + * This makes a SMC call to the TF-A to set the DDR PD
> + * (power-down) timings and to enable or disable the
> + * ODT (on-die termination) resistors.
> + */
> + arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
> + dmcfreq->odt_pd_arg1,
> + ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
> + odt_enable, 0, 0, 0, &res);
> + }
>
> /*
> * If frequency scaling from low to high, adjust voltage first.
> @@ -371,13 +373,14 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
> }
>
> node = of_parse_phandle(np, "rockchip,pmu", 0);
> - if (node) {
> - data->regmap_pmu = syscon_node_to_regmap(node);
> - of_node_put(node);
> - if (IS_ERR(data->regmap_pmu)) {
> - ret = PTR_ERR(data->regmap_pmu);
> - goto err_edev;
> - }
> + if (!node)
> + goto no_pmu;
> +
> + data->regmap_pmu = syscon_node_to_regmap(node);
> + of_node_put(node);
> + if (IS_ERR(data->regmap_pmu)) {
> + ret = PTR_ERR(data->regmap_pmu);
> + goto err_edev;
> }
>
> regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
> @@ -399,6 +402,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
> goto err_edev;
> };
>
> +no_pmu:
> arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> ROCKCHIP_SIP_CONFIG_DRAM_INIT,
> 0, 0, 0, 0, &res);
>
Applied it. Thanks.
--
Best Regards,
Chanwoo Choi
Samsung Electronics