Re: [PATCH v2 14/19] mmc: renesas-sdhi: Initialize module on RZ/G2L

On 10/9/23 18:46, Paul Barker wrote:

On the Renesas RZ/G2L SoC family, we must ensure that the required clock signals are enabled and the reset signal is de-asserted before we try to communicate with the SDHI module.

Signed-off-by: Paul Barker paul.barker.ct@bp.renesas.com Reviewed-by: Biju Das biju.das.jz@bp.renesas.com Reviewed-by: Lad Prabhakar prabhakar.mahadev-lad.rj@bp.renesas.com

---

v1->v2:

• Move RZ/G2L specific setup to its own function.

drivers/mmc/renesas-sdhi.c | 82 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c index 1536c9782041..370dbce51ca0 100644 --- a/drivers/mmc/renesas-sdhi.c +++ b/drivers/mmc/renesas-sdhi.c @@ -20,6 +20,7 @@ #include <linux/io.h> #include <linux/sizes.h> #include <power/regulator.h> +#include <reset.h> #include <asm/unaligned.h> #include "tmio-common.h"

@@ -958,6 +959,74 @@ static void renesas_sdhi_filter_caps(struct udevice *dev) priv->needs_clkh_fallback = false; }

+static int rzg2l_sdhi_setup(struct udevice *dev) +{

• struct tmio_sd_priv *priv = dev_get_priv(dev);
• struct clk imclk2, aclk;
• struct reset_ctl rst;
• int ret;
• /*

• * On members of the RZ/G2L SoC family, we need to enable
  

• * additional chip detect and bus clocks, then release the SDHI
  

• * module from reset.
  

• */
  

• ret = clk_get_by_name(dev, "cd", &imclk2);
• if (ret < 0) {

• dev_err(dev, "failed to get imclk2 (chip detect clk)\n");
  

• goto err_get_imclk2;
  

• }
• ret = clk_get_by_name(dev, "aclk", &aclk);
• if (ret < 0) {

• dev_err(dev, "failed to get aclk\n");
  

• goto err_get_aclk;
  

• }
• ret = clk_enable(&imclk2);
• if (ret < 0) {

• dev_err(dev, "failed to enable imclk2 (chip detect clk)\n");
  

• goto err_imclk2;
  

• }
• ret = clk_enable(&aclk);
• if (ret < 0) {

• dev_err(dev, "failed to enable aclk\n");
  

• goto err_aclk;
  

• }
• ret = reset_get_by_index(dev, 0, &rst);
• if (ret < 0) {

• dev_err(dev, "failed to get reset line\n");
  

• goto err_reset;
  

• }
• ret = reset_deassert(&rst);
• if (ret < 0) {

• dev_err(dev, "failed to de-assert reset line\n");
  

• goto err_reset;
  

• }
• ret = tmio_sd_probe(dev, priv->quirks);
• if (ret)

• goto err_tmio_probe;
  

• return 0;

+err_tmio_probe:

• reset_assert(&rst);

+err_reset:

• clk_disable(&aclk);

+err_aclk:

• clk_disable(&imclk2);

+err_imclk2:

• clk_free(&aclk);

+err_get_aclk:

• clk_free(&imclk2);

+err_get_imclk2:

• return ret;

+}

• static int renesas_sdhi_probe(struct udevice *dev) { struct tmio_sd_priv *priv = dev_get_priv(dev);

@@ -1012,9 +1081,15 @@ static int renesas_sdhi_probe(struct udevice *dev) goto err_clkh; }

• ret = tmio_sd_probe(dev, priv->quirks);
• if (ret)

• goto err_tmio_probe;
  

• if (IS_ENABLED(CONFIG_RZG2L)) {

Shouldn't there be some compatible string check here too ?

• ret = rzg2l_sdhi_setup(dev);
  

• if (ret)
  

• 	goto err_rzg2l_setup;
  

• } else {

• ret = tmio_sd_probe(dev, priv->quirks);
  

• if (ret)
  

• 	goto err_tmio_probe;
  

• }

  renesas_sdhi_filter_caps(dev);

@@ -1026,6 +1101,7 @@ static int renesas_sdhi_probe(struct udevice *dev) #endif return 0;

+err_rzg2l_setup: err_tmio_probe: clk_disable(&priv->clk);

I think here you can use err_tmio_probe() in any case.