* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
@ 2014-11-11 15:47 Barry Song
2014-11-12 9:35 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-11-11 15:47 UTC (permalink / raw)
To: linux-arm-kernel
From: Minda Chen <Minda.Chen@csr.com>
Add manual tuning function in CSR atlas7 SoC. It is mainly used
for the UHS-I SD card working SDR50 SDR104 mode.
The tuning principle can be seen in SD spec part1 v3.01 4.2.4.5
(tuning command).
SD host send the cmd19 and set the delay value(0-127).
and the sdcard return 64 bytes data. If the data is same with
the tuning data. The delay value is valid. Execute this commmand
128 times. And calculate the longest window of the valid values.
The value in the middle of this window is the best value.
Signed-off-by: Minda Chen <Minda.Chen@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
drivers/mmc/host/sdhci-sirf.c | 105 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 104 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
index dd29d47..c71cf4e 100644
--- a/drivers/mmc/host/sdhci-sirf.c
+++ b/drivers/mmc/host/sdhci-sirf.c
@@ -8,14 +8,18 @@
#include <linux/delay.h>
#include <linux/device.h>
-#include <linux/mmc/host.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/slab.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
#include <linux/mmc/slot-gpio.h>
#include "sdhci-pltfm.h"
+#define SDHCI_CLK_DELAY_SETTING 0x4C
#define SDHCI_SIRF_8BITBUS BIT(3)
+#define SIRF_TUNING_COUNT 128
struct sdhci_sirf_priv {
struct clk *clk;
@@ -49,7 +53,106 @@ static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
}
+static int sdhci_sirf_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+ int tuning_seq_cnt = 3;
+ u8 phase, *data_buf, tuned_phases[SIRF_TUNING_COUNT];
+ u8 tuned_phase_cnt = 0;
+ const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
+ int size = sizeof(tuning_blk_pattern_4bit);
+ int rc, longest_range = 0;
+ int start = -1, end, tuning_value = -1, range = 0;
+ u16 clock_setting;
+ struct mmc_host *mmc = host->mmc;
+
+ data_buf = kmalloc(size, GFP_KERNEL);
+ if (!data_buf)
+ return -ENOMEM;
+
+ clock_setting = sdhci_readw(host, SDHCI_CLK_DELAY_SETTING);
+ clock_setting &= ~0x3fff;
+
+retry:
+ phase = 0;
+ do {
+ struct mmc_command cmd = { 0 };
+ struct mmc_data data = { 0 };
+ struct mmc_request mrq = {
+ .cmd = &cmd,
+ .data = &data
+ };
+ struct scatterlist sg;
+
+ sdhci_writel(host,
+ clock_setting | phase | (phase << 7) | (phase << 16),
+ SDHCI_CLK_DELAY_SETTING);
+
+ cmd.opcode = opcode;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+
+ data.blksz = size;
+ data.blocks = 1;
+ data.flags = MMC_DATA_READ;
+ data.timeout_ns = NSEC_PER_SEC; /* 1 second */
+
+ data.sg = &sg;
+ data.sg_len = 1;
+ sg_init_one(&sg, data_buf, size);
+ memset(data_buf, 0, size);
+ mmc_wait_for_req(mmc, &mrq);
+
+ if (!cmd.error && !data.error &&
+ !memcmp(data_buf, tuning_block_pattern, size)) {
+ /* Tuning is successful@this tuning point */
+ tuned_phases[tuned_phase_cnt++] = phase;
+ dev_dbg(mmc_dev(mmc), "%s: Found good phase = %d\n",
+ mmc_hostname(mmc), phase);
+ if (start == -1)
+ start = phase;
+ end = phase;
+ range++;
+ if (phase == (SIRF_TUNING_COUNT - 1)
+ && range > longest_range)
+ tuning_value = (start + end) / 2;
+ } else {
+ dev_dbg(mmc_dev(mmc), "%s: Found bad phase = %d\n",
+ mmc_hostname(mmc), phase);
+ if (range > longest_range) {
+ tuning_value = (start + end) / 2;
+ longest_range = range;
+ }
+ start = -1;
+ end = range = 0;
+ }
+ } while (++phase < ARRAY_SIZE(tuned_phases));
+
+ if (tuned_phase_cnt && tuning_value > 0) {
+ /*
+ * Finally set the selected phase in delay
+ * line hw block.
+ */
+ phase = tuning_value;
+ sdhci_writel(host,
+ clock_setting | phase | (phase << 7) | (phase << 16),
+ SDHCI_CLK_DELAY_SETTING);
+
+ dev_dbg(mmc_dev(mmc), "%s: Setting the tuning phase to %d\n",
+ mmc_hostname(mmc), phase);
+ } else {
+ if (--tuning_seq_cnt)
+ goto retry;
+ /* Tuning failed */
+ dev_dbg(mmc_dev(mmc), "%s: No tuning point found\n",
+ mmc_hostname(mmc));
+ rc = -EIO;
+ }
+
+ kfree(data_buf);
+ return rc;
+}
+
static struct sdhci_ops sdhci_sirf_ops = {
+ .platform_execute_tuning = sdhci_sirf_execute_tuning,
.set_clock = sdhci_set_clock,
.get_max_clock = sdhci_sirf_get_max_clk,
.set_bus_width = sdhci_sirf_set_bus_width,
--
2.1.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
2014-11-11 15:47 [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19) Barry Song
@ 2014-11-12 9:35 ` Ulf Hansson
2014-11-17 9:30 ` Barry Song
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2014-11-12 9:35 UTC (permalink / raw)
To: linux-arm-kernel
On 11 November 2014 16:47, Barry Song <21cnbao@gmail.com> wrote:
> From: Minda Chen <Minda.Chen@csr.com>
>
> Add manual tuning function in CSR atlas7 SoC. It is mainly used
> for the UHS-I SD card working SDR50 SDR104 mode.
>
> The tuning principle can be seen in SD spec part1 v3.01 4.2.4.5
> (tuning command).
>
> SD host send the cmd19 and set the delay value(0-127).
> and the sdcard return 64 bytes data. If the data is same with
> the tuning data. The delay value is valid. Execute this commmand
> 128 times. And calculate the longest window of the valid values.
> The value in the middle of this window is the best value.
>
> Signed-off-by: Minda Chen <Minda.Chen@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> drivers/mmc/host/sdhci-sirf.c | 105 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
> index dd29d47..c71cf4e 100644
> --- a/drivers/mmc/host/sdhci-sirf.c
> +++ b/drivers/mmc/host/sdhci-sirf.c
> @@ -8,14 +8,18 @@
>
> #include <linux/delay.h>
> #include <linux/device.h>
> -#include <linux/mmc/host.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> +#include <linux/slab.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> #include <linux/mmc/slot-gpio.h>
> #include "sdhci-pltfm.h"
>
> +#define SDHCI_CLK_DELAY_SETTING 0x4C
> #define SDHCI_SIRF_8BITBUS BIT(3)
> +#define SIRF_TUNING_COUNT 128
>
> struct sdhci_sirf_priv {
> struct clk *clk;
> @@ -49,7 +53,106 @@ static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> }
>
> +static int sdhci_sirf_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> + int tuning_seq_cnt = 3;
> + u8 phase, *data_buf, tuned_phases[SIRF_TUNING_COUNT];
> + u8 tuned_phase_cnt = 0;
> + const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
> + int size = sizeof(tuning_blk_pattern_4bit);
> + int rc, longest_range = 0;
> + int start = -1, end, tuning_value = -1, range = 0;
> + u16 clock_setting;
> + struct mmc_host *mmc = host->mmc;
> +
> + data_buf = kmalloc(size, GFP_KERNEL);
> + if (!data_buf)
> + return -ENOMEM;
> +
> + clock_setting = sdhci_readw(host, SDHCI_CLK_DELAY_SETTING);
> + clock_setting &= ~0x3fff;
> +
> +retry:
> + phase = 0;
> + do {
> + struct mmc_command cmd = { 0 };
> + struct mmc_data data = { 0 };
> + struct mmc_request mrq = {
> + .cmd = &cmd,
> + .data = &data
> + };
> + struct scatterlist sg;
> +
> + sdhci_writel(host,
> + clock_setting | phase | (phase << 7) | (phase << 16),
> + SDHCI_CLK_DELAY_SETTING);
> +
> + cmd.opcode = opcode;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> + data.blksz = size;
> + data.blocks = 1;
> + data.flags = MMC_DATA_READ;
> + data.timeout_ns = NSEC_PER_SEC; /* 1 second */
> +
> + data.sg = &sg;
> + data.sg_len = 1;
> + sg_init_one(&sg, data_buf, size);
> + memset(data_buf, 0, size);
> + mmc_wait_for_req(mmc, &mrq);
These piece of code, which sends the tuning command don't belong in
the host driver. Instead I would like this to be handled from a helper
function from the mmc core.
I realize that there already a few existing host drivers that
implemented similar code and I don't like it. We should convert them
to use a common helper function from the core instead.
> +
> + if (!cmd.error && !data.error &&
> + !memcmp(data_buf, tuning_block_pattern, size)) {
> + /* Tuning is successful at this tuning point */
> + tuned_phases[tuned_phase_cnt++] = phase;
> + dev_dbg(mmc_dev(mmc), "%s: Found good phase = %d\n",
> + mmc_hostname(mmc), phase);
> + if (start == -1)
> + start = phase;
> + end = phase;
> + range++;
> + if (phase == (SIRF_TUNING_COUNT - 1)
> + && range > longest_range)
> + tuning_value = (start + end) / 2;
> + } else {
> + dev_dbg(mmc_dev(mmc), "%s: Found bad phase = %d\n",
> + mmc_hostname(mmc), phase);
> + if (range > longest_range) {
> + tuning_value = (start + end) / 2;
> + longest_range = range;
> + }
> + start = -1;
> + end = range = 0;
> + }
> + } while (++phase < ARRAY_SIZE(tuned_phases));
> +
> + if (tuned_phase_cnt && tuning_value > 0) {
> + /*
> + * Finally set the selected phase in delay
> + * line hw block.
> + */
> + phase = tuning_value;
> + sdhci_writel(host,
> + clock_setting | phase | (phase << 7) | (phase << 16),
> + SDHCI_CLK_DELAY_SETTING);
> +
> + dev_dbg(mmc_dev(mmc), "%s: Setting the tuning phase to %d\n",
> + mmc_hostname(mmc), phase);
> + } else {
> + if (--tuning_seq_cnt)
> + goto retry;
> + /* Tuning failed */
> + dev_dbg(mmc_dev(mmc), "%s: No tuning point found\n",
> + mmc_hostname(mmc));
> + rc = -EIO;
> + }
> +
> + kfree(data_buf);
> + return rc;
> +}
> +
> static struct sdhci_ops sdhci_sirf_ops = {
> + .platform_execute_tuning = sdhci_sirf_execute_tuning,
> .set_clock = sdhci_set_clock,
> .get_max_clock = sdhci_sirf_get_max_clk,
> .set_bus_width = sdhci_sirf_set_bus_width,
> --
> 2.1.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
2014-11-12 9:35 ` Ulf Hansson
@ 2014-11-17 9:30 ` Barry Song
2014-11-17 13:54 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-11-17 9:30 UTC (permalink / raw)
To: linux-arm-kernel
2014-11-12 17:35 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 11 November 2014 16:47, Barry Song <21cnbao@gmail.com> wrote:
>> From: Minda Chen <Minda.Chen@csr.com>
>>
>> Add manual tuning function in CSR atlas7 SoC. It is mainly used
>> for the UHS-I SD card working SDR50 SDR104 mode.
>>
>> The tuning principle can be seen in SD spec part1 v3.01 4.2.4.5
>> (tuning command).
>>
>> SD host send the cmd19 and set the delay value(0-127).
>> and the sdcard return 64 bytes data. If the data is same with
>> the tuning data. The delay value is valid. Execute this commmand
>> 128 times. And calculate the longest window of the valid values.
>> The value in the middle of this window is the best value.
>>
>> Signed-off-by: Minda Chen <Minda.Chen@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> drivers/mmc/host/sdhci-sirf.c | 105 +++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 104 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
>> index dd29d47..c71cf4e 100644
>> --- a/drivers/mmc/host/sdhci-sirf.c
>> +++ b/drivers/mmc/host/sdhci-sirf.c
>> @@ -8,14 +8,18 @@
>>
>> #include <linux/delay.h>
>> #include <linux/device.h>
>> -#include <linux/mmc/host.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_gpio.h>
>> +#include <linux/slab.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/mmc.h>
>> #include <linux/mmc/slot-gpio.h>
>> #include "sdhci-pltfm.h"
>>
>> +#define SDHCI_CLK_DELAY_SETTING 0x4C
>> #define SDHCI_SIRF_8BITBUS BIT(3)
>> +#define SIRF_TUNING_COUNT 128
>>
>> struct sdhci_sirf_priv {
>> struct clk *clk;
>> @@ -49,7 +53,106 @@ static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
>> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>> }
>>
>> +static int sdhci_sirf_execute_tuning(struct sdhci_host *host, u32 opcode)
>> +{
>> + int tuning_seq_cnt = 3;
>> + u8 phase, *data_buf, tuned_phases[SIRF_TUNING_COUNT];
>> + u8 tuned_phase_cnt = 0;
>> + const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
>> + int size = sizeof(tuning_blk_pattern_4bit);
>> + int rc, longest_range = 0;
>> + int start = -1, end, tuning_value = -1, range = 0;
>> + u16 clock_setting;
>> + struct mmc_host *mmc = host->mmc;
>> +
>> + data_buf = kmalloc(size, GFP_KERNEL);
>> + if (!data_buf)
>> + return -ENOMEM;
>> +
>> + clock_setting = sdhci_readw(host, SDHCI_CLK_DELAY_SETTING);
>> + clock_setting &= ~0x3fff;
>> +
>> +retry:
>> + phase = 0;
>> + do {
>> + struct mmc_command cmd = { 0 };
>> + struct mmc_data data = { 0 };
>> + struct mmc_request mrq = {
>> + .cmd = &cmd,
>> + .data = &data
>> + };
>> + struct scatterlist sg;
>> +
>> + sdhci_writel(host,
>> + clock_setting | phase | (phase << 7) | (phase << 16),
>> + SDHCI_CLK_DELAY_SETTING);
>> +
>> + cmd.opcode = opcode;
>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>> +
>> + data.blksz = size;
>> + data.blocks = 1;
>> + data.flags = MMC_DATA_READ;
>> + data.timeout_ns = NSEC_PER_SEC; /* 1 second */
>> +
>> + data.sg = &sg;
>> + data.sg_len = 1;
>> + sg_init_one(&sg, data_buf, size);
>> + memset(data_buf, 0, size);
>> + mmc_wait_for_req(mmc, &mrq);
>
> These piece of code, which sends the tuning command don't belong in
> the host driver. Instead I would like this to be handled from a helper
> function from the mmc core.
>
> I realize that there already a few existing host drivers that
> implemented similar code and I don't like it. We should convert them
> to use a common helper function from the core instead.
yes. that is fine.
our engineering is currently very busy and might have no time to
handle this for this moment.
do you think we can move your requirement to a new refine task of next
window after you pick up this in this window?
-barry
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
2014-11-17 9:30 ` Barry Song
@ 2014-11-17 13:54 ` Ulf Hansson
2014-11-20 9:25 ` Barry Song
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2014-11-17 13:54 UTC (permalink / raw)
To: linux-arm-kernel
On 17 November 2014 10:30, Barry Song <21cnbao@gmail.com> wrote:
> 2014-11-12 17:35 GMT+08:00 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 11 November 2014 16:47, Barry Song <21cnbao@gmail.com> wrote:
>>> From: Minda Chen <Minda.Chen@csr.com>
>>>
>>> Add manual tuning function in CSR atlas7 SoC. It is mainly used
>>> for the UHS-I SD card working SDR50 SDR104 mode.
>>>
>>> The tuning principle can be seen in SD spec part1 v3.01 4.2.4.5
>>> (tuning command).
>>>
>>> SD host send the cmd19 and set the delay value(0-127).
>>> and the sdcard return 64 bytes data. If the data is same with
>>> the tuning data. The delay value is valid. Execute this commmand
>>> 128 times. And calculate the longest window of the valid values.
>>> The value in the middle of this window is the best value.
>>>
>>> Signed-off-by: Minda Chen <Minda.Chen@csr.com>
>>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>>> ---
>>> drivers/mmc/host/sdhci-sirf.c | 105 +++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 104 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-sirf.c b/drivers/mmc/host/sdhci-sirf.c
>>> index dd29d47..c71cf4e 100644
>>> --- a/drivers/mmc/host/sdhci-sirf.c
>>> +++ b/drivers/mmc/host/sdhci-sirf.c
>>> @@ -8,14 +8,18 @@
>>>
>>> #include <linux/delay.h>
>>> #include <linux/device.h>
>>> -#include <linux/mmc/host.h>
>>> #include <linux/module.h>
>>> #include <linux/of.h>
>>> #include <linux/of_gpio.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/mmc/host.h>
>>> +#include <linux/mmc/mmc.h>
>>> #include <linux/mmc/slot-gpio.h>
>>> #include "sdhci-pltfm.h"
>>>
>>> +#define SDHCI_CLK_DELAY_SETTING 0x4C
>>> #define SDHCI_SIRF_8BITBUS BIT(3)
>>> +#define SIRF_TUNING_COUNT 128
>>>
>>> struct sdhci_sirf_priv {
>>> struct clk *clk;
>>> @@ -49,7 +53,106 @@ static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
>>> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
>>> }
>>>
>>> +static int sdhci_sirf_execute_tuning(struct sdhci_host *host, u32 opcode)
>>> +{
>>> + int tuning_seq_cnt = 3;
>>> + u8 phase, *data_buf, tuned_phases[SIRF_TUNING_COUNT];
>>> + u8 tuned_phase_cnt = 0;
>>> + const u8 *tuning_block_pattern = tuning_blk_pattern_4bit;
>>> + int size = sizeof(tuning_blk_pattern_4bit);
>>> + int rc, longest_range = 0;
>>> + int start = -1, end, tuning_value = -1, range = 0;
>>> + u16 clock_setting;
>>> + struct mmc_host *mmc = host->mmc;
>>> +
>>> + data_buf = kmalloc(size, GFP_KERNEL);
>>> + if (!data_buf)
>>> + return -ENOMEM;
>>> +
>>> + clock_setting = sdhci_readw(host, SDHCI_CLK_DELAY_SETTING);
>>> + clock_setting &= ~0x3fff;
>>> +
>>> +retry:
>>> + phase = 0;
>>> + do {
>>> + struct mmc_command cmd = { 0 };
>>> + struct mmc_data data = { 0 };
>>> + struct mmc_request mrq = {
>>> + .cmd = &cmd,
>>> + .data = &data
>>> + };
>>> + struct scatterlist sg;
>>> +
>>> + sdhci_writel(host,
>>> + clock_setting | phase | (phase << 7) | (phase << 16),
>>> + SDHCI_CLK_DELAY_SETTING);
>>> +
>>> + cmd.opcode = opcode;
>>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>>> +
>>> + data.blksz = size;
>>> + data.blocks = 1;
>>> + data.flags = MMC_DATA_READ;
>>> + data.timeout_ns = NSEC_PER_SEC; /* 1 second */
>>> +
>>> + data.sg = &sg;
>>> + data.sg_len = 1;
>>> + sg_init_one(&sg, data_buf, size);
>>> + memset(data_buf, 0, size);
>>> + mmc_wait_for_req(mmc, &mrq);
>>
>> These piece of code, which sends the tuning command don't belong in
>> the host driver. Instead I would like this to be handled from a helper
>> function from the mmc core.
>>
>> I realize that there already a few existing host drivers that
>> implemented similar code and I don't like it. We should convert them
>> to use a common helper function from the core instead.
> yes. that is fine.
>
> our engineering is currently very busy and might have no time to
> handle this for this moment.
> do you think we can move your requirement to a new refine task of next
> window after you pick up this in this window?
No.
You don't have to adopt the other host drivers at this point, only to
fix yours accordingly. The rest we can take care of later.
Br
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
2014-11-17 13:54 ` Ulf Hansson
@ 2014-11-20 9:25 ` Barry Song
2014-11-21 11:17 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Barry Song @ 2014-11-20 9:25 UTC (permalink / raw)
To: linux-arm-kernel
>>> These piece of code, which sends the tuning command don't belong in
>>> the host driver. Instead I would like this to be handled from a helper
>>> function from the mmc core.
>>>
>>> I realize that there already a few existing host drivers that
>>> implemented similar code and I don't like it. We should convert them
>>> to use a common helper function from the core instead.
>> yes. that is fine.
>>
>> our engineering is currently very busy and might have no time to
>> handle this for this moment.
>> do you think we can move your requirement to a new refine task of next
>> window after you pick up this in this window?
>
> No.
>
> You don't have to adopt the other host drivers at this point, only to
> fix yours accordingly. The rest we can take care of later.
>
Uffe, how do you think about we provide a core-level function at first:
>From 3f47dfd577ef3ff8c41dbb44d71d383a9e620ada Mon Sep 17 00:00:00 2001
From: Minda Chen <Minda.Chen@csr.com>
Date: Thu, 20 Nov 2014 10:42:41 +0800
Subject: [PATCH] mmc: ops: Add mmc_send_tuning function
According to the SD card spec, Add a manual
tuning command function for SDR104/HS200
Sending command 19 or command 21 to read data
and compare with the tunning block.
Change-Id: Ic481b78b1e247c88942788ff0d10746779f672c0
Signed-off-by: Minda Chen <Minda.Chen@csr.com>
---
drivers/mmc/core/mmc_ops.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/core.h | 1 +
2 files changed, 66 insertions(+)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 7911e05..ecc7789 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -543,6 +543,71 @@ int mmc_switch(struct mmc_card *card, u8 set, u8
index, u8 value,
}
EXPORT_SYMBOL_GPL(mmc_switch);
+int mmc_send_tuning(struct mmc_card *card, u32 opcode)
+{
+ struct mmc_request mrq = {NULL};
+ struct mmc_command cmd = {0};
+ struct mmc_data data = {0};
+ struct scatterlist sg;
+ struct mmc_host *mmc = card->host;
+ struct mmc_ios *ios = &mmc->ios;
+ const u8 *tuning_block_pattern;
+ int size, err = 0;
+ u8 *data_buf;
+
+ if (opcode == MMC_SEND_TUNING_BLOCK_HS200) {
+ if (ios->bus_width == MMC_BUS_WIDTH_8) {
+ tuning_block_pattern = tuning_blk_pattern_8bit;
+ size = sizeof(tuning_blk_pattern_8bit);
+ } else if (ios->bus_width == MMC_BUS_WIDTH_4) {
+ tuning_block_pattern = tuning_blk_pattern_4bit;
+ size = sizeof(tuning_blk_pattern_4bit);
+ } else
+ return -EINVAL;
+ } else if (opcode == MMC_SEND_TUNING_BLOCK) {
+ tuning_block_pattern = tuning_blk_pattern_4bit;
+ size = sizeof(tuning_blk_pattern_4bit);
+ } else
+ return -EINVAL;
+
+ data_buf = kmalloc(size, GFP_KERNEL);
+ if (!data_buf)
+ return -ENOMEM;
+
+ mrq.cmd = &cmd;
+ mrq.data = &data;
+
+ cmd.opcode = opcode;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+
+ data.blksz = size;
+ data.blocks = 1;
+ data.flags = MMC_DATA_READ;
+
+ mmc_set_data_timeout(&data, card);
+ sg_init_one(&sg, data_buf, size);
+ memset(data_buf, 0, size);
+ mmc_wait_for_req(mmc, &mrq);
+
+ if (cmd.error) {
+ err = cmd.error;
+ goto out;
+ }
+
+ if (data.error) {
+ err = data.error;
+ goto out;
+ }
+
+ if (memcmp(data_buf, tuning_block_pattern, size))
+ err = -EIO;
+
+out:
+ kfree(data_buf);
+ return err;
+}
+EXPORT_SYMBOL_GPL(mmc_send_tuning);
+
static int
mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
u8 len)
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index f206e29..82a0119 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -154,6 +154,7 @@ extern void mmc_start_bkops(struct mmc_card *card,
bool from_exception);
extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, bool,
bool, bool);
extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
+extern int mmc_send_tuning(struct mmc_card *, u32);
extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
#define MMC_ERASE_ARG 0x00000000
--
1.9.1
> Br
> Uffe
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19)
2014-11-20 9:25 ` Barry Song
@ 2014-11-21 11:17 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2014-11-21 11:17 UTC (permalink / raw)
To: linux-arm-kernel
On 20 November 2014 10:25, Barry Song <21cnbao@gmail.com> wrote:
>>>> These piece of code, which sends the tuning command don't belong in
>>>> the host driver. Instead I would like this to be handled from a helper
>>>> function from the mmc core.
>>>>
>>>> I realize that there already a few existing host drivers that
>>>> implemented similar code and I don't like it. We should convert them
>>>> to use a common helper function from the core instead.
>>> yes. that is fine.
>>>
>>> our engineering is currently very busy and might have no time to
>>> handle this for this moment.
>>> do you think we can move your requirement to a new refine task of next
>>> window after you pick up this in this window?
>>
>> No.
>>
>> You don't have to adopt the other host drivers at this point, only to
>> fix yours accordingly. The rest we can take care of later.
>>
>
> Uffe, how do you think about we provide a core-level function at first:
That's exactly what I had in mind. :-) Please resend as a separate patch.
Kind regards
Uffe
>
>
> From 3f47dfd577ef3ff8c41dbb44d71d383a9e620ada Mon Sep 17 00:00:00 2001
> From: Minda Chen <Minda.Chen@csr.com>
> Date: Thu, 20 Nov 2014 10:42:41 +0800
> Subject: [PATCH] mmc: ops: Add mmc_send_tuning function
>
> According to the SD card spec, Add a manual
> tuning command function for SDR104/HS200
> Sending command 19 or command 21 to read data
> and compare with the tunning block.
>
> Change-Id: Ic481b78b1e247c88942788ff0d10746779f672c0
> Signed-off-by: Minda Chen <Minda.Chen@csr.com>
> ---
> drivers/mmc/core/mmc_ops.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mmc/core.h | 1 +
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 7911e05..ecc7789 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -543,6 +543,71 @@ int mmc_switch(struct mmc_card *card, u8 set, u8
> index, u8 value,
> }
> EXPORT_SYMBOL_GPL(mmc_switch);
>
> +int mmc_send_tuning(struct mmc_card *card, u32 opcode)
> +{
> + struct mmc_request mrq = {NULL};
> + struct mmc_command cmd = {0};
> + struct mmc_data data = {0};
> + struct scatterlist sg;
> + struct mmc_host *mmc = card->host;
> + struct mmc_ios *ios = &mmc->ios;
> + const u8 *tuning_block_pattern;
> + int size, err = 0;
> + u8 *data_buf;
> +
> + if (opcode == MMC_SEND_TUNING_BLOCK_HS200) {
> + if (ios->bus_width == MMC_BUS_WIDTH_8) {
> + tuning_block_pattern = tuning_blk_pattern_8bit;
> + size = sizeof(tuning_blk_pattern_8bit);
> + } else if (ios->bus_width == MMC_BUS_WIDTH_4) {
> + tuning_block_pattern = tuning_blk_pattern_4bit;
> + size = sizeof(tuning_blk_pattern_4bit);
> + } else
> + return -EINVAL;
> + } else if (opcode == MMC_SEND_TUNING_BLOCK) {
> + tuning_block_pattern = tuning_blk_pattern_4bit;
> + size = sizeof(tuning_blk_pattern_4bit);
> + } else
> + return -EINVAL;
> +
> + data_buf = kmalloc(size, GFP_KERNEL);
> + if (!data_buf)
> + return -ENOMEM;
> +
> + mrq.cmd = &cmd;
> + mrq.data = &data;
> +
> + cmd.opcode = opcode;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> + data.blksz = size;
> + data.blocks = 1;
> + data.flags = MMC_DATA_READ;
> +
> + mmc_set_data_timeout(&data, card);
> + sg_init_one(&sg, data_buf, size);
> + memset(data_buf, 0, size);
> + mmc_wait_for_req(mmc, &mrq);
> +
> + if (cmd.error) {
> + err = cmd.error;
> + goto out;
> + }
> +
> + if (data.error) {
> + err = data.error;
> + goto out;
> + }
> +
> + if (memcmp(data_buf, tuning_block_pattern, size))
> + err = -EIO;
> +
> +out:
> + kfree(data_buf);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(mmc_send_tuning);
> +
> static int
> mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
> u8 len)
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index f206e29..82a0119 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -154,6 +154,7 @@ extern void mmc_start_bkops(struct mmc_card *card,
> bool from_exception);
> extern int __mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int, bool,
> bool, bool);
> extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
> +extern int mmc_send_tuning(struct mmc_card *, u32);
> extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
>
> #define MMC_ERASE_ARG 0x00000000
> --
> 1.9.1
>
>
>> Br
>> Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-21 11:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-11 15:47 [PATCH] mmc: sdhci-sirf: add sirf tuning function (cmd 19) Barry Song
2014-11-12 9:35 ` Ulf Hansson
2014-11-17 9:30 ` Barry Song
2014-11-17 13:54 ` Ulf Hansson
2014-11-20 9:25 ` Barry Song
2014-11-21 11:17 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).