Re: [PATCH] ASoC: ti: omap-mcbsp: duplicate sysfs failure after PROBE_DEFER

[David Owens]

On 6/14/22 11:08, Mark Brown wrote:
> On Tue, Jun 14, 2022 at 10:59:31AM -0500, David Owens wrote:
>
> > The call to sdma_pcm_platform_register() can return PROBE_DEFER, leading
> > to omap_mcbsp_init() being called multiple times.  sysfs node creation
> > fails in subsequent calls to omap_mcbsp_init(), which prevents
> > the driver from ever successfully probing.  The resulting errors can be
> > seen during boot:
> >
> > [    1.749328] sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
> > [    1.759643] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 5.18.0-yocto-standard #1
> > [    1.767181] Hardware name: Generic OMAP36xx (Flattened Device Tree)
> > [    1.773498] Workqueue: events_unbound deferred_probe_work_func
> > [    1.779449]  unwind_backtrace from show_stack+0x10/0x14
> > [    1.784729]  show_stack from sysfs_warn_dup+0x4c/0x60
> Please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative (it often is
> for search engines if nothing else) then it's usually better to pull out
> the relevant sections.
Probably just the "sysfs:" line would suffice.  I'll be sure to limit
traces in the future, thanks for the tip.
> > +++ b/sound/soc/ti/omap-mcbsp.c
> > @@ -1403,6 +1403,10 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
> >          mcbsp->dev = &pdev->dev;
> >          platform_set_drvdata(pdev, mcbsp);
> >
> > +       ret = sdma_pcm_platform_register(&pdev->dev, "tx", "rx");
> > +       if (ret)
> > +               return ret;
> > +
> >          ret = omap_mcbsp_init(pdev);
> >          if (ret)
> >                  return ret;
> > @@ -1412,13 +1416,9 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
> >                  omap_mcbsp_dai.capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
> >          }
> >
> > -       ret = devm_snd_soc_register_component(&pdev->dev,
> > +       return devm_snd_soc_register_component(&pdev->dev,
> >                                                &omap_mcbsp_component,
> >                                                &omap_mcbsp_dai, 1);
> > -       if (ret)
> > -               return ret;
> > -
> > -       return sdma_pcm_platform_register(&pdev->dev, "tx", "rx");
> >   }
> It's not clear to me how this fixes the problem, your commit message
> doesn't mention how?  I was expecting to see more error handling paths
> being added to unwind the sysfs allocation, or a conversion to devm.  As
> things stand it's not clear to me that the error won't persist in the
> case where we defer registering the component.

Moving sdma_pcm_platform_register() before sysfs node creation allowed
probe to exit with EPROBE_DEFER without needing to unwind sysfs nodes.
While this worked in my particular case, as you point out it might not
address all probe deferrals.  I can look for a more robust solution and
submit as a v2 patch.

-Dave

This email is intended solely for the use of the individual to whom it is addressed and may contain confidential and/or privileged material. Any views or opinions presented are solely those of the author and do not necessarily represent those of Precision Planting. If you are not the intended recipient, be advised that you have received this email in error and that any use, dissemination, forwarding, printing, or copying of this email is strictly prohibited. Neither AGCO nor the sender accepts any responsibility for viruses, and it is your responsibility to scan, and virus check the e-mail and its attachment(s) (if any).