2022-11-08 08:57:25

by Kunihiko Hayashi

[permalink] [raw]
Subject: [PATCH 1/4] mmc: f-sdh30: Add reset control support

Add reset control support for F_SDH30 controller. This is optional.

Signed-off-by: Kunihiko Hayashi <[email protected]>
---
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



2022-11-09 12:18:12

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: f-sdh30: Add reset control support

On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
<[email protected]> wrote:
>
> Add reset control support for F_SDH30 controller. This is optional.
>
> Signed-off-by: Kunihiko Hayashi <[email protected]>

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
>

2022-11-11 06:33:35

by Kunihiko Hayashi

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: f-sdh30: Add reset control support

Hi Ulf,


On 2022/11/09 21:15, Ulf Hansson wrote:
> On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
> <[email protected]> wrote:
>>
>> Add reset control support for F_SDH30 controller. This is optional.
>>
>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>
> 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

2022-11-11 12:55:12

by Kunihiko Hayashi

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: f-sdh30: Add reset control support

On 2022/11/11 21:13, Ulf Hansson wrote:
> On Fri, 11 Nov 2022 at 07:15, Kunihiko Hayashi
> <[email protected]> wrote:
>>
>> Hi Ulf,
>>
>>
>> On 2022/11/09 21:15, Ulf Hansson wrote:
>>> On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
>>> <[email protected]> wrote:
>>>>
>>>> Add reset control support for F_SDH30 controller. This is optional.
>>>>
>>>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>>>
>>> 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

2022-11-11 12:55:46

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 1/4] mmc: f-sdh30: Add reset control support

On Fri, 11 Nov 2022 at 07:15, Kunihiko Hayashi
<[email protected]> wrote:
>
> Hi Ulf,
>
>
> On 2022/11/09 21:15, Ulf Hansson wrote:
> > On Tue, 8 Nov 2022 at 09:25, Kunihiko Hayashi
> > <[email protected]> wrote:
> >>
> >> Add reset control support for F_SDH30 controller. This is optional.
> >>
> >> Signed-off-by: Kunihiko Hayashi <[email protected]>
> >
> > 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