Re: [PATCH 2/3] soc: qcom: smem: better track SMEM uninitialized state

[Bjorn Andersson]

On Wed, Oct 29, 2025 at 04:32:35PM +0100, Christian Marangi wrote:
> On Wed, Oct 29, 2025 at 05:27:33PM +0200, Andy Shevchenko wrote:
> > On Wed, Oct 29, 2025 at 02:33:20PM +0100, Christian Marangi wrote:
> > > There is currently a problem where, in the specific case of SMEM not
> > > initialized by SBL, any SMEM API wrongly returns PROBE_DEFER
> > > communicating wrong info to any user of this API.
> > > 
> > > A better way to handle this would be to track the SMEM state and return
> > > a different kind of error than PROBE_DEFER.
> > > 
> > > Rework the __smem handle to always init it to the error pointer
> > > -EPROBE_DEFER following what is already done by the SMEM API.
> > > If we detect that the SBL didn't initialized SMEM, set the __smem handle
> > > to the error pointer -ENODEV.
> > > Also rework the SMEM API to handle the __smem handle to be an error
> > > pointer and return it appropriately.
> > 
> > ...
> > 
> > >  	if (le32_to_cpu(header->initialized) != 1 ||
> > >  	    le32_to_cpu(header->reserved)) {
> > >  		dev_err(&pdev->dev, "SMEM is not initialized by SBL\n");
> > > +		__smem = ERR_PTR(-ENODEV);
> > >  		return -EINVAL;
> > >  	}
> > 
> > I find this a bit confusing. Why the error code returned to the upper layer is
> > different to the stored one?
> >
> 
> It's INVAL for probe. But for any user of SMEM it's NODEV as there isn't
> an actual SMEM usable.
> 
> Totally ok to change the error condition in probe if maybe NODEV is
> better suited. I assume there isn't a specific pattern of the correct
> error condition in probe.
> 

I'd say ENODEV represents the error better than EINVAL, so I don't have
any concerns with you changing the return value.

> > ...
> > 
> > Also, the series of patches should include the cover letter to explain not only
> > series background but additionally
> > - how it should be applied
> > - if it has dependencies
> > - etc
> > 
> 
> Didn't add one they are trivial patch but I can add it if needed... it's
> pretty stable code so no dependency or branch target
> 

Specifically, I should merge patch 1 and 2 through the qcom/soc tree,
and patch 3 can be merged completely independently through the cpufreq
tree.

Regards,
Bjorn

> > 
> > 
> 
> -- 
> 	Ansuel