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

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

[Dinh Nguyen]

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(+)

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;
 }
 
-- 
2.17.1

[PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property

[Dinh Nguyen]

Add optional reset property.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 3ed08ee9feba..e01c557a9876 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -37,5 +37,7 @@ Example:
 		num-cs = <2>;
 		cs-gpios = <&gpio0 13 0>,
 			   <&gpio0 14 0>;
+		resets = <&rst SPIM0_RST>;
+		reset-names = "spi";
 	};
 
-- 
2.17.1

Re: [PATCH 2/2] dt-bindings: snps,dw-apb-ssi: add optional reset property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On Thu, May 21, 2020 at 12:03:59PM -0500, Dinh Nguyen wrote:
> Add optional reset property.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index 3ed08ee9feba..e01c557a9876 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -37,5 +37,7 @@ Example:
>  		num-cs = <2>;
>  		cs-gpios = <&gpio0 13 0>,
>  			   <&gpio0 14 0>;
> +		resets = <&rst SPIM0_RST>;
> +		reset-names = "spi";

This only adds the example, it doesn't add anything to the actual
binding documentation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Serge Semin]

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
> >