* [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree
@ 2018-11-20 20:06 Liming Sun
2018-12-04 14:19 ` Ulf Hansson
2018-12-04 16:26 ` Alex Lemberg
0 siblings, 2 replies; 4+ messages in thread
From: Liming Sun @ 2018-11-20 20:06 UTC (permalink / raw)
To: Ulf Hansson, Jaehoon Chung, David Woods, Shawn Lin
Cc: Liming Sun, linux-mmc, linux-kernel
Currently the EXT_CSD_CACHE_CTRL register is controlled by API
mmc_card_broken_hpi(), which only works for the quirks settings.
This commit enhances it to use card->ext_csd.hpi instead. This
flag works for both the quirks and the broken-hpi configuration
in the device tree.
Signed-off-by: Liming Sun <lsun@mellanox.com>
---
drivers/mmc/core/mmc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bc1bd2c..2f2b65c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
* If cache size is higher than 0, this indicates
* the existence of cache and it can be turned on.
*/
- if (!mmc_card_broken_hpi(card) &&
- card->ext_csd.cache_size > 0) {
+ if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_CACHE_CTRL, 1,
card->ext_csd.generic_cmd6_time);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree
2018-11-20 20:06 [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree Liming Sun
@ 2018-12-04 14:19 ` Ulf Hansson
2018-12-04 16:26 ` Alex Lemberg
1 sibling, 0 replies; 4+ messages in thread
From: Ulf Hansson @ 2018-12-04 14:19 UTC (permalink / raw)
To: Liming Sun
Cc: Jaehoon Chung, David Woods, Shawn Lin, linux-mmc,
Linux Kernel Mailing List
On Tue, 20 Nov 2018 at 21:06, Liming Sun <lsun@mellanox.com> wrote:
>
> Currently the EXT_CSD_CACHE_CTRL register is controlled by API
> mmc_card_broken_hpi(), which only works for the quirks settings.
> This commit enhances it to use card->ext_csd.hpi instead. This
> flag works for both the quirks and the broken-hpi configuration
> in the device tree.
>
> Signed-off-by: Liming Sun <lsun@mellanox.com>
I realize that the code around the HPI support needs some additional
love. Let me post some patches in a day or two, then you can have a
look a let me know your thoughts.
> ---
> drivers/mmc/core/mmc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index bc1bd2c..2f2b65c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> * If cache size is higher than 0, this indicates
> * the existence of cache and it can be turned on.
> */
> - if (!mmc_card_broken_hpi(card) &&
> - card->ext_csd.cache_size > 0) {
> + if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {
Actually, I don't see a reason to why we should limit to use the
cache, to whether HPI is supported or not.
I guess the thinking behind this, was that we wanted an option to
abort the cache flush as there is no timeout available for that
operation. However, I don't see whenever we should allow a cache flush
to be aborted, as it would introduce a potential option of getting
corrupt data on the flash.
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_CACHE_CTRL, 1,
> card->ext_csd.generic_cmd6_time);
> --
> 1.8.3.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree
2018-11-20 20:06 [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree Liming Sun
2018-12-04 14:19 ` Ulf Hansson
@ 2018-12-04 16:26 ` Alex Lemberg
2018-12-04 22:19 ` Liming Sun
1 sibling, 1 reply; 4+ messages in thread
From: Alex Lemberg @ 2018-12-04 16:26 UTC (permalink / raw)
To: Liming Sun, Ulf Hansson, Jaehoon Chung, David Woods, Shawn Lin
Cc: linux-mmc, linux-kernel
Hi,
On 11/20/18, 10:07 PM, "linux-mmc-owner@vger.kernel.org on behalf of Liming Sun" <linux-mmc-owner@vger.kernel.org on behalf of lsun@mellanox.com> wrote:
Currently the EXT_CSD_CACHE_CTRL register is controlled by API
mmc_card_broken_hpi(), which only works for the quirks settings.
This commit enhances it to use card->ext_csd.hpi instead. This
flag works for both the quirks and the broken-hpi configuration
in the device tree.
Signed-off-by: Liming Sun <lsun@mellanox.com>
---
drivers/mmc/core/mmc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bc1bd2c..2f2b65c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
* If cache size is higher than 0, this indicates
* the existence of cache and it can be turned on.
*/
- if (!mmc_card_broken_hpi(card) &&
- card->ext_csd.cache_size > 0) {
+ if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {
Any reason for making a connection between HPI and Cache
in current driver implementation?
BTW, I missed it before, but the previous commit making this connection -
is also doesn't make any sense.
Although by the spec FLUSH_CACHE is interruptible command, I don't think
we should prevent devices with no HPI support, using the Cache.
Please correct me if I wrong, but the current driver implementation allows
to send HPI for BKOPS and Sanitize (on timeout error) commands only -
so why we should limit the FLUSH_CACHE command?
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_CACHE_CTRL, 1,
card->ext_csd.generic_cmd6_time);
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree
2018-12-04 16:26 ` Alex Lemberg
@ 2018-12-04 22:19 ` Liming Sun
0 siblings, 0 replies; 4+ messages in thread
From: Liming Sun @ 2018-12-04 22:19 UTC (permalink / raw)
To: Alex Lemberg, Ulf Hansson, Jaehoon Chung, David Woods, Shawn Lin
Cc: linux-mmc, linux-kernel
Thanks for the comment. Yes, I actually have the same question.
The change here is to fix a condition based on the existing code. Probably a better fix is to get rid of the 'connection'...
-----Original Message-----
From: Alex Lemberg <Alex.Lemberg@wdc.com>
Sent: Tuesday, December 4, 2018 11:27 AM
To: Liming Sun <lsun@mellanox.com>; Ulf Hansson <ulf.hansson@linaro.org>; Jaehoon Chung <jh80.chung@samsung.com>; David Woods <dwoods@mellanox.com>; Shawn Lin <shawn.lin@rock-chips.com>
Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree
Hi,
On 11/20/18, 10:07 PM, "linux-mmc-owner@vger.kernel.org on behalf of Liming Sun" <linux-mmc-owner@vger.kernel.org on behalf of lsun@mellanox.com> wrote:
Currently the EXT_CSD_CACHE_CTRL register is controlled by API
mmc_card_broken_hpi(), which only works for the quirks settings.
This commit enhances it to use card->ext_csd.hpi instead. This
flag works for both the quirks and the broken-hpi configuration
in the device tree.
Signed-off-by: Liming Sun <lsun@mellanox.com>
---
drivers/mmc/core/mmc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bc1bd2c..2f2b65c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
* If cache size is higher than 0, this indicates
* the existence of cache and it can be turned on.
*/
- if (!mmc_card_broken_hpi(card) &&
- card->ext_csd.cache_size > 0) {
+ if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {
Any reason for making a connection between HPI and Cache in current driver implementation?
BTW, I missed it before, but the previous commit making this connection - is also doesn't make any sense.
Although by the spec FLUSH_CACHE is interruptible command, I don't think we should prevent devices with no HPI support, using the Cache.
Please correct me if I wrong, but the current driver implementation allows to send HPI for BKOPS and Sanitize (on timeout error) commands only - so why we should limit the FLUSH_CACHE command?
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_CACHE_CTRL, 1,
card->ext_csd.generic_cmd6_time);
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-04 22:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20 20:06 [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree Liming Sun
2018-12-04 14:19 ` Ulf Hansson
2018-12-04 16:26 ` Alex Lemberg
2018-12-04 22:19 ` Liming Sun
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.