Skip to contentSkip to conversation

MFD:rtsx: Fix coding style and optimize flow

PATCH In review [email protected] 2 participants 8 messages Jan 29, 2013 – Feb 3, 2013 Multifunction devices (MFD)

lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover MFD:rtsx: Fix coding style and optimize flow

From: Wei WANG <[email protected]>

This patchset fixes some coding style issues, and optimizes card detect flow to improve the stability when inserting card.

Wei WANG (6):
MFD:rtsx: Fix typo in comment
MFD:rtsx: Remove redundant code
MFD:rtsx: Declare that the DMA address limitation is 32bit explicitly MFD:rtsx: Fix checkpatch warning
MFD:rtsx: Use macro defines to replace some variables MFD:rtsx: Optimize card detect flow

 drivers/mfd/rtsx_pcr.c       |   63 +++++++++++++++++++++++-------------------
 drivers/mfd/rtsx_pcr.h       |    3 ++
 include/linux/mfd/rtsx_pci.h |    2 +-
 3 files changed, 38 insertions(+), 30 deletions(-)
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 1/6 MFD:rtsx: Fix typo in comment

From: Wei WANG <[email protected]>

Fix a misspelling word in comment

Signed-off-by Wei WANG <[email protected]>
---
 include/linux/mfd/rtsx_pci.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/mfd/rtsx_pci.h +1 -1
index 4b117a3..3f2bf26 100644
@@ -465,7 +465,7 @@
 #define	SD_RSP_TYPE_R6			0x01
 #define	SD_RSP_TYPE_R7			0x01
 
-/* SD_CONFIURE3 */
+/* SD_CONFIGURE3 */
 #define	SD_RSP_80CLK_TIMEOUT_EN		0x01
 
 /* Card Transfer Reset Register */
Signature · 1 line
1.7.9.5
author
#2 lore raw

PATCH 2/6 MFD:rtsx: Remove redundant code

In reply to [email protected]

From: Wei WANG <[email protected]>

In function rtsx_pci_add_sg_tbl, the statement "ptr++" is useless.

Signed-off-by Wei WANG <[email protected]>
Acked-by Borislav Petkov <[email protected]>
---
 drivers/mfd/rtsx_pcr.c |    1 -
 1 file changed, 1 deletion(-)
drivers/mfd/rtsx_pcr.c +0 -1
index 9fc5700..c021ce3 100644
@@ -325,7 +325,6 @@ static void rtsx_pci_add_sg_tbl(struct rtsx_pcr *pcr,
 	val = ((u64)addr << 32) | ((u64)len << 12) | option;
 
 	put_unaligned_le64(val, ptr);
-	ptr++;
 	pcr->sgi++;
 }
Signature · 1 line
1.7.9.5
author
#3 lore raw

PATCH 3/6 MFD:rtsx: Declare that the DMA address limitation is 32bit explicitly

In reply to [email protected]

From: Wei WANG <[email protected]>

Realtek PCIe card reader only supports 32bit DMA.
This declaration can improve the readability

Signed-off-by Wei WANG <[email protected]>
---
 drivers/mfd/rtsx_pcr.c |    4 ++++
 1 file changed, 4 insertions(+)
drivers/mfd/rtsx_pcr.c +4 -0
index c021ce3..b2cfd2e 100644
@@ -1029,6 +1029,10 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
 		pci_name(pcidev), (int)pcidev->vendor, (int)pcidev->device,
 		(int)pcidev->revision);
 
+	ret = pci_set_dma_mask(pcidev, DMA_BIT_MASK(32));
+	if (ret < 0)
+		return ret;
+
 	ret = pci_enable_device(pcidev);
 	if (ret)
 		return ret;
Signature · 1 line
1.7.9.5
author
#4 lore raw

PATCH 4/6 MFD:rtsx: Fix checkpatch warning

In reply to [email protected]

From: Wei WANG <[email protected]>

WARNING: Avoid CamelCase: <min_N>
+ u8 N, min_N, max_N, clk_divider;

WARNING: Avoid CamelCase: <max_N>
+ u8 N, min_N, max_N, clk_divider;

Signed-off-by Wei WANG <[email protected]>
---
 drivers/mfd/rtsx_pcr.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
drivers/mfd/rtsx_pcr.c +13 -13
index b2cfd2e..4897c39 100644
@@ -590,7 +590,7 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 		u8 ssc_depth, bool initial_mode, bool double_clk, bool vpclk)
 {
 	int err, clk;
-	u8 N, min_N, max_N, clk_divider;
+	u8 n, min_n, max_n, clk_divider;
 	u8 mcu_cnt, div, max_div;
 	u8 depth[] = {
 		[RTSX_SSC_DEPTH_4M] = SSC_DEPTH_4M,

@@ -615,8 +615,8 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 	card_clock /= 1000000;
 	dev_dbg(&(pcr->pci->dev), "Switch card clock to %dMHz\n", card_clock);
 
-	min_N = 80;
-	max_N = 208;
+	min_n = 80;
+	max_n = 208;
 	max_div = CLK_DIV_8;
 
 	clk = card_clock;

@@ -630,30 +630,30 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 		return 0;
 
 	if (pcr->ops->conv_clk_and_div_n)
-		N = (u8)pcr->ops->conv_clk_and_div_n(clk, CLK_TO_DIV_N);
+		n = (u8)pcr->ops->conv_clk_and_div_n(clk, CLK_TO_DIV_N);
 	else
-		N = (u8)(clk - 2);
-	if ((clk <= 2) || (N > max_N))
+		n = (u8)(clk - 2);
+	if ((clk <= 2) || (n > max_n))
 		return -EINVAL;
 
 	mcu_cnt = (u8)(125/clk + 3);
 	if (mcu_cnt > 15)
 		mcu_cnt = 15;
 
-	/* Make sure that the SSC clock div_n is equal or greater than min_N */
+	/* Make sure that the SSC clock div_n is equal or greater than min_n */
 	div = CLK_DIV_1;
-	while ((N < min_N) && (div < max_div)) {
+	while ((n < min_n) && (div < max_div)) {
 		if (pcr->ops->conv_clk_and_div_n) {
-			int dbl_clk = pcr->ops->conv_clk_and_div_n(N,
+			int dbl_clk = pcr->ops->conv_clk_and_div_n(n,
 					DIV_N_TO_CLK) * 2;
-			N = (u8)pcr->ops->conv_clk_and_div_n(dbl_clk,
+			n = (u8)pcr->ops->conv_clk_and_div_n(dbl_clk,
 					CLK_TO_DIV_N);
 		} else {
-			N = (N + 2) * 2 - 2;
+			n = (n + 2) * 2 - 2;
 		}
 		div++;
 	}
-	dev_dbg(&(pcr->pci->dev), "N = %d, div = %d\n", N, div);
+	dev_dbg(&(pcr->pci->dev), "n = %d, div = %d\n", n, div);
 
 	ssc_depth = depth[ssc_depth];
 	if (double_clk)

@@ -670,7 +670,7 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SSC_CTL1, SSC_RSTB, 0);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SSC_CTL2,
 			SSC_DEPTH_MASK, ssc_depth);
-	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SSC_DIV_N_0, 0xFF, N);
+	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SSC_DIV_N_0, 0xFF, n);
 	rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SSC_CTL1, SSC_RSTB, SSC_RSTB);
 	if (vpclk) {
 		rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, SD_VPCLK0_CTL,
Signature · 1 line
1.7.9.5
author
#5 lore raw

PATCH 5/6 MFD:rtsx: Use macro defines to replace some variables

In reply to [email protected]

From: Wei WANG <[email protected]>

In function rtsx_pci_switch_clock, some variables, such as min_n, max_n, and max_div, are not necessary. And those assigned values look very obscure for others. It's more proper to use macro definitions here to replace these variables.

Signed-off-by Wei WANG <[email protected]>
Acked-by Borislav Petkov <[email protected]>
---
 drivers/mfd/rtsx_pcr.c |   13 ++++---------
 drivers/mfd/rtsx_pcr.h |    3 +++
 2 files changed, 7 insertions(+), 9 deletions(-)
drivers/mfd/rtsx_pcr.c +4 -9
index 4897c39..67bb34e 100644
@@ -590,8 +590,7 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 		u8 ssc_depth, bool initial_mode, bool double_clk, bool vpclk)
 {
 	int err, clk;
-	u8 n, min_n, max_n, clk_divider;
-	u8 mcu_cnt, div, max_div;
+	u8 n, clk_divider, mcu_cnt, div;
 	u8 depth[] = {
 		[RTSX_SSC_DEPTH_4M] = SSC_DEPTH_4M,
 		[RTSX_SSC_DEPTH_2M] = SSC_DEPTH_2M,

@@ -615,10 +614,6 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 	card_clock /= 1000000;
 	dev_dbg(&(pcr->pci->dev), "Switch card clock to %dMHz\n", card_clock);
 
-	min_n = 80;
-	max_n = 208;
-	max_div = CLK_DIV_8;
-
 	clk = card_clock;
 	if (!initial_mode && double_clk)
 		clk = card_clock * 2;

@@ -633,16 +628,16 @@ int rtsx_pci_switch_clock(struct rtsx_pcr *pcr, unsigned int card_clock,
 		n = (u8)pcr->ops->conv_clk_and_div_n(clk, CLK_TO_DIV_N);
 	else
 		n = (u8)(clk - 2);
-	if ((clk <= 2) || (n > max_n))
+	if ((clk <= 2) || (n > MAX_DIV_N_PCR))
 		return -EINVAL;
 
 	mcu_cnt = (u8)(125/clk + 3);
 	if (mcu_cnt > 15)
 		mcu_cnt = 15;
 
-	/* Make sure that the SSC clock div_n is equal or greater than min_n */
+	/* Make sure that the SSC clock div_n is not less than MIN_DIV_N_PCR */
 	div = CLK_DIV_1;
-	while ((n < min_n) && (div < max_div)) {
+	while ((n < MIN_DIV_N_PCR) && (div < CLK_DIV_8)) {
 		if (pcr->ops->conv_clk_and_div_n) {
 			int dbl_clk = pcr->ops->conv_clk_and_div_n(n,
 					DIV_N_TO_CLK) * 2;
drivers/mfd/rtsx_pcr.h +3 -0
index 12462c1..33c210b 100644
@@ -25,6 +25,9 @@
 
 #include <linux/mfd/rtsx_pci.h>
 
+#define MIN_DIV_N_PCR		80
+#define MAX_DIV_N_PCR		208
+
 void rts5209_init_params(struct rtsx_pcr *pcr);
 void rts5229_init_params(struct rtsx_pcr *pcr);
 void rtl8411_init_params(struct rtsx_pcr *pcr);
Signature · 1 line
1.7.9.5
author
#6 lore raw

PATCH 6/6 MFD:rtsx: Optimize card detect flow

In reply to [email protected]

From: Wei WANG <[email protected]>

1. Schedule card detect work at the end of ISR
2. Callback function ops->cd_deglitch may delay for a period of time. It is not proper to call this callback when local irq disabled
3. Card detect flow can't be exectued in parallel with other card reader operations, so it's better to be protected by mutex

Signed-off-by Wei WANG <[email protected]>
---
 drivers/mfd/rtsx_pcr.c |   31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
drivers/mfd/rtsx_pcr.c +19 -12
index 67bb34e..9016932 100644
@@ -752,7 +752,7 @@ static void rtsx_pci_card_detect(struct work_struct *work)
 	struct delayed_work *dwork;
 	struct rtsx_pcr *pcr;
 	unsigned long flags;
-	unsigned int card_detect = 0;
+	unsigned int card_detect = 0, card_inserted, card_removed;
 	u32 irq_status;
 
 	dwork = to_delayed_work(work);

@@ -760,25 +760,32 @@ static void rtsx_pci_card_detect(struct work_struct *work)
 
 	dev_dbg(&(pcr->pci->dev), "--> %s\n", __func__);
 
+	mutex_lock(&pcr->pcr_mutex);
 	spin_lock_irqsave(&pcr->lock, flags);
 
 	irq_status = rtsx_pci_readl(pcr, RTSX_BIPR);
 	dev_dbg(&(pcr->pci->dev), "irq_status: 0x%08x\n", irq_status);
 
-	if (pcr->card_inserted || pcr->card_removed) {
+	irq_status &= CARD_EXIST;
+	card_inserted = pcr->card_inserted & irq_status;
+	card_removed = pcr->card_removed;
+	pcr->card_inserted = 0;
+	pcr->card_removed = 0;
+
+	spin_unlock_irqrestore(&pcr->lock, flags);
+
+	if (card_inserted || card_removed) {
 		dev_dbg(&(pcr->pci->dev),
 				"card_inserted: 0x%x, card_removed: 0x%x\n",
-				pcr->card_inserted, pcr->card_removed);
+				card_inserted, card_removed);
 
 		if (pcr->ops->cd_deglitch)
-			pcr->card_inserted = pcr->ops->cd_deglitch(pcr);
+			card_inserted = pcr->ops->cd_deglitch(pcr);
 
-		card_detect = pcr->card_inserted | pcr->card_removed;
-		pcr->card_inserted = 0;
-		pcr->card_removed = 0;
+		card_detect = card_inserted | card_removed;
 	}
 
-	spin_unlock_irqrestore(&pcr->lock, flags);
+	mutex_unlock(&pcr->pcr_mutex);
 
 	if ((card_detect & SD_EXIST) && pcr->slots[RTSX_SD_CARD].card_event)
 		pcr->slots[RTSX_SD_CARD].card_event(

@@ -830,10 +837,6 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
 		}
 	}
 
-	if (pcr->card_inserted || pcr->card_removed)
-		schedule_delayed_work(&pcr->carddet_work,
-				msecs_to_jiffies(200));
-
 	if (int_reg & (NEED_COMPLETE_INT | DELINK_INT)) {
 		if (int_reg & (TRANS_FAIL_INT | DELINK_INT)) {
 			pcr->trans_result = TRANS_RESULT_FAIL;

@@ -846,6 +849,10 @@ static irqreturn_t rtsx_pci_isr(int irq, void *dev_id)
 		}
 	}
 
+	if (pcr->card_inserted || pcr->card_removed)
+		schedule_delayed_work(&pcr->carddet_work,
+				msecs_to_jiffies(200));
+
 	spin_unlock(&pcr->lock);
 	return IRQ_HANDLED;
 }
Signature · 1 line
1.7.9.5
#7 lore raw
In reply to [email protected]

Hi Wei,

Quoted 13 lines · On Tue, Jan 29, 2013 at 03:21:31PM +0800, [email protected] wrote:

From: Wei WANG <[email protected]>

This patchset fixes some coding style issues, and optimizes card detect flow to improve the stability when inserting card.

Wei WANG (6):
MFD:rtsx: Fix typo in comment
MFD:rtsx: Remove redundant code
MFD:rtsx: Declare that the DMA address limitation is 32bit explicitly MFD:rtsx: Fix checkpatch warning
MFD:rtsx: Use macro defines to replace some variables MFD:rtsx: Optimize card detect flow

Quoted 4 lines
 drivers/mfd/rtsx_pcr.c       |   63 +++++++++++++++++++++++-------------------
 drivers/mfd/rtsx_pcr.h       |    3 ++
 include/linux/mfd/rtsx_pci.h |    2 +-
 3 files changed, 38 insertions(+), 30 deletions(-)

All 6 patches applied, thanks.

Cheers,
Samuel.

Signature · 2 lines
Intel Open Source Technology Centre
http://oss.intel.com/