Re: [PATCH 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC
On 07/02/2013, Sachin Kamat sachin.ka...@linaro.org wrote: Added compatibility string for Exynos4412 SoC. If using same drv_data, i would prefer to use exynos_drv_data instead of exynos5250_drv_data. This patch could be separated into two patches. like this compatibility for exynos4412 SOC and rename the drv_data Came across this patch from Dongjin Kim [1] who has already attempetd the above suggestion. Hence dropping my patch. Dongjin, Are you still working on this patch [1] as this has not yet been accepted or merged? [1] http://lkml.indiana.edu/hypermail/linux/kernel/1301.3/00450.html -- With warm regards, Sachin -- 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 v4 12/13] mmc: add DT bindings for more MMC capability flags
Hi Sergei On Sun, 17 Feb 2013, Sergei Shtylyov wrote: Hello. On 02/15/2013 06:14 PM, Guennadi Liakhovetski wrote: Many MMC capability flags are platform-dependent and are traditionally set in platform data. With DT often each such capability requires a special binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED, MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to DT parser to look up keep-power-in-suspend and enable-sdio-wakeup bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively, if found. Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de --- Documentation/devicetree/bindings/mmc/mmc.txt |4 drivers/mmc/core/host.c | 13 + 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index 24c8552..d9ab51f 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -25,6 +25,10 @@ Optional properties: - max-frequency: maximum operating clock frequency - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on this system, even if the controller claims it is. +- cap-sd-highspeed: SD high-speed timing is supported +- cap-mmc-highspeed: MMC high-speed timing is supported +- cap-power-off-card: powering off the card is safe +- cap-sdio-irq: enable SDIO IRQ signalling on this interface *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line polarity properties, we have to fix the meaning of the normal and inverted diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index daa2ed1..1365466 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host) } if (explicit_inv_wp ^ gpio_inv_wp) host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH; + + if (of_find_property(np, cap-sd-highspeed,len)) + host-caps |= MMC_CAP_SD_HIGHSPEED; + if (of_find_property(np, cap-mmc-highspeed,len)) + host-caps |= MMC_CAP_MMC_HIGHSPEED; + if (of_find_property(np, cap-power-off-card,len)) + host-caps |= MMC_CAP_POWER_OFF_CARD; + if (of_find_property(np, cap-sdio-irq,len)) + host-caps |= MMC_CAP_SDIO_IRQ; + if (of_find_property(np, keep-power-in-suspend,len)) + host-pm_caps |= MMC_PM_KEEP_POWER; + if (of_find_property(np, enable-sdio-wakeup,len)) + host-pm_caps |= MMC_PM_WAKE_SDIO_IRQ; Why are you not using of_property_read_bool() instead? Because it's the same. Where I need a boolean return value I so use of_property_read_bool(), e.g. explicit_inv_cd = of_property_read_bool(np, cd-inverted); But where I just need to check, whether a property is present, using of_find_property() isn't any worse, IMHO. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally
On Sun, 17 Feb 2013, Simon Horman wrote: On Sun, Feb 17, 2013 at 04:52:16PM +0900, Simon Horman wrote: On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote: Hi Guennadi, On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote: MMC defines a number of standard DT bindings. Having each driver parse them individually adds code redundancy and is error prone. Provide a standard function to unify the parsing. After all drivers are converted to using it instead of their own parsers, this function can be integrated into mmc_alloc_host(). Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v5: 1. fix an uninitialised variable warning. Note, I don't actually know, whether this will fix the error, reported by the kbuild test robot. None of my compilers reports an error there, at most, I've got a warning with one of them, and, surprisingly, it is gone after this change. Surprisingly, because I only add the bus_width initialisation in the error case - exactly as it actually has to be done. In the success case it is assigned set by the function. But the compiler cannot know that! Maybe the build robot builds with devicetree disabled? In this case of_property_read_u32_array expands to a static inline function and the compiler indeed knows that bus_width is unitialized. It also knows that this function always returns an error, so what you did below should silence the compiler. It seems so. The configuration that flagged the error was atngw100_defconfig. ARCH=avr32 make atngw100_defconfig grep USE_OF .config [nothing] In that vein I managed to reproduce the warning using ap4evb_defconfig and then enabled MMC. I have also verified that v5 does not produce the same warning. I chose this as I don't have a avr32 cross-compile environment but I do have one for ARM and I know that ap4evb doesn't use DT. I will update topic/mmc with v5. Very good, thanks! So, this way I think, v5 should be ok. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 1/2] mmc: dw_mmc: Make dw_mci_exynos_probe static
Silences the following sparse warning:
drivers/mmc/host/dw_mmc-exynos.c:218:5: warning:
symbol 'dw_mci_exynos_probe' was not declared. Should it be static?
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/mmc/host/dw_mmc-exynos.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 72fd0f2..5a09c77 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -215,7 +215,7 @@ static const struct of_device_id dw_mci_exynos_match[] = {
};
MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
-int dw_mci_exynos_probe(struct platform_device *pdev)
+static int dw_mci_exynos_probe(struct platform_device *pdev)
{
const struct dw_mci_drv_data *drv_data;
const struct of_device_id *match;
--
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
[PATCH 2/2] mmc: dw_mmc: exynos: Remove unnecessary use of of_match_ptr()
'dw_mci_exynos_match' is always compiled in. Hence of_match_ptr
is not required.
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/mmc/host/dw_mmc-exynos.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 5a09c77..517a603 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -230,7 +230,7 @@ static struct platform_driver dw_mci_exynos_pltfm_driver = {
.remove = __exit_p(dw_mci_pltfm_remove),
.driver = {
.name = dwmmc_exynos,
- .of_match_table = of_match_ptr(dw_mci_exynos_match),
+ .of_match_table = dw_mci_exynos_match,
.pm = dw_mci_pltfm_pmops,
},
};
--
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 1/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC
Hello Sachin, I have one more patch which use 'exynos_drv_data' for its drv_data, but not submitted yet. I will send it soon anyway, anything else do you like to point out on the patch? :) Regards, Dongjin. On Mon, Feb 18, 2013 at 5:46 PM, Sachin Kamat sachin.ka...@linaro.org wrote: On 07/02/2013, Sachin Kamat sachin.ka...@linaro.org wrote: Added compatibility string for Exynos4412 SoC. If using same drv_data, i would prefer to use exynos_drv_data instead of exynos5250_drv_data. This patch could be separated into two patches. like this compatibility for exynos4412 SOC and rename the drv_data Came across this patch from Dongjin Kim [1] who has already attempetd the above suggestion. Hence dropping my patch. Dongjin, Are you still working on this patch [1] as this has not yet been accepted or merged? [1] http://lkml.indiana.edu/hypermail/linux/kernel/1301.3/00450.html -- With warm regards, Sachin -- 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/2] mmc: dw_mmc: exynos: Add compatibility for exynos4412 SoC
Hi Dongjin, On 18 February 2013 14:34, Dongjin Kim tobet...@gmail.com wrote: Hello Sachin, I have one more patch which use 'exynos_drv_data' for its drv_data, but not submitted yet. I will send it soon anyway, anything else do you like to point out on the patch? :) Please verify if the capabilities of Exynos 4412 and 5 are same? Other comments are already provided by Jaehoon and Alim. -- With warm regards, Sachin -- 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/2] mmc: dw_mmc: Make dw_mci_exynos_probe static
Acked-by: Jaehoon Chung jh80.ch...@samsung.com
On 02/18/2013 05:53 PM, Sachin Kamat wrote:
Silences the following sparse warning:
drivers/mmc/host/dw_mmc-exynos.c:218:5: warning:
symbol 'dw_mci_exynos_probe' was not declared. Should it be static?
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/mmc/host/dw_mmc-exynos.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c
b/drivers/mmc/host/dw_mmc-exynos.c
index 72fd0f2..5a09c77 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -215,7 +215,7 @@ static const struct of_device_id dw_mci_exynos_match[] = {
};
MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
-int dw_mci_exynos_probe(struct platform_device *pdev)
+static int dw_mci_exynos_probe(struct platform_device *pdev)
{
const struct dw_mci_drv_data *drv_data;
const struct of_device_id *match;
--
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 1/2] mmc: dw-mmc: check the host-bus_hz before calling
Check whether host-bus_hz is zero or not. before calling drv_data-setup_clock.
Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/mmc/host/dw_mmc.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 60063cc..c59078c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2203,6 +2203,13 @@ int dw_mci_probe(struct dw_mci *host)
else
host-bus_hz = clk_get_rate(host-ciu_clk);
+ if (!host-bus_hz) {
+ dev_err(host-dev,
+ Platform data must supply bus speed\n);
+ ret = -ENODEV;
+ goto err_clk_ciu;
+ }
+
if (drv_data drv_data-setup_clock) {
ret = drv_data-setup_clock(host);
if (ret) {
@@ -2212,13 +2219,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- if (!host-bus_hz) {
- dev_err(host-dev,
- Platform data must supply bus speed\n);
- ret = -ENODEV;
- goto err_clk_ciu;
- }
-
host-quirks = host-pdata-quirks;
spin_lock_init(host-lock);
--
1.7.9.5
--
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] mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
On 10.01.2013 21:22, Tony Lindgren wrote:
* Andreas Fenkart andreas.fenk...@streamunlimited.com [121220 14:15]:
Without functional clock the omap_hsmmc module can't forward
SDIO IRQs to the system. This patch reconfigures dat1 line
as a gpio while the fclk is off. And uses SDIO IRQ detection of
the module, while fclk is present.
Looks pretty good to me, however I could not figure out what
to apply this on for testing. It fails to apply at least against
current linux next, can you please update against that?
+static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+struct omap_hsmmc_host *host = mmc_priv(mmc);
+u32 irq_mask;
+unsigned long flags;
+
+spin_lock_irqsave(host-irq_lock, flags);
+
+host-sdio_irq_en = (enable != 0) ? true : false;
+
+if (host-active_pinmux) {
+irq_mask = OMAP_HSMMC_READ(host-base, ISE);
+if (enable)
+irq_mask |= CIRQ_ENABLE;
+else
+irq_mask = ~CIRQ_ENABLE;
+OMAP_HSMMC_WRITE(host-base, IE, irq_mask);
+
+if (!host-req_in_progress)
+OMAP_HSMMC_WRITE(host-base, ISE, irq_mask);
+
+#if 0
+OMAP_HSMMC_READ(host-base, IE); /* flush posted write */
+#endif
Maybe just replace #if 0 with just a comment in case it turns out to be
needed for some cases?
Is there any update on this series? Andreas, did you do more tests?
Thanks and best regards,
Daniel
--
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 v3] mmc: dw_mmc: read all data in FIFO after Data transfer over interrupt in pio mode
Hi Chris,
Could you merge this patch in your tree.
We've verified.
Thanks,
Seungwon Jeon
On Tuesday, January 22, 2013, Kyoungil Kim wrote:
In dwc manual, the below contents are described.
During end of packet, interrupt is not generated if threshold
programming is larger than any remaining data. It is responsibility
of host to read remaining bytes on seeing Data Transfer Done
interrupt
We also have seen the data cannot be read fully
when sg_miter-length is less than FIFO size.
Signed-off-by: Kyoungil Kim ki0351@samsung.com
Signed-off-by: Seungwon Jeon tgih@samsung.com
Acked-by: Jaehoon Chung jh80.ch...@samsung.com
---
drivers/mmc/host/dw_mmc.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 323c502..41d59da 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1430,7 +1430,7 @@ static void dw_mci_pull_data(struct dw_mci *host, void
*buf, int cnt)
host-pull_data(host, buf, cnt);
}
-static void dw_mci_read_data_pio(struct dw_mci *host)
+static void dw_mci_read_data_pio(struct dw_mci *host, bool dto)
{
struct sg_mapping_iter *sg_miter = host-sg_miter;
void *buf;
@@ -1465,7 +1465,9 @@ static void dw_mci_read_data_pio(struct dw_mci *host)
sg_miter-consumed = offset;
status = mci_readl(host, MINTSTS);
mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
- } while (status SDMMC_INT_RXDR); /*if the RXDR is ready read again*/
+ /* if the RXDR is ready read again */
+ } while ((status SDMMC_INT_RXDR) ||
+ (dto SDMMC_GET_FCNT(mci_readl(host, STATUS;
data-bytes_xfered += nbytes;
if (!remain) {
@@ -1597,7 +1599,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void
*dev_id)
smp_wmb();
if (host-dir_status == DW_MCI_RECV_STATUS) {
if (host-sg != NULL)
- dw_mci_read_data_pio(host);
+ dw_mci_read_data_pio(host, true);
}
set_bit(EVENT_DATA_COMPLETE, host-pending_events);
tasklet_schedule(host-tasklet);
@@ -1606,7 +1608,7 @@ static irqreturn_t dw_mci_interrupt(int irq, void
*dev_id)
if (pending SDMMC_INT_RXDR) {
mci_writel(host, RINTSTS, SDMMC_INT_RXDR);
if (host-dir_status == DW_MCI_RECV_STATUS host-sg)
- dw_mci_read_data_pio(host);
+ dw_mci_read_data_pio(host, false);
}
if (pending SDMMC_INT_TXDR) {
--
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
--
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/2] mmc: dw_mmc: Make dw_mci_exynos_probe static
Hi Sachin,
You're right.
'dw_mci_exynos_probe' is only used here.
Acked-by: Seungwon Jeon tgih@samsung.com
On Monday, February 18, 2013, Jaehoon Chung wrote:
Acked-by: Jaehoon Chung jh80.ch...@samsung.com
On 02/18/2013 05:53 PM, Sachin Kamat wrote:
Silences the following sparse warning:
drivers/mmc/host/dw_mmc-exynos.c:218:5: warning:
symbol 'dw_mci_exynos_probe' was not declared. Should it be static?
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/mmc/host/dw_mmc-exynos.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c
b/drivers/mmc/host/dw_mmc-exynos.c
index 72fd0f2..5a09c77 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -215,7 +215,7 @@ static const struct of_device_id dw_mci_exynos_match[]
= {
};
MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
-int dw_mci_exynos_probe(struct platform_device *pdev)
+static int dw_mci_exynos_probe(struct platform_device *pdev)
{
const struct dw_mci_drv_data *drv_data;
const struct of_device_id *match;
--
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] mmc: omap_hsmmc: Enable SDIO IRQ using a GPIO in idle mode.
Hi,
On Mon, Feb 18, 2013 at 11:26:38AM +0100, Daniel Mack wrote:
On 10.01.2013 21:22, Tony Lindgren wrote:
* Andreas Fenkart andreas.fenk...@streamunlimited.com [121220 14:15]:
Without functional clock the omap_hsmmc module can't forward
SDIO IRQs to the system. This patch reconfigures dat1 line
as a gpio while the fclk is off. And uses SDIO IRQ detection of
the module, while fclk is present.
Looks pretty good to me, however I could not figure out what
to apply this on for testing. It fails to apply at least against
current linux next, can you please update against that?
+static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+ struct omap_hsmmc_host *host = mmc_priv(mmc);
+ u32 irq_mask;
+ unsigned long flags;
+
+ spin_lock_irqsave(host-irq_lock, flags);
+
+ host-sdio_irq_en = (enable != 0) ? true : false;
+
+ if (host-active_pinmux) {
+ irq_mask = OMAP_HSMMC_READ(host-base, ISE);
+ if (enable)
+ irq_mask |= CIRQ_ENABLE;
+ else
+ irq_mask = ~CIRQ_ENABLE;
+ OMAP_HSMMC_WRITE(host-base, IE, irq_mask);
+
+ if (!host-req_in_progress)
+ OMAP_HSMMC_WRITE(host-base, ISE, irq_mask);
+
+#if 0
+ OMAP_HSMMC_READ(host-base, IE); /* flush posted write */
+#endif
Maybe just replace #if 0 with just a comment in case it turns out to be
needed for some cases?
Is there any update on this series? Andreas, did you do more tests?
Thanks for all the feedback so far. Yes I did more testing. After
reducing the autosuspend delay, I found two more bugs.
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -138,7 +138,7 @@ static void apply_clk_hack(void)
#define SOFTRESET (1 1)
#define RESETDONE (1 0)
-#define MMC_AUTOSUSPEND_DELAY 100
+#define MMC_AUTOSUSPEND_DELAY 1
#define MMC_TIMEOUT_MS 20
One is already fixed[1], the other I'm still working on.
Hope to progress this week, then need to redo the testing.
Plan is to resubmit latest next week.
Andi
--
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 v4 00/13] mmc: core and driver DT and related development
Hi Guennadi, On Fri, Feb 15 2013, Guennadi Liakhovetski wrote: This is v4 of a patch-series, extending mmc subsystem device-tree usage and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / tmio_mmc drivers. Changes since v3 are described in respective patches. Thanks to all who commented on v3. Guennadi Liakhovetski (13): mmc: sdhi, tmio: only check flags in tmio-mmc driver proper mmc: detailed definition of CD and WP MMC line polarities in DT mmc: provide a standard MMC device-tree binding parser centrally mmc: (cosmetic) remove extern from function declarations mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings mmc: tmio-mmc: define device-tree bindings mmc: tmio-mmc: parse device-tree bindings mmc: sh_mobile_sdhi: remove unused .pdata field mmc: sh_mobile_sdhi: use managed resource allocations mmc: tmio: remove unused and deprecated symbols mmc: tmio: add support for the VccQ regulator mmc: add DT bindings for more MMC capability flags mmc: tmio: add barriers to IO operations Thanks! Pushed to mmc-next, will think about merging for 3.9. - 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 v4 13/13] mmc: tmio: add barriers to IO operations
On Friday 15 February 2013, Guennadi Liakhovetski wrote:
Without barriers SDIO operations fail with runtime PM enabled.
I don't understand how the changeset comment relates to the patch.
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..a10ebd0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
{
- return readw(host-ctl + (addr host-bus_shift));
+ return ioread16(host-ctl + (addr host-bus_shift));
}
As far as I know, all architectures are required to have the same barrier
semantics on readw and ioread16. The only difference between the two is
that ioread16 must be able top operate on an __iomem token returned by
ioport_map() or pci_iomap, which readw does not have to, but does on ARM.
static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
u16 *buf, int count)
{
- readsw(host-ctl + (addr host-bus_shift), buf, count);
+ wmb();
+ ioread16_rep(host-ctl + (addr host-bus_shift), buf, count);
}
Same thing here: both readsw and ioread16_rep are supposed to do the same
thing, and I would assume that they should also include that barrier.
For some reason I don't understand, they do not have the barrier on
ARM at the moment, but I cannot say whether that is intentional or not.
Maybe Russell can comment on this.
Also, should the barrier not be /after/ the MMIO read, rather than before it?
Typically the barrier should ensure that any read from memory after an
MMIO read reflects the memory contents after any DMA is complete that
the MMIO read has already claimed to be done.
static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
u16 *buf, int count)
{
- writesw(host-ctl + (addr host-bus_shift), buf, count);
+ iowrite16_rep(host-ctl + (addr host-bus_shift), buf, count);
+ wmb();
}
Similarly here: why do you have the wmb after the iowrite rather than before it?
Arnd
--
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: FW: [PATCH v2] mmc: host: sdhci: Fix parameter of sdhci_do_start_signal_voltage_switch()
Kevin Liu wrote: Thanks a lot for finding this and quick fix! My original purpose is to change the argument type from ios to signal_voltage. But the code change for mmc core was not integrated into Johan's final patchset ([PATCH v6 5/5] mmc: core: Fixup signal voltage switch). So I still prefer your first version. I submit below patch for this just now: Chris has already applied my patch, so if you need to make further changes, just send incremental patches on top of it. -- 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 v4 13/13] mmc: tmio: add barriers to IO operations
Hi Arnd
On Mon, 18 Feb 2013, Arnd Bergmann wrote:
On Friday 15 February 2013, Guennadi Liakhovetski wrote:
Without barriers SDIO operations fail with runtime PM enabled.
I don't understand how the changeset comment relates to the patch.
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..a10ebd0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
{
- return readw(host-ctl + (addr host-bus_shift));
+ return ioread16(host-ctl + (addr host-bus_shift));
}
As far as I know, all architectures are required to have the same barrier
semantics on readw and ioread16. The only difference between the two is
that ioread16 must be able top operate on an __iomem token returned by
ioport_map() or pci_iomap, which readw does not have to, but does on ARM.
Indeed, the real difference are the barriers in repeated IO operations.
static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
u16 *buf, int count)
{
- readsw(host-ctl + (addr host-bus_shift), buf, count);
+ wmb();
+ ioread16_rep(host-ctl + (addr host-bus_shift), buf, count);
}
Same thing here: both readsw and ioread16_rep are supposed to do the same
thing, and I would assume that they should also include that barrier.
For some reason I don't understand, they do not have the barrier on
ARM at the moment, but I cannot say whether that is intentional or not.
Maybe Russell can comment on this.
Also, should the barrier not be /after/ the MMIO read, rather than before it?
Typically the barrier should ensure that any read from memory after an
MMIO read reflects the memory contents after any DMA is complete that
the MMIO read has already claimed to be done.
Errors, that I've been observing were happening with no DMA, in pure PIO
mode.
Unfortunately, I don't have a good explanation, why the barriers _have_ to
be there, where I put them. At some point during my testing, I had
printk()s in the code and SDIO worked. Then the classical - remove
printk()s - stops working. Delays didn't halp, but barriers did. The
motivation to put a write barrier before a (repeated) read was to wait for
completion of any write operations before starting a read. And indeed,
normal write operations, like writew() / iowrite16() have a write barrier
_before_ the write. So, isn't it possible, that the last write hasn't
completed yet, while we begin with reading? But reads / writes should,
probably, anyway be serialised on the bus...
It's also possible, that these errors are related to runtime
power-management, which would involve IO to other SoC peripherals. But
they all should also contain barriers, so, this doesn't explain it
immediately either.
Thanks
Guennadi
static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int
addr,
u16 *buf, int count)
{
- writesw(host-ctl + (addr host-bus_shift), buf, count);
+ iowrite16_rep(host-ctl + (addr host-bus_shift), buf, count);
+ wmb();
}
Similarly here: why do you have the wmb after the iowrite rather than before
it?
Arnd
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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 v4 13/13] mmc: tmio: add barriers to IO operations
On Monday 18 February 2013, Guennadi Liakhovetski wrote:
Also, should the barrier not be after the MMIO read, rather than before it?
Typically the barrier should ensure that any read from memory after an
MMIO read reflects the memory contents after any DMA is complete that
the MMIO read has already claimed to be done.
Errors, that I've been observing were happening with no DMA, in pure PIO
mode.
Unfortunately, I don't have a good explanation, why the barriers have to
be there, where I put them. At some point during my testing, I had
printk()s in the code and SDIO worked. Then the classical - remove
printk()s - stops working. Delays didn't halp, but barriers did. The
motivation to put a write barrier before a (repeated) read was to wait for
completion of any write operations before starting a read. And indeed,
normal write operations, like writew() / iowrite16() have a write barrier
before the write. So, isn't it possible, that the last write hasn't
completed yet, while we begin with reading? But reads / writes should,
probably, anyway be serialised on the bus...
What kind of bus is this? All buses I have looked at do serialize reads
and writes to the same address at the minimum, and all sane buses
serialize them when they happen to the same device, but it's harder
to do when you need to serialize e.g. a read with a previous write
to another device or another bus connected to the same device.
Let me try to say what I understand from reading the code: These
accessors are only used on one function, and there is no DMA
involved here. The function does (simplified):
void tmio_mmc_pio_irq(struct page *, void __iomem *io, size_t count, bool read)
{
void *virt;
/* called from interrupt handler, but let's disable interrupts some
more ;) */
local_irq_disable();
/* if highmem, map the page */
virt = kmap_atomic(page);
if (read) {
wmb();
readsw(io, virt, count);
} else {
writesw(io, virt, count);
rmb();
}
kunmap_atomic(virt);
}
I don't think there is any other I/O involved, so the barriers are probably
needed to synchronize with whoever is accessing the page on the other
side of the kmap/kunmap. Did the bug happen on highmem?
The barriers here should probably be outside of the I/O accessors
and in the tmio_mmc_pio_irq() function, but I still can't explain
why they are needed here.
It's also possible, that these errors are related to runtime
power-management, which would involve IO to other SoC peripherals. But
they all should also contain barriers, so, this doesn't explain it
immediately either.
Maybe the runtime-pm code uses __raw_writel or writel_relaxed where it
should be using writel?
Or maybe there some effect with disabling the caches and not flushing
them properly in advance here?
In any case, I would expect a much more specific changeset text for a
patch like this.
Arnd
--
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 v4 13/13] mmc: tmio: add barriers to IO operations
On Mon, 18 Feb 2013, Arnd Bergmann wrote:
On Monday 18 February 2013, Guennadi Liakhovetski wrote:
Also, should the barrier not be after the MMIO read, rather than before
it?
Typically the barrier should ensure that any read from memory after an
MMIO read reflects the memory contents after any DMA is complete that
the MMIO read has already claimed to be done.
Errors, that I've been observing were happening with no DMA, in pure PIO
mode.
Unfortunately, I don't have a good explanation, why the barriers have to
be there, where I put them. At some point during my testing, I had
printk()s in the code and SDIO worked. Then the classical - remove
printk()s - stops working. Delays didn't halp, but barriers did. The
motivation to put a write barrier before a (repeated) read was to wait for
completion of any write operations before starting a read. And indeed,
normal write operations, like writew() / iowrite16() have a write barrier
before the write. So, isn't it possible, that the last write hasn't
completed yet, while we begin with reading? But reads / writes should,
probably, anyway be serialised on the bus...
What kind of bus is this?
Sorry, I'm not sure how best to describe it, and I don't have sufficient
information myself. In any case on a block-diagram of sh73a0 SDHI devices
aren't directly connected to a common super-highway bus, instead they are
on a bus-splitter. One more thing I forgot to mention - this error has
been observed on SMP.
All buses I have looked at do serialize reads
and writes to the same address at the minimum, and all sane buses
serialize them when they happen to the same device, but it's harder
to do when you need to serialize e.g. a read with a previous write
to another device or another bus connected to the same device.
Let me try to say what I understand from reading the code: These
accessors are only used on one function, and there is no DMA
involved here. The function does (simplified):
void tmio_mmc_pio_irq(struct page *, void __iomem *io, size_t count, bool
read)
{
void *virt;
/* called from interrupt handler, but let's disable interrupts some
more ;) */
local_irq_disable();
/* if highmem, map the page */
virt = kmap_atomic(page);
if (read) {
wmb();
readsw(io, virt, count);
} else {
writesw(io, virt, count);
rmb();
}
kunmap_atomic(virt);
}
I don't think there is any other I/O involved, so the barriers are probably
needed to synchronize with whoever is accessing the page on the other
side of the kmap/kunmap. Did the bug happen on highmem?
No, don't think highmem was involved.
The barriers here should probably be outside of the I/O accessors
and in the tmio_mmc_pio_irq() function, but I still can't explain
why they are needed here.
It's also possible, that these errors are related to runtime
power-management, which would involve IO to other SoC peripherals. But
they all should also contain barriers, so, this doesn't explain it
immediately either.
Maybe the runtime-pm code uses __raw_writel or writel_relaxed where it
should be using writel?
I checked clock code under drivers/sh/clk, I don't see any relevant
__raw_* operations there. One I do see, however, GIC code does use
*_relaxed() accessors, but under raw spinlocks... Do those provide
sufficient barriers?
Thanks
Guennadi
Or maybe there some effect with disabling the caches and not flushing
them properly in advance here?
In any case, I would expect a much more specific changeset text for a
patch like this.
Arnd
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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 v3] mmc: dw_mmc: Add MSHC compatible for Exynos4412
This patch adds the compatible string for MSHC controller of Exynos4412, and
share the controller specific properties with Exynos5250 since they have same
features. Its driver data name is changed to exynos_drv_data instead SoC
specific name.
Cc: Jaehoon Chung jh80.ch...@samsung.com
Cc: Sachin Kamat sachin.ka...@linaro.org
Signed-off-by: Dongjin Kim tobet...@gmail.com
---
drivers/mmc/host/dw_mmc-exynos.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..38cd03c 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -199,8 +199,8 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
return 0;
}
-/* Exynos5250 controller specific capabilities */
-static unsigned long exynos5250_dwmmc_caps[4] = {
+/* Exynos4412/Exynos5250 controller specific capabilities */
+static unsigned long exynos_dwmmc_caps[4] = {
MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR |
MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23,
MMC_CAP_CMD23,
@@ -208,8 +208,8 @@ static unsigned long exynos5250_dwmmc_caps[4] = {
MMC_CAP_CMD23,
};
-static const struct dw_mci_drv_data exynos5250_drv_data = {
- .caps = exynos5250_dwmmc_caps,
+static const struct dw_mci_drv_data exynos_drv_data = {
+ .caps = exynos_dwmmc_caps,
.init = dw_mci_exynos_priv_init,
.setup_clock= dw_mci_exynos_setup_clock,
.prepare_command= dw_mci_exynos_prepare_command,
@@ -219,8 +219,10 @@ static const struct dw_mci_drv_data exynos5250_drv_data = {
};
static const struct of_device_id dw_mci_exynos_match[] = {
+ { .compatible = samsung,exynos4412-dw-mshc,
+ .data = exynos_drv_data, },
{ .compatible = samsung,exynos5250-dw-mshc,
- .data = exynos5250_drv_data, },
+ .data = exynos_drv_data, },
{},
};
MODULE_DEVICE_TABLE(of, dw_mci_exynos_match);
--
1.7.10.4
--
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 v4 13/13] mmc: tmio: add barriers to IO operations
On Monday 18 February 2013, Guennadi Liakhovetski wrote: On Mon, 18 Feb 2013, Arnd Bergmann wrote: Sorry, I'm not sure how best to describe it, and I don't have sufficient information myself. In any case on a block-diagram of sh73a0 SDHI devices aren't directly connected to a common super-highway bus, instead they are on a bus-splitter. One more thing I forgot to mention - this error has been observed on SMP. Ah, I guess that explains it. If you have one CPU filling the buffer and another CPU reading it, you need an smp_wmb/wmp_rmb pair. That case can easily happen with tmio interrupt handler in PIO mode. I checked clock code under drivers/sh/clk, I don't see any relevant __raw_* operations there. One I do see, however, GIC code does use *_relaxed() accessors, but under raw spinlocks... Do those provide sufficient barriers? A spinlock by definition contains an SMP barrier, which seems to be what is missing here. I don't know if that is enough here, because the spinlock only needs barriers to protect against other users behind the same spinlock. In architecture independent code, any smp_wmb() on the writer side must be paired with an smp_rmb on the reader. You probably have the sufficient barriers on the side that initiates the I/O because that uses a dmb() in the MMIO instructions, but that is only by accident and not portable. Also, in the uniprocessor case, you have more barriers than you need, which limits the performance. The best solution for both performance and readability would be to use readw_relaxed() and writew_relaxed() and manually add the barriers you actually need, i.e. two smp_rmb()/smp_wmb() in the PIO case, and a wmb() before you start a real DMA to the device and an rmb() after a DMA from the device is complete. Arnd -- 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 v3] mmc: dw_mmc: Add MSHC compatible for Exynos4412
On Monday 18 February 2013, Dongjin Kim wrote: This patch adds the compatible string for MSHC controller of Exynos4412, and share the controller specific properties with Exynos5250 since they have same features. Its driver data name is changed to exynos_drv_data instead SoC specific name. Cc: Jaehoon Chung jh80.ch...@samsung.com Cc: Sachin Kamat sachin.ka...@linaro.org Signed-off-by: Dongjin Kim tobet...@gmail.com If they are completely compatibly, you can just list samsung,exynos4412-dw-mshc in the compatible property of the exynos5250 device tree, along with the other ones like: compatible = samsung,exynos5250-dw-mshc, samsung,exynos4412-dw-mshc, snps,dw-mshc; then you don't need to change the driver at all. Arnd -- 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
