Re: [PATCH 2/2] ARM: qcom_defconfig: Enable Qualcomm remoteproc and SMP2P drivers

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 07 Mar 03:01 CET 2017, Jonathan Neusch?fer wrote:

> On Sun, Mar 05, 2017 at 04:49:30PM +0000, Bjorn Andersson wrote:
> > On Sun 05 Mar 05:01 GMT 2017, Jonathan Neusch?fer wrote:
> [...]
> > > FWIW, I enabled CONFIG_QCOM_SMSM on my test system (an Asus Padfone,
> > > based on MSM8974; I'm using the Sony Xperia Honami DT because it's close
> > > enough), and I think it failed to initialize:
> > > 
> > 
> > Using Honami should work so far, but please do write a patch adding the
> > Padfone, so that we don't accidentally break your HW at some point.
> 
> I can try how far I get, but unfortunately I don't have hardware
> documentation or schematics, because I'm a hobbyist.
> 

If this is something that you will continue to hack on and you think
anyone else will be interested in having then please do submit patches
for it.

> > > [    0.647743] qcom-smsm smsm: no smsm size info, using defaults
> > > [    0.647775] qcom-smsm smsm: unable to allocate shared state entry
> > > 
> > 
> > Could you please confirm where in qcom_smem_alloc_global() we're
> > failing? As far as I can tell we should fail with -EEXIST or if the
> > passed "size" parameter is bogus -ENOMEM (but the default number of
> > entries really should be less than the amount of free SMEM space).
> 
> * qcom_smem_get returns -EPROBE_DEFER:
> 
> 	void *ptr = ERR_PTR(-EPROBE_DEFER);
> 	if (!__smem)
> 		return ptr;
> 
> * smsm_get_size_info prints "no smsm size info, using defaults\n"
> * qcom_smem_alloc also returns -EPROBE_DEFER early.
> 
> 
> BTW, I think smsm_get_size_info is using uninitialized memory here:
> 
>         size_t size;    /* is uninitialized */
>         struct { ... } *info;
> 
>         /* qcom_smem_get returns early without setting size */
>         info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, &size);
> 
>         /*
> 	 * PTR_ERR(info) is not -ENOENT.
>          * size (still uninitialized) is compared with the size of the local
>          * struct defined above.
> 	 */
>         if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
>                 ...
>         }
> 

Thanks for your analysis!

As you say the smsm driver is missing handling of probe deferral - which
is wrong. Could you please propose a patch checking for EPROBE_DEFER and
propagate this error as return value from probe()?  (Without an error
message)

Regards,
Bjorn