[U-Boot] [PATCH 0/2] With the update of the RK3399 DTS (to sync it with what the kernel uses),
the support for the Designware driver in the RK3399 needs some minor adjustments to successfully attach: - the clock identifier that the simple clk driver for the RK3399 sees has changed (SCLK_SDMMC -> HCLK_SDMMC) - the 'clock-freq-min-max' property has been deprecated upstream
For the submitted update of the DTS see: https://patchwork.ozlabs.org/patch/752192/
For the deprecation of the 'clock-freq-min-max' property, see https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64...
Philipp Tomsich (2): rockchip: clk: rk3399: adapt MMC clk configuration to the updated RK3399 DTS rockchip: mmc: handle deprecation of 'clock-freq-min-max'
drivers/clk/rockchip/clk_rk3399.c | 4 ++++ drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-)
The clocking of the designware MMC controller in the upstream (i.e. Linux) RK3399 has changed/does not match what the current DTS in U-Boot uses: the first clock entry now is HCLK_SDMMC instead of SCLK_SDMMC.
With the simple clock driver used for the RK3399, this needs a change in the selector understood by the various case statements in the driver to ensure that the driver still loads successfully.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
drivers/clk/rockchip/clk_rk3399.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/clk/rockchip/clk_rk3399.c b/drivers/clk/rockchip/clk_rk3399.c index ac658b9..4e807f1 100644 --- a/drivers/clk/rockchip/clk_rk3399.c +++ b/drivers/clk/rockchip/clk_rk3399.c @@ -747,6 +747,7 @@ static ulong rk3399_mmc_get_clk(struct rk3399_cru *cru, uint clk_id) u32 div, con;
switch (clk_id) { + case HCLK_SDMMC: case SCLK_SDMMC: con = readl(&cru->clksel_con[16]); break; @@ -772,6 +773,7 @@ static ulong rk3399_mmc_set_clk(struct rk3399_cru *cru, int aclk_emmc = 198*MHz;
switch (clk_id) { + case HCLK_SDMMC: case SCLK_SDMMC: /* Select clk_sdmmc source from GPLL by default */ src_clk_div = GPLL_HZ / set_rate; @@ -861,6 +863,7 @@ static ulong rk3399_clk_get_rate(struct clk *clk) switch (clk->id) { case 0 ... 63: return 0; + case HCLK_SDMMC: case SCLK_SDMMC: case SCLK_EMMC: rate = rk3399_mmc_get_clk(priv->cru, clk->id); @@ -898,6 +901,7 @@ static ulong rk3399_clk_set_rate(struct clk *clk, ulong rate) switch (clk->id) { case 0 ... 63: return 0; + case HCLK_SDMMC: case SCLK_SDMMC: case SCLK_EMMC: ret = rk3399_mmc_set_clk(priv->cru, clk->id, rate);
On 25 April 2017 at 01:52, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
The clocking of the designware MMC controller in the upstream (i.e. Linux) RK3399 has changed/does not match what the current DTS in U-Boot uses: the first clock entry now is HCLK_SDMMC instead of SCLK_SDMMC.
With the simple clock driver used for the RK3399, this needs a change in the selector understood by the various case statements in the driver to ensure that the driver still loads successfully.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/clk/rockchip/clk_rk3399.c | 4 ++++ 1 file changed, 4 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
On 25 April 2017 at 01:52, Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
The clocking of the designware MMC controller in the upstream (i.e. Linux) RK3399 has changed/does not match what the current DTS in U-Boot uses: the first clock entry now is HCLK_SDMMC instead of SCLK_SDMMC.
With the simple clock driver used for the RK3399, this needs a change in the selector understood by the various case statements in the driver to ensure that the driver still loads successfully.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/clk/rockchip/clk_rk3399.c | 4 ++++ 1 file changed, 4 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip/next, thanks!
The 'clock-freq-min-max' property was deprecated in the upstream (i.e. Linux) DTS bindings in favor of the 'max-frequency' property.
With the latest RK3399 DTSI does no longer include the deprecated property and the rockchip_dw_mmc driver requiring it to be present, the driver doesn't bind to the node in the RK3399 DTSI any longer (thus breaking access to the SD card on the RK3399-Q7 board).
To fix this, we implement a similar logic as in the Linux driver: if the deprecated property is present, we issue a warning (if DEBUG is enabled); if it is missing, we require 'max-frequency' to be set and use it to create a min/max value-pair.
See https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64... for the deprecation/matching change in Linux.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index c36eda0..432ae20 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -76,9 +76,25 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct udevice *dev) return -EINVAL; priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev), "fifo-mode"); + + /* + * 'clock-freq-min-max' is deprecated + * (see https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64...) + */ if (fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev), - "clock-freq-min-max", priv->minmax, 2)) - return -EINVAL; + "clock-freq-min-max", priv->minmax, 2)) { + int val = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), + "max-frequency", -EINVAL); + + if (val < 0) + return val; + + priv->minmax[0] = 400000; /* 400 kHz */ + priv->minmax[1] = val; + } else { + debug("%s: 'clock-freq-min-max' property was deprecated.\n", + __func__); + } #endif return 0; }
On 04/25/2017 04:52 PM, Philipp Tomsich wrote:
The 'clock-freq-min-max' property was deprecated in the upstream (i.e. Linux) DTS bindings in favor of the 'max-frequency' property.
It's difference wit Linux kernel. "clock-freq-min-max" was deprecated in Linux. Linux kernel is supporting to find the best clock value in core.c.
There are defined the freqs[] = {400000, ...., 100000} then it should be looped to find the value from 400K to 100K.
As i know, u-boot doesn't support this.. If you set to min value as 400K..doesn't check the other values..
Best Regards, Jaehoon Chung
With the latest RK3399 DTSI does no longer include the deprecated property and the rockchip_dw_mmc driver requiring it to be present, the driver doesn't bind to the node in the RK3399 DTSI any longer (thus breaking access to the SD card on the RK3399-Q7 board).
To fix this, we implement a similar logic as in the Linux driver: if the deprecated property is present, we issue a warning (if DEBUG is enabled); if it is missing, we require 'max-frequency' to be set and use it to create a min/max value-pair.
See https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64... for the deprecation/matching change in Linux.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index c36eda0..432ae20 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -76,9 +76,25 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct udevice *dev) return -EINVAL; priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev), "fifo-mode");
- /*
* 'clock-freq-min-max' is deprecated* (see https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64c9f7b)*/
"clock-freq-min-max", priv->minmax, 2))return -EINVAL;
"clock-freq-min-max", priv->minmax, 2)) {int val = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),"max-frequency", -EINVAL);if (val < 0)return val;priv->minmax[0] = 400000; /* 400 kHz */priv->minmax[1] = val;- } else {
debug("%s: 'clock-freq-min-max' property was deprecated.\n",__func__);- }
#endif return 0; }
I am aware of this kernel behaviour, but felt that it does not directly apply for this specific change, as the original DTS (before the deprecation of this property) had defined the minimum frequency for the affected chips as <400000>.
For reference, here’s the DTS change that this is tracking: - clock-freq-min-max = <400000 150000000>; + max-frequency = <150000000>;
During the init-op, the following code in dw_mmc.c ensures that we are always enumerating at f_min (which is always 400kHz with this driver): /* Enumerate at 400KHz */ dwmci_setup_bus(host, mmc->cfg->f_min); So the UBoot MMC-core relies on the drivers to select a single frequency.
In other words: this commit does not change the current state of affairs, as all affected devices (i.e. those that the rockchip-dw-mmc binds to) already have the 400kHz f_min set.
If you feel that this it is worthwhile, we can also change mmc/mmc.c to not just rely on the init-op, but explicitly set the frequency and then iterate over the 400KHz … 100kHz array. However, this would then affect all mmc-drivers, so I would rather keep this as a separate change that allows everyone to test on their boards.
Best regards, Philipp.
On 27 Apr 2017, at 06:21, Jaehoon Chung jh80.chung@samsung.com wrote:
On 04/25/2017 04:52 PM, Philipp Tomsich wrote:
The 'clock-freq-min-max' property was deprecated in the upstream (i.e. Linux) DTS bindings in favor of the 'max-frequency' property.
It's difference wit Linux kernel. "clock-freq-min-max" was deprecated in Linux. Linux kernel is supporting to find the best clock value in core.c.
There are defined the freqs[] = {400000, ...., 100000} then it should be looped to find the value from 400K to 100K.
As i know, u-boot doesn't support this.. If you set to min value as 400K..doesn't check the other values..
Best Regards, Jaehoon Chung
With the latest RK3399 DTSI does no longer include the deprecated property and the rockchip_dw_mmc driver requiring it to be present, the driver doesn't bind to the node in the RK3399 DTSI any longer (thus breaking access to the SD card on the RK3399-Q7 board).
To fix this, we implement a similar logic as in the Linux driver: if the deprecated property is present, we issue a warning (if DEBUG is enabled); if it is missing, we require 'max-frequency' to be set and use it to create a min/max value-pair.
See https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64... for the deprecation/matching change in Linux.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index c36eda0..432ae20 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -76,9 +76,25 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct udevice *dev) return -EINVAL; priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev), "fifo-mode");
- /*
* 'clock-freq-min-max' is deprecated* (see https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64c9f7b)*/
"clock-freq-min-max", priv->minmax, 2))return -EINVAL;
"clock-freq-min-max", priv->minmax, 2)) {int val = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),"max-frequency", -EINVAL);if (val < 0)return val;priv->minmax[0] = 400000; /* 400 kHz */priv->minmax[1] = val;- } else {
debug("%s: 'clock-freq-min-max' property was deprecated.\n",__func__);- }
#endif return 0; }
Hi Jaehoon,
On 04/27/2017 12:21 PM, Jaehoon Chung wrote:
On 04/25/2017 04:52 PM, Philipp Tomsich wrote:
The 'clock-freq-min-max' property was deprecated in the upstream (i.e. Linux) DTS bindings in favor of the 'max-frequency' property.
It's difference wit Linux kernel. "clock-freq-min-max" was deprecated in Linux. Linux kernel is supporting to find the best clock value in core.c.
We need to sync the dts definition with kernel, because we might using just the dts from kernel without modification in the future. I'm not sure Philipp's patch is correct or not, but we can figure it out, right?
Thanks, - Kever
There are defined the freqs[] = {400000, ...., 100000} then it should be looped to find the value from 400K to 100K.
As i know, u-boot doesn't support this.. If you set to min value as 400K..doesn't check the other values..
Best Regards, Jaehoon Chung
With the latest RK3399 DTSI does no longer include the deprecated property and the rockchip_dw_mmc driver requiring it to be present, the driver doesn't bind to the node in the RK3399 DTSI any longer (thus breaking access to the SD card on the RK3399-Q7 board).
To fix this, we implement a similar logic as in the Linux driver: if the deprecated property is present, we issue a warning (if DEBUG is enabled); if it is missing, we require 'max-frequency' to be set and use it to create a min/max value-pair.
See https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64... for the deprecation/matching change in Linux.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index c36eda0..432ae20 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -76,9 +76,25 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct udevice *dev) return -EINVAL; priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev), "fifo-mode");
- /*
* 'clock-freq-min-max' is deprecated* (see https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64c9f7b)*/
"clock-freq-min-max", priv->minmax, 2))return -EINVAL;
"clock-freq-min-max", priv->minmax, 2)) {int val = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),"max-frequency", -EINVAL);if (val < 0)return val;priv->minmax[0] = 400000; /* 400 kHz */priv->minmax[1] = val;- } else {
debug("%s: 'clock-freq-min-max' property was deprecated.\n",__func__);- } #endif return 0; }
Hi Jaehoon,
On 04/27/2017 12:21 PM, Jaehoon Chung wrote:
On 04/25/2017 04:52 PM, Philipp Tomsich wrote:
The 'clock-freq-min-max' property was deprecated in the upstream (i.e. Linux) DTS bindings in favor of the 'max-frequency' property.
It's difference wit Linux kernel. "clock-freq-min-max" was deprecated in Linux. Linux kernel is supporting to find the best clock value in core.c.
We need to sync the dts definition with kernel, because we might using just the dts from kernel without modification in the future. I'm not sure Philipp's patch is correct or not, but we can figure it out, right?
Thanks, - Kever
There are defined the freqs[] = {400000, ...., 100000} then it should be looped to find the value from 400K to 100K.
As i know, u-boot doesn't support this.. If you set to min value as 400K..doesn't check the other values..
Best Regards, Jaehoon Chung
With the latest RK3399 DTSI does no longer include the deprecated property and the rockchip_dw_mmc driver requiring it to be present, the driver doesn't bind to the node in the RK3399 DTSI any longer (thus breaking access to the SD card on the RK3399-Q7 board).
To fix this, we implement a similar logic as in the Linux driver: if the deprecated property is present, we issue a warning (if DEBUG is enabled); if it is missing, we require 'max-frequency' to be set and use it to create a min/max value-pair.
See https://github.com/torvalds/linux/commit/b023030f10573de738bbe8df63d43acab64... for the deprecation/matching change in Linux.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
drivers/mmc/rockchip_dw_mmc.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
Applied to u-boot-rockchip/next, thanks!
participants (6)
-
Dr. Philipp Tomsich -
Jaehoon Chung -
Kever Yang -
Philipp Tomsich
-
Simon Glass -
sjg＠google.com