Re: [PATCH] MMC: remove regulator refcount fiddling in mmc core
Linus Walleij wrote:
Currently the mmc_regulator_set_ocr() fiddles with the regulator
refcount by selectively calling regulator_[enable|disable]
depending on the state of the regulator. This will confuse the
reference count if case the regulator is for example shared with
other MMC slots or user for other stuff than the MMC card.
Push regulator_[enable|disable] out into the MMC host drivers
and remove this from the MMC core so the reference count can be
trusted.
That is not a technical reason. It would be better to provide
more regulator support in core so there is less duplication in
the drivers.
At least it should be a separate patch.
Cc: Andrew Morton a...@linux-foundation.org
Cc: Liam Girdwood l...@slimlogic.co.uk
Cc: Mark Brown broo...@opensource.wolfsonmicro.com
Cc: Tony Lindgren t...@atomide.com
Cc: Adrian Hunter adrian.hun...@nokia.com
Cc: Robert Jarzmik robert.jarz...@free.fr
Cc: Sundar Iyer sundar.i...@stericsson.com
Cc: Bengt Jonsson bengt.jons...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@stericsson.com
---
This has been regression compiled for U300, OMAP3, PXA3XX
defconfigs, tested on U300.
We're facing problems with our regulator reference counters if
this is not fixed.
---
drivers/mmc/core/core.c | 66 ++---
drivers/mmc/host/mmci.c | 10 --
drivers/mmc/host/omap_hsmmc.c | 40 ++--
drivers/mmc/host/pxamci.c | 17 --
4 files changed, 78 insertions(+), 55 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 569e94d..904f245 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -784,47 +784,39 @@ int mmc_regulator_set_ocr(struct regulator *supply,
unsigned short vdd_bit)
{
int result = 0;
int min_uV, max_uV;
- int enabled;
+ int tmp;
+ int voltage;
- enabled = regulator_is_enabled(supply);
- if (enabled 0)
- return enabled;
-
- if (vdd_bit) {
- int tmp;
- int voltage;
-
- /* REVISIT mmc_vddrange_to_ocrmask() may have set some
-* bits this regulator doesn't quite support ... don't
-* be too picky, most cards and regulators are OK with
-* a 0.1V range goof (it's a small error percentage).
-*/
- tmp = vdd_bit - ilog2(MMC_VDD_165_195);
- if (tmp == 0) {
- min_uV = 1650 * 1000;
- max_uV = 1950 * 1000;
- } else {
- min_uV = 1900 * 1000 + tmp * 100 * 1000;
- max_uV = min_uV + 100 * 1000;
- }
-
- /* avoid needless changes to this voltage; the regulator
-* might not allow this operation
-*/
- voltage = regulator_get_voltage(supply);
- if (voltage 0)
- result = voltage;
- else if (voltage min_uV || voltage max_uV)
- result = regulator_set_voltage(supply, min_uV, max_uV);
- else
- result = 0;
+ if (!vdd_bit)
+ return 0;
- if (result == 0 !enabled)
- result = regulator_enable(supply);
- } else if (enabled) {
- result = regulator_disable(supply);
+ /*
+* REVISIT mmc_vddrange_to_ocrmask() may have set some
+* bits this regulator doesn't quite support ... don't
+* be too picky, most cards and regulators are OK with
+* a 0.1V range goof (it's a small error percentage).
+*/
+ tmp = vdd_bit - ilog2(MMC_VDD_165_195);
+ if (tmp == 0) {
+ min_uV = 1650 * 1000;
+ max_uV = 1950 * 1000;
+ } else {
+ min_uV = 1900 * 1000 + tmp * 100 * 1000;
+ max_uV = min_uV + 100 * 1000;
}
+ /*
+* Avoid needless changes to this voltage; the regulator
+* might not allow this operation
+*/
+ voltage = regulator_get_voltage(supply);
+ if (voltage 0)
+ result = voltage;
+ else if (voltage min_uV || voltage max_uV)
+ result = regulator_set_voltage(supply, min_uV, max_uV);
+ else
+ result = 0;
+
return result;
}
EXPORT_SYMBOL(mmc_regulator_set_ocr);
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..5f530b1 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -467,15 +467,17 @@ static void mmci_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
switch (ios-power_mode) {
case MMC_POWER_OFF:
- if(host-vcc
- regulator_is_enabled(host-vcc))
+ if (host-vcc)
Re: [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller
Adrian ,
Sorry for the late response
snip
As per my email 5/5/10, I would suggest the only change to omap_hsmmc is:
Agreed and followed the changes mostly but made some more changes on top of it.
snip
And that the late init function is used to do the rest e.g.
find a home for these 3 functions:
I agree just having the 3 functions makes it work.
static int omap4_twl6030_hsmmc_late_init(struct device *dev)
{
int ret = 0;
struct platform_device *pdev = container_of(dev,
struct platform_device, dev);
struct omap_mmc_platform_data *pdata = dev-platform_data;
/* MMC1 Card detect Configuration */
if (pdev-id == 0) {
ret = omap4_hsmmc1_card_detect_config();
if (ret 0)
pr_err(Unable to configure Card detect for MMC1\n);
pdata-slots[0].card_detect = twl6030_mmc_card_detect;
pdata-slots[0].card_detect_irq = TWL6030_IRQ_BASE +
MMCDETECT_INTR_OFFSET;
}
return ret;
snip
Few Comments below:
1) In the above function, initializing card_detect in the driver as
done in omap_hsmmc_gpio_init might be more readable and this has been
done in nongpio_init instead.
Even having initialization of card_detect_irq inside nongpio_init is fine.
2)Also calling omap_hsmmc_gpio_init in case of a card detect line
which is not GPIO
doesn't make sense though it assigns -EINVAL to switch_pin in case of
invalid GPIO
which is intended for a non-removable card .
3) And also having some thing like GPIO and NON_GPIO flag to
distinguish might make sense.
Regards,
Kishore
--
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] SDHCI: Don't assign mmc-caps at SDHCI directly
On Sat, 12 Jun 2010 14:44:50 +0900 Kyungmin Park kyungmin.p...@samsung.com wrote: From: Kyungmin Park kyungmin.p...@samsung.com Some host controller can set mmc-caps before sdhci_add_host. Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/mmc/host/sdhci.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 4321e0c..142419c 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1791,7 +1791,7 @@ int sdhci_add_host(struct sdhci_host *host) else mmc-f_min = host-max_clk / 256; mmc-f_max = host-max_clk; - mmc-caps = MMC_CAP_SDIO_IRQ; + mmc-caps |= MMC_CAP_SDIO_IRQ; if (!(host-quirks SDHCI_QUIRK_FORCE_1_BIT_DATA)) mmc-caps |= MMC_CAP_4_BIT_DATA; A great shower of MMC patches have magically turned up in linux-next, apparently via some tree of Grant's. Those patches changed the above code to look like: if (!(host-quirks SDHCI_QUIRK_NO_SDIO_IRQ)) mmc-caps |= MMC_CAP_SDIO_IRQ; So it appears that this bug is fixed in that code as well. So I'll drop your patch. If the above changes end up not getting merged into mainline then your fix will be lost. That fix was unchangelogged. In fact the patch was completely unchangelogged and I haven't looked at it at all and as far as I can tell none of it has been sent to the mmc list. -- 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] SDHCI: 8-bit data transfer width support
On Sat, 12 Jun 2010 14:45:02 +0900
Kyungmin Park kyungmin.p...@samsung.com wrote:
From: Kyungmin Park kyungmin.p...@samsung.com
Some host constroller such as s5pc110 has WIDE8 support feature.
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/sdhci.c |5 +
drivers/mmc/host/sdhci.h |1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 142419c..6cf018a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1159,6 +1159,11 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct
mmc_ios *ios)
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+ if (ios-bus_width == MMC_BUS_WIDTH_8)
+ ctrl |= SDHCI_CTRL_8BITBUS;
+ else
+ ctrl = ~SDHCI_CTRL_8BITBUS;
+
if (ios-bus_width == MMC_BUS_WIDTH_4)
ctrl |= SDHCI_CTRL_4BITBUS;
else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..eb5efe0 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -72,6 +72,7 @@
#define SDHCI_CTRL_ADMA1 0x08
#define SDHCI_CTRL_ADMA32 0x10
#define SDHCI_CTRL_ADMA64 0x18
+#define SDHCI_CTRL_8BITBUS 0x20
#define SDHCI_POWER_CONTROL 0x29
#define SDHCI_POWER_ON 0x01
This change (or something similar) also seems to have been lumped into
the unchangelogged, unreviewed mmc: sdhci: Initial Tegra sdhci driver
patch. So again, I'll drop your patch and if mmc: sdhci: Initial
Tegra sdhci driver doesn't get merged, your patch will be lost.
I wonder what else it does. Here it is:
commit feed6702dc4bb130869171cbd8167637ea13c33c
Author: Colin Cross ccr...@android.com
AuthorDate: Wed Mar 10 20:42:35 2010 -0800
Commit: Grant Likely grant.lik...@secretlab.ca
CommitDate: Fri Jun 25 09:47:58 2010 -0600
mmc: sdhci: Initial Tegra sdhci driver
Signed-off-by: Colin Cross ccr...@android.com
[Olof: Fixed up merge conflicts]
Signed-off-by: Olof Johansson o...@lixom.net
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
diff --git a/arch/arm/mach-tegra/include/mach/sdhci.h
b/arch/arm/mach-tegra/include/mach/sdhci.h
new file mode 100644
index 000..34e2686
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/sdhci.h
@@ -0,0 +1,33 @@
+/*
+ * include/asm-arm/arch-tegra/sdhci.h
+ *
+ * Copyright (C) 2009 Palm, Inc.
+ * Author: Yvonne Yip y...@palm.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __ASM_ARM_ARCH_TEGRA_SDHCI_H
+#define __ASM_ARM_ARCH_TEGRA_SDHCI_H
+
+#include linux/mmc/host.h
+
+struct tegra_sdhci_platform_data {
+ const char *clk_id;
+ int force_hs;
+ int cd_gpio;
+ int wp_gpio;
+ int power_gpio;
+
+ void (*board_probe)(int id, struct mmc_host *);
+ void (*board_remove)(int id, struct mmc_host *);
+};
+
+#endif
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index b9dee28..85c473e 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -229,11 +229,13 @@ static int sdio_enable_hs(struct mmc_card *card)
int ret;
u8 speed;
- if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
- return 0;
+ if (!(card-host-caps MMC_CAP_FORCE_HS)) {
+ if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
+ return 0;
- if (!card-cccr.high_speed)
- return 0;
+ if (!card-cccr.high_speed)
+ return 0;
+ }
ret = mmc_io_rw_direct(card, 0, 0, SDIO_CCCR_SPEED, 0, speed);
if (ret)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index f06d06e..357c294 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -382,6 +382,12 @@ config MMC_TMIO
This provides support for the SD/MMC cell found in TC6393XB,
T7L66XB and also HTC ASIC3
+config MMC_SDHCI_TEGRA
+ tristate Tegra SD/MMC Controller Support
+ depends on ARCH_TEGRA MMC_SDHCI
+ help
+ This selects the Tegra SD/MMC controller.
+
config MMC_CB710
tristate ENE CB710 MMC/SD Interface support
depends on PCI
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e30c2ee..fb04448 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
obj-$(CONFIG_MMC_SDHCI_S3C)+= sdhci-s3c.o
Re: [PATCH 3/3] SDHCI: 8-bit data transfer width support
These patches are not coming from the tegra/for-next branch, they are
coming from Grant's devicetree-next branch. Grant, why are these
patches in your tree, and why is tegra/for-next in your tree? It's
going to cause conflicts when we rebase our for-next branch. Please
remove tegra and this sdhci patch from your tree.
As for the mmc patch, it seems to have been over-squashed to contain
the changes to the common code and the tegra sdhci driver. Now that
tegra is in for-next, we'll be cleaning up and sending subsystem
drivers for review. Expect a patch within a week or two.
On Mon, Jun 28, 2010 at 11:39 AM, Andrew Morton
a...@linux-foundation.org wrote:
On Sat, 12 Jun 2010 14:45:02 +0900
Kyungmin Park kyungmin.p...@samsung.com wrote:
From: Kyungmin Park kyungmin.p...@samsung.com
Some host constroller such as s5pc110 has WIDE8 support feature.
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/sdhci.c | 5 +
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 142419c..6cf018a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1159,6 +1159,11 @@ static void sdhci_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+ if (ios-bus_width == MMC_BUS_WIDTH_8)
+ ctrl |= SDHCI_CTRL_8BITBUS;
+ else
+ ctrl = ~SDHCI_CTRL_8BITBUS;
+
if (ios-bus_width == MMC_BUS_WIDTH_4)
ctrl |= SDHCI_CTRL_4BITBUS;
else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..eb5efe0 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -72,6 +72,7 @@
#define SDHCI_CTRL_ADMA1 0x08
#define SDHCI_CTRL_ADMA32 0x10
#define SDHCI_CTRL_ADMA64 0x18
+#define SDHCI_CTRL_8BITBUS 0x20
#define SDHCI_POWER_CONTROL 0x29
#define SDHCI_POWER_ON 0x01
This change (or something similar) also seems to have been lumped into
the unchangelogged, unreviewed mmc: sdhci: Initial Tegra sdhci driver
patch. So again, I'll drop your patch and if mmc: sdhci: Initial
Tegra sdhci driver doesn't get merged, your patch will be lost.
I wonder what else it does. Here it is:
commit feed6702dc4bb130869171cbd8167637ea13c33c
Author: Colin Cross ccr...@android.com
AuthorDate: Wed Mar 10 20:42:35 2010 -0800
Commit: Grant Likely grant.lik...@secretlab.ca
CommitDate: Fri Jun 25 09:47:58 2010 -0600
mmc: sdhci: Initial Tegra sdhci driver
Signed-off-by: Colin Cross ccr...@android.com
[Olof: Fixed up merge conflicts]
Signed-off-by: Olof Johansson o...@lixom.net
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
diff --git a/arch/arm/mach-tegra/include/mach/sdhci.h
b/arch/arm/mach-tegra/include/mach/sdhci.h
new file mode 100644
index 000..34e2686
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/sdhci.h
@@ -0,0 +1,33 @@
+/*
+ * include/asm-arm/arch-tegra/sdhci.h
+ *
+ * Copyright (C) 2009 Palm, Inc.
+ * Author: Yvonne Yip y...@palm.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __ASM_ARM_ARCH_TEGRA_SDHCI_H
+#define __ASM_ARM_ARCH_TEGRA_SDHCI_H
+
+#include linux/mmc/host.h
+
+struct tegra_sdhci_platform_data {
+ const char *clk_id;
+ int force_hs;
+ int cd_gpio;
+ int wp_gpio;
+ int power_gpio;
+
+ void (*board_probe)(int id, struct mmc_host *);
+ void (*board_remove)(int id, struct mmc_host *);
+};
+
+#endif
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index b9dee28..85c473e 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -229,11 +229,13 @@ static int sdio_enable_hs(struct mmc_card *card)
int ret;
u8 speed;
- if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
- return 0;
+ if (!(card-host-caps MMC_CAP_FORCE_HS)) {
+ if (!(card-host-caps MMC_CAP_SD_HIGHSPEED))
+ return 0;
- if (!card-cccr.high_speed)
- return 0;
+ if (!card-cccr.high_speed)
+ return 0;
+ }
ret = mmc_io_rw_direct(card, 0, 0, SDIO_CCCR_SPEED, 0, speed);
if (ret)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index f06d06e..357c294 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -382,6 +382,12 @@ config MMC_TMIO
Re: [PATCH 2/3] SDHCI: Don't assign mmc-caps at SDHCI directly
On Mon, Jun 28, 2010 at 11:34 AM, Andrew Morton a...@linux-foundation.org wrote: On Sat, 12 Jun 2010 14:44:50 +0900 Kyungmin Park kyungmin.p...@samsung.com wrote: From: Kyungmin Park kyungmin.p...@samsung.com Some host controller can set mmc-caps before sdhci_add_host. Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/mmc/host/sdhci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 4321e0c..142419c 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1791,7 +1791,7 @@ int sdhci_add_host(struct sdhci_host *host) else mmc-f_min = host-max_clk / 256; mmc-f_max = host-max_clk; - mmc-caps = MMC_CAP_SDIO_IRQ; + mmc-caps |= MMC_CAP_SDIO_IRQ; if (!(host-quirks SDHCI_QUIRK_FORCE_1_BIT_DATA)) mmc-caps |= MMC_CAP_4_BIT_DATA; A great shower of MMC patches have magically turned up in linux-next, apparently via some tree of Grant's. Those patches changed the above code to look like: if (!(host-quirks SDHCI_QUIRK_NO_SDIO_IRQ)) mmc-caps |= MMC_CAP_SDIO_IRQ; So it appears that this bug is fixed in that code as well. So I'll drop your patch. If the above changes end up not getting merged into mainline then your fix will be lost. That fix was unchangelogged. In fact the patch was completely unchangelogged and I haven't looked at it at all and as far as I can tell none of it has been sent to the mmc list. I messed it up by pushing the wrong branch. I'm pulling it out now and it won't be in the next linux-next. 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 3/3] SDHCI: 8-bit data transfer width support
On Mon, Jun 28, 2010 at 11:56:26AM -0700, Colin Cross wrote: These patches are not coming from the tegra/for-next branch, they are coming from Grant's devicetree-next branch. Grant, why are these patches in your tree, and why is tegra/for-next in your tree? It's going to cause conflicts when we rebase our for-next branch. Please remove tegra and this sdhci patch from your tree. Yeah, this seems to be a mixup by Grant. I gave him a few patches I had picked up locally to work with, and he seems to have published the work accidentally. -Olof -- 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
