* [PATCHv2 09/10] mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, heiko-4mtYJXux2i+zQB+pC5nmwQ,
shawn.lin-TNX95d0MmH7DzftRWevZcw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
The "clock-freq-min-max" property was deprecated.
There is "max-frequency" property in drivers/mmc/core/host.c
"max-frequency" can be replaced with "clock-freq-min-max".
Minimum clock value might be set to 100K by default.
Then MMC core should try to find the correct value from 400K to 100K.
So it just needs to set Maximum clock value.
Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 3 ++-
drivers/mmc/host/dw_mmc.c | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index bfa461a..1279a22 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -59,8 +59,9 @@ Optional properties:
is specified and the ciu clock is specified then we'll try to set the ciu
clock to this at probe time.
-* clock-freq-min-max: Minimum and Maximum clock frequency for card output
+* clock-freq-min-max (DEPRECATED): Minimum and Maximum clock frequency for card output
clock(cclk_out). If it's not specified, max is 200MHZ and min is 400KHz by default.
+ (Use the "max-frequency" instead of "clock-freq-min-max".)
* num-slots: specifies the number of slots supported by the controller.
The number of physical slots actually used could be equal or less than the
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ec0ba79..48e968a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2608,6 +2608,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_min = DW_MCI_FREQ_MIN;
mmc->f_max = DW_MCI_FREQ_MAX;
} else {
+ dev_info(host->dev,
+ "'clock-freq-min-max' property was deprecated.\n");
mmc->f_min = freq[0];
mmc->f_max = freq[1];
}
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv2 10/10] Documentation: synopsys-dw-mshc: remove the unused properties
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, heiko-4mtYJXux2i+zQB+pC5nmwQ,
shawn.lin-TNX95d0MmH7DzftRWevZcw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
"support-highspeed" was the obsoleted property.
And "broken-cd" is not synopsys specific property.
It can be referred to mmc.txt binding Documentation.
Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 5 -----
1 file changed, 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 1279a22..7fd17c3 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -75,11 +75,6 @@ Optional properties:
* card-detect-delay: Delay in milli-seconds before detecting card after card
insert event. The default value is 0.
-* supports-highspeed (DEPRECATED): Enables support for high speed cards (up to 50MHz)
- (use "cap-mmc-highspeed" or "cap-sd-highspeed" instead)
-
-* broken-cd: as documented in mmc core bindings.
-
* vmmc-supply: The phandle to the regulator to use for vmmc. If this is
specified we'll defer probe until we can find this regulator.
--
2.10.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv2 00/10] mmc: dw_mmc: clean the codes for dwmmc controller
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
This patchset is modified the some minor fixing and cleaning codes.
* Major changes
- Use the cookie enum values like sdhci controller.
- Remove the unnecessary codes and use_stop_abort() by default.
- Remove the obsoleted property "supports-highspeed"
- Deprecated the "clock-freq-min-max" property. Instead, use "max-frequency"
- Minimum clock value is set to 100K by default.
Change in v2:
- Applied patches relevant to dt files
- Use "deperecated" instead of removing about "clock-freq-min-max"
- Added Heiko's Tested-by tag
Jaehoon Chung (10):
mmc: dw_mmc: display the real register value on debugfs
mmc: dw_mmc: fix the debug message for checking card's present
mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
mmc: dw_mmc: use the hold register when send stop command
mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
mmc: core: move the cookie's enum values from sdhci.h to mmc.h
mmc: dw_mmc: use the cookie's enum values for post/pre_req()
mmc: dw_mmc: remove the unnecessary mmc_data structure
mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
Documentation: synopsys-dw-mshc: remove the unused properties
.../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 +-
drivers/mmc/host/dw_mmc.c | 88 +++++++++++-----------
drivers/mmc/host/sdhci.h | 6 --
include/linux/mmc/core.h | 6 ++
4 files changed, 50 insertions(+), 58 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCHv2 02/10] mmc: dw_mmc: fix the debug message for checking card's present
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung@samsung.com>
If display the debug message, this message should be spamming.
If flags is maintained the previous value, didn't display the debug
message.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/mmc/host/dw_mmc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index e53899e..6c0c4c5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1537,13 +1537,10 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
== 0 ? 1 : 0;
spin_lock_bh(&host->lock);
- if (present) {
- set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
+ if (present && !test_and_set_bit(DW_MMC_CARD_PRESENT, &slot->flags))
dev_dbg(&mmc->class_dev, "card is present\n");
- } else {
- clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
+ else if (!test_and_clear_bit(DW_MMC_CARD_PRESENT, &slot->flags))
dev_dbg(&mmc->class_dev, "card is not present\n");
- }
spin_unlock_bh(&host->lock);
return present;
--
2.10.1
^ permalink raw reply related
* [PATCHv2 03/10] mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung@samsung.com>
If there is no property "clock-freq-min-max", mmc->f_min should be set
to 400K by default. But Some SoC can be used 100K.
When 100K is used, MMC core will try to check from 400K to 100K.
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/mmc/host/dw_mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 6c0c4c5..be36f48 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -54,7 +54,7 @@
#define DW_MCI_DMA_THRESHOLD 16
#define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
-#define DW_MCI_FREQ_MIN 400000 /* unit: HZ */
+#define DW_MCI_FREQ_MIN 100000 /* unit: HZ */
#define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
--
2.10.1
^ permalink raw reply related
* [PATCHv2 04/10] mmc: dw_mmc: use the hold register when send stop command
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung@samsung.com>
If DW_MMC_CARD_NO_USE_HOLD isn't set, it's usesd by default.
Enve if SDMMC_CMD_USB_HOLD_REG is set in prepare_command(), but it
doesn't set in pre_stop_abort().
To maintain the consistency, add the checking condition for this.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/mmc/host/dw_mmc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index be36f48..3cda68c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -337,6 +337,9 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci *host, struct mmc_command *cmd)
cmdr = stop->opcode | SDMMC_CMD_STOP |
SDMMC_CMD_RESP_CRC | SDMMC_CMD_RESP_EXP;
+ if (!test_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags))
+ cmdr |= SDMMC_CMD_USE_HOLD_REG;
+
return cmdr;
}
--
2.10.1
^ permalink raw reply related
* [PATCHv2 05/10] mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung@samsung.com>
stop_cmdr should be set to values relevant to stop command.
It migth be assigned to values whatever there is mrq->stop or not.
Then it doesn't need to use dw_mci_prepare_command().
It's enough to use the prep_stop_abort for preparing stop command.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/mmc/host/dw_mmc.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 3cda68c..12e1107 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -385,7 +385,7 @@ static void dw_mci_start_command(struct dw_mci *host,
static inline void send_stop_abort(struct dw_mci *host, struct mmc_data *data)
{
- struct mmc_command *stop = data->stop ? data->stop : &host->stop_abort;
+ struct mmc_command *stop = &host->stop_abort;
dw_mci_start_command(host, stop, host->stop_cmdr);
}
@@ -1277,10 +1277,7 @@ static void __dw_mci_start_request(struct dw_mci *host,
spin_unlock_irqrestore(&host->irq_lock, irqflags);
}
- if (mrq->stop)
- host->stop_cmdr = dw_mci_prepare_command(slot->mmc, mrq->stop);
- else
- host->stop_cmdr = dw_mci_prep_stop_abort(host, cmd);
+ host->stop_cmdr = dw_mci_prep_stop_abort(host, cmd);
}
static void dw_mci_start_request(struct dw_mci *host,
@@ -1890,8 +1887,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
if (test_and_clear_bit(EVENT_DATA_ERROR,
&host->pending_events)) {
dw_mci_stop_dma(host);
- if (data->stop ||
- !(host->data_status & (SDMMC_INT_DRTO |
+ if (!(host->data_status & (SDMMC_INT_DRTO |
SDMMC_INT_EBE)))
send_stop_abort(host, data);
state = STATE_DATA_ERROR;
@@ -1927,8 +1923,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
if (test_and_clear_bit(EVENT_DATA_ERROR,
&host->pending_events)) {
dw_mci_stop_dma(host);
- if (data->stop ||
- !(host->data_status & (SDMMC_INT_DRTO |
+ if (!(host->data_status & (SDMMC_INT_DRTO |
SDMMC_INT_EBE)))
send_stop_abort(host, data);
state = STATE_DATA_ERROR;
@@ -2004,7 +1999,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
host->cmd = NULL;
host->data = NULL;
- if (mrq->stop)
+ if (!mrq->sbc && mrq->stop)
dw_mci_command_complete(host, mrq->stop);
else
host->cmd_status = 0;
--
2.10.1
^ permalink raw reply related
* [PATCHv2 07/10] mmc: dw_mmc: use the cookie's enum values for post/pre_req()
From: Jaehoon Chung @ 2016-11-15 10:12 UTC (permalink / raw)
To: linux-mmc
Cc: devicetree, ulf.hansson, heiko, shawn.lin, robh+dt, Jaehoon Chung
In-Reply-To: <20161115101232.3854-1-jh80.chung@samsung.com>
This patch removed the meaningless value. Instead, use the cookie's enum
values for executing correctly.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/mmc/host/dw_mmc.c | 39 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 12e1107..e912de6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -414,12 +414,13 @@ static void dw_mci_dma_cleanup(struct dw_mci *host)
{
struct mmc_data *data = host->data;
- if (data)
- if (!data->host_cookie)
- dma_unmap_sg(host->dev,
- data->sg,
- data->sg_len,
- dw_mci_get_dma_dir(data));
+ if (data && data->host_cookie == COOKIE_MAPPED) {
+ dma_unmap_sg(host->dev,
+ data->sg,
+ data->sg_len,
+ dw_mci_get_dma_dir(data));
+ data->host_cookie = COOKIE_UNMAPPED;
+ }
}
static void dw_mci_idmac_reset(struct dw_mci *host)
@@ -850,13 +851,13 @@ static const struct dw_mci_dma_ops dw_mci_edmac_ops = {
static int dw_mci_pre_dma_transfer(struct dw_mci *host,
struct mmc_data *data,
- bool next)
+ int cookie)
{
struct scatterlist *sg;
unsigned int i, sg_len;
- if (!next && data->host_cookie)
- return data->host_cookie;
+ if (data->host_cookie == COOKIE_PRE_MAPPED)
+ return data->sg_len;
/*
* We don't do DMA on "complex" transfers, i.e. with
@@ -881,8 +882,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
if (sg_len == 0)
return -EINVAL;
- if (next)
- data->host_cookie = sg_len;
+ data->host_cookie = cookie;
return sg_len;
}
@@ -897,13 +897,12 @@ static void dw_mci_pre_req(struct mmc_host *mmc,
if (!slot->host->use_dma || !data)
return;
- if (data->host_cookie) {
- data->host_cookie = 0;
- return;
- }
+ /* This data might be unmapped at this time */
+ data->host_cookie = COOKIE_UNMAPPED;
- if (dw_mci_pre_dma_transfer(slot->host, mrq->data, 1) < 0)
- data->host_cookie = 0;
+ if (dw_mci_pre_dma_transfer(slot->host, mrq->data,
+ COOKIE_PRE_MAPPED) < 0)
+ data->host_cookie = COOKIE_UNMAPPED;
}
static void dw_mci_post_req(struct mmc_host *mmc,
@@ -916,12 +915,12 @@ static void dw_mci_post_req(struct mmc_host *mmc,
if (!slot->host->use_dma || !data)
return;
- if (data->host_cookie)
+ if (data->host_cookie != COOKIE_UNMAPPED)
dma_unmap_sg(slot->host->dev,
data->sg,
data->sg_len,
dw_mci_get_dma_dir(data));
- data->host_cookie = 0;
+ data->host_cookie = COOKIE_UNMAPPED;
}
static void dw_mci_adjust_fifoth(struct dw_mci *host, struct mmc_data *data)
@@ -1027,7 +1026,7 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data)
if (!host->use_dma)
return -ENODEV;
- sg_len = dw_mci_pre_dma_transfer(host, data, 0);
+ sg_len = dw_mci_pre_dma_transfer(host, data, COOKIE_MAPPED);
if (sg_len < 0) {
host->dma_ops->stop(host);
return sg_len;
--
2.10.1
^ permalink raw reply related
* [PATCH v4] mmc: sdhci-of-esdhc: fixup PRESENT_STATE read
From: Michael Walle @ 2016-11-15 10:13 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mmc, Ulf Hansson, Adrian Hunter, yangbo lu, Alexander Stein,
Michael Walle
Since commit 87a18a6a5652 ("mmc: mmc: Use ->card_busy() to detect busy
cards in __mmc_switch()") the ESDHC driver is broken:
mmc0: Card stuck in programming state! __mmc_switch
mmc0: error -110 whilst initialising MMC card
Since this commit __mmc_switch() uses ->card_busy(), which is
sdhci_card_busy() for the esdhc driver. sdhci_card_busy() uses the
PRESENT_STATE register, specifically the DAT0 signal level bit. But the
ESDHC uses a non-conformant PRESENT_STATE register, thus a read fixup is
required to make the driver work again.
Signed-off-by: Michael Walle <michael@walle.cc>
Fixes: 87a18a6a5652 ("mmc: mmc: Use ->card_busy() to detect busy cards in __mmc_switch()")
Acked-by: Yangbo Lu <yangbo.lu@nxp.com>
---
v4:
- add and use SDHCI_CMD_LVL macro
- fix typo in comment, thanks alex
- add acked-by
v3:
- explain the bits in the comments
- use bits[19:0] from the original value, all other will be taken from the
fixup value.
v2:
- use lower bits of the original value (that was actually a typo)
- add fixes tag
- fix typo
drivers/mmc/host/sdhci-of-esdhc.c | 14 ++++++++++++++
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index fb71c86..1bb11e4 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -66,6 +66,20 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host,
return ret;
}
}
+ /*
+ * The DAT[3:0] line signal levels and the CMD line signal level are
+ * not compatible with standard SDHC register. The line signal levels
+ * DAT[7:0] are at bits 31:24 and the command line signal level is at
+ * bit 23. All other bits are the same as in the standard SDHC
+ * register.
+ */
+ if (spec_reg == SDHCI_PRESENT_STATE) {
+ ret = value & 0x000fffff;
+ ret |= (value >> 4) & SDHCI_DATA_LVL_MASK;
+ ret |= (value << 1) & SDHCI_CMD_LVL;
+ return ret;
+ }
+
ret = value;
return ret;
}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c722cd2..ea38962 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -73,6 +73,7 @@
#define SDHCI_DATA_LVL_MASK 0x00F00000
#define SDHCI_DATA_LVL_SHIFT 20
#define SDHCI_DATA_0_LVL_MASK 0x00100000
+#define SDHCI_CMD_LVL 0x01000000
#define SDHCI_HOST_CONTROL 0x28
#define SDHCI_CTRL_LED 0x01
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v4] mmc: sdhci-of-esdhc: fixup PRESENT_STATE read
From: Adrian Hunter @ 2016-11-15 10:25 UTC (permalink / raw)
To: Michael Walle, linux-kernel
Cc: linux-mmc, Ulf Hansson, yangbo lu, Alexander Stein
In-Reply-To: <1479204796-23982-1-git-send-email-michael@walle.cc>
On 15/11/16 12:13, Michael Walle wrote:
> Since commit 87a18a6a5652 ("mmc: mmc: Use ->card_busy() to detect busy
> cards in __mmc_switch()") the ESDHC driver is broken:
> mmc0: Card stuck in programming state! __mmc_switch
> mmc0: error -110 whilst initialising MMC card
>
> Since this commit __mmc_switch() uses ->card_busy(), which is
> sdhci_card_busy() for the esdhc driver. sdhci_card_busy() uses the
> PRESENT_STATE register, specifically the DAT0 signal level bit. But the
> ESDHC uses a non-conformant PRESENT_STATE register, thus a read fixup is
> required to make the driver work again.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> Fixes: 87a18a6a5652 ("mmc: mmc: Use ->card_busy() to detect busy cards in __mmc_switch()")
> Acked-by: Yangbo Lu <yangbo.lu@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> v4:
> - add and use SDHCI_CMD_LVL macro
> - fix typo in comment, thanks alex
> - add acked-by
>
> v3:
> - explain the bits in the comments
> - use bits[19:0] from the original value, all other will be taken from the
> fixup value.
>
> v2:
> - use lower bits of the original value (that was actually a typo)
> - add fixes tag
> - fix typo
>
> drivers/mmc/host/sdhci-of-esdhc.c | 14 ++++++++++++++
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index fb71c86..1bb11e4 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -66,6 +66,20 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host,
> return ret;
> }
> }
> + /*
> + * The DAT[3:0] line signal levels and the CMD line signal level are
> + * not compatible with standard SDHC register. The line signal levels
> + * DAT[7:0] are at bits 31:24 and the command line signal level is at
> + * bit 23. All other bits are the same as in the standard SDHC
> + * register.
> + */
> + if (spec_reg == SDHCI_PRESENT_STATE) {
> + ret = value & 0x000fffff;
> + ret |= (value >> 4) & SDHCI_DATA_LVL_MASK;
> + ret |= (value << 1) & SDHCI_CMD_LVL;
> + return ret;
> + }
> +
> ret = value;
> return ret;
> }
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index c722cd2..ea38962 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -73,6 +73,7 @@
> #define SDHCI_DATA_LVL_MASK 0x00F00000
> #define SDHCI_DATA_LVL_SHIFT 20
> #define SDHCI_DATA_0_LVL_MASK 0x00100000
> +#define SDHCI_CMD_LVL 0x01000000
>
> #define SDHCI_HOST_CONTROL 0x28
> #define SDHCI_CTRL_LED 0x01
>
^ permalink raw reply
* Re: [RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI
From: Adrian Hunter @ 2016-11-15 13:20 UTC (permalink / raw)
To: Zach Brown; +Cc: Julia Cartwright, ulf.hansson, linux-mmc, linux-kernel
In-Reply-To: <20161111194952.GC16850@jcartwri.amer.corp.natinst.com>
On 11/11/16 21:49, Julia Cartwright wrote:
> On Wed, Nov 09, 2016 at 10:08:29AM -0600, Zach Brown wrote:
>> On Wed, Nov 09, 2016 at 03:24:24PM +0200, Adrian Hunter wrote:
>>> On 08/11/16 22:07, Zach Brown wrote:
>>>> On NI 9037 boards the max SDIO frequency is limited by trace lengths
>>>> and other layout choices. The max SDIO frequency is stored in an ACPI
>>>> table, as MXFQ.
>>>>
>>>> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
>>>> f_max field of the host with it.
>>>>
>>>> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
>>>> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
>>>> Reviewed-by: Josh Cartwright <joshc@ni.com>
>>>> Signed-off-by: Zach Brown <zach.brown@ni.com>
> [..]
>>>> static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>>>> {
>>>> +#ifdef CONFIG_ACPI
>>>> + /* Get max freq from ACPI for NI hardware */
>>>> + acpi_handle acpi_hdl;
>>>> + acpi_status status;
>>>> + struct acpi_buffer acpi_result = {
>>>> + ACPI_ALLOCATE_BUFFER, NULL };
>>>> + union acpi_object *acpi_buffer;
>>>> + int max_freq;
>>>> +
>>>> + status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
>>>> + &acpi_hdl);
>>>
>>> Is "MXFQ" an object that has already been deployed or are you inventing it
>>> now? In the latter case, did you consider device properties as an alternative?
>>>
>> "MXFQ" is an object that we have already deployed on some of our devices.
>
> Unfortunately, the whole ACPI device properties table discussion was
> just starting at the point where we were putting the firmware together
> for these devices :(. Had we engineered the firmware today, we would
> certainly have looked at using it.
No problem.
WRT the code, could acpi_evaluate_integer() be used instead of
acpi_get_handle()/acpi_evaluate_object()?
^ permalink raw reply
* [PATCH 1/2] MMC: davinci: use mmc_of_parse to parse common mmc configuration
From: Axel Haslam @ 2016-11-15 16:28 UTC (permalink / raw)
To: ulf.hansson, nsekhar, khilman, david; +Cc: linux-mmc, linux-kernel, Axel Haslam
In-Reply-To: <20161115162822.25791-1-ahaslam@baylibre.com>
Card detect and write protect are currently not working on a DT
boot, and the driver relies on polling to get the state
of the card. The current code depends on platform data callbacks
to register and get the state of the gpios.
mmc core provides a generic way to parse device tree configuration,
which will take care of registering the gpios for us, lets use it
so that we don't need to poll, and parse the same properties.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/mmc/host/davinci_mmc.c | 119 +++++++++++++++++++----------------------
1 file changed, 55 insertions(+), 64 deletions(-)
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 8fa478c..619e50e 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -35,6 +35,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/platform_data/mmc-davinci.h>
@@ -1029,9 +1030,10 @@ static int mmc_davinci_get_cd(struct mmc_host *mmc)
struct platform_device *pdev = to_platform_device(mmc->parent);
struct davinci_mmc_config *config = pdev->dev.platform_data;
- if (!config || !config->get_cd)
- return -ENOSYS;
- return config->get_cd(pdev->id);
+ if (config && config->get_cd)
+ return config->get_cd(pdev->id);
+
+ return mmc_gpio_get_cd(mmc);
}
static int mmc_davinci_get_ro(struct mmc_host *mmc)
@@ -1039,9 +1041,10 @@ static int mmc_davinci_get_ro(struct mmc_host *mmc)
struct platform_device *pdev = to_platform_device(mmc->parent);
struct davinci_mmc_config *config = pdev->dev.platform_data;
- if (!config || !config->get_ro)
- return -ENOSYS;
- return config->get_ro(pdev->id);
+ if (config && config->get_ro)
+ return config->get_ro(pdev->id);
+
+ return mmc_gpio_get_ro(mmc);
}
static void mmc_davinci_enable_sdio_irq(struct mmc_host *mmc, int enable)
@@ -1159,49 +1162,42 @@ static const struct of_device_id davinci_mmc_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, davinci_mmc_dt_ids);
-static struct davinci_mmc_config
- *mmc_parse_pdata(struct platform_device *pdev)
+static int mmc_davinci_parse_pdata(struct mmc_host *mmc)
{
- struct device_node *np;
+ struct platform_device *pdev = to_platform_device(mmc->parent);
struct davinci_mmc_config *pdata = pdev->dev.platform_data;
- const struct of_device_id *match =
- of_match_device(davinci_mmc_dt_ids, &pdev->dev);
- u32 data;
-
- np = pdev->dev.of_node;
- if (!np)
- return pdata;
-
- pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata) {
- dev_err(&pdev->dev, "Failed to allocate memory for struct davinci_mmc_config\n");
- goto nodata;
- }
+ struct mmc_davinci_host *host;
- if (match)
- pdev->id_entry = match->data;
+ if (!pdata)
+ return -EINVAL;
- if (of_property_read_u32(np, "max-frequency", &pdata->max_freq))
- dev_info(&pdev->dev, "'max-frequency' property not specified, defaulting to 25MHz\n");
+ host = mmc_priv(mmc);
+ if (!host)
+ return -EINVAL;
- of_property_read_u32(np, "bus-width", &data);
- switch (data) {
- case 1:
- case 4:
- case 8:
- pdata->wires = data;
- break;
- default:
- pdata->wires = 1;
- dev_info(&pdev->dev, "Unsupported buswidth, defaulting to 1 bit\n");
- }
-nodata:
- return pdata;
+ if (pdata && pdata->nr_sg)
+ host->nr_sg = pdata->nr_sg - 1;
+
+ if (pdata && (pdata->wires == 4 || pdata->wires == 0))
+ mmc->caps |= MMC_CAP_4_BIT_DATA;
+
+ if (pdata && (pdata->wires == 8))
+ mmc->caps |= (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
+
+ mmc->caps |= MMC_CAP_NEEDS_POLL;
+ mmc->f_min = 312500;
+ mmc->f_max = 25000000;
+ if (pdata && pdata->max_freq)
+ mmc->f_max = pdata->max_freq;
+ if (pdata && pdata->caps)
+ mmc->caps |= pdata->caps;
+
+ return 0;
}
static int __init davinci_mmcsd_probe(struct platform_device *pdev)
{
- struct davinci_mmc_config *pdata = NULL;
+ const struct of_device_id *match;
struct mmc_davinci_host *host = NULL;
struct mmc_host *mmc = NULL;
struct resource *r, *mem = NULL;
@@ -1209,12 +1205,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
size_t mem_size;
const struct platform_device_id *id_entry;
- pdata = mmc_parse_pdata(pdev);
- if (pdata == NULL) {
- dev_err(&pdev->dev, "Couldn't get platform data\n");
- return -ENOENT;
- }
-
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!r)
return -ENODEV;
@@ -1253,14 +1243,28 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
host->mmc_input_clk = clk_get_rate(host->clk);
- init_mmcsd_host(host);
-
- if (pdata->nr_sg)
- host->nr_sg = pdata->nr_sg - 1;
+ match = of_match_device(davinci_mmc_dt_ids, &pdev->dev);
+ if (match) {
+ pdev->id_entry = match->data;
+ ret = mmc_of_parse(mmc);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "could not parse of data: %d\n", ret);
+ goto parse_fail;
+ }
+ } else {
+ ret = mmc_davinci_parse_pdata(mmc);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "could not parse platform data: %d\n", ret);
+ goto parse_fail;
+ } }
if (host->nr_sg > MAX_NR_SG || !host->nr_sg)
host->nr_sg = MAX_NR_SG;
+ init_mmcsd_host(host);
+
host->use_dma = use_dma;
host->mmc_irq = irq;
host->sdio_irq = platform_get_irq(pdev, 1);
@@ -1273,27 +1277,13 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
host->use_dma = 0;
}
- /* REVISIT: someday, support IRQ-driven card detection. */
- mmc->caps |= MMC_CAP_NEEDS_POLL;
mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
- if (pdata && (pdata->wires == 4 || pdata->wires == 0))
- mmc->caps |= MMC_CAP_4_BIT_DATA;
-
- if (pdata && (pdata->wires == 8))
- mmc->caps |= (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
-
id_entry = platform_get_device_id(pdev);
if (id_entry)
host->version = id_entry->driver_data;
mmc->ops = &mmc_davinci_ops;
- mmc->f_min = 312500;
- mmc->f_max = 25000000;
- if (pdata && pdata->max_freq)
- mmc->f_max = pdata->max_freq;
- if (pdata && pdata->caps)
- mmc->caps |= pdata->caps;
mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
/* With no iommu coalescing pages, each phys_seg is a hw_seg.
@@ -1354,6 +1344,7 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
mmc_davinci_cpufreq_deregister(host);
cpu_freq_fail:
davinci_release_dma_channels(host);
+parse_fail:
dma_probe_defer:
clk_disable_unprepare(host->clk);
clk_prepare_enable_fail:
--
2.10.1
^ permalink raw reply related
* [PATCH 0/2] MMC: davinci: fix card detect and write protect
From: Axel Haslam @ 2016-11-15 16:28 UTC (permalink / raw)
To: ulf.hansson, nsekhar, khilman, david; +Cc: linux-mmc, linux-kernel, Axel Haslam
This series fixes the card detect and write protect parsing for
the davinci_mmc driver, and takes care of a technical debt to
remove card polling when a card detect gpio is available.
In the case of a platform based boot we register the gpios
using the APIs provided by slot-gpio.
In the case of a DT based boot we use the mmc_of_parse API to parse
all DT properties and register the gpios.
If this series is accepted, the next series will convert all users
to use gpio descriptors and we could then remove the platform
callbacks.
This was tested on the omap138-lcdk, and the da850-evm, with
additional patches to platform data and dts files.
Axel Haslam (2):
MMC: davinci: use mmc_of_parse to parse common mmc configuration
MMC: davinci: request gpios using gpio descriptors
drivers/mmc/host/davinci_mmc.c | 130 +++++++++++++++++++++--------------------
1 file changed, 66 insertions(+), 64 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCH 2/2] MMC: davinci: request gpios using gpio descriptors
From: Axel Haslam @ 2016-11-15 16:28 UTC (permalink / raw)
To: ulf.hansson, nsekhar, khilman, david; +Cc: linux-mmc, linux-kernel, Axel Haslam
In-Reply-To: <20161115162822.25791-1-ahaslam@baylibre.com>
Request card detect and write protect gpios using the provided API
by mmc core.
If a gpio is provided for card detect, we don't need to poll.
So only use polling when a gpio is not provided.
Once all pdata users register the gpios using gpio descriptors,
we could remove the platform callbacks.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/mmc/host/davinci_mmc.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 619e50e..36b5af8 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -1167,6 +1167,7 @@ static int mmc_davinci_parse_pdata(struct mmc_host *mmc)
struct platform_device *pdev = to_platform_device(mmc->parent);
struct davinci_mmc_config *pdata = pdev->dev.platform_data;
struct mmc_davinci_host *host;
+ int ret;
if (!pdata)
return -EINVAL;
@@ -1184,7 +1185,6 @@ static int mmc_davinci_parse_pdata(struct mmc_host *mmc)
if (pdata && (pdata->wires == 8))
mmc->caps |= (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
- mmc->caps |= MMC_CAP_NEEDS_POLL;
mmc->f_min = 312500;
mmc->f_max = 25000000;
if (pdata && pdata->max_freq)
@@ -1192,6 +1192,17 @@ static int mmc_davinci_parse_pdata(struct mmc_host *mmc)
if (pdata && pdata->caps)
mmc->caps |= pdata->caps;
+ /* Register a cd gpio, if there is not one, enable polling */
+ ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ else if (ret)
+ mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+ ret = mmc_gpiod_request_ro(mmc, "wp", 0, false, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+
return 0;
}
--
2.10.1
^ permalink raw reply related
* [RFC v2 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI
From: Zach Brown @ 2016-11-15 16:32 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
In-Reply-To: <1479227541-21201-1-git-send-email-zach.brown@ni.com>
Add PCI ID for Intel byt sdio host controller sub-vended by NI.
The controller has different behavior because of the board layout NI
puts it on.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci-pci-core.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1d9e00a..9741505 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
return 0;
}
+static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
+{
+ slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
+ MMC_CAP_WAIT_WHILE_BUSY;
+ return 0;
+}
+
static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
{
slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
@@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc = {
.ops = &sdhci_intel_byt_ops,
};
+static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
+ .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+ .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
+ SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+ .allow_runtime_pm = true,
+ .probe_slot = ni_byt_sdio_probe_slot,
+ .ops = &sdhci_intel_byt_ops,
+};
+
static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
.quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
.quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
@@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
+ .subvendor = PCI_VENDOR_ID_NI,
+ .subdevice = 0x7884,
+ .driver_data = (kernel_ulong_t)&sdhci_ni_byt_sdio,
+ },
+
+ {
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
.driver_data = (kernel_ulong_t)&sdhci_intel_byt_sdio,
--
2.7.4
^ permalink raw reply related
* [RFC v2 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Zach Brown @ 2016-11-15 16:32 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
In-Reply-To: <1479227541-21201-1-git-send-email-zach.brown@ni.com>
On NI 9037 boards the max SDIO frequency is limited by trace lengths
and other layout choices. The max SDIO frequency is stored in an ACPI
table.
The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
f_max field of the host.
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
Reviewed-by: Josh Cartwright <joshc@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci-pci-core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 9741505..4c31d16 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -27,6 +27,7 @@
#include <linux/pm_runtime.h>
#include <linux/mmc/slot-gpio.h>
#include <linux/mmc/sdhci-pci-data.h>
+#include <linux/acpi.h>
#include "sdhci.h"
#include "sdhci-pci.h"
@@ -377,6 +378,18 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
{
+#ifdef CONFIG_ACPI
+ /* Get max freq from ACPI for NI hardware */
+ acpi_status status;
+ unsigned long long max_freq;
+
+ status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
+ "MXFQ", NULL, &max_freq);
+ if (ACPI_FAILURE(status))
+ return -EINVAL;
+
+ slot->host->mmc->f_max = max_freq * 1000000;
+#endif
slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
MMC_CAP_WAIT_WHILE_BUSY;
return 0;
--
2.7.4
^ permalink raw reply related
* [RFC v2 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host controller
From: Zach Brown @ 2016-11-15 16:32 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
On some boards, max SDIO frequency is limited by trace lengths and other layout
choices. We would like a way to specify this limitation so the driver can
behave accordingly.
This patch set assumes that the limitation has been reported in an ACPI table
which the driver can check to get the max frequency.
The first patch creates a PCI ID and support for the Intel byt sdio where NI is
the subvendor.
The second patch uses the ACPI table to set f_max during the new
ni_byt_sdio_probe_slot.
v2:
* Use acpi_evaluate_integer() instead of
acpi_get_handle()/acpi_evaluate_object()
Zach Brown (2):
mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller
sub-vended by NI
mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio
controller sub-vended by NI
drivers/mmc/host/sdhci-pci-core.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Stephen Boyd @ 2016-11-15 19:27 UTC (permalink / raw)
To: Ritesh Harjani
Cc: adrian.hunter, ulf.hansson, linux-mmc, shawn.lin, andy.gross,
devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
david.griego, stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <3c1a7c72-0ac1-8ed0-87fc-238331f0645b@codeaurora.org>
On 11/15, Ritesh Harjani wrote:
> On 11/15/2016 1:07 AM, Stephen Boyd wrote:
> >On 11/14, Ritesh Harjani wrote:
> >
> >>+}
> >>+
> >>+/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
> >>+static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> >>+{
> >>+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >>+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> >>+ int rc;
> >>+
> >>+ if (!clock) {
> >>+ msm_host->clk_rate = clock;
> >>+ goto out;
> >>+ }
> >>+
> >>+ spin_unlock_irq(&host->lock);
> >>+ if (clock != msm_host->clk_rate) {
> >
> >Why do we need to check here? Can't we call clk_set_rate()
> >Unconditionally?
> Since it may so happen that above layers may call for ->set_clock
> function with same requested clock more than once, hence we cache
> the host->clock here.
> Also, since requested clock (host->clock) can be say 400Mhz but the
> actual pltfm supported clock would be say 384MHz.
clk_set_rate() detects the same rate being set even after it
internally rounds the rate. We're not going to touch the clk
hardware if 400 is requested once but 384 is what's set and then
400 is requested again. Caching the rate here in the driver can
lead to problems too if the driver is out of sync with the clk
hardware state, so it's best to avoid doing anything fancy here
and just let the framework handle duplicates.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Jaehoon Chung @ 2016-11-16 1:54 UTC (permalink / raw)
To: Shawn Lin, Jun Nie
Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu, linux-mmc
In-Reply-To: <c0392feb-a634-5e7a-8632-b5c0aa5b9f0a@rock-chips.com>
On 11/15/2016 04:49 PM, Shawn Lin wrote:
> On 2016/11/15 13:21, Jaehoon Chung wrote:
>> On 11/15/2016 09:48 AM, Shawn Lin wrote:
>>> On 2016/11/14 18:04, Jaehoon Chung wrote:
>>>> On 11/14/2016 07:00 PM, Jun Nie wrote:
>>>>> 2016-11-14 15:58 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
>>>>>> On 2016/11/8 9:24, Jun Nie wrote:
>>>>>>>
>>>>>>> Document the device-tree binding of ZTE MMC host on
>>>>>>> ZX296718 SoC.
>>>>>>>
>>>>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/mmc/zx-dw-mshc.txt | 35
>>>>>>> ++++++++++++++++++++++
>>>>>>> 1 file changed, 35 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..c175c4b
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>>>>> @@ -0,0 +1,35 @@
>>>>>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>>>>>> + Host Controller
>>>>>>> +
>>>>>>> +The Synopsys designware mobile storage host controller is used to
>>>>>>> interface
>>>>>>> +a SoC with storage medium such as eMMC or SD/MMC cards. This file
>>>>>>> documents
>>>>>>> +differences between the core Synopsys dw mshc controller properties
>>>>>>> described
>>>>>>> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
>>>>>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +
>>>>>>> +* compatible: should be
>>>>>>> + - "zte,zx296718-dw-mshc": for ZX SoCs
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +
>>>>>>> + mmc1: mmc@1110000 {
>>>>>>> + compatible = "zte,zx296718-dw-mshc";
>>>>>>> + #address-cells = <1>;
>>>>>>> + #size-cells = <0>;
>>>>>>> + reg = <0x01110000 0x1000>;
>>>>>>> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>>>>>> + fifo-depth = <32>;
>>>>>>> + data-addr = <0x200>;
>>>>>>> + fifo-watermark-aligned;
>>>>>>> + bus-width = <4>;
>>>>>>> + clock-frequency = <50000000>;
>>>>>>
>>>>>>
>>>>>> do you need both clock-frequency and max-frequency here?
>>>>>
>>>>> According to dts document, clock-frequency is for clock configuration
>>>>> when dw core probe. max-frequency is for mmc core to limit max
>>>>> frequency for any cards at any time. Do you have any suggestion? Thank
>>>>> you for your time!
>>>>
>>>> As i know, Jun's comment is right. :)
>>>> clock-frequency should be used with clk_set_rate().
>>>
>>> yup, I was thinking that should we reuse max-frequency instead of
>>> clock-frequency in the future? I saw most of the DT(I didn't check all
>>> of them) assign clock-frequency to the same value as max of clock-
>>> freq-min-max. I think it's pointless if the clock-frequency is different
>>> from max-frequency as both of them will be setted via dw_mmc and finally
>>> we only take min(clock-frequency, max-frquency). Was I missing
>>> something?
>>
>> Well, clock-frequency is for setting CMU. but max-frequency is not really used for CMU.
>> For example, clock-frequency is 100MHz. Max-frequency is 50MHz.
>> It's possible to use. then dwmmc controller should divide to 2 for max-frequency.
>
> yes, but why shouldn't we ask clock unit to generate max-frequency, so
> we don't need to divide 2 here?
>
>>
>> There are too many cases. Source clock can be 400MHz or 200MHz..etc..
>> but maximum clock is decided according to busmode.
>>
>> I'm not sure because i didn't have HW knowledge..
>> but in my experience,
>> 1) 400MHz/2 = 200MHz,
>> 2) 800MHz/4 = 200MHz.
>>
>> 1) and 2) are same value as 200MHz. but those clock phase might be a little difference.
>
> clock rate shouldn't be problem but the clock phase maybe according to
> the different clock design.
>
>>
>> So i want to keep the clock-frequency for setting the initial CMU value.
>> What your opinion? :)
>
> it' okay and I was just think that
> (1) most of the host drivers direct use max-frequenct(f_max) for
> setting clock source rate and use internal divider to generate other different rate. Also you can look at mmc_set_clock function.
> That impiles they doesn't care the phase or any differences at all.
> (2) If clock-frequency may be a little different with max-frequency,
> but I now check all the DT using clock-frequency with clock-freq-min-max
> before your cleanup and comfirm that all of the clock-frequency(s) are
> the same as the max of clock-freq-min-max, namely max-frequency after
> your cleanup. That implies that there are no difference between
> setting source rate with clock-frequency and with max-frequency.
>
>
> Anyway, that is just some random thoughts and shouldn't block this
> patchset. We could disscuss this topic later. :)
Yes, we can discuss about this in future. :)
Best Regards,
Jaehoon Chung
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> What is your opinion, Jaehoon and Jun? :)
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> + clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>>>>>> + clock-names = "biu", "ciu";
>>>>>>> + num-slots = <1>;
>>>>>>> + max-frequency = <50000000>;
>>>>>>> + cap-sdio-irq;
>>>>>>> + cap-sd-highspeed;
>>>>>>> + status = "disabled";
>>>>>>> + };
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best Regards
>>>>>> Shawn Lin
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@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 majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
^ permalink raw reply
* Re: [PATCH v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Ritesh Harjani @ 2016-11-16 4:42 UTC (permalink / raw)
To: Stephen Boyd, adrian.hunter, ulf.hansson
Cc: linux-mmc, shawn.lin, andy.gross, devicetree, linux-clk,
david.brown, linux-arm-msm, georgi.djakov, alex.lemberg,
mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman, david.griego,
stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <3c1a7c72-0ac1-8ed0-87fc-238331f0645b@codeaurora.org>
Hi,
On 11/15/2016 10:40 AM, Ritesh Harjani wrote:
> Hi Stephen/Adrian,
>
> On 11/15/2016 1:07 AM, Stephen Boyd wrote:
>> On 11/14, Ritesh Harjani wrote:
>>> @@ -577,6 +578,90 @@ static unsigned int
>>> sdhci_msm_get_min_clock(struct sdhci_host *host)
>>> return SDHCI_MSM_MIN_CLOCK;
>>> }
>>>
>>> +/**
>>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>>> + *
>>> + * Description:
>>> + * Implement MSM version of sdhci_set_clock.
>>> + * This is required since MSM controller does not
>>> + * use internal divider and instead directly control
>>> + * the GCC clock as per HW recommendation.
>>> + **/
>>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> + u16 clk;
>>> + unsigned long timeout;
>>> +
>>> + /*
>>> + * Keep actual_clock as zero -
>>> + * - since there is no divider used so no need of having
>>> actual_clock.
>>> + * - MSM controller uses SDCLK for data timeout calculation. If
>>> + * actual_clock is zero, host->clock is taken for calculation.
>>> + */
>>> + host->mmc->actual_clock = 0;
>>> +
>>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>> +
>>> + if (clock == 0)
>>> + return;
>>> +
>>> + /*
>>> + * MSM controller do not use clock divider.
>>> + * Thus read SDHCI_CLOCK_CONTROL and only enable
>>> + * clock with no divider value programmed.
>>> + */
>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> +
>>> + clk |= SDHCI_CLOCK_INT_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +
>>> + /* Wait max 20 ms */
>>> + timeout = 20;
>>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>>> + & SDHCI_CLOCK_INT_STABLE)) {
>>> + if (timeout == 0) {
>>> + pr_err("%s: Internal clock never stabilised\n",
>>> + mmc_hostname(host->mmc));
>>> + return;
>>> + }
>>> + timeout--;
>>> + mdelay(1);
>>> + }
>>> +
>>> + clk |= SDHCI_CLOCK_CARD_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>
>> This is almost a copy/paste of sdhci_set_clock(). Can we make
>> sdhci_set_clock() call a __sdhci_set_clock() function that takes
>> unsigned int clock, and also a flag indicating if we want to set
>> the internal clock divider or not? Then we can call
>> __sdhci_set_clock() from sdhci_set_clock() with (clock, true) as
>> arguments and (clock, false).
Actually what you may be referring here is some sort of quirks which is
not entertained any more for sdhci driver.
sdhci is tending towards becoming a library and hence such changes are
restricted.
But I think we may do below changes to avoid duplication of code which
enables the sdhci internal clock and waits for internal clock to be stable.
Adrian, could you please tell if this should be ok?
Then we may be able to call for sdhci_set_clock_enable function from
sdhci_msm_set_clock.
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 42ef3eb..28e605c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1343,19 +1343,8 @@ u16 sdhci_calc_clk(struct sdhci_host *host,
unsigned int clock,
}
EXPORT_SYMBOL_GPL(sdhci_calc_clk);
-void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+void sdhci_set_clock_enable(struct sdhci_host *host, unsigned short clk)
{
- u16 clk;
- unsigned long timeout;
-
- host->mmc->actual_clock = 0;
-
- sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
-
- if (clock == 0)
- return;
-
- clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
clk |= SDHCI_CLOCK_INT_EN;
sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
@@ -1377,6 +1366,24 @@ void sdhci_set_clock(struct sdhci_host *host,
unsigned int clock)
clk |= SDHCI_CLOCK_CARD_EN;
sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
}
+EXPORT_SYMBOL_GPL(sdhci_set_clock_enable);
+
+void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ u16 clk;
+ unsigned long timeout;
+
+ host->mmc->actual_clock = 0;
+
+ sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+
+ if (clock == 0)
+ return;
+
+ clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
+
+ sdhci_set_clock_enable(host, clk);
+}
EXPORT_SYMBOL_GPL(sdhci_set_clock);
static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char
mode,
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 766df17..43382e1 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -681,6 +681,7 @@ static inline bool sdhci_sdio_irq_enabled(struct
sdhci_host *host)
u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
unsigned int *actual_clock);
void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
+void sdhci_set_clock_enable(struct sdhci_host *host, unsigned short clk);
void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
unsigned short vdd);
void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
> Adrian,
> Could you please comment here ?
>
>>
>>> +}
>>> +
>>> +/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
>>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned
>>> int clock)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> + int rc;
>>> +
>>> + if (!clock) {
>>> + msm_host->clk_rate = clock;
>>> + goto out;
>>> + }
>>> +
>>> + spin_unlock_irq(&host->lock);
>>> + if (clock != msm_host->clk_rate) {
>>
>> Why do we need to check here? Can't we call clk_set_rate()
>> Unconditionally?
> Since it may so happen that above layers may call for ->set_clock
> function with same requested clock more than once, hence we cache the
> host->clock here.
> Also, since requested clock (host->clock) can be say 400Mhz but the
> actual pltfm supported clock would be say 384MHz.
>
>
>>
>>> + rc = clk_set_rate(msm_host->clk, clock);
>>> + if (rc) {
>>> + pr_err("%s: Failed to set clock at rate %u\n",
>>> + mmc_hostname(host->mmc), clock);
>>> + spin_lock_irq(&host->lock);
>>> + goto out;
>>
>> Or replace the above two lines with goto err;
> Ok, I will have another label out_lock instead of err.
>>
>>> + }
>>> + msm_host->clk_rate = clock;
>>> + pr_debug("%s: Setting clock at rate %lu\n",
>>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>>> + }
>>
>> And put an err label here.
> will put the label here as out_lock;
>>
>>> + spin_lock_irq(&host->lock);
>>> +out:
>>> + __sdhci_msm_set_clock(host, clock);
>>> +}
>>> +
>>> static const struct of_device_id sdhci_msm_dt_match[] = {
>>> { .compatible = "qcom,sdhci-msm-v4" },
>>> {},
>>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Adrian Hunter @ 2016-11-16 7:42 UTC (permalink / raw)
To: Ritesh Harjani, Stephen Boyd, ulf.hansson
Cc: linux-mmc, shawn.lin, andy.gross, devicetree, linux-clk,
david.brown, linux-arm-msm, georgi.djakov, alex.lemberg,
mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman, david.griego,
stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <d723b9fb-0cfc-e72b-ad78-36c8b996b012@codeaurora.org>
On 16/11/16 06:42, Ritesh Harjani wrote:
> Hi,
>
> On 11/15/2016 10:40 AM, Ritesh Harjani wrote:
>> Hi Stephen/Adrian,
>>
>> On 11/15/2016 1:07 AM, Stephen Boyd wrote:
>>> On 11/14, Ritesh Harjani wrote:
>>>> @@ -577,6 +578,90 @@ static unsigned int
>>>> sdhci_msm_get_min_clock(struct sdhci_host *host)
>>>> return SDHCI_MSM_MIN_CLOCK;
>>>> }
>>>>
>>>> +/**
>>>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>>>> + *
>>>> + * Description:
>>>> + * Implement MSM version of sdhci_set_clock.
>>>> + * This is required since MSM controller does not
>>>> + * use internal divider and instead directly control
>>>> + * the GCC clock as per HW recommendation.
>>>> + **/
>>>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>>> +{
>>>> + u16 clk;
>>>> + unsigned long timeout;
>>>> +
>>>> + /*
>>>> + * Keep actual_clock as zero -
>>>> + * - since there is no divider used so no need of having
>>>> actual_clock.
>>>> + * - MSM controller uses SDCLK for data timeout calculation. If
>>>> + * actual_clock is zero, host->clock is taken for calculation.
>>>> + */
>>>> + host->mmc->actual_clock = 0;
>>>> +
>>>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>>> +
>>>> + if (clock == 0)
>>>> + return;
>>>> +
>>>> + /*
>>>> + * MSM controller do not use clock divider.
>>>> + * Thus read SDHCI_CLOCK_CONTROL and only enable
>>>> + * clock with no divider value programmed.
>>>> + */
>>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>>> +
>>>> + clk |= SDHCI_CLOCK_INT_EN;
>>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>>> +
>>>> + /* Wait max 20 ms */
>>>> + timeout = 20;
>>>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>>>> + & SDHCI_CLOCK_INT_STABLE)) {
>>>> + if (timeout == 0) {
>>>> + pr_err("%s: Internal clock never stabilised\n",
>>>> + mmc_hostname(host->mmc));
>>>> + return;
>>>> + }
>>>> + timeout--;
>>>> + mdelay(1);
>>>> + }
>>>> +
>>>> + clk |= SDHCI_CLOCK_CARD_EN;
>>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>>
>>> This is almost a copy/paste of sdhci_set_clock(). Can we make
>>> sdhci_set_clock() call a __sdhci_set_clock() function that takes
>>> unsigned int clock, and also a flag indicating if we want to set
>>> the internal clock divider or not? Then we can call
>>> __sdhci_set_clock() from sdhci_set_clock() with (clock, true) as
>>> arguments and (clock, false).
> Actually what you may be referring here is some sort of quirks which is not
> entertained any more for sdhci driver.
> sdhci is tending towards becoming a library and hence such changes are
> restricted.
>
> But I think we may do below changes to avoid duplication of code which
> enables the sdhci internal clock and waits for internal clock to be stable.
>
> Adrian, could you please tell if this should be ok?
That seems fine, but the name seems too long - how about changing
sdhci_set_clock_enable to sdhci_enable_clk.
> Then we may be able to call for sdhci_set_clock_enable function from
> sdhci_msm_set_clock.
>
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 42ef3eb..28e605c 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1343,19 +1343,8 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned
> int clock,
> }
> EXPORT_SYMBOL_GPL(sdhci_calc_clk);
>
> -void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +void sdhci_set_clock_enable(struct sdhci_host *host, unsigned short clk)
> {
> - u16 clk;
> - unsigned long timeout;
> -
> - host->mmc->actual_clock = 0;
> -
> - sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> -
> - if (clock == 0)
> - return;
> -
> - clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
>
> clk |= SDHCI_CLOCK_INT_EN;
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> @@ -1377,6 +1366,24 @@ void sdhci_set_clock(struct sdhci_host *host,
> unsigned int clock)
> clk |= SDHCI_CLOCK_CARD_EN;
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> }
> +EXPORT_SYMBOL_GPL(sdhci_set_clock_enable);
> +
> +void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> + u16 clk;
> + unsigned long timeout;
> +
> + host->mmc->actual_clock = 0;
> +
> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> + if (clock == 0)
> + return;
> +
> + clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
> +
> + sdhci_set_clock_enable(host, clk);
> +}
> EXPORT_SYMBOL_GPL(sdhci_set_clock);
>
> static void sdhci_set_power_reg(struct sdhci_host *host, unsigned char mode,
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 766df17..43382e1 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -681,6 +681,7 @@ static inline bool sdhci_sdio_irq_enabled(struct
> sdhci_host *host)
> u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
> unsigned int *actual_clock);
> void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);
> +void sdhci_set_clock_enable(struct sdhci_host *host, unsigned short clk);
> void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
> unsigned short vdd);
> void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>
>
>
>> Adrian,
>> Could you please comment here ?
>>
>>>
>>>> +}
>>>> +
>>>> +/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
>>>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned
>>>> int clock)
>>>> +{
>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>> + int rc;
>>>> +
>>>> + if (!clock) {
>>>> + msm_host->clk_rate = clock;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + spin_unlock_irq(&host->lock);
>>>> + if (clock != msm_host->clk_rate) {
>>>
>>> Why do we need to check here? Can't we call clk_set_rate()
>>> Unconditionally?
>> Since it may so happen that above layers may call for ->set_clock
>> function with same requested clock more than once, hence we cache the
>> host->clock here.
>> Also, since requested clock (host->clock) can be say 400Mhz but the
>> actual pltfm supported clock would be say 384MHz.
>>
>>
>>>
>>>> + rc = clk_set_rate(msm_host->clk, clock);
>>>> + if (rc) {
>>>> + pr_err("%s: Failed to set clock at rate %u\n",
>>>> + mmc_hostname(host->mmc), clock);
>>>> + spin_lock_irq(&host->lock);
>>>> + goto out;
>>>
>>> Or replace the above two lines with goto err;
>> Ok, I will have another label out_lock instead of err.
>>>
>>>> + }
>>>> + msm_host->clk_rate = clock;
>>>> + pr_debug("%s: Setting clock at rate %lu\n",
>>>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>>>> + }
>>>
>>> And put an err label here.
>> will put the label here as out_lock;
>>>
>>>> + spin_lock_irq(&host->lock);
>>>> +out:
>>>> + __sdhci_msm_set_clock(host, clock);
>>>> +}
>>>> +
>>>> static const struct of_device_id sdhci_msm_dt_match[] = {
>>>> { .compatible = "qcom,sdhci-msm-v4" },
>>>> {},
>>>
>>
>
^ permalink raw reply
* Re: [PATCHv2 06/10] mmc: core: move the cookie's enum values from sdhci.h to mmc.h
From: Jaehoon Chung @ 2016-11-16 7:53 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson,
heiko-4mtYJXux2i+zQB+pC5nmwQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Adrian Hunter
In-Reply-To: <20161115101232.3854-7-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Added Adrian for sdhci.h
On 11/15/2016 07:12 PM, Jaehoon Chung wrote:
> It's not for only sdhci controller.
> So it can be moved from sdhci.h to mmc.h. And renamed from sdhci_cookie
> to mmc_cookie.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
> drivers/mmc/host/sdhci.h | 6 ------
> include/linux/mmc/core.h | 6 ++++++
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 766df17..325663b 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -321,12 +321,6 @@ struct sdhci_adma2_64_desc {
> /* Allow for a a command request and a data request at the same time */
> #define SDHCI_MAX_MRQS 2
>
> -enum sdhci_cookie {
> - COOKIE_UNMAPPED,
> - COOKIE_PRE_MAPPED, /* mapped by sdhci_pre_req() */
> - COOKIE_MAPPED, /* mapped by sdhci_prepare_data() */
> -};
> -
> struct sdhci_host {
> /* Data set by hardware interface driver */
> const char *hw_name; /* Hardware bus name */
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 0ce928b..82d707f 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -118,6 +118,12 @@ struct mmc_command {
> struct mmc_request *mrq; /* associated request */
> };
>
> +enum mmc_cookie {
> + COOKIE_UNMAPPED,
> + COOKIE_PRE_MAPPED, /* mapped by pre_req() of controller */
> + COOKIE_MAPPED, /* mapped by prepare_data() of controller */
> +};
> +
> struct mmc_data {
> unsigned int timeout_ns; /* data timeout (in ns, max 80ms) */
> unsigned int timeout_clks; /* data timeout (in clocks) */
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2 06/10] mmc: core: move the cookie's enum values from sdhci.h to mmc.h
From: Adrian Hunter @ 2016-11-16 8:09 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson,
heiko-4mtYJXux2i+zQB+pC5nmwQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <12405f58-695d-03d8-407f-d7471e93a6de-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On 16/11/16 09:53, Jaehoon Chung wrote:
> Added Adrian for sdhci.h
>
> On 11/15/2016 07:12 PM, Jaehoon Chung wrote:
>> It's not for only sdhci controller.
>> So it can be moved from sdhci.h to mmc.h. And renamed from sdhci_cookie
>> to mmc_cookie.
The cookie is currently host private data, so I don't understand the
motivation behind this.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>> ---
>> drivers/mmc/host/sdhci.h | 6 ------
>> include/linux/mmc/core.h | 6 ++++++
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 766df17..325663b 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -321,12 +321,6 @@ struct sdhci_adma2_64_desc {
>> /* Allow for a a command request and a data request at the same time */
>> #define SDHCI_MAX_MRQS 2
>>
>> -enum sdhci_cookie {
>> - COOKIE_UNMAPPED,
>> - COOKIE_PRE_MAPPED, /* mapped by sdhci_pre_req() */
>> - COOKIE_MAPPED, /* mapped by sdhci_prepare_data() */
>> -};
>> -
>> struct sdhci_host {
>> /* Data set by hardware interface driver */
>> const char *hw_name; /* Hardware bus name */
>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>> index 0ce928b..82d707f 100644
>> --- a/include/linux/mmc/core.h
>> +++ b/include/linux/mmc/core.h
>> @@ -118,6 +118,12 @@ struct mmc_command {
>> struct mmc_request *mrq; /* associated request */
>> };
>>
>> +enum mmc_cookie {
>> + COOKIE_UNMAPPED,
>> + COOKIE_PRE_MAPPED, /* mapped by pre_req() of controller */
>> + COOKIE_MAPPED, /* mapped by prepare_data() of controller */
>> +};
>> +
>> struct mmc_data {
>> unsigned int timeout_ns; /* data timeout (in ns, max 80ms) */
>> unsigned int timeout_clks; /* data timeout (in clocks) */
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2 06/10] mmc: core: move the cookie's enum values from sdhci.h to mmc.h
From: Adrian Hunter @ 2016-11-16 8:28 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson,
heiko-4mtYJXux2i+zQB+pC5nmwQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <89535076-4a77-64a5-68d9-ce15e03b8fb3-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On 16/11/16 10:25, Jaehoon Chung wrote:
> On 11/16/2016 05:09 PM, Adrian Hunter wrote:
>> On 16/11/16 09:53, Jaehoon Chung wrote:
>>> Added Adrian for sdhci.h
>>>
>>> On 11/15/2016 07:12 PM, Jaehoon Chung wrote:
>>>> It's not for only sdhci controller.
>>>> So it can be moved from sdhci.h to mmc.h. And renamed from sdhci_cookie
>>>> to mmc_cookie.
>>
>> The cookie is currently host private data, so I don't understand the
>> motivation behind this.
>
> dwmmc controller can also use the data->host_cookie. because it's working with post/pre_req().
>
> So i think it can be used about both sdhci and dwmmc.
> Is there no reason that add the private dwmmc data?
>
> With these cookie value, update the dwmmc controller for post/pre_req().
>
> https://patchwork.kernel.org/patch/9429287/
So why not define dwmmc cookies in dw_mmc.c ?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2 06/10] mmc: core: move the cookie's enum values from sdhci.h to mmc.h
From: Jaehoon Chung @ 2016-11-16 8:25 UTC (permalink / raw)
To: Adrian Hunter, linux-mmc
Cc: devicetree, Ulf Hansson, heiko, shawn.lin, robh+dt
In-Reply-To: <ecb4b5d0-4cbf-bf44-3287-f1580b2e990a@intel.com>
On 11/16/2016 05:09 PM, Adrian Hunter wrote:
> On 16/11/16 09:53, Jaehoon Chung wrote:
>> Added Adrian for sdhci.h
>>
>> On 11/15/2016 07:12 PM, Jaehoon Chung wrote:
>>> It's not for only sdhci controller.
>>> So it can be moved from sdhci.h to mmc.h. And renamed from sdhci_cookie
>>> to mmc_cookie.
>
> The cookie is currently host private data, so I don't understand the
> motivation behind this.
dwmmc controller can also use the data->host_cookie. because it's working with post/pre_req().
So i think it can be used about both sdhci and dwmmc.
Is there no reason that add the private dwmmc data?
With these cookie value, update the dwmmc controller for post/pre_req().
https://patchwork.kernel.org/patch/9429287/
Best Regards,
Jaehoon Chung
>
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>> drivers/mmc/host/sdhci.h | 6 ------
>>> include/linux/mmc/core.h | 6 ++++++
>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 766df17..325663b 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -321,12 +321,6 @@ struct sdhci_adma2_64_desc {
>>> /* Allow for a a command request and a data request at the same time */
>>> #define SDHCI_MAX_MRQS 2
>>>
>>> -enum sdhci_cookie {
>>> - COOKIE_UNMAPPED,
>>> - COOKIE_PRE_MAPPED, /* mapped by sdhci_pre_req() */
>>> - COOKIE_MAPPED, /* mapped by sdhci_prepare_data() */
>>> -};
>>> -
>>> struct sdhci_host {
>>> /* Data set by hardware interface driver */
>>> const char *hw_name; /* Hardware bus name */
>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>>> index 0ce928b..82d707f 100644
>>> --- a/include/linux/mmc/core.h
>>> +++ b/include/linux/mmc/core.h
>>> @@ -118,6 +118,12 @@ struct mmc_command {
>>> struct mmc_request *mrq; /* associated request */
>>> };
>>>
>>> +enum mmc_cookie {
>>> + COOKIE_UNMAPPED,
>>> + COOKIE_PRE_MAPPED, /* mapped by pre_req() of controller */
>>> + COOKIE_MAPPED, /* mapped by prepare_data() of controller */
>>> +};
>>> +
>>> struct mmc_data {
>>> unsigned int timeout_ns; /* data timeout (in ns, max 80ms) */
>>> unsigned int timeout_clks; /* data timeout (in clocks) */
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox