Skip to contentSkip to conversation

arm: mmci: Add ARM variant with extended FIFO

PATCH In review Pawel Moll 5 participants 8 messages Mar 11, 2011 – Mar 24, 2011

Subsystems (2)
lore
Share & tools
lore

Pawel Moll

author
#0 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

---
 drivers/mmc/host/mmci.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
drivers/mmc/host/mmci.c +12 -1
index 2d6de3e..e7197b0 100644
@@ -66,6 +66,12 @@ static struct variant_data variant_arm = {
 	.datalength_bits	= 16,
 };
 
+static struct variant_data variant_arm_extended_fifo = {
+	.fifosize		= 128 * 4,
+	.fifohalfsize		= 64 * 4,
+	.datalength_bits	= 16,
+};
+
 static struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
@@ -1019,10 +1025,15 @@ static int mmci_resume(struct amba_device *dev)
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
-		.mask	= 0x000fffff,
+		.mask	= 0xff0fffff,
 		.data	= &variant_arm,
 	},
 	{
+		.id	= 0x01041180,
+		.mask	= 0xff0fffff,
+		.data	= &variant_arm_extended_fifo,
+	},
+	{
 		.id	= 0x00041181,
 		.mask	= 0x000fffff,
 		.data	= &variant_arm,
Signature · 1 line
1.6.3.3
#1 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

Quoted 4 lines · Pawel Moll

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

This patch improves MMC performance on my vexpress system. Also fixes "mmcblk0: error -5 transferring data" errors.

Quoted 38 lines
---
 drivers/mmc/host/mmci.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
drivers/mmc/host/mmci.c +12 -1
index 2d6de3e..e7197b0 100644
@@ -66,6 +66,12 @@ static struct variant_data variant_arm = {
 	.datalength_bits	= 16,
 };
 
+static struct variant_data variant_arm_extended_fifo = {
+	.fifosize		= 128 * 4,
+	.fifohalfsize		= 64 * 4,
+	.datalength_bits	= 16,
+};
+
 static struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
@@ -1019,10 +1025,15 @@ static int mmci_resume(struct amba_device *dev)
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
-		.mask	= 0x000fffff,
+		.mask	= 0xff0fffff,
 		.data	= &variant_arm,
 	},
 	{
+		.id	= 0x01041180,
+		.mask	= 0xff0fffff,
+		.data	= &variant_arm_extended_fifo,
+	},
+	{
 		.id	= 0x00041181,
 		.mask	= 0x000fffff,
 		.data	= &variant_arm,
#2 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

Quoted 10 lines · Matt Waddel
Quoted 4 lines · Pawel Moll

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

This patch improves MMC performance on my vexpress system. Also fixes "mmcblk0: error -5 transferring data" errors.

You should use the patch I sent to do dynamic clocking if you're concerned about performance. That patch improves performance by a factor of 8 compared to anything possible by increasing the FIFO depth.

I am very much of the opinion that deepening the FIFOs is just a sticky plaster over the problem - and I said as much when it was first proposed, pointing out that there's other better solutions.

author
#3 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

Quoted 10 lines
Quoted 9 lines
Quoted 4 lines

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

This patch improves MMC performance on my vexpress system. Also fixes "mmcblk0: error -5 transferring data" errors.

Quoted 8 lines

You should use the patch I sent to do dynamic clocking if you're concerned about performance. That patch improves performance by a factor of 8 compared to anything possible by increasing the FIFO depth.

I am very much of the opinion that deepening the FIFOs is just a sticky plaster over the problem - and I said as much when it was first proposed, pointing out that there's other better solutions.

# cat /sys/devices/mb\:mmci/id                             
01041180
# cat /sys/module/mmci/parameters/fmax                     
12000000                                                                        
# dd if=/dev/mmcblk0 of=/dev/null bs=128k count=100        
100+0 records in                                                                
100+0 records out                                                               
13107200 bytes (13 MB) copied, 9.04218 s, 1.4 MB/s

Of course without the proper error handling, as introduced by your patch, one will face the same problems with interrupt latency as previously. But the peak performance seems to noticeably better...

Regards

Pawe?

PS. The FPGA with extended MMCI FIFO will be available on the next VE CD update, which is going to be released next month.

#4 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

In reply to Pawel Moll
Quoted 2 lines · Pawel Moll

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

OK that's the quick fix, and deeper FIFOs are always nice.

If your hardware engineers are listening, I suggest that hardware clock gating (as suggested in some earlier mail) plus DMA engine support will yet improve this by orders of magnitude and also bring you less errors.

Linus Walleij

author
#5 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

Quoted 2 lines

If your hardware engineers are listening, I suggest that hardware clock gating (as suggested in some earlier mail)

I've discussed this idea with our FPGA magician at the very beginning, but for some reason we wasn't very happy about this... I can't really blame him - it's not really his job to maintain individual components. And the original cell authors are long time... em, not dead probably ;-) just not available.

Quoted 2 lines

plus DMA engine support will
yet improve this by orders of magnitude and

Some of future test chips will have DMA330 (aka PL330), some won't. It's all about size, and as most of those chips are some kind of ASIC or semi-FPGA designs, the area is scarce...

Quoted 1 line

also bring you less errors.

One thing is not to generate the errors, second thing is to handle them correctly. And I must agree with Russell that error handling in the MMC stack is far from ideal.

Cheers!

Pawe?

author
#6 lore raw
In reply to Pawel Moll

Hi Chris,

Quoted 4 lines

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

How about this change? Matt Waddel tested it and Linus Walleij doesn't mind it ;-)

Cheers!

Paweł

Applied
#7 lore raw

PATCH 1/1 arm: mmci: Add ARM variant with extended FIFO

In reply to Pawel Moll

Hi Pawel,

Quoted 32 lines · Pawel Moll

New IO FPGA implementation for Versatile Express boards contain MMCI (PL180) cell with FIFO extended to 128 words (512 bytes).

---
 drivers/mmc/host/mmci.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
drivers/mmc/host/mmci.c +7 -1
index 2d6de3e..e7197b0 100644
@@ -66,6 +66,12 @@ static struct variant_data variant_arm = {
 	.datalength_bits	= 16,
 };
 
+static struct variant_data variant_arm_extended_fifo = {
+	.fifosize		= 128 * 4,
+	.fifohalfsize		= 64 * 4,
+	.datalength_bits	= 16,
+};
+
 static struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
@@ -1019,10 +1025,15 @@ static int mmci_resume(struct amba_device *dev)
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
-		.mask	= 0x000fffff,
+		.mask	= 0xff0fffff,
 		.data	= &variant_arm,

drivers/mmc/host/mmci.c static int mmci_resume(struct amba_device *dev) · line 1039

Quoted 10 lines
drivers/mmc/host/mmci.c +5 -0
 	},
 	{
+		.id	= 0x01041180,
+		.mask	= 0xff0fffff,
+		.data	= &variant_arm_extended_fifo,
+	},
+	{
 		.id	= 0x00041181,
 		.mask	= 0x000fffff,
 		.data	= &variant_arm,

Pushed to mmc-next for .39-rc2, thanks.

- Chris.

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