Hello Ulf,
here are two small cleanups for the meson-mx-sdhc driver.
The first one is resolves a TODO comment from when the driver was
originally introduced where devm_clk_hw_get_clk() was not available
yet. Nowadays it is and we can and should use it.
The second one removes an incorrect .card_hw_reset callback from the
driver's mmc_host_ops. This part has never been correct as we're
resetting the host controller, not the card. It didn't seem to cause
any issues so far, which is why I don't think that it's -stable
material.
Martin Blumenstingl (2):
mmc: meson-mx-sdhc: Use devm_clk_hw_get_clk() for clock retrieval
mmc: meson-mx-sdhc: Remove .card_hw_reset callback
drivers/mmc/host/meson-mx-sdhc-clkc.c | 43 ++++++++++++++-------------
drivers/mmc/host/meson-mx-sdhc-mmc.c | 13 ++++----
2 files changed, 28 insertions(+), 28 deletions(-)
--
2.43.2
Commit 32f18e596141 ("mmc: improve API to make clear hw_reset callback
is for cards") made it clear that the hw_reset callback is intended for
resetting the card. Remove the .card_hw_reset callback from the
meson-mx-sdhc-mmc driver because it's purpose is to reset the SDHC
controller (FIFOs, PHY, DMA interface, ...).
While here also rename and change the argument of meson_mx_sdhc_hw_reset
so it cannot be called by accident as a replacement for card_hw_reset in
the future.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/mmc/host/meson-mx-sdhc-mmc.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index 1ed9731e77ef..31f750301dc1 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -65,10 +65,8 @@ static const struct regmap_config meson_mx_sdhc_regmap_config = {
.max_register = MESON_SDHC_CLK2,
};
-static void meson_mx_sdhc_hw_reset(struct mmc_host *mmc)
+static void meson_mx_sdhc_reset(struct meson_mx_sdhc_host *host)
{
- struct meson_mx_sdhc_host *host = mmc_priv(mmc);
-
regmap_write(host->regmap, MESON_SDHC_SRST, MESON_SDHC_SRST_MAIN_CTRL |
MESON_SDHC_SRST_RXFIFO | MESON_SDHC_SRST_TXFIFO |
MESON_SDHC_SRST_DPHY_RX | MESON_SDHC_SRST_DPHY_TX |
@@ -116,7 +114,7 @@ static void meson_mx_sdhc_wait_cmd_ready(struct mmc_host *mmc)
dev_warn(mmc_dev(mmc),
"Failed to poll for CMD_BUSY while processing CMD%d\n",
host->cmd->opcode);
- meson_mx_sdhc_hw_reset(mmc);
+ meson_mx_sdhc_reset(host);
}
ret = regmap_read_poll_timeout(host->regmap, MESON_SDHC_ESTA, esta,
@@ -127,7 +125,7 @@ static void meson_mx_sdhc_wait_cmd_ready(struct mmc_host *mmc)
dev_warn(mmc_dev(mmc),
"Failed to poll for ESTA[13:11] while processing CMD%d\n",
host->cmd->opcode);
- meson_mx_sdhc_hw_reset(mmc);
+ meson_mx_sdhc_reset(host);
}
}
@@ -495,7 +493,6 @@ static int meson_mx_sdhc_execute_tuning(struct mmc_host *mmc, u32 opcode)
}
static const struct mmc_host_ops meson_mx_sdhc_ops = {
- .card_hw_reset = meson_mx_sdhc_hw_reset,
.request = meson_mx_sdhc_request,
.set_ios = meson_mx_sdhc_set_ios,
.card_busy = meson_mx_sdhc_card_busy,
@@ -618,7 +615,7 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
}
if (cmd->error == -EIO || cmd->error == -ETIMEDOUT)
- meson_mx_sdhc_hw_reset(host->mmc);
+ meson_mx_sdhc_reset(host);
else if (cmd->data)
/*
* Clear the FIFOs after completing data transfers to prevent
@@ -728,7 +725,7 @@ static void meson_mx_sdhc_init_hw(struct mmc_host *mmc)
{
struct meson_mx_sdhc_host *host = mmc_priv(mmc);
- meson_mx_sdhc_hw_reset(mmc);
+ meson_mx_sdhc_reset(host);
regmap_write(host->regmap, MESON_SDHC_CTRL,
FIELD_PREP(MESON_SDHC_CTRL_RX_PERIOD, 0xf) |
--
2.43.2
Now that devm_clk_hw_get_clk() has been available for a while we can
resolve an older TODO where this API did not exist yet. No functional
changes intended.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/mmc/host/meson-mx-sdhc-clkc.c | 43 ++++++++++++++-------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/mmc/host/meson-mx-sdhc-clkc.c b/drivers/mmc/host/meson-mx-sdhc-clkc.c
index 19200b7079a6..cbd17a596cd2 100644
--- a/drivers/mmc/host/meson-mx-sdhc-clkc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-clkc.c
@@ -71,12 +71,23 @@ static int meson_mx_sdhc_clk_hw_register(struct device *dev,
static int meson_mx_sdhc_gate_clk_hw_register(struct device *dev,
const char *name_suffix,
struct clk_hw *parent,
- struct clk_hw *hw)
+ struct clk_hw *hw,
+ struct clk_bulk_data *clk_bulk_data,
+ u8 bulk_index)
{
struct clk_parent_data parent_data = { .hw = parent };
+ int ret;
+
+ ret = meson_mx_sdhc_clk_hw_register(dev, name_suffix, &parent_data, 1,
+ &clk_gate_ops, hw);
+ if (ret)
+ return ret;
+
+ clk_bulk_data[bulk_index].clk = devm_clk_hw_get_clk(dev, hw, name_suffix);
+ if (IS_ERR(clk_bulk_data[bulk_index].clk))
+ return PTR_ERR(clk_bulk_data[bulk_index].clk);
- return meson_mx_sdhc_clk_hw_register(dev, name_suffix, &parent_data, 1,
- &clk_gate_ops, hw);
+ return 0;
}
int meson_mx_sdhc_register_clkc(struct device *dev, void __iomem *base,
@@ -115,7 +126,8 @@ int meson_mx_sdhc_register_clkc(struct device *dev, void __iomem *base,
clkc_data->mod_clk_en.bit_idx = 15;
ret = meson_mx_sdhc_gate_clk_hw_register(dev, "mod_clk_on",
&clkc_data->div.hw,
- &clkc_data->mod_clk_en.hw);
+ &clkc_data->mod_clk_en.hw,
+ clk_bulk_data, 0);
if (ret)
return ret;
@@ -123,7 +135,8 @@ int meson_mx_sdhc_register_clkc(struct device *dev, void __iomem *base,
clkc_data->tx_clk_en.bit_idx = 14;
ret = meson_mx_sdhc_gate_clk_hw_register(dev, "tx_clk_on",
&clkc_data->div.hw,
- &clkc_data->tx_clk_en.hw);
+ &clkc_data->tx_clk_en.hw,
+ clk_bulk_data, 1);
if (ret)
return ret;
@@ -131,7 +144,8 @@ int meson_mx_sdhc_register_clkc(struct device *dev, void __iomem *base,
clkc_data->rx_clk_en.bit_idx = 13;
ret = meson_mx_sdhc_gate_clk_hw_register(dev, "rx_clk_on",
&clkc_data->div.hw,
- &clkc_data->rx_clk_en.hw);
+ &clkc_data->rx_clk_en.hw,
+ clk_bulk_data, 2);
if (ret)
return ret;
@@ -139,18 +153,7 @@ int meson_mx_sdhc_register_clkc(struct device *dev, void __iomem *base,
clkc_data->sd_clk_en.bit_idx = 12;
ret = meson_mx_sdhc_gate_clk_hw_register(dev, "sd_clk_on",
&clkc_data->div.hw,
- &clkc_data->sd_clk_en.hw);
- if (ret)
- return ret;
-
- /*
- * TODO: Replace clk_hw.clk with devm_clk_hw_get_clk() once that is
- * available.
- */
- clk_bulk_data[0].clk = clkc_data->mod_clk_en.hw.clk;
- clk_bulk_data[1].clk = clkc_data->sd_clk_en.hw.clk;
- clk_bulk_data[2].clk = clkc_data->tx_clk_en.hw.clk;
- clk_bulk_data[3].clk = clkc_data->rx_clk_en.hw.clk;
-
- return 0;
+ &clkc_data->sd_clk_en.hw,
+ clk_bulk_data, 3);
+ return ret;
}
--
2.43.2
On Sat, 17 Feb 2024 at 11:02, Martin Blumenstingl
<[email protected]> wrote:
>
> Hello Ulf,
>
> here are two small cleanups for the meson-mx-sdhc driver.
>
> The first one is resolves a TODO comment from when the driver was
> originally introduced where devm_clk_hw_get_clk() was not available
> yet. Nowadays it is and we can and should use it.
>
> The second one removes an incorrect .card_hw_reset callback from the
> driver's mmc_host_ops. This part has never been correct as we're
> resetting the host controller, not the card. It didn't seem to cause
> any issues so far, which is why I don't think that it's -stable
> material.
>
>
>
> Martin Blumenstingl (2):
> mmc: meson-mx-sdhc: Use devm_clk_hw_get_clk() for clock retrieval
> mmc: meson-mx-sdhc: Remove .card_hw_reset callback
>
> drivers/mmc/host/meson-mx-sdhc-clkc.c | 43 ++++++++++++++-------------
> drivers/mmc/host/meson-mx-sdhc-mmc.c | 13 ++++----
> 2 files changed, 28 insertions(+), 28 deletions(-)
Applied for next, thanks!
Kind regards
Uffe