| Message ID | 20231209215601.3543895-2-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] dt-bindings: soc: qcom: stats: add compatible for SM8150 platform | expand |
Hi, On Mon, Dec 11, 2023 at 1:11 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 9.12.2023 22:55, Dmitry Baryshkov wrote: > > The stats ram on sm8150 platform contains invalid data at the > > DDR_DYNAMIC_OFFSET. Most likely this is because the platform didn't > > support DDR sleep stats. > Interesting. Can you read back DDR_DYNAMIC_OFFSET on 8350/8280 and > see if 8150 has correct data in there? > > > However this platform uses generic > > "qcom,rpmh-stats" compatible, which implies presense of the DDR data. > > Add safety net to prevent old DTB files from crashing the > > qcom,rpmh-stats driver. > Yeah I'dve never thought there would be garbage in there.. > > I'd advocate for simply not doing anything wrt sleep stats if DDR > stats are unavailable though. The QMP handle can stay, as there > may (I don't know) be more data available that we want to export > through this driver. FWIW, I'm getting a crash on sc7180-trogdor like this too. In kgdb it says I'm on line: key = readl(ddrd->base); ...and (gdb) print ddrd->base $1 = (void *) 0xffffffc0833a3149 (gdb) print reg $2 = (void *) 0xffffffc0833a3000 ...so I guess my "stats_offset" must have been 0x149. Can we get a fix landed or a revert? Thanks! :-) -Doug
diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c index 4763d62a8cb0..813c9f3c6bec 100644 --- a/drivers/soc/qcom/qcom_stats.c +++ b/drivers/soc/qcom/qcom_stats.c @@ -319,6 +319,7 @@ static void qcom_create_subsystem_stat_files(struct dentry *root, static int qcom_create_ddr_stats_files(struct device *dev, struct dentry *root, void __iomem *reg, + resource_size_t reg_size, const struct stats_config *config) { struct ddr_stats_data *ddrd; @@ -337,6 +338,8 @@ static int qcom_create_ddr_stats_files(struct device *dev, /* Get the offset of DDR stats */ stats_offset = readl(reg + DDR_DYNAMIC_OFFSET) & DDR_OFFSET_MASK; + if (stats_offset >= reg_size || stats_offset % 4) + return -EINVAL; ddrd->base = reg + stats_offset; /* Check if DDR stats are present */ @@ -364,6 +367,7 @@ static int qcom_stats_probe(struct platform_device *pdev) void __iomem *reg; struct dentry *root; const struct stats_config *config; + struct resource *res; struct stats_data *d; int i, ret; @@ -371,7 +375,7 @@ static int qcom_stats_probe(struct platform_device *pdev) if (!config) return -ENODEV; - reg = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); + reg = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(reg)) return -ENOMEM; @@ -387,7 +391,9 @@ static int qcom_stats_probe(struct platform_device *pdev) qcom_create_subsystem_stat_files(root, config); qcom_create_soc_sleep_stat_files(root, reg, d, config); - ret = qcom_create_ddr_stats_files(&pdev->dev, root, reg, config); + ret = qcom_create_ddr_stats_files(&pdev->dev, root, reg, + resource_size(res), + config); if (ret) { debugfs_remove_recursive(root); return ret;
The stats ram on sm8150 platform contains invalid data at the DDR_DYNAMIC_OFFSET. Most likely this is because the platform didn't support DDR sleep stats. However this platform uses generic "qcom,rpmh-stats" compatible, which implies presense of the DDR data. Add safety net to prevent old DTB files from crashing the qcom,rpmh-stats driver. Fixes: e84e61bdb97c ("soc: qcom: stats: Add DDR sleep stats") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/soc/qcom/qcom_stats.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)