RE: [PATCH v3 1/4] mmc: dw-mmc: relocate the position called dw_mci_setup_bus()

2012-11-26 Thread Seungwon Jeon 
On Thursday, November 08, 2012, Jaehoon Chung  wrote:
> To ensure the stable clock need to enable before set the 
> DW_MMC_CARD_NEED_INIT flag.
> If set DW_MMC_CARD_NEED_INIT flag, wait for 80-clock before first command 
> after power-up.
As we have discussed, the location of 'dw_mci_setup_bus' seems proper in 
'dw_mci_set_ios'.
But commit message doesn't really hit me. Origin code can also ensure the same.
Mr. Newton, do you have any opinion?

Thanks,
Seungwon Jeon
> 
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Kyungmin Park 
> Acked-by: Seungwon Jeon 
> ---
> Changelog v2-v3:
>   - Modified the commit-message

>  drivers/mmc/host/dw_mmc.c |6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> ---
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index c0667c8..a1369aa 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -683,9 +683,6 @@ static void __dw_mci_start_request(struct dw_mci *host,
>   if (host->pdata->select_slot)
>   host->pdata->select_slot(slot->id);
> 
> - /* Slot specific timing and width adjustment */
> - dw_mci_setup_bus(slot);
> -
>   host->cur_slot = slot;
>   host->mrq = mrq;
> 
> @@ -812,6 +809,9 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
> mmc_ios *ios)
>   if (drv_data && drv_data->set_ios)
>   drv_data->set_ios(slot->host, ios);
> 
> + /* Slot specific timing and width adjustment */
> + dw_mci_setup_bus(slot);
> +
>   switch (ios->power_mode) {
>   case MMC_POWER_UP:
>   set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
> --
> 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: linux-next: Tree for Nov 26 (mmc/host/tifm_sd.c)

2012-11-26 Thread Randy Dunlap 
On 11/26/2012 06:17 AM, Stephen Rothwell wrote:

> Hi all,
> 
> Changes since 20121115:
> 


on i386:

drivers/built-in.o: In function `tifm_sd_request':
tifm_sd.c:(.text+0x588f95): undefined reference to `__udivdi3'


somewhere inside tifm_sd_request():

I'm guessing that the call to tifm_sd_set_data_timeout() was
inlined by gcc and it's the divide there that is the problem.

-- 

~Randy
--
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: Added quirks for Ricoh 1180:e823 lower base clock frequency

2012-11-26 Thread Chris Ball 
Hi Manoj,

On Mon, Nov 26 2012, Manoj Iyer wrote:
> Do you know what systems produce these errors? I can look and see if
> we have those for testing your patch.

One T430s using the ExpressCard reader¹, and one X220, both with :e823.

Thanks,

- Chris.

¹: http://support.lenovo.com/en_US/product-and-parts/detail.page?&DocID=PD023556
-- 
Chris Ball  
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] mmc: Added quirks for Ricoh 1180:e823 lower base clock frequency

2012-11-26 Thread Manoj Iyer 


Chris,

Do you know what systems produce these errors? I can look and see if we 
have those for testing your patch.


Cheers
Manoj

On Wed, 21 Nov 2012, Chris Ball wrote:


Hi Manoj, Matsumuro-san,

On Mon, Jul 18 2011, Manoj Iyer wrote:

Right, without the patch I get..

[   52.526665] mmc0: new SDHC card at address e624
[   52.571228] mmcblk0: mmc0:e624 SD16G 14.8 GiB
[   52.591071] mmcblk0: retrying using single block read
[   52.593105] mmcblk0: error -84 transferring data, sector 0, nr 8,
card status 0x900
[   52.593109] end_request: I/O error, dev mmcblk0, sector 0
[   52.594594] mmcblk0: error -84 transferring data, sector 1, nr 7,
card status 0x900
[   52.594604] end_request: I/O error, dev mmcblk0, sector 1
[   52.602893] quiet_error: 24 callbacks suppressed
[   52.602902] Buffer I/O error on device mmcblk0, logical block 0
[   52.605349] ldm_validate_partition_table(): Disk read failed.
[   52.605384] Dev mmcblk0: unable to read RDB block 0
[   52.607729]  mmcblk0: unable to read partition table
u@u:~$

So, I cannot generate any comparison data with this SD card.


I don't know if you remember this bug, but I have reports from two users
who see "error -84"s on :e823 controllers even with this patch active on
modern ThinkPads.

Maybe 50MHz isn't low enough sometimes?  Is it possible to lower the base
clock further?

I noticed that the frequency is encoded ine one of the PCI writes:
+* 0x32  - 50Mhz new clock frequency

So if I get access to one of these systems, I might try lowering that
value (by half, say) and see what happens.  But at the moment I haven't
found someone who's willing to try kernel patches yet, and I don't have
one of these systems myself.

Thanks!

- Chris.
--
Chris Ball  
One Laptop Per Child




--

Manoj Iyer
Ubuntu/Canonical
Hardware Enablement

--
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: sdhci-spear: Don't call clk_{un}prepare() in suspend resume

2012-11-26 Thread Chris Ball 
Hi,

On Thu, Nov 08 2012, Viresh Kumar wrote:
> clk_{un}prepare is mandatory for platforms using common clock framework. 
> Because
> for SPEAr we don't do anything in clk_{un}prepare() calls, just call them ones
> in probe/remove.
>
> Signed-off-by: Viresh Kumar 
> ---
>  drivers/mmc/host/sdhci-spear.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> index fea8bf9..87a7009 100644
> --- a/drivers/mmc/host/sdhci-spear.c
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -302,7 +302,7 @@ static int sdhci_suspend(struct device *dev)
>  
>   ret = sdhci_suspend_host(host);
>   if (!ret)
> - clk_disable_unprepare(sdhci->clk);
> + clk_disable(sdhci->clk);
>  
>   return ret;
>  }
> @@ -313,7 +313,7 @@ static int sdhci_resume(struct device *dev)
>   struct spear_sdhci *sdhci = dev_get_platdata(dev);
>   int ret;
>  
> - ret = clk_prepare_enable(sdhci->clk);
> + ret = clk_enable(sdhci->clk);
>   if (ret) {
>   dev_dbg(dev, "Resume: Error enabling clock\n");
>   return ret;

Thanks, pushed to mmc-next for 3.8.

- Chris.
-- 
Chris Ball  
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 1/2] mmc: sdhci-spear: Initialize sdhci clk to 50 MHz

2012-11-26 Thread Chris Ball 
Hi,

On Thu, Nov 08 2012, Viresh Kumar wrote:
> From: Vipul Kumar Samar 
>
> SPEAr sdhci driver expects the clock to be set to 50 MHz for proper 
> functioning.
> This patch sets clk to 50 MHz in probe.
>
> Signed-off-by: Vipul Kumar Samar 
> Signed-off-by: Viresh Kumar 
> ---
>  drivers/mmc/host/sdhci-spear.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> index 6be89c0..fea8bf9 100644
> --- a/drivers/mmc/host/sdhci-spear.c
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -146,6 +146,11 @@ static int __devinit sdhci_probe(struct platform_device 
> *pdev)
>   goto put_clk;
>   }
>  
> + ret = clk_set_rate(sdhci->clk, 5000);
> + if (ret)
> + dev_dbg(&pdev->dev, "Error setting desired clk, clk=%lu\n",
> + clk_get_rate(sdhci->clk));
> +
>   if (np) {
>   sdhci->data = sdhci_probe_config_dt(pdev);
>   if (IS_ERR(sdhci->data)) {

Thanks, pushed to mmc-next for 3.8.

- Chris.
-- 
Chris Ball  
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 v1] mmc: fix async request mechanism for sequential read scenarios

2012-11-26 Thread Konstantin Dorfman 

Hello,

On 11/19/2012 11:34 PM, Per Förlin wrote:

On 11/19/2012 03:32 PM, Per Förlin wrote:

On 11/19/2012 10:48 AM, Konstantin Dorfman wrote:


I'm fine with wait_event() (block request transfers) combined with completion 
(for other transfer), as long as the core.c API is intact. I don't see a reason 
for a new start_data_req()

mmc_start_req() is only used by the mmc block transfers.
Making a test case in mmc_test.c is a good way to stress test the feature (e.g. 
random timing of incoming new requests) and it will show how the API works too.

BR
Per

Right now there are couple of tests in mmc_test module that use async 
request mechanism. After applying my fix (v3 mmc: fix async request 
mechanism for sequential read scenarios), these test were broken.


The patch attached fixes those broken tests and also you can see exactly 
what is API change. It is not in mmc_start_req() signature, it is new 
context_info field that we need to synchronize with NEW_REQUEST 
notification. mmc_test is not uses the notification, but it is very easy 
to implement.




My main concern is to avoid adding new API to core.c in order to add the 
NEW_REQ feature. My patch is an example of changes to be done in core.c without 
adding new functions.

Now you can review API changes.





We would like to have a generic capability to handle additional events,
such as HPI/stop flow, in addition to the NEW_REQUEST notification.
Therefore, an event mechanism seems to be a better design than completion.

I've looked at SDIO code and from what I can understand, right now SDIO
is not using async request mechanism and works from 'wait_for_cmd()`
level. This means that such work as exposing MMC core API's is major
change and definitely should be separate design and implementation
effort, while my current patch right now will fix mmc thread behavior
and give better performance for upper layers.

You are right. There is no support in the SDIO implementation for pre/post/async feature. 
Still I had it in mind design the "struct mmc_async". It's possible to re-use 
the mmc core parts in order to utilize this support in the SDIO case. I'm not saying we 
should design for SDIO but at least keep the API in a way that it's potential usable from 
SDIO too. The API of core.c (start/wait of requests) should make sense without the 
existence of MMC block driver (block/queue).

One way to test the design is to add a test in mmc_test.c for penging NEW_REQ. 
In mmc_test.c there is not block.c or queue.c.
If the test case if simple to write in mmc_test.c it's means that the API is on 
a good level.
I can simulate single NEW_PACKET notification in the mmc_test, but this 
will check only correctness, without real queue of requests we can't 
see/measure tpt/latency gain of this.


Kind reminder about patch v3, waiting for your review.

Thanks

--
Konstantin Dorfman,
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center,
Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
>From 240fa5757e9c784679b391ba42ec58e70fe856d9 Mon Sep 17 00:00:00 2001
From: Konstantin Dorfman 
Date: Mon, 26 Nov 2012 11:50:35 +0200
Subject: [RFC/PATCH] mmc_test: fix non-blocking req test to support mmc core
 wait_event mechanism

After introduce new wait_event synchronization mechanism at mmc/core/core.c level,
non-blocking request tests from mmc_test ut module are broken.

This patch fixes the tests by updating test environment to work
correctly on top of new wait_event mechanism.

Signed-off-by: Konstantin Dorfman 

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 759714e..764471f 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -764,7 +764,8 @@ static int mmc_test_check_broken_result(struct mmc_test_card *test,
 static void mmc_test_nonblock_reset(struct mmc_request *mrq,
 struct mmc_command *cmd,
 struct mmc_command *stop,
-struct mmc_data *data)
+struct mmc_data *data,
+struct mmc_context_info *context_info)
 {
 	memset(mrq, 0, sizeof(struct mmc_request));
 	memset(cmd, 0, sizeof(struct mmc_command));
@@ -774,6 +775,7 @@ static void mmc_test_nonblock_reset(struct mmc_request *mrq,
 	mrq->cmd = cmd;
 	mrq->data = data;
 	mrq->stop = stop;
+	mrq->context_info = context_info;
 }
 static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
   struct scatterlist *sg, unsigned sg_len,
@@ -790,6 +792,8 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
 	struct mmc_command stop2;
 	struct mmc_data data2;
 
+	struct mmc_context_info context_info;
+
 	struct mmc_test_async_req test_areq[2];
 	struct mmc_async_req *done_areq;
 	struct mmc_async_req *cur_areq = &test_areq[0].areq;
@@ -800,14 +804,18 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
 	test_areq[0].test = test;
 	test_areq[1].test = test;
 
-	mmc_test_nonblock_reset(&mrq1, &cmd1, &stop1, &data1);
-	mmc_test_nonblock_reset(&mrq2, &cmd2, &stop2, &data2);
+	mem

Re: [PATCH 1/2] mmc: sdhci-spear: Initialize sdhci clk to 50 MHz

2012-11-26 Thread Viresh Kumar 
On 26 November 2012 20:39, Chris Ball  wrote:
> On Thu, Nov 08 2012, Viresh Kumar wrote:
>> + ret = clk_set_rate(sdhci->clk, 5000);
>> + if (ret)
>> + dev_dbg(&pdev->dev, "Error setting desired clk, clk=%lu\n",
>> + clk_get_rate(sdhci->clk));
>> +
>>   if (np) {
>>   sdhci->data = sdhci_probe_config_dt(pdev);
>>   if (IS_ERR(sdhci->data)) {
>
> Can I change this to a dev_err()?  No-one's going to see a dev_dbg().

Yes and thanks :)

--
viresh
--
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: sdhci-spear: Initialize sdhci clk to 50 MHz

2012-11-26 Thread Chris Ball 
Hi,

On Thu, Nov 08 2012, Viresh Kumar wrote:
> From: Vipul Kumar Samar 
>
> SPEAr sdhci driver expects the clock to be set to 50 MHz for proper 
> functioning.
> This patch sets clk to 50 MHz in probe.
>
> Signed-off-by: Vipul Kumar Samar 
> Signed-off-by: Viresh Kumar 
> ---
>  drivers/mmc/host/sdhci-spear.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> index 6be89c0..fea8bf9 100644
> --- a/drivers/mmc/host/sdhci-spear.c
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -146,6 +146,11 @@ static int __devinit sdhci_probe(struct platform_device 
> *pdev)
>   goto put_clk;
>   }
>  
> + ret = clk_set_rate(sdhci->clk, 5000);
> + if (ret)
> + dev_dbg(&pdev->dev, "Error setting desired clk, clk=%lu\n",
> + clk_get_rate(sdhci->clk));
> +
>   if (np) {
>   sdhci->data = sdhci_probe_config_dt(pdev);
>   if (IS_ERR(sdhci->data)) {

Can I change this to a dev_err()?  No-one's going to see a dev_dbg().

Thanks,

- Chris.
-- 
Chris Ball  
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 1/2] mmc: sdhci-spear: Initialize sdhci clk to 50 MHz

2012-11-26 Thread Viresh Kumar 
On 20 November 2012 12:11, Viresh Kumar  wrote:
> On 8 November 2012 20:39, Viresh Kumar  wrote:
>> From: Vipul Kumar Samar 
>>
>> SPEAr sdhci driver expects the clock to be set to 50 MHz for proper 
>> functioning.
>> This patch sets clk to 50 MHz in probe.
>>
>> Signed-off-by: Vipul Kumar Samar 
>> Signed-off-by: Viresh Kumar 
>
> Ping!!

Hi Chris,

Are you taking these for v3.8?
--
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/7] mmc: omap_hsmmc: Fix Oops in case of data errors

2012-11-26 Thread Venkatraman S 
On Mon, Nov 26, 2012 at 2:19 AM, Chris Ball  wrote:
> Hi Venkat,
>
> On Mon, Nov 19 2012, Venkatraman S wrote:
>> From: Balaji T K 
>>
>> "commit ae4bf788ee9bf7c2d51b0309117d1fcccbdd50a2
>> mmc: omap_hsmmc: consolidate error report handling of HSMMC IRQ"
>> sets both end_cmd and end_trans to 1.
>>
>> Setting end_cmd to 1 for Data Timeout/CRC leads to NULL pointer dereference 
>> of
>> host->cmd as the command complete has previously been handled.
>> Set end_cmd only in case of command Timeout/CRC.
>>
>> Moreover host->cmd->error should not be updated on data error case, only
>> host->data->error needs to be updated.
>>
>> Signed-off-by: Balaji T K 
>> Reviewed-by: Felipe Balbi 
>> Signed-off-by: Venkatraman S 
>
> Thanks, pushed all 7 patches to mmc-next for 3.8.
>
Great - Thank you !
--
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: mmci: Support non-power-of-two block sizes for ux500v2 variant

2012-11-26 Thread Per Förlin 
On 11/26/2012 11:27 AM, Russell King - ARM Linux wrote:
> On Mon, Nov 26, 2012 at 11:20:32AM +0100, Per Förlin wrote:
>> On 11/22/2012 06:37 PM, Russell King - ARM Linux wrote:
>>> On Thu, Nov 22, 2012 at 06:28:30PM +0100, Per Forlin wrote:
 On Wed, Nov 21, 2012 at 5:50 PM, Russell King - ARM Linux
  wrote:
> On Wed, Nov 21, 2012 at 05:13:55PM +0100, Per Forlin wrote:
>> On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
>>  wrote:
>>> On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
  /*
 + * Validate mmc prerequisites
 + */
 +static int mmci_validate_data(struct mmci_host *host,
 +   struct mmc_data *data)
 +{
 + if (!data)
 + return 0;
 +
 + if (!host->variant->non_power_of_2_blksize &&
 + !is_power_of_2(data->blksz)) {
 + dev_err(mmc_dev(host->mmc),
 + "unsupported block size (%d bytes)\n", 
 data->blksz);
 + return -EINVAL;
 + }
 +
 + if (data->sg->offset & 3) {
 + dev_err(mmc_dev(host->mmc),
 + "unsupported alginment (0x%x)\n", 
 data->sg->offset);
 + return -EINVAL;
 + }
>>>
>>> Why?  What's the reasoning behind this suddenly introduced restriction?
>>> readsl()/writesl() copes just fine with non-aligned pointers.  It may be
>>> that your DMA engine can not, but that's no business interfering with
>>> non-DMA transfers, and no reason to fail such transfers.
>>>
>>> If your DMA engine can't do that then its your DMA engine code which
>>> should refuse to prepare the transfer.
>>>
>>> Yes, that means problems with the way things are ordered - or it needs a
>>> proper API where DMA engine can export these kinds of properties.
>> The alignment constraint is related to PIO, sg_miter and that FIFO
>> access must be done with 4 bytes.
>
> Total claptrap.  No it isn't.
>
> - sg_miter just deals with bytes, and number of bytes transferred; there
>   is no word assumptions in that code.  Indeed many ATA disks transfer
>   by half-word accesses so such a restriction would be insane.
>
> - the FIFO access itself needs to be 32-bit words, so readsl or writesl
>   (or their io* equivalents must be used).
>
> - but - and this is the killer item to your argument as I said above -
>   readsl and writesl _can_ take misaligned pointers and cope with them
>   fine.
>
> The actual alignment of the buffer address is totally irrelevant here.
>
> What isn't irrelevant is the _number_ of bytes to be transferred, but
> that's something totally different and completely unrelated from
> data->sg->offset.
 Let's try again :)

 Keep in mind that the mmc -block layer is aligned so from that aspect
 everything is fine.
 SDIO may have any length or alignment but sg-len is always 1.

 There is just one sg-element and one continues buffer.

 sg-miter splits the continues buffer in chunks that may not be aligned
 with 4 byte size. It depends on the start address alignment of the
 buffer.

 Is it more clear now?
>>>
>>> Is this more clear: you may be passed a single buffer which is misaligned.
>>> We cope with that just fine.  There is *no* reason to reject that transfer
>>> because readsl/writesl cope just fine with it.
>>>
>> The MMCI driver doesn't support alignment smaller than 4 bytes (it may
>> result in data corruption).
> 
> Explain yourself.  That's what's lacking here.  I'm explaining why I
> think you're wrong, but you're just asserting all the time that I'm
> wrong without giving any real details.
> 
>> There are two options
>> 1. Either one should fix the driver to support it. Currently the driver
>> only supports miss-alignment of the last sg-miter buffer.
>> 2. Or be kind to inform the user that the alignment is not supported.
> 
> Look, it's very very simple.
> 
> If you have a multi-sg transfer, and the pointer starts off being
> misaligned, the first transfer to the end of the page _MAY_ be a
> non-word aligned number of bytes.  _THAT_ is what you should be checking.
> _THAT_ is what the limitation is in the driver.  _NOT_ that the pointer
> is misaligned.
>
There will be no multi-sg transfer that is misaligned because SDIO-fwk set the 
sg-length to 1. Still the transfer may be multi-PAGE with sg-length 1 which is 
something that mmci driver cannot handle.

The intention of "data->sg->offset & 3" is to check for misaligned data. What 
would you replace this check with?

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

[MX25][MMC]: SDHC very slow (300-600 KB/s) on i.MX25 board

2012-11-26 Thread Stefan Koch 

Hi,

I have ported linux-3.7-rc4 to a board with i.MX257 CPU.
As distribution I use Debian squeeze.
Booting time (with Xorg and IceWM) is different with different SD-Cards.
Boot time with 2GB Standard micro SD-Card: over 2:30 minutes
Boot time with 4GB Class 10 micro SDHC: 1:40 minutes
Boot time with 8GB UHS-1 micro SDHC: 1:15 minutes
Boot time with NFS (Network without SD/SDHC): 40 seconds

Starting applications is faster with NFS as with SD/SDHC.
At SD/SDHC linux is system is waiting a lot of time.
Then CPU can not run at 100% usage, so system is not so fast as possible.

Tests with dd give ratings from a. e. 300 KB/s (this could be a Class 10 
card).


But there is a relationship with slower and faster SD/SDHC-cards:
A UHS card works faster than a class 10 card an so on.
And a Standard 2GB SD card is the slowest.
But the Standard 2GB SC card works with a card reader connected to an 
Notebook faster than the UHS-1 Card in the imx257 Target system with imx 
integrated SDHC controller.


How to solve this problem?
Is it a driver issue?

Thanks

Stefan Koch

--
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: Remove redundant null check before kfree in sdio_bus.c

2012-11-26 Thread Sachin Kamat 
Hi Chris

Any comments on these 2 patches?

On 20 November 2012 14:43, Sachin Kamat  wrote:
> kfree on a null pointer is a no-op.
>
> Signed-off-by: Sachin Kamat 
> ---
>  drivers/mmc/core/sdio_bus.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index 6bf6879..fdcf9e3 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -258,8 +258,7 @@ static void sdio_release_func(struct device *dev)
>
> sdio_free_func_cis(func);
>
> -   if (func->info)
> -   kfree(func->info);
> +   kfree(func->info);
>
> kfree(func);
>  }
> --
> 1.7.4.1
>



-- 
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] mmc: mmci: Support non-power-of-two block sizes for ux500v2 variant

2012-11-26 Thread Russell King - ARM Linux 
On Mon, Nov 26, 2012 at 11:20:32AM +0100, Per Förlin wrote:
> On 11/22/2012 06:37 PM, Russell King - ARM Linux wrote:
> > On Thu, Nov 22, 2012 at 06:28:30PM +0100, Per Forlin wrote:
> >> On Wed, Nov 21, 2012 at 5:50 PM, Russell King - ARM Linux
> >>  wrote:
> >>> On Wed, Nov 21, 2012 at 05:13:55PM +0100, Per Forlin wrote:
>  On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
>   wrote:
> > On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
> >>  /*
> >> + * Validate mmc prerequisites
> >> + */
> >> +static int mmci_validate_data(struct mmci_host *host,
> >> +   struct mmc_data *data)
> >> +{
> >> + if (!data)
> >> + return 0;
> >> +
> >> + if (!host->variant->non_power_of_2_blksize &&
> >> + !is_power_of_2(data->blksz)) {
> >> + dev_err(mmc_dev(host->mmc),
> >> + "unsupported block size (%d bytes)\n", 
> >> data->blksz);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (data->sg->offset & 3) {
> >> + dev_err(mmc_dev(host->mmc),
> >> + "unsupported alginment (0x%x)\n", 
> >> data->sg->offset);
> >> + return -EINVAL;
> >> + }
> >
> > Why?  What's the reasoning behind this suddenly introduced restriction?
> > readsl()/writesl() copes just fine with non-aligned pointers.  It may be
> > that your DMA engine can not, but that's no business interfering with
> > non-DMA transfers, and no reason to fail such transfers.
> >
> > If your DMA engine can't do that then its your DMA engine code which
> > should refuse to prepare the transfer.
> >
> > Yes, that means problems with the way things are ordered - or it needs a
> > proper API where DMA engine can export these kinds of properties.
>  The alignment constraint is related to PIO, sg_miter and that FIFO
>  access must be done with 4 bytes.
> >>>
> >>> Total claptrap.  No it isn't.
> >>>
> >>> - sg_miter just deals with bytes, and number of bytes transferred; there
> >>>   is no word assumptions in that code.  Indeed many ATA disks transfer
> >>>   by half-word accesses so such a restriction would be insane.
> >>>
> >>> - the FIFO access itself needs to be 32-bit words, so readsl or writesl
> >>>   (or their io* equivalents must be used).
> >>>
> >>> - but - and this is the killer item to your argument as I said above -
> >>>   readsl and writesl _can_ take misaligned pointers and cope with them
> >>>   fine.
> >>>
> >>> The actual alignment of the buffer address is totally irrelevant here.
> >>>
> >>> What isn't irrelevant is the _number_ of bytes to be transferred, but
> >>> that's something totally different and completely unrelated from
> >>> data->sg->offset.
> >> Let's try again :)
> >>
> >> Keep in mind that the mmc -block layer is aligned so from that aspect
> >> everything is fine.
> >> SDIO may have any length or alignment but sg-len is always 1.
> >>
> >> There is just one sg-element and one continues buffer.
> >>
> >> sg-miter splits the continues buffer in chunks that may not be aligned
> >> with 4 byte size. It depends on the start address alignment of the
> >> buffer.
> >>
> >> Is it more clear now?
> > 
> > Is this more clear: you may be passed a single buffer which is misaligned.
> > We cope with that just fine.  There is *no* reason to reject that transfer
> > because readsl/writesl cope just fine with it.
> > 
> The MMCI driver doesn't support alignment smaller than 4 bytes (it may
> result in data corruption).

Explain yourself.  That's what's lacking here.  I'm explaining why I
think you're wrong, but you're just asserting all the time that I'm
wrong without giving any real details.

> There are two options
> 1. Either one should fix the driver to support it. Currently the driver
> only supports miss-alignment of the last sg-miter buffer.
> 2. Or be kind to inform the user that the alignment is not supported.

Look, it's very very simple.

If you have a multi-sg transfer, and the pointer starts off being
misaligned, the first transfer to the end of the page _MAY_ be a
non-word aligned number of bytes.  _THAT_ is what you should be checking.
_THAT_ is what the limitation is in the driver.  _NOT_ that the pointer
is misaligned.

--
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: mmci: Support non-power-of-two block sizes for ux500v2 variant

2012-11-26 Thread Per Förlin 
On 11/22/2012 06:37 PM, Russell King - ARM Linux wrote:
> On Thu, Nov 22, 2012 at 06:28:30PM +0100, Per Forlin wrote:
>> On Wed, Nov 21, 2012 at 5:50 PM, Russell King - ARM Linux
>>  wrote:
>>> On Wed, Nov 21, 2012 at 05:13:55PM +0100, Per Forlin wrote:
 On Wed, Nov 21, 2012 at 4:38 PM, Russell King - ARM Linux
  wrote:
> On Fri, Oct 12, 2012 at 04:02:02PM +0200, Ulf Hansson wrote:
>>  /*
>> + * Validate mmc prerequisites
>> + */
>> +static int mmci_validate_data(struct mmci_host *host,
>> +   struct mmc_data *data)
>> +{
>> + if (!data)
>> + return 0;
>> +
>> + if (!host->variant->non_power_of_2_blksize &&
>> + !is_power_of_2(data->blksz)) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unsupported block size (%d bytes)\n", 
>> data->blksz);
>> + return -EINVAL;
>> + }
>> +
>> + if (data->sg->offset & 3) {
>> + dev_err(mmc_dev(host->mmc),
>> + "unsupported alginment (0x%x)\n", 
>> data->sg->offset);
>> + return -EINVAL;
>> + }
>
> Why?  What's the reasoning behind this suddenly introduced restriction?
> readsl()/writesl() copes just fine with non-aligned pointers.  It may be
> that your DMA engine can not, but that's no business interfering with
> non-DMA transfers, and no reason to fail such transfers.
>
> If your DMA engine can't do that then its your DMA engine code which
> should refuse to prepare the transfer.
>
> Yes, that means problems with the way things are ordered - or it needs a
> proper API where DMA engine can export these kinds of properties.
 The alignment constraint is related to PIO, sg_miter and that FIFO
 access must be done with 4 bytes.
>>>
>>> Total claptrap.  No it isn't.
>>>
>>> - sg_miter just deals with bytes, and number of bytes transferred; there
>>>   is no word assumptions in that code.  Indeed many ATA disks transfer
>>>   by half-word accesses so such a restriction would be insane.
>>>
>>> - the FIFO access itself needs to be 32-bit words, so readsl or writesl
>>>   (or their io* equivalents must be used).
>>>
>>> - but - and this is the killer item to your argument as I said above -
>>>   readsl and writesl _can_ take misaligned pointers and cope with them
>>>   fine.
>>>
>>> The actual alignment of the buffer address is totally irrelevant here.
>>>
>>> What isn't irrelevant is the _number_ of bytes to be transferred, but
>>> that's something totally different and completely unrelated from
>>> data->sg->offset.
>> Let's try again :)
>>
>> Keep in mind that the mmc -block layer is aligned so from that aspect
>> everything is fine.
>> SDIO may have any length or alignment but sg-len is always 1.
>>
>> There is just one sg-element and one continues buffer.
>>
>> sg-miter splits the continues buffer in chunks that may not be aligned
>> with 4 byte size. It depends on the start address alignment of the
>> buffer.
>>
>> Is it more clear now?
> 
> Is this more clear: you may be passed a single buffer which is misaligned.
> We cope with that just fine.  There is *no* reason to reject that transfer
> because readsl/writesl cope just fine with it.
> 
The MMCI driver doesn't support alignment smaller than 4 bytes (it may result 
in data corruption).
There are two options
1. Either one should fix the driver to support it. Currently the driver only 
supports miss-alignment of the last sg-miter buffer.
2. Or be kind to inform the user that the alignment is not supported.

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

RE: [mmc:mmc-next 45/63] of_iommu.c:(.text+0x114dbc): undefined reference to `__aeabi_uldivmod'

2012-11-26 Thread Zhang Haijun-B42677 
Hi Chris

I see patches modified the public C file and Powerpc Host C file. But it also 
used by other host C file.
So need to replace all the u64 division by macro div_u64 to avoid this error.
Two new patches had send and reviewed by Anton.
I think they can fix the `__aeabi_uldivmod' error.
All the patches work fine on my branch.

Regards
Haijun.


> -Original Message-
> From: linux-mmc-ow...@vger.kernel.org [mailto:linux-mmc-
> ow...@vger.kernel.org] On Behalf Of Chris Ball
> Sent: Monday, November 26, 2012 7:51 AM
> To: kbuild test robot
> Cc: Zhang Haijun-B42677; linux-mmc@vger.kernel.org; Huang Changming-
> R66093
> Subject: Re: [mmc:mmc-next 45/63] of_iommu.c:(.text+0x114dbc): undefined
> reference to `__aeabi_uldivmod'
> 
> Hi,
> 
> On Sun, Nov 25 2012, kbuild test robot wrote:
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc.git mmc-
> next
> > head:   b1dc46e6cd5f314f52a9864cc7ba1b7624b54bd7
> > commit: d29254e21ee37b3c8525817398d96d6403f3 [45/63] mmc: core:
> > Use u64 to calculate the timeout value to avoid overflow
> > config: make ARCH=arm omap2plus_defconfig
> >
> > All error/warnings:
> >
> > drivers/built-in.o: In function `mmc_omap_start_request':
> > of_iommu.c:(.text+0x114dbc): undefined reference to `__aeabi_uldivmod'
> 
> I've removed Haijun's three patches from mmc-next while they're reworked
> to fix this.  Thanks!
> 
> - Chris.
> --
> Chris Ball  
> 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


--
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 V2] mmc:block : Convert the cmd_timeou_ms to u64 to avoid overflow

2012-11-26 Thread Haijun Zhang 
Convert the cmd_timeou_ms to u64 to avoid overflow

Signed-off-by: Jerry Huang 
Signed-off-by: Haijun Zhang 
Reviewed-by: Anton Vorontsov 
CC: Chris Ball 
---
 drivers/mmc/card/block.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 21056b9..29887b1 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -469,7 +469,8 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
 * mrq.data = NULL;
 * cmd.cmd_timeout = idata->ic.cmd_timeout_ms;
 */
-   data.timeout_ns = idata->ic.cmd_timeout_ms * 100;
+   data.timeout_ns = (u64)idata->ic.cmd_timeout_ms *
+   100;
}
 
mrq.data = &data;
-- 
1.7.0.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

[PATCH 2/2 V2] mmc:host: Use u64 to calculate the timeout value to avoid overflow

2012-11-26 Thread Haijun Zhang 
As data timeout_ns use u64 to avoid overflow.
So we use macro div_u64 to perform a division.
Below C file modified:
 - drivers/mmc/host/atmel-mci.c
 - drivers/mmc/host/bfin_sdh.c
 - drivers/mmc/host/davinci_mmc.c
 - drivers/mmc/host/mmci.c
 - drivers/mmc/host/mvsdio.c
 - drivers/mmc/host/mxs-mmc.c
 - drivers/mmc/host/omap.c
 - drivers/mmc/host/tifm_sd.c
 - drivers/mmc/host/wbsd.c

Signed-off-by: Jerry Huang 
Signed-off-by: Haijun Zhang 
Reviewed-by: Anton Vorontsov 
CC: Chris Ball 
---
changes for v2:
- Remove the needless type convertion.

 drivers/mmc/host/atmel-mci.c   |7 +++
 drivers/mmc/host/bfin_sdh.c|2 +-
 drivers/mmc/host/davinci_mmc.c |4 ++--
 drivers/mmc/host/mmci.c|2 +-
 drivers/mmc/host/mvsdio.c  |2 +-
 drivers/mmc/host/mxs-mmc.c |4 ++--
 drivers/mmc/host/omap.c|2 +-
 drivers/mmc/host/tifm_sd.c |4 ++--
 drivers/mmc/host/wbsd.c|2 +-
 9 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index ddf096e..ae16ed4 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -593,17 +593,16 @@ static void atmci_timeout_timer(unsigned long data)
tasklet_schedule(&host->tasklet);
 }
 
-static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host,
-   unsigned int ns)
+static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host, u64 ns)
 {
/*
 * It is easier here to use us instead of ns for the timeout,
 * it prevents from overflows during calculation.
 */
-   unsigned int us = DIV_ROUND_UP(ns, 1000);
+   u64 us = DIV_ROUND_UP_ULL(ns, 1000);
 
/* Maximum clock frequency is host->bus_hz/2 */
-   return us * (DIV_ROUND_UP(host->bus_hz, 200));
+   return us * (DIV_ROUND_UP_ULL(host->bus_hz, 200));
 }
 
 static void atmci_set_timeout(struct atmel_mci *host,
diff --git a/drivers/mmc/host/bfin_sdh.c b/drivers/mmc/host/bfin_sdh.c
index b9b463e..88e0abb 100644
--- a/drivers/mmc/host/bfin_sdh.c
+++ b/drivers/mmc/host/bfin_sdh.c
@@ -143,7 +143,7 @@ static int sdh_setup_data(struct sdh_host *host, struct 
mmc_data *data)
bfin_write_SDH_DATA_CTL(data_ctl);
/* the time of a host clock period in ns */
cycle_ns = 10 / (host->sclk / (2 * (host->clk_div + 1)));
-   timeout = data->timeout_ns / cycle_ns;
+   timeout = div_u64(data->timeout_ns, cycle_ns);
timeout += data->timeout_clks;
bfin_write_SDH_DATA_TIMER(timeout);
SSYNC();
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 2063677..a48b475 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -568,10 +568,10 @@ mmc_davinci_prepare_data(struct mmc_davinci_host *host, 
struct mmc_request *req)
(data->flags & MMC_DATA_STREAM) ? "stream" : "block",
(data->flags & MMC_DATA_WRITE) ? "write" : "read",
data->blocks, data->blksz);
-   dev_dbg(mmc_dev(host->mmc), "  DTO %d cycles + %d ns\n",
+   dev_dbg(mmc_dev(host->mmc), "  DTO %d cycles + %lld ns\n",
data->timeout_clks, data->timeout_ns);
timeout = data->timeout_clks +
-   (data->timeout_ns / host->ns_in_one_cycle);
+   div_u64(data->timeout_ns, host->ns_in_one_cycle);
if (timeout > 0x)
timeout = 0x;
 
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index edc3e9b..09fe317 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -632,7 +632,7 @@ static void mmci_start_data(struct mmci_host *host, struct 
mmc_data *data)
host->size = data->blksz * data->blocks;
data->bytes_xfered = 0;
 
-   clks = (unsigned long long)data->timeout_ns * host->cclk;
+   clks = data->timeout_ns * host->cclk;
do_div(clks, 10UL);
 
timeout = data->timeout_clks + (unsigned int)clks;
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index de4c20b..3badc4f 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -92,7 +92,7 @@ static int mvsd_setup_data(struct mvsd_host *host, struct 
mmc_data *data)
}
 
/* If timeout=0 then maximum timeout index is used. */
-   tmout = DIV_ROUND_UP(data->timeout_ns, host->ns_per_clk);
+   tmout = DIV_ROUND_UP_ULL(data->timeout_ns, host->ns_per_clk);
tmout += data->timeout_clks;
tmout_index = fls(tmout - 1) - 12;
if (tmout_index < 0)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 206fe49..9f3617b 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -331,7 +331,7 @@ out:
 "%s: failed to prep dma\n", __func__);
 }
 
-static unsigned short mxs_ns_to_ssp_ticks(unsigned clock_rate, unsigned ns)
+static unsigned short mxs_ns_to_ssp_ticks(unsigned

Re: [PATCH 3/3] MMC: sdhci-dove: allow GPIOs to be used for card detection on Dove

2012-11-26 Thread Guennadi Liakhovetski 
Hi Chris

On Sun, 25 Nov 2012, Chris Ball wrote:

> Hi, adding Guennadi,
> 
> On Fri, Nov 23 2012, Russell King - ARM Linux wrote:
> > On Fri, Nov 23, 2012 at 08:14:27PM +0800, Shawn Guo wrote:
> >> On Fri, Nov 23, 2012 at 08:57:26AM +, Russell King - ARM Linux wrote:
> >> > Does this work with the sdhci stuff?
> >> 
> >> Honestly, I'm not sure, but I guess it does, since I have seen
> >> sdhci-pxav3 driver using the helpers.  Anyway, I'm going to adopt
> >> the helpers for sdhci-esdhc-imx driver to find it out.
> >
> > The thing that worries me is this:
> >
> > static void sdhci_tasklet_card(unsigned long param)
> > {
> > ...
> > /* Check host->mrq first in case we are runtime suspended */
> > if (host->mrq &&
> > !(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) 
> > {
> > pr_err("%s: Card removed during transfer!\n",
> > mmc_hostname(host->mmc));
> > pr_err("%s: Resetting controller.\n",
> > mmc_hostname(host->mmc));
> >
> > sdhci_reset(host, SDHCI_RESET_CMD);
> > sdhci_reset(host, SDHCI_RESET_DATA);
> >
> > host->mrq->cmd->error = -ENOMEDIUM;
> > tasklet_schedule(&host->finish_tasklet);
> > }
> > ...
> > mmc_detect_change(host->mmc, msecs_to_jiffies(200));
> > }
> >
> >
> > That gets called whenever a card is inserted/removed by the SDHCI code (if
> > the SDHCI card detect is wired), or in my case by the interrupt routine
> > the code in my patch adds.
> >
> > The slot-gpio.c stuff directly calls into mmc_detect_change (a) with a
> > shorter delay, and (b) completely omits the above handling and resetting.
> > My guess from the above code is that it'll work fine 90% of the time
> > (because you'll remove the card without an active request), but the above
> > code looks like it's addressing a corner case which will be omitted with
> > the "generic" slot-gpio.c solution.
> >
> > So I don't think it's a good idea to use slot-gpio.c in this case.
> 
> Guennadi, what are your thoughts about consolidating this reset logic
> between the sdhci tasklet and slot-gpio?  It would certainly be nice to
> use slot-gpio in cases like this one, so it's worth fixing.

Sure, this can be added. As for how - I see at least two possibilities: 
(1) put the complete above block in a new mmc host operation and just call 
it from the GPIO card-detect ISR, (2) taking into account, that many 
(nearly all? all?) host drivers keep a pointer to the current mrq in their 
private data struct, we could instead add such a pointer to struct 
mmc_host, then the check "request in progress" could also be generalised, 
and the operation would just have to reset the host and complete the 
request (in the sdhci case schedule the task).

Note, that there's already a .hw_reset() operation, used by sdhci-pci only 
so far, still, it seems we cannot (easily) hijack it.

So, what would be the preferred way?

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] Powerpc eSDHC: Convert the cmd_timeou_ms to u64 to avoid overflow

2012-11-26 Thread Zhang Haijun-B42677 
Hi, Anton

This patch was for public C file. That was for different Host controller.
I think let them standalone will be better.

The u32 conversion will be removed.
Thanks. 


Regards
Haijun.


> -Original Message-
> From: Anton Vorontsov [mailto:cbouatmai...@gmail.com]
> Sent: Monday, November 26, 2012 5:12 PM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093; Chris Ball
> Subject: Re: [PATCH] Powerpc eSDHC: Convert the cmd_timeou_ms to u64 to
> avoid overflow
> 
> On Mon, Nov 26, 2012 at 04:35:58PM +0800, Haijun Zhang wrote:
> > Convert the cmd_timeou_ms to u64 to avoid overflow
> >
> > Signed-off-by: Jerry Huang 
> > Signed-off-by: Haijun Zhang 
> 
> Reviewed-by: Anton Vorontsov 
> 
> But I think you should merge the two patches...
> 
> Thanks,
> 
> > CC: Chris Ball 
> > ---
> >  drivers/mmc/card/block.c |3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index
> > 21056b9..51903de 100644
> > --- a/drivers/mmc/card/block.c
> > +++ b/drivers/mmc/card/block.c
> > @@ -469,7 +469,8 @@ static int mmc_blk_ioctl_cmd(struct block_device
> *bdev,
> >  * mrq.data = NULL;
> >  * cmd.cmd_timeout = idata->ic.cmd_timeout_ms;
> >  */
> > -   data.timeout_ns = idata->ic.cmd_timeout_ms * 100;
> > +   data.timeout_ns = (u64)idata->ic.cmd_timeout_ms *
> > +   100;
> > }
> >
> > mrq.data = &data;
> > --
> > 1.7.0.4

N�r��yb�X��ǧv�^�)޺{.n�+{��g"��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: [PATCH] Powerpc eSDHC: Convert the cmd_timeou_ms to u64 to avoid overflow

2012-11-26 Thread Anton Vorontsov 
On Mon, Nov 26, 2012 at 04:35:58PM +0800, Haijun Zhang wrote:
> Convert the cmd_timeou_ms to u64 to avoid overflow
> 
> Signed-off-by: Jerry Huang 
> Signed-off-by: Haijun Zhang 

Reviewed-by: Anton Vorontsov 

But I think you should merge the two patches...

Thanks,

> CC: Chris Ball 
> ---
>  drivers/mmc/card/block.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 21056b9..51903de 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -469,7 +469,8 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>* mrq.data = NULL;
>* cmd.cmd_timeout = idata->ic.cmd_timeout_ms;
>*/
> - data.timeout_ns = idata->ic.cmd_timeout_ms * 100;
> + data.timeout_ns = (u64)idata->ic.cmd_timeout_ms *
> + 100;
>   }
>  
>   mrq.data = &data;
> -- 
> 1.7.0.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] Use u64 to calculate the timeout value to avoid overflow

2012-11-26 Thread Anton Vorontsov 
On Mon, Nov 26, 2012 at 04:35:59PM +0800, Haijun Zhang wrote:
> As data timeout_ns use u64 to avoid overflow.
> So we use macro div_u64 to perform a division.
[...]
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -593,17 +593,16 @@ static void atmci_timeout_timer(unsigned long data)
>   tasklet_schedule(&host->tasklet);
>  }
>  
> -static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host,
> - unsigned int ns)
> +static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host, u64 ns)
>  {
>   /*
>* It is easier here to use us instead of ns for the timeout,
>* it prevents from overflows during calculation.
>*/
> - unsigned int us = DIV_ROUND_UP(ns, 1000);
> + u64 us = DIV_ROUND_UP_ULL(ns, 1000);
>  
>   /* Maximum clock frequency is host->bus_hz/2 */
> - return us * (DIV_ROUND_UP(host->bus_hz, 200));
> + return (u32)(us * (DIV_ROUND_UP_ULL(host->bus_hz, 200)));

Why do you need u32 cast here?..

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

[PATCH] Use u64 to calculate the timeout value to avoid overflow

2012-11-26 Thread Haijun Zhang 
As data timeout_ns use u64 to avoid overflow.
So we use macro div_u64 to perform a division.
Below C file modified:
 - drivers/mmc/host/atmel-mci.c
 - drivers/mmc/host/bfin_sdh.c
 - drivers/mmc/host/davinci_mmc.c
 - drivers/mmc/host/mmci.c
 - drivers/mmc/host/mvsdio.c
 - drivers/mmc/host/mxs-mmc.c
 - drivers/mmc/host/omap.c
 - drivers/mmc/host/tifm_sd.c
 - drivers/mmc/host/wbsd.c

Signed-off-by: Jerry Huang 
Signed-off-by: Haijun Zhang 
CC: Anton Vorontsov 
CC: Chris Ball 
---
 drivers/mmc/host/atmel-mci.c   |7 +++
 drivers/mmc/host/bfin_sdh.c|2 +-
 drivers/mmc/host/davinci_mmc.c |4 ++--
 drivers/mmc/host/mmci.c|2 +-
 drivers/mmc/host/mvsdio.c  |2 +-
 drivers/mmc/host/mxs-mmc.c |4 ++--
 drivers/mmc/host/omap.c|2 +-
 drivers/mmc/host/tifm_sd.c |4 ++--
 drivers/mmc/host/wbsd.c|2 +-
 9 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index ddf096e..0f74d37 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -593,17 +593,16 @@ static void atmci_timeout_timer(unsigned long data)
tasklet_schedule(&host->tasklet);
 }
 
-static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host,
-   unsigned int ns)
+static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host, u64 ns)
 {
/*
 * It is easier here to use us instead of ns for the timeout,
 * it prevents from overflows during calculation.
 */
-   unsigned int us = DIV_ROUND_UP(ns, 1000);
+   u64 us = DIV_ROUND_UP_ULL(ns, 1000);
 
/* Maximum clock frequency is host->bus_hz/2 */
-   return us * (DIV_ROUND_UP(host->bus_hz, 200));
+   return (u32)(us * (DIV_ROUND_UP_ULL(host->bus_hz, 200)));
 }
 
 static void atmci_set_timeout(struct atmel_mci *host,
diff --git a/drivers/mmc/host/bfin_sdh.c b/drivers/mmc/host/bfin_sdh.c
index b9b463e..88e0abb 100644
--- a/drivers/mmc/host/bfin_sdh.c
+++ b/drivers/mmc/host/bfin_sdh.c
@@ -143,7 +143,7 @@ static int sdh_setup_data(struct sdh_host *host, struct 
mmc_data *data)
bfin_write_SDH_DATA_CTL(data_ctl);
/* the time of a host clock period in ns */
cycle_ns = 10 / (host->sclk / (2 * (host->clk_div + 1)));
-   timeout = data->timeout_ns / cycle_ns;
+   timeout = div_u64(data->timeout_ns, cycle_ns);
timeout += data->timeout_clks;
bfin_write_SDH_DATA_TIMER(timeout);
SSYNC();
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 2063677..a48b475 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -568,10 +568,10 @@ mmc_davinci_prepare_data(struct mmc_davinci_host *host, 
struct mmc_request *req)
(data->flags & MMC_DATA_STREAM) ? "stream" : "block",
(data->flags & MMC_DATA_WRITE) ? "write" : "read",
data->blocks, data->blksz);
-   dev_dbg(mmc_dev(host->mmc), "  DTO %d cycles + %d ns\n",
+   dev_dbg(mmc_dev(host->mmc), "  DTO %d cycles + %lld ns\n",
data->timeout_clks, data->timeout_ns);
timeout = data->timeout_clks +
-   (data->timeout_ns / host->ns_in_one_cycle);
+   div_u64(data->timeout_ns, host->ns_in_one_cycle);
if (timeout > 0x)
timeout = 0x;
 
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index edc3e9b..09fe317 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -632,7 +632,7 @@ static void mmci_start_data(struct mmci_host *host, struct 
mmc_data *data)
host->size = data->blksz * data->blocks;
data->bytes_xfered = 0;
 
-   clks = (unsigned long long)data->timeout_ns * host->cclk;
+   clks = data->timeout_ns * host->cclk;
do_div(clks, 10UL);
 
timeout = data->timeout_clks + (unsigned int)clks;
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index de4c20b..3badc4f 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -92,7 +92,7 @@ static int mvsd_setup_data(struct mvsd_host *host, struct 
mmc_data *data)
}
 
/* If timeout=0 then maximum timeout index is used. */
-   tmout = DIV_ROUND_UP(data->timeout_ns, host->ns_per_clk);
+   tmout = DIV_ROUND_UP_ULL(data->timeout_ns, host->ns_per_clk);
tmout += data->timeout_clks;
tmout_index = fls(tmout - 1) - 12;
if (tmout_index < 0)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 206fe49..9f3617b 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -331,7 +331,7 @@ out:
 "%s: failed to prep dma\n", __func__);
 }
 
-static unsigned short mxs_ns_to_ssp_ticks(unsigned clock_rate, unsigned ns)
+static unsigned short mxs_ns_to_ssp_ticks(unsigned clock_rate, u64 ns)
 {
const unsigned int ssp_timeout_mul

[PATCH] Powerpc eSDHC: Convert the cmd_timeou_ms to u64 to avoid overflow

2012-11-26 Thread Haijun Zhang 
Convert the cmd_timeou_ms to u64 to avoid overflow

Signed-off-by: Jerry Huang 
Signed-off-by: Haijun Zhang 
CC: Anton Vorontsov 
CC: Chris Ball 
---
 drivers/mmc/card/block.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 21056b9..51903de 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -469,7 +469,8 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
 * mrq.data = NULL;
 * cmd.cmd_timeout = idata->ic.cmd_timeout_ms;
 */
-   data.timeout_ns = idata->ic.cmd_timeout_ms * 100;
+   data.timeout_ns = (u64)idata->ic.cmd_timeout_ms *
+   100;
}
 
mrq.data = &data;
-- 
1.7.0.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