Re: [PATCH v5] powerpc/esdhc: disable CMD23 for some Freescale SoCs
Sorry for the late reply, Huang.
On Tue, Oct 09, 2012 at 06:24:13AM +, Huang Changming-R66093 wrote:
[...]
+static void esdhc_of_detect_limitation(struct platform_device *pdev,
+ struct sdhci_pltfm_data *pdata) {
Wrong indentation. Should be one more tab, at least (or align to opening
brace).
+ void __iomem *ioaddr;
+ struct resource *iomem;
+ u32 vvn;
+
+ iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!iomem) {
+ dev_warn(pdev-dev, failed to get resource\n);
+ goto end;
+ }
+ if (resource_size(iomem) 0x100)
+ dev_warn(pdev-dev, Invalid iomem size!\n);
+
+ ioaddr = ioremap(iomem-start, resource_size(iomem));
+ if (!ioaddr) {
+ dev_warn(pdev-dev, failed to remap registers\n);
+ goto end;
+ }
+
+ /* P102x and P4080 has IP version VVN2.2, CMD23 is not
supported */
+ vvn = in_be32(ioaddr + SDHCI_SLOT_INT_STATUS);
+ vvn = (vvn SDHCI_VENDOR_VER_MASK) SDHCI_VENDOR_VER_SHIFT;
+ if (vvn == VENDOR_V_22)
+ pdata-quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
+
+ iounmap(ioaddr);
+end:
+ return;
No need for the 'end' label.
+}
+
static struct sdhci_ops sdhci_esdhc_ops = {
.read_l = esdhc_readl,
.read_w = esdhc_readw,
@@ -199,6 +231,7 @@ static struct sdhci_pltfm_data sdhci_esdhc_pdata =
{
static int __devinit sdhci_esdhc_probe(struct platform_device *pdev)
{
+ esdhc_of_detect_limitation(pdev, sdhci_esdhc_pdata);
I would rather prefer it to be in sdhci_ops (i.e. introduce
sdhci_ops-platform_init), so that way you wouldn't need to ioremap()
stuff by yourself. And make drivers/mmc/host/sdhci-pltfm.c call
platform_init after ioremap().
Then your detect_limitation() function would only need to check revision
and set additional quirks.
Thanks,
Anton.
--
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 v1] mmc: fix async request mechanism for sequential read scenarios
Hello Per, On Mon, October 22, 2012 1:02 am, Per Forlin wrote: When mmcqt reports on completion of a request there should be a context switch to allow the insertion of the next read ahead BIOs to the block layer. Since the mmcqd tries to fetch another request immediately after the completion of the previous request it gets NULL and starts waiting for the completion of the previous request. This wait on completion gives the FS the opportunity to insert the next request but the MMC layer is already blocked on the previous request completion and is not aware of the new request waiting to be fetched. I thought that I could trigger a context switch in order to give execution time for FS to add the new request to the MMC queue. I made a simple hack to call yield() in case the request gets NULL. I thought it may give the FS layer enough time to add a new request to the MMC queue. This would not delay the MMC transfer since the yield() is done in parallel with an ongoing transfer. Anyway it was just meant to be a simple test. One yield was not enough. Just for sanity check I added a msleep as well and that was enough to let FS add a new request, Would it be possible to gain throughput by delaying the fetch of new request? Too avoid unnecessary NULL requests If (ongoing request is read AND size is max read ahead AND new request is NULL) yield(); BR Per We did the same experiment and it will not give maximum possible performance. There is no guarantee that the context switch which was manually caused by the MMC layer comes just in time: when it was early then next fetch still results in NULL, when it was later, then we miss possibility to fetch/prepare new request. Any delay in fetch of the new request that comes after the new request has arrived hits throughput and latency. The solution we are talking about here will fix not only situation with FS read ahead mechanism, but also it will remove penalty of the MMC context waiting on completion while any new request arrives. Thanks, -- Konstantin Dorfman, QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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/2] mmc: core: Power cycle card on voltage switch fail
Hi Chris and Daniel,
Chris: Thank you for commenting on this patch! Yes, the sdhci power
cycle code needs to be removed just as Daniel's patch does, and as I
suggested in another patch: [RFC] mmc: sdhci: Let core handle UHS
switch failure (https://patchwork.kernel.org/patch/1517211/).
Daniel: Thank you for testing the patch! Comments below.
2012/10/24 Daniel Drake d...@laptop.org:
On Tue, Oct 23, 2012 at 2:39 PM, Chris Ball c...@laptop.org wrote:
Dan, maybe you could see if this patch (there's a 1/2 patch too) solves
your UHS problem?
I tested [RFC/PATCH] mmc: core: Fixup signal voltage switch
https://patchwork.kernel.org/patch/1514691/
This was previously failing on both XO-1.75 (kernel 3.0) and XO-4
(kernel 3.5) - the 1.8V switch would fail, but it would not
successfully switch back to 3.3V.
Can you enlighten me a bit; what is XO? :) Host controller hardware?
Testing on XO-4, it worked fine: the 1.8V failed switch was detected,
it came back as 3.3V and everything was fine.
Ok, so since you didn't have the card_busy function at this point, the
failure was detected by the sdhci code, right? It power-cycles the
card, returns -EAGAIN, the new code in mmc_sd_get_cid will try 10
times and then come back to 3.3V. Is this what happened, did you get
this print?
pr_warning(%s: Skipping voltage switch\n,
mmc_hostname(host));
Testing on XO-1.75, it didn't work: it thought that the 1.8V switch
was successful so it left it at that, then the card reacted in a very
unstable manner (failed/retried reads, huge amount of kernel log spam,
etc).
This points to that the way of checking if the card is busy or not may
not work on XO-1.75? But then it seems strange that the card was
semi-usable...
So I came up with the attached sdhci patch. That fixes the XO-1.75
case, which now correctly detects the 1.8V switch failure and goes to
3.3V. However, that broke XO-4, which now just does:
mmc2: Signal voltage switch failed, power cycling card (retries = 10)
mmc2: error -110 whilst initialising SD card
(no more time to debug exactly whats happening)
Ok, so failure is detected in mmc_set_signal_voltage by your card_busy
function, good. Then the card is power cycled, and should be
initialized again, but this fails, it's probably mmc_send_app_op_cond
which returns -110. Maybe the power cycle fails?
So the strange thing here is that on kernel 3.5, without my patch,
1.8V switch fails and fall-back to 3.3V fails. With my patch, 1.8V
fails and fall-back to 3.3V succeeds. But, when we move the detection
and error-handling to my patch (the upper layer), the fall-back fails.
I'm thinking of a couple of things that could have gone wrong here...
If you used v2 of the patch, is assumes that the signal voltage is
reset to 3.30 V in mmc_power_up, but this was introduced quite
recently, in v3.6 (mmc: core: reset signal voltage on power up), but
you used v1, right? So then there are at least two other differences:
1) The way the card is power cycled.
sdhci toggles the SDHCI_POWER_ON bit in SDHCI_POWER_CONTROL, but
my patch calls mmc_power_off / mmc_power_up.
Do you know if mmc_power_off / up is a good way to power-cycle the card?
2) The way the clock is stopped.
sdhci clears SDHCI_CLOCK_CARD_EN in SDHCI_CLOCK_CONTROL, my patch
sets ios.clock = 0 and calls mmc_set_ios.
But maybe this is not the issue, since the 1.8V switch fails in all cases.
I don't really know how to proceed. Do you have the complete dmesgs
from the 3.5 kernel: with my patch and without your patch + with my
patch and with your patch? With these I could try to do a deeper
analysis.
By the way, in your patch
0001-update-sdhci-for-voltage-changing-fixes.txt you don't check if
the signal voltage switch succeeds electrically:
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
if (ctrl SDHCI_CTRL_VDD_180) {
but maybe we can assume that this will be fine after the delay of 5 ms
in mmc_set_signal_voltage? Also in sdhci_card_busy you don't do
runtime_pm_get before you check the busy status, but since the busy
check returns busy, perhaps this is not the issue here either.
All tests with the same 32GB SD storage card.
I wonder if this difference in behaviour is related to the difference
in kernel versions on each platform (3.0 vs 3.5). I would like to test
with 3.5 or newer running on both, but this requires a bit of setup
work for the XO-1.75 first (long story). And I'm out of time at this
point :( this was a borrowed SD card.
Ok, I hope you'll be able to get time and SD-card in the future. :)
Btw, what brand was the SD-card?
Kind regards, Johan
--
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/1] mmc: Standardise capability type
There are discrepancies with regards to how MMC capabilities
are carried throughout the subsystem. Let's standardise them
to elevate any confusion.
Cc: Chris Ball c...@laptop.org
Cc: linux-mmc@vger.kernel.org
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
drivers/mmc/core/mmc.c |2 +-
include/linux/mmc/dw_mmc.h |4 ++--
include/linux/mmc/host.h |2 +-
include/linux/mmc/sdhci.h |4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 7cc4638..f31bf80 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -239,7 +239,7 @@ static void mmc_select_card_type(struct mmc_card *card)
{
struct mmc_host *host = card-host;
u8 card_type = card-ext_csd.raw_card_type EXT_CSD_CARD_TYPE_MASK;
- unsigned int caps = host-caps, caps2 = host-caps2;
+ unsigned long caps = host-caps, caps2 = host-caps2;
unsigned int hs_max_dtr = 0;
if (card_type EXT_CSD_CARD_TYPE_26)
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 7c6a113..60b71ae 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -229,8 +229,8 @@ struct dw_mci_board {
u32 quirks; /* Workaround / Quirk flags */
unsigned int bus_hz; /* Clock speed at the cclk_in pad */
- unsigned int caps; /* Capabilities */
- unsigned int caps2; /* More capabilities */
+ unsigned long caps; /* Capabilities */
+ unsigned long caps2;/* More capabilities */
/*
* Override fifo depth. If 0, autodetect it from the FIFOTH register,
* but note that this may not be reliable after a bootloader has used
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 7abb0e1..f89c968 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -241,7 +241,7 @@ struct mmc_host {
#define MMC_CAP_CMD23 (1 30) /* CMD23 supported. */
#define MMC_CAP_HW_RESET (1 31) /* Hardware reset */
- unsigned intcaps2; /* More host capabilities */
+ unsigned long caps2; /* More host capabilities */
#define MMC_CAP2_BOOTPART_NOACC(1 0)/* Boot partition no
access */
#define MMC_CAP2_CACHE_CTRL(1 1)/* Allow cache control */
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index fa8529a..8c20910 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -157,8 +157,8 @@ struct sdhci_host {
struct timer_list timer;/* Timer for timeouts */
- unsigned int caps; /* Alternative CAPABILITY_0 */
- unsigned int caps1; /* Alternative CAPABILITY_1 */
+ unsigned long caps; /* Alternative CAPABILITY_0 */
+ unsigned long caps1;/* Alternative CAPABILITY_1 */
unsigned intocr_avail_sdio; /* OCR bit masks */
unsigned intocr_avail_sd;
--
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 1/1] mmc: Standardise capability type
On Wednesday 24 October 2012, Lee Jones wrote: There are discrepancies with regards to how MMC capabilities are carried throughout the subsystem. Let's standardise them to elevate any confusion. Cc: Chris Ball c...@laptop.org Cc: linux-mmc@vger.kernel.org Signed-off-by: Lee Jones lee.jo...@linaro.org Why make it unsigned long then? I think that adds to the confusion because it's sometimes 32 bits and sometimes 64 bits, depending on the CPU. Since it's a bitmask, I would suggest using u32 to make the size explicit. 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 1/1] mmc: Standardise capability type
On Wed, 24 Oct 2012, Arnd Bergmann wrote: On Wednesday 24 October 2012, Lee Jones wrote: There are discrepancies with regards to how MMC capabilities are carried throughout the subsystem. Let's standardise them to elevate any confusion. Cc: Chris Ball c...@laptop.org Cc: linux-mmc@vger.kernel.org Signed-off-by: Lee Jones lee.jo...@linaro.org Why make it unsigned long then? I think that adds to the confusion because it's sometimes 32 bits and sometimes 64 bits, depending on the CPU. Since it's a bitmask, I would suggest using u32 to make the size explicit. I'm not sure that it leaves any confusion. It perhaps wastes a little space on 64bit architectures, but that also applies to a great deal of other bitmasks floating around. I can do it if you feel that passionate about it, but it's a bigger job to hunt down all occurrences and change them over. I only felt strongly enough about it to craft this patch because I noticed the inconsistency as I created new populate caps for OF functionality. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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 v1] mmc: fix async request mechanism for sequential read scenarios
On 10/24/2012 11:41 AM, Konstantin Dorfman wrote: Hello Per, On Mon, October 22, 2012 1:02 am, Per Forlin wrote: When mmcqt reports on completion of a request there should be a context switch to allow the insertion of the next read ahead BIOs to the block layer. Since the mmcqd tries to fetch another request immediately after the completion of the previous request it gets NULL and starts waiting for the completion of the previous request. This wait on completion gives the FS the opportunity to insert the next request but the MMC layer is already blocked on the previous request completion and is not aware of the new request waiting to be fetched. I thought that I could trigger a context switch in order to give execution time for FS to add the new request to the MMC queue. I made a simple hack to call yield() in case the request gets NULL. I thought it may give the FS layer enough time to add a new request to the MMC queue. This would not delay the MMC transfer since the yield() is done in parallel with an ongoing transfer. Anyway it was just meant to be a simple test. One yield was not enough. Just for sanity check I added a msleep as well and that was enough to let FS add a new request, Would it be possible to gain throughput by delaying the fetch of new request? Too avoid unnecessary NULL requests If (ongoing request is read AND size is max read ahead AND new request is NULL) yield(); BR Per We did the same experiment and it will not give maximum possible performance. There is no guarantee that the context switch which was manually caused by the MMC layer comes just in time: when it was early then next fetch still results in NULL, when it was later, then we miss possibility to fetch/prepare new request. Any delay in fetch of the new request that comes after the new request has arrived hits throughput and latency. The solution we are talking about here will fix not only situation with FS read ahead mechanism, but also it will remove penalty of the MMC context waiting on completion while any new request arrives. Thanks, It seems strange that the block layer cannot keep up with relatively slow flash media devices. There must be a limitation on number of outstanding request towards MMC. I need to make up my mind if it's the best way to address this issue in the MMC framework or block layer. I have started to look into the block layer code but it will take some time to dig out the relevant parts. BR Per -- 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
