Skip to contentSkip to conversation

mmc: dw_mmc: miscaculated the fifo-depth with wrong bit operation

PATCH Jaehoon Chung 3 participants 5 messages Jan 9, 2012 – Jan 11, 2012

Subsystems (2)
lore
Share & tools
lore

Jaehoon Chung

author maintainer
#0 lore raw

In FIFOTH register, can find "bit[27:16] = FIFO_DEPTH - 1". Finally, FIFO_DEPTH = bit[27:16] + 1.

Now, Used the 0x7ff. but 0xfff is right.

---
 drivers/mmc/host/dw_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/dw_mmc.c +1 -1
index 94e22382..0e34279 100644
@@ -1949,7 +1949,7 @@ static int dw_mci_probe(struct platform_device *pdev)
 		 * should put it in the platform data.
 		 */
 		fifo_size = mci_readl(host, FIFOTH);
-		fifo_size = 1 + ((fifo_size >> 16) & 0x7ff);
+		fifo_size = 1 + ((fifo_size >> 16) & 0xfff);
 	} else {
 		fifo_size = host->pdata->fifo_depth;
 	}
#1 lore raw
Quoted 4 lines · Jaehoon Chung

In FIFOTH register, can find "bit[27:16] = FIFO_DEPTH - 1". Finally, FIFO_DEPTH = bit[27:16] + 1.

Now, Used the 0x7ff. but 0xfff is right.

Nice catch. The patch itself looks okay, but I don't think the commit message is very understandable, maybe something like this would be better?:

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

Cheers
James

Reviewed-by
#2 lore raw
Quoted 9 lines · James Hogan
Quoted 4 lines · Jaehoon Chung

In FIFOTH register, can find "bit[27:16] = FIFO_DEPTH - 1". Finally, FIFO_DEPTH = bit[27:16] + 1.

Now, Used the 0x7ff. but 0xfff is right.

Nice catch. The patch itself looks okay, but I don't think the commit message is very understandable, maybe something like this would be better?:

Quoted 6 lines

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

Cheers
James

Are you happy having your signed-off-by against this identical patch with a modified message Jaehoon?

From: Jaehoon Chung <[email protected]>

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

---
 drivers/mmc/host/dw_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/dw_mmc.c +1 -1
index cc54808..6ea6185 100644
@@ -2075,7 +2075,7 @@ static int dw_mci_probe(struct platform_device *pdev)
 		 * should put it in the platform data.
 		 */
 		fifo_size = mci_readl(host, FIFOTH);
-		fifo_size = 1 + ((fifo_size >> 16) & 0x7ff);
+		fifo_size = 1 + ((fifo_size >> 16) & 0xfff);
 	} else {
 		fifo_size = host->pdata->fifo_depth;
 	}
Signature · 1 line
1.7.2.3
author maintainer
#3 lore raw

Hi James,

Thank you..:)

Best Regards,
Jaehoon Chung

Quoted 45 lines · James Hogan
Quoted 9 lines · James Hogan
Quoted 4 lines · Jaehoon Chung

In FIFOTH register, can find "bit[27:16] = FIFO_DEPTH - 1". Finally, FIFO_DEPTH = bit[27:16] + 1.

Now, Used the 0x7ff. but 0xfff is right.

Nice catch. The patch itself looks okay, but I don't think the commit message is very understandable, maybe something like this would be better?:

Quoted 6 lines

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

Cheers
James

Are you happy having your signed-off-by against this identical patch with a modified message Jaehoon?

From: Jaehoon Chung <[email protected]>

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

---
 drivers/mmc/host/dw_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/dw_mmc.c +1 -1
index cc54808..6ea6185 100644
@@ -2075,7 +2075,7 @@ static int dw_mci_probe(struct platform_device *pdev)
 		 * should put it in the platform data.
 		 */
 		fifo_size = mci_readl(host, FIFOTH);
-		fifo_size = 1 + ((fifo_size >> 16) & 0x7ff);
+		fifo_size = 1 + ((fifo_size >> 16) & 0xfff);
 	} else {
 		fifo_size = host->pdata->fifo_depth;
 	}
#4 lore raw

Hi,

Quoted 37 lines · Jaehoon Chung

Hi James,

Thank you..:)

Best Regards,
Jaehoon Chung

Quoted 28 lines · James Hogan
Quoted 9 lines · James Hogan
Quoted 4 lines · Jaehoon Chung

In FIFOTH register, can find "bit[27:16] = FIFO_DEPTH - 1". Finally, FIFO_DEPTH = bit[27:16] + 1.

Now, Used the 0x7ff. but 0xfff is right.

Nice catch. The patch itself looks okay, but I don't think the commit message is very understandable, maybe something like this would be better?:

Quoted 6 lines

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

Cheers
James

Are you happy having your signed-off-by against this identical patch with a modified message Jaehoon?

From: Jaehoon Chung <[email protected]>

In FIFOTH register, the RX_WMark field (bits[27:16]) defaults to FIFO_DEPTH - 1. When reading it, bits[26:16] were being used, so fix it to use the mask 0xfff instead of 0x7ff.

Thanks, merged to mmc-next for 3.3.

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child