PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent
Subsystems (2)
PATCH v3 1/1 PM / devfreq: rk3399_dmc: Fix kernel oops when rockchip,pmu is absent
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.
---
* From v2:
- Trimmed down commit message
- Cc stable
drivers/devfreq/rk3399_dmc.c | 42 ++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 19 deletions(-)
drivers/devfreq/rk3399_dmc.c +23 -19
@@ -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);
Signature · 1 line
2.27.0
Hi Marc,
Quoted 85 lines · Marc Zyngier
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.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 stabledrivers/devfreq/rk3399_dmc.c | 42 ++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 19 deletions(-)drivers/devfreq/rk3399_dmc.c +22
-19@@ -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);
drivers/devfreq/rk3399_dmc.c static int rk3399_dmcfreq_probe(struct platform_device *pdev) · line 408
Quoted 7 lines
drivers/devfreq/rk3399_dmc.c +1
-0@@ -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.
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Sasha Levin
maintainerHi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag
fixing commit: 9173c5ceb035 ("PM / devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.").
The bot has tested the following trees: v5.7.7, v5.4.50.
v5.7.7: Build OK!
v5.4.50: Failed to apply! Possible dependencies:
29d867e97f7d7 ("PM / devfreq: rk3399_dmc: Add missing of_node_put()")
39a6e4739c19d ("PM / devfreq: rk3399_dmc: Disable devfreq-event device when fails")NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
Signature · 2 lines
Thanks Sasha