Re: [PATCH v2] mmc: tmio: enable odd number size access
Hi Morimoto-san, On Wed, Sep 10, 2014 at 3:29 AM, Kuninori Morimoto kuninori.morimoto...@gmail.com wrote: + if (is_read) { + extra = sd_ctrl_read16(host, CTL_SD_DATA_PORT); + + *buf8 = (u8)(extra 0xff); + } else { + extra = (u16)(*buf8); + + sd_ctrl_write16(host, CTL_SD_DATA_PORT, extra); The casts to u8 resp. u16 are not needed. You can even avoid introducing the extra variable: if (is_read) *buf8 = sd_ctrl_read16(host, CTL_SD_DATA_PORT) 0xff; else sd_ctrl_write16(host, CTL_SD_DATA_PORT, *buf8); Technically, the 0xff is also not needed, but I'd keep it for clarity. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say programmer or something like that. -- Linus Torvalds -- 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: [GIT PULL 1/2] SOCFPGA updates for 3.18
On 9 September 2014 23:02, Pavel Machek pa...@denx.de wrote: On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote: On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote: Jaehoon Chung (1): ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc This patch is a bad idea. It removes support for two mmc cards on a single controller -- configuration hardware supports and configuration that allows using u-SD card on mcvevk board. Your objection comes too late, and to the wrong patch, since the driver and all other users have already been changed. We had a long discussion about this when the issue first came up, and we could not find any example of dw-mmc actually being used in a scenario with one controller that has multiple devices attached. Well, this is not first time I raised this. 3.17 is not yet out, so we still have chance to fix regressions without major fuss. Apparently every user out there instead uses multiple controller instances instead. Are you sure that the socfpga implementation is an exception from this? Marek Vasut has the hardware. His board apprently has uSD and eMMC, and I believe it has just one controller. I'll try to get details. Just wanted to highlight some of the reasons to why the earlier discussion we have had, came to the conclusion to remove the slot node. 1) The mmc core don't support multiple cards attached to the same host, it never has. Also, I am not aware of any requests that suggested us to add it. Due to obvious reasons from performance perspective it's simply a really bad idea, that's likely also why there haven't been any requests for it. 2) On the host level, the support for handle multiple slots in DT for dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it screwed up configurations. Thus the support for slots have never worked as expected from DT point of view. Kind regards Uffe -- 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: SD-card endurance, wear and crappiness
2014-09-09 11:11 GMT+02:00 Arnd Bergmann a...@arndb.de: On Tuesday 09 September 2014 10:54:51 Johan Rudholm wrote: 2014-09-03 16:24 GMT+02:00 Johan Rudholm jrudh...@gmail.com: Hi all, as you know, NAND flash can be programmed a limited number of times before it reaches end of life, the number of times varies with the NAND technology used, among other things. As far as I can tell from the simplified SD-spec, there is no way of asking the card about how many program/erase cycles it can handle, or how many p/e cycles are left before reaching EOL. Right? I think that is correct. So, if one should want to give the user some kind of early warning that it's time to change SD-cards, is there a way? Also, when a card has reached EOL, is there a way of telling this condition apart from all other error conditions that may arise? As you know, depending on the quality of the card and controller, read timeouts, write timeouts, lockups etc may occur but can usually be fixed with a power cycle. I'm thinking of collecting simple statistics from for instance card/block.c and exposing it via an ioctl or sysfs. The statistics can be gathered and processed by some user space process which can determine if the user needs to be alerted. The statistics can be, for instance: * Writes/reads that timeout, but succeed after a retry * Writes/reads that timeout and never succeeds * Different kinds of errors in the card status * Anything else? Perhaps it's not possible to detect worn out cards this way, but at least it could point out and warn about crappy cards? Any thoughts about this? Have you tried if this works? In my experience, the worn-out cards I have either just fail completely, or they return incorrect data, but I have not looked at this side of the problem much. Do you have cards that sometimes time out but always still return correct data on retry? I have noticed that some cards time out on a multi block read, but then succeeds when single block reads are attempted. I've also experimented with retrying the multi block read instead of falling back to single block reads, and some cards succeed after a number ( 10) of retries. However, these cards have not been close to being worn out. I have almost never seen an SD-card that's died because of wear, at least not under controlled circumstances. So, thanks for sharing your experiences! Maybe the bottom line will be that there is no guaranteed way of detecting that a card is nearing EOL. //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 v3] mmc: tmio: enable odd number size access
From: Kuninori Morimoto kuninori.morimoto...@renesas.com Current tmio is using sd_ctrl_read16/write16_rep() for data transfer. It works if transfer size was even number, but, last 1 byte will be ignored if transfer size was odd number. This patch adds new tmio_mmc_transfer_data() and solve this issue. Tested-by: Shinobu Uehara shinobu.uehara...@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto...@renesas.com --- v2 - v3 - remove cast - remove extra variable drivers/mmc/host/tmio_mmc_pio.c | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c index ff5ff0f..692e578 100644 --- a/drivers/mmc/host/tmio_mmc_pio.c +++ b/drivers/mmc/host/tmio_mmc_pio.c @@ -376,6 +376,40 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host, struct mmc_command return 0; } +static void tmio_mmc_transfer_data(struct tmio_mmc_host *host, + unsigned short *buf, + unsigned int count) +{ + int is_read = host-data-flags MMC_DATA_READ; + u8 *buf8; + + /* +* Transfer the data +*/ + if (is_read) + sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count 1); + else + sd_ctrl_write16_rep(host, CTL_SD_DATA_PORT, buf, count 1); + + /* if count was even number */ + if (!(count 0x1)) + return; + + /* if count was odd number */ + buf8 = (u8 *)(buf + (count 1)); + + /* +* FIXME +* +* driver and this function are assuming that +* it is used as little endian +*/ + if (is_read) + *buf8 = sd_ctrl_read16(host, CTL_SD_DATA_PORT) 0xff; + else + sd_ctrl_write16(host, CTL_SD_DATA_PORT, *buf8); +} + /* * This chip always returns (at least?) as much data as you ask for. * I'm unsure what happens if you ask for less than a block. This should be @@ -408,10 +442,7 @@ static void tmio_mmc_pio_irq(struct tmio_mmc_host *host) count, host-sg_off, data-flags); /* Transfer the data */ - if (data-flags MMC_DATA_READ) - sd_ctrl_read16_rep(host, CTL_SD_DATA_PORT, buf, count 1); - else - sd_ctrl_write16_rep(host, CTL_SD_DATA_PORT, buf, count 1); + tmio_mmc_transfer_data(host, buf, count); host-sg_off += count; -- 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 v3 2/3] mmc: mmci: Add sdio enable mask in variant data
On 22 August 2014 06:54, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: This patch adds sdio enable mask in variant data, SOCs like ST have special bits in datactrl register to enable sdio. Unconditionally setting this bit in this driver breaks other SOCs like Qualcomm which maps this bits to something else, so making this enable bit to come from variant data solves the issue. Originally the issue is detected while testing WLAN ath6kl on Qualcomm APQ8064. Reviewed-by: Linus Walleij linus.wall...@linaro.org Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Thanks! Applied for next! Kind regards Uffe --- drivers/mmc/host/mmci.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 533ad2b..a25759e 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -67,6 +67,7 @@ static unsigned int fmax = 515633; * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register * @blksz_datactrl4: true if Block size is at b4..b16 position in datactrl * register + * @datactrl_mask_sdio: SDIO enable mask in datactrl register * @pwrreg_powerup: power up value for MMCIPOWER register * @f_max: maximum clk frequency supported by the controller. * @signal_direction: input/out direction of bus signals can be indicated @@ -89,6 +90,7 @@ struct variant_data { unsigned intfifohalfsize; unsigned intdata_cmd_enable; unsigned intdatactrl_mask_ddrmode; + unsigned intdatactrl_mask_sdio; boolsdio; boolst_clkdiv; boolblksz_datactrl16; @@ -138,6 +140,7 @@ static struct variant_data variant_u300 = { .clkreg_enable = MCI_ST_U300_HWFCEN, .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS, .datalength_bits= 16, + .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, .sdio = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, @@ -151,6 +154,7 @@ static struct variant_data variant_nomadik = { .fifohalfsize = 8 * 4, .clkreg = MCI_CLK_ENABLE, .datalength_bits= 24, + .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, .sdio = true, .st_clkdiv = true, .pwrreg_powerup = MCI_PWR_ON, @@ -168,6 +172,7 @@ static struct variant_data variant_ux500 = { .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS, .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE, .datalength_bits= 24, + .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, .sdio = true, .st_clkdiv = true, .pwrreg_powerup = MCI_PWR_ON, @@ -187,6 +192,7 @@ static struct variant_data variant_ux500v2 = { .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE, .datactrl_mask_ddrmode = MCI_ST_DPSM_DDRMODE, .datalength_bits= 24, + .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, .sdio = true, .st_clkdiv = true, .blksz_datactrl16 = true, @@ -812,16 +818,10 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) if (data-flags MMC_DATA_READ) datactrl |= MCI_DPSM_DIRECTION; - /* The ST Micro variants has a special bit to enable SDIO */ if (variant-sdio host-mmc-card) if (mmc_card_sdio(host-mmc-card)) { - /* -* The ST Micro variants has a special bit -* to enable SDIO. -*/ u32 clk; - - datactrl |= MCI_ST_DPSM_SDIOEN; + datactrl |= variant-datactrl_mask_sdio; /* * The ST Micro variant for SDIO small write transfers -- 1.9.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 v3 3/3] mmc: mmci: rename sdio flag in vendor data to st_sdio
On 22 August 2014 06:55, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: This patch renames sdio flag in vendor data to st_sdio, as this flag is only used to enable ST specific sdio setup. This will also ensure that the ST specfic setup is not done on other vendor like Qualcomm. Originally the issue was detected while testing WLAN ath6kl on IFC6410 board with APQ8064 SOC. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Thanks! Applied for next! Kind regards Uffe --- drivers/mmc/host/mmci.c | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a25759e..264c947 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -61,7 +61,7 @@ static unsigned int fmax = 515633; * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY * is asserted (likewise for RX) * @data_cmd_enable: enable value for data commands. - * @sdio: variant supports SDIO + * @st_sdio: enable ST specific SDIO logic * @st_clkdiv: true if using a ST-specific clock divider algorithm * @datactrl_mask_ddrmode: ddr mode mask in datactrl register. * @blksz_datactrl16: true if Block size is at b16..b30 position in datactrl register @@ -91,7 +91,7 @@ struct variant_data { unsigned intdata_cmd_enable; unsigned intdatactrl_mask_ddrmode; unsigned intdatactrl_mask_sdio; - boolsdio; + boolst_sdio; boolst_clkdiv; boolblksz_datactrl16; boolblksz_datactrl4; @@ -141,7 +141,7 @@ static struct variant_data variant_u300 = { .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS, .datalength_bits= 16, .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, - .sdio = true, + .st_sdio= true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, .signal_direction = true, @@ -155,7 +155,7 @@ static struct variant_data variant_nomadik = { .clkreg = MCI_CLK_ENABLE, .datalength_bits= 24, .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, - .sdio = true, + .st_sdio= true, .st_clkdiv = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, @@ -173,7 +173,7 @@ static struct variant_data variant_ux500 = { .clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE, .datalength_bits= 24, .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, - .sdio = true, + .st_sdio= true, .st_clkdiv = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, @@ -193,7 +193,7 @@ static struct variant_data variant_ux500v2 = { .datactrl_mask_ddrmode = MCI_ST_DPSM_DDRMODE, .datalength_bits= 24, .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN, - .sdio = true, + .st_sdio= true, .st_clkdiv = true, .blksz_datactrl16 = true, .pwrreg_powerup = MCI_PWR_ON, @@ -818,26 +818,26 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) if (data-flags MMC_DATA_READ) datactrl |= MCI_DPSM_DIRECTION; - if (variant-sdio host-mmc-card) - if (mmc_card_sdio(host-mmc-card)) { - u32 clk; - datactrl |= variant-datactrl_mask_sdio; + if (host-mmc-card mmc_card_sdio(host-mmc-card)) { + u32 clk; - /* -* The ST Micro variant for SDIO small write transfers -* needs to have clock H/W flow control disabled, -* otherwise the transfer will not start. The threshold -* depends on the rate of MCLK. -*/ - if (data-flags MMC_DATA_WRITE - (host-size 8 || -(host-size = 8 host-mclk 5000))) - clk = host-clk_reg ~variant-clkreg_enable; - else - clk = host-clk_reg | variant-clkreg_enable; + datactrl |= variant-datactrl_mask_sdio; - mmci_write_clkreg(host, clk); - } + /* +* The ST Micro variant for SDIO small write transfers +* needs to have clock H/W flow control disabled, +* otherwise
Re: [PATCH v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant
On 22 August 2014 06:54, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: From: Ulf Hansson ulf.hans...@linaro.org For the ux500v2 variant of the PL18x block, any block sizes are supported. This will make it possible to decrease data overhead for SDIO transfers. This patch is based on Ulf Hansson patch http://www.spinics.net/lists/linux-mmc/msg12160.html Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org enabled this support on qcom variant. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org I am not sure how to handle this patch. It will as you say in the cover letter for this patchset, improve situations for the ath6kl driver when it's issuing 12 bytes and 24 bytes reads and solve those issues. On the other hand, as stated earlier mmci_pio_write need to be fixed to have full support for any block size. That applies to the qcom variant as well. For ux500, I am sure this won't cause any regressions since the cw1200 isn't probed. Also, I am not sure the any block size support is even enabled for that driver. How about, that we add a comment in the pio_write function describing that we need to fix it for SDIO any block size support? And leave that as a future improvement? Kind regards Uffe --- drivers/mmc/host/mmci.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c11cb05..533ad2b 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -77,6 +77,7 @@ static unsigned int fmax = 515633; * @qcom_fifo: enables qcom specific fifo pio read logic. * @reversed_irq_handling: handle data irq before cmd irq. * @qcom_dml: enables qcom specific dma glue for dma transfers. + * @any_blksize: true if block any sizes are supported */ struct variant_data { unsigned intclkreg; @@ -102,6 +103,7 @@ struct variant_data { boolqcom_fifo; boolreversed_irq_handling; boolqcom_dml; + boolany_blksize; }; static struct variant_data variant_arm = { @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = { .pwrreg_clkgate = true, .busy_detect= true, .pwrreg_nopower = true, + .any_blksize= true, }; static struct variant_data variant_qcom = { @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = { .explicit_mclk_control = true, .qcom_fifo = true, .qcom_dml = true, + .any_blksize= true, }; static int mmci_card_busy(struct mmc_host *mmc) @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc) static int mmci_validate_data(struct mmci_host *host, struct mmc_data *data) { + struct variant_data *variant = host-variant; + if (!data) return 0; - - if (!is_power_of_2(data-blksz)) { + if (!is_power_of_2(data-blksz) !variant-any_blksize) { dev_err(mmc_dev(host-mmc), unsupported block size (%d bytes)\n, data-blksz); return -EINVAL; @@ -796,7 +801,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) writel(host-size, base + MMCIDATALENGTH); blksz_bits = ffs(data-blksz) - 1; - BUG_ON(1 blksz_bits != data-blksz); if (variant-blksz_datactrl16) datactrl = MCI_DPSM_ENABLE | (data-blksz 16); -- 1.9.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: [GIT PULL 1/2] SOCFPGA updates for 3.18
On Wed 2014-09-10 09:13:27, Ulf Hansson wrote: On 9 September 2014 23:02, Pavel Machek pa...@denx.de wrote: On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote: On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote: Jaehoon Chung (1): ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc This patch is a bad idea. It removes support for two mmc cards on a single controller -- configuration hardware supports and configuration that allows using u-SD card on mcvevk board. Your objection comes too late, and to the wrong patch, since the driver and all other users have already been changed. We had a long discussion about this when the issue first came up, and we could not find any example of dw-mmc actually being used in a scenario with one controller that has multiple devices attached. Well, this is not first time I raised this. 3.17 is not yet out, so we still have chance to fix regressions without major fuss. Apparently every user out there instead uses multiple controller instances instead. Are you sure that the socfpga implementation is an exception from this? Marek Vasut has the hardware. His board apprently has uSD and eMMC, and I believe it has just one controller. I'll try to get details. Just wanted to highlight some of the reasons to why the earlier discussion we have had, came to the conclusion to remove the slot node. 1) The mmc core don't support multiple cards attached to the same host, it never has. Also, I am not aware of any requests that suggested us to add it. Due to obvious reasons from performance perspective it's simply a really bad idea, that's likely also why there haven't been any requests for it. Well, that's Linux problem, right? (And why is it bad idea, btw? The bandwidth will be shared between the controllers, ok, that does not sound too bad.) 2) On the host level, the support for handle multiple slots in DT for dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it screwed up configurations. Thus the support for slots have never worked as expected from DT point of view. Well, DT is supposed to describe the hardware. From your description, it seems that linux does not support two slots on one controller and DT parsing code basically ignores the slots. (Logical, if it can't support two slots). So now we are breaking DT description due to Linux limitations. Which a) makes it hard for any other os not having same limitation b) makes it hard for people to fix the limitation c) does not really solve anything d) breaks backward compatibility with old dts If nothing, d) should be argument not to do this. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v3 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant
Hi Ulf, On 10/09/14 08:58, Ulf Hansson wrote: On 22 August 2014 06:54, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: From: Ulf Hansson ulf.hans...@linaro.org For the ux500v2 variant of the PL18x block, any block sizes are supported. This will make it possible to decrease data overhead for SDIO transfers. This patch is based on Ulf Hansson patch http://www.spinics.net/lists/linux-mmc/msg12160.html Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org enabled this support on qcom variant. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org I am not sure how to handle this patch. It will as you say in the cover letter for this patchset, improve situations for the ath6kl driver when it's issuing 12 bytes and 24 bytes reads and solve those issues. On the other hand, as stated earlier mmci_pio_write need to be fixed to have full support for any block size. That applies to the qcom variant as well. looking at current mmci_pio_write, I see it can support any block sizes as it is. Unless Am missing something obvious. block size aligned to 4 is taken care in the code and is straight forward. block sizes not aligned to 4 are also partly taken care in pio_write and partly by programming blksz in datactrl register. However Am not sure if it was safe to handle the buffer pointer out of its boundary. For ux500, I am sure this won't cause any regressions since the cw1200 isn't probed. Also, I am not sure the any block size support is even enabled for that driver. How about, that we add a comment in the pio_write function describing that we need to fix it for SDIO any block size support? And leave that as a future improvement? Is below patch any good? With the below patch It should be possible to address the case where buffer passed the length is handled safely. If you ok with the approach I can send a patch as RFC. cut here diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 264c947..8480c02 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1134,7 +1134,20 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem * byte become a 32bit write, 7 bytes will be two * 32bit writes etc. */ - iowrite32_rep(base + MMCIFIFO, ptr, (count + 3) 2); + if (unlikely(count 0x3)) { + unsigned char buf[4] = {0, }; + + if (count 4) { + memcpy(buf, ptr, count); + iowrite32_rep(base + MMCIFIFO, buf, 1); + } else { + iowrite32_rep(base + MMCIFIFO, ptr, count 2); + memcpy(buf, ptr + (count ~0x3), count 0x3); + iowrite32_rep(base + MMCIFIFO, buf, 1); + } + } else { + iowrite32_rep(base + MMCIFIFO, ptr, count 2); + } ptr += count; remain -= count; cut here thanks, srini Kind regards Uffe --- drivers/mmc/host/mmci.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c11cb05..533ad2b 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -77,6 +77,7 @@ static unsigned int fmax = 515633; * @qcom_fifo: enables qcom specific fifo pio read logic. * @reversed_irq_handling: handle data irq before cmd irq. * @qcom_dml: enables qcom specific dma glue for dma transfers. + * @any_blksize: true if block any sizes are supported */ struct variant_data { unsigned intclkreg; @@ -102,6 +103,7 @@ struct variant_data { boolqcom_fifo; boolreversed_irq_handling; boolqcom_dml; + boolany_blksize; }; static struct variant_data variant_arm = { @@ -194,6 +196,7 @@ static struct variant_data variant_ux500v2 = { .pwrreg_clkgate = true, .busy_detect= true, .pwrreg_nopower = true, + .any_blksize= true, }; static struct variant_data variant_qcom = { @@ -212,6 +215,7 @@ static struct variant_data variant_qcom = { .explicit_mclk_control = true, .qcom_fifo = true, .qcom_dml = true, + .any_blksize= true, }; static int mmci_card_busy(struct mmc_host *mmc) @@ -239,10 +243,11 @@ static int mmci_card_busy(struct mmc_host *mmc) static int mmci_validate_data(struct mmci_host *host, struct mmc_data *data) { + struct variant_data *variant = host-variant; + if (!data) return 0; - - if (!is_power_of_2(data-blksz)) { +
Re: [GIT PULL 1/2] SOCFPGA updates for 3.18
On 10 September 2014 10:33, Pavel Machek pa...@denx.de wrote: On Wed 2014-09-10 09:13:27, Ulf Hansson wrote: On 9 September 2014 23:02, Pavel Machek pa...@denx.de wrote: On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote: On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote: Jaehoon Chung (1): ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc This patch is a bad idea. It removes support for two mmc cards on a single controller -- configuration hardware supports and configuration that allows using u-SD card on mcvevk board. Your objection comes too late, and to the wrong patch, since the driver and all other users have already been changed. We had a long discussion about this when the issue first came up, and we could not find any example of dw-mmc actually being used in a scenario with one controller that has multiple devices attached. Well, this is not first time I raised this. 3.17 is not yet out, so we still have chance to fix regressions without major fuss. Apparently every user out there instead uses multiple controller instances instead. Are you sure that the socfpga implementation is an exception from this? Marek Vasut has the hardware. His board apprently has uSD and eMMC, and I believe it has just one controller. I'll try to get details. Just wanted to highlight some of the reasons to why the earlier discussion we have had, came to the conclusion to remove the slot node. 1) The mmc core don't support multiple cards attached to the same host, it never has. Also, I am not aware of any requests that suggested us to add it. Due to obvious reasons from performance perspective it's simply a really bad idea, that's likely also why there haven't been any requests for it. Well, that's Linux problem, right? (And why is it bad idea, btw? The bandwidth will be shared between the controllers, ok, that does not sound too bad.) Bandwidth is just one issue. Latency is yet another, which is probably far worse to handle than bandwidth. 2) On the host level, the support for handle multiple slots in DT for dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it screwed up configurations. Thus the support for slots have never worked as expected from DT point of view. Well, DT is supposed to describe the hardware. From your description, it seems that linux does not support two slots on one controller and DT parsing code basically ignores the slots. (Logical, if it can't support two slots). So now we are breaking DT description due to Linux limitations. Which a) makes it hard for any other os not having same limitation b) makes it hard for people to fix the limitation c) does not really solve anything Yes it does, the problem in 2) gets fixed. d) breaks backward compatibility with old dts According to 2), it has never worked - so we don't break anything. If nothing, d) should be argument not to do this. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html Kind regards Uffe -- 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 1/3] mmc: mmci: Support any block sizes for ux500v2 and qcom variant
On 10 September 2014 11:07, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: Hi Ulf, On 10/09/14 08:58, Ulf Hansson wrote: On 22 August 2014 06:54, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: From: Ulf Hansson ulf.hans...@linaro.org For the ux500v2 variant of the PL18x block, any block sizes are supported. This will make it possible to decrease data overhead for SDIO transfers. This patch is based on Ulf Hansson patch http://www.spinics.net/lists/linux-mmc/msg12160.html Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org enabled this support on qcom variant. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Linus Walleij linus.wall...@linaro.org I am not sure how to handle this patch. It will as you say in the cover letter for this patchset, improve situations for the ath6kl driver when it's issuing 12 bytes and 24 bytes reads and solve those issues. On the other hand, as stated earlier mmci_pio_write need to be fixed to have full support for any block size. That applies to the qcom variant as well. looking at current mmci_pio_write, I see it can support any block sizes as it is. Unless Am missing something obvious. block size aligned to 4 is taken care in the code and is straight forward. block sizes not aligned to 4 are also partly taken care in pio_write and partly by programming blksz in datactrl register. However Am not sure if it was safe to handle the buffer pointer out of its boundary. There are some prerequisites of the data buffers to supports any block size. Make sure you read up on this discussion to really understand the problem. http://marc.info/?t=13500506242r=2w=2 For ux500, I am sure this won't cause any regressions since the cw1200 isn't probed. Also, I am not sure the any block size support is even enabled for that driver. How about, that we add a comment in the pio_write function describing that we need to fix it for SDIO any block size support? And leave that as a future improvement? Is below patch any good? With the below patch It should be possible to address the case where buffer passed the length is handled safely. If you ok with the approach I can send a patch as RFC. Please go ahead an send a patch :-) This has been on my todo list forever, I would happily like to remove it from there. Kind regards Uffe -- 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: [GIT PULL 1/2] SOCFPGA updates for 3.18
Hi! 2) On the host level, the support for handle multiple slots in DT for dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it screwed up configurations. Thus the support for slots have never worked as expected from DT point of view. Well, DT is supposed to describe the hardware. From your description, it seems that linux does not support two slots on one controller and DT parsing code basically ignores the slots. (Logical, if it can't support two slots). So now we are breaking DT description due to Linux limitations. Which a) makes it hard for any other os not having same limitation b) makes it hard for people to fix the limitation c) does not really solve anything Yes it does, the problem in 2) gets fixed. d) breaks backward compatibility with old dts According to 2), it has never worked - so we don't break anything. Umm? u-SD worked for me in 3.14, 3.15 and 3.16 with the device tree, and with single-slot described in the device tree. Now I have to change dtb-s to keep working configuration, which is something device tree should never ever do. Is it so hard to just fix the multiple slot parsing? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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: sdhci-pxav3: fix error handling of sdhci_add_host
On 10 September 2014 15:24, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Wednesday 16 July 2014 11:53:21 Laurent Pinchart wrote: On Wednesday 16 July 2014 15:50:09 Xiang Wang wrote: From: Xiang Wang wa...@marvell.com Commit 0dcaa2499b7d111bd70da5b0976c34210c850fb3 improved error handling of sdhci_add_host. However, err_of_parse and err_cd_req should be placed after pm_runtime_disable(pdev-dev). Signed-off-by: Xiang Wang wa...@marvell.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com I can't find the patch in mainline, has it been lost somewhere ? It was lost, but now found, resolved a conflict for it and queued for 3.18. Thanks for the reminder! Kind regards Uffe -- 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: [GIT PULL 1/2] SOCFPGA updates for 3.18
On Tuesday, September 09, 2014 at 11:02:32 PM, Pavel Machek wrote: On Tue 2014-09-09 17:02:34, Arnd Bergmann wrote: On Tuesday 09 September 2014 16:17:56 Pavel Machek wrote: Jaehoon Chung (1): ARM: dts: socfpga: unuse the slot-node and deprecate the supports-highspeed for dw-mmc This patch is a bad idea. It removes support for two mmc cards on a single controller -- configuration hardware supports and configuration that allows using u-SD card on mcvevk board. This is not true, the MCVEVK does _NOT_ have two cards on a single controller. Your objection comes too late, and to the wrong patch, since the driver and all other users have already been changed. We had a long discussion about this when the issue first came up, and we could not find any example of dw-mmc actually being used in a scenario with one controller that has multiple devices attached. Well, this is not first time I raised this. 3.17 is not yet out, so we still have chance to fix regressions without major fuss. Apparently every user out there instead uses multiple controller instances instead. Are you sure that the socfpga implementation is an exception from this? Marek Vasut has the hardware. His board apprently has uSD and eMMC, and I believe it has just one controller. I'll try to get details. So please go ahead with this PR, sorry for blocking it. Best regards, Marek Vasut -- 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: [GIT PULL 1/2] SOCFPGA updates for 3.18
On 9/10/14, 6:00 AM, Pavel Machek wrote: Hi! 2) On the host level, the support for handle multiple slots in DT for dw-mmc has been broken. While dw-mmc parsed the DT nodes for slots, it screwed up configurations. Thus the support for slots have never worked as expected from DT point of view. Well, DT is supposed to describe the hardware. From your description, it seems that linux does not support two slots on one controller and DT parsing code basically ignores the slots. (Logical, if it can't support two slots). So now we are breaking DT description due to Linux limitations. Which a) makes it hard for any other os not having same limitation b) makes it hard for people to fix the limitation c) does not really solve anything Yes it does, the problem in 2) gets fixed. d) breaks backward compatibility with old dts According to 2), it has never worked - so we don't break anything. Umm? u-SD worked for me in 3.14, 3.15 and 3.16 with the device tree, and with single-slot described in the device tree. Now I have to change dtb-s to keep working configuration, which is something device tree should never ever do. I tested the DTB with and without the patch, and it all works fine. Dinh Is it so hard to just fix the multiple slot parsing? Pavel -- 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
3.15+: omap4: mmc: multi_v7: can't boot off mmc
I'm having an hard time making the vanilla v7_defconfig boot off the mmc on my pandaboard: [1.698272] omap_hsmmc 4809c000.mmc: unable to get vmmc regulator -517 [1.705139] platform 4809c000.mmc: Driver omap_hsmmc requests probe deferral [1.712890] omap_hsmmc 480d5000.mmc: unable to get vmmc regulator -517 [1.719787] platform 480d5000.mmc: Driver omap_hsmmc requests probe deferral [1.727691] sdhci-pltfm: SDHCI platform and OF driver helper [1.734619] usbcore: registered new interface driver usbhid [1.740478] usbhid: USB HID core driver [1.749359] TCP: cubic registered [1.752838] NET: Registered protocol family 17 [1.757690] Key type dns_resolver registered [1.762756] Power Management for TI OMAP4+ devices. [1.767883] Power Management for TI OMAP4. [1.772186] OMAP4 PM: u-boot = v2012.07 is required for full PM support [1.779296] ThumbEE CPU extension supported. [1.779296] Registering SWP/SWPB emulation handler [1.790008] vwl1271: 1800 mV [1.794952] Skipping twl internal clock init and using bootloader value (unknown osc rate) [1.805358] twl 0-0048: PIH (irq 39) nested IRQs [1.811340] twl_rtc rtc.14: Power up reset detected. [1.817260] twl_rtc rtc.14: Enabling TWL-RTC [1.824249] twl_rtc rtc.14: rtc core: registered rtc.14 as rtc0 [1.831451] VAUX1_6030: 1000 -- 3000 mV at 1800 mV [1.837677] VAUX2_6030: 1200 -- 2800 mV at 1800 mV [1.844024] VAUX3_6030: 1000 -- 3000 mV at 1200 mV [1.850311] VMMC: 1200 -- 3000 mV at 3000 mV [1.855957] VPP: 1800 -- 2500 mV at 1900 mV [1.861572] VUSIM: 1200 -- 2900 mV at 1800 mV [1.866577] VDAC: 1800 mV [1.869964] VANA: 2100 mV [1.874114] VCXIO: 1800 mV [1.877624] VUSB: 3300 mV [1.881317] V1V8: 1800 mV [1.884979] V2V1: 2100 mV [1.967407] usb 1-1: new high-speed USB device number 2 using ehci-omap [2.094512] omap_i2c 4807.i2c: bus 0 rev0.10 at 400 kHz [2.104583] omap_i2c 48072000.i2c: bus 1 rev0.10 at 400 kHz [2.114044] omap_i2c 4806.i2c: bus 2 rev0.10 at 100 kHz [2.121582] hub 1-1:1.0: USB hub found [2.125854] hub 1-1:1.0: 5 ports detected [2.131744] omap_i2c 4835.i2c: bus 3 rev0.10 at 400 kHz [2.140686] omap_hsmmc 4809c000.mmc: pins are not configured from the driver [2.321166] twl_rtc rtc.14: setting system clock to 2000-01-01 00:00:00 UTC (946684800) [2.336578] ALSA device list: [2.339782] No soundcards found. [2.339782] VFS: Cannot open root device mmcblk0p2 or unknown-block(0,0): error -6 [2.352478] Please append a correct root= boot option; here are the available partitions: [2.361297] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) [2.367980] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.14.0-12143-g552e691 #70 [2.377716] [c02155f4] (unwind_backtrace) from [c0211234] (show_stack+0x10/0x14) [2.385864] [c0211234] (show_stack) from [c080b2c8] (dump_stack+0x88/0x98) [2.390289] [c080b2c8] (dump_stack) from [c0808d10] (panic+0xa0/0x208) [2.400695] [c0808d10] (panic) from [c0ade00c] (mount_block_root+0x1a0/0x230) [2.408569] [c0ade00c] (mount_block_root) from [c0ade1a4] (mount_root+0x108/0x110) [2.408569] [c0ade1a4] (mount_root) from [c0ade304] (prepare_namespace+0x158/0x1a0) [2.425292] [c0ade304] (prepare_namespace) from [c0addcc8] (kernel_init_freeable+0x1cc/0x1dc) [2.430084] [c0addcc8] (kernel_init_freeable) from [c0806184] (kernel_init+0x8/0xf0) [2.443145] [c0806184] (kernel_init) from [c020e338] (ret_from_fork+0x14/0x3c) [2.447967] CPU0: stopping [2.451110] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-12143-g552e691 #70 [2.451110] [c02155f4] (unwind_backtrace) from [c0211234] (show_stack+0x10/0x14) [2.451110] [c0211234] (show_stack) from [c080b2c8] (dump_stack+0x88/0x98) [2.451110] [c080b2c8] (dump_stack) from [c0213e1c] (handle_IPI+0x148/0x174) [2.477386] [c0213e1c] (handle_IPI) from [c0208818] (gic_handle_irq+0x58/0x5c) [2.477386] [c0208818] (gic_handle_irq) from [c0211dc0] (__irq_svc+0x40/0x50) [2.477386] Exception stack(0xc0b6def0 to 0xc0b6df38) [2.506256] dee0: c0b7bf54 c0b7bf54 004c [2.506256] df00: 91be7a33 92080d44 eaf94de8 c0c8f2bc ea4bdd94 [2.523437] df20: 0010 c0b6df38 c06bcbf8 c06bcc04 6153 [2.523437] [c0211dc0] (__irq_svc) from [c06bcc04] (cpuidle_enter_state+0x68/0xf8) [2.523437] [c06bcc04] (cpuidle_enter_state) from [c06be65c] (cpuidle_enter_state_coupled+0x130/0x378) [2.523437] [c06be65c] (cpuidle_enter_state_coupled) from [c0277ed4] (cpu_startup_entry+0x200/0x230) [2.558868] [c0277ed4] (cpu_startup_entry) from [c0addaf0] (start_kernel+0x348/0x354) [2.567474] [c0addaf0] (start_kernel) from [80208074] (0x80208074) [2.567474] ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on
Re: [PATCH] mmc: Add delay between CMD6 and CMD13 for Sandisk eMMC cards
Hi, On 09/09/2014 09:26 PM, Jean-Michel Hautbois wrote: Tested on a i.MX6 board, with Sandisk SDIN5D1-2G. Without this patch, I/O errors occur. This eMMC seems to have a different Manufacturer ID as it reads 0x45 and not 0x2 as specified in datasheet. I think this patch don't merge into mainline. This is not solution for problem. you mentioned the below comment, this is workaround. Signed-off-by: Jean-Michel Hautbois jean-michel.hautb...@vodalys.com --- drivers/mmc/core/mmc_ops.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index f51b5ba..91babaa 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -458,6 +458,15 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value, if (!use_busy_signal) return 0; + /* WORKAROUND: for Sandisk eMMC cards, it might need certain delay + * before sending CMD13 after CMD6 + * On SDIN5D1-2G MANFID is 0x45 and not 0x2 as specified in datasheet + */ + if (card-cid.manfid == CID_MANFID_SANDISK || + card-cid.manfid == 0x45) { + msleep(1); + } If it's a general problem of Sandisk SDIN5D1-2G, I think you need to verify this problem. And can you use the MMC_FIXUP() and QUIRK? Best Regards, Jaehoon Chung + /* * CRC errors shall only be ignored in cases were CMD13 is used to poll * to detect busy completion. -- 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: dw_mmc: add support for ARM64
Hi, Alim. On 09/11/2014 11:35 AM, Alim Akhtar wrote: Hi Jaehoon, On Sep 11, 2014 11:18 AM, Jaehoon Chung jh80.ch...@samsung.com mailto:jh80.ch...@samsung.com wrote: On 09/09/2014 08:31 PM, Alim Akhtar wrote: Hi Ulf, On Tue, Sep 9, 2014 at 12:21 PM, Ulf Hansson ulf.hans...@linaro.org mailto:ulf.hans...@linaro.org wrote: On 29 August 2014 12:24, Alim Akhtar alim.akh...@samsung.com mailto:alim.akh...@samsung.com wrote: There are upcoming ARM64 SoCs with dw_mmc host controller. Signed-off-by: Alim Akhtar alim.akh...@samsung.com mailto:alim.akh...@samsung.com --- drivers/mmc/host/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index a43295c..72dd6c2 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -580,7 +580,7 @@ config SDH_BFIN_MISSING_CMD_PULLUP_WORKAROUND config MMC_DW tristate Synopsys DesignWare Memory Card Interface depends on HAS_DMA - depends on ARC || ARM || MIPS || COMPILE_TEST + depends on ARC || ARM || ARM64 || MIPS || COMPILE_TEST Before enabling this, wouldn't it be better to add the needed support in dw_mmc first? Or there are no changes needed? There are also ongoing development and discussions for exynos7, which I guess relates to this? http://www.spinics.net/lists/linux-mmc/msg28294.html Thanks for looking into this. Yes, this patch is being tested on exynos7 platform which is ongoing development. The above link is one of them. I feel this patch is too generic in nature and we now known at least exynos7 is going to use this. I don't know if there are any other ARM64 platform with dw_mmc controller. If we need to prepare the ARM64, the above patch(linked patch) has to merge, why not? I am not in hurry to merge this now, as I said, its upto the maintainers to decide when to take this and I am ok with that. But it needs to test for other SoCs and rework it. What rework you are anticipating here? Sorry for confusing you. (I didn't see the above URL.) You need to use this patch (http://www.spinics.net/lists/linux-mmc/msg21742.html), right? Then need to rework it. Best Regards, Jaehoon Chung Thanks. Best Regards, Jaehoon Chung So, I am going to leave this upto you to decide when to take this patch, incase you decided to take it after exynos7 support lands, I will send a reminder for this. Kind regards Uffe -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org mailto: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