Messages in this thread | | | From | Jernej Škrabec <> | Subject | Re: Re: [PATCH v1 2/3] media: cedrus: Set the platform driver data earlier | Date | Thu, 25 Aug 2022 23:04:50 +0200 |
| |
Dne nedelja, 21. avgust 2022 ob 22:40:21 CEST je Dmitry Osipenko napisal(a): > On 8/20/22 11:25, Jernej Škrabec wrote: > > Dne petek, 19. avgust 2022 ob 17:37:20 CEST je Nicolas Dufresne napisal(a): > >> Le vendredi 19 août 2022 à 06:17 +0200, Jernej Škrabec a écrit : > >>> Dne četrtek, 18. avgust 2022 ob 22:33:07 CEST je Nicolas Dufresne > > > > napisal(a): > >>>> From: Dmitry Osipenko <dmitry.osipenko@collabora.com> > >>>> > >>>> The cedrus_hw_resume() crashes with NULL deference on driver probe if > >>>> runtime PM is disabled because it uses platform data that hasn't been > >>>> set up yet. Fix this by setting the platform data earlier during probe. > >>> > >>> Does it even work without PM? Maybe it would be better if Cedrus would > >>> select PM in Kconfig. > >> > >> I cannot comment myself on this, but it does not seem to invalidate this > >> Dmitry's fix. > > > > If NULL pointer dereference happens only when PM is disabled, then it > > does. I have PM always enabled and I never experienced above issue. > > Originally this fix was needed when I changed cedrus_hw_probe() to do > the rpm-resume while was debugging the hang issue and then also noticed > that the !PM should be broken. It's a good common practice for all > drivers to have the drv data set early to avoid such problems, hence it > won't hurt to make this change anyways.
Ok, as others pointed out, this is still a good thing, so:
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > In practice nobody disables PM other than for debugging purposes and !PM > handling is often broken in drivers. I assume that it might be even > better to enable PM for all Allwiner SoCs and remove !PM handling from > all the affected drivers, like it was done for Tegra some time ago. >
Maybe in the future :)
Best regards, Jernej
> -- > Best regards, > Dmitry
| |