* [PATCH v3 0/2] fix dw-mmc-rockchip rk356x clock rates
@ 2022-03-05 21:58 Peter Geis
2022-03-05 21:58 ` [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers Peter Geis
2022-03-05 21:58 ` [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates Peter Geis
0 siblings, 2 replies; 5+ messages in thread
From: Peter Geis @ 2022-03-05 21:58 UTC (permalink / raw)
Cc: robin.murphy, linux-rockchip, Peter Geis, Jaehoon Chung,
Ulf Hansson, Heiko Stuebner, linux-mmc, linux-arm-kernel,
linux-kernel
cc: Jaehoon Chung <jh80.chung@samsung.com>
cc: Ulf Hansson <ulf.hansson@linaro.org>
cc: Heiko Stuebner <heiko@sntech.de>
cc: linux-mmc@vger.kernel.org
cc: linux-arm-kernel@lists.infradead.org
cc: linux-rockchip@lists.infradead.org
cc: linux-kernel@vger.kernel.org
While lighting off support for the SoQuartz module, it was discoved the
dw-mmc-rockchip driver has tremendous log spam when the cd-broken flag
is enabled and no card is inserted.
The SoQuart requires the cd-broken flag as the CM4 module pinout it
follows has no card-detect pin.
These errors occur during card initialization on all rk356x chips, but
are amplified by cd-broken as the function is called multiple times each
poll cycle.
It was discovered the lowest possible clock rate the rk356x cru can
provide for the ciu clock in the default configuration is 750khz. There
is an internal clock divider that makes the final minimum clock 375khz.
We could hardcode this, but it is possible it could change if the
default clock configuration changes.
To fix this, we must make two changes:
First, the dw-mmc core needs to be updated to allow a host driver to
save its requested minimum frequency. This is necessary as the mmc_host
struct isn't available when the host drivers initialization code is
called. The dw-mmc core can then apply the f_min later when the struct
is available.
Second, the dw-mmc-rockchip driver is extended to test the frequencies
mmc core will use during card initialization. It finds the lowest
supported frequency from the cru and saves it for later use by dw-mmc
core.
Changelog:
v3:
- add support in dw_mmc core for saving the minimum frequency (fixes
setting f_min)
- add test for the lowest supported frequency to avoid clamping configs
that don't have an error
v2:
- change from muting the error to attempting to fix the underlying issue
by setting f_min in driver initialization
Peter Geis (2):
mmc: host: dw_mmc: support setting f_min from host drivers
mmc: host: dw-mmc-rockchip: fix handling invalid clock rates
drivers/mmc/host/dw_mmc-rockchip.c | 27 +++++++++++++++++++++++----
drivers/mmc/host/dw_mmc.c | 7 ++++++-
drivers/mmc/host/dw_mmc.h | 2 ++
3 files changed, 31 insertions(+), 5 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers
2022-03-05 21:58 [PATCH v3 0/2] fix dw-mmc-rockchip rk356x clock rates Peter Geis
@ 2022-03-05 21:58 ` Peter Geis
2022-03-07 12:17 ` Ulf Hansson
2022-03-05 21:58 ` [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates Peter Geis
1 sibling, 1 reply; 5+ messages in thread
From: Peter Geis @ 2022-03-05 21:58 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: robin.murphy, linux-rockchip, Peter Geis, linux-mmc, linux-kernel
Host drivers may not be able to support frequencies as low as dw-mmc
supports. Unfortunately f_min isn't available when the drv_data->init
function is called, as the mmc_host struct hasn't been set up yet.
Support the host drivers saving the requested minimum frequency, so we
can later set f_min when it is available.
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
drivers/mmc/host/dw_mmc.c | 7 ++++++-
drivers/mmc/host/dw_mmc.h | 2 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 42bf8a2287ba..0d90d0201759 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2898,7 +2898,12 @@ static int dw_mci_init_slot_caps(struct dw_mci_slot *slot)
if (host->pdata->caps2)
mmc->caps2 = host->pdata->caps2;
- mmc->f_min = DW_MCI_FREQ_MIN;
+ /* if host has set a minimum_freq, we should respect it */
+ if (host->minimum_speed)
+ mmc->f_min = host->minimum_speed;
+ else
+ mmc->f_min = DW_MCI_FREQ_MIN;
+
if (!mmc->f_max)
mmc->f_max = DW_MCI_FREQ_MAX;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 7f1e38621d13..4ed81f94f7ca 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -99,6 +99,7 @@ struct dw_mci_dma_slave {
* @bus_hz: The rate of @mck in Hz. This forms the basis for MMC bus
* rate and timeout calculations.
* @current_speed: Configured rate of the controller.
+ * @minimum_speed: Stored minimum rate of the controller.
* @fifoth_val: The value of FIFOTH register.
* @verid: Denote Version ID.
* @dev: Device associated with the MMC controller.
@@ -201,6 +202,7 @@ struct dw_mci {
u32 bus_hz;
u32 current_speed;
+ u32 minimum_speed;
u32 fifoth_val;
u16 verid;
struct device *dev;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates
2022-03-05 21:58 [PATCH v3 0/2] fix dw-mmc-rockchip rk356x clock rates Peter Geis
2022-03-05 21:58 ` [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers Peter Geis
@ 2022-03-05 21:58 ` Peter Geis
2022-03-07 12:17 ` Ulf Hansson
1 sibling, 1 reply; 5+ messages in thread
From: Peter Geis @ 2022-03-05 21:58 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson, Heiko Stuebner
Cc: robin.murphy, linux-rockchip, Peter Geis, linux-mmc,
linux-arm-kernel, linux-kernel
The Rockchip rk356x ciu clock cannot be set as low as the dw-mmc
hardware supports. This leads to a situation during card initialization
where the clock is set lower than the clock driver can support. The
dw-mmc-rockchip driver spews errors when this happens.
For normal operation this only happens a few times during boot, but when
cd-broken is enabled (in cases such as the SoQuartz module) this fires
multiple times each poll cycle.
Fix this by testing the lowest possible frequency that the clock driver
can support which is within the mmc specification. Divide that rate by
the internal divider and set f_min to this.
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---
drivers/mmc/host/dw_mmc-rockchip.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 95d0ec0f5f3a..f825487aa739 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -15,7 +15,9 @@
#include "dw_mmc.h"
#include "dw_mmc-pltfm.h"
-#define RK3288_CLKGEN_DIV 2
+#define RK3288_CLKGEN_DIV 2
+
+static const unsigned int freqs[] = { 100000, 200000, 300000, 400000 };
struct dw_mci_rockchip_priv_data {
struct clk *drv_clk;
@@ -51,7 +53,7 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
ret = clk_set_rate(host->ciu_clk, cclkin);
if (ret)
- dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
+ dev_warn(host->dev, "failed to set rate %uHz err: %d\n", cclkin, ret);
bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
if (bus_hz != host->bus_hz) {
@@ -290,13 +292,30 @@ static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
static int dw_mci_rockchip_init(struct dw_mci *host)
{
+ int ret, i;
+
/* It is slot 8 on Rockchip SoCs */
host->sdio_id0 = 8;
- if (of_device_is_compatible(host->dev->of_node,
- "rockchip,rk3288-dw-mshc"))
+ if (of_device_is_compatible(host->dev->of_node, "rockchip,rk3288-dw-mshc")) {
host->bus_hz /= RK3288_CLKGEN_DIV;
+ /* clock driver will fail if the clock is less than the lowest source clock
+ * divided by the internal clock divider. Test for the lowest available
+ * clock and set the minimum freq to clock / clock divider.
+ */
+
+ for (i = 0; i < ARRAY_SIZE(freqs); i++) {
+ ret = clk_round_rate(host->ciu_clk, freqs[i] * RK3288_CLKGEN_DIV);
+ if (ret > 0) {
+ host->minimum_speed = ret / RK3288_CLKGEN_DIV;
+ break;
+ }
+ }
+ if (ret < 0)
+ dev_warn(host->dev, "no valid minimum freq: %d\n", ret);
+ }
+
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers
2022-03-05 21:58 ` [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers Peter Geis
@ 2022-03-07 12:17 ` Ulf Hansson
0 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2022-03-07 12:17 UTC (permalink / raw)
To: Peter Geis
Cc: Jaehoon Chung, robin.murphy, linux-rockchip, linux-mmc, linux-kernel
On Sat, 5 Mar 2022 at 22:58, Peter Geis <pgwipeout@gmail.com> wrote:
>
> Host drivers may not be able to support frequencies as low as dw-mmc
> supports. Unfortunately f_min isn't available when the drv_data->init
> function is called, as the mmc_host struct hasn't been set up yet.
>
> Support the host drivers saving the requested minimum frequency, so we
> can later set f_min when it is available.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/dw_mmc.c | 7 ++++++-
> drivers/mmc/host/dw_mmc.h | 2 ++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 42bf8a2287ba..0d90d0201759 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2898,7 +2898,12 @@ static int dw_mci_init_slot_caps(struct dw_mci_slot *slot)
> if (host->pdata->caps2)
> mmc->caps2 = host->pdata->caps2;
>
> - mmc->f_min = DW_MCI_FREQ_MIN;
> + /* if host has set a minimum_freq, we should respect it */
> + if (host->minimum_speed)
> + mmc->f_min = host->minimum_speed;
> + else
> + mmc->f_min = DW_MCI_FREQ_MIN;
> +
> if (!mmc->f_max)
> mmc->f_max = DW_MCI_FREQ_MAX;
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 7f1e38621d13..4ed81f94f7ca 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -99,6 +99,7 @@ struct dw_mci_dma_slave {
> * @bus_hz: The rate of @mck in Hz. This forms the basis for MMC bus
> * rate and timeout calculations.
> * @current_speed: Configured rate of the controller.
> + * @minimum_speed: Stored minimum rate of the controller.
> * @fifoth_val: The value of FIFOTH register.
> * @verid: Denote Version ID.
> * @dev: Device associated with the MMC controller.
> @@ -201,6 +202,7 @@ struct dw_mci {
>
> u32 bus_hz;
> u32 current_speed;
> + u32 minimum_speed;
> u32 fifoth_val;
> u16 verid;
> struct device *dev;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates
2022-03-05 21:58 ` [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates Peter Geis
@ 2022-03-07 12:17 ` Ulf Hansson
0 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2022-03-07 12:17 UTC (permalink / raw)
To: Peter Geis
Cc: Jaehoon Chung, Heiko Stuebner, robin.murphy, linux-rockchip,
linux-mmc, linux-arm-kernel, linux-kernel
On Sat, 5 Mar 2022 at 22:58, Peter Geis <pgwipeout@gmail.com> wrote:
>
> The Rockchip rk356x ciu clock cannot be set as low as the dw-mmc
> hardware supports. This leads to a situation during card initialization
> where the clock is set lower than the clock driver can support. The
> dw-mmc-rockchip driver spews errors when this happens.
> For normal operation this only happens a few times during boot, but when
> cd-broken is enabled (in cases such as the SoQuartz module) this fires
> multiple times each poll cycle.
>
> Fix this by testing the lowest possible frequency that the clock driver
> can support which is within the mmc specification. Divide that rate by
> the internal divider and set f_min to this.
>
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/dw_mmc-rockchip.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index 95d0ec0f5f3a..f825487aa739 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -15,7 +15,9 @@
> #include "dw_mmc.h"
> #include "dw_mmc-pltfm.h"
>
> -#define RK3288_CLKGEN_DIV 2
> +#define RK3288_CLKGEN_DIV 2
> +
> +static const unsigned int freqs[] = { 100000, 200000, 300000, 400000 };
>
> struct dw_mci_rockchip_priv_data {
> struct clk *drv_clk;
> @@ -51,7 +53,7 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>
> ret = clk_set_rate(host->ciu_clk, cclkin);
> if (ret)
> - dev_warn(host->dev, "failed to set rate %uHz\n", ios->clock);
> + dev_warn(host->dev, "failed to set rate %uHz err: %d\n", cclkin, ret);
>
> bus_hz = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
> if (bus_hz != host->bus_hz) {
> @@ -290,13 +292,30 @@ static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>
> static int dw_mci_rockchip_init(struct dw_mci *host)
> {
> + int ret, i;
> +
> /* It is slot 8 on Rockchip SoCs */
> host->sdio_id0 = 8;
>
> - if (of_device_is_compatible(host->dev->of_node,
> - "rockchip,rk3288-dw-mshc"))
> + if (of_device_is_compatible(host->dev->of_node, "rockchip,rk3288-dw-mshc")) {
> host->bus_hz /= RK3288_CLKGEN_DIV;
>
> + /* clock driver will fail if the clock is less than the lowest source clock
> + * divided by the internal clock divider. Test for the lowest available
> + * clock and set the minimum freq to clock / clock divider.
> + */
> +
> + for (i = 0; i < ARRAY_SIZE(freqs); i++) {
> + ret = clk_round_rate(host->ciu_clk, freqs[i] * RK3288_CLKGEN_DIV);
> + if (ret > 0) {
> + host->minimum_speed = ret / RK3288_CLKGEN_DIV;
> + break;
> + }
> + }
> + if (ret < 0)
> + dev_warn(host->dev, "no valid minimum freq: %d\n", ret);
> + }
> +
> return 0;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-07 12:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-05 21:58 [PATCH v3 0/2] fix dw-mmc-rockchip rk356x clock rates Peter Geis
2022-03-05 21:58 ` [PATCH v3 1/2] mmc: host: dw_mmc: support setting f_min from host drivers Peter Geis
2022-03-07 12:17 ` Ulf Hansson
2022-03-05 21:58 ` [PATCH v3 2/2] mmc: host: dw-mmc-rockchip: fix handling invalid clock rates Peter Geis
2022-03-07 12: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).