On 18/06/24 01:04, Dmitry Baryshkov wrote: > On Mon, 17 Jun 2024 at 17:16, Aradhya Bhatia <a-bhatia1@xxxxxx> wrote: >> >> Hi Dmitry, >> >> Thanks for reviewing the patches! >> >> On 17/06/24 17:29, Dmitry Baryshkov wrote: >>> On Mon, Jun 17, 2024 at 04:23:03PM GMT, Aradhya Bhatia wrote: >>>> Update the Phy initialized state to "not initialized" when the driver >>>> (and the hardware by extension) gets suspended. This will allow the Phy >>>> to get initialized again after resume. >>>> >>>> Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver") >>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx> >>>> --- >>>> drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c >>>> index b016f2ba06bb..42565e253b2d 100644 >>>> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c >>>> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c >>>> @@ -1153,6 +1153,7 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev) >>>> clk_disable_unprepare(dsi->dsi_p_clk); >>>> reset_control_assert(dsi->dsi_p_rst); >>>> dsi->link_initialized = false; >>> >>> Most likely you should also call phy_exit() here. And in _remove() too. >> >> I agree that phy_exit should be called here. But why in _remove()? >> Wouldn't having phy_exit in 2 places mess up the internal ref count? > > If suspend() is going to be called in the teardown path, then it's > fine to have just one call here. Otherwise you might add one guarded > with if (phy_initialized) to _remove() too. Since the _suspend() and _resume() hooks are registered for both, system sleep PM and runtime PM, the _suspend() hook does get called when I remove the cdns_dsi module using modprobe. Tested that on my setup. I believe having phy_exit() only in _suspend() would suffice for now. > >> >>> >>>> + dsi->phy_initialized = false; >>>> return 0; >>>> } >>>> >>>> -- >>>> 2.34.1 >>>> >>> >> -- >> Regards >> Aradhya > > > -- Regards Aradhya