* [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB
@ 2016-05-04 11:38 Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning Adrian Hunter
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Adrian Hunter @ 2016-05-04 11:38 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
Hi
The RPMB partition only allows certain commands. In particular,
the tuning command (CMD21) is not allowed - refer JEDEC eMMC
standard v5.1 section 6.2.2 Command restrictions.
That means commands will begin failing if re-tuning is needed
while switched to the RPMB partition.
As we discussed here:
http://marc.info/?l=linux-mmc&m=146218765801869
this patch set takes a new approach which is:
Re-tune before switching to the RPMB partition
Don't allow re-tuning while switched to RPMB
Switch back from the RPMB partition immediately
I gave it a brief test with a HS400 eMMC and using mmc utils
to access RPMB. Despite all the partition switches and
re-tuning in between, all RPMB operations were successful.
Changes in V2:
New approach entirely
Adrian Hunter (3):
mmc: core: Add a facility to "pause" re-tuning
mmc: block: Always switch back to main area after RPMB access
mmc: block: Pause re-tuning while switched to the RPMB partition
drivers/mmc/card/block.c | 19 ++++++++++++++++++-
drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 4 ++++
3 files changed, 44 insertions(+), 1 deletion(-)
Regards
Adrian
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
@ 2016-05-04 11:38 ` Adrian Hunter
2016-05-10 12:24 ` Ulf Hansson
2016-05-04 11:38 ` [PATCH V2 2/3] mmc: block: Always switch back to main area after RPMB access Adrian Hunter
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-04 11:38 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
Re-tuning is not possible when switched to the RPMB
partition. However re-tuning should not be needed
if re-tuning is done immediately before switching,
a small set of operations is done, and then we
immediately switch back to the main partition.
To ensure that re-tuning can't be done for a short
while, add a facility to "pause" re-tuning.
The existing facility to hold / release re-tuning
is used but it also flags re-tuning as needed to cause
re-tuning before the next command (which will be the
switch to RPMB).
We also need to "unpause" in the recovery path, which
is catered for by adding it to mmc_retune_disable().
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
include/linux/mmc/host.h | 4 ++++
2 files changed, 26 insertions(+)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee16c0d3..302e5858755a 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
jiffies + host->retune_period * HZ);
}
+/*
+ * Pause re-tuning for a small set of operations. The pause begins after the
+ * next command and after first doing re-tuning.
+ */
+void mmc_retune_pause(struct mmc_host *host)
+{
+ if (!host->retune_paused) {
+ host->retune_paused = 1;
+ mmc_retune_needed(host);
+ mmc_retune_hold(host);
+ }
+}
+
+void mmc_retune_unpause(struct mmc_host *host)
+{
+ if (host->retune_paused) {
+ host->retune_paused = 0;
+ mmc_retune_release(host);
+ }
+}
+
void mmc_retune_disable(struct mmc_host *host)
{
+ mmc_retune_unpause(host);
host->can_retune = 0;
del_timer_sync(&host->retune_timer);
host->retune_now = 0;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 85800b48241f..45cde8cd39f2 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -329,6 +329,7 @@ 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 */
+ unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
int rescan_disable; /* disable card detection */
int rescan_entered; /* used with nonremovable devices */
@@ -526,4 +527,7 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
host->retune_now = 1;
}
+void mmc_retune_pause(struct mmc_host *host);
+void mmc_retune_unpause(struct mmc_host *host);
+
#endif /* LINUX_MMC_HOST_H */
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V2 2/3] mmc: block: Always switch back to main area after RPMB access
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning Adrian Hunter
@ 2016-05-04 11:38 ` Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 3/3] mmc: block: Pause re-tuning while switched to the RPMB partition Adrian Hunter
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2016-05-04 11:38 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
In preparation to support the use of the RPMB partition with transfer
modes that might require re-tuning, always switch back to the main
area after RPMB access.
RPMB is accessible only via IOCTL so only those paths are affected.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/block.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 9ce679255775..a65043854458 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -618,6 +618,10 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
ioc_err = __mmc_blk_ioctl_cmd(card, md, idata);
+ /* Always switch back to main area after RPMB access */
+ if (md->area_type & MMC_BLK_DATA_AREA_RPMB)
+ mmc_blk_part_switch(card, dev_get_drvdata(&card->dev));
+
mmc_put_card(card);
err = mmc_blk_ioctl_copy_to_user(ic_ptr, idata);
@@ -685,6 +689,10 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
for (i = 0; i < num_of_cmds && !ioc_err; i++)
ioc_err = __mmc_blk_ioctl_cmd(card, md, idata[i]);
+ /* Always switch back to main area after RPMB access */
+ if (md->area_type & MMC_BLK_DATA_AREA_RPMB)
+ mmc_blk_part_switch(card, dev_get_drvdata(&card->dev));
+
mmc_put_card(card);
/* copy to user if data and response */
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V2 3/3] mmc: block: Pause re-tuning while switched to the RPMB partition
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 2/3] mmc: block: Always switch back to main area after RPMB access Adrian Hunter
@ 2016-05-04 11:38 ` Adrian Hunter
2016-05-04 11:54 ` [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Winkler, Tomas
2016-05-10 10:28 ` Ulf Hansson
4 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2016-05-04 11:38 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
Re-tuning is not possible when switched to the RPMB
partition. However re-tuning should not be needed
if re-tuning is done immediately before switching,
a small set of operations is done, and then we
immediately switch back to the main partition.
A previous patch ensured that we immediately switch
back to the main partition.
This patch uses the new facility to "pause" re-tuning
before switching to the RPMB partition, and to "unpause"
it after switching from the RPMB partition.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/block.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index a65043854458..565009b190cd 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -756,16 +756,25 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
if (mmc_card_mmc(card)) {
u8 part_config = card->ext_csd.part_config;
+ if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
+ mmc_retune_pause(card->host);
+
part_config &= ~EXT_CSD_PART_CONFIG_ACC_MASK;
part_config |= md->part_type;
ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_PART_CONFIG, part_config,
card->ext_csd.part_time);
- if (ret)
+ if (ret) {
+ if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
+ mmc_retune_unpause(card->host);
return ret;
+ }
card->ext_csd.part_config = part_config;
+
+ if (main_md->part_curr == EXT_CSD_PART_CONFIG_ACC_RPMB)
+ mmc_retune_unpause(card->host);
}
main_md->part_curr = md->part_type;
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
` (2 preceding siblings ...)
2016-05-04 11:38 ` [PATCH V2 3/3] mmc: block: Pause re-tuning while switched to the RPMB partition Adrian Hunter
@ 2016-05-04 11:54 ` Winkler, Tomas
2016-05-10 10:28 ` Ulf Hansson
4 siblings, 0 replies; 16+ messages in thread
From: Winkler, Tomas @ 2016-05-04 11:54 UTC (permalink / raw)
To: Hunter, Adrian, Ulf Hansson; +Cc: linux-mmc
> -----Original Message-----
> From: Hunter, Adrian
> Sent: Wednesday, May 04, 2016 14:38
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc <linux-mmc@vger.kernel.org>; Winkler, Tomas
> <tomas.winkler@intel.com>
> Subject: [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB
>
> Hi
>
> The RPMB partition only allows certain commands. In particular, the tuning
> command (CMD21) is not allowed - refer JEDEC eMMC standard v5.1 section
> 6.2.2 Command restrictions.
>
> That means commands will begin failing if re-tuning is needed while switched
> to the RPMB partition.
>
> As we discussed here:
>
> http://marc.info/?l=linux-mmc&m=146218765801869
>
> this patch set takes a new approach which is:
>
> Re-tune before switching to the RPMB partition
> Don't allow re-tuning while switched to RPMB
> Switch back from the RPMB partition immediately
>
> I gave it a brief test with a HS400 eMMC and using mmc utils to access RPMB.
> Despite all the partition switches and re-tuning in between, all RPMB
> operations were successful.
>
>
> Changes in V2:
>
> New approach entirely
>
>
> Adrian Hunter (3):
> mmc: core: Add a facility to "pause" re-tuning
> mmc: block: Always switch back to main area after RPMB access
> mmc: block: Pause re-tuning while switched to the RPMB partition
>
> drivers/mmc/card/block.c | 19 ++++++++++++++++++-
> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
> include/linux/mmc/host.h | 4 ++++
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
The series looks good, it will be easy to adjust to my RPMB patch.
Thanks
Tomas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
` (3 preceding siblings ...)
2016-05-04 11:54 ` [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Winkler, Tomas
@ 2016-05-10 10:28 ` Ulf Hansson
4 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2016-05-10 10:28 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Tomas Winkler
On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> The RPMB partition only allows certain commands. In particular,
> the tuning command (CMD21) is not allowed - refer JEDEC eMMC
> standard v5.1 section 6.2.2 Command restrictions.
>
> That means commands will begin failing if re-tuning is needed
> while switched to the RPMB partition.
>
> As we discussed here:
>
> http://marc.info/?l=linux-mmc&m=146218765801869
>
> this patch set takes a new approach which is:
>
> Re-tune before switching to the RPMB partition
> Don't allow re-tuning while switched to RPMB
> Switch back from the RPMB partition immediately
>
> I gave it a brief test with a HS400 eMMC and using mmc utils
> to access RPMB. Despite all the partition switches and
> re-tuning in between, all RPMB operations were successful.
>
>
> Changes in V2:
>
> New approach entirely
>
>
> Adrian Hunter (3):
> mmc: core: Add a facility to "pause" re-tuning
> mmc: block: Always switch back to main area after RPMB access
> mmc: block: Pause re-tuning while switched to the RPMB partition
>
> drivers/mmc/card/block.c | 19 ++++++++++++++++++-
> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
> include/linux/mmc/host.h | 4 ++++
> 3 files changed, 44 insertions(+), 1 deletion(-)
>
>
> Regards
> Adrian
Thanks, applied for next!
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-04 11:38 ` [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning Adrian Hunter
@ 2016-05-10 12:24 ` Ulf Hansson
2016-05-10 13:03 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2016-05-10 12:24 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Tomas Winkler
On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Re-tuning is not possible when switched to the RPMB
> partition. However re-tuning should not be needed
> if re-tuning is done immediately before switching,
> a small set of operations is done, and then we
> immediately switch back to the main partition.
>
> To ensure that re-tuning can't be done for a short
> while, add a facility to "pause" re-tuning.
>
> The existing facility to hold / release re-tuning
> is used but it also flags re-tuning as needed to cause
> re-tuning before the next command (which will be the
> switch to RPMB).
>
> We also need to "unpause" in the recovery path, which
> is catered for by adding it to mmc_retune_disable().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
> include/linux/mmc/host.h | 4 ++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index e0a3ee16c0d3..302e5858755a 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
> jiffies + host->retune_period * HZ);
> }
>
> +/*
> + * Pause re-tuning for a small set of operations. The pause begins after the
> + * next command and after first doing re-tuning.
> + */
> +void mmc_retune_pause(struct mmc_host *host)
> +{
> + if (!host->retune_paused) {
> + host->retune_paused = 1;
> + mmc_retune_needed(host);
> + mmc_retune_hold(host);
> + }
> +}
> +
When the mmc block device driver is built as a module, this doesn't
build. I will drop the series from my next branch to sort this out.
Should we export these via EXPORT_SYMBOL_GPL, or implement them as
inline functions?
This also made me think about the SDIO/WLAN driver issue, during
system PM suspend/resume, which also needed temporary to disable
re-tuning.
*If* we are going to export these, I want to make it works for the
SDIO case well...
Kind regards
Uffe
> +void mmc_retune_unpause(struct mmc_host *host)
> +{
> + if (host->retune_paused) {
> + host->retune_paused = 0;
> + mmc_retune_release(host);
> + }
> +}
> +
> void mmc_retune_disable(struct mmc_host *host)
> {
> + mmc_retune_unpause(host);
> host->can_retune = 0;
> del_timer_sync(&host->retune_timer);
> host->retune_now = 0;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 85800b48241f..45cde8cd39f2 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -329,6 +329,7 @@ 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 */
> + unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
>
> int rescan_disable; /* disable card detection */
> int rescan_entered; /* used with nonremovable devices */
> @@ -526,4 +527,7 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
> host->retune_now = 1;
> }
>
> +void mmc_retune_pause(struct mmc_host *host);
> +void mmc_retune_unpause(struct mmc_host *host);
> +
> #endif /* LINUX_MMC_HOST_H */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-10 12:24 ` Ulf Hansson
@ 2016-05-10 13:03 ` Adrian Hunter
2016-05-11 6:48 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-10 13:03 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
On 10/05/16 15:24, Ulf Hansson wrote:
> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Re-tuning is not possible when switched to the RPMB
>> partition. However re-tuning should not be needed
>> if re-tuning is done immediately before switching,
>> a small set of operations is done, and then we
>> immediately switch back to the main partition.
>>
>> To ensure that re-tuning can't be done for a short
>> while, add a facility to "pause" re-tuning.
>>
>> The existing facility to hold / release re-tuning
>> is used but it also flags re-tuning as needed to cause
>> re-tuning before the next command (which will be the
>> switch to RPMB).
>>
>> We also need to "unpause" in the recovery path, which
>> is catered for by adding it to mmc_retune_disable().
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>> include/linux/mmc/host.h | 4 ++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>> index e0a3ee16c0d3..302e5858755a 100644
>> --- a/drivers/mmc/core/host.c
>> +++ b/drivers/mmc/core/host.c
>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>> jiffies + host->retune_period * HZ);
>> }
>>
>> +/*
>> + * Pause re-tuning for a small set of operations. The pause begins after the
>> + * next command and after first doing re-tuning.
>> + */
>> +void mmc_retune_pause(struct mmc_host *host)
>> +{
>> + if (!host->retune_paused) {
>> + host->retune_paused = 1;
>> + mmc_retune_needed(host);
>> + mmc_retune_hold(host);
>> + }
>> +}
>> +
>
> When the mmc block device driver is built as a module, this doesn't
> build. I will drop the series from my next branch to sort this out.
Oops. Sorry!
> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
> inline functions?
They need to be exported. I tend to go with what else is in the same file
i.e. host.c is exporting using EXPORT_SYMBOL()
>
> This also made me think about the SDIO/WLAN driver issue, during
> system PM suspend/resume, which also needed temporary to disable
> re-tuning.
>
> *If* we are going to export these, I want to make it works for the
> SDIO case well...
SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
>
> Kind regards
> Uffe
>
>> +void mmc_retune_unpause(struct mmc_host *host)
>> +{
>> + if (host->retune_paused) {
>> + host->retune_paused = 0;
>> + mmc_retune_release(host);
>> + }
>> +}
>> +
>> void mmc_retune_disable(struct mmc_host *host)
>> {
>> + mmc_retune_unpause(host);
>> host->can_retune = 0;
>> del_timer_sync(&host->retune_timer);
>> host->retune_now = 0;
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 85800b48241f..45cde8cd39f2 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -329,6 +329,7 @@ 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 */
>> + unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
>>
>> int rescan_disable; /* disable card detection */
>> int rescan_entered; /* used with nonremovable devices */
>> @@ -526,4 +527,7 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
>> host->retune_now = 1;
>> }
>>
>> +void mmc_retune_pause(struct mmc_host *host);
>> +void mmc_retune_unpause(struct mmc_host *host);
>> +
>> #endif /* LINUX_MMC_HOST_H */
>> --
>> 1.9.1
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-10 13:03 ` Adrian Hunter
@ 2016-05-11 6:48 ` Ulf Hansson
2016-05-11 9:00 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2016-05-11 6:48 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Tomas Winkler
On 10 May 2016 at 15:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 10/05/16 15:24, Ulf Hansson wrote:
>> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> Re-tuning is not possible when switched to the RPMB
>>> partition. However re-tuning should not be needed
>>> if re-tuning is done immediately before switching,
>>> a small set of operations is done, and then we
>>> immediately switch back to the main partition.
>>>
>>> To ensure that re-tuning can't be done for a short
>>> while, add a facility to "pause" re-tuning.
>>>
>>> The existing facility to hold / release re-tuning
>>> is used but it also flags re-tuning as needed to cause
>>> re-tuning before the next command (which will be the
>>> switch to RPMB).
>>>
>>> We also need to "unpause" in the recovery path, which
>>> is catered for by adding it to mmc_retune_disable().
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>>> include/linux/mmc/host.h | 4 ++++
>>> 2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>> index e0a3ee16c0d3..302e5858755a 100644
>>> --- a/drivers/mmc/core/host.c
>>> +++ b/drivers/mmc/core/host.c
>>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>>> jiffies + host->retune_period * HZ);
>>> }
>>>
>>> +/*
>>> + * Pause re-tuning for a small set of operations. The pause begins after the
>>> + * next command and after first doing re-tuning.
>>> + */
>>> +void mmc_retune_pause(struct mmc_host *host)
>>> +{
>>> + if (!host->retune_paused) {
>>> + host->retune_paused = 1;
>>> + mmc_retune_needed(host);
>>> + mmc_retune_hold(host);
>>> + }
>>> +}
>>> +
>>
>> When the mmc block device driver is built as a module, this doesn't
>> build. I will drop the series from my next branch to sort this out.
>
> Oops. Sorry!
>
>> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
>> inline functions?
>
> They need to be exported. I tend to go with what else is in the same file
> i.e. host.c is exporting using EXPORT_SYMBOL()
Yes, okay!
>
>>
>> This also made me think about the SDIO/WLAN driver issue, during
>> system PM suspend/resume, which also needed temporary to disable
>> re-tuning.
>>
>> *If* we are going to export these, I want to make it works for the
>> SDIO case well...
>
> SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
I what way is it different?
Regarding the header file, my point is that I want to keep the numbers
of exported functions to a minimum.
Do you think there is way to combine these two use cases, such only
one pair of new functions would be needed?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-11 6:48 ` Ulf Hansson
@ 2016-05-11 9:00 ` Adrian Hunter
2016-05-12 6:14 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-11 9:00 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
On 11/05/16 09:48, Ulf Hansson wrote:
> On 10 May 2016 at 15:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 10/05/16 15:24, Ulf Hansson wrote:
>>> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> Re-tuning is not possible when switched to the RPMB
>>>> partition. However re-tuning should not be needed
>>>> if re-tuning is done immediately before switching,
>>>> a small set of operations is done, and then we
>>>> immediately switch back to the main partition.
>>>>
>>>> To ensure that re-tuning can't be done for a short
>>>> while, add a facility to "pause" re-tuning.
>>>>
>>>> The existing facility to hold / release re-tuning
>>>> is used but it also flags re-tuning as needed to cause
>>>> re-tuning before the next command (which will be the
>>>> switch to RPMB).
>>>>
>>>> We also need to "unpause" in the recovery path, which
>>>> is catered for by adding it to mmc_retune_disable().
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>>>> include/linux/mmc/host.h | 4 ++++
>>>> 2 files changed, 26 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>> index e0a3ee16c0d3..302e5858755a 100644
>>>> --- a/drivers/mmc/core/host.c
>>>> +++ b/drivers/mmc/core/host.c
>>>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>>>> jiffies + host->retune_period * HZ);
>>>> }
>>>>
>>>> +/*
>>>> + * Pause re-tuning for a small set of operations. The pause begins after the
>>>> + * next command and after first doing re-tuning.
>>>> + */
>>>> +void mmc_retune_pause(struct mmc_host *host)
>>>> +{
>>>> + if (!host->retune_paused) {
>>>> + host->retune_paused = 1;
>>>> + mmc_retune_needed(host);
>>>> + mmc_retune_hold(host);
>>>> + }
>>>> +}
>>>> +
>>>
>>> When the mmc block device driver is built as a module, this doesn't
>>> build. I will drop the series from my next branch to sort this out.
>>
>> Oops. Sorry!
>>
>>> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
>>> inline functions?
>>
>> They need to be exported. I tend to go with what else is in the same file
>> i.e. host.c is exporting using EXPORT_SYMBOL()
>
> Yes, okay!
>
>>
>>>
>>> This also made me think about the SDIO/WLAN driver issue, during
>>> system PM suspend/resume, which also needed temporary to disable
>>> re-tuning.
>>>
>>> *If* we are going to export these, I want to make it works for the
>>> SDIO case well...
>>
>> SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
>
> I what way is it different?
In the RPMB case there are 3 things to do:
1. Do re-tuning at next command
2. Hold re-tuning
3. Release re-tuning
In the SDIO case there are 3 things to do:
1. Prevent re-tuning at next command
2. Hold re-tuning
3. Release re-tuning
So the first thing is different.
>
> Regarding the header file, my point is that I want to keep the numbers
> of exported functions to a minimum.
>
> Do you think there is way to combine these two use cases, such only
> one pair of new functions would be needed?
To make them the same we would need to add a parameter to mmc_retune_pause()
i.e. something like
void mmc_retune_pause(struct mmc_host *host, bool retune_now)
{
if (!host->retune_paused) {
host->retune_paused = 1;
mmc_retune_hold(host);
if (retune_now)
mmc_retune_needed(host);
else
host->retune_now = 0;
}
}
For SDIO we would need to put the function declarations in sdio_func.h as
well as host.h.
Shall I make a V3 of these patches like that?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-11 9:00 ` Adrian Hunter
@ 2016-05-12 6:14 ` Adrian Hunter
2016-05-12 13:20 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-12 6:14 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
On 11/05/16 12:00, Adrian Hunter wrote:
> On 11/05/16 09:48, Ulf Hansson wrote:
>> On 10 May 2016 at 15:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 10/05/16 15:24, Ulf Hansson wrote:
>>>> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> Re-tuning is not possible when switched to the RPMB
>>>>> partition. However re-tuning should not be needed
>>>>> if re-tuning is done immediately before switching,
>>>>> a small set of operations is done, and then we
>>>>> immediately switch back to the main partition.
>>>>>
>>>>> To ensure that re-tuning can't be done for a short
>>>>> while, add a facility to "pause" re-tuning.
>>>>>
>>>>> The existing facility to hold / release re-tuning
>>>>> is used but it also flags re-tuning as needed to cause
>>>>> re-tuning before the next command (which will be the
>>>>> switch to RPMB).
>>>>>
>>>>> We also need to "unpause" in the recovery path, which
>>>>> is catered for by adding it to mmc_retune_disable().
>>>>>
>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>> ---
>>>>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>>>>> include/linux/mmc/host.h | 4 ++++
>>>>> 2 files changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>>> index e0a3ee16c0d3..302e5858755a 100644
>>>>> --- a/drivers/mmc/core/host.c
>>>>> +++ b/drivers/mmc/core/host.c
>>>>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>>>>> jiffies + host->retune_period * HZ);
>>>>> }
>>>>>
>>>>> +/*
>>>>> + * Pause re-tuning for a small set of operations. The pause begins after the
>>>>> + * next command and after first doing re-tuning.
>>>>> + */
>>>>> +void mmc_retune_pause(struct mmc_host *host)
>>>>> +{
>>>>> + if (!host->retune_paused) {
>>>>> + host->retune_paused = 1;
>>>>> + mmc_retune_needed(host);
>>>>> + mmc_retune_hold(host);
>>>>> + }
>>>>> +}
>>>>> +
>>>>
>>>> When the mmc block device driver is built as a module, this doesn't
>>>> build. I will drop the series from my next branch to sort this out.
>>>
>>> Oops. Sorry!
>>>
>>>> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
>>>> inline functions?
>>>
>>> They need to be exported. I tend to go with what else is in the same file
>>> i.e. host.c is exporting using EXPORT_SYMBOL()
>>
>> Yes, okay!
>>
>>>
>>>>
>>>> This also made me think about the SDIO/WLAN driver issue, during
>>>> system PM suspend/resume, which also needed temporary to disable
>>>> re-tuning.
>>>>
>>>> *If* we are going to export these, I want to make it works for the
>>>> SDIO case well...
>>>
>>> SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
>>
>> I what way is it different?
>
> In the RPMB case there are 3 things to do:
> 1. Do re-tuning at next command
> 2. Hold re-tuning
> 3. Release re-tuning
>
> In the SDIO case there are 3 things to do:
> 1. Prevent re-tuning at next command
> 2. Hold re-tuning
> 3. Release re-tuning
>
> So the first thing is different.
>
>>
>> Regarding the header file, my point is that I want to keep the numbers
>> of exported functions to a minimum.
>>
>> Do you think there is way to combine these two use cases, such only
>> one pair of new functions would be needed?
>
> To make them the same we would need to add a parameter to mmc_retune_pause()
> i.e. something like
>
> void mmc_retune_pause(struct mmc_host *host, bool retune_now)
> {
> if (!host->retune_paused) {
> host->retune_paused = 1;
> mmc_retune_hold(host);
> if (retune_now)
> mmc_retune_needed(host);
> else
> host->retune_now = 0;
> }
> }
>
> For SDIO we would need to put the function declarations in sdio_func.h as
> well as host.h.
>
> Shall I make a V3 of these patches like that?
I looked again at sdio_func.h and it seems to have its own paradigm i.e. it
is a completely separate set of functions that take the SDIO function as a
parameter, and that hide and encapsulate core and host functions.
It would be inconsistent with that paradigm to expose mmc_retune_pause() and
mmc_retune_unpause() there. Is that what you want to do?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-12 13:20 ` Ulf Hansson
@ 2016-05-12 13:19 ` Adrian Hunter
2016-05-16 12:35 ` [PATCH V3 " Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-12 13:19 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
On 12/05/16 16:20, Ulf Hansson wrote:
> On 12 May 2016 at 08:14, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 11/05/16 12:00, Adrian Hunter wrote:
>>> On 11/05/16 09:48, Ulf Hansson wrote:
>>>> On 10 May 2016 at 15:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 10/05/16 15:24, Ulf Hansson wrote:
>>>>>> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>>> Re-tuning is not possible when switched to the RPMB
>>>>>>> partition. However re-tuning should not be needed
>>>>>>> if re-tuning is done immediately before switching,
>>>>>>> a small set of operations is done, and then we
>>>>>>> immediately switch back to the main partition.
>>>>>>>
>>>>>>> To ensure that re-tuning can't be done for a short
>>>>>>> while, add a facility to "pause" re-tuning.
>>>>>>>
>>>>>>> The existing facility to hold / release re-tuning
>>>>>>> is used but it also flags re-tuning as needed to cause
>>>>>>> re-tuning before the next command (which will be the
>>>>>>> switch to RPMB).
>>>>>>>
>>>>>>> We also need to "unpause" in the recovery path, which
>>>>>>> is catered for by adding it to mmc_retune_disable().
>>>>>>>
>>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>>>> ---
>>>>>>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>>>>>>> include/linux/mmc/host.h | 4 ++++
>>>>>>> 2 files changed, 26 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>>>>> index e0a3ee16c0d3..302e5858755a 100644
>>>>>>> --- a/drivers/mmc/core/host.c
>>>>>>> +++ b/drivers/mmc/core/host.c
>>>>>>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>>>>>>> jiffies + host->retune_period * HZ);
>>>>>>> }
>>>>>>>
>>>>>>> +/*
>>>>>>> + * Pause re-tuning for a small set of operations. The pause begins after the
>>>>>>> + * next command and after first doing re-tuning.
>>>>>>> + */
>>>>>>> +void mmc_retune_pause(struct mmc_host *host)
>>>>>>> +{
>>>>>>> + if (!host->retune_paused) {
>>>>>>> + host->retune_paused = 1;
>>>>>>> + mmc_retune_needed(host);
>>>>>>> + mmc_retune_hold(host);
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>
>>>>>> When the mmc block device driver is built as a module, this doesn't
>>>>>> build. I will drop the series from my next branch to sort this out.
>>>>>
>>>>> Oops. Sorry!
>>>>>
>>>>>> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
>>>>>> inline functions?
>>>>>
>>>>> They need to be exported. I tend to go with what else is in the same file
>>>>> i.e. host.c is exporting using EXPORT_SYMBOL()
>>>>
>>>> Yes, okay!
>>>>
>>>>>
>>>>>>
>>>>>> This also made me think about the SDIO/WLAN driver issue, during
>>>>>> system PM suspend/resume, which also needed temporary to disable
>>>>>> re-tuning.
>>>>>>
>>>>>> *If* we are going to export these, I want to make it works for the
>>>>>> SDIO case well...
>>>>>
>>>>> SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
>>>>
>>>> I what way is it different?
>>>
>>> In the RPMB case there are 3 things to do:
>>> 1. Do re-tuning at next command
>>> 2. Hold re-tuning
>>> 3. Release re-tuning
>>>
>>> In the SDIO case there are 3 things to do:
>>> 1. Prevent re-tuning at next command
>>> 2. Hold re-tuning
>>> 3. Release re-tuning
>>>
>>> So the first thing is different.
>>>
>>>>
>>>> Regarding the header file, my point is that I want to keep the numbers
>>>> of exported functions to a minimum.
>>>>
>>>> Do you think there is way to combine these two use cases, such only
>>>> one pair of new functions would be needed?
>>>
>>> To make them the same we would need to add a parameter to mmc_retune_pause()
>>> i.e. something like
>>>
>>> void mmc_retune_pause(struct mmc_host *host, bool retune_now)
>>> {
>>> if (!host->retune_paused) {
>>> host->retune_paused = 1;
>>> mmc_retune_hold(host);
>>> if (retune_now)
>>> mmc_retune_needed(host);
>>> else
>>> host->retune_now = 0;
>>> }
>>> }
>>>
>>> For SDIO we would need to put the function declarations in sdio_func.h as
>>> well as host.h.
>>>
>>> Shall I make a V3 of these patches like that?
>
> No.
>
>>
>> I looked again at sdio_func.h and it seems to have its own paradigm i.e. it
>> is a completely separate set of functions that take the SDIO function as a
>> parameter, and that hide and encapsulate core and host functions.
>>
>> It would be inconsistent with that paradigm to expose mmc_retune_pause() and
>> mmc_retune_unpause() there. Is that what you want to do?
>
> I agree, we shouldn't mess up the SDIO API with these functions.
>
> Instead, let's keep it simple and just leave out the SDIO case for
> now. So do EXPORT_SYMBOL for those APIs you added in $subject patch,
> without further changes.
>
> Okay?
Yes please :-)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-12 6:14 ` Adrian Hunter
@ 2016-05-12 13:20 ` Ulf Hansson
2016-05-12 13:19 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2016-05-12 13:20 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Tomas Winkler
On 12 May 2016 at 08:14, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/05/16 12:00, Adrian Hunter wrote:
>> On 11/05/16 09:48, Ulf Hansson wrote:
>>> On 10 May 2016 at 15:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 10/05/16 15:24, Ulf Hansson wrote:
>>>>> On 4 May 2016 at 13:38, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>> Re-tuning is not possible when switched to the RPMB
>>>>>> partition. However re-tuning should not be needed
>>>>>> if re-tuning is done immediately before switching,
>>>>>> a small set of operations is done, and then we
>>>>>> immediately switch back to the main partition.
>>>>>>
>>>>>> To ensure that re-tuning can't be done for a short
>>>>>> while, add a facility to "pause" re-tuning.
>>>>>>
>>>>>> The existing facility to hold / release re-tuning
>>>>>> is used but it also flags re-tuning as needed to cause
>>>>>> re-tuning before the next command (which will be the
>>>>>> switch to RPMB).
>>>>>>
>>>>>> We also need to "unpause" in the recovery path, which
>>>>>> is catered for by adding it to mmc_retune_disable().
>>>>>>
>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>>> ---
>>>>>> drivers/mmc/core/host.c | 22 ++++++++++++++++++++++
>>>>>> include/linux/mmc/host.h | 4 ++++
>>>>>> 2 files changed, 26 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>>>> index e0a3ee16c0d3..302e5858755a 100644
>>>>>> --- a/drivers/mmc/core/host.c
>>>>>> +++ b/drivers/mmc/core/host.c
>>>>>> @@ -68,8 +68,30 @@ void mmc_retune_enable(struct mmc_host *host)
>>>>>> jiffies + host->retune_period * HZ);
>>>>>> }
>>>>>>
>>>>>> +/*
>>>>>> + * Pause re-tuning for a small set of operations. The pause begins after the
>>>>>> + * next command and after first doing re-tuning.
>>>>>> + */
>>>>>> +void mmc_retune_pause(struct mmc_host *host)
>>>>>> +{
>>>>>> + if (!host->retune_paused) {
>>>>>> + host->retune_paused = 1;
>>>>>> + mmc_retune_needed(host);
>>>>>> + mmc_retune_hold(host);
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>
>>>>> When the mmc block device driver is built as a module, this doesn't
>>>>> build. I will drop the series from my next branch to sort this out.
>>>>
>>>> Oops. Sorry!
>>>>
>>>>> Should we export these via EXPORT_SYMBOL_GPL, or implement them as
>>>>> inline functions?
>>>>
>>>> They need to be exported. I tend to go with what else is in the same file
>>>> i.e. host.c is exporting using EXPORT_SYMBOL()
>>>
>>> Yes, okay!
>>>
>>>>
>>>>>
>>>>> This also made me think about the SDIO/WLAN driver issue, during
>>>>> system PM suspend/resume, which also needed temporary to disable
>>>>> re-tuning.
>>>>>
>>>>> *If* we are going to export these, I want to make it works for the
>>>>> SDIO case well...
>>>>
>>>> SDIO case is slightly different, and SDIO uses its own header file sdio_func.h.
>>>
>>> I what way is it different?
>>
>> In the RPMB case there are 3 things to do:
>> 1. Do re-tuning at next command
>> 2. Hold re-tuning
>> 3. Release re-tuning
>>
>> In the SDIO case there are 3 things to do:
>> 1. Prevent re-tuning at next command
>> 2. Hold re-tuning
>> 3. Release re-tuning
>>
>> So the first thing is different.
>>
>>>
>>> Regarding the header file, my point is that I want to keep the numbers
>>> of exported functions to a minimum.
>>>
>>> Do you think there is way to combine these two use cases, such only
>>> one pair of new functions would be needed?
>>
>> To make them the same we would need to add a parameter to mmc_retune_pause()
>> i.e. something like
>>
>> void mmc_retune_pause(struct mmc_host *host, bool retune_now)
>> {
>> if (!host->retune_paused) {
>> host->retune_paused = 1;
>> mmc_retune_hold(host);
>> if (retune_now)
>> mmc_retune_needed(host);
>> else
>> host->retune_now = 0;
>> }
>> }
>>
>> For SDIO we would need to put the function declarations in sdio_func.h as
>> well as host.h.
>>
>> Shall I make a V3 of these patches like that?
No.
>
> I looked again at sdio_func.h and it seems to have its own paradigm i.e. it
> is a completely separate set of functions that take the SDIO function as a
> parameter, and that hide and encapsulate core and host functions.
>
> It would be inconsistent with that paradigm to expose mmc_retune_pause() and
> mmc_retune_unpause() there. Is that what you want to do?
I agree, we shouldn't mess up the SDIO API with these functions.
Instead, let's keep it simple and just leave out the SDIO case for
now. So do EXPORT_SYMBOL for those APIs you added in $subject patch,
without further changes.
Okay?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V3 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-12 13:19 ` Adrian Hunter
@ 2016-05-16 12:35 ` Adrian Hunter
2016-05-17 15:06 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2016-05-16 12:35 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
Re-tuning is not possible when switched to the RPMB
partition. However re-tuning should not be needed
if re-tuning is done immediately before switching,
a small set of operations is done, and then we
immediately switch back to the main partition.
To ensure that re-tuning can't be done for a short
while, add a facility to "pause" re-tuning.
The existing facility to hold / release re-tuning
is used but it also flags re-tuning as needed to cause
re-tuning before the next command (which will be the
switch to RPMB).
We also need to "unpause" in the recovery path, which
is catered for by adding it to mmc_retune_disable().
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in V3:
Added:
EXPORT_SYMBOL(mmc_retune_pause);
EXPORT_SYMBOL(mmc_retune_unpause);
Changes in V2:
New approach entirely
drivers/mmc/core/host.c | 24 ++++++++++++++++++++++++
include/linux/mmc/host.h | 4 ++++
2 files changed, 28 insertions(+)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee16c0d3..1be42fab1a30 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -68,8 +68,32 @@ void mmc_retune_enable(struct mmc_host *host)
jiffies + host->retune_period * HZ);
}
+/*
+ * Pause re-tuning for a small set of operations. The pause begins after the
+ * next command and after first doing re-tuning.
+ */
+void mmc_retune_pause(struct mmc_host *host)
+{
+ if (!host->retune_paused) {
+ host->retune_paused = 1;
+ mmc_retune_needed(host);
+ mmc_retune_hold(host);
+ }
+}
+EXPORT_SYMBOL(mmc_retune_pause);
+
+void mmc_retune_unpause(struct mmc_host *host)
+{
+ if (host->retune_paused) {
+ host->retune_paused = 0;
+ mmc_retune_release(host);
+ }
+}
+EXPORT_SYMBOL(mmc_retune_unpause);
+
void mmc_retune_disable(struct mmc_host *host)
{
+ mmc_retune_unpause(host);
host->can_retune = 0;
del_timer_sync(&host->retune_timer);
host->retune_now = 0;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 85800b48241f..45cde8cd39f2 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -329,6 +329,7 @@ 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 */
+ unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
int rescan_disable; /* disable card detection */
int rescan_entered; /* used with nonremovable devices */
@@ -526,4 +527,7 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
host->retune_now = 1;
}
+void mmc_retune_pause(struct mmc_host *host);
+void mmc_retune_unpause(struct mmc_host *host);
+
#endif /* LINUX_MMC_HOST_H */
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH V3 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-16 12:35 ` [PATCH V3 " Adrian Hunter
@ 2016-05-17 15:06 ` Ulf Hansson
2016-05-18 6:44 ` Adrian Hunter
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2016-05-17 15:06 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Tomas Winkler
On 16 May 2016 at 14:35, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Re-tuning is not possible when switched to the RPMB
> partition. However re-tuning should not be needed
> if re-tuning is done immediately before switching,
> a small set of operations is done, and then we
> immediately switch back to the main partition.
>
> To ensure that re-tuning can't be done for a short
> while, add a facility to "pause" re-tuning.
>
> The existing facility to hold / release re-tuning
> is used but it also flags re-tuning as needed to cause
> re-tuning before the next command (which will be the
> switch to RPMB).
>
> We also need to "unpause" in the recovery path, which
> is catered for by adding it to mmc_retune_disable().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
Thanks, re-applied the series with this v3 version of patch 1/3.
If the tests from kernelci and in linux-next works out okay, I intend
to send this as fixes for 4.7 rc1.
Kind regards
Uffe
>
>
> Changes in V3:
>
> Added:
> EXPORT_SYMBOL(mmc_retune_pause);
> EXPORT_SYMBOL(mmc_retune_unpause);
>
> Changes in V2:
>
> New approach entirely
>
>
> drivers/mmc/core/host.c | 24 ++++++++++++++++++++++++
> include/linux/mmc/host.h | 4 ++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index e0a3ee16c0d3..1be42fab1a30 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -68,8 +68,32 @@ void mmc_retune_enable(struct mmc_host *host)
> jiffies + host->retune_period * HZ);
> }
>
> +/*
> + * Pause re-tuning for a small set of operations. The pause begins after the
> + * next command and after first doing re-tuning.
> + */
> +void mmc_retune_pause(struct mmc_host *host)
> +{
> + if (!host->retune_paused) {
> + host->retune_paused = 1;
> + mmc_retune_needed(host);
> + mmc_retune_hold(host);
> + }
> +}
> +EXPORT_SYMBOL(mmc_retune_pause);
> +
> +void mmc_retune_unpause(struct mmc_host *host)
> +{
> + if (host->retune_paused) {
> + host->retune_paused = 0;
> + mmc_retune_release(host);
> + }
> +}
> +EXPORT_SYMBOL(mmc_retune_unpause);
> +
> void mmc_retune_disable(struct mmc_host *host)
> {
> + mmc_retune_unpause(host);
> host->can_retune = 0;
> del_timer_sync(&host->retune_timer);
> host->retune_now = 0;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 85800b48241f..45cde8cd39f2 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -329,6 +329,7 @@ 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 */
> + unsigned int retune_paused:1; /* re-tuning is temporarily disabled */
>
> int rescan_disable; /* disable card detection */
> int rescan_entered; /* used with nonremovable devices */
> @@ -526,4 +527,7 @@ static inline void mmc_retune_recheck(struct mmc_host *host)
> host->retune_now = 1;
> }
>
> +void mmc_retune_pause(struct mmc_host *host);
> +void mmc_retune_unpause(struct mmc_host *host);
> +
> #endif /* LINUX_MMC_HOST_H */
> --
> 1.9.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V3 1/3] mmc: core: Add a facility to "pause" re-tuning
2016-05-17 15:06 ` Ulf Hansson
@ 2016-05-18 6:44 ` Adrian Hunter
0 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2016-05-18 6:44 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Tomas Winkler
On 17/05/16 18:06, Ulf Hansson wrote:
> On 16 May 2016 at 14:35, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Re-tuning is not possible when switched to the RPMB
>> partition. However re-tuning should not be needed
>> if re-tuning is done immediately before switching,
>> a small set of operations is done, and then we
>> immediately switch back to the main partition.
>>
>> To ensure that re-tuning can't be done for a short
>> while, add a facility to "pause" re-tuning.
>>
>> The existing facility to hold / release re-tuning
>> is used but it also flags re-tuning as needed to cause
>> re-tuning before the next command (which will be the
>> switch to RPMB).
>>
>> We also need to "unpause" in the recovery path, which
>> is catered for by adding it to mmc_retune_disable().
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>
> Thanks, re-applied the series with this v3 version of patch 1/3.
>
> If the tests from kernelci and in linux-next works out okay, I intend
> to send this as fixes for 4.7 rc1.
Thank you!
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-05-18 6:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-04 11:38 [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 1/3] mmc: core: Add a facility to "pause" re-tuning Adrian Hunter
2016-05-10 12:24 ` Ulf Hansson
2016-05-10 13:03 ` Adrian Hunter
2016-05-11 6:48 ` Ulf Hansson
2016-05-11 9:00 ` Adrian Hunter
2016-05-12 6:14 ` Adrian Hunter
2016-05-12 13:20 ` Ulf Hansson
2016-05-12 13:19 ` Adrian Hunter
2016-05-16 12:35 ` [PATCH V3 " Adrian Hunter
2016-05-17 15:06 ` Ulf Hansson
2016-05-18 6:44 ` Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 2/3] mmc: block: Always switch back to main area after RPMB access Adrian Hunter
2016-05-04 11:38 ` [PATCH V2 3/3] mmc: block: Pause re-tuning while switched to the RPMB partition Adrian Hunter
2016-05-04 11:54 ` [PATCH V2 0/3] mmc: block: Fix tuning (by avoiding it) for RPMB Winkler, Tomas
2016-05-10 10:28 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.