Messages in this thread | | | From | Kai-Heng Feng <> | Date | Fri, 14 Jan 2022 20:09:18 +0800 | Subject | Re: [PATCH] misc: rtsx: modify rtd3 flow |
| |
On Fri, Jan 14, 2022 at 4:51 PM Ricky WU <ricky_wu@realtek.com> wrote: [snip] > > > > > > We don’t want to entry D3 frequently > > > So we need to call pm_runtime_get() at start And call pm_runtime_put() > > > in delay-work (rtd3_work) > > > > Maybe use 'cancel_delayed_work(&pcr->rtd3_work)' like what > > rtsx_pci_runtime_suspend() does? > > And for this case maybe cancel_delayed_work_sync() is more preferred. > > > > I think you misunderstand what I means > This delay_work() is for not enter D3 <-> D0 frequently
This is doable with current pm_runtime_*() helpers.
> That delay is we need, we don’t want to power_on and power_off frequently on our device
Is this to avoid the pm_runtime_resume() call before system suspend? IOW, to avoid D3 -> D0 -> D3 for S3?
> > This patch want to solve pcr->is_runtime_suspended this value > because we need to set more register at power_down flow when Device support D3 and System going to S3
So is it possible to introduce a new parameter for force_power_down() and get rid of is_runtime_suspended completely?
> > > > > > > But we found If we keep this if statement in start_run if > > > (pcr->is_runtime_suspended) { > > > pm_runtime_get(&(pcr->pci->dev)); > > > pcr->is_runtime_suspended = false; > > > } > > > pcr->is_runtime_suspended this status are not correct when enter S3 > > > because enter S3 not call start_run() > > > > Maybe because the driver is trying to trick the runtime PM core on its real > > power status? > > I.e. the driver is maintaining its own PM state machine. Fully cooperating the > > driver with PM core should solve the issue. > > > > System not call start_run() because do not have any sd_request at that time, > so we need to update value(pcr->is_runtime_suspended) at rtsx_pci_runtime_resume > but if we only update value here not to call pm_runtime_get(), the if-statement always be FALSE at start_run()
Sounds like the RPM refcount goes to zero so it was runtime suspended again. The correct usage should be merging rtsx_pci_start_run() with rtsx_pci_runtime_resume(), then guard each op of mmc_host_ops with pm_runtime_get_sync() at the beginning and pm_runtime_idle() at and end of each ops.
Increase the RPM refcount in runtime resume routine to prevent the driver from suspending doesn't really make sense to me.
Kai-Heng
> that is why we move this if-statement from start_run() to rtsx_pci_runtime_resume() > > > > > > > > > > + > > > > > mutex_lock(&pcr->pcr_mutex); > > > > > > > > > > rtsx_pci_write_register(pcr, HOST_SLEEP_STATE, 0x03, > > > > > 0x00); diff --git a/drivers/misc/cardreader/rtsx_pcr.h > > > > > b/drivers/misc/cardreader/rtsx_pcr.h > > > > > index daf057c4eea6..b93975268e6d 100644 > > > > > --- a/drivers/misc/cardreader/rtsx_pcr.h > > > > > +++ b/drivers/misc/cardreader/rtsx_pcr.h > > > > > @@ -25,6 +25,7 @@ > > > > > #define REG_EFUSE_POWEROFF 0x00 > > > > > #define RTS5250_CLK_CFG3 0xFF79 > > > > > #define RTS525A_CFG_MEM_PD 0xF0 > > > > > +#define RTS524A_AUTOLOAD_CFG1 0xFF7C > > > > > #define RTS524A_PM_CTRL3 0xFF7E > > > > > #define RTS525A_BIOS_CFG 0xFF2D > > > > > #define RTS525A_LOAD_BIOS_FLAG 0x01 > > > > > -- > > > > > 2.25.1 > > > > ------Please consider the environment before printing this e-mail.
| |