All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ricky WU <ricky_wu@realtek.com>
To: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: "arnd@arndb.de" <arnd@arndb.de>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"christophe.jaillet@wanadoo.fr" <christophe.jaillet@wanadoo.fr>,
	"yang.lee@linux.alibaba.com" <yang.lee@linux.alibaba.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] misc: rtsx: modify rtd3 flow
Date: Fri, 14 Jan 2022 06:56:45 +0000	[thread overview]
Message-ID: <3bae27438b4b4d1eb367baefa7898060@realtek.com> (raw)
In-Reply-To: <CAAd53p6gzZVNov0L-abZ_N=FoD-JpfWp2W096SrVva=FfEv=aw@mail.gmail.com>

> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Thursday, January 13, 2022 7:33 PM
> To: Ricky WU <ricky_wu@realtek.com>
> Cc: arnd@arndb.de; gregkh@linuxfoundation.org;
> christophe.jaillet@wanadoo.fr; yang.lee@linux.alibaba.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] misc: rtsx: modify rtd3 flow
> 
> On Thu, Jan 13, 2022 at 6:50 PM Ricky WU <ricky_wu@realtek.com> wrote:
> >
> > move pm_runtime_get() to _runtime_resume when System enter S3, do not
> > have sd_request and do not call start_run to pm_runtime_get() cause
> > is_runtime_suspended status not correct
> >
> > set more register in power_down flow to make plugin or unplug card do
> > not wake up system when system is at S3
> >
> > Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
> > ---
> >  drivers/misc/cardreader/rts5249.c  | 31
> > ++++++++++++++++++++++++++++--  drivers/misc/cardreader/rtsx_pcr.c |
> > 17 ++++++++--------  drivers/misc/cardreader/rtsx_pcr.h |  1 +
> >  3 files changed, 38 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/misc/cardreader/rts5249.c
> > b/drivers/misc/cardreader/rts5249.c
> > index 53f3a1f45c4a..69e32f075ca9 100644
> > --- a/drivers/misc/cardreader/rts5249.c
> > +++ b/drivers/misc/cardreader/rts5249.c
> > @@ -74,7 +74,8 @@ static void rtsx_base_fetch_vendor_settings(struct
> rtsx_pcr *pcr)
> >         pci_read_config_dword(pdev, PCR_SETTING_REG2, &reg);
> >         pcr_dbg(pcr, "Cfg 0x%x: 0x%x\n", PCR_SETTING_REG2, reg);
> >
> > -       pcr->rtd3_en = rtsx_reg_to_rtd3_uhsii(reg);
> > +       if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr, PID_525A))
> > +               pcr->rtd3_en = rtsx_reg_to_rtd3_uhsii(reg);
> >
> >         if (rtsx_check_mmc_support(reg))
> >                 pcr->extra_caps |= EXTRA_CAPS_NO_MMC; @@ -143,6
> > +144,27 @@ static int rts5249_init_from_hw(struct rtsx_pcr *pcr)
> >         return 0;
> >  }
> >
> > +static void rts52xa_force_power_down(struct rtsx_pcr *pcr, u8
> > +pm_state) {
> > +       /* Set relink_time to 0 */
> > +       rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 1,
> MASK_8_BIT_DEF, 0);
> > +       rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 2,
> MASK_8_BIT_DEF, 0);
> > +       rtsx_pci_write_register(pcr, AUTOLOAD_CFG_BASE + 3,
> > +                               RELINK_TIME_MASK, 0);
> > +
> > +       rtsx_pci_write_register(pcr, RTS524A_PM_CTRL3,
> > +                       D3_DELINK_MODE_EN,
> D3_DELINK_MODE_EN);
> > +
> > +       if (!pcr->is_runtime_suspended) {
> > +               rtsx_pci_write_register(pcr, RTS524A_AUTOLOAD_CFG1,
> > +                               CD_RESUME_EN_MASK, 0);
> > +               rtsx_pci_write_register(pcr, RTS524A_PM_CTRL3, 0x01,
> 0x00);
> > +               rtsx_pci_write_register(pcr, RTS524A_PME_FORCE_CTL,
> 0x30, 0x20);
> > +       }
> > +
> > +       rtsx_pci_write_register(pcr, FPDCTL, ALL_POWER_DOWN,
> > +ALL_POWER_DOWN); }
> > +
> >  static void rts52xa_save_content_from_efuse(struct rtsx_pcr *pcr)  {
> >         u8 cnt, sv;
> > @@ -281,8 +303,11 @@ static int rts5249_extra_init_hw(struct rtsx_pcr
> > *pcr)
> >
> >         rtsx_pci_send_cmd(pcr, CMD_TIMEOUT_DEF);
> >
> > -       if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr, PID_525A))
> > +       if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr, PID_525A))
> > + {
> >                 rtsx_pci_write_register(pcr, REG_VREF,
> PWD_SUSPND_EN,
> > PWD_SUSPND_EN);
> > +               rtsx_pci_write_register(pcr, RTS524A_AUTOLOAD_CFG1,
> > +                       CD_RESUME_EN_MASK,
> CD_RESUME_EN_MASK);
> > +       }
> >
> >         if (pcr->rtd3_en) {
> >                 if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr,
> > PID_525A)) { @@ -724,6 +749,7 @@ static const struct pcr_ops
> rts524a_pcr_ops = {
> >         .card_power_on = rtsx_base_card_power_on,
> >         .card_power_off = rtsx_base_card_power_off,
> >         .switch_output_voltage = rtsx_base_switch_output_voltage,
> > +       .force_power_down = rts52xa_force_power_down,
> >         .set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,  };
> >
> > @@ -841,6 +867,7 @@ static const struct pcr_ops rts525a_pcr_ops = {
> >         .card_power_on = rts525a_card_power_on,
> >         .card_power_off = rtsx_base_card_power_off,
> >         .switch_output_voltage = rts525a_switch_output_voltage,
> > +       .force_power_down = rts52xa_force_power_down,
> >         .set_l1off_cfg_sub_d0 = rts5250_set_l1off_cfg_sub_d0,  };
> >
> > diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> > b/drivers/misc/cardreader/rtsx_pcr.c
> > index 6ac509c1821c..a83adfb122dc 100644
> > --- a/drivers/misc/cardreader/rtsx_pcr.c
> > +++ b/drivers/misc/cardreader/rtsx_pcr.c
> > @@ -152,12 +152,6 @@ void rtsx_pci_start_run(struct rtsx_pcr *pcr)
> >         if (pcr->remove_pci)
> >                 return;
> >
> > -       if (pcr->rtd3_en)
> > -               if (pcr->is_runtime_suspended) {
> > -                       pm_runtime_get(&(pcr->pci->dev));
> > -                       pcr->is_runtime_suspended = false;
> > -               }
> > -
> >         if (pcr->state != PDEV_STAT_RUN) {
> >                 pcr->state = PDEV_STAT_RUN;
> >                 if (pcr->ops->enable_auto_blink) @@ -1597,6 +1591,7
> @@
> > static int rtsx_pci_probe(struct pci_dev *pcidev,
> >         pcr->host_sg_tbl_addr = pcr->rtsx_resv_buf_addr +
> HOST_CMDS_BUF_LEN;
> >         pcr->card_inserted = 0;
> >         pcr->card_removed = 0;
> > +       pcr->rtd3_en = 0;
> >         INIT_DELAYED_WORK(&pcr->carddet_work,
> rtsx_pci_card_detect);
> >         INIT_DELAYED_WORK(&pcr->idle_work, rtsx_pci_idle_work);
> >
> > @@ -1796,17 +1791,16 @@ static int rtsx_pci_runtime_suspend(struct
> device *device)
> >         pcr = handle->pcr;
> >         dev_dbg(&(pcidev->dev), "--> %s\n", __func__);
> >
> > +       pcr->is_runtime_suspended = true;
> > +
> >         cancel_delayed_work(&pcr->carddet_work);
> >         cancel_delayed_work(&pcr->rtd3_work);
> >         cancel_delayed_work(&pcr->idle_work);
> >
> >         mutex_lock(&pcr->pcr_mutex);
> >         rtsx_pci_power_off(pcr, HOST_ENTER_S3);
> > -
> >         mutex_unlock(&pcr->pcr_mutex);
> >
> > -       pcr->is_runtime_suspended = true;
> > -
> >         return 0;
> >  }
> >
> > @@ -1820,6 +1814,11 @@ static int rtsx_pci_runtime_resume(struct device
> *device)
> >         pcr = handle->pcr;
> >         dev_dbg(&(pcidev->dev), "--> %s\n", __func__);
> >
> > +       if (pcr->is_runtime_suspended) {
> > +               pm_runtime_get(&(pcr->pci->dev));
> > +               pcr->is_runtime_suspended = false;
> > +       }
> 
> If the runtime resume routine is called for system wide suspend, the runtime
> suspend isn't allowed during the period.
> So I don't quite understand what this patch is for.
> 

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)

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()

> Kai-Heng
> 
> > +
> >         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.

  reply	other threads:[~2022-01-14  6:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 10:50 [PATCH] misc: rtsx: modify rtd3 flow Ricky WU
2022-01-13 11:32 ` Kai-Heng Feng
2022-01-14  6:56   ` Ricky WU [this message]
2022-01-14  7:29     ` Kai-Heng Feng
2022-01-14  8:51       ` Ricky WU
2022-01-14 12:09         ` Kai-Heng Feng
2022-01-17  3:34           ` Ricky WU
2022-01-17  5:11             ` Kai-Heng Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3bae27438b4b4d1eb367baefa7898060@realtek.com \
    --to=ricky_wu@realtek.com \
    --cc=arnd@arndb.de \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=gregkh@linuxfoundation.org \
    --cc=kai.heng.feng@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yang.lee@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.