Skip to contentSkip to conversation

mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Resubmission

PATCH In review Tony Olech 3 participants 6 messages Apr 19, 2011 – May 14, 2011 MMC, SD and SDIO

lore
Share & tools
lore

Tony Olech

author
#0 lore raw

PATCH 1/1 mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Resubmission

Add a driver for Elan Digital System's VUB300 chip which is a USB connected SDIO/SDmem/MMC host controller. A VUB300 chip enables a USB 2.0 or USB 1.1 connected host computer to use SDIO/SD/MMC cards without the need for a directly connected, for example via PCI, SDIO host controller.

---
This is the forth submission attempt.
There are no errors reported by scripts/checkpatch.pl This driver has been tested on
a) 32bit x86
b) 64bit x86
c) dual processor
d) PowerPC
---

MAINTAINERS +7 -0
@@ -6758,6 +6758,13 @@ L:	[email protected]
 S:	Maintained
 F:	drivers/hwmon/vt8231.c
 
+VUB300 USB to SDIO/SD/MMC bridge chip
+M:	Tony Olech <[email protected]>
+L:	[email protected]
+L:	[email protected]
+S:	Supported
+F:	drivers/mmc/host/vub300.c
+
 W1 DALLAS'S 1-WIRE BUS
 M:	Evgeniy Polyakov <[email protected]>
 S:	Maintained
drivers/mmc/host/Makefile +1 -0
@@ -41,6 +41,7 @@ obj-$(CONFIG_SDH_BFIN)		+= bfin_sdh.o
 obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
+obj-$(CONFIG_MMC_VUB300)	+= vub300.o
 obj-$(CONFIG_MMC_USHC)		+= ushc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
drivers/mmc/host/Kconfig +31 -0
@@ -535,6 +535,37 @@ config MMC_JZ4740
 	  If you have a board based on such a SoC and with a SD/MMC slot,
 	  say Y or M here.
 
+config MMC_VUB300
+	tristate "VUB300 USB to SDIO/SD/MMC Host Controller support"
+	depends on USB
+	help
+	  This selects support for Elan Digital Systems' VUB300 chip.
+
+	  The VUB300 is a USB-SDIO Host Controller Interface chip
+	  that enables the host computer to use SDIO/SD/MMC cards
+	  via a USB 2.0 or USB 1.1 host.
+
+	  The VUB300 chip will be found in both physically separate
+	  USB to SDIO/SD/MMC adapters and embedded on some motherboards.
+
+	  The VUB300 chip supports SD and MMC memory cards in addition
+	  to single and multifunction SDIO cards.
+
+	  Some SDIO cards will need a firmware file to be loaded and
+	  sent to VUB300 chip in order to achieve better data throughput.
+	  Download these "Offload Pseudocode" from Elan Digital Systems'
+	  web-site http://www.elandigitalsystems.com/support/downloads.php
+	  and put them in /lib/firmware. Note that without these additional
+	  firmware files the VUB300 chip will still function, but not at
+	  the best obtainable data rate.
+
+	  To compile this mmc host controller driver as a module,
+	  choose M here: the module will be called vub300.
+
+	  If you have a computer with an embedded VUB300 chip
+	  or if you intend connecting a USB adapter based on a
+	  VUB300 chip say Y or M here.
+
 config MMC_USHC
 	tristate "USB SD Host Controller (USHC) support"
 	depends on USB
drivers/mmc/host/vub300.c new file +2529 -0

2,529 lines not on this page · raw message

#1 lore raw

Hi Tony,

Quoted 10 lines

Add a driver for Elan Digital System's VUB300 chip which is a USB connected SDIO/SDmem/MMC host controller. A VUB300 chip enables a USB 2.0 or USB 1.1 connected host computer to use SDIO/SD/MMC cards without the need for a directly connected, for example via PCI, SDIO host controller.

---
This is the forth submission attempt.

Looks to me that the concerns have not been addressed since last submission. I had a few a while ago, but Arnd added to it and summed it up nicely:

	http://ns3.spinics.net/lists/linux-mmc/msg06693.html
Quoted 1 line

There are no errors reported by scripts/checkpatch.pl

Just to make sure: This only means that there are no obvious (= detectable by a script) flaws. That makes it ready for submission, not necessarily for inclusion. So, just resending will not be enough.

Quoted 5 lines

This driver has been tested on
a) 32bit x86
b) 64bit x86
c) dual processor
d) PowerPC

The test coverage is great, yet the driver needs to comply to the kernel coding rules. I'd suggest to look at Arnd's review and fix the issues.

Regards,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw

PATCH 1/1 mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Re-Resubmission

In reply to Tony Olech

Add a driver for Elan Digital System's VUB300 chip which is a USB connected SDIO/SDmem/MMC host controller. A VUB300 chip enables a USB 2.0 or USB 1.1 connected host computer to use SDIO/SD/MMC cards without the need for a directly connected, for example via PCI, SDIO host controller.

---
This is the fifth submission attempt.
There are no errors reported by scripts/checkpatch.pl This driver has been tested on
a) 32bit x86
b) 64bit x86
c) dual processor
d) PowerPC
---

MAINTAINERS +7 -0
@@ -6764,6 +6764,13 @@ L:	[email protected]
 S:	Maintained
 F:	drivers/hwmon/vt8231.c
 
+VUB300 USB to SDIO/SD/MMC bridge chip
+M:	Tony Olech <[email protected]>
+L:	[email protected]
+L:	[email protected]
+S:	Supported
+F:	drivers/mmc/host/vub300.c
+
 W1 DALLAS'S 1-WIRE BUS
 M:	Evgeniy Polyakov <[email protected]>
 S:	Maintained
drivers/mmc/host/Makefile +1 -0
@@ -41,6 +41,7 @@ obj-$(CONFIG_SDH_BFIN)		+= bfin_sdh.o
 obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
+obj-$(CONFIG_MMC_VUB300)	+= vub300.o
 obj-$(CONFIG_MMC_USHC)		+= ushc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
drivers/mmc/host/Kconfig +31 -0
@@ -535,6 +535,37 @@ config MMC_JZ4740
 	  If you have a board based on such a SoC and with a SD/MMC slot,
 	  say Y or M here.
 
+config MMC_VUB300
+	tristate "VUB300 USB to SDIO/SD/MMC Host Controller support"
+	depends on USB
+	help
+	  This selects support for Elan Digital Systems' VUB300 chip.
+
+	  The VUB300 is a USB-SDIO Host Controller Interface chip
+	  that enables the host computer to use SDIO/SD/MMC cards
+	  via a USB 2.0 or USB 1.1 host.
+
+	  The VUB300 chip will be found in both physically separate
+	  USB to SDIO/SD/MMC adapters and embedded on some motherboards.
+
+	  The VUB300 chip supports SD and MMC memory cards in addition
+	  to single and multifunction SDIO cards.
+
+	  Some SDIO cards will need a firmware file to be loaded and
+	  sent to VUB300 chip in order to achieve better data throughput.
+	  Download these "Offload Pseudocode" from Elan Digital Systems'
+	  web-site http://www.elandigitalsystems.com/support/downloads.php
+	  and put them in /lib/firmware. Note that without these additional
+	  firmware files the VUB300 chip will still function, but not at
+	  the best obtainable data rate.
+
+	  To compile this mmc host controller driver as a module,
+	  choose M here: the module will be called vub300.
+
+	  If you have a computer with an embedded VUB300 chip
+	  or if you intend connecting a USB adapter based on a
+	  VUB300 chip say Y or M here.
+
 config MMC_USHC
 	tristate "USB SD Host Controller (USHC) support"
 	depends on USB
drivers/mmc/host/vub300.c new file +2561 -0

2,561 lines not on this page · raw message

author
#3 lore raw

Re: [PATCH] mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Re-Resubmission

With reference to Arnd's summation of issues with the previous version of the patch which can be found at "http://ns3.spinics.net/lists/linux-mmc/msg06693.html" I have made nearly all of the changes requested.

I have not removed of the __packed keyword from those structures that map onto the adapter’s hardware, the reason for Arnd's request, as I understand it, is that there is a compiler issue with other architectures that result in slightly slower code. Since those structures exist for translation to/from the adapter the efficiency of inserting or extracting data can not possibly be an issue. I do not see how having the structures marked with the __packed keyword is anything but good programming practice since if clearly distinguishes those structures from ordinary working structures.

There are 2 "if then else if .." decision branches that I cannot replace with a single "switch case .." statement because there is not a single unique expression those value is tested against constant values.

I have not chopped the void function "send_command" into 2 as Arnd requested, as the function, in spite of being 291 lines long is logically very simple and any split would be wholly arbitrary and in my view just make it more opaque.

Tony Olech
Elan Digital Systems Limited

#4 lore raw

Re: [PATCH] mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Re-Resubmission

Hi,

Quoted 24 lines · Tony Olech

With reference to Arnd's summation of issues with the previous version of the patch which can be found at "http://ns3.spinics.net/lists/linux-mmc/msg06693.html" I have made nearly all of the changes requested.

I have not removed of the __packed keyword from those structures that map onto the adapter’s hardware, the reason for Arnd's request, as I understand it, is that there is a compiler issue with other architectures that result in slightly slower code. Since those structures exist for translation to/from the adapter the efficiency of inserting or extracting data can not possibly be an issue. I do not see how having the structures marked with the __packed keyword is anything but good programming practice since if clearly distinguishes those structures from ordinary working structures.

There are 2 "if then else if .." decision branches that I cannot replace with a single "switch case .." statement because there is not a single unique expression those value is tested against constant values.

Quoted 5 lines

I have not chopped the void function "send_command" into 2 as Arnd requested, as the function, in spite of being 291 lines long is logically very simple and any split would be wholly arbitrary and in my view just make it more opaque.

Since the remaining objections are minor style issues, I think it makes sense to merge this to mmc-next now and deal with further improvements as patches on top of this one (which could get rebased into the original patch later, or not). Would any reviewers like to provide their Reviewed-by before I do so?

Thanks!

- Chris.

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

Re: [PATCH] mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Re-Re-Re-Resubmission

Hi,

Quoted 5 lines · Chris Ball

Since the remaining objections are minor style issues, I think it makes sense to merge this to mmc-next now and deal with further improvements as patches on top of this one (which could get rebased into the original patch later, or not). Would any reviewers like to provide their Reviewed-by before I do so?

I've pushed the vub300 patch to mmc-next now for testing, and will continue accepting Reviewed-bys to add to it. I'm also going to make some small further style changes and send them to Tony to ACK, and then fold them into the original patch before sending it to mainline.

Thanks!

- Chris.

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