Add support for mmc hardware reset using a reset-controller
that would need to be enabled in the device tree with
a supporting driver. The default is disabled for all
existing designs.
Signed-off-by: Brad Larson <[email protected]>
---
Changes since v6:
- Previously patch 17/17
- Changed delay after reset_control_assert() from 9 to 3 usec
- Renamed sdhci_mmc_hw_reset() to sdhci_cdns_mmc_hw_reset()
---
drivers/mmc/host/sdhci-cadence.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index e92aa79a8be2..62321cef41db 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -12,6 +12,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/reset.h>
#include "sdhci-pltfm.h"
@@ -70,6 +71,7 @@ struct sdhci_cdns_priv {
spinlock_t wrlock; /* write lock */
bool enhanced_strobe;
void (*priv_writel)(struct sdhci_cdns_priv *priv, u32 val, void __iomem *reg);
+ struct reset_control *rst_hw;
unsigned int nr_phy_params;
struct sdhci_cdns_phy_param phy_params[];
};
@@ -458,6 +460,24 @@ static void sdhci_cdns_hs400_enhanced_strobe(struct mmc_host *mmc,
SDHCI_CDNS_HRS06_MODE_MMC_HS400);
}
+extern unsigned int sdhci_timeout_val;
+
+static void sdhci_cdns_mmc_hw_reset(struct mmc_host *mmc)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+
+ dev_dbg(mmc_dev(host->mmc), "emmc hardware reset\n");
+
+ reset_control_assert(priv->rst_hw);
+ /* For eMMC, minimum is 1us but give it 3us for good measure */
+ udelay(3);
+
+ reset_control_deassert(priv->rst_hw);
+ /* For eMMC, minimum is 200us but give it 300us for good measure */
+ usleep_range(300, 1000);
+}
+
static int sdhci_cdns_probe(struct platform_device *pdev)
{
struct sdhci_host *host;
@@ -521,6 +541,17 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
if (ret)
goto free;
+ if (host->mmc->caps & MMC_CAP_HW_RESET) {
+ priv->rst_hw = devm_reset_control_get_optional_exclusive(dev, "hw");
+ if (IS_ERR(priv->rst_hw)) {
+ ret = PTR_ERR(priv->rst_hw);
+ if (ret == -ENOENT)
+ priv->rst_hw = NULL;
+ } else {
+ host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
+ }
+ }
+
ret = sdhci_add_host(host);
if (ret)
goto free;
--
2.17.1
On Thu, Jan 19, 2023, at 04:39, Brad Larson wrote:
>
> +extern unsigned int sdhci_timeout_val;
> +
This declaration should not be in the .c file, and I don't think
there should be a global variable with this overly generic name either.
Arnd
Hi Brad,
On Wed, Jan 18, 2023 at 07:51:35PM -0800, Brad Larson wrote:
> Add support for mmc hardware reset using a reset-controller
> that would need to be enabled in the device tree with
> a supporting driver. The default is disabled for all
> existing designs.
>
> Signed-off-by: Brad Larson <[email protected]>
> ---
>
> Changes since v6:
> - Previously patch 17/17
> - Changed delay after reset_control_assert() from 9 to 3 usec
> - Renamed sdhci_mmc_hw_reset() to sdhci_cdns_mmc_hw_reset()
>
> ---
> drivers/mmc/host/sdhci-cadence.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index e92aa79a8be2..62321cef41db 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -12,6 +12,7 @@
[...]
> static int sdhci_cdns_probe(struct platform_device *pdev)
> {
> struct sdhci_host *host;
> @@ -521,6 +541,17 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
> if (ret)
> goto free;
>
> + if (host->mmc->caps & MMC_CAP_HW_RESET) {
> + priv->rst_hw = devm_reset_control_get_optional_exclusive(dev, "hw");
> + if (IS_ERR(priv->rst_hw)) {
> + ret = PTR_ERR(priv->rst_hw);
> + if (ret == -ENOENT)
> + priv->rst_hw = NULL;
The optional reset_control_get variants return NULL instead of -ENOENT
if no reset is specified.
This should return on any error instead.
> + } else {
> + host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
This probably shouldn't be set if reset_control_get_optional returned NULL.
> + }
> + }
> +
> ret = sdhci_add_host(host);
regards
Philipp
On Thu, Jan 19, 2023, at 09:57, Arnd Bergmann wrote:
> On Thu, Jan 19, 2023, at 04:39, Brad Larson wrote:
>>
>> +extern unsigned int sdhci_timeout_val;
>> +
>
> This declaration should not be in the .c file, and I don't think
> there should be a global variable with this overly generic name either.
Thanks for pointing this out, I've deleted that line. It was leftover test
code to force a timeout to trigger the reset driver and reset the timeout.
Regards,
Brad
Hi Philipp,
On Thu, Jan 19, 2023 at 10:34, Philipp Zabel wrote:
> On Wed, Jan 18, 2023 at 07:51:35PM -0800, Brad Larson wrote:
>> Add support for mmc hardware reset using a reset-controller
>> that would need to be enabled in the device tree with
>> a supporting driver. The default is disabled for all
>> existing designs.
>>
>> Signed-off-by: Brad Larson <[email protected]>
>> ---
>>
>> Changes since v6:
>> - Previously patch 17/17
>> - Changed delay after reset_control_assert() from 9 to 3 usec
>> - Renamed sdhci_mmc_hw_reset() to sdhci_cdns_mmc_hw_reset()
>>
>> ---
>> drivers/mmc/host/sdhci-cadence.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
>> index e92aa79a8be2..62321cef41db 100644
>> --- a/drivers/mmc/host/sdhci-cadence.c
>> +++ b/drivers/mmc/host/sdhci-cadence.c
>> @@ -12,6 +12,7 @@
[...]
>> static int sdhci_cdns_probe(struct platform_device *pdev)
>> {
>> struct sdhci_host *host;
>> @@ -521,6 +541,17 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
>> if (ret)
>> goto free;
>>
>> + if (host->mmc->caps & MMC_CAP_HW_RESET) {
>> + priv->rst_hw = devm_reset_control_get_optional_exclusive(dev, "hw");
>> + if (IS_ERR(priv->rst_hw)) {
>> + ret = PTR_ERR(priv->rst_hw);
>> + if (ret == -ENOENT)
>> + priv->rst_hw = NULL;
>
> The optional reset_control_get variants return NULL instead of -ENOENT
> if no reset is specified.
>
> This should return on any error instead.
>
>> + } else {
>> + host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
>
> This probably shouldn't be set if reset_control_get_optional returned NULL.
[...]
Thanks I see now with the argument optional=true in __of_reset_control_get() it is
returning NULL and not -ENOENT. This is the updated version.
+ if (host->mmc->caps & MMC_CAP_HW_RESET) {
+ priv->rst_hw = devm_reset_control_get_optional_exclusive(dev, "hw");
+ if (IS_ERR(priv->rst_hw))
+ return dev_err_probe(mmc_dev(host->mmc), PTR_ERR(priv->rst_hw),
+ "reset controller error\n");
+ if (priv->rst_hw)
+ host->mmc_host_ops.card_hw_reset = sdhci_cdns_mmc_hw_reset;
+ }
Regards,
Brad