From: Chen Wang <unicorn_wang@outlook.com>
In addition to the required core clock and optional
bus clock, the soc will expand its own clocks, so
the bulk clock mechanism is abstracted.
Note, I call the bulk clocks as "other clocks" due
to the bus clock has been called as "optional".
Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
---
drivers/mmc/host/sdhci-of-dwcmshc.c | 90 +++++++++++++++--------------
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index e79aa4b3b6c3..35401616fb2e 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -108,7 +108,6 @@
#define DLL_LOCK_WO_TMOUT(x) \
((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
(((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
-#define RK35xx_MAX_CLKS 3
/* PHY register area pointer */
#define DWC_MSHC_PTR_PHY_R 0x300
@@ -199,23 +198,54 @@ enum dwcmshc_rk_type {
};
struct rk35xx_priv {
- /* Rockchip specified optional clocks */
- struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS];
struct reset_control *reset;
enum dwcmshc_rk_type devtype;
u8 txclk_tapnum;
};
+#define DWCMSHC_MAX_OTHER_CLKS 3
+
struct dwcmshc_priv {
struct clk *bus_clk;
int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
+ int num_other_clks;
+ struct clk_bulk_data other_clks[DWCMSHC_MAX_OTHER_CLKS];
+
void *priv; /* pointer to SoC private stuff */
u16 delay_line;
u16 flags;
};
+static int dwcmshc_get_enable_other_clks(struct device *dev,
+ struct dwcmshc_priv *priv,
+ int num_clks,
+ const char * const clk_ids[])
+{
+ int err;
+
+ if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
+ return -EINVAL;
+
+ for (int i = 0; i < num_clks; i++)
+ priv->other_clks[i].id = clk_ids[i];
+
+ err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
+ if (err) {
+ dev_err(dev, "failed to get clocks %d\n", err);
+ return err;
+ }
+
+ err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
+ if (err)
+ dev_err(dev, "failed to enable clocks %d\n", err);
+
+ priv->num_other_clks = num_clks;
+
+ return err;
+}
+
/*
* If DMA addr spans 128MB boundary, we split the DMA transfer into two
* so that each DMA transfer doesn't exceed the boundary.
@@ -1036,8 +1066,9 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
{
- int err;
+ static const char * const clk_ids[] = {"axi", "block", "timer"};
struct rk35xx_priv *priv = dwc_priv->priv;
+ int err;
priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
if (IS_ERR(priv->reset)) {
@@ -1046,21 +1077,10 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
return err;
}
- priv->rockchip_clks[0].id = "axi";
- priv->rockchip_clks[1].id = "block";
- priv->rockchip_clks[2].id = "timer";
- err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS,
- priv->rockchip_clks);
- if (err) {
- dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err);
- return err;
- }
-
- err = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, priv->rockchip_clks);
- if (err) {
- dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
+ err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
+ ARRAY_SIZE(clk_ids), clk_ids);
+ if (err)
return err;
- }
if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
&priv->txclk_tapnum))
@@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
err_clk:
clk_disable_unprepare(pltfm_host->clk);
clk_disable_unprepare(priv->bus_clk);
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+ clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
free_pltfm:
sdhci_pltfm_free(pdev);
return err;
@@ -1304,7 +1322,6 @@ static void dwcmshc_remove(struct platform_device *pdev)
struct sdhci_host *host = platform_get_drvdata(pdev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
pm_runtime_get_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -1316,9 +1333,7 @@ static void dwcmshc_remove(struct platform_device *pdev)
clk_disable_unprepare(pltfm_host->clk);
clk_disable_unprepare(priv->bus_clk);
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+ clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
sdhci_pltfm_free(pdev);
}
@@ -1328,7 +1343,6 @@ static int dwcmshc_suspend(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
int ret;
pm_runtime_resume(dev);
@@ -1347,9 +1361,7 @@ static int dwcmshc_suspend(struct device *dev)
if (!IS_ERR(priv->bus_clk))
clk_disable_unprepare(priv->bus_clk);
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+ clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
return ret;
}
@@ -1359,7 +1371,6 @@ static int dwcmshc_resume(struct device *dev)
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *rk_priv = priv->priv;
int ret;
ret = clk_prepare_enable(pltfm_host->clk);
@@ -1372,29 +1383,24 @@ static int dwcmshc_resume(struct device *dev)
goto disable_clk;
}
- if (rk_priv) {
- ret = clk_bulk_prepare_enable(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
- if (ret)
- goto disable_bus_clk;
- }
+ ret = clk_bulk_prepare_enable(priv->num_other_clks, priv->other_clks);
+ if (ret)
+ goto disable_bus_clk;
ret = sdhci_resume_host(host);
if (ret)
- goto disable_rockchip_clks;
+ goto disable_other_clks;
if (host->mmc->caps2 & MMC_CAP2_CQE) {
ret = cqhci_resume(host->mmc);
if (ret)
- goto disable_rockchip_clks;
+ goto disable_other_clks;
}
return 0;
-disable_rockchip_clks:
- if (rk_priv)
- clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
- rk_priv->rockchip_clks);
+disable_other_clks:
+ clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
disable_bus_clk:
if (!IS_ERR(priv->bus_clk))
clk_disable_unprepare(priv->bus_clk);
--
2.34.1A bit late for a "review", but Diederik and I have just been
IRC-debugging a crash on RK3568 which by inspection seems to be caused
by this patch:
On 2024-08-05 10:17 am, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
>
> In addition to the required core clock and optional
> bus clock, the soc will expand its own clocks, so
> the bulk clock mechanism is abstracted.
>
> Note, I call the bulk clocks as "other clocks" due
> to the bus clock has been called as "optional".
>
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
> ---
[...]
> +static int dwcmshc_get_enable_other_clks(struct device *dev,
> + struct dwcmshc_priv *priv,
> + int num_clks,
> + const char * const clk_ids[])
> +{
> + int err;
> +
> + if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
> + return -EINVAL;
> +
> + for (int i = 0; i < num_clks; i++)
> + priv->other_clks[i].id = clk_ids[i];
> +
> + err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
This leaves a pointer into "priv" in the devres list...
> + if (err) {
> + dev_err(dev, "failed to get clocks %d\n", err);
> + return err;
> + }
> +
> + err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
> + if (err)
> + dev_err(dev, "failed to enable clocks %d\n", err);
> +
> + priv->num_other_clks = num_clks;
> +
> + return err;
> +}
> +
> /*
> * If DMA addr spans 128MB boundary, we split the DMA transfer into two
> * so that each DMA transfer doesn't exceed the boundary.
[...]
> @@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
> err_clk:
> clk_disable_unprepare(pltfm_host->clk);
> clk_disable_unprepare(priv->bus_clk);
> - if (rk_priv)
> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
> free_pltfm:
> sdhci_pltfm_free(pdev);
...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
regulator isn't ready yet, that "priv" is freed here, so by the time the
devres callbacks eventually run, that "devres->clks" pointer which used
to represent "priv->other_clocks" points to who knows what, and this
sort of thing happens:
[ 12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
[ 12.472104] Mem abort info:
[ 12.472471] ESR = 0x0000000096000004
[ 12.475991] EC = 0x25: DABT (current EL), IL = 32 bits
[ 12.476657] SET = 0, FnV = 0
[ 12.477146] EA = 0, S1PTW = 0
[ 12.477547] FSC = 0x04: level 0 translation fault
[ 12.478127] Data abort info:
[ 12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
[ 12.478413] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 12.479826] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 12.480418] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 12.481282] [002df7b378917664] address between user and kernel address ranges
[ 12.482421] Internal error: Oops: 0000000096000004 [#1] SMP
[ 12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
[ 12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY Debian 6.16~rc7-1
[ 12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
[ 12.488412] Workqueue: async async_run_entry_fn
[ 12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 12.489539] pc : __clk_put+0x2c/0x138
[ 12.489913] lr : __clk_put+0x2c/0x138
[ 12.490281] sp : ffff800080713b10
[ 12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
[ 12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
[ 12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
[ 12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
[ 12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
[ 12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
[ 12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
[ 12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
[ 12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
[ 12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
[ 12.497524] Call trace:
[ 12.497776] __clk_put+0x2c/0x138 (P)
[ 12.498154] clk_put+0x18/0x30
[ 12.498471] clk_bulk_put+0x40/0x68
[ 12.498825] devm_clk_bulk_release+0x24/0x40
[ 12.499248] release_nodes+0x64/0xa0
[ 12.499608] devres_release_all+0x98/0xf8
[ 12.500004] device_unbind_cleanup+0x20/0x70
[ 12.500426] really_probe+0x1e8/0x3a0
[ 12.500793] __driver_probe_device+0x84/0x160
[ 12.501225] driver_probe_device+0x44/0x128
[ 12.501640] __driver_attach_async_helper+0x5c/0x108
[ 12.502125] async_run_entry_fn+0x40/0x180
[ 12.502535] process_one_work+0x23c/0x640
[ 12.502939] worker_thread+0x1b4/0x360
[ 12.503315] kthread+0x150/0x250
[ 12.503646] ret_from_fork+0x10/0x20
[ 12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
[ 12.504598] ---[ end trace 0000000000000000 ]---
TBH I'm not sure what to do as a straight revert seems impractical by
now, so we hope someone else might have a good idea.
Thanks,
Robin.
On 22/07/2025 21:33, Robin Murphy wrote:
> A bit late for a "review", but Diederik and I have just been
> IRC-debugging a crash on RK3568 which by inspection seems to be caused
> by this patch:
>
> On 2024-08-05 10:17 am, Chen Wang wrote:
>> From: Chen Wang <unicorn_wang@outlook.com>
>>
>> In addition to the required core clock and optional
>> bus clock, the soc will expand its own clocks, so
>> the bulk clock mechanism is abstracted.
>>
>> Note, I call the bulk clocks as "other clocks" due
>> to the bus clock has been called as "optional".
>>
>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>> ---
> [...]
>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>> + struct dwcmshc_priv *priv,
>> + int num_clks,
>> + const char * const clk_ids[])
>> +{
>> + int err;
>> +
>> + if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>> + return -EINVAL;
>> +
>> + for (int i = 0; i < num_clks; i++)
>> + priv->other_clks[i].id = clk_ids[i];
>> +
>> + err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
>
> This leaves a pointer into "priv" in the devres list...
>
>> + if (err) {
>> + dev_err(dev, "failed to get clocks %d\n", err);
>> + return err;
>> + }
>> +
>> + err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>> + if (err)
>> + dev_err(dev, "failed to enable clocks %d\n", err);
>> +
>> + priv->num_other_clks = num_clks;
>> +
>> + return err;
>> +}
>> +
>> /*
>> * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>> * so that each DMA transfer doesn't exceed the boundary.
> [...]
>> @@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>> err_clk:
>> clk_disable_unprepare(pltfm_host->clk);
>> clk_disable_unprepare(priv->bus_clk);
>> - if (rk_priv)
>> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
>> - rk_priv->rockchip_clks);
>> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>> free_pltfm:
>> sdhci_pltfm_free(pdev);
>
> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
> regulator isn't ready yet, that "priv" is freed here, so by the time the
> devres callbacks eventually run, that "devres->clks" pointer which used
> to represent "priv->other_clocks" points to who knows what, and this
> sort of thing happens:
>
> [ 12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
> [ 12.472104] Mem abort info:
> [ 12.472471] ESR = 0x0000000096000004
> [ 12.475991] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 12.476657] SET = 0, FnV = 0
> [ 12.477146] EA = 0, S1PTW = 0
> [ 12.477547] FSC = 0x04: level 0 translation fault
> [ 12.478127] Data abort info:
> [ 12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
> [ 12.478413] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [ 12.479826] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [ 12.480418] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [ 12.481282] [002df7b378917664] address between user and kernel address ranges
> [ 12.482421] Internal error: Oops: 0000000096000004 [#1] SMP
> [ 12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
> [ 12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY Debian 6.16~rc7-1
> [ 12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
> [ 12.488412] Workqueue: async async_run_entry_fn
> [ 12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 12.489539] pc : __clk_put+0x2c/0x138
> [ 12.489913] lr : __clk_put+0x2c/0x138
> [ 12.490281] sp : ffff800080713b10
> [ 12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
> [ 12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
> [ 12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
> [ 12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
> [ 12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
> [ 12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
> [ 12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
> [ 12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
> [ 12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
> [ 12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
> [ 12.497524] Call trace:
> [ 12.497776] __clk_put+0x2c/0x138 (P)
> [ 12.498154] clk_put+0x18/0x30
> [ 12.498471] clk_bulk_put+0x40/0x68
> [ 12.498825] devm_clk_bulk_release+0x24/0x40
> [ 12.499248] release_nodes+0x64/0xa0
> [ 12.499608] devres_release_all+0x98/0xf8
> [ 12.500004] device_unbind_cleanup+0x20/0x70
> [ 12.500426] really_probe+0x1e8/0x3a0
> [ 12.500793] __driver_probe_device+0x84/0x160
> [ 12.501225] driver_probe_device+0x44/0x128
> [ 12.501640] __driver_attach_async_helper+0x5c/0x108
> [ 12.502125] async_run_entry_fn+0x40/0x180
> [ 12.502535] process_one_work+0x23c/0x640
> [ 12.502939] worker_thread+0x1b4/0x360
> [ 12.503315] kthread+0x150/0x250
> [ 12.503646] ret_from_fork+0x10/0x20
> [ 12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
> [ 12.504598] ---[ end trace 0000000000000000 ]---
>
>
> TBH I'm not sure what to do as a straight revert seems impractical by
> now, so we hope someone else might have a good idea.
Presumably the problem has gone away with:
commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
Author: Binbin Zhou <zhoubinbin@loongson.cn>
Date: Sat Jun 7 15:39:01 2025 +0800
mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
which is in next.
In which case a separate fix is needed for stable.
Hi Adrian,
On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote:
> On 22/07/2025 21:33, Robin Murphy wrote:
>> A bit late for a "review", but Diederik and I have just been
>> IRC-debugging a crash on RK3568 which by inspection seems to be caused
>> by this patch:
>>
>> On 2024-08-05 10:17 am, Chen Wang wrote:
>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>
>>> In addition to the required core clock and optional
>>> bus clock, the soc will expand its own clocks, so
>>> the bulk clock mechanism is abstracted.
>>>
>>> Note, I call the bulk clocks as "other clocks" due
>>> to the bus clock has been called as "optional".
>>>
>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>>> ---
>> [...]
>>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>>> + struct dwcmshc_priv *priv,
>>> + int num_clks,
>>> + const char * const clk_ids[])
>>> +{
>>> + int err;
>>> +
>>> + if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>>> + return -EINVAL;
>>> +
>>> + for (int i = 0; i < num_clks; i++)
>>> + priv->other_clks[i].id = clk_ids[i];
>>> +
>>> + err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
>>
>> This leaves a pointer into "priv" in the devres list...
>>
>>> + if (err) {
>>> + dev_err(dev, "failed to get clocks %d\n", err);
>>> + return err;
>>> + }
>>> +
>>> + err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>>> + if (err)
>>> + dev_err(dev, "failed to enable clocks %d\n", err);
>>> +
>>> + priv->num_other_clks = num_clks;
>>> +
>>> + return err;
>>> +}
>>> +
>>> /*
>>> * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>> * so that each DMA transfer doesn't exceed the boundary.
>> [...]
>>> @@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>> err_clk:
>>> clk_disable_unprepare(pltfm_host->clk);
>>> clk_disable_unprepare(priv->bus_clk);
>>> - if (rk_priv)
>>> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
>>> - rk_priv->rockchip_clks);
>>> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>>> free_pltfm:
>>> sdhci_pltfm_free(pdev);
>>
>> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
>> regulator isn't ready yet, that "priv" is freed here, so by the time the
>> devres callbacks eventually run, that "devres->clks" pointer which used
>> to represent "priv->other_clocks" points to who knows what, and this
>> sort of thing happens:
>>
>> [ 12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
>> [ 12.472104] Mem abort info:
>> [ 12.472471] ESR = 0x0000000096000004
>> [ 12.475991] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 12.476657] SET = 0, FnV = 0
>> [ 12.477146] EA = 0, S1PTW = 0
>> [ 12.477547] FSC = 0x04: level 0 translation fault
>> [ 12.478127] Data abort info:
>> [ 12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
>> [ 12.478413] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> [ 12.479826] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [ 12.480418] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [ 12.481282] [002df7b378917664] address between user and kernel address ranges
>> [ 12.482421] Internal error: Oops: 0000000096000004 [#1] SMP
>> [ 12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
>> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
>> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
>> [ 12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY Debian 6.16~rc7-1
>> [ 12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
>> [ 12.488412] Workqueue: async async_run_entry_fn
>> [ 12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [ 12.489539] pc : __clk_put+0x2c/0x138
>> [ 12.489913] lr : __clk_put+0x2c/0x138
>> [ 12.490281] sp : ffff800080713b10
>> [ 12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
>> [ 12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
>> [ 12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
>> [ 12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
>> [ 12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
>> [ 12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
>> [ 12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
>> [ 12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
>> [ 12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
>> [ 12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
>> [ 12.497524] Call trace:
>> [ 12.497776] __clk_put+0x2c/0x138 (P)
>> [ 12.498154] clk_put+0x18/0x30
>> [ 12.498471] clk_bulk_put+0x40/0x68
>> [ 12.498825] devm_clk_bulk_release+0x24/0x40
>> [ 12.499248] release_nodes+0x64/0xa0
>> [ 12.499608] devres_release_all+0x98/0xf8
>> [ 12.500004] device_unbind_cleanup+0x20/0x70
>> [ 12.500426] really_probe+0x1e8/0x3a0
>> [ 12.500793] __driver_probe_device+0x84/0x160
>> [ 12.501225] driver_probe_device+0x44/0x128
>> [ 12.501640] __driver_attach_async_helper+0x5c/0x108
>> [ 12.502125] async_run_entry_fn+0x40/0x180
>> [ 12.502535] process_one_work+0x23c/0x640
>> [ 12.502939] worker_thread+0x1b4/0x360
>> [ 12.503315] kthread+0x150/0x250
>> [ 12.503646] ret_from_fork+0x10/0x20
>> [ 12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
>> [ 12.504598] ---[ end trace 0000000000000000 ]---
>>
>>
>> TBH I'm not sure what to do as a straight revert seems impractical by
>> now, so we hope someone else might have a good idea.
>
> Presumably the problem has gone away with:
>
> commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
> Author: Binbin Zhou <zhoubinbin@loongson.cn>
> Date: Sat Jun 7 15:39:01 2025 +0800
>
> mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
>
> which is in next.
>
> In which case a separate fix is needed for stable.
Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies.
Thanks!
Cheers,
Diederik
On 24/07/2025 17:33, Diederik de Haas wrote:
> Hi Adrian,
>
> On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote:
>> On 22/07/2025 21:33, Robin Murphy wrote:
>>> A bit late for a "review", but Diederik and I have just been
>>> IRC-debugging a crash on RK3568 which by inspection seems to be caused
>>> by this patch:
>>>
>>> On 2024-08-05 10:17 am, Chen Wang wrote:
>>>> From: Chen Wang <unicorn_wang@outlook.com>
>>>>
>>>> In addition to the required core clock and optional
>>>> bus clock, the soc will expand its own clocks, so
>>>> the bulk clock mechanism is abstracted.
>>>>
>>>> Note, I call the bulk clocks as "other clocks" due
>>>> to the bus clock has been called as "optional".
>>>>
>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
>>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
>>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
>>>> ---
>>> [...]
>>>> +static int dwcmshc_get_enable_other_clks(struct device *dev,
>>>> + struct dwcmshc_priv *priv,
>>>> + int num_clks,
>>>> + const char * const clk_ids[])
>>>> +{
>>>> + int err;
>>>> +
>>>> + if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
>>>> + return -EINVAL;
>>>> +
>>>> + for (int i = 0; i < num_clks; i++)
>>>> + priv->other_clks[i].id = clk_ids[i];
>>>> +
>>>> + err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
>>>
>>> This leaves a pointer into "priv" in the devres list...
>>>
>>>> + if (err) {
>>>> + dev_err(dev, "failed to get clocks %d\n", err);
>>>> + return err;
>>>> + }
>>>> +
>>>> + err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
>>>> + if (err)
>>>> + dev_err(dev, "failed to enable clocks %d\n", err);
>>>> +
>>>> + priv->num_other_clks = num_clks;
>>>> +
>>>> + return err;
>>>> +}
>>>> +
>>>> /*
>>>> * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>>>> * so that each DMA transfer doesn't exceed the boundary.
>>> [...]
>>>> @@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
>>>> err_clk:
>>>> clk_disable_unprepare(pltfm_host->clk);
>>>> clk_disable_unprepare(priv->bus_clk);
>>>> - if (rk_priv)
>>>> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
>>>> - rk_priv->rockchip_clks);
>>>> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>>>> free_pltfm:
>>>> sdhci_pltfm_free(pdev);
>>>
>>> ...but upon, say, -EPROBE_DEFER from sdhci_setup_host() because a
>>> regulator isn't ready yet, that "priv" is freed here, so by the time the
>>> devres callbacks eventually run, that "devres->clks" pointer which used
>>> to represent "priv->other_clocks" points to who knows what, and this
>>> sort of thing happens:
>>>
>>> [ 12.470827] Unable to handle kernel paging request at virtual address 002df7b378917664
>>> [ 12.472104] Mem abort info:
>>> [ 12.472471] ESR = 0x0000000096000004
>>> [ 12.475991] EC = 0x25: DABT (current EL), IL = 32 bits
>>> [ 12.476657] SET = 0, FnV = 0
>>> [ 12.477146] EA = 0, S1PTW = 0
>>> [ 12.477547] FSC = 0x04: level 0 translation fault
>>> [ 12.478127] Data abort info:
>>> [ 12.478126] rockchip-gpio fdd60000.gpio: probed /pinctrl/gpio@fdd60000
>>> [ 12.478413] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>>> [ 12.479826] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>>> [ 12.480418] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>>> [ 12.481282] [002df7b378917664] address between user and kernel address ranges
>>> [ 12.482421] Internal error: Oops: 0000000096000004 [#1] SMP
>>> [ 12.482980] Modules linked in: sdhci_of_dwcmshc drm_dp_aux_bus gpio_rockchip(+) drm_display_helper dw_mmc_rockchip drm_client_lib sdhci_pltfm drm_dma_helper fwnode_mdio sdhci dw_mmc_pltf
>>> m libphy fixed rockchip_dfi drm_kms_helper cqhci pl330(+) phy_rockchip_naneng_combphy dw_wdt phy_rockchip_snps_pcie3 phy_rockchip_inno_usb2 dw_mmc mdio_bus dwc3 ehci_platform ohci_platform
>>> ehci_hcd drm ohci_hcd udc_core io_domain i2c_rk3x usbcore ulpi usb_common
>>> [ 12.486871] CPU: 0 UID: 0 PID: 64 Comm: kworker/u16:3 Not tainted 6.16-rc7-arm64-cknow #1 PREEMPTLAZY Debian 6.16~rc7-1
>>> [ 12.487901] Hardware name: FriendlyElec NanoPi R5S (DT)
>>> [ 12.488412] Workqueue: async async_run_entry_fn
>>> [ 12.488879] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> [ 12.489539] pc : __clk_put+0x2c/0x138
>>> [ 12.489913] lr : __clk_put+0x2c/0x138
>>> [ 12.490281] sp : ffff800080713b10
>>> [ 12.490607] x29: ffff800080713b10 x28: ffff0001f001a120 x27: 0000000000000000
>>> [ 12.491302] x26: ffff0001f98e01a0 x25: 0000000000000000 x24: ffff0001f0f35408
>>> [ 12.491995] x23: ffffa8da199b4b40 x22: ffff800080713bb0 x21: ffff0001f0f35010
>>> [ 12.492689] x20: ffff0001f94aafd0 x19: 0a2df7b378917634 x18: 00000000ffffffff
>>> [ 12.493381] x17: 3d4d455453595342 x16: 555300307075656b x15: ffff0001f4885650
>>> [ 12.494075] x14: 0000000000000000 x13: ffff0001f025b810 x12: 0000000000008000
>>> [ 12.494765] x11: ffffa8da1a73ef98 x10: ffffa8da1a460000 x9 : 0000000000000078
>>> [ 12.495454] x8 : 0000000000000049 x7 : ffffa8da18c2fbe0 x6 : 0000000000000001
>>> [ 12.496145] x5 : 0000000000000004 x4 : 000000006cb6bb63 x3 : 0000000000000000
>>> [ 12.496833] x2 : 0000000000000000 x1 : ffff0001f1365ac0 x0 : 0000000000000001
>>> [ 12.497524] Call trace:
>>> [ 12.497776] __clk_put+0x2c/0x138 (P)
>>> [ 12.498154] clk_put+0x18/0x30
>>> [ 12.498471] clk_bulk_put+0x40/0x68
>>> [ 12.498825] devm_clk_bulk_release+0x24/0x40
>>> [ 12.499248] release_nodes+0x64/0xa0
>>> [ 12.499608] devres_release_all+0x98/0xf8
>>> [ 12.500004] device_unbind_cleanup+0x20/0x70
>>> [ 12.500426] really_probe+0x1e8/0x3a0
>>> [ 12.500793] __driver_probe_device+0x84/0x160
>>> [ 12.501225] driver_probe_device+0x44/0x128
>>> [ 12.501640] __driver_attach_async_helper+0x5c/0x108
>>> [ 12.502125] async_run_entry_fn+0x40/0x180
>>> [ 12.502535] process_one_work+0x23c/0x640
>>> [ 12.502939] worker_thread+0x1b4/0x360
>>> [ 12.503315] kthread+0x150/0x250
>>> [ 12.503646] ret_from_fork+0x10/0x20
>>> [ 12.504015] Code: aa0003f3 b140041f 540006c8 97ffd9c4 (b9403260)
>>> [ 12.504598] ---[ end trace 0000000000000000 ]---
>>>
>>>
>>> TBH I'm not sure what to do as a straight revert seems impractical by
>>> now, so we hope someone else might have a good idea.
>>
>> Presumably the problem has gone away with:
>>
>> commit 91a001a1a0749e5d24606d46ac5dfd4433c00956
>> Author: Binbin Zhou <zhoubinbin@loongson.cn>
>> Date: Sat Jun 7 15:39:01 2025 +0800
>>
>> mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free()
>>
>> which is in next.
>>
>> In which case a separate fix is needed for stable.
>
> Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies.
> Thanks!
You need the other patches that it depends on, otherwise you are
just leaking the memory. Refer:
https://lore.kernel.org/all/cover.1749127796.git.zhoubinbin@loongson.cn/
On Thu Jul 24, 2025 at 4:57 PM CEST, Adrian Hunter wrote: > On 24/07/2025 17:33, Diederik de Haas wrote: >> On Wed Jul 23, 2025 at 7:33 AM CEST, Adrian Hunter wrote: >>> On 22/07/2025 21:33, Robin Murphy wrote: >>>> A bit late for a "review", but Diederik and I have just been >>>> IRC-debugging a crash on RK3568 which by inspection seems to be caused >>>> by this patch: >>>> >>>> On 2024-08-05 10:17 am, Chen Wang wrote: >>>>> From: Chen Wang <unicorn_wang@outlook.com> >>>>> >>>>> In addition to the required core clock and optional >>>>> bus clock, the soc will expand its own clocks, so >>>>> the bulk clock mechanism is abstracted. >>>>> >>>>> Note, I call the bulk clocks as "other clocks" due >>>>> to the bus clock has been called as "optional". >>>>> >>>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>>> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520 >>>>> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi >>>>> --- >>> >>> Presumably the problem has gone away with: >>> >>> commit 91a001a1a0749e5d24606d46ac5dfd4433c00956 >>> Author: Binbin Zhou <zhoubinbin@loongson.cn> >>> Date: Sat Jun 7 15:39:01 2025 +0800 >>> >>> mmc: sdhci-of-dwcmshc: Drop the use of sdhci_pltfm_free() >>> >>> which is in next. >>> >>> In which case a separate fix is needed for stable. >> >> Adding that patch to my 6.16-rc7 kernel indeed stopped the OOPSies. >> Thanks! > > You need the other patches that it depends on, otherwise you are > just leaking the memory. Refer: > > https://lore.kernel.org/all/cover.1749127796.git.zhoubinbin@loongson.cn/ Also with the other patches, the OOPSies stopped :-) Cheers, Diederik
On 5/08/24 12:17, Chen Wang wrote:
> From: Chen Wang <unicorn_wang@outlook.com>
>
> In addition to the required core clock and optional
> bus clock, the soc will expand its own clocks, so
> the bulk clock mechanism is abstracted.
>
> Note, I call the bulk clocks as "other clocks" due
> to the bus clock has been called as "optional".
>
> Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
> Tested-by: Drew Fustini <drew@pdp7.com> # TH1520
> Tested-by: Inochi Amaoto <inochiama@outlook.com> # Duo and Huashan Pi
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-of-dwcmshc.c | 90 +++++++++++++++--------------
> 1 file changed, 48 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index e79aa4b3b6c3..35401616fb2e 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -108,7 +108,6 @@
> #define DLL_LOCK_WO_TMOUT(x) \
> ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
> (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
> -#define RK35xx_MAX_CLKS 3
>
> /* PHY register area pointer */
> #define DWC_MSHC_PTR_PHY_R 0x300
> @@ -199,23 +198,54 @@ enum dwcmshc_rk_type {
> };
>
> struct rk35xx_priv {
> - /* Rockchip specified optional clocks */
> - struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS];
> struct reset_control *reset;
> enum dwcmshc_rk_type devtype;
> u8 txclk_tapnum;
> };
>
> +#define DWCMSHC_MAX_OTHER_CLKS 3
> +
> struct dwcmshc_priv {
> struct clk *bus_clk;
> int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
> int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
>
> + int num_other_clks;
> + struct clk_bulk_data other_clks[DWCMSHC_MAX_OTHER_CLKS];
> +
> void *priv; /* pointer to SoC private stuff */
> u16 delay_line;
> u16 flags;
> };
>
> +static int dwcmshc_get_enable_other_clks(struct device *dev,
> + struct dwcmshc_priv *priv,
> + int num_clks,
> + const char * const clk_ids[])
> +{
> + int err;
> +
> + if (num_clks > DWCMSHC_MAX_OTHER_CLKS)
> + return -EINVAL;
> +
> + for (int i = 0; i < num_clks; i++)
> + priv->other_clks[i].id = clk_ids[i];
> +
> + err = devm_clk_bulk_get_optional(dev, num_clks, priv->other_clks);
> + if (err) {
> + dev_err(dev, "failed to get clocks %d\n", err);
> + return err;
> + }
> +
> + err = clk_bulk_prepare_enable(num_clks, priv->other_clks);
> + if (err)
> + dev_err(dev, "failed to enable clocks %d\n", err);
> +
> + priv->num_other_clks = num_clks;
> +
> + return err;
> +}
> +
> /*
> * If DMA addr spans 128MB boundary, we split the DMA transfer into two
> * so that each DMA transfer doesn't exceed the boundary.
> @@ -1036,8 +1066,9 @@ static void dwcmshc_cqhci_init(struct sdhci_host *host, struct platform_device *
>
> static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> {
> - int err;
> + static const char * const clk_ids[] = {"axi", "block", "timer"};
> struct rk35xx_priv *priv = dwc_priv->priv;
> + int err;
>
> priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> if (IS_ERR(priv->reset)) {
> @@ -1046,21 +1077,10 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
> return err;
> }
>
> - priv->rockchip_clks[0].id = "axi";
> - priv->rockchip_clks[1].id = "block";
> - priv->rockchip_clks[2].id = "timer";
> - err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS,
> - priv->rockchip_clks);
> - if (err) {
> - dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err);
> - return err;
> - }
> -
> - err = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, priv->rockchip_clks);
> - if (err) {
> - dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err);
> + err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
> + ARRAY_SIZE(clk_ids), clk_ids);
> + if (err)
> return err;
> - }
>
> if (of_property_read_u8(mmc_dev(host->mmc)->of_node, "rockchip,txclk-tapnum",
> &priv->txclk_tapnum))
> @@ -1280,9 +1300,7 @@ static int dwcmshc_probe(struct platform_device *pdev)
> err_clk:
> clk_disable_unprepare(pltfm_host->clk);
> clk_disable_unprepare(priv->bus_clk);
> - if (rk_priv)
> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
> free_pltfm:
> sdhci_pltfm_free(pdev);
> return err;
> @@ -1304,7 +1322,6 @@ static void dwcmshc_remove(struct platform_device *pdev)
> struct sdhci_host *host = platform_get_drvdata(pdev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> - struct rk35xx_priv *rk_priv = priv->priv;
>
> pm_runtime_get_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> @@ -1316,9 +1333,7 @@ static void dwcmshc_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(pltfm_host->clk);
> clk_disable_unprepare(priv->bus_clk);
> - if (rk_priv)
> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
> sdhci_pltfm_free(pdev);
> }
>
> @@ -1328,7 +1343,6 @@ static int dwcmshc_suspend(struct device *dev)
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> - struct rk35xx_priv *rk_priv = priv->priv;
> int ret;
>
> pm_runtime_resume(dev);
> @@ -1347,9 +1361,7 @@ static int dwcmshc_suspend(struct device *dev)
> if (!IS_ERR(priv->bus_clk))
> clk_disable_unprepare(priv->bus_clk);
>
> - if (rk_priv)
> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>
> return ret;
> }
> @@ -1359,7 +1371,6 @@ static int dwcmshc_resume(struct device *dev)
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host);
> - struct rk35xx_priv *rk_priv = priv->priv;
> int ret;
>
> ret = clk_prepare_enable(pltfm_host->clk);
> @@ -1372,29 +1383,24 @@ static int dwcmshc_resume(struct device *dev)
> goto disable_clk;
> }
>
> - if (rk_priv) {
> - ret = clk_bulk_prepare_enable(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> - if (ret)
> - goto disable_bus_clk;
> - }
> + ret = clk_bulk_prepare_enable(priv->num_other_clks, priv->other_clks);
> + if (ret)
> + goto disable_bus_clk;
>
> ret = sdhci_resume_host(host);
> if (ret)
> - goto disable_rockchip_clks;
> + goto disable_other_clks;
>
> if (host->mmc->caps2 & MMC_CAP2_CQE) {
> ret = cqhci_resume(host->mmc);
> if (ret)
> - goto disable_rockchip_clks;
> + goto disable_other_clks;
> }
>
> return 0;
>
> -disable_rockchip_clks:
> - if (rk_priv)
> - clk_bulk_disable_unprepare(RK35xx_MAX_CLKS,
> - rk_priv->rockchip_clks);
> +disable_other_clks:
> + clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
> disable_bus_clk:
> if (!IS_ERR(priv->bus_clk))
> clk_disable_unprepare(priv->bus_clk);
© 2016 - 2025 Red Hat, Inc.