[1/4] mmc: f-sdh30: Add reset control support

Message ID 20221108082533.21384-2-hayashi.kunihiko@socionext.com
State New
Headers
Series mmc: Add support for F_SDH30_E51 |

Commit Message

Kunihiko Hayashi Nov. 8, 2022, 8:25 a.m. UTC
  Add reset control support for F_SDH30 controller. This is optional.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/mmc/host/sdhci_f_sdh30.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
  

Comments

Ulf Hansson Nov. 9, 2022, 12:15 p.m. UTC | #1
On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Add reset control support for F_SDH30 controller. This is optional.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

This needs an update to the DT doc too, which is also the case for patch4.

That said, please convert the DT doc into the yaml based format as the
first step.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci_f_sdh30.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
> index 3f5977979cf2..7f4553b28180 100644
> --- a/drivers/mmc/host/sdhci_f_sdh30.c
> +++ b/drivers/mmc/host/sdhci_f_sdh30.c
> @@ -14,6 +14,7 @@
>  #include <linux/of.h>
>  #include <linux/property.h>
>  #include <linux/clk.h>
> +#include <linux/reset.h>
>
>  #include "sdhci-pltfm.h"
>  #include "sdhci_f_sdh30.h"
> @@ -21,6 +22,7 @@
>  struct f_sdhost_priv {
>         struct clk *clk_iface;
>         struct clk *clk;
> +       struct reset_control *rst;
>         u32 vendor_hs200;
>         struct device *dev;
>         bool enable_cmd_dat_delay;
> @@ -150,6 +152,16 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
>                 ret = clk_prepare_enable(priv->clk);
>                 if (ret)
>                         goto err_clk;
> +
> +               priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
> +               if (IS_ERR(priv->rst)) {
> +                       ret = PTR_ERR(priv->rst);
> +                       goto err_rst;
> +               }
> +
> +               ret = reset_control_deassert(priv->rst);
> +               if (ret)
> +                       goto err_rst;
>         }
>
>         /* init vendor specific regs */
> @@ -175,6 +187,8 @@ static int sdhci_f_sdh30_probe(struct platform_device *pdev)
>         return 0;
>
>  err_add_host:
> +       reset_control_assert(priv->rst);
> +err_rst:
>         clk_disable_unprepare(priv->clk);
>  err_clk:
>         clk_disable_unprepare(priv->clk_iface);
> @@ -191,8 +205,9 @@ static int sdhci_f_sdh30_remove(struct platform_device *pdev)
>         sdhci_remove_host(host, readl(host->ioaddr + SDHCI_INT_STATUS) ==
>                           0xffffffff);
>
> -       clk_disable_unprepare(priv->clk_iface);
> +       reset_control_assert(priv->rst);
>         clk_disable_unprepare(priv->clk);
> +       clk_disable_unprepare(priv->clk_iface);
>
>         sdhci_free_host(host);
>         platform_set_drvdata(pdev, NULL);
> --
> 2.25.1
>
  
Kunihiko Hayashi Nov. 11, 2022, 6:15 a.m. UTC | #2
Hi Ulf,


On 2022/11/09 21:15, Ulf Hansson wrote:
> On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
> <hayashi.kunihiko@socionext.com> wrote:
>>
>> Add reset control support for F_SDH30 controller. This is optional.
>>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> 
> This needs an update to the DT doc too, which is also the case for patch4.
> 
> That said, please convert the DT doc into the yaml based format as the
> first step.

Yes, I also think the document to be converted in order to add new compatible.
I'm concerned about the maintainer and the filename.

I'll convert it anyway.

---
Best Regards
Kunihiko Hayashi
  
Ulf Hansson Nov. 11, 2022, 12:13 p.m. UTC | #3
On Fri, 11 Nov 2022 at 07:15, Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Hi Ulf,
>
>
> On 2022/11/09 21:15, Ulf Hansson wrote:
> > On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
> > <hayashi.kunihiko@socionext.com> wrote:
> >>
> >> Add reset control support for F_SDH30 controller. This is optional.
> >>
> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> >
> > This needs an update to the DT doc too, which is also the case for patch4.
> >
> > That said, please convert the DT doc into the yaml based format as the
> > first step.
>
> Yes, I also think the document to be converted in order to add new compatible.
> I'm concerned about the maintainer and the filename.

If you can't find a maintainer from Socionext, feel free to put my
name in there.

I don't know if there are any good rules to apply for the filename in
cases like this. Let's just try something and see what DT maintainers
think of it. Perhaps just repeating the name of the driver for the
filename? So something along the lines of:
Documentation/devicetree/bindings/mmc/sdhci-f-sdh30.yaml

>
> I'll convert it anyway.

Great, thanks for doing this!

Kind regards
Uffe
  
Kunihiko Hayashi Nov. 11, 2022, 12:39 p.m. UTC | #4
On 2022/11/11 21:13, Ulf Hansson wrote:
> On Fri, 11 Nov 2022 at 07:15, Kunihiko Hayashi
> <hayashi.kunihiko@socionext.com> wrote:
>>
>> Hi Ulf,
>>
>>
>> On 2022/11/09 21:15, Ulf Hansson wrote:
>>> On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
>>> <hayashi.kunihiko@socionext.com> wrote:
>>>>
>>>> Add reset control support for F_SDH30 controller. This is optional.
>>>>
>>>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>>>
>>> This needs an update to the DT doc too, which is also the case for
>>> patch4.
>>>
>>> That said, please convert the DT doc into the yaml based format as the
>>> first step.
>>
>> Yes, I also think the document to be converted in order to add new
>> compatible.
>> I'm concerned about the maintainer and the filename.
> 
> If you can't find a maintainer from Socionext, feel free to put my
> name in there.

I see.
For now I describe my name in v2.

> 
> I don't know if there are any good rules to apply for the filename in
> cases like this. Let's just try something and see what DT maintainers
> think of it. Perhaps just repeating the name of the driver for the
> filename? So something along the lines of:
> Documentation/devicetree/bindings/mmc/sdhci-f-sdh30.yaml
> 
>>
>> I'll convert it anyway.
> 
> Great, thanks for doing this!

I sent v2 series and I'm waiting for the comment.

Thank you,

---
Best Regards
Kunihiko Hayashi
  

Patch

diff --git a/drivers/mmc/host/sdhci_f_sdh30.c b/drivers/mmc/host/sdhci_f_sdh30.c
index 3f5977979cf2..7f4553b28180 100644
--- a/drivers/mmc/host/sdhci_f_sdh30.c
+++ b/drivers/mmc/host/sdhci_f_sdh30.c
@@ -14,6 +14,7 @@ 
 #include <linux/of.h>
 #include <linux/property.h>
 #include <linux/clk.h>
+#include <linux/reset.h>
 
 #include "sdhci-pltfm.h"
 #include "sdhci_f_sdh30.h"
@@ -21,6 +22,7 @@ 
 struct f_sdhost_priv {
 	struct clk *clk_iface;
 	struct clk *clk;
+	struct reset_control *rst;
 	u32 vendor_hs200;
 	struct device *dev;
 	bool enable_cmd_dat_delay;
@@ -150,6 +152,16 @@  static int sdhci_f_sdh30_probe(struct platform_device *pdev)
 		ret = clk_prepare_enable(priv->clk);
 		if (ret)
 			goto err_clk;
+
+		priv->rst = devm_reset_control_get_optional_shared(dev, NULL);
+		if (IS_ERR(priv->rst)) {
+			ret = PTR_ERR(priv->rst);
+			goto err_rst;
+		}
+
+		ret = reset_control_deassert(priv->rst);
+		if (ret)
+			goto err_rst;
 	}
 
 	/* init vendor specific regs */
@@ -175,6 +187,8 @@  static int sdhci_f_sdh30_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_host:
+	reset_control_assert(priv->rst);
+err_rst:
 	clk_disable_unprepare(priv->clk);
 err_clk:
 	clk_disable_unprepare(priv->clk_iface);
@@ -191,8 +205,9 @@  static int sdhci_f_sdh30_remove(struct platform_device *pdev)
 	sdhci_remove_host(host, readl(host->ioaddr + SDHCI_INT_STATUS) ==
 			  0xffffffff);
 
-	clk_disable_unprepare(priv->clk_iface);
+	reset_control_assert(priv->rst);
 	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_iface);
 
 	sdhci_free_host(host);
 	platform_set_drvdata(pdev, NULL);