diff mbox series

[1/2] spi: dw: add reset control

Message ID 20200521170359.20430-1-dinguyen@kernel.org (mailing list archive)
State Superseded
Headers show
Series [1/2] spi: dw: add reset control | expand

Commit Message

Dinh Nguyen May 21, 2020, 5:03 p.m. UTC
Add mechanism to get the reset control and deassert it in order to bring
the IP out of reset.

Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Mark Brown May 21, 2020, 5:12 p.m. UTC | #1
On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> Add mechanism to get the reset control and deassert it in order to bring
> the IP out of reset.
> 
> Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

Is the From: correct here?

Also adding everyone else who's got patches in flight for this driver
right now.

> ---
>  drivers/spi/spi-dw-mmio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 384a3ab6dc2d..5c813e15ed89 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -20,6 +20,7 @@
>  #include <linux/acpi.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  
>  #include "spi-dw.h"
>  
> @@ -30,6 +31,7 @@ struct dw_spi_mmio {
>  	struct clk     *clk;
>  	struct clk     *pclk;
>  	void           *priv;
> +	struct reset_control	*rstc;
>  };
>  
>  #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
> @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>  	if (!dwsmmio)
>  		return -ENOMEM;
>  
> +	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
> +	if (!IS_ERR(dwsmmio->rstc))
> +		reset_control_deassert(dwsmmio->rstc);
> +
>  	dws = &dwsmmio->dws;
>  
>  	/* Get basic io resource and map it */

Should we also undo the reset in error paths from probe()?  It's not the
end of the world if we don't but...

> @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
>  	clk_disable_unprepare(dwsmmio->pclk);
>  	clk_disable_unprepare(dwsmmio->clk);
>  
> +	if (dwsmmio->rstc)
> +		reset_control_assert(dwsmmio->rstc);
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1
>
Serge Semin May 21, 2020, 5:28 p.m. UTC | #2
On Thu, May 21, 2020 at 06:12:03PM +0100, Mark Brown wrote:
> On Thu, May 21, 2020 at 12:03:58PM -0500, Dinh Nguyen wrote:
> > Add mechanism to get the reset control and deassert it in order to bring
> > the IP out of reset.
> > 
> > Signed-off-by: Liang Jin J <liang.j.jin@ericsson.com>
> > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> 
> Is the From: correct here?
> 
> Also adding everyone else who's got patches in flight for this driver
> right now.
> 
> > ---
> >  drivers/spi/spi-dw-mmio.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > index 384a3ab6dc2d..5c813e15ed89 100644
> > --- a/drivers/spi/spi-dw-mmio.c
> > +++ b/drivers/spi/spi-dw-mmio.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/acpi.h>
> >  #include <linux/property.h>
> >  #include <linux/regmap.h>
> > +#include <linux/reset.h>
> >  
> >  #include "spi-dw.h"
> >  
> > @@ -30,6 +31,7 @@ struct dw_spi_mmio {
> >  	struct clk     *clk;
> >  	struct clk     *pclk;
> >  	void           *priv;
> > +	struct reset_control	*rstc;
> >  };
> >  
> >  #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
> > @@ -145,6 +147,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
> >  	if (!dwsmmio)
> >  		return -ENOMEM;
> >  

> > +	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");

It would be better to use the optional-version of the reset getter:
devm_reset_control_get_optional_exclusive(), since for instance our DW APB SSI
doesn't have a dedicated reset line.

Note also that you don't need to check the rstc for being NULL. It's null if the
reset line is optional and isn't provided.

If we get to have this patch applied, then DT binding shall be also updated...
Though we haven't got it reviewed yet.(

-Sergey



> > +	if (!IS_ERR(dwsmmio->rstc))
> > +		reset_control_deassert(dwsmmio->rstc);
> > +
> >  	dws = &dwsmmio->dws;
> >  
> >  	/* Get basic io resource and map it */
> 
> Should we also undo the reset in error paths from probe()?  It's not the
> end of the world if we don't but...
> 
> > @@ -220,6 +226,9 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
> >  	clk_disable_unprepare(dwsmmio->pclk);
> >  	clk_disable_unprepare(dwsmmio->clk);
> >  
> > +	if (dwsmmio->rstc)
> > +		reset_control_assert(dwsmmio->rstc);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 384a3ab6dc2d..5c813e15ed89 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -20,6 +20,7 @@ 
 #include <linux/acpi.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 
 #include "spi-dw.h"
 
@@ -30,6 +31,7 @@  struct dw_spi_mmio {
 	struct clk     *clk;
 	struct clk     *pclk;
 	void           *priv;
+	struct reset_control	*rstc;
 };
 
 #define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
@@ -145,6 +147,10 @@  static int dw_spi_mmio_probe(struct platform_device *pdev)
 	if (!dwsmmio)
 		return -ENOMEM;
 
+	dwsmmio->rstc = devm_reset_control_get_exclusive(&pdev->dev, "spi");
+	if (!IS_ERR(dwsmmio->rstc))
+		reset_control_deassert(dwsmmio->rstc);
+
 	dws = &dwsmmio->dws;
 
 	/* Get basic io resource and map it */
@@ -220,6 +226,9 @@  static int dw_spi_mmio_remove(struct platform_device *pdev)
 	clk_disable_unprepare(dwsmmio->pclk);
 	clk_disable_unprepare(dwsmmio->clk);
 
+	if (dwsmmio->rstc)
+		reset_control_assert(dwsmmio->rstc);
+
 	return 0;
 }