[PATCH V6 1/4] mmc: sdhci-esdhc: remove SDHCI_QUIRK_NO_CARD_NO_RESET from ESDHC_DEFAULT_QUIRKS

2011-03-17 Thread Richard Zhu
sdhci-esdhc-imx does not need SDHCI_QUIRK_NO_CARD_NO_RESET. Make it OF-specific.

Signed-off-by: Richard Zhu hong-xing@freescale.com
Tested-by: Wolfram Sang w.s...@pengutronix.de
---
 drivers/mmc/host/sdhci-esdhc.h|3 +--
 drivers/mmc/host/sdhci-of-esdhc.c |3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index c55aae8..c3b08f1 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -23,8 +23,7 @@
SDHCI_QUIRK_NONSTANDARD_CLOCK | \
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
SDHCI_QUIRK_PIO_NEEDS_DELAY | \
-   SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET | \
-   SDHCI_QUIRK_NO_CARD_NO_RESET)
+   SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
 
 #define ESDHC_SYSTEM_CONTROL   0x2c
 #define ESDHC_CLOCK_MASK   0xfff0
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c 
b/drivers/mmc/host/sdhci-of-esdhc.c
index 08161f6..ba40d6d 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -74,7 +74,8 @@ static unsigned int esdhc_of_get_min_clock(struct sdhci_host 
*host)
 
 struct sdhci_of_data sdhci_esdhc = {
/* card detection could be handled via GPIO */
-   .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION,
+   .quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_BROKEN_CARD_DETECTION
+   | SDHCI_QUIRK_NO_CARD_NO_RESET,
.ops = {
.read_l = sdhci_be32bs_readl,
.read_w = esdhc_readw,
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V6 2/4] mmc: add the abort CMDTYE bits definition

2011-03-17 Thread Richard Zhu
Add the abort CMDTYPE bits definition of command register (offset 0xE)

Signed-off-by: Richard Zhu hong-xing@freescale.com
---
 drivers/mmc/host/sdhci.h |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 6e0969e..25e8bde 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -45,6 +45,7 @@
 #define  SDHCI_CMD_CRC 0x08
 #define  SDHCI_CMD_INDEX   0x10
 #define  SDHCI_CMD_DATA0x20
+#define  SDHCI_CMD_ABORTCMD0xC0
 
 #define  SDHCI_CMD_RESP_NONE   0x00
 #define  SDHCI_CMD_RESP_LONG   0x01
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V6 3/4] mmc: sdhci-esdhc: make the writel/readl as the general APIs

2011-03-17 Thread Richard Zhu
Add one flag to indicate the GPIO CD/WP is enabled or not
on imx platforms, and reuse the writel/readl as the general
APIs for imx SOCs.

Signed-off-by: Richard Zhu hong-xing@freescale.com
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   43 ++--
 drivers/mmc/host/sdhci-pltfm.h |2 +-
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 3b52485..5768e06 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -16,6 +16,7 @@
 #include linux/err.h
 #include linux/clk.h
 #include linux/gpio.h
+#include linux/slab.h
 #include linux/mmc/host.h
 #include linux/mmc/sdhci-pltfm.h
 #include mach/hardware.h
@@ -24,6 +25,13 @@
 #include sdhci-pltfm.h
 #include sdhci-esdhc.h
 
+#define IMX_GPIO_CD_WP (1  0)
+
+struct pltfm_imx_data {
+   int flags;
+   u32 mod_val;
+};
+
 static inline void esdhc_clrset_le(struct sdhci_host *host, u32 mask, u32 val, 
int reg)
 {
void __iomem *base = host-ioaddr + (reg  ~0x3);
@@ -34,10 +42,15 @@ static inline void esdhc_clrset_le(struct sdhci_host *host, 
u32 mask, u32 val, i
 
 static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 {
+   struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+   struct pltfm_imx_data *imx_data =
+   (struct pltfm_imx_data *)pltfm_host-priv;
+
/* fake CARD_PRESENT flag on mx25/35 */
u32 val = readl(host-ioaddr + reg);
 
-   if (unlikely(reg == SDHCI_PRESENT_STATE)) {
+   if (unlikely((reg == SDHCI_PRESENT_STATE)
+(imx_data-flags  IMX_GPIO_CD_WP))) {
struct esdhc_platform_data *boarddata =
host-mmc-parent-platform_data;
 
@@ -55,7 +68,12 @@ static u32 esdhc_readl_le(struct sdhci_host *host, int reg)
 
 static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
 {
-   if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE))
+   struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+   struct pltfm_imx_data *imx_data =
+   (struct pltfm_imx_data *)pltfm_host-priv;
+
+   if (unlikely((reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)
+(imx_data-flags  IMX_GPIO_CD_WP)))
/*
 * these interrupts won't work with a custom card_detect gpio
 * (only applied to mx25/35)
@@ -76,6 +94,8 @@ static u16 esdhc_readw_le(struct sdhci_host *host, int reg)
 static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
 {
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+   struct pltfm_imx_data *imx_data =
+   (struct pltfm_imx_data *)pltfm_host-priv;
 
switch (reg) {
case SDHCI_TRANSFER_MODE:
@@ -83,10 +103,10 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 
val, int reg)
 * Postpone this write, we must do it together with a
 * command write that is down below.
 */
-   pltfm_host-scratchpad = val;
+   imx_data-mod_val = val;
return;
case SDHCI_COMMAND:
-   writel(val  16 | pltfm_host-scratchpad,
+   writel(val  16 | imx_data-mod_val,
host-ioaddr + SDHCI_TRANSFER_MODE);
return;
case SDHCI_BLOCK_SIZE:
@@ -146,7 +166,9 @@ static unsigned int esdhc_pltfm_get_ro(struct sdhci_host 
*host)
 }
 
 static struct sdhci_ops sdhci_esdhc_ops = {
+   .read_l = esdhc_readl_le,
.read_w = esdhc_readw_le,
+   .write_l = esdhc_writel_le,
.write_w = esdhc_writew_le,
.write_b = esdhc_writeb_le,
.set_clock = esdhc_set_clock,
@@ -168,6 +190,7 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct 
sdhci_pltfm_data *pd
struct esdhc_platform_data *boarddata = 
host-mmc-parent-platform_data;
struct clk *clk;
int err;
+   struct pltfm_imx_data *imx_data;
 
clk = clk_get(mmc_dev(host-mmc), NULL);
if (IS_ERR(clk)) {
@@ -177,7 +200,10 @@ static int esdhc_pltfm_init(struct sdhci_host *host, 
struct sdhci_pltfm_data *pd
clk_enable(clk);
pltfm_host-clk = clk;
 
-   if (cpu_is_mx35() || cpu_is_mx51())
+   imx_data = kzalloc(sizeof(struct pltfm_imx_data), GFP_KERNEL);
+   pltfm_host-priv = (void *)imx_data;
+
+   if (!cpu_is_mx25())
host-quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
if (cpu_is_mx25() || cpu_is_mx35()) {
@@ -214,8 +240,7 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct 
sdhci_pltfm_data *pd
goto no_card_detect_irq;
}
 
-   sdhci_esdhc_ops.write_l = esdhc_writel_le;
-   sdhci_esdhc_ops.read_l = esdhc_readl_le;
+   imx_data-flags |= 

[PATCH V6 4/4] mmc: sdhci-esdhc: enable esdhc on imx53

2011-03-17 Thread Richard Zhu
Fix the NO INT in the Multi-BLK IO in SD/MMC, and Multi-BLK
read in SDIO on imx53.

The CMDTYPE of the CMD register (offset 0xE) should be set to
11 when the STOP CMD12 is issued on imx53 to abort one
open ended multi-blk IO. Otherwise one the TC INT wouldn't
be generated.

In exact block transfer, the controller doesn't complete the
operations automatically as required at the end of the
transfer and remains on hold if the abort command is not sent on
imx53.
As a result, the TC flag is not asserted and SW  received timeout
exeception. set bit1 of Vendor Spec registor to fix it.

Signed-off-by: Richard Zhu hong-xing@freescale.com
Signed-off-by: Richard Zhao richard.z...@freescale.com
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   42 
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 5768e06..96d131e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -19,13 +19,31 @@
 #include linux/slab.h
 #include linux/mmc/host.h
 #include linux/mmc/sdhci-pltfm.h
+#include linux/mmc/mmc.h
+#include linux/mmc/sdio.h
 #include mach/hardware.h
 #include mach/esdhc.h
 #include sdhci.h
 #include sdhci-pltfm.h
 #include sdhci-esdhc.h
 
+/* VENDOR SPEC register */
+#define SDHCI_VENDOR_SPEC  0xC0
+#define  SDHCI_VENDOR_SPEC_SDIO_QUIRK  0x0002
+
 #define IMX_GPIO_CD_WP (1  0)
+/*
+ * The CMDTYPE of the CMD register (offset 0xE) should be set to
+ * 11 when the STOP CMD12 is issued on imx53 to abort one
+ * open ended multi-blk IO. Otherwise the TC INT wouldn't
+ * be generated.
+ * In exact block transfer, the controller doesn't complete the
+ * operations automatically as required at the end of the
+ * transfer and remains on hold if the abort command is not sent.
+ * As a result, the TC flag is not asserted and SW  received timeout
+ * exeception. Bit1 of Vendor Spec registor is used to fix it.
+ */
+#define IMX_MULTIBLK_NO_INT(1  1)
 
 struct pltfm_imx_data {
int flags;
@@ -80,6 +98,15 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 
val, int reg)
 */
val = ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
 
+   if (unlikely((imx_data-flags  IMX_MULTIBLK_NO_INT)
+(reg == SDHCI_INT_STATUS)
+(val  SDHCI_INT_DATA_END))) {
+   u32 v;
+   v = readl(host-ioaddr + SDHCI_VENDOR_SPEC);
+   v = ~SDHCI_VENDOR_SPEC_SDIO_QUIRK;
+   writel(v, host-ioaddr + SDHCI_VENDOR_SPEC);
+   }
+
writel(val, host-ioaddr + reg);
 }
 
@@ -103,9 +130,21 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 
val, int reg)
 * Postpone this write, we must do it together with a
 * command write that is down below.
 */
+   if ((host-cmd-opcode == SD_IO_RW_EXTENDED)
+(host-cmd-data-blocks  1)
+(host-cmd-data-flags  MMC_DATA_READ)
+(imx_data-flags  IMX_MULTIBLK_NO_INT)) {
+   u32 v;
+   v = readl(host-ioaddr + SDHCI_VENDOR_SPEC);
+   v |= SDHCI_VENDOR_SPEC_SDIO_QUIRK;
+   writel(v, host-ioaddr + SDHCI_VENDOR_SPEC);
+   }
imx_data-mod_val = val;
return;
case SDHCI_COMMAND:
+   if ((host-cmd-opcode == MMC_STOP_TRANSMISSION)
+(imx_data-flags  IMX_MULTIBLK_NO_INT))
+   val |= SDHCI_CMD_ABORTCMD;
writel(val  16 | imx_data-mod_val,
host-ioaddr + SDHCI_TRANSFER_MODE);
return;
@@ -213,6 +252,9 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct 
sdhci_pltfm_data *pd
sdhci_esdhc_ops.get_ro = esdhc_pltfm_get_ro;
}
 
+   if (!(cpu_is_mx25() || cpu_is_mx35() || cpu_is_mx51()))
+   imx_data-flags |= IMX_MULTIBLK_NO_INT;
+
if (boarddata) {
err = gpio_request_one(boarddata-wp_gpio, GPIOF_IN, 
ESDHC_WP);
if (err) {
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mmci: Add ARM variant with extended FIFO

2011-03-17 Thread Linus Walleij
On Fri, Mar 11, 2011 at 6:18 PM, Pawel Moll pawel.m...@arm.com wrote:

 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
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mmci: Add ARM variant with extended FIFO

2011-03-17 Thread Pawel Moll
 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.

 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...

  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ł



--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] dw_mmc: add quirks unreliable detect and capabilities

2011-03-17 Thread Jaehoon Chung
Hi Chris..

I want to know your opinion..

Regards,
Jaehoon Chung

Will Newton wrote:
 On Fri, Feb 25, 2011 at 2:08 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 This patch added quirks and capabilities in platdata.

 Some card didn't use the CDn pin. In that case, We assume card inserted,
 then the card initialized or not.
 And Some board need other capabilities. So added capabilities in board 
 platdata.

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/mmc/host/dw_mmc.c  |   10 --
  include/linux/mmc/dw_mmc.h |   10 +++---
  2 files changed, 15 insertions(+), 5 deletions(-)
 
 Acked-by: Will Newton will.new...@imgtec.com
 --
 To unsubscribe from this list: send the line unsubscribe linux-mmc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] dw_mmc: support mmc power control with regulator

2011-03-17 Thread Jaehoon Chung
Hi Chris..

I want to know this patch..

Regard,
Jaehoon Chung

Will Newton wrote:
 On Fri, Feb 25, 2011 at 2:08 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 This patch is applied the power control with regulator.

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/mmc/host/dw_mmc.c  |   25 +
  include/linux/mmc/dw_mmc.h |2 ++
  2 files changed, 27 insertions(+), 0 deletions(-)
 
 This looks ok from an mmc point of view. I'm not that familiar with
 the regulator API however, so it would be good to get a review from
 someone who is.
 
 Acked-by: Will Newton will.new...@imgtec.com
 --
 To unsubscribe from this list: send the line unsubscribe linux-mmc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] dw_mmc: fix suspend/resume operation

2011-03-17 Thread Jaehoon Chung
This patch is applied about re-init processing when suspend/resume.

When card is resuming, some register is reset.
If card is removable, maybe controller should be rescan for card.
But if assume card is non-removable, need to restore the old value at registers.

Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/mmc/host/dw_mmc.c  |   22 --
 include/linux/mmc/dw_mmc.h |1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 299c1d6..892bfe3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1633,8 +1633,9 @@ static int dw_mci_probe(struct platform_device *pdev)
 */
fifo_size = mci_readl(host, FIFOTH);
fifo_size = (fifo_size  16)  0x7ff;
-   mci_writel(host, FIFOTH, ((0x2  28) | ((fifo_size/2 - 1)  16) |
- ((fifo_size/2)  0)));
+   host-fifoth_val = ((0x2  28) | ((fifo_size/2 - 1)  16) |
+   ((fifo_size/2)  0));
+   mci_writel(host, FIFOTH, host-fifoth_val);
 
/* disable clock to CIU */
mci_writel(host, CLKENA, 0);
@@ -1766,6 +1767,23 @@ static int dw_mci_resume(struct platform_device *pdev)
int i, ret;
struct dw_mci *host = platform_get_drvdata(pdev);
 
+   if (host-dma_ops-init)
+   host-dma_ops-init(host);
+
+   if (!mci_wait_reset(pdev-dev, host)) {
+   ret = -ENODEV;
+   return ret;
+   }
+
+   /* Restore the old value at FIFOTH register */
+   mci_writel(host, FIFOTH, host-fifoth_val);
+
+   mci_writel(host, RINTSTS, 0x);
+   mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
+  SDMMC_INT_TXDR | SDMMC_INT_RXDR |
+  DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
+   mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
+
for (i = 0; i  host-num_slots; i++) {
struct dw_mci_slot *slot = host-slot[i];
if (!slot)
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 3f22c20..5b2b414 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -140,6 +140,7 @@ struct dw_mci {
u32 bus_hz;
u32 current_speed;
u32 num_slots;
+   u32 fifoth_val;
struct platform_device  *pdev;
struct dw_mci_board *pdata;
struct dw_mci_slot  *slot[MAX_MCI_SLOTS];
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dw_mmc: fix suspend/resume operation

2011-03-17 Thread Will Newton
On Thu, Mar 17, 2011 at 11:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote:
 This patch is applied about re-init processing when suspend/resume.

 When card is resuming, some register is reset.
 If card is removable, maybe controller should be rescan for card.
 But if assume card is non-removable, need to restore the old value at 
 registers.

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

Looks ok to me.

Acked-by: Will Newton will.new...@imgtec.com
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] MMC: use pci_dev-revision

2011-03-17 Thread Sergei Shtylyov
The SDHCI driver uses PCI_CLASS_REVISION instead of PCI_REVISION_ID, so it was
not converted by commit 44c10138fd4bbc4b6d6bff0873c24902f2a9da65 (PCI: Change
all drivers to use pci_device-revision). The newer VIA driver has the similar
code too. So convert both drivers to using the 'revision' field of 'struct
pci_dev' now...

Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com

---
The patch is against the recent Linus' tree.

 drivers/mmc/host/sdhci-pci.c |6 ++
 drivers/mmc/host/via-sdmmc.c |5 ++---
 2 files changed, 4 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/mmc/host/sdhci-pci.c
===
--- linux-2.6.orig/drivers/mmc/host/sdhci-pci.c
+++ linux-2.6/drivers/mmc/host/sdhci-pci.c
@@ -1012,16 +1012,14 @@ static int __devinit sdhci_pci_probe(str
struct sdhci_pci_chip *chip;
struct sdhci_pci_slot *slot;
 
-   u8 slots, rev, first_bar;
+   u8 slots, first_bar;
int ret, i;
 
BUG_ON(pdev == NULL);
BUG_ON(ent == NULL);
 
-   pci_read_config_byte(pdev, PCI_CLASS_REVISION, rev);
-
dev_info(pdev-dev, SDHCI controller found [%04x:%04x] (rev %x)\n,
-(int)pdev-vendor, (int)pdev-device, (int)rev);
+(int)pdev-vendor, (int)pdev-device, (int)pdev-revision);
 
ret = pci_read_config_byte(pdev, PCI_SLOT_INFO, slots);
if (ret)
Index: linux-2.6/drivers/mmc/host/via-sdmmc.c
===
--- linux-2.6.orig/drivers/mmc/host/via-sdmmc.c
+++ linux-2.6/drivers/mmc/host/via-sdmmc.c
@@ -1090,14 +1090,13 @@ static int __devinit via_sd_probe(struct
struct mmc_host *mmc;
struct via_crdr_mmc_host *sdhost;
u32 base, len;
-   u8 rev, gatt;
+   u8  gatt;
int ret;
 
-   pci_read_config_byte(pcidev, PCI_CLASS_REVISION, rev);
pr_info(DRV_NAME
: VIA SDMMC controller found at %s [%04x:%04x] (rev %x)\n,
pci_name(pcidev), (int)pcidev-vendor, (int)pcidev-device,
-   (int)rev);
+   (int)pcidev-revision);
 
ret = pci_enable_device(pcidev);
if (ret)
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] dw_mmc: support mmc power control with regulator

2011-03-17 Thread Chris Ball
Hi Jaehoon,

On Thu, Feb 24 2011, Jaehoon Chung wrote:
 This patch is applied the power control with regulator.

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/mmc/host/dw_mmc.c  |   25 +
  include/linux/mmc/dw_mmc.h |2 ++
  2 files changed, 27 insertions(+), 0 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 46e5a89..338fedc 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -32,6 +32,7 @@
  #include linux/mmc/mmc.h
  #include linux/mmc/dw_mmc.h
  #include linux/bitops.h
 +#include linux/regulator/consumer.h
  
  #include dw_mmc.h
  
 @@ -1438,6 +1439,13 @@ static int __init dw_mci_init_slot(struct dw_mci 
 *host, unsigned int id)
   }
  #endif /* CONFIG_MMC_DW_IDMAC */
  
 + host-vmmc = regulator_get(mmc_dev(mmc), vmmc);
 + if (IS_ERR(host-vmmc)) {
 + printk(KERN_INFO %s: no vmmc regulator found\n, 
 mmc_hostname(mmc));
 + host-vmmc = NULL;
 + } else
 + regulator_enable(host-vmmc);
 +
   if (dw_mci_get_cd(mmc))
   set_bit(DW_MMC_CARD_PRESENT, slot-flags);
   else

I think you want an #ifdef CONFIG_REGULATOR around this hunk.
Please try building the driver with CONFIG_REGULATOR=n.

The other hunks in the patch shouldn't need it, because they test
host-vcc before making any regulator calls.

-- 
Chris Ball   c...@laptop.org   http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] dw_mmc: add quirks unreliable detect and capabilities

2011-03-17 Thread Chris Ball
Hi,

On Thu, Feb 24 2011, Jaehoon Chung wrote:
 This patch added quirks and capabilities in platdata.

 Some card didn't use the CDn pin. In that case, We assume card inserted,
 then the card initialized or not.
 And Some board need other capabilities. So added capabilities in board 
 platdata.

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

Thanks, will merge for .39 with Will's ACK.

-- 
Chris Ball   c...@laptop.org   http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH resend] mmc: Added ioctl to let userspace apps send ACMDs

2011-03-17 Thread Ben Dooks
On Thu, Mar 17, 2011 at 11:28:55AM -0700, John Calixto wrote:
 Part 3 of the SD Specification (SD Card Association; www.sdcard.org) describes
 how to use the security function of an SD card using application specific
 commands in conjunction with CPRM algorithms and keys licensed from the 4C
 Entity (www.4centity.com).  This allows userspace applications to access this
 security feature.
 
 Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 SoC, TI
 OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC.
 
 Signed-off-by: John Calixto john.cali...@modsystems.com
 ---
 (I'm resending because I forgot to cc the maintainer - sorry!)
 
  drivers/mmc/card/block.c  |  149 
 +
  drivers/mmc/core/sd_ops.c |3 +-
  include/linux/mmc/core.h  |1 +
  include/linux/mmc/sd.h|   18 ++
  4 files changed, 170 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
 index bfc8a8a..62f742d 100644
 --- a/drivers/mmc/card/block.c
 +++ b/drivers/mmc/card/block.c
 @@ -39,6 +39,7 @@
 
  #include asm/system.h
  #include asm/uaccess.h
 +#include asm/delay.h
 
  #include queue.h
 
 @@ -158,11 +159,159 @@ mmc_blk_getgeo(struct block_device *bdev, struct 
 hd_geometry *geo)
 return 0;
  }
 
 +static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode, unsigned 
 int ioc_cmd, unsigned long ioc_arg)
 +{
 +   struct sd_ioc_cmd sdic = {0};
 +   struct mmc_blk_data *md = NULL;
 +   struct mmc_host *host = NULL;
 +   struct mmc_card *card = NULL;
 +   struct mmc_command cmd = {0};
 +   struct mmc_data data = {0};
 +   int err = 0;
 +   struct mmc_request mrq = {0};
 +   struct scatterlist sg = {0};
 +   unsigned char *blocks = NULL;
 +   size_t data_bytes = 0;
 +#ifdef CONFIG_MMC_DEBUG
 +   char dbgbuf[64] = {0};
 +#endif
 +
 +   /*
 +* This is primarily used for application specific commands (ACMD), so
 +* the current ioc_cmd validation is trivial.
 +*/
 +   if (ioc_cmd != SD_IOC_ACMD)
 +   return -EINVAL;
 +
 +   if (copy_from_user(sdic, (void __user *) ioc_arg, sizeof(struct 
 sd_ioc_cmd))) {
 +   printk(KERN_ERR %s: error reading ioctl arg from 
 userspace\n, __func__);
 +   return -EFAULT;
 +   }
 +
 +   if (sdic.struct_version != SD_IOC_CMD_STRUCT_VERSION)
 +   return -EINVAL;
 +
 +   /* Find the mmc structures based on the bdev. */
 +   md = mmc_blk_get(bdev-bd_disk);
 +   if (!md)
 +   return -EINVAL;
 +
 +   card = md-queue.card;
 +#ifdef CONFIG_MMC_DEBUG
 +   printk(KERN_DEBUG %s: card = %p\n, __func__, card);
 +#endif
 +   if (IS_ERR(card))
 +   return PTR_ERR(card);
 +
 +#ifdef CONFIG_MMC_DEBUG
 +   printk(KERN_DEBUG %s: host = %p\n, __func__, card-host);
 +#endif
 +   host = card-host;
 +   BUG_ON(!host);
 +
 +   mmc_claim_host(host);
 +
 +   err = mmc_app_cmd(host, card);
 +   if (err) {
 +   dev_err(mmc_dev(host), %s: CMD%d error\n, __func__, 
 MMC_APP_CMD);
 +   goto ioctl_done;
 +   }
 +
 +   mrq.cmd = cmd;
 +   mrq.data = data;
 +
 +   cmd.opcode = sdic.opcode;
 +   cmd.arg = sdic.arg;
 +   cmd.flags = sdic.flags;
 +
 +   data.sg = sg;
 +   data.sg_len = 1;
 +   data.blksz = sdic.blksz;
 +   data.blocks = sdic.blocks;
 +
 +   data_bytes = data.blksz * data.blocks;
 +   blocks = (unsigned char *) kzalloc(data_bytes, GFP_KERNEL);

you shouldn't need this cast, 'void *' will go hapilly to any other type.

 +   if (!blocks) {
 +   err = -ENOMEM;
 +   goto ioctl_done;
 +   }
 +   sg_init_one(data.sg, blocks, data_bytes);
 +
 +
 +   if (copy_from_user(blocks, sdic.data, data_bytes)) {
 +   dev_err(mmc_dev(host), %s: error reading userspace 
 buffer\n, __func__);
 +   err = -EFAULT;
 +   goto ioctl_done;
 +   }
 +   if (sdic.write_flag) {
 +   data.flags = MMC_DATA_WRITE;
 +   } else {
 +   data.flags = MMC_DATA_READ;
 +   }
 +
 +   /* data.flags must already be set before doing this. */
 +   mmc_set_data_timeout(data, card);
 +   /* Allow overriding the timeout_ns for empirical tuning. */
 +   if (sdic.force_timeout_ns)
 +   data.timeout_ns = sdic.force_timeout_ns;
 +
 +#ifdef CONFIG_MMC_DEBUG
 +   hex_dump_to_buffer(blocks, data_bytes, 16, 1, dbgbuf, sizeof(dbgbuf), 
 0);
 +   dev_dbg(mmc_dev(host), %s: first bytes of pre data\n%s\n, __func__, 
 dbgbuf);
 +#endif
 +
 +   mmc_wait_for_req(host, mrq);
 +
 +   if (cmd.error) {
 +   dev_err(mmc_dev(host), %s: cmd error %d\n, __func__, 
 cmd.error);
 +   err = cmd.error;
 +   goto ioctl_done;
 +   }
 +   if (data.error) {
 +   dev_err(mmc_dev(host), %s: data 

[PATCH] mmc: enable ERASE caps for mvsdio host

2011-03-17 Thread Sascha Silbe
From: Sascha Silbe sascha-...@silbe.org

The Marvell SDIO host controller can transmit Erase commands to the card quite
fine.

Signed-off-by: Sascha Silbe sascha-...@silbe.org
---
 drivers/mmc/host/mvsdio.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Tested on OpenRD-Base using a SanDisk 4GB class 4 card (retail). The
BLKDISCARD ioctl returned immediately, no timeout issue encountered.
I had to remove and reinsert the card to read the new (i.e. erased) data, but
that's most likely either by design or a bug on a different layer.


diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index eeb1147..758251d 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -735,7 +735,8 @@ static int __init mvsd_probe(struct platform_device *pdev)

mmc-ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
mmc-caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ |
-   MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
+   MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
+   MMC_CAP_ERASE;

mmc-f_min = DIV_ROUND_UP(host-base_clock, MVSD_BASE_DIV_MAX);
mmc-f_max = maxfreq;
--
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] dw_mmc: support mmc power control with regulator

2011-03-17 Thread Chris Ball
Hi,

On Thu, Mar 17 2011, Chris Ball wrote:
 @@ -1438,6 +1439,13 @@ static int __init dw_mci_init_slot(struct dw_mci 
 *host, unsigned int id)
  }
  #endif /* CONFIG_MMC_DW_IDMAC */
  
 +host-vmmc = regulator_get(mmc_dev(mmc), vmmc);
 +if (IS_ERR(host-vmmc)) {
 +printk(KERN_INFO %s: no vmmc regulator found\n, 
 mmc_hostname(mmc));
 +host-vmmc = NULL;
 +} else
 +regulator_enable(host-vmmc);
 +
  if (dw_mci_get_cd(mmc))
  set_bit(DW_MMC_CARD_PRESENT, slot-flags);
  else

 I think you want an #ifdef CONFIG_REGULATOR around this hunk.
 Please try building the driver with CONFIG_REGULATOR=n.

 The other hunks in the patch shouldn't need it, because they test
 host-vcc before making any regulator calls.

Actually, linux/regulator/consumer.h provides no-op versions of all
these functions if CONFIG_REGULATOR is unset, so I guess this doesn't
matter; the driver builds without CONFIG_REGULATOR as-is.

Will merge for .39 with Will's ACK, thanks.

- Chris.
-- 
Chris Ball   c...@laptop.org   http://printf.net/
One Laptop Per Child
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/7] mmc: support sdhci-esdhc-imx as an OF device

2011-03-17 Thread Grant Likely
[cc'ing linux-mmc to continue this discussion]

On Wed, Mar 16, 2011 at 10:39:16PM +0800, Shawn Guo wrote:
 On Tue, Mar 15, 2011 at 01:59:26PM -0600, Grant Likely wrote:
  On Mon, Mar 14, 2011 at 10:25:57PM +0800, Shawn Guo wrote:
   Signed-off-by: Shawn Guo shawn@linaro.org
  
  dt support can be added directly to sdchi-pltfm.c drivers now.  There
  is no longer any need to use sdhci-of-core.c any more.  For an
  example, see the patch titled of/tegra: add sdhci device tree
  handling in my devicetree/test branch.
  
 I mentioned this a little bit in the cover letter of the patch set
 as below.
 
 This patch set is to support sdhci-esdhc-imx as an OF device.  As
 there is already powerpc based esdhc OF support, it chose to add OF
 support for imx esdhc driver in a different way from what sdhci-tegra
 did.

I should read your descriptions more carefully.  :-)

 The tegra approach you made was one of the two options I had, and I
 happened to love the another more, as it consolidates the eSDHC OF
 driver for Freescale MPCxxx and i.MX family.

Heh, I don't dispute the value of merging code.  However, with this
approach it means that DT and non-DT imx platforms will be using
different drivers for the same device.  Given the choices, I'd
rather see the imx driver used in both DT and non-DT situations
instead of sharing code with the powerpc version.  I've learnt the
hard way that it is just too painful having two drivers for the same
hardware; particularly when the only difference is the method used to
probe them.

Actually, what I'd *really* rather see is the powerpc code migrated
over to sdhci_pltfm.c, and then have the imx compatible value added to
it.  I'll make sure to get some help from the Freescale powerpc folks
to test any patch you produce to that end.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] dw_mmc: set fixed burst in BMOD register

2011-03-17 Thread Will Newton
On Thu, Mar 17, 2011 at 6:24 PM, Chris Ball c...@laptop.org wrote:
 Hi Will,

 On Thu, Feb 24 2011, Jaehoon Chung wrote:
 This patch is applied fixed burst.
 If use internal DMA controller, i think that need to set this bit.

 I tested when set this bit or not. I found that increase performance with 
 IDMAC

 Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  drivers/mmc/host/dw_mmc.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
 index 58476c1..46e5a89 100644
 --- a/drivers/mmc/host/dw_mmc.c
 +++ b/drivers/mmc/host/dw_mmc.c
 @@ -315,7 +315,7 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)

       /* Stop the IDMAC running */
       temp = mci_readl(host, BMOD);
 -     temp = ~SDMMC_IDMAC_ENABLE;
 +     temp = ~(SDMMC_IDMAC_ENABLE | SDMMC_IDMAC_FB);
       mci_writel(host, BMOD, temp);
  }

 @@ -384,7 +384,7 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, 
 unsigned int sg_len)

       /* Enable the IDMAC */
       temp = mci_readl(host, BMOD);
 -     temp |= SDMMC_IDMAC_ENABLE;
 +     temp |= SDMMC_IDMAC_ENABLE | SDMMC_IDMAC_FB;
       mci_writel(host, BMOD, temp);

       /* Start it running */

 Any thoughts on this patch?

From the documentation I have it's not clear what this bit does. I
have very limited access to hardware with internal DMA support, so I
can't really test it either. I'm willing to believe the patch improves
things but I can't really say one way or the other.
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/7] mmc: consolidate sdhci_pltfm_data and sdhci_of_data into one

2011-03-17 Thread Grant Likely
[cc'ing linux-mmc@vger.kernel.org]

On Thu, Mar 17, 2011 at 02:33:20PM +0800, Shawn Guo wrote:
 On Tue, Mar 15, 2011 at 01:55:13PM -0600, Grant Likely wrote:
  On Mon, Mar 14, 2011 at 10:25:56PM +0800, Shawn Guo wrote:
   This patch is motivated by the work of supporting sdhci-esdhc-imx as
   an OF device.  The sdhci-esdhc-imx driver was well designed to be
   able to work with either platform or OF bus, with a little effort to
   decouple the driver from sdhci_pltfm_data.
   
   Like sdhci_ops works for both platform and OF sdhci driver, the patch
   demonstrates that sdhci_pltfm_data and sdhci_of_data can be
   consolidated into sdhci_data, which should work for both.  As one of
   the results, header linux/mmc/sdhci-pltfm.h can be deleted.
   
   Signed-off-by: Shawn Guo shawn@linaro.org
  
  I like the push to unify DT and non-DT usage with the SDHCI drivers,
  but I'm not convinced on the approach.  Actually, that's more a
  comment on the existing code than it is on the this patch.
  
 Yes, this patch is not supposed to mean that much.  It only intends
 to unify one data structure so that sdhci-esdhc-imx.c can work for
 both cases.
 
 The topic you raised here is a bigger one which may involve debate
 with authors of both sdhci-pltfm.c and sdhci-of-core.c.  We can take
 it as the final goal, and this patch could be a first step to that
 goal.

I doubt very much that the sdhci-of-core.c users/developers will have
a problem with this.  There is a strong trend toward unifying DT and
non-DT code, and Linux has a strong pattern of each driver handling
its own registration.

I actually don't have an objection to this patch specifically, but
rather I want to see the overall direction be to eliminate
sdhci-of-core.c and sdhci-pltfm.c entirely instead of using it more..

On another note, this patch changes a number of names, both of 
structures and variables.  Specifically:

{sdhci_pltfm_data,sdhci_of_data} == sdhci_data and
pdata == data

However, it would be easier to review if the pdata==data change was
dropped (the name of the local variable doesn't matter that much), and
if sdhci_of_data was renamed to sdhci_pltfm_data.  Doing so would make
the diff much smaller without changing the sanity of the resulting
code.

g.

 
 -- 
 Regards,
 Shawn
 
  I don't like the way that sdhci-pltfm.c and sdhci-of-core.c each take
  responsibility for the .probe hook on each of the implementations.
  Doing it that way means that each implementation needs to add a set of
  hooks into those core files protected by #ifdefs based on whether or
  not the driver is enabled.  It also means that loading one driver
  means that all the configured sdhci drivers get loaded into the
  kernel.  This seems backwards.
  
  From what I can see, sdhci-pltfm.c and sdhci-of-core.c both provide
  useful common functions that would be used by all sdhci host drivers.
  The interface would be a lot cleaner if those routines were exported
  for use by other modules, and each driver registered its own probe
  hook.  It would keep all the driver specific stuff out of
  sdhci_pltfm_ids and I think it would result in a cleaner interface
  overall.
  
  Here is how I would do it:
  
  1) break the bulk of the sdhci_pltfm.c and sdhci_of_core.c .probe and
  .remove hooks out into separate functions; callable by other modules
  2) for each driver, add its own probe and remove hooks which calls
  into the new functions created in step 1.  This would eliminate the
  need for the -exit and -init callbacks since they would get rolled
  into the new .probe and .remove callbacks
  3) finally, when all drivers are removed; eliminate the driver
  registration from sdhci_pltfm.c and sdhci_of_core.c because there
  wouldn't be any users left.
  
  drivers/mmc/host/sdhci-pxa.c appears to be a good example of how I
  think sdhci platform_drivers should be structured.
  
  At the same time, the functionality from sdhci_of_core.c could easily
  be migrated into sdhci_pltfm.c.
  
 
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/7] mmc: support sdhci-esdhc-imx as an OF device

2011-03-17 Thread Rob Herring

On 03/17/2011 03:22 PM, Grant Likely wrote:

[cc'ing linux-mmc to continue this discussion]

On Wed, Mar 16, 2011 at 10:39:16PM +0800, Shawn Guo wrote:

On Tue, Mar 15, 2011 at 01:59:26PM -0600, Grant Likely wrote:

On Mon, Mar 14, 2011 at 10:25:57PM +0800, Shawn Guo wrote:

Signed-off-by: Shawn Guoshawn@linaro.org


dt support can be added directly to sdchi-pltfm.c drivers now.  There
is no longer any need to use sdhci-of-core.c any more.  For an
example, see the patch titled of/tegra: add sdhci device tree
handling in my devicetree/test branch.


I mentioned this a little bit in the cover letter of the patch set
as below.

This patch set is to support sdhci-esdhc-imx as an OF device.  As
there is already powerpc based esdhc OF support, it chose to add OF
support for imx esdhc driver in a different way from what sdhci-tegra
did.


I should read your descriptions more carefully.  :-)


The tegra approach you made was one of the two options I had, and I
happened to love the another more, as it consolidates the eSDHC OF
driver for Freescale MPCxxx and i.MX family.


Heh, I don't dispute the value of merging code.  However, with this
approach it means that DT and non-DT imx platforms will be using
different drivers for the same device.  Given the choices, I'd
rather see the imx driver used in both DT and non-DT situations
instead of sharing code with the powerpc version.  I've learnt the
hard way that it is just too painful having two drivers for the same
hardware; particularly when the only difference is the method used to
probe them.

Actually, what I'd *really* rather see is the powerpc code migrated
over to sdhci_pltfm.c, and then have the imx compatible value added to
it.  I'll make sure to get some help from the Freescale powerpc folks
to test any patch you produce to that end.


Based on past experience, there will be differences between imx and ppc 
h/w even though it is the same block.


Rob
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V6 3/4] mmc: sdhci-esdhc: make the writel/readl as the general APIs

2011-03-17 Thread Wolfram Sang
Hi Richard,

On Thu, Mar 17, 2011 at 02:34:06PM +0800, Richard Zhu wrote:
 Add one flag to indicate the GPIO CD/WP is enabled or not
 on imx platforms, and reuse the writel/readl as the general
 APIs for imx SOCs.
 
 Signed-off-by: Richard Zhu hong-xing@freescale.com
 ---
  drivers/mmc/host/sdhci-esdhc-imx.c |   43 
 ++--
  drivers/mmc/host/sdhci-pltfm.h |2 +-
  2 files changed, 37 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
 b/drivers/mmc/host/sdhci-esdhc-imx.c
 index 3b52485..5768e06 100644
 --- a/drivers/mmc/host/sdhci-esdhc-imx.c
 +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
 @@ -16,6 +16,7 @@
  #include linux/err.h
  #include linux/clk.h
  #include linux/gpio.h
 +#include linux/slab.h
  #include linux/mmc/host.h
  #include linux/mmc/sdhci-pltfm.h
  #include mach/hardware.h
 @@ -24,6 +25,13 @@
  #include sdhci-pltfm.h
  #include sdhci-esdhc.h
  
 +#define IMX_GPIO_CD_WP   (1  0)

Hmm, that sounds like a macro for a gpio-number. What about
ESDHC_FLAG_GPIO_FOR_CD_WP or something like that?

 +
 +struct pltfm_imx_data {
 + int flags;
 + u32 mod_val;

Maybe 'scratchpad' was not the best name, but 'mod_val' seems worse to me.

Those are minor things, but there is one thing which needs respinning IMHO:
While it looks functionally OK, I'll tell you (once again) that you cast too
much. Please be careful about that in the future!

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature


Re: [PATCH resend] mmc: Added ioctl to let userspace apps send ACMDs

2011-03-17 Thread Arnd Bergmann
On Thursday 17 March 2011 19:28:55 John Calixto wrote:
 Part 3 of the SD Specification (SD Card Association; www.sdcard.org) describes
 how to use the security function of an SD card using application specific
 commands in conjunction with CPRM algorithms and keys licensed from the 4C
 Entity (www.4centity.com).  This allows userspace applications to access this
 security feature.

Having the ability to send commands from user space sounds useful,
a number of other block drivers can do this, too.

However, for the specific example you mention, I think it would be
nicer to implement it in kernel space, and have a high-level
interface.

A few more comments about the implementation below.

 Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 SoC, TI
 OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC.

Is this safe to do while the device is operating and a file system
writes data?

Does it allow sending all commands or just the ones you require?

I've been thinking about adding something like this to allow
a passthrough mode for e.g. qemu, or perhaps for combining
it with a user space mmc host (to be written) to allow tracing
the communication between host and card from user space.

 +{
 +   struct sd_ioc_cmd sdic = {0};
 +   struct mmc_blk_data *md = NULL;
 +   struct mmc_host *host = NULL;
 +   struct mmc_card *card = NULL;
 +   struct mmc_command cmd = {0};
 +   struct mmc_data data = {0};
 +   int err = 0;
 +   struct mmc_request mrq = {0};
 +   struct scatterlist sg = {0};
 +   unsigned char *blocks = NULL;
 +   size_t data_bytes = 0;

[style] Don't initialize variables to zero if they get assigned
to something else later, otherwise you prevent the compiler
from warning you about accesses to uninitialized variables.

 +#ifdef CONFIG_MMC_DEBUG
 +   char dbgbuf[64] = {0};
 +#endif

 +
 +   if (copy_from_user(sdic, (void __user *) ioc_arg, sizeof(struct 
 sd_ioc_cmd))) {
 +   printk(KERN_ERR %s: error reading ioctl arg from 
 userspace\n, __func__);
 +   return -EFAULT;
 +   }

Don't printk information about user input, only about unexpected
data you get from the card. If you print an error, use dev_err().

 +   if (sdic.struct_version != SD_IOC_CMD_STRUCT_VERSION)
 +   return -EINVAL;

Don't use version information for ioctl data structures. We cannot
change them anyways, so the version number is redundant.
If we ever need to extend the structure, it has to be done in
a compatible way, or by introducing a new ioctl command with
a different structure, but leaving the old one in place.

 +   /* Find the mmc structures based on the bdev. */
 +   md = mmc_blk_get(bdev-bd_disk);
 +   if (!md)
 +   return -EINVAL;
 +
 +   card = md-queue.card;
 +#ifdef CONFIG_MMC_DEBUG
 +   printk(KERN_DEBUG %s: card = %p\n, __func__, card);
 +#endif

dev_dbg()

 +   if (IS_ERR(card))
 +   return PTR_ERR(card);
 +
 +#ifdef CONFIG_MMC_DEBUG
 +   printk(KERN_DEBUG %s: host = %p\n, __func__, card-host);
 +#endif

dev_dbg()

 +   host = card-host;
 +   BUG_ON(!host);

try to be very careful about BUG_ON(), it's always better to return
an error if you can avoid crashing the current process.

 +   mmc_claim_host(host);
 +
 +   err = mmc_app_cmd(host, card);
 +   if (err) {
 +   dev_err(mmc_dev(host), %s: CMD%d error\n, __func__, 
 MMC_APP_CMD);
 +   goto ioctl_done;
 +   }

Is this an unexpected error case? If it can be triggered by sending
specific commands, it would be better to just return the error
number ot user space without printing anything.

 +   data_bytes = data.blksz * data.blocks;
 +   blocks = (unsigned char *) kzalloc(data_bytes, GFP_KERNEL);

No need for the typecast, kzalloc returns a void pointer.

 +   if (copy_from_user(blocks, sdic.data, data_bytes)) {
 +   dev_err(mmc_dev(host), %s: error reading userspace 
 buffer\n, __func__);

no printing here again

 +#ifdef CONFIG_MMC_DEBUG
 +   hex_dump_to_buffer(blocks, data_bytes, 16, 1, dbgbuf, sizeof(dbgbuf), 
 0);
 +   dev_dbg(mmc_dev(host), %s: first bytes of pre data\n%s\n, __func__, 
 dbgbuf);
 +#endif

Is this really still needed? I would assume that if your code works
fine, you can just remove the hex dump.

  static const struct block_device_operations mmc_bdops = {
 .open   = mmc_blk_open,
 .release= mmc_blk_release,
 .getgeo = mmc_blk_getgeo,
 .owner  = THIS_MODULE,
 +   .ioctl  = mmc_blk_ioctl,
  };

This also needs a compat_ioctl pointer, new drivers should always
work in both 32 and 64 bit mode.
 
 diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
 index 797cdb5..0453dcd 100644
 --- a/drivers/mmc/core/sd_ops.c
 +++ b/drivers/mmc/core/sd_ops.c
 @@ -20,7 +20,7 @@
  #include core.h