Re: [PATCH v2 07/24] mmc: sdhci: command response CRC error handling

[Adrian Hunter]

On 02/01/16 14:25, Russell King - ARM Linux wrote:
> On Tue, Dec 29, 2015 at 03:08:20PM +0200, Adrian Hunter wrote:
>> On 21/12/15 13:40, Russell King wrote:
>>> When we get a response CRC error on a command, it means that the
>>> response we received back from the card was not correct.  It does not
>>> mean that the card did not receive the command correctly.  If the
>>
>> Pedantically, if the timeout bit is set as well (CMD line conflict),
>> it does mean the card did not receive the command, so it should be coded
>> that way.
> 
> Good catch, the SDHCI spec contains a table which describes the CRC and
> timeout bit states, though it's not quite as you describe above...
> CRC and timeout indicates a command line conflict at some point.

In the case of CMD line conflict, the host controller aborts the command, so
presumably there will not be any data timeout.  Will you change it?

> 
>>> Fix this by handing a response CRC error slightly differently: record
>>> the failure of the data initiating command, but allow the remainder of
>>> the request to be processed normally.  This is safe as core MMC checks
>>
>> "processed normally" confused me at first because it sounded like you are
>> ignoring the error.  Not sure why you have a much better explanation in the
>> cover email than here.
> 
> They're written at different times?  I don't accept your comment though -
> "record the failure" _clearly_ does not mean that we're ignoring the error.
> 
>>> the status of all commands and data transfer phases of the request.
>>
>> MMC core is not the only initiator of requests, but it is safe because the
>> command error takes precedence by design.
>>
>> Also you don't explain why it is better to continue rather than attempt to
>> send a stop command and clean up the request properly.  It looks simpler and
>> less racy, but if that is the reason then it seems worth saying so.
> 
> This patch results from the analysis of failures seen on iMX6 hardware,
> where the card has entered data mode, and started to send its data.
> Right now, this screws up the next command.
> 
>>> If the card does not initiate a data transfer, then we should time out
>>> according to the data transfer parameters.
>>>
>>> Signed-off-by: Russell King <rmk+ker...@arm.linux.org.uk>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 17 +
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 86310b162304..3e718e465a1b 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2340,6 +2340,23 @@ static void sdhci_cmd_irq(struct sdhci_host *host, 
>>> u32 intmask, u32 *mask)
>>> else
>>> host->cmd->error = -EILSEQ;
>>>  
>>> +   /*
>>> +* If this command initiates a data phase and a response
>>> +* CRC error is signalled, the card can start transferring
>>> +* data - the card may have received the command without
>>> +* error.  We must not terminate the request early.
>>
>> This is misleading.  We could terminate the request early if we cleaned it
>> up.  You should say here why it is better to continue.
> 
> That is _not_ misleading, it is entirely accurate.  What the code
> currently does when it encounters a CRC error is it terminates the
> _request_ early.  The _request_ being "struct mmc_request" - and
> it terminates it _without_ sending a STOP command.

Sure, but the person reading the comment not should have to know the history
of the code to interpret it.  But it is not a big thing - the comment could
just be:

We must not terminate early because we don't bother to clean up.

> 
> Resetting the host controller does not influence what state the card
> is in.
> 
> So what happens at the moment is that we send a command which initiates
> a data phase from the card.  The card responds with a valid response,
> and starts sending data to the host.  The host incorrectly receives
> the card response with a CRC error.
> 
> At this point, the code decides that it had a failure, queues the
> finish tasklet, which resets the SDHCI controller, leaving the card
> transmitting data to the host, potentially endlessly.  The driver
> reports to the MMC layer that the mmc_request is complete, and we
> get the next request to process.
> 
> We try sending the next request to the card, but the card is still
> sending data to the host...  That's the p

Re: [PATCH v2 08/24] mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

[Adrian Hunter]

On 02/01/16 16:31, Russell King - ARM Linux wrote:
> On Sat, Jan 02, 2016 at 12:29:19PM +, Russell King - ARM Linux wrote:
>> On Tue, Dec 29, 2015 at 03:44:38PM +0200, Adrian Hunter wrote:
>>> On 21/12/15 13:41, Russell King wrote:
>>>> Unnecessarily mapping and unmapping the align buffer for SD cards is
>>>> expensive: performance measurements on iMX6 show that this gives a hit
>>>> of 10% on hdparm buffered disk reads.
>>>>
>>>> MMC/SD card IO comes from the mm/vfs which gives us page based IO, so
>>>> for this case, the align buffer is not going to be used.  However, we
>>>> still map and unmap this buffer.
>>>>
>>>> Eliminate this by switching the align buffer to be a DMA coherent
>>>> buffer, which needs no DMA maintenance to access the buffer.
>>>
>>> Did you consider putting the align buffer in the same allocation
>>> as the adma_table?
>>
>> It's not clear whether host->adma_table_sz would be appropriately
>> aligned.
> 
> Looking at this closer, it would not be appropriately aligned to place
> the alignment buffer after the adma table, but placing the alignment
> buffer in the allocation first would give appropriate alignment.
> 
> The buffer sizes are:
> 
> Table 32-bit  64-bit
> alignment 512 1024(entry sz * 128)
> adma  20563084(desc sz * (128 * 2 + 1))
> 
> Allocating the two together gives advantages and disadvantages:
> 
> * for 32-bit address sizes, instead of allocating two order-0 pages,
>   we end up allocating one order-0 page, so halve the coherent DMA
>   allocation of the driver.
> 
> * for 64-bit address sizes, we allocate one order-1 page instead,
>   which may be more prone to failure with a 4k page size, and it
>   also means the ADMA table will overlap a page boundary.  I've no
>   idea whether that is an issue for any SDHCI controllers or not.
> 
> I could add additional complexity to do different allocations in the
> 32-bit and 64-bit paths, but that results in a _far_ more complicated
> cleanup path, and much more prone to errors.
> 
> In any case, such a patch should be entirely separate from _this_
> patch given that such a change may cause breakage and could need
> to be reworked as a result.  Indeed, it's a _separate_ change in
> itself, for a different purpose from _this_ patch.

Agreed.

--
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 v2 08/24] mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

[Adrian Hunter]

On 02/01/16 14:29, Russell King - ARM Linux wrote:
> On Tue, Dec 29, 2015 at 03:44:38PM +0200, Adrian Hunter wrote:
>> On 21/12/15 13:41, Russell King wrote:
>>> Unnecessarily mapping and unmapping the align buffer for SD cards is
>>> expensive: performance measurements on iMX6 show that this gives a hit
>>> of 10% on hdparm buffered disk reads.
>>>
>>> MMC/SD card IO comes from the mm/vfs which gives us page based IO, so
>>> for this case, the align buffer is not going to be used.  However, we
>>> still map and unmap this buffer.
>>>
>>> Eliminate this by switching the align buffer to be a DMA coherent
>>> buffer, which needs no DMA maintenance to access the buffer.
>>
>> Did you consider putting the align buffer in the same allocation
>> as the adma_table?
> 
> It's not clear whether host->adma_table_sz would be appropriately
> aligned.
> 
>>> @@ -3003,6 +2972,10 @@ int sdhci_add_host(struct sdhci_host *host)
>>>   host->adma_table_sz,
>>>   >adma_addr,
>>>   GFP_KERNEL);
>>> +   host->align_buffer = dma_alloc_coherent(mmc_dev(mmc),
>>> +   host->align_buffer_sz,
>>> +   >align_addr,
>>> +   GFP_KERNEL);
>>> host->align_buffer = kmalloc(host->align_buffer_sz, GFP_KERNEL);
>>
>> kmalloc line is still there
> 
> Good catch, thanks.
> 
>>> +
>>> +   /* dma_alloc_coherent returns page aligned and sized buffers */
>>> +   BUG_ON(host->align_addr & host->align_mask);
>>
>> It would be nicer not to have any BUG_ON()
> 
> This is a situation that should _never_ occur (if it does, then the
> dma_alloc_coherent() implementation is violating the API requirements,
> which are to return a page-sized page-aligned buffer.)  I guess it
> could be a WARN_ON(), but if it fails we're likely to cause data
> corruption.  So, a WARN_ON() and failing the probe seems more
> appropriate - but then that means yet more messy cleanup in an already
> messy part of the driver.

Isn't there already a cleanup path? i.e.

} else if (host->adma_addr & host->align_mask) {
pr_warn("%s: unable to allocate aligned ADMA 
descriptor\n",
mmc_hostname(mmc));
host->flags &= ~SDHCI_USE_ADMA;
dma_free_coherent(mmc_dev(mmc), host->adma_table_sz,
  host->adma_table, host->adma_addr);
kfree(host->align_buffer);
host->adma_table = NULL;
host->align_buffer = NULL;
}


--
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 v2 07/24] mmc: sdhci: command response CRC error handling

[Adrian Hunter]

On 21/12/15 13:40, Russell King wrote:
> When we get a response CRC error on a command, it means that the
> response we received back from the card was not correct.  It does not
> mean that the card did not receive the command correctly.  If the

Pedantically, if the timeout bit is set as well (CMD line conflict),
it does mean the card did not receive the command, so it should be coded
that way.

> command is one which initiates a data transfer, the card can enter the
> data transfer state, and start sending data.
> 
> Moreover, if the request contained a data phase, we do not clean this
> up, and this results in the driver triggering DMA API debug warnings,
> and also creates a race condition in the driver, between running the
> finish_tasklet and the data transfer interrupts, which can trigger a
> "Got data interrupt" state dump.
> 
> Fix this by handing a response CRC error slightly differently: record
> the failure of the data initiating command, but allow the remainder of
> the request to be processed normally.  This is safe as core MMC checks

"processed normally" confused me at first because it sounded like you are
ignoring the error.  Not sure why you have a much better explanation in the
cover email than here.

> the status of all commands and data transfer phases of the request.

MMC core is not the only initiator of requests, but it is safe because the
command error takes precedence by design.

Also you don't explain why it is better to continue rather than attempt to
send a stop command and clean up the request properly.  It looks simpler and
less racy, but if that is the reason then it seems worth saying so.

> 
> If the card does not initiate a data transfer, then we should time out
> according to the data transfer parameters.
> 
> Signed-off-by: Russell King 
> ---
>  drivers/mmc/host/sdhci.c | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 86310b162304..3e718e465a1b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2340,6 +2340,23 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 
> intmask, u32 *mask)
>   else
>   host->cmd->error = -EILSEQ;
>  
> + /*
> +  * If this command initiates a data phase and a response
> +  * CRC error is signalled, the card can start transferring
> +  * data - the card may have received the command without
> +  * error.  We must not terminate the request early.

This is misleading.  We could terminate the request early if we cleaned it
up.  You should say here why it is better to continue.

> +  *
> +  * If the card did not receive the command, the data phase
> +  * will time out.
> +  *
> +  * FIXME: we also need to clean up the data phase if any
> +  * command fails, not just the data initiating command.

This FIXME is too vague.  Please give at least one example of what needs fixing.

> +  */
> + if (host->cmd->data && intmask & SDHCI_INT_CRC) {
> + host->cmd = NULL;
> + return;
> + }
> +
>   tasklet_schedule(>finish_tasklet);
>   return;
>   }
> 

--
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 v2 08/24] mmc: sdhci: avoid unnecessary mapping/unmapping of align buffer

[Adrian Hunter]

On 21/12/15 13:41, Russell King wrote:
> Unnecessarily mapping and unmapping the align buffer for SD cards is
> expensive: performance measurements on iMX6 show that this gives a hit
> of 10% on hdparm buffered disk reads.
> 
> MMC/SD card IO comes from the mm/vfs which gives us page based IO, so
> for this case, the align buffer is not going to be used.  However, we
> still map and unmap this buffer.
> 
> Eliminate this by switching the align buffer to be a DMA coherent
> buffer, which needs no DMA maintenance to access the buffer.

Did you consider putting the align buffer in the same allocation
as the adma_table?

> 
> Signed-off-by: Russell King 
> ---
>  drivers/mmc/host/sdhci.c | 53 
> 
>  1 file changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 3e718e465a1b..8a4eb1c41f3e 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -465,8 +465,6 @@ static void sdhci_adma_mark_end(void *desc)
>  static int sdhci_adma_table_pre(struct sdhci_host *host,
>   struct mmc_data *data)
>  {
> - int direction;
> -
>   void *desc;
>   void *align;
>   dma_addr_t addr;
> @@ -483,20 +481,9 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
>* We currently guess that it is LE.
>*/
>  
> - if (data->flags & MMC_DATA_READ)
> - direction = DMA_FROM_DEVICE;
> - else
> - direction = DMA_TO_DEVICE;
> -
> - host->align_addr = dma_map_single(mmc_dev(host->mmc),
> - host->align_buffer, host->align_buffer_sz, direction);
> - if (dma_mapping_error(mmc_dev(host->mmc), host->align_addr))
> - goto fail;
> - BUG_ON(host->align_addr & host->align_mask);
> -
>   host->sg_count = sdhci_pre_dma_transfer(host, data);
>   if (host->sg_count < 0)
> - goto unmap_align;
> + return -EINVAL;
>  
>   desc = host->adma_table;
>   align = host->align_buffer;
> @@ -567,22 +554,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
>   /* nop, end, valid */
>   sdhci_adma_write_desc(host, desc, 0, 0, ADMA2_NOP_END_VALID);
>   }
> -
> - /*
> -  * Resync align buffer as we might have changed it.
> -  */
> - if (data->flags & MMC_DATA_WRITE) {
> - dma_sync_single_for_device(mmc_dev(host->mmc),
> - host->align_addr, host->align_buffer_sz, direction);
> - }
> -
>   return 0;
> -
> -unmap_align:
> - dma_unmap_single(mmc_dev(host->mmc), host->align_addr,
> - host->align_buffer_sz, direction);
> -fail:
> - return -EINVAL;
>  }
>  
>  static void sdhci_adma_table_post(struct sdhci_host *host,
> @@ -602,9 +574,6 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
>   else
>   direction = DMA_TO_DEVICE;
>  
> - dma_unmap_single(mmc_dev(host->mmc), host->align_addr,
> - host->align_buffer_sz, direction);
> -
>   /* Do a quick scan of the SG list for any unaligned mappings */
>   has_unaligned = false;
>   for_each_sg(data->sg, sg, host->sg_count, i)
> @@ -3003,6 +2972,10 @@ int sdhci_add_host(struct sdhci_host *host)
> host->adma_table_sz,
> >adma_addr,
> GFP_KERNEL);
> + host->align_buffer = dma_alloc_coherent(mmc_dev(mmc),
> + host->align_buffer_sz,
> + >align_addr,
> + GFP_KERNEL);
>   host->align_buffer = kmalloc(host->align_buffer_sz, GFP_KERNEL);

kmalloc line is still there

>   if (!host->adma_table || !host->align_buffer) {
>   if (host->adma_table)
> @@ -3010,7 +2983,11 @@ int sdhci_add_host(struct sdhci_host *host)
> host->adma_table_sz,
> host->adma_table,
> host->adma_addr);
> - kfree(host->align_buffer);
> + if (host->align_buffer)
> + dma_free_coherent(mmc_dev(mmc),
> +   host->align_buffer_sz,
> +   host->align_buffer,
> +   host->align_addr);
>   pr_warn("%s: Unable to allocate ADMA buffers - falling 
> back to standard DMA\n",
>   mmc_hostname(mmc));
>   host->flags &= ~SDHCI_USE_ADMA;
> @@ -3022,10 +2999,14 @@ int sdhci_add_host(struct sdhci_host *host)
>   host->flags &= 

Re: [PATCH RESEND] mmc:Fix error handling in the function mmc_blk_issue_rq

[Adrian Hunter]

On 26/12/15 22:10, Nicholas Krause wrote:
> This fixes error handling in the function mmc_blk_issue_rq for
> checking if the calls to the function mmc_blk_issue_rw_rq have
> failed by returning zero as there return value and if so jump
> immediately to the goto label out for cleanup of resources used
> and allocated by the function mmc_blk_issue_rq before returning
> this return value to the caller of this particular function to
> indicate to the caller that it's call(s) have failed.

Are you experiencing a problem with the existing code, because it looks more
correct to me?

There does seem to be a gap in the way MMC_QUEUE_NEW_REQUEST is handled,
since the code is expecting that mmc_blk_issue_rw_rq() will have done
waiting for any ongoing request, but it is not clear it will have if
MMC_QUEUE_NEW_REQUEST is possible.  But that is a different issue.

> 
> Signed-off-by: Nicholas Krause 
> ---
>  drivers/mmc/card/block.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index a1b820f..354a151 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2055,7 +2055,9 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, 
> struct request *req)
>   if (cmd_flags & REQ_DISCARD) {
>   /* complete ongoing async transfer before issuing discard */
>   if (card->host->areq)
> - mmc_blk_issue_rw_rq(mq, NULL);
> + ret = mmc_blk_issue_rw_rq(mq, NULL);
> + if (!ret)
> + goto out;
>   if (req->cmd_flags & REQ_SECURE)
>   ret = mmc_blk_issue_secdiscard_rq(mq, req);
>   else
> @@ -2063,7 +2065,9 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, 
> struct request *req)
>   } else if (cmd_flags & REQ_FLUSH) {
>   /* complete ongoing async transfer before issuing flush */
>   if (card->host->areq)
> - mmc_blk_issue_rw_rq(mq, NULL);
> + ret = mmc_blk_issue_rw_rq(mq, NULL);
> + if (!ret)
> + goto out;
>   ret = mmc_blk_issue_flush(mq, req);
>   } else {
>   if (!req && host->areq) {
> 

--
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: It is not an error for the card to be removed while suspended

[Adrian Hunter]

On 15/12/15 06:01, Jaehoon Chung wrote:
> On 12/14/2015 10:51 PM, Adrian Hunter wrote:
>> A card can be removed while it is runtime suspended.
>> Do not print an error message.
> 
> Well, if card is non-removable (in case of eMMC, card can be non-removable),
>  it needs to print the error message?
> I'm not sure what's correct..but just my opinion.

I guess this patch should have been a "V2" but the original was so long ago.
Ulf's comment about your concern is here:

http://marc.info/?l=linux-mmc=141163363902017=2


>>
>> Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
>> ---
>>  drivers/mmc/core/mmc.c | 2 +-
>>  drivers/mmc/core/sd.c  | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 549c56e8cf6b..bf49e44571f2 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1935,7 +1935,7 @@ static int mmc_runtime_resume(struct mmc_host *host)
>>  int err;
>>  
>>  err = _mmc_resume(host);
>> -if (err)
>> +if (err && err != -ENOMEDIUM)
>>  pr_err("%s: error %d doing runtime resume\n",
>>  mmc_hostname(host), err);
>>  
>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> index 8f3b46a56b3d..f2b164b214ae 100644
>> --- a/drivers/mmc/core/sd.c
>> +++ b/drivers/mmc/core/sd.c
>> @@ -1158,7 +1158,7 @@ static int mmc_sd_runtime_resume(struct mmc_host *host)
>>  int err;
>>  
>>  err = _mmc_sd_resume(host);
>> -if (err)
>> +if (err && err != -ENOMEDIUM)
>>  pr_err("%s: error %d doing runtime resume\n",
>>  mmc_hostname(host), err);
>>  
>>
> 
> 

--
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: 4.3 kernel panics when MMC/SDHC card is inserted on thinkpad

[Adrian Hunter]

: 
>> 
>> [22946.955131] R10: 563f08fc R11: 1849050d R12: 
>> 880231e7d098
>> [22946.958423] R13: 8800bacbbc20 R14: fffebda0 R15: 
>> 
>> [22946.961723] FS:  () GS:88023bd8()
>> knlGS:
>> [22946.965051] CS:  0010 DS:  ES:  CR0: 8005003b
>> [22946.968387] CR2: e4d9c0e0 CR3: 01a0c000 CR4: 
>> 06e0
>> [22946.971760] Stack:
>> [22946.975131]  8800bacbbc60  880231ea5580
>> 880231ea5580
>> [22946.978598]  8800bacbbc20 0010 
>> 88023bd83df0
>> [22946.982064]  813cad22 88023bd83e48 c01090c2
>> 0282
>> [22946.985546] Call Trace:
>> [22946.988984]  
>> [22946.989016]  [] intel_unmap_sg+0x12/0x20
>> [22946.995844]  [] sdhci_finish_data+0x142/0x340 [sdhci]
>> [22946.999296]  [] sdhci_irq+0x484/0x9b5 [sdhci]
>> [22947.002759]  [] ? notifier_call_chain+0x4a/0x70
>> [22947.006222]  [] handle_irq_event_percpu+0x39/0x1b0
>> [22947.009694]  [] handle_irq_event+0x40/0x60
>> [22947.013160]  [] handle_fasteoi_irq+0xc2/0x180
>> [22947.016633]  [] handle_irq+0x1a/0x30
>> [22947.020095]  [] do_IRQ+0x57/0xf0
>> [22947.023553]  [] common_interrupt+0x81/0x81
>> [22947.026992]  
>> [22947.027023]  [] ? cpuidle_enter_state+0x13e/0x2b0
>> [22947.033852]  [] ? cpuidle_enter_state+0x133/0x2b0
>> [22947.037286]  [] cpuidle_enter+0x17/0x20
>> [22947.040717]  [] call_cpuidle+0x32/0x60
>> [22947.044131]  [] ? cpuidle_select+0x13/0x20
>> [22947.047554]  [] cpu_startup_entry+0x29e/0x360
>> [22947.050969]  [] start_secondary+0x15b/0x190
>> [22947.054379] Code: 01 44 29 f1 e8 12 c6 ff ff 4c 89 ee 4c 89 ff e8
>> b7 8d ff ff 4c 89 e7 e8 0f c7 ff ff 48 83 c4 10 5b 41 5c 41 5d 41 5e
>> 41 5f 5d c3 <0f> 0b 49 8b 54 24 50 48 85 d2 74 29 4c 8b 45 d0 4c 89 f1
>> 48 c7
>> [22947.058834] RIP  [] intel_unmap+0x1d0/0x210
>> [22947.062568]  RSP 
>> [22947.066285] ---[ end trace 12b22e7424e94db4 ]---
>> [22947.06] Kernel panic - not syncing: Fatal exception in interrupt
>> [22947.073803] Kernel Offset: disabled
>> [22947.077240] ---[ end Kernel panic - not syncing: Fatal exception in 
>> interrupt
>>
> 
> Hi Denis,
> 
> Thanks for reporting and sorry for the delay!
> 
> Unfortunate, this isn't really my area of expertise and I don't have
> the HW. In other words, I don't think I will be able to help much.
> 
> Instead, I am looping in Adrian Hunter, who might be able to have a
> look at this.

Have you tried bisecting to find the commit that causes this?

--
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] mmc: It is not an error for the card to be removed while suspended

[Adrian Hunter]

A card can be removed while it is runtime suspended.
Do not print an error message.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/core/mmc.c | 2 +-
 drivers/mmc/core/sd.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 549c56e8cf6b..bf49e44571f2 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1935,7 +1935,7 @@ static int mmc_runtime_resume(struct mmc_host *host)
int err;
 
err = _mmc_resume(host);
-   if (err)
+   if (err && err != -ENOMEDIUM)
pr_err("%s: error %d doing runtime resume\n",
mmc_hostname(host), err);
 
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 8f3b46a56b3d..f2b164b214ae 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1158,7 +1158,7 @@ static int mmc_sd_runtime_resume(struct mmc_host *host)
int err;
 
err = _mmc_sd_resume(host);
-   if (err)
+   if (err && err != -ENOMEDIUM)
pr_err("%s: error %d doing runtime resume\n",
mmc_hostname(host), err);
 
-- 
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] mmc: sdhci-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 10/12/15 22:57, Philip Elcan wrote:
> 
> On 12/07/2015 03:30 AM, Adrian Hunter wrote:
>> On 04/12/15 17:40, Philip Elcan wrote:
>>> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>>>> On 03/12/15 15:48, Philip Elcan wrote:
>>>>> This allows setting an SDHC controller as non-removable
>>>>> by using the _RMV method in the ACPI table. It doesn't
>>>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. 
>>>> child
>>>> device node?
>>> Yes, this is on the host controller. The ACPI table only describes the
>>> host controller, not the child nodes.
>>>
>> If you look at Intel devices, the _RMV is on the child e.g.
>>
>> Device (SDHA)
>> {
>> Name (_HID, "80860F14")  // _HID: Hardware ID
>> Name (_CID, "PNP0D40")  // _CID: Compatible ID
>> Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS 
>> Device Name
>>  ...
>> Device (EMMD)
>> {
>>  ...
>> Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
>> {
>> Return (Zero)
>> }
>> }
>> }
>>
>> I am not an ACPI expert but that seems like the correct place for it.
> My understanding is that in ACPI you don't generally create child devices on 
> buses that are discoverable.

I've cc'ed Rafael and the linux-acpi mailing list.  Maybe someone there can
comment.

>>
>>>>> mark it as non-removable if GPIO card detection is
>>>>> already setup.
>>>>>
>>>>> Signed-off-by: Philip Elcan <pel...@codeaurora.org>
>>>>> ---
>>>>>
> 

--
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-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 04/12/15 17:40, Philip Elcan wrote:
> 
> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>> On 03/12/15 15:48, Philip Elcan wrote:
>>> This allows setting an SDHC controller as non-removable
>>> by using the _RMV method in the ACPI table. It doesn't
>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
>> device node?
> 
> Yes, this is on the host controller. The ACPI table only describes the
> host controller, not the child nodes.
> 

If you look at Intel devices, the _RMV is on the child e.g.

Device (SDHA)
{
Name (_HID, "80860F14")  // _HID: Hardware ID
Name (_CID, "PNP0D40")  // _CID: Compatible ID
Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS 
Device Name
...
Device (EMMD)
{
...
Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
{
Return (Zero)
}
}
}

I am not an ACPI expert but that seems like the correct place for it.

>>
>>> mark it as non-removable if GPIO card detection is
>>> already setup.
>>>
>>> Signed-off-by: Philip Elcan <pel...@codeaurora.org>
>>> ---
>>>
> 

--
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-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 03/12/15 15:48, Philip Elcan wrote:
> This allows setting an SDHC controller as non-removable
> by using the _RMV method in the ACPI table. It doesn't

Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
device node?

> mark it as non-removable if GPIO card detection is
> already setup.
> 
> Signed-off-by: Philip Elcan 
> ---
>  drivers/mmc/host/sdhci-acpi.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index f6047fc..8c06ba6 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -288,6 +288,20 @@ static const struct sdhci_acpi_slot 
> *sdhci_acpi_get_slot(const char *hid,
>   return NULL;
>  }
>  
> +static bool sdhci_acpi_is_removable(acpi_handle handle)
> +{
> + acpi_status status;
> + unsigned long long removable = 1; /* default to removable */
> +
> + if (acpi_has_method(handle, "_EJ0"))
> + return true;
> + status = acpi_evaluate_integer(handle, "_RMV", NULL, );
> + if (ACPI_SUCCESS(status) && !removable)
> + return false;
> +
> + return true;
> +}
> +
>  static int sdhci_acpi_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> @@ -300,6 +314,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>   const char *hid;
>   const char *uid;
>   int err;
> + bool gpio_cd = false;
>  
>   if (acpi_bus_get_device(handle, ))
>   return -ENODEV;
> @@ -373,9 +388,14 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>   if (mmc_gpiod_request_cd(host->mmc, NULL, 0, v, 0, NULL)) {
>   dev_warn(dev, "failed to setup card detect gpio\n");
>   c->use_runtime_pm = false;
> + } else {
> + gpio_cd = true;
>   }
>   }
>  
> + if (!gpio_cd && !sdhci_acpi_is_removable(handle))
> + host->mmc->caps |= MMC_CAP_NONREMOVABLE;
> +
>   err = sdhci_add_host(host);
>   if (err)
>   goto err_free;
> 

--
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/7] mmc: mmc: Fix incorrect use of driver strength switching HS200 and HS400

[Adrian Hunter]

From: Wenkai Du <wenkai...@intel.com>

Commit cc4f414c885c ("mmc: mmc: Add driver strength selection")
added driver strength selection for eMMC HS200 and HS400 modes.
That patch also set the driver stength when transitioning through
High Speed mode to HS200/HS400, but driver strength is not defined
for High Speed mode.  While the JEDEC specification is not clear
on this point it has been observed to cause problems for some eMMC,
and removing the driver strength setting in this case makes it
consistent with the normal use of High Speed mode.

Signed-off-by: Wenkai Du <wenkai...@intel.com>
Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
Cc: sta...@vger.kernel.org # v4.2+
---
 drivers/mmc/core/mmc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 66957addf9e4..549c56e8cf6b 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1076,8 +1076,7 @@ static int mmc_select_hs400(struct mmc_card *card)
mmc_set_clock(host, max_dtr);
 
/* Switch card to HS mode */
-   val = EXT_CSD_TIMING_HS |
- card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+   val = EXT_CSD_TIMING_HS;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
   EXT_CSD_HS_TIMING, val,
   card->ext_csd.generic_cmd6_time,
@@ -1160,8 +1159,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
mmc_set_clock(host, max_dtr);
 
/* Switch HS400 to HS DDR */
-   val = EXT_CSD_TIMING_HS |
- card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
+   val = EXT_CSD_TIMING_HS;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
   val, card->ext_csd.generic_cmd6_time,
   true, send_status, true);
-- 
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

[PATCH 0/7] mmc: Some fixes

[Adrian Hunter]

Hi

Here are some fixes.  The important ones have cc: stable.
It doesn't matter to me whether you queue them as fixes
or for v4.5.


Adrian Hunter (6):
  mmc: sdhci-pci: Do not default to 33 Ohm driver strength for Intel SPT
  mmc: sdhci: Do not BUG on invalid vdd
  mmc: sdio: Fix invalid vdd in voltage switch power cycle
  mmc: sdhc: Fix DMA descriptor with zero data length
  mmc: sdhci: 64-bit DMA actually has 4-byte alignment
  mmc: sdhci: Fix sdhci_runtime_pm_bus_on/off()

Wenkai Du (1):
  mmc: mmc: Fix incorrect use of driver strength switching HS200 and HS400

 drivers/mmc/core/mmc.c|  6 ++---
 drivers/mmc/core/sdio.c   |  2 +-
 drivers/mmc/host/sdhci-pci-core.c |  2 +-
 drivers/mmc/host/sdhci.c  | 49 +++
 drivers/mmc/host/sdhci.h  | 21 ++---
 5 files changed, 40 insertions(+), 40 deletions(-)


Regards
Adrian
--
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 3/7] mmc: sdhci: Do not BUG on invalid vdd

[Adrian Hunter]

The driver may not be able to set the power correctly but that
is not a reason to BUG().

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/host/sdhci.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2b17cc1246ca..5f8b0766428c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1299,7 +1299,10 @@ static void sdhci_set_power(struct sdhci_host *host, 
unsigned char mode,
pwr = SDHCI_POWER_330;
break;
default:
-   BUG();
+   WARN(1, "%s: Invalid vdd %#x\n",
+mmc_hostname(host->mmc), vdd);
+   pwr = 0;
+   break;
}
}
 
-- 
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

[PATCH 4/7] mmc: sdio: Fix invalid vdd in voltage switch power cycle

[Adrian Hunter]

The 'ocr' parameter passed to mmc_set_signal_voltage()
defines the power-on voltage used when power cycling
after a failure to set the voltage.  However, in the
case of mmc_sdio_init_card(), the value passed has the
R4_18V_PRESENT flag set which is not valid for power-on
and results in an invalid vdd.  Fix by passing the card's
ocr value which does not have the flag.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
Cc: sta...@vger.kernel.org # v3.13+
---
 drivers/mmc/core/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 16d838e6d623..d61ba1a0495e 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -630,7 +630,7 @@ try_again:
 */
if (!powered_resume && (rocr & ocr & R4_18V_PRESENT)) {
err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180,
-   ocr);
+   ocr_card);
if (err == -EAGAIN) {
sdio_reset(host);
mmc_go_idle(host);
-- 
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

[PATCH 6/7] mmc: sdhci: 64-bit DMA actually has 4-byte alignment

[Adrian Hunter]

The version 3.00 SDHCI spec. was a bit unclear about the
required data alignment for 64-bit DMA, whereas the version
4.10 spec. uses different language and indicates that only
4-byte alignment is required rather than the 8-byte alignment
currently implemented.  That make no difference to SD and EMMC
which invariably transfer data in sector-aligned blocks.
However with SDIO, it results in using more DMA descriptors
than necessary.  Theoretically that slows DMA slightly although
DMA is not the limiting factor for throughput, so there is no
discernable impact on performance.  Nevertheless, the driver
should follw the spec unless there is good reason not to, so
this patch corrects the alignment criterion.

There is a more complicated criterion for the DMA descriptor
table itself.  However the table is allocated by dma_alloc_coherent()
which allocates pages (i.e. aligned to a page boundary).
For simplicity just check it is 8-byte aligned, but add a comment
that some Intel controllers actually require 8-byte alignment
even when using 32-bit DMA.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/host/sdhci.c | 31 ---
 drivers/mmc/host/sdhci.h | 21 -
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6ac5db8e86ab..7fe326f76fb7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -492,7 +492,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
host->align_buffer, host->align_buffer_sz, direction);
if (dma_mapping_error(mmc_dev(host->mmc), host->align_addr))
goto fail;
-   BUG_ON(host->align_addr & host->align_mask);
+   BUG_ON(host->align_addr & SDHCI_ADMA2_MASK);
 
host->sg_count = sdhci_pre_dma_transfer(host, data);
if (host->sg_count < 0)
@@ -514,8 +514,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 * the (up to three) bytes that screw up the
 * alignment.
 */
-   offset = (host->align_sz - (addr & host->align_mask)) &
-host->align_mask;
+   offset = (SDHCI_ADMA2_ALIGN - (addr & SDHCI_ADMA2_MASK)) &
+SDHCI_ADMA2_MASK;
if (offset) {
if (data->flags & MMC_DATA_WRITE) {
buffer = sdhci_kmap_atomic(sg, );
@@ -529,8 +529,8 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 
BUG_ON(offset > 65536);
 
-   align += host->align_sz;
-   align_addr += host->align_sz;
+   align += SDHCI_ADMA2_ALIGN;
+   align_addr += SDHCI_ADMA2_ALIGN;
 
desc += host->desc_sz;
 
@@ -611,7 +611,7 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
/* Do a quick scan of the SG list for any unaligned mappings */
has_unaligned = false;
for_each_sg(data->sg, sg, host->sg_count, i)
-   if (sg_dma_address(sg) & host->align_mask) {
+   if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
has_unaligned = true;
break;
}
@@ -623,15 +623,15 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
align = host->align_buffer;
 
for_each_sg(data->sg, sg, host->sg_count, i) {
-   if (sg_dma_address(sg) & host->align_mask) {
-   size = host->align_sz -
-  (sg_dma_address(sg) & host->align_mask);
+   if (sg_dma_address(sg) & SDHCI_ADMA2_MASK) {
+   size = SDHCI_ADMA2_ALIGN -
+  (sg_dma_address(sg) & SDHCI_ADMA2_MASK);
 
buffer = sdhci_kmap_atomic(sg, );
memcpy(buffer, align, size);
sdhci_kunmap_atomic(buffer, );
 
-   align += host->align_sz;
+   align += SDHCI_ADMA2_ALIGN;
}
}
}
@@ -2962,24 +2962,17 @@ int sdhci_add_host(struct sdhci_host *host)
if (host->flags & SDHCI_USE_64_BIT_DMA) {
host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) *
  SDHCI_ADMA2_64_DESC_SZ;
-   host->align_buffer_sz = SDHCI_MAX_SEGS *
-   SDHCI_ADMA2_64_ALIGN;
host->desc_sz = SDHCI_ADMA2_64_DESC_SZ;
-   host->align_sz = SDHCI_ADMA2_64_ALIGN;
- 

[PATCH 5/7] mmc: sdhc: Fix DMA descriptor with zero data length

[Adrian Hunter]

SDHCI has built-in DMA called ADMA2.  ADMA2 uses a descriptor
table to define DMA scatter-gather.  Each desciptor can specify
a data length up to 65536 bytes, however the length field is
only 16-bits so zero means 65536.  Consequently, putting zero
when the size is zero must not be allowed.  This patch fixes
one case where zero data length could be set inadvertently.

The problem happens because unaligned data gets split and the
code did not consider that the remaining aligned portion might
be zero length.  That case really only happens for SDIO because
SD and eMMC cards transfer blocks that are invariably sector-
aligned.  For SDIO, access to function registers is done by
data transfer (CMD53) when the register is bigger than 1 byte.
Generally registers are 4 bytes but 2-byte registers are possible.
So DMA of 4 bytes or less can happen.  When 32-bit DMA is used,
the data alignment must be 4, so 4-byte transfers won't casue a
problem, but a 2-byte transfer could.  However with the introduction
of 64-bit DMA, the data alignment for 64-bit DMA was made 8 bytes,
so all 4-byte transfers not on 8-byte boundaries get "split" into
a 4-byte chunk and a 0-byte chunk, thereby hitting the bug.

In fact, a closer look at the SDHCI specs indicates that only the
descriptor table requires 8-byte alignment for 64-bit DMA.  That
will be dealt with in a separate patch, but the potential for a
2-byte access remains, so this fix is needed anyway.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
Cc: sta...@vger.kernel.org # v3.19+
---
 drivers/mmc/host/sdhci.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5f8b0766428c..6ac5db8e86ab 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -540,9 +540,12 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
 
BUG_ON(len > 65536);
 
-   /* tran, valid */
-   sdhci_adma_write_desc(host, desc, addr, len, ADMA2_TRAN_VALID);
-   desc += host->desc_sz;
+   if (len) {
+   /* tran, valid */
+   sdhci_adma_write_desc(host, desc, addr, len,
+ ADMA2_TRAN_VALID);
+   desc += host->desc_sz;
+   }
 
/*
 * If this triggers then we have a calculation bug
-- 
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

[PATCH 2/7] mmc: sdhci-pci: Do not default to 33 Ohm driver strength for Intel SPT

[Adrian Hunter]

In some cases, the stronger 33 Ohm driver strength must not be used
so it is not a suitable default.  Change it to the standard default
50 Ohm value.

The patch applies to v4.2+ except the file name changed.  It is
drivers/mmc/host/sdhci-pci.c prior to v.4.4.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
Cc: sta...@vger.kernel.org # v4.2+
---
 drivers/mmc/host/sdhci-pci-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c 
b/drivers/mmc/host/sdhci-pci-core.c
index cf7ad458b4f4..08f4a9fe8550 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -277,7 +277,7 @@ static int spt_select_drive_strength(struct sdhci_host 
*host,
if (sdhci_pci_spt_drive_strength > 0)
drive_strength = sdhci_pci_spt_drive_strength & 0xf;
else
-   drive_strength = 1; /* 33-ohm */
+   drive_strength = 0; /* Default 50-ohm */
 
if ((mmc_driver_type_mask(drive_strength) & card_drv) == 0)
drive_strength = 0; /* Default 50-ohm */
-- 
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 0/2] sdio: Prevent re-tuning conflicting with custom sleep

[Adrian Hunter]

On 28/04/15 17:45, Adrian Hunter wrote:
> Here are 2 patches that allow brcmfmac to awake from a
> custom sleep state that conflicts with re-tuning.
> 
> The first patch adds sdio_retune_hold_now() and
> sdio_retune_release(). They are used in the 2nd patch
> to prevent re-tuning for the 'wake-up' command.
> 
> 
> Adrian Hunter (2):
>   mmc: core: Add functions for SDIO to hold re-tuning
>   brcmfmac: Prevent re-tuning conflicting with 'wake-up'
> 
>  drivers/mmc/core/host.c|  6 ++
>  drivers/mmc/core/host.h|  1 +
>  drivers/mmc/core/sdio_io.c | 13 +
>  drivers/net/wireless/brcm80211/brcmfmac/sdio.c |  6 ++
>  include/linux/mmc/sdio_func.h  |  3 +++
>  5 files changed, 29 insertions(+)

Hi

These patches still apply and are still needed.
Can they be applied?

Regards
Adrian

--
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 0/4] mmc: mmc: Improve reliability of HS200/HS400 selection

[Adrian Hunter]

On 06/11/15 13:03, Ulf Hansson wrote:
> On 28 October 2015 at 13:25, Adrian Hunter <adrian.hun...@intel.com> wrote:
>> Hi
>>
>> Here are some patches that improve the reliability of selecting
>> eMMC HS200 or HS400 mode.
>>
>> Generally the improvement is to match the host controller timing
>> setting with the card mode before sending commands.
>>
>>
>> Adrian Hunter (4):
>>   mmc: mmc: Improve reliability of mmc_select_hs200()
>>   mmc: mmc: Fix HS setting in mmc_select_hs400()
>>   mmc: mmc: Move mmc_switch_status()
>>   mmc: mmc: Improve reliability of mmc_select_hs400()
>>
>>  drivers/mmc/core/mmc.c | 93 
>> +-
>>  1 file changed, 69 insertions(+), 24 deletions(-)
>>
> 

Thanks for looking at the patches.

> Adrian, these patches looks good to me. I plan to queue them for 4.5
> but perhaps those should even go as fixes!?

4.5 is fine for me.  A quick check seemed to show the patches apply
cleanly back to 4.2, so I guess you could cc stable and then the patches
get exposure in next before going to stable.

> 
> Additionally, it would be great if someone could help testing this on
> a few platforms.

Yes, indeed.

--
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 0/4] mmc: mmc: Improve reliability of HS200/HS400 selection

[Adrian Hunter]

Hi

Here are some patches that improve the reliability of selecting
eMMC HS200 or HS400 mode.

Generally the improvement is to match the host controller timing
setting with the card mode before sending commands.


Adrian Hunter (4):
  mmc: mmc: Improve reliability of mmc_select_hs200()
  mmc: mmc: Fix HS setting in mmc_select_hs400()
  mmc: mmc: Move mmc_switch_status()
  mmc: mmc: Improve reliability of mmc_select_hs400()

 drivers/mmc/core/mmc.c | 93 +-
 1 file changed, 69 insertions(+), 24 deletions(-)


Regards
Adrian
--
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 3/4] mmc: mmc: Move mmc_switch_status()

[Adrian Hunter]

Move the mmc_switch_status() function in preparation for calling it
in mmc_select_hs400().

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/core/mmc.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 14fb767ee8dd..5884c79346da 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1040,6 +1040,19 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
return err;
 }
 
+/* Caller must hold re-tuning */
+static int mmc_switch_status(struct mmc_card *card)
+{
+   u32 status;
+   int err;
+
+   err = mmc_send_status(card, );
+   if (err)
+   return err;
+
+   return mmc_switch_status_error(card->host, status);
+}
+
 static int mmc_select_hs400(struct mmc_card *card)
 {
struct mmc_host *host = card->host;
@@ -1107,19 +1120,6 @@ int mmc_hs200_to_hs400(struct mmc_card *card)
return mmc_select_hs400(card);
 }
 
-/* Caller must hold re-tuning */
-static int mmc_switch_status(struct mmc_card *card)
-{
-   u32 status;
-   int err;
-
-   err = mmc_send_status(card, );
-   if (err)
-   return err;
-
-   return mmc_switch_status_error(card->host, status);
-}
-
 int mmc_hs400_to_hs200(struct mmc_card *card)
 {
struct mmc_host *host = card->host;
-- 
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

[PATCH 1/4] mmc: mmc: Improve reliability of mmc_select_hs200()

[Adrian Hunter]

Currently mmc_select_hs200() uses __mmc_switch() which checks the
success of the switch to HS200 mode using CMD13 (SEND_STATUS).
The problem is that it does that using the timing settings of legacy
mode.  That is prone to error, not least because the timing parameters
for legacy mode are tighter than the timing parameters for HS200 mode.

In the case when CMD13 polling is used (i.e. not MMC_CAP_WAIT_WHILE_BUSY)
with the switch command, it must be assumed that using different modes on
the card and host must work.

However in the case when CMD13 polling is not used
(i.e. MMC_CAP_WAIT_WHILE_BUSY) mmc_select_hs200() can be made more
reliable by setting the host to the correct timing before sending CMD13.

This patch does that.

A complication is that the caller, mmc_select_timing(), will ignore a
switch error (indicated by -EBADMSG), assume the old mode is valid
and continue, so the old timing must be restored in that case.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/core/mmc.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c793fda27321..2cef40ce9851 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1219,6 +1219,8 @@ static void mmc_select_driver_type(struct mmc_card *card)
 static int mmc_select_hs200(struct mmc_card *card)
 {
struct mmc_host *host = card->host;
+   bool send_status = true;
+   unsigned int old_timing;
int err = -EINVAL;
u8 val;
 
@@ -1234,6 +1236,9 @@ static int mmc_select_hs200(struct mmc_card *card)
 
mmc_select_driver_type(card);
 
+   if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
+   send_status = false;
+
/*
 * Set the bus width(4 or 8) with host's support and
 * switch to HS200 mode if bus width is set successfully.
@@ -1245,11 +1250,25 @@ static int mmc_select_hs200(struct mmc_card *card)
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
   EXT_CSD_HS_TIMING, val,
   card->ext_csd.generic_cmd6_time,
-  true, true, true);
-   if (!err)
-   mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+  true, send_status, true);
+   if (err)
+   goto err;
+   old_timing = host->ios.timing;
+   mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   /*
+* mmc_select_timing() assumes timing has not changed if
+* it is a switch error.
+*/
+   if (err == -EBADMSG)
+   mmc_set_timing(host, old_timing);
+   }
}
 err:
+   if (err)
+   pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
+  __func__, err);
return err;
 }
 
-- 
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

[PATCH 2/4] mmc: mmc: Fix HS setting in mmc_select_hs400()

[Adrian Hunter]

mmc_select_hs400() begins with the card and host in HS200 mode.
Therefore, any commands sent to the card should use HS200 timing.
It is incorrect to set the host to High Speed (HS) timing before
sending the switch command.  Doing so is unreliable because
the timing parameters for HS mode are tighter than the timing
parameters for HS200 mode.  Thus the HS timings should be set
only after the card has switched mode.

However, it is not unreasonable first to reduce the frequency to
the HS mode frequency, which should make the switch command and
subsequent CMD13 commands more reliable.

This patch does that.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/core/mmc.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 2cef40ce9851..14fb767ee8dd 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1043,6 +1043,7 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
 static int mmc_select_hs400(struct mmc_card *card)
 {
struct mmc_host *host = card->host;
+   unsigned int max_dtr;
int err = 0;
u8 val;
 
@@ -1053,13 +1054,11 @@ static int mmc_select_hs400(struct mmc_card *card)
  host->ios.bus_width == MMC_BUS_WIDTH_8))
return 0;
 
-   /*
-* Before switching to dual data rate operation for HS400,
-* it is required to convert from HS200 mode to HS mode.
-*/
-   mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
-   mmc_set_bus_speed(card);
+   /* Reduce frequency to HS frequency */
+   max_dtr = card->ext_csd.hs_max_dtr;
+   mmc_set_clock(host, max_dtr);
 
+   /* Switch card to HS mode */
val = EXT_CSD_TIMING_HS |
  card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
@@ -1072,6 +1071,9 @@ static int mmc_select_hs400(struct mmc_card *card)
return err;
}
 
+   /* Set host controller to HS timing */
+   mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
+
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 EXT_CSD_BUS_WIDTH,
 EXT_CSD_DDR_BUS_WIDTH_8,
-- 
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

[PATCH 4/4] mmc: mmc: Improve reliability of mmc_select_hs400()

[Adrian Hunter]

mmc_select_hs400() calls __mmc_switch() which checks the switch is
successful using CMD13 (SEND_STATUS).  The problem is that it does that
using the timing settings of the previous mode.  That is prone to error,
especially when switching from HS to HS400 because the timing parameters
for HS mode are tighter than the timing parameters for HS400 mode.

In the case when CMD13 polling is used (i.e. not MMC_CAP_WAIT_WHILE_BUSY)
with the switch command, it must be assumed that using different modes on
the card and host must work.

However in the case when CMD13 polling is not used
(i.e. MMC_CAP_WAIT_WHILE_BUSY) mmc_select_hs400() can be made more
reliable by setting the host to the correct timing before sending CMD13.

This patch does that.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/core/mmc.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 5884c79346da..3a9a79ec4343 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1056,6 +1056,7 @@ static int mmc_switch_status(struct mmc_card *card)
 static int mmc_select_hs400(struct mmc_card *card)
 {
struct mmc_host *host = card->host;
+   bool send_status = true;
unsigned int max_dtr;
int err = 0;
u8 val;
@@ -1067,6 +1068,9 @@ static int mmc_select_hs400(struct mmc_card *card)
  host->ios.bus_width == MMC_BUS_WIDTH_8))
return 0;
 
+   if (host->caps & MMC_CAP_WAIT_WHILE_BUSY)
+   send_status = false;
+
/* Reduce frequency to HS frequency */
max_dtr = card->ext_csd.hs_max_dtr;
mmc_set_clock(host, max_dtr);
@@ -1077,7 +1081,7 @@ static int mmc_select_hs400(struct mmc_card *card)
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
   EXT_CSD_HS_TIMING, val,
   card->ext_csd.generic_cmd6_time,
-  true, true, true);
+  true, send_status, true);
if (err) {
pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
mmc_hostname(host), err);
@@ -1087,6 +1091,13 @@ static int mmc_select_hs400(struct mmc_card *card)
/* Set host controller to HS timing */
mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
 
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   /* Switch card to DDR */
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 EXT_CSD_BUS_WIDTH,
 EXT_CSD_DDR_BUS_WIDTH_8,
@@ -1097,22 +1108,35 @@ static int mmc_select_hs400(struct mmc_card *card)
return err;
}
 
+   /* Switch card to HS400 */
val = EXT_CSD_TIMING_HS400 |
  card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
   EXT_CSD_HS_TIMING, val,
   card->ext_csd.generic_cmd6_time,
-  true, true, true);
+  true, send_status, true);
if (err) {
pr_err("%s: switch to hs400 failed, err:%d\n",
 mmc_hostname(host), err);
return err;
}
 
+   /* Set host controller to HS400 timing and frequency */
mmc_set_timing(host, MMC_TIMING_MMC_HS400);
mmc_set_bus_speed(card);
 
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
return 0;
+
+out_err:
+   pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host),
+  __func__, err);
+   return err;
 }
 
 int mmc_hs200_to_hs400(struct mmc_card *card)
-- 
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

[PATCH 1/2] mmc: sdhci-pci: Add more PCI IDs for Intel controllers

[Adrian Hunter]

Add PCI IDs for Intel host controllers

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 48 +++
 drivers/mmc/host/sdhci-pci.h  |  6 +
 2 files changed, 54 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-core.c 
b/drivers/mmc/host/sdhci-pci-core.c
index b6f6117cb233..cf7ad458b4f4 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1117,6 +1117,54 @@ static const struct pci_device_id pci_ids[] = {
},
 
{
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_BXT_EMMC,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_emmc,
+   },
+
+   {
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_BXT_SDIO,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_sdio,
+   },
+
+   {
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_BXT_SD,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_sd,
+   },
+
+   {
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_APL_EMMC,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_emmc,
+   },
+
+   {
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_APL_SDIO,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_sdio,
+   },
+
+   {
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_APL_SD,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_sd,
+   },
+
+   {
.vendor = PCI_VENDOR_ID_O2,
.device = PCI_DEVICE_ID_O2_8120,
.subvendor  = PCI_ANY_ID,
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index cd4f4d74377e..d1a0b4db60db 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -25,6 +25,12 @@
 #define PCI_DEVICE_ID_INTEL_SPT_SDIO   0x9d2c
 #define PCI_DEVICE_ID_INTEL_SPT_SD 0x9d2d
 #define PCI_DEVICE_ID_INTEL_DNV_EMMC   0x19db
+#define PCI_DEVICE_ID_INTEL_BXT_SD 0x0aca
+#define PCI_DEVICE_ID_INTEL_BXT_EMMC   0x0acc
+#define PCI_DEVICE_ID_INTEL_BXT_SDIO   0x0ad0
+#define PCI_DEVICE_ID_INTEL_APL_SD 0x5aca
+#define PCI_DEVICE_ID_INTEL_APL_EMMC   0x5acc
+#define PCI_DEVICE_ID_INTEL_APL_SDIO   0x5ad0
 
 /*
  * PCI registers
-- 
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

[PATCH 2/2] mmc: sdhci-acpi: Add more ACPI HIDs for Intel controllers

[Adrian Hunter]

Add ACPI HIDs for Intel host controllers including one
supporting HS400.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 78aa16aed07b..f6047fc94062 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -207,7 +207,9 @@ static const struct sdhci_acpi_slot 
sdhci_acpi_slot_int_emmc = {
.caps2   = MMC_CAP2_HC_ERASE_SZ,
.flags   = SDHCI_ACPI_RUNTIME_PM,
.quirks  = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
-   .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | SDHCI_QUIRK2_STOP_WITH_TC,
+   .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+  SDHCI_QUIRK2_STOP_WITH_TC |
+  SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
.probe_slot = sdhci_acpi_emmc_probe_slot,
 };
 
@@ -239,6 +241,9 @@ struct sdhci_acpi_uid_slot {
 };
 
 static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] = {
+   { "80865ACA", NULL, _acpi_slot_int_sd },
+   { "80865ACC", NULL, _acpi_slot_int_emmc },
+   { "80865AD0", NULL, _acpi_slot_int_sdio },
{ "80860F14" , "1" , _acpi_slot_int_emmc },
{ "80860F14" , "3" , _acpi_slot_int_sd   },
{ "80860F16" , NULL, _acpi_slot_int_sd   },
@@ -253,6 +258,9 @@ static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] = 
{
 };
 
 static const struct acpi_device_id sdhci_acpi_ids[] = {
+   { "80865ACA" },
+   { "80865ACC" },
+   { "80865AD0" },
{ "80860F14" },
{ "80860F16" },
{ "INT33BB"  },
-- 
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 v2] mmc: core: Fix init_card in 52Mhz

[Adrian Hunter]

On 08/10/15 16:35, Ulf Hansson wrote:
> On 8 October 2015 at 15:03, Adrian Hunter <adrian.hun...@intel.com> wrote:
>> On 08/10/15 13:59, Ulf Hansson wrote:
>>> On 8 October 2015 at 09:09, Chaotian Jing <chaotian.j...@mediatek.com> 
>>> wrote:
>>>> Suppose that we got a data crc error, and it triggers the mmc_reset.
>>>> mmc_reset will call mmc_send_status to see if HW reset was supported.
>>>> before issue CMD13, it will do retune, and if EMMC was in HS400 mode,
>>>> it will reduce frequency to 52Mhz firstly, then results in card init
>>>> was doing at 52Mhz.
>>>> The mmc_send_status was originally only done for mmc_test, so if retune
>>>> needed, do not call mmc_send_status.
>>>>
>>>> Signed-off-by: Chaotian Jing <chaotian.j...@mediatek.com>
>>>> Suggested-by: Adrian Hunter <adrian.hun...@intel.com>
>>>> ---
>>>>  drivers/mmc/card/mmc_test.c | 6 ++
>>>>  drivers/mmc/core/mmc.c  | 2 +-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
>>>> index b78cf5d..2f78bfb 100644
>>>> --- a/drivers/mmc/card/mmc_test.c
>>>> +++ b/drivers/mmc/card/mmc_test.c
>>>> @@ -2272,6 +2272,12 @@ static int mmc_test_hw_reset(struct mmc_test_card 
>>>> *test)
>>>> if (!mmc_card_mmc(card) || !mmc_can_reset(card))
>>>> return RESULT_UNSUP_CARD;
>>>>
>>>> +   if (host->need_retune) {
>>>> +   pr_info("%s: cannot test hw reset because retune needed\n",
>>>> +   mmc_hostname(test->card->host));
>>>> +   return RESULT_FAIL;
>>>> +   }
>>>> +
>>>> err = mmc_hw_reset(host);
>>>> if (!err)
>>>> return RESULT_OK;
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index e726903..647c96d 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
>>>> host->ops->hw_reset(host);
>>>>
>>>> /* If the reset has happened, then a status command will fail */
>>>> -   if (!mmc_send_status(card, )) {
>>>> +   if (!host->need_retune && !mmc_send_status(card, )) {
>>>
>>> No, this seem like the wrong solution! The main purpose of mmc_reset()
>>> is to try to reset and re-initiate the card, to make it operational
>>> again.
>>>
>>> I can't find a good reason to why we want to do a mmc_send_status() at
>>> this point, as even if it succeeds it will only tell us that the card
>>> has not been "hw-reset". No matter what, we should still try to make
>>> it fully operational again and thus give mmc_init_card() a try.
>>>
>>> >From this reasoning, I suggest we remove the call to mmc_send_status()
>>> from this path, as that will also address your issue with CRC errors
>>> in combination with re-tune.
>>
>> Then you need to remove the hw_reset test from mmc_test.  Refer:
>>
>> http://marc.info/?l=linux-mmc=144360165906544=2
>>
> 
> I realize that the test becomes a bit different, but I don't think it's 
> useless.
> 
> If we add a check for MMC_CAP_HW_RESET and verify that the
> host->ops->hw_reset exists, then we can assume that the "hw_reset"
> sequence has executed. And if mmc_init_card() fails, that would
> probably mean that the reset also failed, right?

In the test case, the card is in a working state.  Generally I would then
expect reinitialization to work irrespective of whether or not the hardware
is actually reset.

Here are some other options:
1. have mmc_test hook the host->ops->hw_reset() fn and do the 
send_status
itself.
2. have mmc_test set a flag on the card that it is being tested
and only do the send_status if the flag is set
3. remove the send_status call and rename the mmc_test from "eMMC 
hardware
reset" to just "Reset test (doesn't check hw reset did reset)"

--
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 v2] mmc: core: Fix init_card in 52Mhz

[Adrian Hunter]

On 08/10/15 13:59, Ulf Hansson wrote:
> On 8 October 2015 at 09:09, Chaotian Jing <chaotian.j...@mediatek.com> wrote:
>> Suppose that we got a data crc error, and it triggers the mmc_reset.
>> mmc_reset will call mmc_send_status to see if HW reset was supported.
>> before issue CMD13, it will do retune, and if EMMC was in HS400 mode,
>> it will reduce frequency to 52Mhz firstly, then results in card init
>> was doing at 52Mhz.
>> The mmc_send_status was originally only done for mmc_test, so if retune
>> needed, do not call mmc_send_status.
>>
>> Signed-off-by: Chaotian Jing <chaotian.j...@mediatek.com>
>> Suggested-by: Adrian Hunter <adrian.hun...@intel.com>
>> ---
>>  drivers/mmc/card/mmc_test.c | 6 ++
>>  drivers/mmc/core/mmc.c  | 2 +-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
>> index b78cf5d..2f78bfb 100644
>> --- a/drivers/mmc/card/mmc_test.c
>> +++ b/drivers/mmc/card/mmc_test.c
>> @@ -2272,6 +2272,12 @@ static int mmc_test_hw_reset(struct mmc_test_card 
>> *test)
>> if (!mmc_card_mmc(card) || !mmc_can_reset(card))
>> return RESULT_UNSUP_CARD;
>>
>> +   if (host->need_retune) {
>> +   pr_info("%s: cannot test hw reset because retune needed\n",
>> +   mmc_hostname(test->card->host));
>> +   return RESULT_FAIL;
>> +   }
>> +
>> err = mmc_hw_reset(host);
>> if (!err)
>> return RESULT_OK;
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index e726903..647c96d 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
>> host->ops->hw_reset(host);
>>
>> /* If the reset has happened, then a status command will fail */
>> -   if (!mmc_send_status(card, )) {
>> +   if (!host->need_retune && !mmc_send_status(card, )) {
> 
> No, this seem like the wrong solution! The main purpose of mmc_reset()
> is to try to reset and re-initiate the card, to make it operational
> again.
> 
> I can't find a good reason to why we want to do a mmc_send_status() at
> this point, as even if it succeeds it will only tell us that the card
> has not been "hw-reset". No matter what, we should still try to make
> it fully operational again and thus give mmc_init_card() a try.
> 
>>From this reasoning, I suggest we remove the call to mmc_send_status()
> from this path, as that will also address your issue with CRC errors
> in combination with re-tune.

Then you need to remove the hw_reset test from mmc_test.  Refer:

http://marc.info/?l=linux-mmc=144360165906544=2

--
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] mmc: sdhci-pci: Add another PCI ID for an Intel eMMC host controller

[Adrian Hunter]

Add another PCI ID for an Intel eMMC host controller.

Signed-off-by: Adrian Hunter <adrian.hun...@intel.com>
---
 drivers/mmc/host/sdhci-pci.c | 8 
 drivers/mmc/host/sdhci-pci.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
index b3b0a3e4fca1..7ccd4220076c 100644
--- a/drivers/mmc/host/sdhci-pci.c
+++ b/drivers/mmc/host/sdhci-pci.c
@@ -1113,6 +1113,14 @@ static const struct pci_device_id pci_ids[] = {
},
 
{
+   .vendor = PCI_VENDOR_ID_INTEL,
+   .device = PCI_DEVICE_ID_INTEL_DNV_EMMC,
+   .subvendor  = PCI_ANY_ID,
+   .subdevice  = PCI_ANY_ID,
+   .driver_data= (kernel_ulong_t)_intel_byt_emmc,
+   },
+
+   {
.vendor = PCI_VENDOR_ID_O2,
.device = PCI_DEVICE_ID_O2_8120,
.subvendor  = PCI_ANY_ID,
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 541f1cad5247..cd4f4d74377e 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -24,6 +24,7 @@
 #define PCI_DEVICE_ID_INTEL_SPT_EMMC   0x9d2b
 #define PCI_DEVICE_ID_INTEL_SPT_SDIO   0x9d2c
 #define PCI_DEVICE_ID_INTEL_SPT_SD 0x9d2d
+#define PCI_DEVICE_ID_INTEL_DNV_EMMC   0x19db
 
 /*
  * PCI registers
-- 
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] mmc: core: Fix init_card in 52Mhz

[Adrian Hunter]

On 30/09/15 06:28, Chaotian Jing wrote:
> Suppose that we got a data crc error, and it triggers the mmc_reset.
> mmc_reset will call mmc_send_status to see if HW reset was supported.
> before issue CMD13, it will do retune, and if EMMC was in HS400 mode,
> it will reduce frequency to 52Mhz firstly, that results in card init
> was doing at 52Mhz, So need ensure frequency is lower than 400Khz when
> re-init card.

The call to mmc_send_status() was originally only done for mmc_test and
doesn't belong in a real reset.  However the test parameter was removed
to simplify the code by:

commit 83533ab28380f6957af39a7b322e639e42dbdaf1
Author: Johan Rudholm 
Date:   Mon Jan 12 15:38:04 2015 +0100

mmc: core: always check status after reset

Always check if the card is alive after a successful reset. This allows
us to remove mmc_hw_reset_check(), leaving mmc_hw_reset() as the only
card reset interface.

Signed-off-by: Johan Rudholm 
Signed-off-by: Ulf Hansson 

IMHO having a test to check whether hw reset actually did reset the card
is useful, so I would suggest putting back mmc_hw_reset_check() and
removing the check from the non-test path.

An alternative is to recognize that we don't want retuning here, and it is
unlikely to be needed in the mmc_test case.  So if retuning is needed, just
skip the call to mmc_send_status() i.e.

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d0c2ae85b1..c3749ba42739 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
host->ops->hw_reset(host);
 
/* If the reset has happened, then a status command will fail */
-   if (!mmc_send_status(card, )) {
+   if (!host->need_retune && !mmc_send_status(card, )) {
mmc_host_clk_release(host);
return -ENOSYS;
}

Could then make mmc_test aware of that:

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index b78cf5d403a3..7b105b97be78 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -2272,6 +2272,16 @@ static int mmc_test_hw_reset(struct mmc_test_card *test)
if (!mmc_card_mmc(card) || !mmc_can_reset(card))
return RESULT_UNSUP_CARD;
 
+   err = mmc_send_status(card, NULL);
+   if (err)
+   return err;
+
+   if (host->need_retune) {
+   pr_info("%s: cannot test hw reset because retune needed\n",
+   mmc_hostname(test->card->host));
+   return RESULT_FAIL;
+   }
+
err = mmc_hw_reset(host);
if (!err)
return RESULT_OK;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d0c2ae85b1..c3749ba42739 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
host->ops->hw_reset(host);
 
/* If the reset has happened, then a status command will fail */
-   if (!mmc_send_status(card, )) {
+   if (!host->need_retune && !mmc_send_status(card, )) {
mmc_host_clk_release(host);
return -ENOSYS;
}




> 
> Signed-off-by: Chaotian Jing 
> ---
>  drivers/mmc/core/mmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index e726903..f2d0c2a 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1945,6 +1945,7 @@ static int mmc_reset(struct mmc_host *host)
>  
>   /* Set initial state and call mmc_set_ios */
>   mmc_set_initial_state(host);
> + mmc_set_clock(host, host->f_init);
>   mmc_host_clk_release(host);
>  
>   return mmc_init_card(host, card->ocr, card);
> 

--
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: core: fix dead loop of mmc_retune

[Adrian Hunter]

On 30/09/15 12:37, Chaotian Jing wrote:
> When get a CRC error, start the mmc_retune, it will issue CMD19/CMD21
> to do tune, assume there were 10 clock phase need to try, phase 0 to
> phase 6 is ok, phase 7 to phase 9 is NG, we try it from 0 to 9, so
> the last CMD19/CMD21 will get CRC error, host->need_retune was set and
> cause mmc_retune was called, then dead loop of mmc_retune
> 
> Signed-off-by: Chaotian Jing <chaotian.j...@mediatek.com>

Wasn't my idea to have this CRC checking in the core ;-) and sdhci
doesn't put it's tuning commands through a mmc request so it doesn't
see this issue.

But definitely the CRC logic is not meant for the tuning commands, so

Acked-by: Adrian Hunter <adrian.hun...@intel.com>

> ---
>  drivers/mmc/core/core.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 0520064..a3eb20b 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -134,9 +134,11 @@ void mmc_request_done(struct mmc_host *host, struct 
> mmc_request *mrq)
>   int err = cmd->error;
>  
>   /* Flag re-tuning needed on CRC errors */
> - if (err == -EILSEQ || (mrq->sbc && mrq->sbc->error == -EILSEQ) ||
> + if ((cmd->opcode != MMC_SEND_TUNING_BLOCK &&
> + cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200) &&
> + (err == -EILSEQ || (mrq->sbc && mrq->sbc->error == -EILSEQ) ||
>   (mrq->data && mrq->data->error == -EILSEQ) ||
> - (mrq->stop && mrq->stop->error == -EILSEQ))
> + (mrq->stop && mrq->stop->error == -EILSEQ)))
>   mmc_retune_needed(host);
>  
>   if (err && cmd->retries && mmc_host_is_spi(host)) {
> 

--
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: core: Fix init_card in 52Mhz

[Adrian Hunter]

On 30/09/15 12:03, Chaotian Jing wrote:
> On Wed, 2015-09-30 at 11:24 +0300, Adrian Hunter wrote:
>> On 30/09/15 06:28, Chaotian Jing wrote:
>>> Suppose that we got a data crc error, and it triggers the mmc_reset.
>>> mmc_reset will call mmc_send_status to see if HW reset was supported.
>>> before issue CMD13, it will do retune, and if EMMC was in HS400 mode,
>>> it will reduce frequency to 52Mhz firstly, that results in card init
>>> was doing at 52Mhz, So need ensure frequency is lower than 400Khz when
>>> re-init card.
>>
>> The call to mmc_send_status() was originally only done for mmc_test and
>> doesn't belong in a real reset.  However the test parameter was removed
>> to simplify the code by:
>>
>> commit 83533ab28380f6957af39a7b322e639e42dbdaf1
>> Author: Johan Rudholm <johan.rudh...@axis.com>
>> Date:   Mon Jan 12 15:38:04 2015 +0100
>>
>> mmc: core: always check status after reset
>> 
>> Always check if the card is alive after a successful reset. This allows
>> us to remove mmc_hw_reset_check(), leaving mmc_hw_reset() as the only
>> card reset interface.
>> 
>> Signed-off-by: Johan Rudholm <joha...@axis.com>
>> Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org>
>>
>> IMHO having a test to check whether hw reset actually did reset the card
>> is useful, so I would suggest putting back mmc_hw_reset_check() and
>> removing the check from the non-test path.
>>
>> An alternative is to recognize that we don't want retuning here, and it is
>> unlikely to be needed in the mmc_test case.  So if retuning is needed, just
>> skip the call to mmc_send_status() i.e.
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f2d0c2ae85b1..c3749ba42739 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
>>  host->ops->hw_reset(host);
>>  
>>  /* If the reset has happened, then a status command will fail */
>> -if (!mmc_send_status(card, )) {
>> +if (!host->need_retune && !mmc_send_status(card, )) {
>>  mmc_host_clk_release(host);
>>  return -ENOSYS;
>>  }
>>
>> Could then make mmc_test aware of that:
>>  
> 
>> This solution is better, please help to upstream it. Thx!

Then please, make it a patch, and test it and submit it.  You can add:

Suggested-by: Adrian Hunter <adrian.hun...@intel.com>


>> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
>> index b78cf5d403a3..7b105b97be78 100644
>> --- a/drivers/mmc/card/mmc_test.c
>> +++ b/drivers/mmc/card/mmc_test.c
>> @@ -2272,6 +2272,16 @@ static int mmc_test_hw_reset(struct mmc_test_card 
>> *test)
>>  if (!mmc_card_mmc(card) || !mmc_can_reset(card))
>>  return RESULT_UNSUP_CARD;
>>  
>> +err = mmc_send_status(card, NULL);
>> +if (err)
>> +return err;
>> +
>> +if (host->need_retune) {
>> +pr_info("%s: cannot test hw reset because retune needed\n",
>> +mmc_hostname(test->card->host));
>> +return RESULT_FAIL;
>> +}
>> +
>>  err = mmc_hw_reset(host);
>>  if (!err)
>>  return RESULT_OK;
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f2d0c2ae85b1..c3749ba42739 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1938,7 +1938,7 @@ static int mmc_reset(struct mmc_host *host)
>>  host->ops->hw_reset(host);
>>  
>>  /* If the reset has happened, then a status command will fail */
>> -if (!mmc_send_status(card, )) {
>> +if (!host->need_retune && !mmc_send_status(card, )) {
>>  mmc_host_clk_release(host);
>>  return -ENOSYS;
>>  }
>>
>>
>>
>>
>>>
>>> Signed-off-by: Chaotian Jing <chaotian.j...@mediatek.com>
>>> ---
>>>  drivers/mmc/core/mmc.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index e726903..f2d0c2a 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -1945,6 +1945,7 @@ static int mmc_reset(struct mmc_host *host)
>>>  
>>> /* Set initial state and call mmc_set_ios */
>>> mmc_set_initial_state(host);
>>> +   mmc_set_clock(host, host->f_init);
>>> mmc_host_clk_release(host);
>>>  
>>> return mmc_init_card(host, card->ocr, card);
>>>
>>
> 
> 
> 

--
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/SDIO: enable SDIO device to suspend/resume asynchronously

[Adrian Hunter]

On 21/09/15 08:29, Fu, Zhonghui wrote:
> 
> 
> On 2015/8/24 15:07, Fu, Zhonghui wrote:
>>
>> On 2015/8/17 14:48, Adrian Hunter wrote:
>>> On 17/08/15 06:26, Fu, Zhonghui wrote:
>>>> Hi,
>>>>
>>>> Any comments are welcome.
>>>>
>>>>
>>>> Thanks,
>>>> Zhonghui
>>>>
>>>> On 2015/7/30 15:40, Fu, Zhonghui wrote:
>>>>> Enable SDIO card and function device to suspend/resume asynchronously.
>>>>> This can improve system suspend/resume speed.
>>> For me, it needs more explanation.
>>>
>>> For example, why is this worth doing?  Can you give an example where it does
>>> significantly improve suspend/resume speed?  Are there any cases where it
>>> could be worse?
>>>
>>> Why is it safe?  Presumably it is safe if there are no dependencies on the
>>> device outside the device hierarchy. Is that so?  Are there any other
>>> potential pitfalls to enabling async_suspend?
>> Now, PM core support asynchronous device suspend/resume mode. If one device 
>> has been set to support asynchronous PM mode, it's suspend/resume operation 
>> can be performed in a separate kernel thread and take advantage of multicore 
>> feature to improve overall system suspend/resume speed. The worst case is 
>> that all device suspend/resume threads will be scheduled to the same CPU, it 
>> hardly occur.
>>
>> PM core ensure all the suspend/resume dependency related to one device. 
>> Actually, async suspend/resume mode is one feature of PM core, every device 
>> subsystem may use it or not use it. Once one device subsystem choose to use 
>> this feature, its safety is up to PM core as long as device subsystem has 
>> initialized fully this device.
> 
> The original suspend time is 1645ms and resume time is 940ms on ASUS T100TA
> machine. After enabling "wiphy device", "SDIO device", "mmc host" and
> "sdhci-acpi device" to suspend/resume asynchronously, the suspend time is
> 1096ms and resume time is 908ms. The test environment is listed as follows:
> 
> OS: Ubuntu 14.04
> Kernel: mainline v4.1
> Machine: ASUS T100TA(Baytrail-T platform)
> Tool: analyze_suspend
> “analyze_suspend.py –f –m freeze” to suspend system
> Press power button to resume system
> 
> I have resent this patch with updated commit message - "[PATCH v2] MMC/SDIO:
> enable SDIO device to suspend/resume asynchronously".

It is really cool that you tested this.  Thank you!

I am a bit baffled and bemused that you didn't put a summary of the testing
and results in the commit message.  Can you do that.

As I wrote, we are assuming that there are no dependencies on the device
outside the device hierarchy. That is a reasonable assumption for an SDIO
controller because it doesn't provide resources for other devices to use,
except for the card itself which is a child device, and therefore a
dependency that PM core knows about.

Does that make sense?  If it does then shouldn't that explanation be added
to the commit message too.  i.e. this is why we think it is always going to
work?

> 
> 
> Thanks,
> Zhonghui
>>
>>
>> Thanks,
>> Zhonghui  
>>>>> Signed-off-by: Zhonghui Fu <zhonghui...@linux.intel.com>
>>>>> ---
>>>>>  drivers/mmc/core/sdio.c |4 
>>>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>> index b91abed..6719b77 100644
>>>>> --- a/drivers/mmc/core/sdio.c
>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>> @@ -1106,6 +1106,8 @@ int mmc_attach_sdio(struct mmc_host *host)
>>>>>   pm_runtime_enable(>dev);
>>>>>   }
>>>>>  
>>>>> + device_enable_async_suspend(>dev);
>>>>> +
>>>>>   /*
>>>>>* The number of functions on the card is encoded inside
>>>>>* the ocr.
>>>>> @@ -1126,6 +1128,8 @@ int mmc_attach_sdio(struct mmc_host *host)
>>>>>*/
>>>>>   if (host->caps & MMC_CAP_POWER_OFF_CARD)
>>>>>   pm_runtime_enable(>sdio_func[i]->dev);
>>>>> +
>>>>> + device_enable_async_suspend(>sdio_func[i]->dev);
>>>>>   }
>>>>>  
>>>>>   /*
>>>>> -- 1.7.1
>>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
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/SDIO: enable SDIO device to suspend/resume asynchronously

[Adrian Hunter]

On 21/09/15 11:15, Adrian Hunter wrote:
> On 21/09/15 08:29, Fu, Zhonghui wrote:
>>
>>
>> On 2015/8/24 15:07, Fu, Zhonghui wrote:
>>>
>>> On 2015/8/17 14:48, Adrian Hunter wrote:
>>>> On 17/08/15 06:26, Fu, Zhonghui wrote:
>>>>> Hi,
>>>>>
>>>>> Any comments are welcome.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Zhonghui
>>>>>
>>>>> On 2015/7/30 15:40, Fu, Zhonghui wrote:
>>>>>> Enable SDIO card and function device to suspend/resume asynchronously.
>>>>>> This can improve system suspend/resume speed.
>>>> For me, it needs more explanation.
>>>>
>>>> For example, why is this worth doing?  Can you give an example where it 
>>>> does
>>>> significantly improve suspend/resume speed?  Are there any cases where it
>>>> could be worse?
>>>>
>>>> Why is it safe?  Presumably it is safe if there are no dependencies on the
>>>> device outside the device hierarchy. Is that so?  Are there any other
>>>> potential pitfalls to enabling async_suspend?
>>> Now, PM core support asynchronous device suspend/resume mode. If one device 
>>> has been set to support asynchronous PM mode, it's suspend/resume operation 
>>> can be performed in a separate kernel thread and take advantage of 
>>> multicore feature to improve overall system suspend/resume speed. The worst 
>>> case is that all device suspend/resume threads will be scheduled to the 
>>> same CPU, it hardly occur.
>>>
>>> PM core ensure all the suspend/resume dependency related to one device. 
>>> Actually, async suspend/resume mode is one feature of PM core, every device 
>>> subsystem may use it or not use it. Once one device subsystem choose to use 
>>> this feature, its safety is up to PM core as long as device subsystem has 
>>> initialized fully this device.
>>
>> The original suspend time is 1645ms and resume time is 940ms on ASUS T100TA
>> machine. After enabling "wiphy device", "SDIO device", "mmc host" and
>> "sdhci-acpi device" to suspend/resume asynchronously, the suspend time is
>> 1096ms and resume time is 908ms. The test environment is listed as follows:
>>
>> OS: Ubuntu 14.04
>> Kernel: mainline v4.1
>> Machine: ASUS T100TA(Baytrail-T platform)
>> Tool: analyze_suspend
>> “analyze_suspend.py –f –m freeze” to suspend system
>> Press power button to resume system
>>
>> I have resent this patch with updated commit message - "[PATCH v2] MMC/SDIO:
>> enable SDIO device to suspend/resume asynchronously".
> 
> It is really cool that you tested this.  Thank you!
> 
> I am a bit baffled and bemused that you didn't put a summary of the testing
> and results in the commit message.  Can you do that.
> 
> As I wrote, we are assuming that there are no dependencies on the device
> outside the device hierarchy. That is a reasonable assumption for an SDIO
> controller because it doesn't provide resources for other devices to use,
> except for the card itself which is a child device, and therefore a
> dependency that PM core knows about.
> 
> Does that make sense?  If it does then shouldn't that explanation be added
> to the commit message too.  i.e. this is why we think it is always going to
> work?

Just realized this patch is for the card, not the host controller, so those
last 2 paragraphs don't apply.

> 
>>
>>
>> Thanks,
>> Zhonghui
>>>
>>>
>>> Thanks,
>>> Zhonghui  
>>>>>> Signed-off-by: Zhonghui Fu <zhonghui...@linux.intel.com>
>>>>>> ---
>>>>>>  drivers/mmc/core/sdio.c |4 
>>>>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>> index b91abed..6719b77 100644
>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>> @@ -1106,6 +1106,8 @@ int mmc_attach_sdio(struct mmc_host *host)
>>>>>>  pm_runtime_enable(>dev);
>>>>>>  }
>>>>>>  
>>>>>> +device_enable_async_suspend(>dev);
>>>>>> +
>>>>>>  /*
>>>>>>   * The number of functions on the card is encoded inside
>>>>>>   * the ocr.
>>>>>> @@ -1126,6 +1128,8 @@ int mmc_attach_sdio(struct mmc_host *host)
>>>>>>   */
>>>>>>  if (host->caps & MMC_CAP_POWER_OFF_CARD)
>>>>>>  pm_runtime_enable(>sdio_func[i]->dev);
>>>>>> +
>>>>>> +device_enable_async_suspend(>sdio_func[i]->dev);
>>>>>>  }
>>>>>>  
>>>>>>  /*
>>>>>> -- 1.7.1
>>>>>>
>>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>> the body of a message to majord...@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> 

--
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-acpi: detect sd card reader on asus x205ta

[Adrian Hunter]

On 05/09/15 09:49, Michele Curti wrote:
> Add an entry to the sdhci_acpi_uids list to detect the SD card 
> reader on the Asus X205Ta laptop.
> 
> dstd table:
> 
> Device (SDHC)
> {
> Name (_ADR, Zero)  // _ADR: Address
> Name (_HID, "PNP0FFF")  // _HID: Hardware ID
> Name (_CID, "PNP0D40" /* SDA Standard Compliant SD Host Controller */)
> Name (_DDN, "Intel(R) SD Card Controller - 80860F16")  // _DDN: DOS Dev
> Name (_UID, 0x03)  // _UID: Unique ID
> Name (RDEP, Package (0x02)
> 
> Signed-off-by: Michele Curti <michele.cu...@gmail.com>

Acked-by: Adrian Hunter <adrian.hun...@intel.com>

> ---
>  drivers/mmc/host/sdhci-acpi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 22d929f..78aa16a 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -247,6 +247,7 @@ static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] 
> = {
>   { "INT33C6"  , NULL, _acpi_slot_int_sdio },
>   { "INT3436"  , NULL, _acpi_slot_int_sdio },
>   { "INT344D"  , NULL, _acpi_slot_int_sdio },
> + { "PNP0FFF"  , "3" , _acpi_slot_int_sd   },
>   { "PNP0D40"  },
>   { },
>  };
> 

--
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/SDIO: enable SDIO device to suspend/resume asynchronously

[Adrian Hunter]

On 17/08/15 06:26, Fu, Zhonghui wrote:
 
 Hi,
 
 Any comments are welcome.
 
 
 Thanks,
 Zhonghui
 
 On 2015/7/30 15:40, Fu, Zhonghui wrote:
 Enable SDIO card and function device to suspend/resume asynchronously.
 This can improve system suspend/resume speed.

For me, it needs more explanation.

For example, why is this worth doing?  Can you give an example where it does
significantly improve suspend/resume speed?  Are there any cases where it
could be worse?

Why is it safe?  Presumably it is safe if there are no dependencies on the
device outside the device hierarchy. Is that so?  Are there any other
potential pitfalls to enabling async_suspend?


 Signed-off-by: Zhonghui Fu zhonghui...@linux.intel.com
 ---
  drivers/mmc/core/sdio.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

 diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
 index b91abed..6719b77 100644
 --- a/drivers/mmc/core/sdio.c
 +++ b/drivers/mmc/core/sdio.c
 @@ -1106,6 +1106,8 @@ int mmc_attach_sdio(struct mmc_host *host)
  pm_runtime_enable(card-dev);
  }
  
 +device_enable_async_suspend(card-dev);
 +
  /*
   * The number of functions on the card is encoded inside
   * the ocr.
 @@ -1126,6 +1128,8 @@ int mmc_attach_sdio(struct mmc_host *host)
   */
  if (host-caps  MMC_CAP_POWER_OFF_CARD)
  pm_runtime_enable(card-sdio_func[i]-dev);
 +
 +device_enable_async_suspend(card-sdio_func[i]-dev);
  }
  
  /*
 -- 1.7.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] mmc: enable mmc host device to suspend/resume asynchronously

[Adrian Hunter]

On 17/08/15 06:36, Fu, Zhonghui wrote:
 
 Hi,
 
 Any comments are welcome.

Same comments as here:

http://marc.info/?l=linux-kernelm=143979428424353w=2

 
 
 Thanks,
 Zhonghui
 
 On 2015/8/3 20:39, Fu, Zhonghui wrote:
 Enable mmc host device to suspend/resume asynchronously.
 This can improve system suspend/resume speed.

 Signed-off-by: Zhonghui Fu zhonghui...@linux.intel.com
 ---
  drivers/mmc/core/host.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
 index 99a9c90..85f2bbb 100644
 --- a/drivers/mmc/core/host.c
 +++ b/drivers/mmc/core/host.c
 @@ -577,6 +577,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
 *dev)
  host-class_dev.parent = dev;
  host-class_dev.class = mmc_host_class;
  device_initialize(host-class_dev);
 +device_enable_async_suspend(host-class_dev);
  
  if (mmc_gpio_alloc(host)) {
  put_device(host-class_dev);
 -- 1.7.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] mmc/sdhci-acpi: enable sdhci-acpi device to suspend/resume asynchronously

[Adrian Hunter]

On 17/08/15 06:38, Fu, Zhonghui wrote:
 
 Hi,
 
 Any comments are welcome.

Same comments as here:

http://marc.info/?l=linux-kernelm=143979428424353w=2

 
 
 Thanks,
 Zhonghui
 
 On 2015/8/3 21:10, Fu, Zhonghui wrote:
 Enable sdhci-acpi device to suspend/resume asynchronously.
 This can improve system suspend/resume speed.

 Signed-off-by: Zhonghui Fu zhonghui...@linux.intel.com
 ---
  drivers/mmc/host/sdhci-acpi.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

 diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
 index 22d929f..67e6263 100644
 --- a/drivers/mmc/host/sdhci-acpi.c
 +++ b/drivers/mmc/host/sdhci-acpi.c
 @@ -379,6 +379,8 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
  pm_runtime_enable(dev);
  }
  
 +device_enable_async_suspend(dev);
 +
  return 0;
  
  err_free:
 -- 1.7.1

 
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] mmc: sdhci: let GPIO based card detection have higher precedence

[Adrian Hunter]

On 26/06/15 13:00, Ivan T. Ivanov wrote:
 Controller could have BROKEN_CARD_DETECTION quirk set, but drivers
 could use GPIO to detect card present state. Let, when defined, GPIO
 take precedence, so drivers could properly detect card state and not
 use polling.
 
 Signed-off-by: Ivan T. Ivanov ivan.iva...@linaro.org
 ---
  drivers/mmc/host/sdhci.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index bc14452..8bafb9f 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -1601,15 +1601,18 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
   if (host-flags  SDHCI_DEVICE_DEAD)
   return 0;
 
 + /*
 +  * Try slot gpio detect, if defined it take precedence
 +  * over build in controller functionality
 +  */
 + if (!IS_ERR_VALUE(gpio_cd))
 + return !!gpio_cd;
 +

You've also put it above the MMC_CAP_NONREMOVABLE check which doesn't seem
right.

   /* If polling/nonremovable, assume that the card is always present. */
   if ((host-quirks  SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
   (host-mmc-caps  MMC_CAP_NONREMOVABLE))
   return 1;
 
 - /* Try slot gpio detect */
 - if (!IS_ERR_VALUE(gpio_cd))
 - return !!gpio_cd;
 -
   /* Host native card detect */
   return !!(sdhci_readl(host, SDHCI_PRESENT_STATE)  SDHCI_CARD_PRESENT);
  }
 --
 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 1/3] mmc: sdhci: let GPIO based card detection have higher precedence

[Adrian Hunter]

On 26/06/15 14:00, Ivan T. Ivanov wrote:
 
 On Fri, 2015-06-26 at 13:19 +0300, Adrian Hunter wrote:
 On 26/06/15 13:00, Ivan T. Ivanov wrote:
 Controller could have BROKEN_CARD_DETECTION quirk set, but drivers
 could use GPIO to detect card present state. Let, when defined, GPIO
 take precedence, so drivers could properly detect card state and not
 use polling.

 Signed-off-by: Ivan T. Ivanov iva...@linaro.org
 ---
  drivers/mmc/host/sdhci.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index bc14452..8bafb9f 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -1601,15 +1601,18 @@ static int sdhci_do_get_cd(struct sdhci_host *host)
 if (host-flags  SDHCI_DEVICE_DEAD)
 return 0;

 +   /*
 +   * Try slot gpio detect, if defined it take precedence
 +   * over build in controller functionality
 +   */
 +   if (!IS_ERR_VALUE(gpio_cd))
 +   return !!gpio_cd;
 +

 You've also put it above the MMC_CAP_NONREMOVABLE check which doesn't seem
 right.

 
 Probably, but what are the chances that this is valid GIO for non-removable 
 cards.
 I could rework it if you insist.

It is nicer not to have to think what are the chances, and nicer that the
logic is strictly correct, so yes please.

 
 Thank you,
 Ivan
 
 /* If polling/nonremovable, assume that the card is always present. 
 */
 if ((host-quirks  SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
 (host-mmc-caps  
 MMC_CAP_NONREMOVABLE))
 return 1;

 -   /* Try slot gpio detect */
 -   if (!IS_ERR_VALUE(gpio_cd))
 -   return !!gpio_cd;
 -
 /* Host native card detect */
 return !!(sdhci_readl(host, SDHCI_PRESENT_STATE)  
 SDHCI_CARD_PRESENT);
  }
 --
 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] mmc: host: sdhci check parameters before call dma_free_coherent

[Adrian Hunter]

: SDHCI controller on 30b4.usdhc [30b4.usdhc] using DMA
 
 
 Signed-off-by: Peng Fan van.free...@gmail.com

Acked-by: Adrian Hunter adrian.hun...@intel.com

Looks like the problem started in v3.16 with:

commit d1e49f77d7c7b75fdc022e1d46c1549bbc91c5b7
Author: Russell King rmk+ker...@arm.linux.org.uk
Date:   Fri Apr 25 12:58:34 2014 +0100

mmc: sdhci: convert ADMA descriptors to a coherent allocation



 ---
  drivers/mmc/host/sdhci.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index 706bb60..52e2327 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -2978,8 +2978,11 @@ int sdhci_add_host(struct sdhci_host *host)
 GFP_KERNEL);
   host-align_buffer = kmalloc(host-align_buffer_sz, GFP_KERNEL);
   if (!host-adma_table || !host-align_buffer) {
 - dma_free_coherent(mmc_dev(mmc), host-adma_table_sz,
 -   host-adma_table, host-adma_addr);
 + if (host-adma_table)
 + dma_free_coherent(mmc_dev(mmc),
 +   host-adma_table_sz,
 +   host-adma_table,
 +   host-adma_addr);
   kfree(host-align_buffer);
   pr_warn(%s: Unable to allocate ADMA buffers - falling 
 back to standard DMA\n,
   mmc_hostname(mmc));
 

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in

Re: [PATCH] mmc: host: Fix mmc_alloc_host() error path

[Adrian Hunter]

On 12/06/15 07:39, Fabio Estevam wrote:
 From: Fabio Estevam fabio.este...@freescale.com
 
 If mmc_gpio_alloc() fails we miss to call 'kfree(host)', so rearrange
 the error path to fix it. 

Are you sure it doesn't get freed through put_device()?

 
 Signed-off-by: Fabio Estevam fabio.este...@freescale.com
 ---
  drivers/mmc/core/host.c | 18 ++
  1 file changed, 10 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
 index 99a9c90..01fa1ed 100644
 --- a/drivers/mmc/core/host.c
 +++ b/drivers/mmc/core/host.c
 @@ -566,10 +566,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
 *dev)
   host-index = err;
   spin_unlock(mmc_host_lock);
   idr_preload_end();
 - if (err  0) {
 - kfree(host);
 - return NULL;
 - }
 + if (err  0)
 + goto err_kfree;
  
   dev_set_name(host-class_dev, mmc%d, host-index);
  
 @@ -578,10 +576,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
 *dev)
   host-class_dev.class = mmc_host_class;
   device_initialize(host-class_dev);
  
 - if (mmc_gpio_alloc(host)) {
 - put_device(host-class_dev);
 - return NULL;
 - }
 + if (mmc_gpio_alloc(host))
 + goto err_put_device;
  
   mmc_host_clk_init(host);
  
 @@ -605,6 +601,12 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
 *dev)
   host-max_blk_count = PAGE_CACHE_SIZE / 512;
  
   return host;
 +
 +err_put_device:
 + put_device(host-class_dev);
 +err_kfree:
 + kfree(host);
 + return NULL;
  }
  
  EXPORT_SYMBOL(mmc_alloc_host);
 

--
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: [RFC PATCH 2/2] mmc: core.c: Add comment to clarify special cases of ERASE/TRIM

[Adrian Hunter]

On 04/06/15 13:20, David Jander wrote:
 Signed-off-by: David Jander da...@protonic.nl

Please never send delta patches.  Always send a new version of the whole patch.

 ---
  drivers/mmc/core/core.c | 28 +++-
  1 file changed, 27 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index 6c9611b..b6aa9ad 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -2109,11 +2109,20 @@ int mmc_erase(struct mmc_card *card, unsigned int 
 from, unsigned int nr,
   !(card-ext_csd.sec_feature_support  EXT_CSD_SEC_GB_CL_EN))
   return -EOPNOTSUPP;
  
 + /*
 +  * Sanity check: If we do not erase aligned, whole erase-groups, return
 +  * an error, since we intended a secure erase, silently not erasing
 +  * something would be unacceptable.
 +  */

I am not sure the value of a comment that can anyway be inferred from the code.

   if (arg == MMC_SECURE_ERASE_ARG) {
   if (from % card-erase_size || nr % card-erase_size)
   return -EINVAL;
   }
  
 + /*
 +  * Make sure only erase-groups that are fully contained in the erase
 +  * region are erased. Silently ignore the rest.
 +  */

Ditto

   if (arg == MMC_ERASE_ARG) {
   rem = from % card-erase_size;
   if (rem) {
 @@ -2140,6 +2149,14 @@ int mmc_erase(struct mmc_card *card, unsigned int 
 from, unsigned int nr,
   /* 'from' and 'to' are inclusive */
   to -= 1;
  
 + /*
 +  * Special case where only one erase-group fits in the timout budget:

timout - timeout

 +  * If the region crosses an erase-group boundary on this particular
 +  * case, we will be trimming more than one erase-group which, does not
 +  * fit in the timeout budget of the controller, so we need to split it
 +  * and call mmc_do_erase() twice if necessary. This special case is
 +  * identified by the card-eg_boundary flag.
 +  */
   if ((arg  MMC_TRIM_ARGS)  (card-eg_boundary) 
   (from % card-erase_size)) {
   rem = card-erase_size - (from % card-erase_size);
 @@ -2244,7 +2261,16 @@ static unsigned int mmc_do_calc_max_discard(struct 
 mmc_card *card,
   if (!qty)
   return 0;
  
 - /* We can only erase one erase group special case */
 + /*
 +  * When specifying a sector range to trim, chances are we might cross
 +  * an erase-group boundary even if the amount of sectors is less than
 +  * one erase-group.
 +  * If we can only fit one erase-group in the controller timeout budget,
 +  * we have to care that erase-group boundaries are not crossed by a
 +  * single trim operation. We flag that special case with eg_boundary.
 +  * In all other cases we can just decrement qty and pretend that we
 +  * always touch (qty + 1) erase-groups as a simple optimization.

The language seems a little odd here. We are setting the max_discard limit
which does not involve pretending or optimization, it is just a
calculation.  The important point is that the calculation has to count the
maximum number of erase blocks affected not the size in erase blocks.  You
could give an example e.g. if a 2 sector trim crosses an erase block
boundary then that counts as 2 erase blocks affected.

 +  */
   if (qty == 1)
   card-eg_boundary = 1;
   else
 

--
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: core: Fix off-by-one error in mmc_do_calc_max_discard()

[Adrian Hunter]

On 01/06/15 12:20, David Jander wrote:
 qty is the maximum number of discard that _do_ fit in the timeout, not
 the first amount that does _not_ fit anymore.
 This seemingly harmless error has a very severe performance impact when
 the timeout value is enough for only 1 erase group.
 
 Signed-off-by: David Jander da...@protonic.nl
 ---
  drivers/mmc/core/core.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index 92e7671..1f9573b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -2234,16 +2234,13 @@ static unsigned int mmc_do_calc_max_discard(struct 
 mmc_card *card,
   if (!qty)
   return 0;
  
 - if (qty == 1)
 - return 1;
 -
   /* Convert qty to sectors */
   if (card-erase_shift)
 - max_discard = --qty  card-erase_shift;
 + max_discard = qty  card-erase_shift;
   else if (mmc_card_sd(card))
   max_discard = qty;
   else
 - max_discard = --qty * card-erase_size;
 + max_discard = qty * card-erase_size;
  
   return max_discard;
  }
 

This keeps coming up but there is more to it than that.  See here:

http://marc.info/?l=linux-mmcm=142504164427546

--
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: core: Fix off-by-one error in mmc_do_calc_max_discard()

[Adrian Hunter]

On 01/06/15 14:32, David Jander wrote:
 On Mon, 01 Jun 2015 13:36:45 +0300
 Adrian Hunter adrian.hun...@intel.com wrote:
 
 On 01/06/15 12:20, David Jander wrote:
 qty is the maximum number of discard that _do_ fit in the timeout, not
 the first amount that does _not_ fit anymore.
 This seemingly harmless error has a very severe performance impact when
 the timeout value is enough for only 1 erase group.

 Signed-off-by: David Jander da...@protonic.nl
 ---
  drivers/mmc/core/core.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index 92e7671..1f9573b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -2234,16 +2234,13 @@ static unsigned int mmc_do_calc_max_discard(struct
 mmc_card *card, if (!qty)
 return 0;
  
 -   if (qty == 1)
 -   return 1;
 -
 /* Convert qty to sectors */
 if (card-erase_shift)
 -   max_discard = --qty  card-erase_shift;
 +   max_discard = qty  card-erase_shift;
 else if (mmc_card_sd(card))
 max_discard = qty;
 else
 -   max_discard = --qty * card-erase_size;
 +   max_discard = qty * card-erase_size;
  
 return max_discard;
  }


 This keeps coming up but there is more to it than that.  See here:

  http://marc.info/?l=linux-mmcm=142504164427546

 
 Thanks for the link. I think it is time to put a comment on that piece of code
 to clarify this.
 Also, this code badly needs optimizing. I happen to have one of those
 unfortunate cases, where the maximum timeout of the MMC controller (Freescale
 i.MX6 uSDHCI) is 5.4 seconds, and the eMMC device (Micron 16GB eMMC) TRIM_MULT
 is 15 (4.5 seconds). As a result mmc_do_calc_max_discard() returns 1 and
 mkfs.ext4 takes several hours!! I think it is pretty clear that this is
 unacceptable and needs to be fixed.
 AFAICS, the correct fix for this would implicate that discard knows about
 the erase-group boundaries... something that could reach into the block-layer
 even... right?

Not necessarily. You could regard the can only do 1 erase block at a time
case as special, flag it, and in that case have mmc_erase() split along
erase block boundaries and call mmc_do_erase() multiple times. Then you
could set max_discard to something arbitrarily bigger.

 Has anybody even started to look into this?

Ulf was looking at supporting R1 response instead of R1b response from the
erase command and using a software timeout instead of the host controller's
hardware timeout.

--
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 V2 00/11] mmc: Add support for drive strength for eMMCs

[Adrian Hunter]

On 11/05/15 13:23, Ulf Hansson wrote:
 On 11 May 2015 at 11:29, Adrian Hunter adrian.hun...@intel.com wrote:
 On 06/02/15 14:12, Adrian Hunter wrote:
 Hi

 Here is V2 of some patches to enable a host controller to select
 driver strength for eMMCs using HS200 or HS400. These are based
 on top of the re-tuning series.

 There can be some confusion over the term driver strength.
 SD calls it drive strength for the card but driver type
 for the host. Whereas JEDEC calls it both driver strength
 and driver type. The values are the same for both SD
 and eMMC:

   Value   Driver Type   Relative strength
 0   B x1  default and mandatory support
 1   A x1.5
 2   C x0.75
 3   D x0.5

 Except eMMC also defines value 4:

   Value   Driver Type   Relative strength
 4 x1.2


 Changes in V2:

 mmc: core: Add function to read driver-strength device property
 Dropped because there are still questions over how to use
 device properties.

 mmc: sdhci-pci: Add support for drive strength selection for SPT
 Amended to reflect the change above.


 Adrian Hunter (11):
   mmc: core: Reset driver type to default
   mmc: core: Allow card drive strength to be different to host
   mmc: core: Simplify card drive strength mask
   mmc: core: Add 'card' to drive strength selection callback
   mmc: core: Factor out common code in drive strength selection
   mmc: core: Record card drive strength
   mmc: mmc: Read card's valid driver strength mask
   mmc: mmc: Add driver strength selection
   mmc: sdhci: Add a callback to select drive strength
   mmc: sdhci-pci: Add support for drive strength selection for SPT
   mmc: sdhci-pci: Enable HS400 for some Intel host controllers

  drivers/mmc/core/core.c| 39 ++
  drivers/mmc/core/core.h|  2 +
  drivers/mmc/core/mmc.c | 46 +
  drivers/mmc/core/sd.c  | 69 ---
  drivers/mmc/core/sdio.c| 77 +++---
  drivers/mmc/host/sdhci-pci-data.c  |  3 ++
  drivers/mmc/host/sdhci-pci.c   | 84 
 ++
  drivers/mmc/host/sdhci-pci.h   |  4 ++
  drivers/mmc/host/sdhci.c   | 13 ++
  drivers/mmc/host/sdhci.h   |  4 ++
  include/linux/mmc/card.h   |  2 +
  include/linux/mmc/host.h   |  4 +-
  include/linux/mmc/mmc.h|  4 ++
  include/linux/mmc/sdhci-pci-data.h |  2 +
  14 files changed, 240 insertions(+), 113 deletions(-)

 Hi Ulf

 These patches still apply. Would have time to look at them?
 
 Thanks for the reminder!
 
 I am walking through my backlog, but I don't want to give you any
 promises about when, since I never seems to be able to keep them. :-)

Hi

Here's another reminder :-)

I would really like these patches and there is really not much to review.

Patch 1 is quite trivial - only 1 line changed.

Patch 2 is a change the select_drive_strength() host op to allow card driver
strength to be different from host driver strength. It has no effect on
current drivers.

Patch 3 is just tidying up.

Patch 4 is another change to the select_drive_strength() host op (to add
'card' as a parameter) and also has no effect on current drivers.

Patch 5 is another tidy up.

Patch 6 is a very small patch to add drive_strength to struct mmc_card.

Patch 7 is a very small change to read eMMC supported driver strengths from
EXT-CSD.

Patch 8 adds driver strength selection for eMMC

Patch 9, 10 implement the select_drive_strength() for Intel SPT with the
sdhci-pci driver.

Patch 11 finally just enables HS400 for Intel

This shouldn't really take that long to review ;-)

Regards
Adrian

--
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 V7 02/14] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

Enable re-tuning when tuning is executed and
disable re-tuning when card is no longer initialized.

In the case of SDIO suspend, the card can keep power.
In that case, re-tuning need not be disabled, but, if
a re-tuning timer is being used, ensure it is disabled
and assume that re-tuning will be needed upon resume
since it is not known how long the suspend will last.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 4 
 drivers/mmc/core/sdio.c | 6 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 92e7671..007c444 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
 
if (err)
pr_err(%s: tuning execution failed\n, mmc_hostname(host));
+   else
+   mmc_retune_enable(host);
 
return err;
 }
@@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
int width)
  */
 void mmc_set_initial_state(struct mmc_host *host)
 {
+   mmc_retune_disable(host);
+
if (mmc_host_is_spi(host))
host-ios.chip_select = MMC_CS_HIGH;
else
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index f6c28a7..5c1423a3 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -934,8 +934,12 @@ static int mmc_sdio_suspend(struct mmc_host *host)
mmc_release_host(host);
}
 
-   if (!mmc_card_keep_power(host))
+   if (!mmc_card_keep_power(host)) {
mmc_power_off(host);
+   } else if (host-retune_period) {
+   mmc_retune_timer_stop(host);
+   mmc_retune_needed(host);
+   }
 
return 0;
 }
-- 
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

[PATCH V7 12/14] mmc: block: Check re-tuning in the recovery path

[Adrian Hunter]

If re-tuning is needed, do it in the recovery path to
give recovery commands a better chance of success.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/card/block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 2c25271..325ae1e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -913,6 +913,9 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
if (!err)
break;
 
+   /* Re-tune if needed */
+   mmc_retune_recheck(card-host);
+
prev_cmd_status_valid = false;
pr_err(%s: error %d sending status command, %sing\n,
   req-rq_disk-disk_name, err, retry ? retry : abort);
-- 
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 V6 03/15] mmc: core: Add support for re-tuning before each request

[Adrian Hunter]

On 06/05/15 12:45, Ulf Hansson wrote:
 On 6 May 2015 at 10:02, Adrian Hunter adrian.hun...@intel.com wrote:
 On 04/05/15 16:28, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunter adrian.hun...@intel.com wrote:
 At the start of each request, re-tune if needed and
 then hold off re-tuning again until the request is done.

 Note that though there is one function that starts
 requests (mmc_start_request) there are two that wait for
 the request to be done (mmc_wait_for_req_done and
 mmc_wait_for_data_req_done).  Also note that
 mmc_wait_for_data_req_done can return even when the
 request is not done (which allows the block driver
 to prepare a newly arrived request while still
 waiting for the previous request).

 This patch ensures re-tuning is held for the duration
 of a request.  Subsequent patches will also hold
 re-tuning at other times when it might cause a
 conflict.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com

 Patch2 is calling mmc_retune_needed() and thus actually triggers a
 re-tune to potentially happen.

 That won't happen because host-retune_period is not set, so
 mmc_retune_needed() won't be called.
 
 mmc_retune_needed() is indeed called in patch2 (v7). From
 mmc_sdio_suspend() and when mmc_card_keep_power().

Yes, here is the chunk but it checks host-retune_period which  is zero
because it never gets set.

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 5bc6c7d..16e1f39 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -934,8 +934,12 @@ static int mmc_sdio_suspend(struct mmc_host *host)
mmc_release_host(host);
}

-   if (!mmc_card_keep_power(host))
+   if (!mmc_card_keep_power(host)) {
mmc_power_off(host);
+   } else if (host-retune_period) {
+   mmc_retune_timer_stop(host);
+   mmc_retune_needed(host);
+   }

return 0;
 }

 
 I guess that wont be an issue!?

So I don't see an issue.

 
 But I just felt that it seemed more appropriate to manage hold/release
 of re-tune before actually enabling the feature.

Yes the feature is gated until setting host-retune_period or calling
mmc_retune_needed().

--
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 V6 03/15] mmc: core: Add support for re-tuning before each request

[Adrian Hunter]

On 04/05/15 16:28, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunter adrian.hun...@intel.com wrote:
 At the start of each request, re-tune if needed and
 then hold off re-tuning again until the request is done.

 Note that though there is one function that starts
 requests (mmc_start_request) there are two that wait for
 the request to be done (mmc_wait_for_req_done and
 mmc_wait_for_data_req_done).  Also note that
 mmc_wait_for_data_req_done can return even when the
 request is not done (which allows the block driver
 to prepare a newly arrived request while still
 waiting for the previous request).

 This patch ensures re-tuning is held for the duration
 of a request.  Subsequent patches will also hold
 re-tuning at other times when it might cause a
 conflict.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 
 Patch2 is calling mmc_retune_needed() and thus actually triggers a
 re-tune to potentially happen.

That won't happen because host-retune_period is not set, so
mmc_retune_needed() won't be called.

 
 To avoid bisectability issues about not holding re-tune when needed, I
 suggest we move $subject patch to after patch8.
 
 Kind regards
 Uffe
 
 ---
  drivers/mmc/core/core.c | 32 +---
  1 file changed, 25 insertions(+), 7 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index dd43f9b..a9936cb 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -186,12 +186,29 @@ void mmc_request_done(struct mmc_host *host, struct 
 mmc_request *mrq)

  EXPORT_SYMBOL(mmc_request_done);

 +static void __mmc_start_request(struct mmc_host *host, struct mmc_request 
 *mrq)
 +{
 +   int err;
 +
 +   /* Assumes host controller has been runtime resumed by 
 mmc_claim_host */
 +   err = mmc_retune(host);
 +   if (err) {
 +   mrq-cmd-error = err;
 +   mmc_request_done(host, mrq);
 +   return;
 +   }
 +
 +   host-ops-request(host, mrq);
 +}
 +
  static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
  {
  #ifdef CONFIG_MMC_DEBUG
 unsigned int i, sz;
 struct scatterlist *sg;
  #endif
 +   mmc_retune_hold(host);
 +
 if (mmc_card_removed(host-card))
 return -ENOMEDIUM;

 @@ -252,7 +269,7 @@ static int mmc_start_request(struct mmc_host *host, 
 struct mmc_request *mrq)
 }
 mmc_host_clk_hold(host);
 led_trigger_event(host-led, LED_FULL);
 -   host-ops-request(host, mrq);
 +   __mmc_start_request(host, mrq);

 return 0;
  }
 @@ -422,17 +439,16 @@ static int mmc_wait_for_data_req_done(struct mmc_host 
 *host,
 cmd-opcode, cmd-error);
 cmd-retries--;
 cmd-error = 0;
 -   host-ops-request(host, mrq);
 +   __mmc_start_request(host, mrq);
 continue; /* wait for done/new event again */
 }
 } else if (context_info-is_new_req) {
 context_info-is_new_req = false;
 -   if (!next_req) {
 -   err = MMC_BLK_NEW_REQUEST;
 -   break; /* return err */
 -   }
 +   if (!next_req)
 +   return MMC_BLK_NEW_REQUEST;
 }
 }
 +   mmc_retune_release(host);
 return err;
  }

 @@ -471,8 +487,10 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
  mmc_hostname(host), cmd-opcode, cmd-error);
 cmd-retries--;
 cmd-error = 0;
 -   host-ops-request(host, mrq);
 +   __mmc_start_request(host, mrq);
 }
 +
 +   mmc_retune_release(host);
  }

  /**
 --
 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
 
 

--
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 V6 00/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

On 20/04/15 15:09, Adrian Hunter wrote:
 Hi
 
 Here is V6 of some patches to move re-tuning support
 out of sdhci and into the core, and add support for HS400
 re-tuning.
 
 Currently sdhci does re-tuning transparently by
 calling sdhci_execute_tuning() from its -request()
 function.
 
 The problem with HS400 re-tuning is that it must be
 done in HS200 mode. That means using switch commands
 and making ios changes. That means it potentially
 conflicts with other command sequences. The new
 re-tuning support accomodates that.
 

Hi

I sent a V7 of patch 2, and a couple of patches that fix the
brcm custom sleep re-tuning issue. So is there anything else
that needs to be done?

Regards
Adrian


--
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 0/2] sdio: Prevent re-tuning conflicting with custom sleep

[Adrian Hunter]

Hi

Here are 2 patches that allow brcmfmac to awake from a
custom sleep state that conflicts with re-tuning.

The first patch adds sdio_retune_hold_now() and
sdio_retune_release(). They are used in the 2nd patch
to prevent re-tuning for the 'wake-up' command.


Adrian Hunter (2):
  mmc: core: Add functions for SDIO to hold re-tuning
  brcmfmac: Prevent re-tuning conflicting with 'wake-up'

 drivers/mmc/core/host.c|  6 ++
 drivers/mmc/core/host.h|  1 +
 drivers/mmc/core/sdio_io.c | 13 +
 drivers/net/wireless/brcm80211/brcmfmac/sdio.c |  6 ++
 include/linux/mmc/sdio_func.h  |  3 +++
 5 files changed, 29 insertions(+)


Regards
Adrian

--
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] brcmfmac: Prevent re-tuning conflicting with 'wake-up'

[Adrian Hunter]

If the device is in a custom sleep state, then re-tuning
will fail. Add calls to sdio_retune_hold_now() and
sdio_retune_release() to prevent re-tuning before the
wake-up command. In the case re-tuning was needed, the
wake-up command might return an error, but the wake-up
is expected still to have happened, and the error is
anyway ignored.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/net/wireless/brcm80211/brcmfmac/sdio.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/brcm80211/brcmfmac/sdio.c
index ab0c898..2ce81fb 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/sdio.c
@@ -773,11 +773,17 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
brcmf_dbg(TRACE, Enter: on=%d\n, on);
 
wr_val = (on  SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
+
+   /* Cannot re-tune if device is asleep */
+   if (on)
+   sdio_retune_hold_now(bus-sdiodev-func[1]);
+
/* 1st KSO write goes to AOS wake up core if device is asleep  */
brcmf_sdiod_regwb(bus-sdiodev, SBSDIO_FUNC1_SLEEPCSR,
  wr_val, err);
 
if (on) {
+   sdio_retune_release(bus-sdiodev-func[1]);
/* device WAKEUP through KSO:
 * write bit 0  read back until
 * both bits 0 (kso bit)  1 (dev on status) are set
-- 
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

[PATCH V7 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

Enable re-tuning when tuning is executed and
disable re-tuning when card is no longer initialized.

In the case of SDIO suspend, the card can keep power.
In that case, re-tuning need not be disabled, but, if
a re-tuning timer is being used, ensure it is disabled
and assume that re-tuning will be needed upon resume
since it is not known how long the suspend will last.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---


Changes in V7:


Also flag re-tune needed in SDIO 'keep_power'
case, when a re-tuning timer is being used.


 drivers/mmc/core/core.c | 4 
 drivers/mmc/core/sdio.c | 6 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 92e7671..007c444 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
 
if (err)
pr_err(%s: tuning execution failed\n, mmc_hostname(host));
+   else
+   mmc_retune_enable(host);
 
return err;
 }
@@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
int width)
  */
 void mmc_set_initial_state(struct mmc_host *host)
 {
+   mmc_retune_disable(host);
+
if (mmc_host_is_spi(host))
host-ios.chip_select = MMC_CS_HIGH;
else
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 5bc6c7d..16e1f39 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -934,8 +934,12 @@ static int mmc_sdio_suspend(struct mmc_host *host)
mmc_release_host(host);
}
 
-   if (!mmc_card_keep_power(host))
+   if (!mmc_card_keep_power(host)) {
mmc_power_off(host);
+   } else if (host-retune_period) {
+   mmc_retune_timer_stop(host);
+   mmc_retune_needed(host);
+   }
 
return 0;
 }
-- 
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 V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

[Adrian Hunter]

On 21/04/15 21:25, Arend van Spriel wrote:
 On 04/21/15 14:26, Adrian Hunter wrote:
 On 21/04/15 14:53, Ulf Hansson wrote:
 On 21 April 2015 at 13:00, Adrian Hunteradrian.hun...@intel.com  wrote:
 On 21/04/15 12:42, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunteradrian.hun...@intel.com  wrote:
 Currently mmc sleep is used before power off and
 is not paired with waking up. Nevertheless hold
 re-tuning.

 Signed-off-by: Adrian Hunteradrian.hun...@intel.com
 ---
   drivers/mmc/core/mmc.c | 14 +++---
   1 file changed, 11 insertions(+), 3 deletions(-)

 diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
 index f36c76f..daf9954 100644
 --- a/drivers/mmc/core/mmc.c
 +++ b/drivers/mmc/core/mmc.c
 @@ -21,6 +21,7 @@
   #includelinux/mmc/mmc.h

   #include core.h
 +#include host.h
   #include bus.h
   #include mmc_ops.h
   #include sd_ops.h
 @@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
  return (card  card-ext_csd.rev= 3);
   }

 +/* If necessary, callers must hold re-tuning */
   static int mmc_sleep(struct mmc_host *host)
   {
  struct mmc_command cmd = {0};
 @@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host,
 bool is_suspend)
  int err = 0;
  unsigned int notify_type = is_suspend ?
 EXT_CSD_POWER_OFF_SHORT :
  EXT_CSD_POWER_OFF_LONG;
 +   bool retune_release = false;

  BUG_ON(!host);
  BUG_ON(!host-card);
 @@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host,
 bool is_suspend)
  goto out;

  if (mmc_can_poweroff_notify(host-card)
 -   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
 +   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) ||
 !is_suspend)) {
  err = mmc_poweroff_notify(host-card, notify_type);
 -   else if (mmc_can_sleep(host-card))
 +   } else if (mmc_can_sleep(host-card)) {
 +   mmc_retune_hold(host);
  err = mmc_sleep(host);
 -   else if (!mmc_host_is_spi(host))
 +   } else if (!mmc_host_is_spi(host)) {
  err = mmc_deselect_cards(host);
 +   }

  if (!err) {
  mmc_power_off(host);
  mmc_card_set_suspended(host-card);
  }
 +
 +   if (retune_release)
 +   mmc_retune_release(host);
   out:
  mmc_release_host(host);
  return err;
 -- 
 1.9.1


 According to our previous discussions I have given this some more
 thinking.

 I don't think we can allow to hold/disable re-tune in this path at
 all. That's because we are claiming the host here and the sleep
 command might then be the first command we invoke during the system PM
 sequence.

 That means sdhci might have flagged need_retune, since it's been
 runtime PM suspended. And for those scenarios I guess we really need
 to do a re-tune prior sending the sleep command, right?

 Yes, although that is how it works.

 Ohh, you are one step ahead of me. Good! :-)


 Previously I had two functions mmc_retune_hold() and mmc_retune_and_hold()
 but after one of the revisions I found that only one was needed. I stuck
 with the mmc_retune_hold() name because it doesn't necessarily cause a
 re-tune, but only if the hold count was zero and a retune is needed.


 Earlier I only had the re-tune timer in mind, which is why I was less
 restrictive and suggesting you to add hold/disable. Sorry about that.

 Now, with the above in mind I believe you have similar issues with
 patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
 (mmc: core: Hold re-tuning during erase commands). And that's because
 there are cases when the switch/erase commands are the first commands
 sent, after the sdhci host has been runtime PM suspended. I guess we
 need a way to make sure we don't hold re-tune for these cases.

 An option to deal with that is to use a separate flag set by host
 drivers, though the mmc_needs_retune() API and let that one override
 another.

 Forgive me for pushing you back and forth for how to do this, but it

 Not a problem. Thanks for persevering.

 seems like we still have some outstanding issues to resolve.

 So that then more or less leaves us with one outstanding issue. The
 SDIO irq wakeup scenario.

 How will that work for sdhci?

 Your suggestion is to hold re-tune for the SDIO wakeup command. If I
 understand correct that could be overridden when the host flags
 need_retune from its runtime PM suspend callback, right?

 That then mean that the re-tuning will be done prior sending the
 wakeup command? That wouldn't work, unless the re-tune command also
 act as wakeup, which I doubt.

 The wakeup command has to come first.


 If I _haven't_ understand correctly and you mean that the SDIO wakeup
 command shall be invoked prior re-tuning is done; that would mean that
 SDHCI will send a command to the card without first satisfying its
 need for a re-tune

Re: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

[Adrian Hunter]

On 21/04/15 13:18, Dov Levenglick wrote:
 On 20/04/15 17:00, Dov Levenglick wrote:
 Add pm_qos_cancel_request_lazy() which is convenient for
 contexts that may not sleep.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  include/linux/pm_qos.h |  2 ++
  kernel/power/qos.c | 20 
  2 files changed, 22 insertions(+)

 diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
 index 7b3ae0c..f44d353 100644
 --- a/include/linux/pm_qos.h
 +++ b/include/linux/pm_qos.h
 @@ -126,6 +126,8 @@ void pm_qos_update_request(struct pm_qos_request
 *req,
   s32 new_value);
  void pm_qos_update_request_timeout(struct pm_qos_request *req,
   s32 new_value, unsigned long
 timeout_us);
 +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
 +  unsigned int timeout_us);
  void pm_qos_remove_request(struct pm_qos_request *req);


 I think that this could be acheived using existing API if
 pm_qos_update_request_timeout() were to be called with the existing
 timeout value.

 I don't follow what you mean. There is no existing timeout value.
 Did you mean existing request value? There is still the difference wrt
 cancel_delayed_work_sync.
 
 I did mean the existing request value. Thanks.
 There is also the cancel_delayed_work_sync, however I think that that
 should be called in any case in order to cancel any other pending timeout
 changes.

That might sleep which defeats one of the reasons for creating
pm_qos_cancel_request_lazy().

 

 Since reading the existing timeout value is missing - and I think would
 be
 a useful feature to have for other use-cases - do you agree with such an
 approach?

  int pm_qos_request(int pm_qos_class);
 diff --git a/kernel/power/qos.c b/kernel/power/qos.c
 index 97b0df7..ac131cb 100644
 --- a/kernel/power/qos.c
 +++ b/kernel/power/qos.c
 @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
 pm_qos_request *req, s32 new_value,
  }

  /**
 + * pm_qos_cancel_request_lazy - cancels an existing qos request
 lazily.
 + * @req : handle to list element holding a pm_qos request to use
 + * @timeout_us: the delay before cancelling this qos request in usecs.
 + *
 + * After timeout_us, this qos request is cancelled.
 + */
 +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
 +  unsigned int timeout_us)
 +{
 +  if (!req)
 +  return;
 +  if (WARN(!pm_qos_request_active(req),
 +   %s called for unknown object., __func__))
 +  return;
 +
 +  schedule_delayed_work(req-work, usecs_to_jiffies(timeout_us));
 +}
 +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
 +
 +/**
   * pm_qos_remove_request - modifies an existing qos request
   * @req: handle to request list element
   *
 --
 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



 QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
 Forum,
 a Linux Foundation Collaborative Project




 --
 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

 
 
 QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
 
 

--
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: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

[Adrian Hunter]

On 20/04/15 17:00, Dov Levenglick wrote:
 Add pm_qos_cancel_request_lazy() which is convenient for
 contexts that may not sleep.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  include/linux/pm_qos.h |  2 ++
  kernel/power/qos.c | 20 
  2 files changed, 22 insertions(+)

 diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
 index 7b3ae0c..f44d353 100644
 --- a/include/linux/pm_qos.h
 +++ b/include/linux/pm_qos.h
 @@ -126,6 +126,8 @@ void pm_qos_update_request(struct pm_qos_request *req,
 s32 new_value);
  void pm_qos_update_request_timeout(struct pm_qos_request *req,
 s32 new_value, unsigned long
 timeout_us);
 +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
 +unsigned int timeout_us);
  void pm_qos_remove_request(struct pm_qos_request *req);

 
 I think that this could be acheived using existing API if
 pm_qos_update_request_timeout() were to be called with the existing
 timeout value.

I don't follow what you mean. There is no existing timeout value.
Did you mean existing request value? There is still the difference wrt
cancel_delayed_work_sync.

 Since reading the existing timeout value is missing - and I think would be
 a useful feature to have for other use-cases - do you agree with such an
 approach?
 
  int pm_qos_request(int pm_qos_class);
 diff --git a/kernel/power/qos.c b/kernel/power/qos.c
 index 97b0df7..ac131cb 100644
 --- a/kernel/power/qos.c
 +++ b/kernel/power/qos.c
 @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
 pm_qos_request *req, s32 new_value,
  }

  /**
 + * pm_qos_cancel_request_lazy - cancels an existing qos request lazily.
 + * @req : handle to list element holding a pm_qos request to use
 + * @timeout_us: the delay before cancelling this qos request in usecs.
 + *
 + * After timeout_us, this qos request is cancelled.
 + */
 +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
 +unsigned int timeout_us)
 +{
 +if (!req)
 +return;
 +if (WARN(!pm_qos_request_active(req),
 + %s called for unknown object., __func__))
 +return;
 +
 +schedule_delayed_work(req-work, usecs_to_jiffies(timeout_us));
 +}
 +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
 +
 +/**
   * pm_qos_remove_request - modifies an existing qos request
   * @req: handle to request list element
   *
 --
 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

 
 
 QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
 The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
 a Linux Foundation Collaborative Project
 
 
 

--
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 V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

[Adrian Hunter]

On 21/04/15 14:53, Ulf Hansson wrote:
 On 21 April 2015 at 13:00, Adrian Hunter adrian.hun...@intel.com wrote:
 On 21/04/15 12:42, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunter adrian.hun...@intel.com wrote:
 Currently mmc sleep is used before power off and
 is not paired with waking up. Nevertheless hold
 re-tuning.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/mmc.c | 14 +++---
  1 file changed, 11 insertions(+), 3 deletions(-)

 diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
 index f36c76f..daf9954 100644
 --- a/drivers/mmc/core/mmc.c
 +++ b/drivers/mmc/core/mmc.c
 @@ -21,6 +21,7 @@
  #include linux/mmc/mmc.h

  #include core.h
 +#include host.h
  #include bus.h
  #include mmc_ops.h
  #include sd_ops.h
 @@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
 return (card  card-ext_csd.rev = 3);
  }

 +/* If necessary, callers must hold re-tuning */
  static int mmc_sleep(struct mmc_host *host)
  {
 struct mmc_command cmd = {0};
 @@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host, bool 
 is_suspend)
 int err = 0;
 unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
 EXT_CSD_POWER_OFF_LONG;
 +   bool retune_release = false;

 BUG_ON(!host);
 BUG_ON(!host-card);
 @@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host, 
 bool is_suspend)
 goto out;

 if (mmc_can_poweroff_notify(host-card) 
 -   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
 +   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend)) {
 err = mmc_poweroff_notify(host-card, notify_type);
 -   else if (mmc_can_sleep(host-card))
 +   } else if (mmc_can_sleep(host-card)) {
 +   mmc_retune_hold(host);
 err = mmc_sleep(host);
 -   else if (!mmc_host_is_spi(host))
 +   } else if (!mmc_host_is_spi(host)) {
 err = mmc_deselect_cards(host);
 +   }

 if (!err) {
 mmc_power_off(host);
 mmc_card_set_suspended(host-card);
 }
 +
 +   if (retune_release)
 +   mmc_retune_release(host);
  out:
 mmc_release_host(host);
 return err;
 --
 1.9.1


 According to our previous discussions I have given this some more thinking.

 I don't think we can allow to hold/disable re-tune in this path at
 all. That's because we are claiming the host here and the sleep
 command might then be the first command we invoke during the system PM
 sequence.

 That means sdhci might have flagged need_retune, since it's been
 runtime PM suspended. And for those scenarios I guess we really need
 to do a re-tune prior sending the sleep command, right?

 Yes, although that is how it works.
 
 Ohh, you are one step ahead of me. Good! :-)
 

 Previously I had two functions mmc_retune_hold() and mmc_retune_and_hold()
 but after one of the revisions I found that only one was needed. I stuck
 with the mmc_retune_hold() name because it doesn't necessarily cause a
 re-tune, but only if the hold count was zero and a retune is needed.


 Earlier I only had the re-tune timer in mind, which is why I was less
 restrictive and suggesting you to add hold/disable. Sorry about that.

 Now, with the above in mind I believe you have similar issues with
 patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
 (mmc: core: Hold re-tuning during erase commands). And that's because
 there are cases when the switch/erase commands are the first commands
 sent, after the sdhci host has been runtime PM suspended. I guess we
 need a way to make sure we don't hold re-tune for these cases.

 An option to deal with that is to use a separate flag set by host
 drivers, though the mmc_needs_retune() API and let that one override
 another.

 Forgive me for pushing you back and forth for how to do this, but it

 Not a problem. Thanks for persevering.

 seems like we still have some outstanding issues to resolve.
 
 So that then more or less leaves us with one outstanding issue. The
 SDIO irq wakeup scenario.
 
 How will that work for sdhci?
 
 Your suggestion is to hold re-tune for the SDIO wakeup command. If I
 understand correct that could be overridden when the host flags
 need_retune from its runtime PM suspend callback, right?
 
 That then mean that the re-tuning will be done prior sending the
 wakeup command? That wouldn't work, unless the re-tune command also
 act as wakeup, which I doubt.

The wakeup command has to come first.

 
 If I _haven't_ understand correctly and you mean that the SDIO wakeup
 command shall be invoked prior re-tuning is done; that would mean that
 SDHCI will send a command to the card without first satisfying its
 need for a re-tune. And that wouldn't work either, right?

My understanding is that the wakeup command will still work but there might

Re: [PATCH V6 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 21/04/15 11:59, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunter adrian.hun...@intel.com wrote:
 Enable re-tuning when tuning is executed and
 disable re-tuning when card is no longer initialized.

 In the case of SDIO suspend, the card can keep power.
 In that case, re-tuning need not be disabled, but ensure
 the re-tuning timer is disabled.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/core.c | 4 
  drivers/mmc/core/sdio.c | 2 ++
  2 files changed, 6 insertions(+)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index c296bc0..dd43f9b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)

 if (err)
 pr_err(%s: tuning execution failed\n, mmc_hostname(host));
 +   else
 +   mmc_retune_enable(host);

 return err;
  }
 @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
 int width)
   */
  void mmc_set_initial_state(struct mmc_host *host)
  {
 +   mmc_retune_disable(host);
 +
 if (mmc_host_is_spi(host))
 host-ios.chip_select = MMC_CS_HIGH;
 else
 diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
 index 5bc6c7d..ee7bfd4 100644
 --- a/drivers/mmc/core/sdio.c
 +++ b/drivers/mmc/core/sdio.c
 @@ -936,6 +936,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)

 if (!mmc_card_keep_power(host))
 mmc_power_off(host);
 +   else
 +   mmc_retune_timer_stop(host);

 
 We also need to re-start the timer at the end of mmc_sdio_resume() in
 case of mmc_card_keep_power().

It seems to me if a timer is being used, and the suspend can last an
arbitrary length of time, then the safe thing to do is to re-tune after
resume. i.e.

if (!mmc_card_keep_power(host)) {
mmc_power_off(host);
} else if (host-host-retune_period) {
mmc_retune_timer_stop(host);
mmc_retune_needed(host);
}

That is what sdhci has to do anyway.

 
 return 0;
  }
 --
 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
 
 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 V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

[Adrian Hunter]

On 21/04/15 12:42, Ulf Hansson wrote:
 On 20 April 2015 at 14:09, Adrian Hunter adrian.hun...@intel.com wrote:
 Currently mmc sleep is used before power off and
 is not paired with waking up. Nevertheless hold
 re-tuning.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/mmc.c | 14 +++---
  1 file changed, 11 insertions(+), 3 deletions(-)

 diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
 index f36c76f..daf9954 100644
 --- a/drivers/mmc/core/mmc.c
 +++ b/drivers/mmc/core/mmc.c
 @@ -21,6 +21,7 @@
  #include linux/mmc/mmc.h

  #include core.h
 +#include host.h
  #include bus.h
  #include mmc_ops.h
  #include sd_ops.h
 @@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
 return (card  card-ext_csd.rev = 3);
  }

 +/* If necessary, callers must hold re-tuning */
  static int mmc_sleep(struct mmc_host *host)
  {
 struct mmc_command cmd = {0};
 @@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host, bool 
 is_suspend)
 int err = 0;
 unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
 EXT_CSD_POWER_OFF_LONG;
 +   bool retune_release = false;

 BUG_ON(!host);
 BUG_ON(!host-card);
 @@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host, bool 
 is_suspend)
 goto out;

 if (mmc_can_poweroff_notify(host-card) 
 -   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
 +   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend)) {
 err = mmc_poweroff_notify(host-card, notify_type);
 -   else if (mmc_can_sleep(host-card))
 +   } else if (mmc_can_sleep(host-card)) {
 +   mmc_retune_hold(host);
 err = mmc_sleep(host);
 -   else if (!mmc_host_is_spi(host))
 +   } else if (!mmc_host_is_spi(host)) {
 err = mmc_deselect_cards(host);
 +   }

 if (!err) {
 mmc_power_off(host);
 mmc_card_set_suspended(host-card);
 }
 +
 +   if (retune_release)
 +   mmc_retune_release(host);
  out:
 mmc_release_host(host);
 return err;
 --
 1.9.1

 
 According to our previous discussions I have given this some more thinking.
 
 I don't think we can allow to hold/disable re-tune in this path at
 all. That's because we are claiming the host here and the sleep
 command might then be the first command we invoke during the system PM
 sequence.
 
 That means sdhci might have flagged need_retune, since it's been
 runtime PM suspended. And for those scenarios I guess we really need
 to do a re-tune prior sending the sleep command, right?

Yes, although that is how it works.

Previously I had two functions mmc_retune_hold() and mmc_retune_and_hold()
but after one of the revisions I found that only one was needed. I stuck
with the mmc_retune_hold() name because it doesn't necessarily cause a
re-tune, but only if the hold count was zero and a retune is needed.

 
 Earlier I only had the re-tune timer in mind, which is why I was less
 restrictive and suggesting you to add hold/disable. Sorry about that.
 
 Now, with the above in mind I believe you have similar issues with
 patch5 (mmc: core: Hold re-tuning during switch commands) and patch6
 (mmc: core: Hold re-tuning during erase commands). And that's because
 there are cases when the switch/erase commands are the first commands
 sent, after the sdhci host has been runtime PM suspended. I guess we
 need a way to make sure we don't hold re-tune for these cases.
 
 An option to deal with that is to use a separate flag set by host
 drivers, though the mmc_needs_retune() API and let that one override
 another.
 
 Forgive me for pushing you back and forth for how to do this, but it

Not a problem. Thanks for persevering.

 seems like we still have some outstanding issues to resolve.



--
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 V6 13/15] mmc: block: Check re-tuning in the recovery path

[Adrian Hunter]

If re-tuning is needed, do it in the recovery path to
give recovery commands a better chance of success.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/card/block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 2c25271..325ae1e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -913,6 +913,9 @@ static int mmc_blk_cmd_recovery(struct mmc_card *card, 
struct request *req,
if (!err)
break;
 
+   /* Re-tune if needed */
+   mmc_retune_recheck(card-host);
+
prev_cmd_status_valid = false;
pr_err(%s: error %d sending status command, %sing\n,
   req-rq_disk-disk_name, err, retry ? retry : abort);
-- 
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

[PATCH V6 11/15] mmc: sdhci: Change to new way of doing re-tuning

[Adrian Hunter]

Make use of mmc core support for re-tuning instead
of doing it all in the sdhci driver.

This patch also changes to flag the need for re-tuning
always after runtime suspend when tuning has been used
at initialization. Previously it was only done if
the re-tuning timer was in use.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/host/sdhci.c | 112 ++-
 drivers/mmc/host/sdhci.h |   3 --
 2 files changed, 13 insertions(+), 102 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c80287a..b345844 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -52,7 +52,6 @@ static void sdhci_finish_data(struct sdhci_host *);
 
 static void sdhci_finish_command(struct sdhci_host *);
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
-static void sdhci_tuning_timer(unsigned long data);
 static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
 static int sdhci_pre_dma_transfer(struct sdhci_host *host,
struct mmc_data *data,
@@ -254,17 +253,6 @@ static void sdhci_init(struct sdhci_host *host, int soft)
 static void sdhci_reinit(struct sdhci_host *host)
 {
sdhci_init(host, 0);
-   /*
-* Retuning stuffs are affected by different cards inserted and only
-* applicable to UHS-I cards. So reset these fields to their initial
-* value when card is removed.
-*/
-   if (host-flags  SDHCI_USING_RETUNING_TIMER) {
-   host-flags = ~SDHCI_USING_RETUNING_TIMER;
-
-   del_timer_sync(host-tuning_timer);
-   host-flags = ~SDHCI_NEEDS_RETUNING;
-   }
sdhci_enable_card_detection(host);
 }
 
@@ -1353,7 +1341,6 @@ static void sdhci_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
struct sdhci_host *host;
int present;
unsigned long flags;
-   u32 tuning_opcode;
 
host = mmc_priv(mmc);
 
@@ -1387,39 +1374,6 @@ static void sdhci_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
host-mrq-cmd-error = -ENOMEDIUM;
tasklet_schedule(host-finish_tasklet);
} else {
-   u32 present_state;
-
-   present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
-   /*
-* Check if the re-tuning timer has already expired and there
-* is no on-going data transfer and DAT0 is not busy. If so,
-* we need to execute tuning procedure before sending command.
-*/
-   if ((host-flags  SDHCI_NEEDS_RETUNING) 
-   !(present_state  (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) 
-   (present_state  SDHCI_DATA_0_LVL_MASK)) {
-   if (mmc-card) {
-   /* eMMC uses cmd21 but sd and sdio use cmd19 */
-   tuning_opcode =
-   mmc-card-type == MMC_TYPE_MMC ?
-   MMC_SEND_TUNING_BLOCK_HS200 :
-   MMC_SEND_TUNING_BLOCK;
-
-   /* Here we need to set the host-mrq to NULL,
-* in case the pending finish_tasklet
-* finishes it incorrectly.
-*/
-   host-mrq = NULL;
-
-   spin_unlock_irqrestore(host-lock, flags);
-   sdhci_execute_tuning(mmc, tuning_opcode);
-   spin_lock_irqsave(host-lock, flags);
-
-   /* Restore original mmc_request structure */
-   host-mrq = mrq;
-   }
-   }
-
if (mrq-sbc  !(host-flags  SDHCI_AUTO_CMD23))
sdhci_send_command(host, mrq-sbc);
else
@@ -2065,23 +2019,18 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, 
u32 opcode)
}
 
 out:
-   host-flags = ~SDHCI_NEEDS_RETUNING;
-
if (tuning_count) {
-   host-flags |= SDHCI_USING_RETUNING_TIMER;
-   mod_timer(host-tuning_timer, jiffies + tuning_count * HZ);
+   /*
+* In case tuning fails, host controllers which support
+* re-tuning can try tuning again at a later time, when the
+* re-tuning timer expires.  So for these controllers, we
+* return 0. Since there might be other controllers who do not
+* have this capability, we return error for them.
+*/
+   err = 0;
}
 
-   /*
-* In case tuning fails, host controllers which support re-tuning can
-* try tuning again at a later time, when the re-tuning timer expires.
-* So for these controllers, we return 0. Since

[PATCH V6 15/15] mmc: core: Don't print reset warning if reset is not supported

[Adrian Hunter]

Check the error code for EOPNOTSUPP and do not print
reset warning in that case.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7aae0ff..0b25e70 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2365,7 +2365,8 @@ int mmc_hw_reset(struct mmc_host *host)
ret = host-bus_ops-reset(host);
mmc_bus_put(host);
 
-   pr_warn(%s: tried to reset card\n, mmc_hostname(host));
+   if (ret != -EOPNOTSUPP)
+   pr_warn(%s: tried to reset card\n, mmc_hostname(host));
 
return ret;
 }
-- 
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

[PATCH V6 03/15] mmc: core: Add support for re-tuning before each request

[Adrian Hunter]

At the start of each request, re-tune if needed and
then hold off re-tuning again until the request is done.

Note that though there is one function that starts
requests (mmc_start_request) there are two that wait for
the request to be done (mmc_wait_for_req_done and
mmc_wait_for_data_req_done).  Also note that
mmc_wait_for_data_req_done can return even when the
request is not done (which allows the block driver
to prepare a newly arrived request while still
waiting for the previous request).

This patch ensures re-tuning is held for the duration
of a request.  Subsequent patches will also hold
re-tuning at other times when it might cause a
conflict.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dd43f9b..a9936cb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -186,12 +186,29 @@ void mmc_request_done(struct mmc_host *host, struct 
mmc_request *mrq)
 
 EXPORT_SYMBOL(mmc_request_done);
 
+static void __mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
+{
+   int err;
+
+   /* Assumes host controller has been runtime resumed by mmc_claim_host */
+   err = mmc_retune(host);
+   if (err) {
+   mrq-cmd-error = err;
+   mmc_request_done(host, mrq);
+   return;
+   }
+
+   host-ops-request(host, mrq);
+}
+
 static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 {
 #ifdef CONFIG_MMC_DEBUG
unsigned int i, sz;
struct scatterlist *sg;
 #endif
+   mmc_retune_hold(host);
+
if (mmc_card_removed(host-card))
return -ENOMEDIUM;
 
@@ -252,7 +269,7 @@ static int mmc_start_request(struct mmc_host *host, struct 
mmc_request *mrq)
}
mmc_host_clk_hold(host);
led_trigger_event(host-led, LED_FULL);
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
 
return 0;
 }
@@ -422,17 +439,16 @@ static int mmc_wait_for_data_req_done(struct mmc_host 
*host,
cmd-opcode, cmd-error);
cmd-retries--;
cmd-error = 0;
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
continue; /* wait for done/new event again */
}
} else if (context_info-is_new_req) {
context_info-is_new_req = false;
-   if (!next_req) {
-   err = MMC_BLK_NEW_REQUEST;
-   break; /* return err */
-   }
+   if (!next_req)
+   return MMC_BLK_NEW_REQUEST;
}
}
+   mmc_retune_release(host);
return err;
 }
 
@@ -471,8 +487,10 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 mmc_hostname(host), cmd-opcode, cmd-error);
cmd-retries--;
cmd-error = 0;
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
}
+
+   mmc_retune_release(host);
 }
 
 /**
-- 
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

[PATCH V6 12/15] mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors

[Adrian Hunter]

CRC or End-Bit errors could possibly be alleviated by
re-tuning so flag re-tuning needed in those cases.
Note this has no effect if re-tuning has not been
enabled.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/host/sdhci.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b345844..d11fae7 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2307,8 +2307,10 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 
intmask, u32 *mask)
if (intmask  SDHCI_INT_TIMEOUT)
host-cmd-error = -ETIMEDOUT;
else if (intmask  (SDHCI_INT_CRC | SDHCI_INT_END_BIT |
-   SDHCI_INT_INDEX))
+   SDHCI_INT_INDEX)) {
host-cmd-error = -EILSEQ;
+   mmc_retune_needed(host-mmc);
+   }
 
if (host-cmd-error) {
tasklet_schedule(host-finish_tasklet);
@@ -2433,13 +2435,15 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 
intmask)
 
if (intmask  SDHCI_INT_DATA_TIMEOUT)
host-data-error = -ETIMEDOUT;
-   else if (intmask  SDHCI_INT_DATA_END_BIT)
+   else if (intmask  SDHCI_INT_DATA_END_BIT) {
host-data-error = -EILSEQ;
-   else if ((intmask  SDHCI_INT_DATA_CRC) 
+   mmc_retune_needed(host-mmc);
+   } else if ((intmask  SDHCI_INT_DATA_CRC) 
SDHCI_GET_CMD(sdhci_readw(host, SDHCI_COMMAND))
-   != MMC_BUS_TEST_R)
+   != MMC_BUS_TEST_R) {
host-data-error = -EILSEQ;
-   else if (intmask  SDHCI_INT_ADMA_ERROR) {
+   mmc_retune_needed(host-mmc);
+   } else if (intmask  SDHCI_INT_ADMA_ERROR) {
pr_err(%s: ADMA error\n, mmc_hostname(host-mmc));
sdhci_adma_show_error(host);
host-data-error = -EIO;
-- 
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

[PATCH V6 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Adrian Hunter]

Hold re-tuning during bkops to prevent
it from conflicting with the busy state.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dbd7a77..7aae0ff 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -318,12 +318,15 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
use_busy_signal = false;
}
 
+   mmc_retune_hold(card-host);
+
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_BKOPS_START, 1, timeout,
use_busy_signal, true, false);
if (err) {
pr_warn(%s: Error %d starting bkops\n,
mmc_hostname(card-host), err);
+   mmc_retune_release(card-host);
goto out;
}
 
@@ -334,6 +337,8 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
 */
if (!use_busy_signal)
mmc_card_set_doing_bkops(card);
+   else
+   mmc_retune_release(card-host);
 out:
mmc_release_host(card-host);
 }
@@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
 */
if (!err || (err == -EINVAL)) {
mmc_card_clr_doing_bkops(card);
+   mmc_retune_release(card-host);
err = 0;
}
 
-- 
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

[PATCH V6 05/15] mmc: core: Hold re-tuning during switch commands

[Adrian Hunter]

Hold re-tuning during switch commands to prevent
it from conflicting with the busy state or the CMD13
verification.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/mmc_ops.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0ea042d..4cad5f0 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -19,6 +19,7 @@
 #include linux/mmc/mmc.h
 
 #include core.h
+#include host.h
 #include mmc_ops.h
 
 #define MMC_OPS_TIMEOUT_MS (10 * 60 * 1000) /* 10 minute timeout */
@@ -474,6 +475,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
u32 status = 0;
bool use_r1b_resp = use_busy_signal;
 
+   mmc_retune_hold(host);
+
/*
 * If the cmd timeout and the max_busy_timeout of the host are both
 * specified, let's validate them. A failure means we need to prevent
@@ -506,11 +509,11 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
 
err = mmc_wait_for_cmd(host, cmd, MMC_CMD_RETRIES);
if (err)
-   return err;
+   goto out;
 
/* No need to check card status in case of unblocking command */
if (!use_busy_signal)
-   return 0;
+   goto out;
 
/*
 * CRC errors shall only be ignored in cases were CMD13 is used to poll
@@ -529,7 +532,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
if (send_status) {
err = __mmc_send_status(card, status, ignore_crc);
if (err)
-   return err;
+   goto out;
}
if ((host-caps  MMC_CAP_WAIT_WHILE_BUSY)  use_r1b_resp)
break;
@@ -543,29 +546,36 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
 */
if (!send_status) {
mmc_delay(timeout_ms);
-   return 0;
+   goto out;
}
 
/* Timeout if the device never leaves the program state. */
if (time_after(jiffies, timeout)) {
pr_err(%s: Card stuck in programming state! %s\n,
mmc_hostname(host), __func__);
-   return -ETIMEDOUT;
+   err = -ETIMEDOUT;
+   goto out;
}
} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
if (mmc_host_is_spi(host)) {
-   if (status  R1_SPI_ILLEGAL_COMMAND)
-   return -EBADMSG;
+   if (status  R1_SPI_ILLEGAL_COMMAND) {
+   err = -EBADMSG;
+   goto out;
+   }
} else {
if (status  0xFDFFA000)
pr_warn(%s: unexpected status %#x after switch\n,
mmc_hostname(host), status);
-   if (status  R1_SWITCH_ERROR)
-   return -EBADMSG;
+   if (status  R1_SWITCH_ERROR) {
+   err = -EBADMSG;
+   goto out;
+   }
}
+out:
+   mmc_retune_release(host);
 
-   return 0;
+   return err;
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
-- 
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

[PATCH V6 14/15] mmc: block: Retry errored data requests when re-tuning is needed

[Adrian Hunter]

Retry errored data requests when re-tuning is needed and
add a flag to struct mmc_blk_request so that the retry
is only done once.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/card/block.c | 11 ++-
 drivers/mmc/card/queue.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 325ae1e..509fcca 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1195,6 +1195,7 @@ static int mmc_blk_err_check(struct mmc_card *card,
mmc_active);
struct mmc_blk_request *brq = mq_mrq-brq;
struct request *req = mq_mrq-req;
+   int need_retune = card-host-need_retune;
int ecc_err = 0, gen_err = 0;
 
/*
@@ -1262,6 +1263,12 @@ static int mmc_blk_err_check(struct mmc_card *card,
}
 
if (brq-data.error) {
+   if (need_retune  !brq-retune_retry_done) {
+   pr_info(%s: retrying because a re-tune was needed\n,
+   req-rq_disk-disk_name);
+   brq-retune_retry_done = 1;
+   return MMC_BLK_RETRY;
+   }
pr_err(%s: error %d transferring data, sector %u, nr %u, cmd 
response %#x, card status %#x\n,
   req-rq_disk-disk_name, brq-data.error,
   (unsigned)blk_rq_pos(req),
@@ -1821,7 +1828,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
struct mmc_blk_data *md = mq-data;
struct mmc_card *card = md-queue.card;
struct mmc_blk_request *brq = mq-mqrq_cur-brq;
-   int ret = 1, disable_multi = 0, retry = 0, type;
+   int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
enum mmc_blk_status status;
struct mmc_queue_req *mq_rq;
struct request *req = rqc;
@@ -1905,6 +1912,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
break;
goto cmd_abort;
case MMC_BLK_RETRY:
+   retune_retry_done = brq-retune_retry_done;
if (retry++  5)
break;
/* Fall through */
@@ -1967,6 +1975,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
mmc_start_req(card-host,
mq_rq-mmc_active, NULL);
}
+   mq_rq-brq.retune_retry_done = retune_retry_done;
}
} while (ret);
 
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 5752d50..7e27915 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -12,6 +12,7 @@ struct mmc_blk_request {
struct mmc_command  cmd;
struct mmc_command  stop;
struct mmc_data data;
+   int retune_retry_done;
 };
 
 enum mmc_packed_type {
-- 
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

[PATCH V6 06/15] mmc: core: Hold re-tuning during erase commands

[Adrian Hunter]

Hold re-tuning during erase commands to prevent
it from conflicting with the sequence of commands.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 08bf7e3..dbd7a77 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1995,6 +1995,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned 
int from,
unsigned long timeout;
int err;
 
+   mmc_retune_hold(card-host);
+
/*
 * qty is used to calculate the erase timeout which depends on how many
 * erase groups (or allocation units in SD terminology) are affected.
@@ -2098,6 +2100,7 @@ static int mmc_do_erase(struct mmc_card *card, unsigned 
int from,
} while (!(cmd.resp[0]  R1_READY_FOR_DATA) ||
 (R1_CURRENT_STATE(cmd.resp[0]) == R1_STATE_PRG));
 out:
+   mmc_retune_release(card-host);
return err;
 }
 
-- 
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

[PATCH V6 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

Enable re-tuning when tuning is executed and
disable re-tuning when card is no longer initialized.

In the case of SDIO suspend, the card can keep power.
In that case, re-tuning need not be disabled, but ensure
the re-tuning timer is disabled.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 4 
 drivers/mmc/core/sdio.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c296bc0..dd43f9b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
 
if (err)
pr_err(%s: tuning execution failed\n, mmc_hostname(host));
+   else
+   mmc_retune_enable(host);
 
return err;
 }
@@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
int width)
  */
 void mmc_set_initial_state(struct mmc_host *host)
 {
+   mmc_retune_disable(host);
+
if (mmc_host_is_spi(host))
host-ios.chip_select = MMC_CS_HIGH;
else
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 5bc6c7d..ee7bfd4 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -936,6 +936,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 
if (!mmc_card_keep_power(host))
mmc_power_off(host);
+   else
+   mmc_retune_timer_stop(host);
 
return 0;
 }
-- 
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

[PATCH V6 04/15] mmc: core: Check re-tuning before retrying

[Adrian Hunter]

Possibly a command is failing because re-tuning is needed.
Use mmc_retune_recheck() to check re-tuning. At that point
re-tuning is held, at least by the request, so
mmc_retune_recheck() flags host-retune_now if the hold
count is 1.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a9936cb..08bf7e3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -434,6 +434,7 @@ static int mmc_wait_for_data_req_done(struct mmc_host *host,
host-areq);
break; /* return err */
} else {
+   mmc_retune_recheck(host);
pr_info(%s: req failed (CMD%u): %d, 
retrying...\n,
mmc_hostname(host),
cmd-opcode, cmd-error);
@@ -483,6 +484,8 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
mmc_card_removed(host-card))
break;
 
+   mmc_retune_recheck(host);
+
pr_debug(%s: req failed (CMD%u): %d, retrying...\n,
 mmc_hostname(host), cmd-opcode, cmd-error);
cmd-retries--;
-- 
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

[PATCH V6 08/15] mmc: mmc: Hold re-tuning if the card is put to sleep

[Adrian Hunter]

Currently mmc sleep is used before power off and
is not paired with waking up. Nevertheless hold
re-tuning.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/mmc.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f36c76f..daf9954 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -21,6 +21,7 @@
 #include linux/mmc/mmc.h
 
 #include core.h
+#include host.h
 #include bus.h
 #include mmc_ops.h
 #include sd_ops.h
@@ -1504,6 +1505,7 @@ static int mmc_can_sleep(struct mmc_card *card)
return (card  card-ext_csd.rev = 3);
 }
 
+/* If necessary, callers must hold re-tuning */
 static int mmc_sleep(struct mmc_host *host)
 {
struct mmc_command cmd = {0};
@@ -1631,6 +1633,7 @@ static int _mmc_suspend(struct mmc_host *host, bool 
is_suspend)
int err = 0;
unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
EXT_CSD_POWER_OFF_LONG;
+   bool retune_release = false;
 
BUG_ON(!host);
BUG_ON(!host-card);
@@ -1651,17 +1654,22 @@ static int _mmc_suspend(struct mmc_host *host, bool 
is_suspend)
goto out;
 
if (mmc_can_poweroff_notify(host-card) 
-   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
+   ((host-caps2  MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend)) {
err = mmc_poweroff_notify(host-card, notify_type);
-   else if (mmc_can_sleep(host-card))
+   } else if (mmc_can_sleep(host-card)) {
+   mmc_retune_hold(host);
err = mmc_sleep(host);
-   else if (!mmc_host_is_spi(host))
+   } else if (!mmc_host_is_spi(host)) {
err = mmc_deselect_cards(host);
+   }
 
if (!err) {
mmc_power_off(host);
mmc_card_set_suspended(host-card);
}
+
+   if (retune_release)
+   mmc_retune_release(host);
 out:
mmc_release_host(host);
return err;
-- 
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

[PATCH V6 10/15] mmc: core: Add support for HS400 re-tuning

[Adrian Hunter]

HS400 re-tuning must be done in HS200 mode. Add
the ability to switch from HS400 mode to HS200
mode before re-tuning and switch back to HS400
after re-tuning.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.h |  2 ++
 drivers/mmc/core/host.c | 17 ++
 drivers/mmc/core/mmc.c  | 88 +
 3 files changed, 107 insertions(+)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index cfba3c0..e6f2de7 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -88,6 +88,8 @@ void mmc_remove_card_debugfs(struct mmc_card *card);
 void mmc_init_context_info(struct mmc_host *host);
 
 int mmc_execute_tuning(struct mmc_card *card);
+int mmc_hs200_to_hs400(struct mmc_card *card);
+int mmc_hs400_to_hs200(struct mmc_card *card);
 
 #endif
 
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e90e02f..86c495b 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -340,6 +340,7 @@ void mmc_retune_release(struct mmc_host *host)
 
 int mmc_retune(struct mmc_host *host)
 {
+   bool return_to_hs400 = false;
int err;
 
if (host-retune_now)
@@ -354,8 +355,24 @@ int mmc_retune(struct mmc_host *host)
 
host-doing_retune = 1;
 
+   if (host-ios.timing == MMC_TIMING_MMC_HS400) {
+   err = mmc_hs400_to_hs200(host-card);
+   if (err)
+   goto out;
+
+   return_to_hs400 = true;
+
+   if (host-ops-prepare_hs400_tuning)
+   host-ops-prepare_hs400_tuning(host, host-ios);
+   }
+
err = mmc_execute_tuning(host-card);
+   if (err)
+   goto out;
 
+   if (return_to_hs400)
+   err = mmc_hs200_to_hs400(host-card);
+out:
host-doing_retune = 0;
 
return err;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index daf9954..70a091d 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1092,6 +1092,94 @@ static int mmc_select_hs400(struct mmc_card *card)
return 0;
 }
 
+int mmc_hs200_to_hs400(struct mmc_card *card)
+{
+   return mmc_select_hs400(card);
+}
+
+/* Caller must hold re-tuning */
+static int mmc_switch_status(struct mmc_card *card)
+{
+   u32 status;
+   int err;
+
+   err = mmc_send_status(card, status);
+   if (err)
+   return err;
+
+   return mmc_switch_status_error(card-host, status);
+}
+
+int mmc_hs400_to_hs200(struct mmc_card *card)
+{
+   struct mmc_host *host = card-host;
+   bool send_status = true;
+   unsigned int max_dtr;
+   int err;
+
+   if (host-caps  MMC_CAP_WAIT_WHILE_BUSY)
+   send_status = false;
+
+   /* Reduce frequency to HS */
+   max_dtr = card-ext_csd.hs_max_dtr;
+   mmc_set_clock(host, max_dtr);
+
+   /* Switch HS400 to HS DDR */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+  EXT_CSD_TIMING_HS, card-ext_csd.generic_cmd6_time,
+  true, send_status, true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   /* Switch HS DDR to HS */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
+  EXT_CSD_BUS_WIDTH_8, card-ext_csd.generic_cmd6_time,
+  true, send_status, true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_HS);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   /* Switch HS to HS200 */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+  EXT_CSD_TIMING_HS200,
+  card-ext_csd.generic_cmd6_time, true, send_status,
+  true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   mmc_set_bus_speed(card);
+
+   return 0;
+
+out_err:
+   pr_err(%s: %s failed, error %d\n, mmc_hostname(card-host),
+  __func__, err);
+   return err;
+}
+
 /*
  * For device supporting HS200 mode, the following sequence
  * should be done before executing the tuning process.
-- 
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

[PATCH V6 01/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

Currently, there is core support for tuning during
initialization. There can also be a need to re-tune
periodically (e.g. sdhci) or to re-tune after the
host controller is powered off (e.g. after PM
runtime suspend / resume) or to re-tune in response
to CRC errors.

The main requirements for re-tuning are:
  - ability to enable / disable re-tuning
  - ability to flag that re-tuning is needed
  - ability to re-tune before any request
  - ability to hold off re-tuning if the card is busy
  - ability to hold off re-tuning if re-tuning is in
  progress
  - ability to run a re-tuning timer

To support those requirements 7 members are added to struct
mmc_host:

  unsigned int  can_retune:1;   /* re-tuning can be used */
  unsigned int  doing_retune:1; /* re-tuning in progress */
  unsigned int  retune_now:1;   /* do re-tuning at next req */
  int   need_retune;/* re-tuning is needed */
  int   hold_retune;/* hold off re-tuning */
  unsigned int  retune_period;  /* re-tuning period in secs */
  struct timer_list retune_timer;   /* for periodic re-tuning */

need_retune is an integer so it can be set without needing
synchronization. hold_retune is a integer to allow nesting.

Various simple functions are provided to set / clear those
variables.

Subsequent patches take those functions into use.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/host.c  | 68 
 drivers/mmc/core/host.h  |  6 +
 include/linux/mmc/host.h | 28 
 3 files changed, 102 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 8be0df7..e90e02f 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,73 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host 
*host)
 
 #endif
 
+void mmc_retune_enable(struct mmc_host *host)
+{
+   host-can_retune = 1;
+   if (host-retune_period)
+   mod_timer(host-retune_timer,
+ jiffies + host-retune_period * HZ);
+}
+
+void mmc_retune_disable(struct mmc_host *host)
+{
+   host-can_retune = 0;
+   del_timer_sync(host-retune_timer);
+   host-retune_now = 0;
+   host-need_retune = 0;
+}
+
+void mmc_retune_timer_stop(struct mmc_host *host)
+{
+   del_timer_sync(host-retune_timer);
+}
+EXPORT_SYMBOL(mmc_retune_timer_stop);
+
+void mmc_retune_hold(struct mmc_host *host)
+{
+   if (!host-hold_retune)
+   host-retune_now = 1;
+   host-hold_retune += 1;
+}
+
+void mmc_retune_release(struct mmc_host *host)
+{
+   if (host-hold_retune)
+   host-hold_retune -= 1;
+   else
+   WARN_ON(1);
+}
+
+int mmc_retune(struct mmc_host *host)
+{
+   int err;
+
+   if (host-retune_now)
+   host-retune_now = 0;
+   else
+   return 0;
+
+   if (!host-need_retune || host-doing_retune || !host-card)
+   return 0;
+
+   host-need_retune = 0;
+
+   host-doing_retune = 1;
+
+   err = mmc_execute_tuning(host-card);
+
+   host-doing_retune = 0;
+
+   return err;
+}
+
+static void mmc_retune_timer(unsigned long data)
+{
+   struct mmc_host *host = (struct mmc_host *)data;
+
+   mmc_retune_needed(host);
+}
+
 /**
  * mmc_of_parse() - parse host's device-tree node
  * @host: host whose node should be parsed.
@@ -504,6 +571,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
*dev)
 #ifdef CONFIG_PM
host-pm_notify.notifier_call = mmc_pm_notify;
 #endif
+   setup_timer(host-retune_timer, mmc_retune_timer, (unsigned long)host);
 
/*
 * By default, hosts do not support SGIO or large requests.
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index f2ab9e5..992bf53 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -15,5 +15,11 @@
 int mmc_register_host_class(void);
 void mmc_unregister_host_class(void);
 
+void mmc_retune_enable(struct mmc_host *host);
+void mmc_retune_disable(struct mmc_host *host);
+void mmc_retune_hold(struct mmc_host *host);
+void mmc_retune_release(struct mmc_host *host);
+int mmc_retune(struct mmc_host *host);
+
 #endif
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b5bedae..9c9e51b 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -12,6 +12,7 @@
 
 #include linux/leds.h
 #include linux/mutex.h
+#include linux/timer.h
 #include linux/sched.h
 #include linux/device.h
 #include linux/fault-inject.h
@@ -321,10 +322,18 @@ struct mmc_host {
 #ifdef CONFIG_MMC_DEBUG
unsigned intremoved:1;  /* host is being removed */
 #endif
+   unsigned intcan_retune:1;   /* re-tuning can be used */
+   unsigned intdoing_retune:1; /* re-tuning in progress */
+   unsigned intretune_now:1;   /* do re-tuning at next req

[PATCH V6 00/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

* changing the I/O state to match the
switch i.e. the new I/O state is the correct one to use
after a switch.

mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors
New patch

mmc: block: Check re-tuning in the recovery path
New patch

mmc: block: Retry data requests when re-tuning is needed
New patch

mmc: core: Don't print reset warning if reset is not supported
New patch


Adrian Hunter (15):
  mmc: host: Add facility to support re-tuning
  mmc: core: Enable / disable re-tuning
  mmc: core: Add support for re-tuning before each request
  mmc: core: Check re-tuning before retrying
  mmc: core: Hold re-tuning during switch commands
  mmc: core: Hold re-tuning during erase commands
  mmc: core: Hold re-tuning while bkops ongoing
  mmc: mmc: Hold re-tuning if the card is put to sleep
  mmc: core: Separate out the mmc_switch status check so it can be re-used
  mmc: core: Add support for HS400 re-tuning
  mmc: sdhci: Change to new way of doing re-tuning
  mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors
  mmc: block: Check re-tuning in the recovery path
  mmc: block: Retry errored data requests when re-tuning is needed
  mmc: core: Don't print reset warning if reset is not supported

 drivers/mmc/card/block.c   |  14 -
 drivers/mmc/card/queue.h   |   1 +
 drivers/mmc/core/core.c|  51 +++---
 drivers/mmc/core/core.h|   2 +
 drivers/mmc/core/host.c|  85 ++
 drivers/mmc/core/host.h|   6 +++
 drivers/mmc/core/mmc.c | 102 ++--
 drivers/mmc/core/mmc_ops.c |  44 ++--
 drivers/mmc/core/mmc_ops.h |   1 +
 drivers/mmc/core/sdio.c|   2 +
 drivers/mmc/host/sdhci.c   | 126 -
 drivers/mmc/host/sdhci.h   |   3 --
 include/linux/mmc/host.h   |  28 ++
 13 files changed, 330 insertions(+), 135 deletions(-)


Regards
Adrian

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 18:19, Ulf Hansson wrote:
 [...]
 
 1) Card removal/detect (hold/release?)

 Re-tuning will be done if needed before detect (because it is done before a
 request), which is necessary because detect can fail if tuning is needed.

 Re-tuning is done before a request. Requests aren't started if the card has
 been removed i.e. mmc_start_request() calls mmc_card_removed()

 If tuning is executed while a card is being removed, then the tuning will
 fail and the request will be errored out.

 So you are saying that we instead of relying on the CMD13 (for SD/MMC)
 to check whether the card is still alive, it's fine to trigger a
 re-tuning instead.

 (Oops forgot to answer this one, sorry)

 Yes, why not?


 I don't think that is an effective way to remove a card.

 Generally we know the card is removed from card-detect so no commands are
 sent in either case.
 
 Not sure what you mean here.

The sdhci driver won't issue commands if card-detect shows no card. It just
sets the error to -ENOMEDIUM.

 
 In case when the card is idle and the host sees a GPIO CD irq, it
 will trigger a detect work to run mmc_rescan().
 
 In this case, it's the responsibility of mmc_rescan() to find out if
 the card is being removed. It does so by invoking the
 bus_ops-detect() callback, which eventually will send a CMD13 for
 mmc/sd.
 
 In this scenario, I can't see why we want to allow re-tuning to happen
 in front of the CMD13 command.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

 


 Moreover, I find it quite unreasonable to say the check for an alive
 card, would fail because of that a re-tuning is needed. That would in
 principle mean that we never should be able to hold re-tune for any
 commands sequences.

 Re-tuning must work if the card is alive. CMD13 might get a CRC error if
 re-tuning is needed.
 
 And that then applies to all commands which we hold re-tuning for. So
 then we can't _ever_  hold re-tuning for any command, since we might
 get CRC errors.

Periodic re-tuning and re-tuning after pm-runtime-resume should ensure no
CRC errors while re-tuning is held.

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 16:57, Ulf Hansson wrote:
 [...]
 
 2) system PM (disable?)

 System pm does mmc_power_off() which calls mmc_set_initial_state()

 At System PM other commands will be sent to put the card into sleep
 state. For example CMD5. These commands are invoked prior
 mmc_power_off() is called.

 You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake 
 up.

 So if you want to wake-up from sleep, then you need to hold re-tuning, but
 that is not what is happening at the moment.
 
 So then we need to fix this.
 
 Also, it seems like disabling the re-tuning timer would also be the
 proper thing to do, thus we should rather do disable instead of
 hold/release.

I can add hold/release. The hold should be done before sleeping
and the release after waking up. In this case there is no wake-up, instead
there is power-off, so the release can be done then.

Disabling before CMD5 is not the right thing to do because we might want to
re-tune before issuing CMD5.

 


 In the SDIO case, mmc_power_off() might not even be called at all,
 since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.

 If the card is not going to be re-initialized then disabling is not 
 necessary.
 
 I don't want the re-tuning timer to be running, thus it seems like we
 should do disable. Right?

Re-tuning remains enabled while the card has a transfer mode that requires
it. Re-tuning is only done before a request so it doesn't need to be
disabled during suspend i.e. no requests implies no-retuning

I can add disabling the re-tuning timer, although the host does that too.

 



 3) runtime PM (disable?)

 If the card powers off then mmc_set_initial_state() will called.

 Again that's too late, since for example the CMD5 might have been sent
 before this.

 CMD5 is only used by _mmc_suspend()

 Yes if it were used elsewhere then re-tuning would have to be held, which is
 why I added a comment before mmc_sleep()

 /* If necessary, callers must hold re-tuning */
 static int mmc_sleep(struct mmc_host *host)

 
 I don't follow here. Why would we like to allow a re-tuning to be done
 in this part of the system PM phase? It doesn't make sense to me.

Because it is needed.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

 


 For anything else the card might be doing, the mmc_retune_hold() /
 mmc_retune_release() functions are used.

 4) reset (?)

 Reset goes through mmc_set_initial_state()

 In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
 during that period?

 If reset happens, then the next command will fail, whether it is re-tuning
 or CMD13, so no different.
 
 That depends on how each an every host has implemented their tuning mechanism.

No it doesn't. Tuning either succeeds or fails. When there is no card, if it
fails then the CMD13 is not needed, and if it succeeds then CMD13 will fail.

 
 CMD13 is more light weight, so I believe we should hold re-tune to
 make sure we do the CMD13 and fail quickly.

If re-tuning is needed and can be done, it is done. It doesn't need to be
more complicated than that.

 

 If reset doesn't happen, then it is no different to normal operations.


--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 17/04/15 11:56, Ulf Hansson wrote:
 On 17 April 2015 at 09:06, Adrian Hunter adrian.hun...@intel.com wrote:
 On 16/04/15 18:19, Ulf Hansson wrote:
 [...]

 1) Card removal/detect (hold/release?)

 Re-tuning will be done if needed before detect (because it is done 
 before a
 request), which is necessary because detect can fail if tuning is needed.

 Re-tuning is done before a request. Requests aren't started if the card 
 has
 been removed i.e. mmc_start_request() calls mmc_card_removed()

 If tuning is executed while a card is being removed, then the tuning will
 fail and the request will be errored out.

 So you are saying that we instead of relying on the CMD13 (for SD/MMC)
 to check whether the card is still alive, it's fine to trigger a
 re-tuning instead.

 (Oops forgot to answer this one, sorry)

 Yes, why not?


 I don't think that is an effective way to remove a card.

 Generally we know the card is removed from card-detect so no commands are
 sent in either case.

 Not sure what you mean here.

 The sdhci driver won't issue commands if card-detect shows no card. It just
 sets the error to -ENOMEDIUM.
 
 That's sdhci, but we have have a lot more drivers than sdhci to care about.
 


 In case when the card is idle and the host sees a GPIO CD irq, it
 will trigger a detect work to run mmc_rescan().

 In this case, it's the responsibility of mmc_rescan() to find out if
 the card is being removed. It does so by invoking the
 bus_ops-detect() callback, which eventually will send a CMD13 for
 mmc/sd.

 In this scenario, I can't see why we want to allow re-tuning to happen
 in front of the CMD13 command.

 If re-tuning is needed and can be done, it is done. It doesn't need to be
 more complicated than that.




 Moreover, I find it quite unreasonable to say the check for an alive
 card, would fail because of that a re-tuning is needed. That would in
 principle mean that we never should be able to hold re-tune for any
 commands sequences.

 Re-tuning must work if the card is alive. CMD13 might get a CRC error if
 re-tuning is needed.

 And that then applies to all commands which we hold re-tuning for. So
 then we can't _ever_  hold re-tuning for any command, since we might
 get CRC errors.

 Periodic re-tuning and re-tuning after pm-runtime-resume should ensure no
 CRC errors while re-tuning is held.

 
 When re-tune is being held, there's no guarantee the re-tune timer
 will not timeout. So that means every time you decide to hold re-tune
 for a command (or sequence of commands) you will risk getting CRC
 errors.
 
 The hole idea with the periodic re-tuning timer is to _minimize_ the
 probability of getting CRC errors, it's not to remove them completely,
 right!?

Yes I shouldn't have asserted there will be no CRC errors. As you say, there
could be. Currently there is no attempt to recover from such errors and
AFAIK there is also no indication that that has been a problem yet. In my
patches I have added recovery for mmc block data transfers. So that is at
least a step forward.

 
 So from that reasoning, I don't see why the mmc core shouldn't be
 holding/disabling re-tuning under those circumstances when it make
 sense. As for those I suggested.

To reduce the chance of CRC errors when re-tuning is held, we should hold
re-tuning only when absolutely necessary. Wouldn't that mean not in the
extra places you suggested?

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 01/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

On 16/04/15 10:24, Ulf Hansson wrote:
 [...]
 

 Using runtime pm for custom sleep states would seem to conflict with its 
 use
 by the power domain. For example ACPI will enumerate embedded SDIO function
 devices and link them to the ACPI power domain. Then ACPI will choose the
 lowest power state for runtime suspend.

 It isn't obvious how a driver that doesn't know its power domain should
 handle runtime pm, other than assuming it means power off.

 I am not sure what you mean by power off in this context. Is that

 I guess power off is the wrong term. They might have to assume they have
 lost device context.

 You should ask the power management mailing list what assumptions a device
 driver can make about the use of runtime pm. If you do, please cc me.

 the power of the actual SDIO card? I don't think so, but I may be
 wrong.

 I was talking about SDIO function devices.
 
 OK!
 


 To enumerate a SDIO card the mmc core first needs to apply power to
 it. At this point the PM domain isn't yet attached to the SDIO func
 device (the device doesn't even exist yet) and thus it can't be used
 to provide power the card. Right?

 In the ACPI case the SDIO function device ACPI nodes are children of the
 host controller ACPI node. One possibility is to have the host controller
 driver power on its children.




 So, from mmc core perspective we should be able to get notifications
 through runtime PM callbacks (from mmc or sdio bus) to understand
 whether we need to hold of re-tune.

 That doesn't match the requirement. Re-tuning needs to be held for the
 wake-up command, not while asleep.


 Why should we ever want to re-tune if the card is in a sleep state?
 Isn't it better to postpone that until it wakes up?

 That is what I was saying i.e. hold re-tuning for the wake-up command.
 So if re-tuning is needed it is done after the wake-up command.

 
 Okay. So that means we might be able to use runtime PM from the SDIO
 func driver to notify the mmc core through the mmc or sdio bus's
 runtime PM callback to understand whether we should hold/allow
 re-tune.

I don't see how that would work. Only the SDIO function driver knows that
tuning can't be done before it's wake-up command. So you still need an API.

 
 That do requires some additional re-work, both from mmc core point of
 view and from SDIO func drivers point of view. I will have a look in
 more detail to see if this really is viable.
 
 The goal from my side is to prevent us from exporting unnecessary APIs
 and thus mmc_retune_hold() and mmc_retune_release() could also be
 internal functions of the mmc core.

That is fine if you have an alternative, but we can't wait forever.

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 08/15] mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep

[Adrian Hunter]

On 16/04/15 12:01, Ulf Hansson wrote:
 On 14 April 2015 at 15:12, Adrian Hunter adrian.hun...@intel.com wrote:
 Currently mmc sleep is only used before power off and
 is not paired with waking up.  If that ever changed,
 re-tuning might need to be held, so add a comment for that.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/mmc.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
 index c84131e..53a9cb3 100644
 --- a/drivers/mmc/core/mmc.c
 +++ b/drivers/mmc/core/mmc.c
 @@ -1504,6 +1504,7 @@ static int mmc_can_sleep(struct mmc_card *card)
 return (card  card-ext_csd.rev = 3);
  }

 +/* If necessary, callers must hold re-tuning */
 
 Instead of adding a comment, let's fix what needs to be fixed.
 
 I believe the proper thing would be to disable re-tuning when the card
 is about to enter system PM sleep state. So somewhere early in
 _mmc_suspend() we should disable re-tune. Likewise for SD and SDIO.

_mmc_suspend() calls mmc_power_off() which calls  mmc_set_initial_state()
which disables re-tuning. Likewise for SD and SDIO.

So it is already done?

 
  static int mmc_sleep(struct mmc_host *host)
  {
 struct mmc_command cmd = {0};
 --
 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
 
 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 V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 11:57, Ulf Hansson wrote:
 On 14 April 2015 at 15:12, Adrian Hunter adrian.hun...@intel.com wrote:
 Enable re-tuning when tuning is executed and
 disable re-tuning when card is no longer initialized.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/core.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index c296bc0..dd43f9b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)

 if (err)
 pr_err(%s: tuning execution failed\n, mmc_hostname(host));
 +   else
 +   mmc_retune_enable(host);

 return err;
  }
 @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
 int width)
   */
  void mmc_set_initial_state(struct mmc_host *host)
  {
 +   mmc_retune_disable(host);
 +
 if (mmc_host_is_spi(host))
 host-ios.chip_select = MMC_CS_HIGH;
 else
 --
 1.9.1
 
 I don't think you have fully considered these cases for the mmc/sd/sdio cards.

Thanks for look at this, but I don't see a problem - see below.

 
 1) Card removal/detect (hold/release?)

Re-tuning will be done if needed before detect (because it is done before a
request), which is necessary because detect can fail if tuning is needed.

Re-tuning is done before a request. Requests aren't started if the card has
been removed i.e. mmc_start_request() calls mmc_card_removed()

If tuning is executed while a card is being removed, then the tuning will
fail and the request will be errored out.

 2) system PM (disable?)

System pm does mmc_power_off() which calls mmc_set_initial_state()

 3) runtime PM (disable?)

If the card powers off then mmc_set_initial_state() will called.

For anything else the card might be doing, the mmc_retune_hold() /
mmc_retune_release() functions are used.

 4) reset (?)

Reset goes through mmc_set_initial_state()


--
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 V6 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Adrian Hunter]

Hold re-tuning during bkops to prevent
it from conflicting with the busy state.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---



Changes in V6:

Hold re-tuning always and release only if
bkops is not ongoing.



 drivers/mmc/core/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dbd7a77..7aae0ff 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -318,12 +318,15 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
use_busy_signal = false;
}
 
+   mmc_retune_hold(card-host);
+
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_BKOPS_START, 1, timeout,
use_busy_signal, true, false);
if (err) {
pr_warn(%s: Error %d starting bkops\n,
mmc_hostname(card-host), err);
+   mmc_retune_release(card-host);
goto out;
}
 
@@ -334,6 +337,8 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
 */
if (!use_busy_signal)
mmc_card_set_doing_bkops(card);
+   else
+   mmc_retune_release(card-host);
 out:
mmc_release_host(card-host);
 }
@@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
 */
if (!err || (err == -EINVAL)) {
mmc_card_clr_doing_bkops(card);
+   mmc_retune_release(card-host);
err = 0;
}
 
-- 
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 V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 15:00, Ulf Hansson wrote:
 On 16 April 2015 at 11:26, Adrian Hunter adrian.hun...@intel.com wrote:
 On 16/04/15 11:57, Ulf Hansson wrote:
 On 14 April 2015 at 15:12, Adrian Hunter adrian.hun...@intel.com wrote:
 Enable re-tuning when tuning is executed and
 disable re-tuning when card is no longer initialized.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/core.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index c296bc0..dd43f9b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)

 if (err)
 pr_err(%s: tuning execution failed\n, 
 mmc_hostname(host));
 +   else
 +   mmc_retune_enable(host);

 return err;
  }
 @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, 
 unsigned int width)
   */
  void mmc_set_initial_state(struct mmc_host *host)
  {
 +   mmc_retune_disable(host);
 +
 if (mmc_host_is_spi(host))
 host-ios.chip_select = MMC_CS_HIGH;
 else
 --
 1.9.1

 I don't think you have fully considered these cases for the mmc/sd/sdio 
 cards.

 Thanks for look at this, but I don't see a problem - see below.


 1) Card removal/detect (hold/release?)

 Re-tuning will be done if needed before detect (because it is done before a
 request), which is necessary because detect can fail if tuning is needed.

 Re-tuning is done before a request. Requests aren't started if the card has
 been removed i.e. mmc_start_request() calls mmc_card_removed()

 If tuning is executed while a card is being removed, then the tuning will
 fail and the request will be errored out.
 
 So you are saying that we instead of relying on the CMD13 (for SD/MMC)
 to check whether the card is still alive, it's fine to trigger a
 re-tuning instead.
 
 I don't think that is an effective way to remove a card.
 
 Moreover, I find it quite unreasonable to say the check for an alive
 card, would fail because of that a re-tuning is needed. That would in
 principle mean that we never should be able to hold re-tune for any
 commands sequences.
 

 2) system PM (disable?)

 System pm does mmc_power_off() which calls mmc_set_initial_state()
 
 At System PM other commands will be sent to put the card into sleep
 state. For example CMD5. These commands are invoked prior
 mmc_power_off() is called.

You can't do *any* commands after CMD5 except CMD0 or another CMD5 to wake up.

So if you want to wake-up from sleep, then you need to hold re-tuning, but
that is not what is happening at the moment.

 
 In the SDIO case, mmc_power_off() might not even be called at all,
 since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.

If the card is not going to be re-initialized then disabling is not necessary.

 

 3) runtime PM (disable?)

 If the card powers off then mmc_set_initial_state() will called.
 
 Again that's too late, since for example the CMD5 might have been sent
 before this.

CMD5 is only used by _mmc_suspend()

Yes if it were used elsewhere then re-tuning would have to be held, which is
why I added a comment before mmc_sleep()

/* If necessary, callers must hold re-tuning */
static int mmc_sleep(struct mmc_host *host)

 

 For anything else the card might be doing, the mmc_retune_hold() /
 mmc_retune_release() functions are used.

 4) reset (?)

 Reset goes through mmc_set_initial_state()
 
 In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
 during that period?

If reset happens, then the next command will fail, whether it is re-tuning
or CMD13, so no different.

If reset doesn't happen, then it is no different to normal operations.

--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

On 16/04/15 15:00, Ulf Hansson wrote:
 On 16 April 2015 at 11:26, Adrian Hunter adrian.hun...@intel.com wrote:
 On 16/04/15 11:57, Ulf Hansson wrote:
 On 14 April 2015 at 15:12, Adrian Hunter adrian.hun...@intel.com wrote:
 Enable re-tuning when tuning is executed and
 disable re-tuning when card is no longer initialized.

 Signed-off-by: Adrian Hunter adrian.hun...@intel.com
 ---
  drivers/mmc/core/core.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index c296bc0..dd43f9b 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)

 if (err)
 pr_err(%s: tuning execution failed\n, 
 mmc_hostname(host));
 +   else
 +   mmc_retune_enable(host);

 return err;
  }
 @@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, 
 unsigned int width)
   */
  void mmc_set_initial_state(struct mmc_host *host)
  {
 +   mmc_retune_disable(host);
 +
 if (mmc_host_is_spi(host))
 host-ios.chip_select = MMC_CS_HIGH;
 else
 --
 1.9.1

 I don't think you have fully considered these cases for the mmc/sd/sdio 
 cards.

 Thanks for look at this, but I don't see a problem - see below.


 1) Card removal/detect (hold/release?)

 Re-tuning will be done if needed before detect (because it is done before a
 request), which is necessary because detect can fail if tuning is needed.

 Re-tuning is done before a request. Requests aren't started if the card has
 been removed i.e. mmc_start_request() calls mmc_card_removed()

 If tuning is executed while a card is being removed, then the tuning will
 fail and the request will be errored out.
 
 So you are saying that we instead of relying on the CMD13 (for SD/MMC)
 to check whether the card is still alive, it's fine to trigger a
 re-tuning instead.

(Oops forgot to answer this one, sorry)

Yes, why not?

 
 I don't think that is an effective way to remove a card.

Generally we know the card is removed from card-detect so no commands are
sent in either case.

 
 Moreover, I find it quite unreasonable to say the check for an alive
 card, would fail because of that a re-tuning is needed. That would in
 principle mean that we never should be able to hold re-tune for any
 commands sequences.

Re-tuning must work if the card is alive. CMD13 might get a CRC error if
re-tuning is needed.

 

 2) system PM (disable?)

 System pm does mmc_power_off() which calls mmc_set_initial_state()
 
 At System PM other commands will be sent to put the card into sleep
 state. For example CMD5. These commands are invoked prior
 mmc_power_off() is called.
 
 In the SDIO case, mmc_power_off() might not even be called at all,
 since SDIO func drivers might have enabled MMC_PM_KEEP_POWER.
 

 3) runtime PM (disable?)

 If the card powers off then mmc_set_initial_state() will called.
 
 Again that's too late, since for example the CMD5 might have been sent
 before this.
 

 For anything else the card might be doing, the mmc_retune_hold() /
 mmc_retune_release() functions are used.

 4) reset (?)

 Reset goes through mmc_set_initial_state()
 
 In the mmc case, CMD13 is sent prior that. Shouldn't we hold re-tune
 during that period?
 


 
 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 V4 01/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

On 13/04/15 15:07, Ulf Hansson wrote:
 On 8 April 2015 at 09:42, Adrian Hunter adrian.hun...@intel.com wrote:
 On 02/04/15 17:00, Ulf Hansson wrote:
 [...]

 Also SDIO cards can have a custom sleep state where tuning won't 
 work.



 Our SDIO wifi device has such a state and it can only come out of it
 upon
 CMD52 write or CMD14 (ExitSleep).


 So how will the mmc core know about these states? I guess we will
 require SDIO func drivers to deal with enable/disable or hold/release
 of re-tuning then?


 This is actually why in the past we tried to only kick off retuning
 before a
 request that requires use of data line(s) so mmc core (or sdhci) 
 would not
 do re-tuning when SDIO func used CMD52 to wakeup the device. I never 
 tried
 CMD14 approach and not even sure from which spec it comes (maybe 
 eMMC).

 That's an interesting idea. It would eliminate the need for SDIO func
 drivers to care about holding/releasing re-tuning.

 Would be nice to hear about Adrian's thoughts around this as well.

 I don't see how it would work because re-tuning is needed after the host
 controller runtime resumes. i.e. once the SDIO wifi card stops being 
 active
 the host controller will runtime suspend.

 Why do we always need to re-tune from this phase?

 What Arend points out is that we could delay the re-tune until we
 know there is a DATA request. Wouldn't that work for SDHCI as well?

 You beat me to it, but that is indeed what I meant.

 The CMD line needs tuning too, so delaying tuning on every command will
 cause errors. It is better to hold tuning for the specific command used to
 wake-up.

 Hmm.

 This touches the discussion where Neil Brown also was involved, around
 how to handle idle operations for SDIO.

 How does SDIO func drivers detects request inactivity, which
 triggers them to send its device to sleep mode? That answer should
 be runtime PM.

 Using runtime pm for custom sleep states would seem to conflict with its use
 by the power domain. For example ACPI will enumerate embedded SDIO function
 devices and link them to the ACPI power domain. Then ACPI will choose the
 lowest power state for runtime suspend.

 It isn't obvious how a driver that doesn't know its power domain should
 handle runtime pm, other than assuming it means power off.
 
 I am not sure what you mean by power off in this context. Is that

I guess power off is the wrong term. They might have to assume they have
lost device context.

You should ask the power management mailing list what assumptions a device
driver can make about the use of runtime pm. If you do, please cc me.

 the power of the actual SDIO card? I don't think so, but I may be
 wrong.

I was talking about SDIO function devices.

 
 To enumerate a SDIO card the mmc core first needs to apply power to
 it. At this point the PM domain isn't yet attached to the SDIO func
 device (the device doesn't even exist yet) and thus it can't be used
 to provide power the card. Right?

In the ACPI case the SDIO function device ACPI nodes are children of the
host controller ACPI node. One possibility is to have the host controller
driver power on its children.

 


 So, from mmc core perspective we should be able to get notifications
 through runtime PM callbacks (from mmc or sdio bus) to understand
 whether we need to hold of re-tune.

 That doesn't match the requirement. Re-tuning needs to be held for the
 wake-up command, not while asleep.

 
 Why should we ever want to re-tune if the card is in a sleep state?
 Isn't it better to postpone that until it wakes up?

That is what I was saying i.e. hold re-tuning for the wake-up command.
So if re-tuning is needed it is done after the wake-up command.


--
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 V5 03/15] mmc: core: Add support for re-tuning before each request

[Adrian Hunter]

At the start of each request, re-tune if needed and
then hold off re-tuning again until the request is done.

Note that though there is one function that starts
requests (mmc_start_request) there are two that wait for
the request to be done (mmc_wait_for_req_done and
mmc_wait_for_data_req_done).  Also note that
mmc_wait_for_data_req_done can return even when the
request is not done (which allows the block driver
to prepare a newly arrived request while still
waiting for the previous request).

This patch ensures re-tuning is held for the duration
of a request.  Subsequent patches will also hold
re-tuning at other times when it might cause a
conflict.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dd43f9b..a9936cb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -186,12 +186,29 @@ void mmc_request_done(struct mmc_host *host, struct 
mmc_request *mrq)
 
 EXPORT_SYMBOL(mmc_request_done);
 
+static void __mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
+{
+   int err;
+
+   /* Assumes host controller has been runtime resumed by mmc_claim_host */
+   err = mmc_retune(host);
+   if (err) {
+   mrq-cmd-error = err;
+   mmc_request_done(host, mrq);
+   return;
+   }
+
+   host-ops-request(host, mrq);
+}
+
 static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 {
 #ifdef CONFIG_MMC_DEBUG
unsigned int i, sz;
struct scatterlist *sg;
 #endif
+   mmc_retune_hold(host);
+
if (mmc_card_removed(host-card))
return -ENOMEDIUM;
 
@@ -252,7 +269,7 @@ static int mmc_start_request(struct mmc_host *host, struct 
mmc_request *mrq)
}
mmc_host_clk_hold(host);
led_trigger_event(host-led, LED_FULL);
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
 
return 0;
 }
@@ -422,17 +439,16 @@ static int mmc_wait_for_data_req_done(struct mmc_host 
*host,
cmd-opcode, cmd-error);
cmd-retries--;
cmd-error = 0;
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
continue; /* wait for done/new event again */
}
} else if (context_info-is_new_req) {
context_info-is_new_req = false;
-   if (!next_req) {
-   err = MMC_BLK_NEW_REQUEST;
-   break; /* return err */
-   }
+   if (!next_req)
+   return MMC_BLK_NEW_REQUEST;
}
}
+   mmc_retune_release(host);
return err;
 }
 
@@ -471,8 +487,10 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
 mmc_hostname(host), cmd-opcode, cmd-error);
cmd-retries--;
cmd-error = 0;
-   host-ops-request(host, mrq);
+   __mmc_start_request(host, mrq);
}
+
+   mmc_retune_release(host);
 }
 
 /**
-- 
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

[PATCH V5 06/15] mmc: core: Hold re-tuning during erase commands

[Adrian Hunter]

Hold re-tuning during erase commands to prevent
it from conflicting with the sequence of commands.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 08bf7e3..dbd7a77 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1995,6 +1995,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned 
int from,
unsigned long timeout;
int err;
 
+   mmc_retune_hold(card-host);
+
/*
 * qty is used to calculate the erase timeout which depends on how many
 * erase groups (or allocation units in SD terminology) are affected.
@@ -2098,6 +2100,7 @@ static int mmc_do_erase(struct mmc_card *card, unsigned 
int from,
} while (!(cmd.resp[0]  R1_READY_FOR_DATA) ||
 (R1_CURRENT_STATE(cmd.resp[0]) == R1_STATE_PRG));
 out:
+   mmc_retune_release(card-host);
return err;
 }
 
-- 
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

[PATCH V5 05/15] mmc: core: Hold re-tuning during switch commands

[Adrian Hunter]

Hold re-tuning during switch commands to prevent
it from conflicting with the busy state or the CMD13
verification.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/mmc_ops.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0ea042d..f67b0fd 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -474,6 +474,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
u32 status = 0;
bool use_r1b_resp = use_busy_signal;
 
+   mmc_retune_hold(host);
+
/*
 * If the cmd timeout and the max_busy_timeout of the host are both
 * specified, let's validate them. A failure means we need to prevent
@@ -506,11 +508,11 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
 
err = mmc_wait_for_cmd(host, cmd, MMC_CMD_RETRIES);
if (err)
-   return err;
+   goto out;
 
/* No need to check card status in case of unblocking command */
if (!use_busy_signal)
-   return 0;
+   goto out;
 
/*
 * CRC errors shall only be ignored in cases were CMD13 is used to poll
@@ -529,7 +531,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
if (send_status) {
err = __mmc_send_status(card, status, ignore_crc);
if (err)
-   return err;
+   goto out;
}
if ((host-caps  MMC_CAP_WAIT_WHILE_BUSY)  use_r1b_resp)
break;
@@ -543,29 +545,36 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
 */
if (!send_status) {
mmc_delay(timeout_ms);
-   return 0;
+   goto out;
}
 
/* Timeout if the device never leaves the program state. */
if (time_after(jiffies, timeout)) {
pr_err(%s: Card stuck in programming state! %s\n,
mmc_hostname(host), __func__);
-   return -ETIMEDOUT;
+   err = -ETIMEDOUT;
+   goto out;
}
} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
if (mmc_host_is_spi(host)) {
-   if (status  R1_SPI_ILLEGAL_COMMAND)
-   return -EBADMSG;
+   if (status  R1_SPI_ILLEGAL_COMMAND) {
+   err = -EBADMSG;
+   goto out;
+   }
} else {
if (status  0xFDFFA000)
pr_warn(%s: unexpected status %#x after switch\n,
mmc_hostname(host), status);
-   if (status  R1_SWITCH_ERROR)
-   return -EBADMSG;
+   if (status  R1_SWITCH_ERROR) {
+   err = -EBADMSG;
+   goto out;
+   }
}
+out:
+   mmc_retune_release(host);
 
-   return 0;
+   return err;
 }
 EXPORT_SYMBOL_GPL(__mmc_switch);
 
-- 
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

[PATCH V5 04/15] mmc: core: Check re-tuning before retrying

[Adrian Hunter]

Possibly a command is failing because re-tuning is needed.
Use mmc_retune_recheck() to check re-tuning. At that point
re-tuning is held, at least by the request, so
mmc_retune_recheck() flags host-retune_now if the hold
count is 1.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a9936cb..08bf7e3 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -434,6 +434,7 @@ static int mmc_wait_for_data_req_done(struct mmc_host *host,
host-areq);
break; /* return err */
} else {
+   mmc_retune_recheck(host);
pr_info(%s: req failed (CMD%u): %d, 
retrying...\n,
mmc_hostname(host),
cmd-opcode, cmd-error);
@@ -483,6 +484,8 @@ static void mmc_wait_for_req_done(struct mmc_host *host,
mmc_card_removed(host-card))
break;
 
+   mmc_retune_recheck(host);
+
pr_debug(%s: req failed (CMD%u): %d, retrying...\n,
 mmc_hostname(host), cmd-opcode, cmd-error);
cmd-retries--;
-- 
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

[PATCH V5 07/15] mmc: core: Hold re-tuning while bkops ongoing

[Adrian Hunter]

Hold re-tuning during bkops to prevent
it from conflicting with the busy state.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dbd7a77..4a42174 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -316,6 +316,8 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
} else {
timeout = 0;
use_busy_signal = false;
+   /* Hold re-tuning for ongoing bkops */
+   mmc_retune_hold(card-host);
}
 
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
@@ -324,6 +326,9 @@ void mmc_start_bkops(struct mmc_card *card, bool 
from_exception)
if (err) {
pr_warn(%s: Error %d starting bkops\n,
mmc_hostname(card-host), err);
+   /* bkops not ongoing, so release re-tuning */
+   if (!use_busy_signal)
+   mmc_retune_release(card-host);
goto out;
}
 
@@ -749,6 +754,7 @@ int mmc_stop_bkops(struct mmc_card *card)
 */
if (!err || (err == -EINVAL)) {
mmc_card_clr_doing_bkops(card);
+   mmc_retune_release(card-host);
err = 0;
}
 
-- 
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

[PATCH V5 00/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

Hi

Here is V5 of some patches to move re-tuning support
out of sdhci and into the core, and add support for HS400
re-tuning.

Currently sdhci does re-tuning transparently by
calling sdhci_execute_tuning() from its -request()
function.

The problem with HS400 re-tuning is that it must be
done in HS200 mode. That means using switch commands
and making ios changes. That means it potentially
conflicts with other command sequences. The new
re-tuning support accomodates that.


Changes in V5:

mmc: host: Add facility to support re-tuning
Make mmc_retune_enable() / mmc_retune_disable()
only called from core.

mmc: core: Enable / disable re-tuning
Replaces mmc: core: Disable re-tuning when card is no longer initialized
Enables re-tuning when tuning is executed

mmc: sdhci: Change to new way of doing re-tuning
Set host-retune_period instead of enabling re-tuning.

Changes in V4:

These patches now depend on Ulf's patch:
mmc: core: Enable runtime PM management of host devices

mmc: host: Add facility to support re-tuning

Assume mmc_claim_host() runtime resumes the host
controller so there are no races with runtime pm.
Consequently remove now un-needed re-tuning host
operations.

mmc: core: Add support for re-tuning before each request

Call mmc_retune() prior to -request()

mmc: sdhci: Change to new way of doing re-tuning

Updated to reflect the changes above.

Changes in V3:

mmc: host: Add facility to support re-tuning

Add host-retune_now flag so re-tuning can be
started in the host drivers -request()
function to avoid racing with Runtime PM.

Add host operations to let the host know when
re-tuning is held, for eaxmple, to enable
synchronization with runtime suspend / resume.

Ensure functions are exported.

mmc: core: Add support for re-tuning before each request
Updated to reflect the change above.

mmc: core: Check re-tuning before retrying
Updated to reflect the change above.

mmc: core: Hold re-tuning during switch commands
Updated to reflect the change above.

mmc: core: Hold re-tuning during erase commands
Updated to reflect the change above.

mmc: core: Hold re-tuning while bkops ongoing
Updated to reflect the change above.

mmc: core: Add support for HS400 re-tuning
Updated and as already sent separately as V3:
Remember to mmc_set_bus_speed(card) in mmc_hs400_to_hs200()

mmc: sdhci: Change to new way of doing re-tuning
Call mmc_retune() from -request() function to
avoid racing with Runtime PM. And implement
hold_tuning / release_tuning operations to prevent
runtime suspend while re-tuning is held.

mmc: block: Retry errored data requests when re-tuning is needed
Updated and as already sent separately as V3:
Only retry when there is an error

Changes in V2:

Added support to the block driver for re-tuning
and retrying after a CRC error. The host driver
is left to decide when an error indicates re-tuning
is needed. The block driver will retry a data request
once if re-tuning is flagged as needed.

SDIO drivers need not be aware of re-tuning because
retrying will anyway cause re-tuning when re-tuning
is flagged as needed. Nevertheless SDIO drivers could
use the need_retune flag to instigate a retry when
otherwise they might not have.

mmc: core: Simplify by adding mmc_execute_tuning()
Dropped because it has been applied

mmc: host: Add facility to support re-tuning
Renamed mmc_retune_retry() to mmc_retune_recheck()
to better reflect what it does.

mmc: core: Move mmc_card_removed() into mmc_start_request()
Dropped because it has been applied

mmc: core: Add support for re-tuning before each request
Fixed un-balanced re-tune hold / release

mmc: sdhci: Always init buf_ready_int
Dropped because it has been applied

mmc: core: Separate out the mmc_switch status check so it can be re-used
New patch

mmc: core: Add support for HS400 re-tuning
It was found that that the original code was not reliable
after a CRC error. The problem was that the CMD13 after a
switch was faiing. So the code was changed to check the
switch status *after* changing the I/O state to match the
switch i.e. the new I/O state is the correct one to use
after a switch.

mmc: sdhci: Flag re-tuning is needed on CRC or End-Bit errors
New patch

mmc: block: Check re-tuning in the recovery path
New patch

mmc: block: Retry data requests when re-tuning is needed
New patch

mmc: core: Don't print reset warning if reset is not supported
New patch


Adrian Hunter (15):
  mmc: host: Add facility to support re-tuning

[PATCH V5 01/15] mmc: host: Add facility to support re-tuning

[Adrian Hunter]

Currently, there is core support for tuning during
initialization. There can also be a need to re-tune
periodically (e.g. sdhci) or to re-tune after the
host controller is powered off (e.g. after PM
runtime suspend / resume) or to re-tune in response
to CRC errors.

The main requirements for re-tuning are:
  - ability to enable / disable re-tuning
  - ability to flag that re-tuning is needed
  - ability to re-tune before any request
  - ability to hold off re-tuning if the card is busy
  - ability to hold off re-tuning if re-tuning is in
  progress
  - ability to run a re-tuning timer

To support those requirements 7 members are added to struct
mmc_host:

  unsigned int  can_retune:1;   /* re-tuning can be used */
  unsigned int  doing_retune:1; /* re-tuning in progress */
  unsigned int  retune_now:1;   /* do re-tuning at next req */
  int   need_retune;/* re-tuning is needed */
  int   hold_retune;/* hold off re-tuning */
  unsigned int  retune_period;  /* re-tuning period in secs */
  struct timer_list retune_timer;   /* for periodic re-tuning */

need_retune is an integer so it can be set without needing
synchronization. hold_retune is a integer to allow nesting.

Various simple functions are provided to set / clear those
variables.

Subsequent patches take those functions into use.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/host.c  | 71 
 include/linux/mmc/host.h | 33 ++
 2 files changed, 104 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 8be0df7..271986c 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -301,6 +301,76 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host 
*host)
 
 #endif
 
+void mmc_retune_enable(struct mmc_host *host)
+{
+   host-can_retune = 1;
+   if (host-retune_period)
+   mod_timer(host-retune_timer,
+ jiffies + host-retune_period * HZ);
+}
+
+void mmc_retune_disable(struct mmc_host *host)
+{
+   host-can_retune = 0;
+   del_timer_sync(host-retune_timer);
+   host-retune_now = 0;
+   host-need_retune = 0;
+}
+
+void mmc_retune_timer_stop(struct mmc_host *host)
+{
+   del_timer_sync(host-retune_timer);
+}
+EXPORT_SYMBOL(mmc_retune_timer_stop);
+
+void mmc_retune_hold(struct mmc_host *host)
+{
+   if (!host-hold_retune)
+   host-retune_now = 1;
+   host-hold_retune += 1;
+}
+EXPORT_SYMBOL(mmc_retune_hold);
+
+void mmc_retune_release(struct mmc_host *host)
+{
+   if (host-hold_retune)
+   host-hold_retune -= 1;
+   else
+   WARN_ON(1);
+}
+EXPORT_SYMBOL(mmc_retune_release);
+
+int mmc_retune(struct mmc_host *host)
+{
+   int err;
+
+   if (host-retune_now)
+   host-retune_now = 0;
+   else
+   return 0;
+
+   if (!host-need_retune || host-doing_retune || !host-card)
+   return 0;
+
+   host-need_retune = 0;
+
+   host-doing_retune = 1;
+
+   err = mmc_execute_tuning(host-card);
+
+   host-doing_retune = 0;
+
+   return err;
+}
+EXPORT_SYMBOL(mmc_retune);
+
+static void mmc_retune_timer(unsigned long data)
+{
+   struct mmc_host *host = (struct mmc_host *)data;
+
+   mmc_retune_needed(host);
+}
+
 /**
  * mmc_of_parse() - parse host's device-tree node
  * @host: host whose node should be parsed.
@@ -504,6 +574,7 @@ struct mmc_host *mmc_alloc_host(int extra, struct device 
*dev)
 #ifdef CONFIG_PM
host-pm_notify.notifier_call = mmc_pm_notify;
 #endif
+   setup_timer(host-retune_timer, mmc_retune_timer, (unsigned long)host);
 
/*
 * By default, hosts do not support SGIO or large requests.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index b5bedae..e9f693e 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -12,6 +12,7 @@
 
 #include linux/leds.h
 #include linux/mutex.h
+#include linux/timer.h
 #include linux/sched.h
 #include linux/device.h
 #include linux/fault-inject.h
@@ -321,10 +322,18 @@ struct mmc_host {
 #ifdef CONFIG_MMC_DEBUG
unsigned intremoved:1;  /* host is being removed */
 #endif
+   unsigned intcan_retune:1;   /* re-tuning can be used */
+   unsigned intdoing_retune:1; /* re-tuning in progress */
+   unsigned intretune_now:1;   /* do re-tuning at next req */
 
int rescan_disable; /* disable card detection */
int rescan_entered; /* used with nonremovable 
devices */
 
+   int need_retune;/* re-tuning is needed */
+   int hold_retune;/* hold off re-tuning */
+   unsigned intretune_period;  /* re-tuning period in secs */
+   struct timer_list   retune_timer

[PATCH V5 02/15] mmc: core: Enable / disable re-tuning

[Adrian Hunter]

Enable re-tuning when tuning is executed and
disable re-tuning when card is no longer initialized.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c296bc0..dd43f9b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1109,6 +1109,8 @@ int mmc_execute_tuning(struct mmc_card *card)
 
if (err)
pr_err(%s: tuning execution failed\n, mmc_hostname(host));
+   else
+   mmc_retune_enable(host);
 
return err;
 }
@@ -1140,6 +1142,8 @@ void mmc_set_bus_width(struct mmc_host *host, unsigned 
int width)
  */
 void mmc_set_initial_state(struct mmc_host *host)
 {
+   mmc_retune_disable(host);
+
if (mmc_host_is_spi(host))
host-ios.chip_select = MMC_CS_HIGH;
else
-- 
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

[PATCH V5 14/15] mmc: block: Retry errored data requests when re-tuning is needed

[Adrian Hunter]

Retry errored data requests when re-tuning is needed and
add a flag to struct mmc_blk_request so that the retry
is only done once.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/card/block.c | 11 ++-
 drivers/mmc/card/queue.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 293e938..3baf7ea 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1195,6 +1195,7 @@ static int mmc_blk_err_check(struct mmc_card *card,
mmc_active);
struct mmc_blk_request *brq = mq_mrq-brq;
struct request *req = mq_mrq-req;
+   int need_retune = card-host-need_retune;
int ecc_err = 0, gen_err = 0;
 
/*
@@ -1262,6 +1263,12 @@ static int mmc_blk_err_check(struct mmc_card *card,
}
 
if (brq-data.error) {
+   if (need_retune  !brq-retune_retry_done) {
+   pr_info(%s: retrying because a re-tune was needed\n,
+   req-rq_disk-disk_name);
+   brq-retune_retry_done = 1;
+   return MMC_BLK_RETRY;
+   }
pr_err(%s: error %d transferring data, sector %u, nr %u, cmd 
response %#x, card status %#x\n,
   req-rq_disk-disk_name, brq-data.error,
   (unsigned)blk_rq_pos(req),
@@ -1821,7 +1828,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
struct mmc_blk_data *md = mq-data;
struct mmc_card *card = md-queue.card;
struct mmc_blk_request *brq = mq-mqrq_cur-brq;
-   int ret = 1, disable_multi = 0, retry = 0, type;
+   int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
enum mmc_blk_status status;
struct mmc_queue_req *mq_rq;
struct request *req = rqc;
@@ -1905,6 +1912,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
break;
goto cmd_abort;
case MMC_BLK_RETRY:
+   retune_retry_done = brq-retune_retry_done;
if (retry++  5)
break;
/* Fall through */
@@ -1967,6 +1975,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, 
struct request *rqc)
mmc_start_req(card-host,
mq_rq-mmc_active, NULL);
}
+   mq_rq-brq.retune_retry_done = retune_retry_done;
}
} while (ret);
 
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 5752d50..7e27915 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -12,6 +12,7 @@ struct mmc_blk_request {
struct mmc_command  cmd;
struct mmc_command  stop;
struct mmc_data data;
+   int retune_retry_done;
 };
 
 enum mmc_packed_type {
-- 
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

[PATCH V5 08/15] mmc: mmc: Comment that callers need to hold re-tuning if the card is put to sleep

[Adrian Hunter]

Currently mmc sleep is only used before power off and
is not paired with waking up.  If that ever changed,
re-tuning might need to be held, so add a comment for that.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c84131e..53a9cb3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1504,6 +1504,7 @@ static int mmc_can_sleep(struct mmc_card *card)
return (card  card-ext_csd.rev = 3);
 }
 
+/* If necessary, callers must hold re-tuning */
 static int mmc_sleep(struct mmc_host *host)
 {
struct mmc_command cmd = {0};
-- 
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

[PATCH V5 10/15] mmc: core: Add support for HS400 re-tuning

[Adrian Hunter]

HS400 re-tuning must be done in HS200 mode. Add
the ability to switch from HS400 mode to HS200
mode before re-tuning and switch back to HS400
after re-tuning.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/core.h |  2 ++
 drivers/mmc/core/host.c | 17 ++
 drivers/mmc/core/mmc.c  | 88 +
 3 files changed, 107 insertions(+)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index cfba3c0..e6f2de7 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -88,6 +88,8 @@ void mmc_remove_card_debugfs(struct mmc_card *card);
 void mmc_init_context_info(struct mmc_host *host);
 
 int mmc_execute_tuning(struct mmc_card *card);
+int mmc_hs200_to_hs400(struct mmc_card *card);
+int mmc_hs400_to_hs200(struct mmc_card *card);
 
 #endif
 
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 271986c..f480cb7 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -342,6 +342,7 @@ EXPORT_SYMBOL(mmc_retune_release);
 
 int mmc_retune(struct mmc_host *host)
 {
+   bool return_to_hs400 = false;
int err;
 
if (host-retune_now)
@@ -356,8 +357,24 @@ int mmc_retune(struct mmc_host *host)
 
host-doing_retune = 1;
 
+   if (host-ios.timing == MMC_TIMING_MMC_HS400) {
+   err = mmc_hs400_to_hs200(host-card);
+   if (err)
+   goto out;
+
+   return_to_hs400 = true;
+
+   if (host-ops-prepare_hs400_tuning)
+   host-ops-prepare_hs400_tuning(host, host-ios);
+   }
+
err = mmc_execute_tuning(host-card);
+   if (err)
+   goto out;
 
+   if (return_to_hs400)
+   err = mmc_hs200_to_hs400(host-card);
+out:
host-doing_retune = 0;
 
return err;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 53a9cb3..5a52b26 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1091,6 +1091,94 @@ static int mmc_select_hs400(struct mmc_card *card)
return 0;
 }
 
+int mmc_hs200_to_hs400(struct mmc_card *card)
+{
+   return mmc_select_hs400(card);
+}
+
+/* Caller must hold re-tuning */
+static int mmc_switch_status(struct mmc_card *card)
+{
+   u32 status;
+   int err;
+
+   err = mmc_send_status(card, status);
+   if (err)
+   return err;
+
+   return mmc_switch_status_error(card-host, status);
+}
+
+int mmc_hs400_to_hs200(struct mmc_card *card)
+{
+   struct mmc_host *host = card-host;
+   bool send_status = true;
+   unsigned int max_dtr;
+   int err;
+
+   if (host-caps  MMC_CAP_WAIT_WHILE_BUSY)
+   send_status = false;
+
+   /* Reduce frequency to HS */
+   max_dtr = card-ext_csd.hs_max_dtr;
+   mmc_set_clock(host, max_dtr);
+
+   /* Switch HS400 to HS DDR */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+  EXT_CSD_TIMING_HS, card-ext_csd.generic_cmd6_time,
+  true, send_status, true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   /* Switch HS DDR to HS */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
+  EXT_CSD_BUS_WIDTH_8, card-ext_csd.generic_cmd6_time,
+  true, send_status, true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_HS);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   /* Switch HS to HS200 */
+   err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
+  EXT_CSD_TIMING_HS200,
+  card-ext_csd.generic_cmd6_time, true, send_status,
+  true);
+   if (err)
+   goto out_err;
+
+   mmc_set_timing(host, MMC_TIMING_MMC_HS200);
+
+   if (!send_status) {
+   err = mmc_switch_status(card);
+   if (err)
+   goto out_err;
+   }
+
+   mmc_set_bus_speed(card);
+
+   return 0;
+
+out_err:
+   pr_err(%s: %s failed, error %d\n, mmc_hostname(card-host),
+  __func__, err);
+   return err;
+}
+
 /*
  * For device supporting HS200 mode, the following sequence
  * should be done before executing the tuning process.
-- 
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

[PATCH V5 11/15] mmc: sdhci: Change to new way of doing re-tuning

[Adrian Hunter]

Make use of mmc core support for re-tuning instead
of doing it all in the sdhci driver.

This patch also changes to flag the need for re-tuning
always after runtime suspend when tuning has been used
at initialization. Previously it was only done if
the re-tuning timer was in use.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/host/sdhci.c | 112 ++-
 drivers/mmc/host/sdhci.h |   3 --
 2 files changed, 13 insertions(+), 102 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c80287a..b345844 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -52,7 +52,6 @@ static void sdhci_finish_data(struct sdhci_host *);
 
 static void sdhci_finish_command(struct sdhci_host *);
 static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
-static void sdhci_tuning_timer(unsigned long data);
 static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
 static int sdhci_pre_dma_transfer(struct sdhci_host *host,
struct mmc_data *data,
@@ -254,17 +253,6 @@ static void sdhci_init(struct sdhci_host *host, int soft)
 static void sdhci_reinit(struct sdhci_host *host)
 {
sdhci_init(host, 0);
-   /*
-* Retuning stuffs are affected by different cards inserted and only
-* applicable to UHS-I cards. So reset these fields to their initial
-* value when card is removed.
-*/
-   if (host-flags  SDHCI_USING_RETUNING_TIMER) {
-   host-flags = ~SDHCI_USING_RETUNING_TIMER;
-
-   del_timer_sync(host-tuning_timer);
-   host-flags = ~SDHCI_NEEDS_RETUNING;
-   }
sdhci_enable_card_detection(host);
 }
 
@@ -1353,7 +1341,6 @@ static void sdhci_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
struct sdhci_host *host;
int present;
unsigned long flags;
-   u32 tuning_opcode;
 
host = mmc_priv(mmc);
 
@@ -1387,39 +1374,6 @@ static void sdhci_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
host-mrq-cmd-error = -ENOMEDIUM;
tasklet_schedule(host-finish_tasklet);
} else {
-   u32 present_state;
-
-   present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
-   /*
-* Check if the re-tuning timer has already expired and there
-* is no on-going data transfer and DAT0 is not busy. If so,
-* we need to execute tuning procedure before sending command.
-*/
-   if ((host-flags  SDHCI_NEEDS_RETUNING) 
-   !(present_state  (SDHCI_DOING_WRITE | SDHCI_DOING_READ)) 
-   (present_state  SDHCI_DATA_0_LVL_MASK)) {
-   if (mmc-card) {
-   /* eMMC uses cmd21 but sd and sdio use cmd19 */
-   tuning_opcode =
-   mmc-card-type == MMC_TYPE_MMC ?
-   MMC_SEND_TUNING_BLOCK_HS200 :
-   MMC_SEND_TUNING_BLOCK;
-
-   /* Here we need to set the host-mrq to NULL,
-* in case the pending finish_tasklet
-* finishes it incorrectly.
-*/
-   host-mrq = NULL;
-
-   spin_unlock_irqrestore(host-lock, flags);
-   sdhci_execute_tuning(mmc, tuning_opcode);
-   spin_lock_irqsave(host-lock, flags);
-
-   /* Restore original mmc_request structure */
-   host-mrq = mrq;
-   }
-   }
-
if (mrq-sbc  !(host-flags  SDHCI_AUTO_CMD23))
sdhci_send_command(host, mrq-sbc);
else
@@ -2065,23 +2019,18 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, 
u32 opcode)
}
 
 out:
-   host-flags = ~SDHCI_NEEDS_RETUNING;
-
if (tuning_count) {
-   host-flags |= SDHCI_USING_RETUNING_TIMER;
-   mod_timer(host-tuning_timer, jiffies + tuning_count * HZ);
+   /*
+* In case tuning fails, host controllers which support
+* re-tuning can try tuning again at a later time, when the
+* re-tuning timer expires.  So for these controllers, we
+* return 0. Since there might be other controllers who do not
+* have this capability, we return error for them.
+*/
+   err = 0;
}
 
-   /*
-* In case tuning fails, host controllers which support re-tuning can
-* try tuning again at a later time, when the re-tuning timer expires.
-* So for these controllers, we return 0. Since

[PATCH V5 09/15] mmc: core: Separate out the mmc_switch status check so it can be re-used

[Adrian Hunter]

Make a separate function to do the mmc_switch status check
so it can be re-used. This is preparation for adding support
for HS400 re-tuning.

Signed-off-by: Adrian Hunter adrian.hun...@intel.com
---
 drivers/mmc/core/mmc_ops.c | 30 --
 drivers/mmc/core/mmc_ops.h |  1 +
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index f67b0fd..02be77e 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -449,6 +449,21 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc)
return err;
 }
 
+int mmc_switch_status_error(struct mmc_host *host, u32 status)
+{
+   if (mmc_host_is_spi(host)) {
+   if (status  R1_SPI_ILLEGAL_COMMAND)
+   return -EBADMSG;
+   } else {
+   if (status  0xFDFFA000)
+   pr_warn(%s: unexpected status %#x after switch\n,
+   mmc_hostname(host), status);
+   if (status  R1_SWITCH_ERROR)
+   return -EBADMSG;
+   }
+   return 0;
+}
+
 /**
  * __mmc_switch - modify EXT_CSD register
  * @card: the MMC card associated with the data transfer
@@ -557,20 +572,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, 
u8 value,
}
} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
 
-   if (mmc_host_is_spi(host)) {
-   if (status  R1_SPI_ILLEGAL_COMMAND) {
-   err = -EBADMSG;
-   goto out;
-   }
-   } else {
-   if (status  0xFDFFA000)
-   pr_warn(%s: unexpected status %#x after switch\n,
-   mmc_hostname(host), status);
-   if (status  R1_SWITCH_ERROR) {
-   err = -EBADMSG;
-   goto out;
-   }
-   }
+   err = mmc_switch_status_error(host, status);
 out:
mmc_retune_release(host);
 
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 6f4b00e..f498f9a 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -27,6 +27,7 @@ int mmc_spi_set_crc(struct mmc_host *host, int use_crc);
 int mmc_bus_test(struct mmc_card *card, u8 bus_width);
 int mmc_send_hpi_cmd(struct mmc_card *card, u32 *status);
 int mmc_can_ext_csd(struct mmc_card *card);
+int mmc_switch_status_error(struct mmc_host *host, u32 status);
 
 #endif
 
-- 
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