Re: [PATCH v5 1/5] remoteproc: qcom: qcom_wcnss: Add support for pronto-v3

["Sireesh Kodali" <sireeshkodali1@gmail.com>]

On Thu Sep 29, 2022 at 3:09 PM IST, Stephan Gerhold wrote:
> On Thu, Sep 29, 2022 at 10:32:05AM +0530, Sireesh Kodali wrote:
> > From: Vladimir Lypak <vladimir.lypak@gmail.com>
> > 
> > Pronto-v3 is similar to pronto-v2. It requires two power domains, and it
> > requires the xo clock. It is used on the MSM8953 platform.
> > 
> > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com>
> > Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>
> > ---
> >  drivers/remoteproc/qcom_wcnss.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> > index 68f37296b151..ff18bfae5eb6 100644
> > --- a/drivers/remoteproc/qcom_wcnss.c
> > +++ b/drivers/remoteproc/qcom_wcnss.c
> > @@ -141,6 +141,18 @@ static const struct wcnss_data pronto_v2_data = {
> >  	.num_vregs = 1,
> >  };
> >  
> > +static const struct wcnss_data pronto_v3_data = {
> > +	.pmu_offset = 0x1004,
> > +	.spare_offset = 0x1088,
> > +
> > +	.pd_names = { "mx", "cx" },
> > +	.vregs = (struct wcnss_vreg_info[]) {
> > +		{ "vddpx", 1800000, 1800000, 0 },
> > +	},
> > +	.num_pd_vregs = 2,
>
> Can you try dropping this line? num_pd_vregs = 2 means:
> "If power domains are specified in the device tree, skip the first two
>  entries in 'vregs'." For pronto-v1/v2 it would skip the "vddmx" and
> "vddcx" entry, since those are already covered by the power domains.
>
> However, since pronto-v3 should always have power domains in DT and
> "vregs" has just a single entry this means that it would always access
> the array out of bounds at runtime and request some garbage regulator.
> I'm confused why this does not crash more spectacularly...

Indeed it should have crashed, thanks for catching this. Will be fixed
in v6
>
> Thanks,
> Stephan
>
> > +	.num_vregs = 1,
> > +};
> > +
> >  static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
> >  {
> >  	struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv;
> > @@ -675,6 +687,7 @@ static const struct of_device_id wcnss_of_match[] = {
> >  	{ .compatible = "qcom,riva-pil", &riva_data },
> >  	{ .compatible = "qcom,pronto-v1-pil", &pronto_v1_data },
> >  	{ .compatible = "qcom,pronto-v2-pil", &pronto_v2_data },
> > +	{ .compatible = "qcom,pronto-v3-pil", &pronto_v3_data },
> >  	{ },
> >  };
> >  MODULE_DEVICE_TABLE(of, wcnss_of_match);
> > -- 
> > 2.37.3
> >