Re: [PATCH] mmc: rtsx: Fixed TX/RX register and optimized TX parameter

[Ulf Hansson]

On Tue, 17 Mar 2020 at 03:45, åææ Ricky <ricky_wu@xxxxxxxxxxx> wrote:
>
>
> > -----Original Message-----
> > From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx]
> > Sent: Monday, March 16, 2020 10:17 PM
> > To: åææ Ricky
> > Cc: linux-mmc@xxxxxxxxxxxxxxx; Linux Kernel Mailing List; Arnd Bergmann; Greg
> > Kroah-Hartman
> > Subject: Re: [PATCH] mmc: rtsx: Fixed TX/RX register and optimized TX
> > parameter
> >
> > On Mon, 16 Mar 2020 at 03:52, <ricky_wu@xxxxxxxxxxx> wrote:
> > >
> > > From: Ricky Wu <ricky_wu@xxxxxxxxxxx>
> > >
> > > Fixed sd_change_phase TX/RX register
> > > Optimized rts522a rts524a rts525a rts5260 rts5261 TX initial parameter
> >
> > I don't understand what this actually fixes. Can you try to elaborate
> > a bit on this, please?
> >
> > Do you think this is needed for stable, then why?
>
> Yes, I think it need patch to stable
>
> In function sd_change_phase() whether it is TX or RX always write the same register(TX), so patch this function make RX and TX can change well.
> At mmc stack mechanism do not tuning TX phase, so need give a stable parameter for TX phase(sdr104 sdr50 ddr50) at initial.

Alright, I took the liberty to clarify the changelog a bit according
to this. Please have a look at my fixes branch and tell me if I should
do some additional changes of the changelog.

Applied for fixes and by adding a stable tag, thanks!

Arnd, Greg, so I am picking this via my mmc tree, please shout at me
(or ack it) if you have any concerns with this. :-)

Kind regards
Uffe


>
> >
> > Kind regards
> > Uffe
> >
> > >
> > > Signed-off-by: Ricky Wu <ricky_wu@xxxxxxxxxxx>
> > > ---
> > >  drivers/misc/cardreader/rts5227.c |  2 +-
> > >  drivers/misc/cardreader/rts5249.c |  2 ++
> > >  drivers/misc/cardreader/rts5260.c |  2 +-
> > >  drivers/misc/cardreader/rts5261.c |  2 +-
> > >  drivers/mmc/host/rtsx_pci_sdmmc.c | 13 ++++++++-----
> > >  5 files changed, 13 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/misc/cardreader/rts5227.c
> > b/drivers/misc/cardreader/rts5227.c
> > > index 4feed296a327..423fecc19fc4 100644
> > > --- a/drivers/misc/cardreader/rts5227.c
> > > +++ b/drivers/misc/cardreader/rts5227.c
> > > @@ -394,7 +394,7 @@ static const struct pcr_ops rts522a_pcr_ops = {
> > >  void rts522a_init_params(struct rtsx_pcr *pcr)
> > >  {
> > >         rts5227_init_params(pcr);
> > > -
> > > +       pcr->tx_initial_phase = SET_CLOCK_PHASE(20, 20, 11);
> > >         pcr->reg_pm_ctrl3 = RTS522A_PM_CTRL3;
> > >
> > >         pcr->option.ocp_en = 1;
> > > diff --git a/drivers/misc/cardreader/rts5249.c
> > b/drivers/misc/cardreader/rts5249.c
> > > index db936e4d6e56..1a81cda948c1 100644
> > > --- a/drivers/misc/cardreader/rts5249.c
> > > +++ b/drivers/misc/cardreader/rts5249.c
> > > @@ -618,6 +618,7 @@ static const struct pcr_ops rts524a_pcr_ops = {
> > >  void rts524a_init_params(struct rtsx_pcr *pcr)
> > >  {
> > >         rts5249_init_params(pcr);
> > > +       pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 29, 11);
> > >         pcr->option.ltr_l1off_sspwrgate =
> > LTR_L1OFF_SSPWRGATE_5250_DEF;
> > >         pcr->option.ltr_l1off_snooze_sspwrgate =
> > >                 LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEF;
> > > @@ -733,6 +734,7 @@ static const struct pcr_ops rts525a_pcr_ops = {
> > >  void rts525a_init_params(struct rtsx_pcr *pcr)
> > >  {
> > >         rts5249_init_params(pcr);
> > > +       pcr->tx_initial_phase = SET_CLOCK_PHASE(25, 29, 11);
> > >         pcr->option.ltr_l1off_sspwrgate =
> > LTR_L1OFF_SSPWRGATE_5250_DEF;
> > >         pcr->option.ltr_l1off_snooze_sspwrgate =
> > >                 LTR_L1OFF_SNOOZE_SSPWRGATE_5250_DEF;
> > > diff --git a/drivers/misc/cardreader/rts5260.c
> > b/drivers/misc/cardreader/rts5260.c
> > > index 4214f02a17fd..711054ebad74 100644
> > > --- a/drivers/misc/cardreader/rts5260.c
> > > +++ b/drivers/misc/cardreader/rts5260.c
> > > @@ -662,7 +662,7 @@ void rts5260_init_params(struct rtsx_pcr *pcr)
> > >         pcr->sd30_drive_sel_1v8 = CFG_DRIVER_TYPE_B;
> > >         pcr->sd30_drive_sel_3v3 = CFG_DRIVER_TYPE_B;
> > >         pcr->aspm_en = ASPM_L1_EN;
> > > -       pcr->tx_initial_phase = SET_CLOCK_PHASE(1, 29, 16);
> > > +       pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 29, 11);
> > >         pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
> > >
> > >         pcr->ic_version = rts5260_get_ic_version(pcr);
> > > diff --git a/drivers/misc/cardreader/rts5261.c
> > b/drivers/misc/cardreader/rts5261.c
> > > index bc4967a6efa1..78c3b1d424c3 100644
> > > --- a/drivers/misc/cardreader/rts5261.c
> > > +++ b/drivers/misc/cardreader/rts5261.c
> > > @@ -764,7 +764,7 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
> > >         pcr->sd30_drive_sel_1v8 = CFG_DRIVER_TYPE_B;
> > >         pcr->sd30_drive_sel_3v3 = CFG_DRIVER_TYPE_B;
> > >         pcr->aspm_en = ASPM_L1_EN;
> > > -       pcr->tx_initial_phase = SET_CLOCK_PHASE(20, 27, 16);
> > > +       pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 27, 11);
> > >         pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
> > >
> > >         pcr->ic_version = rts5261_get_ic_version(pcr);
> > > diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c
> > b/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > index bd50935dc37d..11087976ab19 100644
> > > --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> > > @@ -606,19 +606,22 @@ static int sd_change_phase(struct
> > realtek_pci_sdmmc *host,
> > >                 u8 sample_point, bool rx)
> > >  {
> > >         struct rtsx_pcr *pcr = host->pcr;
> > > -
> > > +       u16 SD_VP_CTL = 0;
> > >         dev_dbg(sdmmc_dev(host), "%s(%s): sample_point = %d\n",
> > >                         __func__, rx ? "RX" : "TX", sample_point);
> > >
> > >         rtsx_pci_write_register(pcr, CLK_CTL, CHANGE_CLK, CHANGE_CLK);
> > > -       if (rx)
> > > +       if (rx) {
> > > +               SD_VP_CTL = SD_VPRX_CTL;
> > >                 rtsx_pci_write_register(pcr, SD_VPRX_CTL,
> > >                         PHASE_SELECT_MASK, sample_point);
> > > -       else
> > > +       } else {
> > > +               SD_VP_CTL = SD_VPTX_CTL;
> > >                 rtsx_pci_write_register(pcr, SD_VPTX_CTL,
> > >                         PHASE_SELECT_MASK, sample_point);
> > > -       rtsx_pci_write_register(pcr, SD_VPCLK0_CTL, PHASE_NOT_RESET, 0);
> > > -       rtsx_pci_write_register(pcr, SD_VPCLK0_CTL, PHASE_NOT_RESET,
> > > +       }
> > > +       rtsx_pci_write_register(pcr, SD_VP_CTL, PHASE_NOT_RESET, 0);
> > > +       rtsx_pci_write_register(pcr, SD_VP_CTL, PHASE_NOT_RESET,
> > >                                 PHASE_NOT_RESET);
> > >         rtsx_pci_write_register(pcr, CLK_CTL, CHANGE_CLK, 0);
> > >         rtsx_pci_write_register(pcr, SD_CFG1, SD_ASYNC_FIFO_NOT_RST,
> > 0);
> > > --
> > > 2.17.1
> > >
> >
> > ------Please consider the environment before printing this e-mail.