On Wed, Jul 03, 2024 at 01:46:54PM +0300, Dmitry Baryshkov wrote: > On Wed, Jul 03, 2024 at 02:46:42PM GMT, Varadarajan Narayanan wrote: > > cpr3 code assumes that 'acc_desc' is available for SoCs > > implementing CPR version 4 or less. However, IPQ9574 SoC > > implements CPRv4 without ACC. This causes NULL pointer accesses > > resulting in crashes. Hence, check if 'acc_desc' is populated > > before using it. > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > > --- > > v4: Undo the acc_desc validation in probe function as that could > > affect other SoC. > > --- > > drivers/pmdomain/qcom/cpr3.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pmdomain/qcom/cpr3.c b/drivers/pmdomain/qcom/cpr3.c > > index c7790a71e74f..6ceb7605f84d 100644 > > --- a/drivers/pmdomain/qcom/cpr3.c > > +++ b/drivers/pmdomain/qcom/cpr3.c > > @@ -2399,12 +2399,12 @@ static int cpr_pd_attach_dev(struct generic_pm_domain *domain, > > if (ret) > > goto exit; > > > > - if (acc_desc->config) > > + if (acc_desc && acc_desc->config) > > regmap_multi_reg_write(drv->tcsr, acc_desc->config, > > acc_desc->num_regs_per_fuse); > > > > /* Enable ACC if required */ > > - if (acc_desc->enable_mask) > > + if (acc_desc && acc_desc->enable_mask) > > regmap_update_bits(drv->tcsr, acc_desc->enable_reg, > > acc_desc->enable_mask, > > acc_desc->enable_mask); > > Should the same fix be applied to other places which access acc_desc? > For example cpr_pre_voltage() and cpr_post_voltage() which call > cpr_set_acc()? With this patch alone, if acc_desc is NULL, cpr_probe() will fail at the start itself because of this check if (!data->acc_desc && desc->cpr_type < CTRL_TYPE_CPRH) return -EINVAL; After applying this patch series, cpr_probe will cross the above check to accomodate IPQ9574. However, the check below will ensure drv->tcsr is not initialized. if (desc->cpr_type < CTRL_TYPE_CPRH && !of_device_is_compatible(dev->of_node, "qcom,ipq9574-cpr4")) cpr_pre_voltage() and cpr_post_voltage() call cpr_set_acc() only if drv->tcsr is not NULL. Hence acc_desc need not be checked. Will add the check to cpr_pre_voltage() and cpr_post_voltage() if you feel it will make it more robust regardless of the changes to cpr_probe in future. Please let me know. Thanks Varada
