2014-07-05 12:59:59

by addy ke

[permalink] [raw]
Subject: [PATCH] mmc: dw_mmc: add support for RK3288.

This patch focuses on clock setting for RK3288 mmc controller.

In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
and if DDR 8bit mode, CLKDIV register must be set 1.

Signed-off-by: addy ke <[email protected]>
---
.../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index c559f3f..e3f95cd 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
Required Properties:

* compatible: should be
- - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
+ - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
+ before RK3288
+ - "rockchip,rk3288-dw-mshc": for Rockchip RK3288

Example:

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index d4a47a9..15d796e 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -21,17 +21,61 @@
#include <linux/mmc/mmc.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/of.h>
+#include <linux/clk.h>

#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"

+#define RK3288_CLKGEN_DIV 2
+
static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
{
*cmdr |= SDMMC_CMD_USE_HOLD_REG;
}

-static const struct dw_mci_drv_data rockchip_drv_data = {
+static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
+{
+ host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+
+ return 0;
+}
+
+static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+ int ret;
+ unsigned int cclkin;
+
+ /*
+ * cclkin: source clock of mmc controller.
+ * bus_hz: card interface clock generated by CLKGEN.
+ * bus_hz = cclkin / RK3288_CLKGEN_DIV;
+ * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
+ *
+ * Note: div can only be 0 or 1
+ * if DDR50 8bit mode, div must be set 1
+ */
+ if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
+ (ios->timing == MMC_TIMING_UHS_DDR50 ||
+ ios->timing == MMC_TIMING_MMC_DDR52))
+ cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
+ else
+ cclkin = ios->clock * RK3288_CLKGEN_DIV;
+
+ ret = clk_set_rate(host->ciu_clk, cclkin);
+ if (ret)
+ dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+
+ host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+}
+
+static const struct dw_mci_drv_data rk2928_drv_data = {
+ .prepare_command = dw_mci_pltfm_prepare_command,
+};
+
+static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_pltfm_prepare_command,
+ .set_ios = dw_mci_rk3288_set_ios,
+ .setup_clock = dw_mci_rk3288_setup_clock,
};

static const struct dw_mci_drv_data socfpga_drv_data = {
@@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
static const struct of_device_id dw_mci_pltfm_match[] = {
{ .compatible = "snps,dw-mshc", },
{ .compatible = "rockchip,rk2928-dw-mshc",
- .data = &rockchip_drv_data },
+ .data = &rk2928_drv_data },
+ { .compatible = "rockchip,rk3288-dw-mshc",
+ .data = &rk3288_drv_data },
{ .compatible = "altr,socfpga-dw-mshc",
.data = &socfpga_drv_data },
{},
--
1.8.3.2


2014-07-07 02:07:16

by Jaehoon Chung

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: add support for RK3288.

Hi, Addy,

On 07/05/2014 09:59 PM, addy ke wrote:
> This patch focuses on clock setting for RK3288 mmc controller.
>
> In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
> and if DDR 8bit mode, CLKDIV register must be set 1.
>
> Signed-off-by: addy ke <[email protected]>
> ---
> .../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
> drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
> 2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> index c559f3f..e3f95cd 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> @@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
> Required Properties:
>
> * compatible: should be
> - - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
> + - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
> + before RK3288
> + - "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>
> Example:
>
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index d4a47a9..15d796e 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -21,17 +21,61 @@
> #include <linux/mmc/mmc.h>
> #include <linux/mmc/dw_mmc.h>
> #include <linux/of.h>
> +#include <linux/clk.h>
>
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
>
> +#define RK3288_CLKGEN_DIV 2
"2" is used to the general div value at rockchip?

> +
> static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
> {
> *cmdr |= SDMMC_CMD_USE_HOLD_REG;
> }
>
> -static const struct dw_mci_drv_data rockchip_drv_data = {
> +static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
> +{
> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
I knew that you need not to call clk_get_rate(). In dw-mmc.c, it's already called.
So you can just use the host->bus_hz.

host->bus_hz /= RK3288_CLKGEN_DIV;

Best Regards,
Jaehoon Chung

> +
> + return 0;
> +}
> +
> +static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> + int ret;
> + unsigned int cclkin;
> +
> + /*
> + * cclkin: source clock of mmc controller.
> + * bus_hz: card interface clock generated by CLKGEN.
> + * bus_hz = cclkin / RK3288_CLKGEN_DIV;
> + * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
> + *
> + * Note: div can only be 0 or 1
> + * if DDR50 8bit mode, div must be set 1
> + */
> + if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
> + (ios->timing == MMC_TIMING_UHS_DDR50 ||
> + ios->timing == MMC_TIMING_MMC_DDR52))
> + cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
> + else
> + cclkin = ios->clock * RK3288_CLKGEN_DIV;
> +
> + ret = clk_set_rate(host->ciu_clk, cclkin);
> + if (ret)
> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> +
> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> +}
> +
> +static const struct dw_mci_drv_data rk2928_drv_data = {
> + .prepare_command = dw_mci_pltfm_prepare_command,
> +};
> +
> +static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_pltfm_prepare_command,
> + .set_ios = dw_mci_rk3288_set_ios,
> + .setup_clock = dw_mci_rk3288_setup_clock,
> };
>
> static const struct dw_mci_drv_data socfpga_drv_data = {
> @@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
> static const struct of_device_id dw_mci_pltfm_match[] = {
> { .compatible = "snps,dw-mshc", },
> { .compatible = "rockchip,rk2928-dw-mshc",
> - .data = &rockchip_drv_data },
> + .data = &rk2928_drv_data },
> + { .compatible = "rockchip,rk3288-dw-mshc",
> + .data = &rk3288_drv_data },
> { .compatible = "altr,socfpga-dw-mshc",
> .data = &socfpga_drv_data },
> {},
>

2014-07-07 02:42:47

by addy ke

[permalink] [raw]
Subject: Re: [PATCH] mmc: dw_mmc: add support for RK3288.

> Hi, Addy,
>
> On 07/05/2014 09:59 PM, addy ke wrote:
>> This patch focuses on clock setting for RK3288 mmc controller.
>>
>> In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
>> and if DDR 8bit mode, CLKDIV register must be set 1.
>>
>> Signed-off-by: addy ke <[email protected]>
>> ---
>> .../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
>> drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
>> 2 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> index c559f3f..e3f95cd 100644
>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> @@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
>> Required Properties:
>>
>> * compatible: should be
>> - - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
>> + - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
>> + before RK3288
>> + - "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>>
>> Example:
>>
>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>> index d4a47a9..15d796e 100644
>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>> @@ -21,17 +21,61 @@
>> #include <linux/mmc/mmc.h>
>> #include <linux/mmc/dw_mmc.h>
>> #include <linux/of.h>
>> +#include <linux/clk.h>
>>
>> #include "dw_mmc.h"
>> #include "dw_mmc-pltfm.h"
>>
>> +#define RK3288_CLKGEN_DIV 2
> "2" is used to the general div value at rockchip?
>
Yes, In RK3288, the div generated by CLKGEN is 2 and can not be changed by software.
>> +
>> static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
>> {
>> *cmdr |= SDMMC_CMD_USE_HOLD_REG;
>> }
>>
>> -static const struct dw_mci_drv_data rockchip_drv_data = {
>> +static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
>> +{
>> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> I knew that you need not to call clk_get_rate(). In dw-mmc.c, it's already called.
> So you can just use the host->bus_hz.
>
> host->bus_hz /= RK3288_CLKGEN_DIV;
>
> Best Regards,
> Jaehoon Chung
>
>> +
>> + return 0;
>> +}
>> +
>> +static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> + int ret;
>> + unsigned int cclkin;
>> +
>> + /*
>> + * cclkin: source clock of mmc controller.
>> + * bus_hz: card interface clock generated by CLKGEN.
>> + * bus_hz = cclkin / RK3288_CLKGEN_DIV;
>> + * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
>> + *
>> + * Note: div can only be 0 or 1
>> + * if DDR50 8bit mode, div must be set 1
>> + */
>> + if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
>> + (ios->timing == MMC_TIMING_UHS_DDR50 ||
>> + ios->timing == MMC_TIMING_MMC_DDR52))
>> + cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
>> + else
>> + cclkin = ios->clock * RK3288_CLKGEN_DIV;
>> +
>> + ret = clk_set_rate(host->ciu_clk, cclkin);
>> + if (ret)
>> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
>> +
>> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
>> +}
>> +
>> +static const struct dw_mci_drv_data rk2928_drv_data = {
>> + .prepare_command = dw_mci_pltfm_prepare_command,
>> +};
>> +
>> +static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_pltfm_prepare_command,
>> + .set_ios = dw_mci_rk3288_set_ios,
>> + .setup_clock = dw_mci_rk3288_setup_clock,
>> };
>>
>> static const struct dw_mci_drv_data socfpga_drv_data = {
>> @@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
>> static const struct of_device_id dw_mci_pltfm_match[] = {
>> { .compatible = "snps,dw-mshc", },
>> { .compatible = "rockchip,rk2928-dw-mshc",
>> - .data = &rockchip_drv_data },
>> + .data = &rk2928_drv_data },
>> + { .compatible = "rockchip,rk3288-dw-mshc",
>> + .data = &rk3288_drv_data },
>> { .compatible = "altr,socfpga-dw-mshc",
>> .data = &socfpga_drv_data },
>> {},
>>
>
>
>
>

2014-07-10 03:32:26

by addy ke

[permalink] [raw]
Subject: [PATCH v2] mmc: dw_mmc: add support for RK3288

This patch focuses on clock setting for RK3288 mmc controller.

In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
and if DDR 8bit mode, CLKDIV register must be set 1.

Signed-off-by: Addy Ke <[email protected]>
---
changes since v1:
- dw_mci_rk3288_setup_clock: do not call clk_get_rate(), just use the
host->bus_hz which is already called by dw_mmc.c, suggested by Jaehoon Chung

.../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index c559f3f..e3f95cd 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
Required Properties:

* compatible: should be
- - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
+ - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
+ before RK3288
+ - "rockchip,rk3288-dw-mshc": for Rockchip RK3288

Example:

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index d4a47a9..809c28b 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -21,17 +21,61 @@
#include <linux/mmc/mmc.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/of.h>
+#include <linux/clk.h>

#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"

+#define RK3288_CLKGEN_DIV 2
+
static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
{
*cmdr |= SDMMC_CMD_USE_HOLD_REG;
}

-static const struct dw_mci_drv_data rockchip_drv_data = {
+static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
+{
+ host->bus_hz /= RK3288_CLKGEN_DIV;
+
+ return 0;
+}
+
+static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+ int ret;
+ unsigned int cclkin;
+
+ /*
+ * cclkin: source clock of mmc controller.
+ * bus_hz: card interface clock generated by CLKGEN.
+ * bus_hz = cclkin / RK3288_CLKGEN_DIV;
+ * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
+ *
+ * Note: div can only be 0 or 1
+ * if DDR50 8bit mode, div must be set 1
+ */
+ if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
+ (ios->timing == MMC_TIMING_UHS_DDR50 ||
+ ios->timing == MMC_TIMING_MMC_DDR52))
+ cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
+ else
+ cclkin = ios->clock * RK3288_CLKGEN_DIV;
+
+ ret = clk_set_rate(host->ciu_clk, cclkin);
+ if (ret)
+ dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+
+ host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+}
+
+static const struct dw_mci_drv_data rk2928_drv_data = {
+ .prepare_command = dw_mci_pltfm_prepare_command,
+};
+
+static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_pltfm_prepare_command,
+ .set_ios = dw_mci_rk3288_set_ios,
+ .setup_clock = dw_mci_rk3288_setup_clock,
};

static const struct dw_mci_drv_data socfpga_drv_data = {
@@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
static const struct of_device_id dw_mci_pltfm_match[] = {
{ .compatible = "snps,dw-mshc", },
{ .compatible = "rockchip,rk2928-dw-mshc",
- .data = &rockchip_drv_data },
+ .data = &rk2928_drv_data },
+ { .compatible = "rockchip,rk3288-dw-mshc",
+ .data = &rk3288_drv_data },
{ .compatible = "altr,socfpga-dw-mshc",
.data = &socfpga_drv_data },
{},
--
1.8.3.2

2014-07-29 04:53:04

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v2] mmc: dw_mmc: add support for RK3288

Addy,

On Wed, Jul 9, 2014 at 8:31 PM, Addy Ke <[email protected]> wrote:
> This patch focuses on clock setting for RK3288 mmc controller.
>
> In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
> and if DDR 8bit mode, CLKDIV register must be set 1.
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> changes since v1:
> - dw_mci_rk3288_setup_clock: do not call clk_get_rate(), just use the
> host->bus_hz which is already called by dw_mmc.c, suggested by Jaehoon Chung
>
> .../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
> drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
> 2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> index c559f3f..e3f95cd 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> @@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
> Required Properties:
>
> * compatible: should be
> - - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
> + - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
> + before RK3288
> + - "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>
> Example:
>
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index d4a47a9..809c28b 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -21,17 +21,61 @@
> #include <linux/mmc/mmc.h>
> #include <linux/mmc/dw_mmc.h>
> #include <linux/of.h>
> +#include <linux/clk.h>
>
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
>
> +#define RK3288_CLKGEN_DIV 2

Yup, this matches what I see in the TRM. It will always divide by 2
to allow for 4 phases (picking the phases not supported yet). Default
phase looks to be 180 degrees which is why we've (currently) got
USE_HOLD_REG hardcoded. :)

> +
> static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
> {
> *cmdr |= SDMMC_CMD_USE_HOLD_REG;
> }
>
> -static const struct dw_mci_drv_data rockchip_drv_data = {
> +static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
> +{
> + host->bus_hz /= RK3288_CLKGEN_DIV;
> +
> + return 0;
> +}
> +
> +static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> + int ret;
> + unsigned int cclkin;
> +
> + /*
> + * cclkin: source clock of mmc controller.
> + * bus_hz: card interface clock generated by CLKGEN.
> + * bus_hz = cclkin / RK3288_CLKGEN_DIV;
> + * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
> + *
> + * Note: div can only be 0 or 1
> + * if DDR50 8bit mode, div must be set 1

Makes sense. So this function is essentially reversing the logic in
dw_mmc and making sure that we'll get the right DIV (0 or 1) in
dw_mci_setup_bus().


> + */
> + if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
> + (ios->timing == MMC_TIMING_UHS_DDR50 ||

Probably don't need UHS_DDR50 since (I think) you can't have an 8-bit
SD card--only MMC, right? ...but it doesn't hurt.


> + ios->timing == MMC_TIMING_MMC_DDR52))
> + cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
> + else
> + cclkin = ios->clock * RK3288_CLKGEN_DIV;
> +
> + ret = clk_set_rate(host->ciu_clk, cclkin);
> + if (ret)
> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> +
> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> +}
> +
> +static const struct dw_mci_drv_data rk2928_drv_data = {
> + .prepare_command = dw_mci_pltfm_prepare_command,
> +};
> +
> +static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_pltfm_prepare_command,
> + .set_ios = dw_mci_rk3288_set_ios,
> + .setup_clock = dw_mci_rk3288_setup_clock,
> };
>
> static const struct dw_mci_drv_data socfpga_drv_data = {
> @@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
> static const struct of_device_id dw_mci_pltfm_match[] = {
> { .compatible = "snps,dw-mshc", },
> { .compatible = "rockchip,rk2928-dw-mshc",
> - .data = &rockchip_drv_data },
> + .data = &rk2928_drv_data },
> + { .compatible = "rockchip,rk3288-dw-mshc",
> + .data = &rk3288_drv_data },
> { .compatible = "altr,socfpga-dw-mshc",
> .data = &socfpga_drv_data },
> {},

Reviewed-by: Doug Anderson <[email protected]>
Tested-by: Doug Anderson <[email protected]>

2014-07-29 16:38:17

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v2] mmc: dw_mmc: add support for RK3288

Addy,

On Mon, Jul 28, 2014 at 9:52 PM, Doug Anderson <[email protected]> wrote:
> Addy,
>
> On Wed, Jul 9, 2014 at 8:31 PM, Addy Ke <[email protected]> wrote:
>> This patch focuses on clock setting for RK3288 mmc controller.
>>
>> In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
>> and if DDR 8bit mode, CLKDIV register must be set 1.
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> changes since v1:
>> - dw_mci_rk3288_setup_clock: do not call clk_get_rate(), just use the
>> host->bus_hz which is already called by dw_mmc.c, suggested by Jaehoon Chung
>>
>> .../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 +-
>> drivers/mmc/host/dw_mmc-pltfm.c | 50 +++++++++++++++++++++-
>> 2 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> index c559f3f..e3f95cd 100644
>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> @@ -10,7 +10,9 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
>> Required Properties:
>>
>> * compatible: should be
>> - - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
>> + - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
>> + before RK3288
>> + - "rockchip,rk3288-dw-mshc": for Rockchip RK3288
>>
>> Example:
>>
>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>> index d4a47a9..809c28b 100644
>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>> @@ -21,17 +21,61 @@
>> #include <linux/mmc/mmc.h>
>> #include <linux/mmc/dw_mmc.h>
>> #include <linux/of.h>
>> +#include <linux/clk.h>
>>
>> #include "dw_mmc.h"
>> #include "dw_mmc-pltfm.h"
>>
>> +#define RK3288_CLKGEN_DIV 2
>
> Yup, this matches what I see in the TRM. It will always divide by 2
> to allow for 4 phases (picking the phases not supported yet). Default
> phase looks to be 180 degrees which is why we've (currently) got
> USE_HOLD_REG hardcoded. :)
>
>> +
>> static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
>> {
>> *cmdr |= SDMMC_CMD_USE_HOLD_REG;
>> }
>>
>> -static const struct dw_mci_drv_data rockchip_drv_data = {
>> +static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
>> +{
>> + host->bus_hz /= RK3288_CLKGEN_DIV;
>> +
>> + return 0;
>> +}
>> +
>> +static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> + int ret;
>> + unsigned int cclkin;
>> +
>> + /*
>> + * cclkin: source clock of mmc controller.
>> + * bus_hz: card interface clock generated by CLKGEN.
>> + * bus_hz = cclkin / RK3288_CLKGEN_DIV;
>> + * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
>> + *
>> + * Note: div can only be 0 or 1
>> + * if DDR50 8bit mode, div must be set 1
>
> Makes sense. So this function is essentially reversing the logic in
> dw_mmc and making sure that we'll get the right DIV (0 or 1) in
> dw_mci_setup_bus().
>
>
>> + */
>> + if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
>> + (ios->timing == MMC_TIMING_UHS_DDR50 ||
>
> Probably don't need UHS_DDR50 since (I think) you can't have an 8-bit
> SD card--only MMC, right? ...but it doesn't hurt.
>
>
>> + ios->timing == MMC_TIMING_MMC_DDR52))
>> + cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
>> + else
>> + cclkin = ios->clock * RK3288_CLKGEN_DIV;
>> +
>> + ret = clk_set_rate(host->ciu_clk, cclkin);
>> + if (ret)
>> + dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
>> +
>> + host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
>> +}
>> +
>> +static const struct dw_mci_drv_data rk2928_drv_data = {
>> + .prepare_command = dw_mci_pltfm_prepare_command,
>> +};
>> +
>> +static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_pltfm_prepare_command,
>> + .set_ios = dw_mci_rk3288_set_ios,
>> + .setup_clock = dw_mci_rk3288_setup_clock,
>> };
>>
>> static const struct dw_mci_drv_data socfpga_drv_data = {
>> @@ -95,7 +139,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
>> static const struct of_device_id dw_mci_pltfm_match[] = {
>> { .compatible = "snps,dw-mshc", },
>> { .compatible = "rockchip,rk2928-dw-mshc",
>> - .data = &rockchip_drv_data },
>> + .data = &rk2928_drv_data },
>> + { .compatible = "rockchip,rk3288-dw-mshc",
>> + .data = &rk3288_drv_data },
>> { .compatible = "altr,socfpga-dw-mshc",
>> .data = &socfpga_drv_data },
>> {},
>
> Reviewed-by: Doug Anderson <[email protected]>
> Tested-by: Doug Anderson <[email protected]>

It turns out that I spoke too soon. I realized that in my testing I
didn't have "DDR" mode enabled for my eMMC card. When I did that then
your patch didn't work.

The problem is that dw_mci_setup_bus() doesn't realize that you've
changed "bus_hz" so it needs to re-run. You can fix it like this:


diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index 809c28b..bcc96d0 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -44,6 +44,7 @@ static void dw_mci_rk3288_set_ios(struct dw_mci
*host, struct mmc_ios *ios)
{
int ret;
unsigned int cclkin;
+ u32 bus_hz;

/*
* cclkin: source clock of mmc controller.
@@ -65,7 +66,11 @@ static void dw_mci_rk3288_set_ios(struct dw_mci
*host, struct mmc_ios *ios)
if (ret)
dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);

- host->bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+ bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+ if (bus_hz != host->bus_hz) {
+ host->bus_hz = bus_hz;
+ host->current_speed = 0; /* force dw_mci_setup_bus() */
+ }
}

static const struct dw_mci_drv_data rk2928_drv_data = {


Do you want to spin the patch for this?

-Doug

2014-07-31 06:02:22

by addy ke

[permalink] [raw]
Subject: [PATCH] mmc: dw_mmc: add support for RK3288

This patch focuses on clock setting for RK3288 mmc controller.

In RK3288 mmc controller, CLKDIV register can only be set 0 or 1,
and if DDR 8bit mode, CLKDIV register must be set 1.

Reported-by Doug Anderson <[email protected]>
Suggested-by: Jaehoon Chung <[email protected]>
Suggested-by: Doug Anderson <[email protected]>
Signed-off-by: Addy Ke <[email protected]>
---
changes since v1:
- dw_mci_rk3288_setup_clock: do not call clk_get_rate(), just use the
host->bus_hz which is already called by dw_mmc.c, suggested by Jaehoon Chung

changes since v2:
- merge from ChromiumOS tree, fix up clock settting bug,
which cause DDR50 mode for emmc not to work, reported by Doug Anderson
- remove MMC_TIMING_UHS_DDR50 condition, because on RK3288 only emmc can work
in 8bit mode, suggested by Doug Anderson

.../devicetree/bindings/mmc/rockchip-dw-mshc.txt | 6 ++-
drivers/mmc/host/dw_mmc-pltfm.c | 56 +++++++++++++++++++++-
2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
index c559f3f..c327c2d 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
@@ -10,12 +10,14 @@ extensions to the Synopsys Designware Mobile Storage Host Controller.
Required Properties:

* compatible: should be
- - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following
+ - "rockchip,rk2928-dw-mshc": for Rockchip RK2928 and following,
+ before RK3288
+ - "rockchip,rk3288-dw-mshc": for Rockchip RK3288

Example:

rkdwmmc0@12200000 {
- compatible = "rockchip,rk2928-dw-mshc";
+ compatible = "rockchip,rk3288-dw-mshc";
reg = <0x12200000 0x1000>;
interrupts = <0 75 0>;
#address-cells = <1>;
diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index d4a47a9..b547f7a 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -21,17 +21,67 @@
#include <linux/mmc/mmc.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/of.h>
+#include <linux/clk.h>

#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"

+#define RK3288_CLKGEN_DIV 2
+
static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
{
*cmdr |= SDMMC_CMD_USE_HOLD_REG;
}

-static const struct dw_mci_drv_data rockchip_drv_data = {
+static int dw_mci_rk3288_setup_clock(struct dw_mci *host)
+{
+ host->bus_hz /= RK3288_CLKGEN_DIV;
+
+ return 0;
+}
+
+static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+ int ret;
+ unsigned int cclkin;
+ u32 bus_hz;
+
+ /*
+ * cclkin: source clock of mmc controller.
+ * bus_hz: card interface clock generated by CLKGEN.
+ * bus_hz = cclkin / RK3288_CLKGEN_DIV;
+ * ios->clock = (div == 0) ? bus_hz : (bus_hz / (2 * div))
+ *
+ * Note: div can only be 0 or 1
+ * if DDR50 8bit mode(only emmc work in 8bit mode),
+ * div must be set 1
+ */
+ if ((ios->bus_width == MMC_BUS_WIDTH_8) &&
+ (ios->timing == MMC_TIMING_MMC_DDR52))
+ cclkin = 2 * ios->clock * RK3288_CLKGEN_DIV;
+ else
+ cclkin = ios->clock * RK3288_CLKGEN_DIV;
+
+ ret = clk_set_rate(host->ciu_clk, cclkin);
+ if (ret)
+ dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+
+ bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
+ if (bus_hz != host->bus_hz) {
+ host->bus_hz = bus_hz;
+ /* force dw_mci_setup_bus() */
+ host->current_speed = 0;
+ }
+}
+
+static const struct dw_mci_drv_data rk2928_drv_data = {
+ .prepare_command = dw_mci_pltfm_prepare_command,
+};
+
+static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_pltfm_prepare_command,
+ .set_ios = dw_mci_rk3288_set_ios,
+ .setup_clock = dw_mci_rk3288_setup_clock,
};

static const struct dw_mci_drv_data socfpga_drv_data = {
@@ -95,7 +145,9 @@ EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
static const struct of_device_id dw_mci_pltfm_match[] = {
{ .compatible = "snps,dw-mshc", },
{ .compatible = "rockchip,rk2928-dw-mshc",
- .data = &rockchip_drv_data },
+ .data = &rk2928_drv_data },
+ { .compatible = "rockchip,rk3288-dw-mshc",
+ .data = &rk3288_drv_data },
{ .compatible = "altr,socfpga-dw-mshc",
.data = &socfpga_drv_data },
{},
--
1.8.3.2