linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG
@ 2021-06-04 14:42 ricky_wu
  2021-06-04 14:48 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: ricky_wu @ 2021-06-04 14:42 UTC (permalink / raw)
  To: arnd, gregkh, bhelgaas, rui_feng, ulf.hansson, vaibhavgupta40,
	yang.lee, keitasuzuki.park, chris.chiu, linux-kernel
  Cc: Ricky Wu

From: Ricky Wu <ricky_wu@realtek.com>

Divide Realtek Card Reader Group into two different modes
ASPM_MODE_CFG: 8411 5209 5227 5229 5249 5250
Change back to use original way to control aspm
ASPM_MODE_REG: 5227A 524A 5250A 5260 5261 5228
Keep the new way to control aspm

Signed-off-by: Ricky Wu <ricky_wu@realtek.com>
---
 drivers/misc/cardreader/rtl8411.c  |  1 +
 drivers/misc/cardreader/rts5209.c  |  1 +
 drivers/misc/cardreader/rts5227.c  |  2 ++
 drivers/misc/cardreader/rts5228.c  |  1 +
 drivers/misc/cardreader/rts5229.c  |  1 +
 drivers/misc/cardreader/rts5249.c  |  3 ++
 drivers/misc/cardreader/rts5260.c  |  1 +
 drivers/misc/cardreader/rts5261.c  |  1 +
 drivers/misc/cardreader/rtsx_pcr.c | 44 +++++++++++++++++++++---------
 include/linux/rtsx_pci.h           |  3 ++
 10 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/cardreader/rtl8411.c b/drivers/misc/cardreader/rtl8411.c
index a07674ed0596..4c5621b17a6f 100644
--- a/drivers/misc/cardreader/rtl8411.c
+++ b/drivers/misc/cardreader/rtl8411.c
@@ -468,6 +468,7 @@ static void rtl8411_init_common_params(struct rtsx_pcr *pcr)
 	pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
 	pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
 	pcr->aspm_en = ASPM_L1_EN;
+	pcr->aspm_mode = ASPM_MODE_CFG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(23, 7, 14);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
 	pcr->ic_version = rtl8411_get_ic_version(pcr);
diff --git a/drivers/misc/cardreader/rts5209.c b/drivers/misc/cardreader/rts5209.c
index 39a6a7ecc32e..29f5414072bf 100644
--- a/drivers/misc/cardreader/rts5209.c
+++ b/drivers/misc/cardreader/rts5209.c
@@ -255,6 +255,7 @@ void rts5209_init_params(struct rtsx_pcr *pcr)
 	pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
 	pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
 	pcr->aspm_en = ASPM_L1_EN;
+	pcr->aspm_mode = ASPM_MODE_CFG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 27, 16);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
 
diff --git a/drivers/misc/cardreader/rts5227.c b/drivers/misc/cardreader/rts5227.c
index 8200af22b529..4bcfbc9afbac 100644
--- a/drivers/misc/cardreader/rts5227.c
+++ b/drivers/misc/cardreader/rts5227.c
@@ -358,6 +358,7 @@ void rts5227_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->aspm_mode = ASPM_MODE_CFG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 27, 15);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(30, 7, 7);
 
@@ -483,6 +484,7 @@ void rts522a_init_params(struct rtsx_pcr *pcr)
 
 	rts5227_init_params(pcr);
 	pcr->ops = &rts522a_pcr_ops;
+	pcr->aspm_mode = ASPM_MODE_REG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(20, 20, 11);
 	pcr->reg_pm_ctrl3 = RTS522A_PM_CTRL3;
 
diff --git a/drivers/misc/cardreader/rts5228.c b/drivers/misc/cardreader/rts5228.c
index 781a86def59a..ffc128278613 100644
--- a/drivers/misc/cardreader/rts5228.c
+++ b/drivers/misc/cardreader/rts5228.c
@@ -718,6 +718,7 @@ void rts5228_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->aspm_mode = ASPM_MODE_REG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(28, 27, 11);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
 
diff --git a/drivers/misc/cardreader/rts5229.c b/drivers/misc/cardreader/rts5229.c
index 89e6f124ca5c..c748eaf1ec1f 100644
--- a/drivers/misc/cardreader/rts5229.c
+++ b/drivers/misc/cardreader/rts5229.c
@@ -246,6 +246,7 @@ void rts5229_init_params(struct rtsx_pcr *pcr)
 	pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
 	pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
 	pcr->aspm_en = ASPM_L1_EN;
+	pcr->aspm_mode = ASPM_MODE_CFG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 27, 15);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(30, 6, 6);
 
diff --git a/drivers/misc/cardreader/rts5249.c b/drivers/misc/cardreader/rts5249.c
index b2676e7f5027..53f3a1f45c4a 100644
--- a/drivers/misc/cardreader/rts5249.c
+++ b/drivers/misc/cardreader/rts5249.c
@@ -566,6 +566,7 @@ void rts5249_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->aspm_mode = ASPM_MODE_CFG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(1, 29, 16);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
 
@@ -729,6 +730,7 @@ static const struct pcr_ops rts524a_pcr_ops = {
 void rts524a_init_params(struct rtsx_pcr *pcr)
 {
 	rts5249_init_params(pcr);
+	pcr->aspm_mode = ASPM_MODE_REG;
 	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 =
@@ -845,6 +847,7 @@ static const struct pcr_ops rts525a_pcr_ops = {
 void rts525a_init_params(struct rtsx_pcr *pcr)
 {
 	rts5249_init_params(pcr);
+	pcr->aspm_mode = ASPM_MODE_REG;
 	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 =
diff --git a/drivers/misc/cardreader/rts5260.c b/drivers/misc/cardreader/rts5260.c
index 080a7d67a8e1..9b42b20a3e5a 100644
--- a/drivers/misc/cardreader/rts5260.c
+++ b/drivers/misc/cardreader/rts5260.c
@@ -628,6 +628,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->aspm_mode = ASPM_MODE_REG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 29, 11);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
 
diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
index 6c64dade8e1a..1fd4e0e50730 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -783,6 +783,7 @@ void rts5261_init_params(struct rtsx_pcr *pcr)
 	pcr->sd30_drive_sel_1v8 = 0x00;
 	pcr->sd30_drive_sel_3v3 = 0x00;
 	pcr->aspm_en = ASPM_L1_EN;
+	pcr->aspm_mode = ASPM_MODE_REG;
 	pcr->tx_initial_phase = SET_CLOCK_PHASE(27, 27, 11);
 	pcr->rx_initial_phase = SET_CLOCK_PHASE(24, 6, 5);
 
diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index 273311184669..baf83594a01d 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -85,12 +85,18 @@ static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable)
 	if (pcr->aspm_enabled == enable)
 		return;
 
-	if (pcr->aspm_en & 0x02)
-		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, FORCE_ASPM_CTL0 |
-			FORCE_ASPM_CTL1, enable ? 0 : FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1);
-	else
-		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, FORCE_ASPM_CTL0 |
-			FORCE_ASPM_CTL1, FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1);
+	if (pcr->aspm_mode == ASPM_MODE_CFG) {
+		pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
+						PCI_EXP_LNKCTL_ASPMC,
+						enable ? pcr->aspm_en : 0);
+	} else if (pcr->aspm_mode == ASPM_MODE_REG) {
+		if (pcr->aspm_en & 0x02)
+			rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, FORCE_ASPM_CTL0 |
+				FORCE_ASPM_CTL1, enable ? 0 : FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1);
+		else
+			rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, FORCE_ASPM_CTL0 |
+				FORCE_ASPM_CTL1, FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1);
+	}
 
 	if (!enable && (pcr->aspm_en & 0x02))
 		mdelay(10);
@@ -1394,7 +1400,8 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
 			return err;
 	}
 
-	rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0x30, 0x30);
+	if (pcr->aspm_mode == ASPM_MODE_REG)
+		rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0x30, 0x30);
 
 	/* No CD interrupt if probing driver with card inserted.
 	 * So we need to initialize pcr->card_exist here.
@@ -1410,6 +1417,8 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
 static int rtsx_pci_init_chip(struct rtsx_pcr *pcr)
 {
 	int err;
+	u16 cfg_val;
+	u8 val;
 
 	spin_lock_init(&pcr->lock);
 	mutex_init(&pcr->pcr_mutex);
@@ -1477,6 +1486,21 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr)
 	if (!pcr->slots)
 		return -ENOMEM;
 
+	if (pcr->aspm_mode == ASPM_MODE_CFG) {
+		pcie_capability_read_word(pcr->pci, PCI_EXP_LNKCTL, &cfg_val);
+		if (cfg_val & PCI_EXP_LNKCTL_ASPM_L1)
+			pcr->aspm_enabled = true;
+		else
+			pcr->aspm_enabled = false;
+
+	} else if (pcr->aspm_mode == ASPM_MODE_REG) {
+		rtsx_pci_read_register(pcr, ASPM_FORCE_CTL, &val);
+		if (val & FORCE_ASPM_CTL0 && val & FORCE_ASPM_CTL1)
+			pcr->aspm_enabled = false;
+		else
+			pcr->aspm_enabled = true;
+	}
+
 	if (pcr->ops->fetch_vendor_settings)
 		pcr->ops->fetch_vendor_settings(pcr);
 
@@ -1506,7 +1530,6 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
 	struct pcr_handle *handle;
 	u32 base, len;
 	int ret, i, bar = 0;
-	u8 val;
 
 	dev_dbg(&(pcidev->dev),
 		": Realtek PCI-E Card Reader found at %s [%04x:%04x] (rev %x)\n",
@@ -1572,11 +1595,6 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
 	pcr->host_cmds_addr = pcr->rtsx_resv_buf_addr;
 	pcr->host_sg_tbl_ptr = pcr->rtsx_resv_buf + HOST_CMDS_BUF_LEN;
 	pcr->host_sg_tbl_addr = pcr->rtsx_resv_buf_addr + HOST_CMDS_BUF_LEN;
-	rtsx_pci_read_register(pcr, ASPM_FORCE_CTL, &val);
-	if (val & FORCE_ASPM_CTL0 && val & FORCE_ASPM_CTL1)
-		pcr->aspm_enabled = false;
-	else
-		pcr->aspm_enabled = true;
 	pcr->card_inserted = 0;
 	pcr->card_removed = 0;
 	INIT_DELAYED_WORK(&pcr->carddet_work, rtsx_pci_card_detect);
diff --git a/include/linux/rtsx_pci.h b/include/linux/rtsx_pci.h
index 6f155f99aa16..6c16837ee235 100644
--- a/include/linux/rtsx_pci.h
+++ b/include/linux/rtsx_pci.h
@@ -1234,6 +1234,9 @@ struct rtsx_pcr {
 	u8				card_drive_sel;
 #define ASPM_L1_EN			0x02
 	u8				aspm_en;
+#define ASPM_MODE_CFG		0x01
+#define ASPM_MODE_REG		0x02
+	u8				aspm_mode;
 	bool				aspm_enabled;
 
 #define PCR_MS_PMOS			(1 << 0)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG
  2021-06-04 14:42 [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG ricky_wu
@ 2021-06-04 14:48 ` Greg KH
  2021-06-07  2:34   ` Ricky WU
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2021-06-04 14:48 UTC (permalink / raw)
  To: ricky_wu
  Cc: arnd, bhelgaas, rui_feng, ulf.hansson, vaibhavgupta40, yang.lee,
	keitasuzuki.park, chris.chiu, linux-kernel

On Fri, Jun 04, 2021 at 10:42:11PM +0800, ricky_wu@realtek.com wrote:
> From: Ricky Wu <ricky_wu@realtek.com>
> 
> Divide Realtek Card Reader Group into two different modes
> ASPM_MODE_CFG: 8411 5209 5227 5229 5249 5250
> Change back to use original way to control aspm

What is "aspm"?

> +#define ASPM_MODE_CFG		0x01
> +#define ASPM_MODE_REG		0x02
> +	u8				aspm_mode;

Why not use an enum?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG
  2021-06-04 14:48 ` Greg KH
@ 2021-06-07  2:34   ` Ricky WU
  2021-06-07  4:48     ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Ricky WU @ 2021-06-07  2:34 UTC (permalink / raw)
  To: Greg KH
  Cc: arnd, bhelgaas, rui_feng, ulf.hansson, vaibhavgupta40, yang.lee,
	keitasuzuki.park, chris.chiu, linux-kernel

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, June 4, 2021 10:48 PM
> To: Ricky WU
> Cc: arnd@arndb.de; bhelgaas@google.com; rui_feng@realsil.com.cn;
> ulf.hansson@linaro.org; vaibhavgupta40@gmail.com;
> yang.lee@linux.alibaba.com; keitasuzuki.park@sslab.ics.keio.ac.jp;
> chris.chiu@canonical.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] misc: rtsx: separate aspm mode into MODE_REG and
> MODE_CFG
> 
> On Fri, Jun 04, 2021 at 10:42:11PM +0800, ricky_wu@realtek.com wrote:
> > From: Ricky Wu <ricky_wu@realtek.com>
> >
> > Divide Realtek Card Reader Group into two different modes
> > ASPM_MODE_CFG: 8411 5209 5227 5229 5249 5250 Change back to use
> > original way to control aspm
> 
> What is "aspm"?
> 
Active State Power Management (ASPM)
rtsx_comm_set_aspm: ASPM control function for our driver
to make sure not enter power saving in process of init and card_detcct 

> > +#define ASPM_MODE_CFG		0x01
> > +#define ASPM_MODE_REG		0x02
> > +	u8				aspm_mode;
> 
> Why not use an enum?
> 

It is ok, we can have a V2 to use enum

> thanks,
> 
> greg k-h
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG
  2021-06-07  2:34   ` Ricky WU
@ 2021-06-07  4:48     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2021-06-07  4:48 UTC (permalink / raw)
  To: Ricky WU
  Cc: arnd, bhelgaas, rui_feng, ulf.hansson, vaibhavgupta40, yang.lee,
	keitasuzuki.park, chris.chiu, linux-kernel

On Mon, Jun 07, 2021 at 02:34:36AM +0000, Ricky WU wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Friday, June 4, 2021 10:48 PM
> > To: Ricky WU
> > Cc: arnd@arndb.de; bhelgaas@google.com; rui_feng@realsil.com.cn;
> > ulf.hansson@linaro.org; vaibhavgupta40@gmail.com;
> > yang.lee@linux.alibaba.com; keitasuzuki.park@sslab.ics.keio.ac.jp;
> > chris.chiu@canonical.com; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] misc: rtsx: separate aspm mode into MODE_REG and
> > MODE_CFG
> > 
> > On Fri, Jun 04, 2021 at 10:42:11PM +0800, ricky_wu@realtek.com wrote:
> > > From: Ricky Wu <ricky_wu@realtek.com>
> > >
> > > Divide Realtek Card Reader Group into two different modes
> > > ASPM_MODE_CFG: 8411 5209 5227 5229 5249 5250 Change back to use
> > > original way to control aspm
> > 
> > What is "aspm"?
> > 
> Active State Power Management (ASPM)

Great, please spell it out in the changelog text when you resubmit this.

> rtsx_comm_set_aspm: ASPM control function for our driver
> to make sure not enter power saving in process of init and card_detcct

Ok, can you please say all of this in the changelog text?

> > > +#define ASPM_MODE_CFG		0x01
> > > +#define ASPM_MODE_REG		0x02
> > > +	u8				aspm_mode;
> > 
> > Why not use an enum?
> > 
> 
> It is ok, we can have a V2 to use enum

Please do so.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-07  4:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 14:42 [PATCH] misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG ricky_wu
2021-06-04 14:48 ` Greg KH
2021-06-07  2:34   ` Ricky WU
2021-06-07  4:48     ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).