[PATCH 0/6] spi/ath79: various fixes

[PATCH 0/6] spi/ath79: various fixes

[Gabor Juhos]

This patch set contain various fixes for the spi-ath79 driver.

Gabor Juhos (6):
  spi/ath79: add delay between SCK changes
  spi/ath79: add missing HIGH->LOW SCK transition
  spi/ath79: remove superfluous chip select code
  spi/ath79: use gpio_request_one
  spi/ath79: avoid multiple initialization of the SPI controller
  spi/ath79: add shutdown handler

 drivers/spi/spi-ath79.c |  121 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 88 insertions(+), 33 deletions(-)

-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 1/6] spi/ath79: add delay between SCK changes

[Gabor Juhos]

The driver uses the "as fast as it can" approach
to drive the SCK signal. However this does not
work with certain low speed SPI chips (e.g. the
PCF2123 RTC chip).

The patch adds per-bit slowdowns in order to be
able to use the driver with such chips as well.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index 9a5d779..e025282 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -24,17 +24,24 @@
 #include <linux/spi/spi_bitbang.h>
 #include <linux/bitops.h>
 #include <linux/gpio.h>
+#include <linux/clk.h>
+#include <linux/err.h>
 
 #include <asm/mach-ath79/ar71xx_regs.h>
 #include <asm/mach-ath79/ath79_spi_platform.h>
 
 #define DRV_NAME	"ath79-spi"
 
+#define ATH79_SPI_RRW_DELAY_FACTOR	12000
+#define MHZ				(1000 * 1000)
+
 struct ath79_spi {
 	struct spi_bitbang	bitbang;
 	u32			ioc_base;
 	u32			reg_ctrl;
 	void __iomem		*base;
+	struct clk		*clk;
+	unsigned		rrw_delay;
 };
 
 static inline u32 ath79_spi_rr(struct ath79_spi *sp, unsigned reg)
@@ -52,6 +59,12 @@ static inline struct ath79_spi *ath79_spidev_to_sp(struct spi_device *spi)
 	return spi_master_get_devdata(spi->master);
 }
 
+static inline void ath79_spi_delay(struct ath79_spi *sp, unsigned nsecs)
+{
+	if (nsecs > sp->rrw_delay)
+		ndelay(nsecs - sp->rrw_delay);
+}
+
 static void ath79_spi_chipselect(struct spi_device *spi, int is_active)
 {
 	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
@@ -184,7 +197,9 @@ static u32 ath79_spi_txrx_mode0(struct spi_device *spi, unsigned nsecs,
 
 		/* setup MSB (to slave) on trailing edge */
 		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out);
+		ath79_spi_delay(sp, nsecs);
 		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out | AR71XX_SPI_IOC_CLK);
+		ath79_spi_delay(sp, nsecs);
 
 		word <<= 1;
 	}
@@ -198,6 +213,7 @@ static int ath79_spi_probe(struct platform_device *pdev)
 	struct ath79_spi *sp;
 	struct ath79_spi_platform_data *pdata;
 	struct resource	*r;
+	unsigned long rate;
 	int ret;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(*sp));
@@ -236,12 +252,36 @@ static int ath79_spi_probe(struct platform_device *pdev)
 		goto err_put_master;
 	}
 
+	sp->clk = clk_get(&pdev->dev, "ahb");
+	if (IS_ERR(sp->clk)) {
+		ret = PTR_ERR(sp->clk);
+		goto err_unmap;
+	}
+
+	ret = clk_enable(sp->clk);
+	if (ret)
+		goto err_clk_put;
+
+	rate = DIV_ROUND_UP(clk_get_rate(sp->clk), MHZ);
+	if (!rate) {
+		ret = -EINVAL;
+		goto err_clk_disable;
+	}
+
+	sp->rrw_delay = ATH79_SPI_RRW_DELAY_FACTOR / rate;
+	dev_dbg(&pdev->dev, "register read/write delay is %u nsecs\n",
+		sp->rrw_delay);
+
 	ret = spi_bitbang_start(&sp->bitbang);
 	if (ret)
-		goto err_unmap;
+		goto err_clk_disable;
 
 	return 0;
 
+err_clk_disable:
+	clk_disable(sp->clk);
+err_clk_put:
+	clk_put(sp->clk);
 err_unmap:
 	iounmap(sp->base);
 err_put_master:
@@ -256,6 +296,8 @@ static int ath79_spi_remove(struct platform_device *pdev)
 	struct ath79_spi *sp = platform_get_drvdata(pdev);
 
 	spi_bitbang_stop(&sp->bitbang);
+	clk_disable(sp->clk);
+	clk_put(sp->clk);
 	iounmap(sp->base);
 	platform_set_drvdata(pdev, NULL);
 	spi_master_put(sp->bitbang.master);
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 2/6] spi/ath79: add missing HIGH->LOW SCK transition

[Gabor Juhos]

The 'ath79_spi_txrx_mode0' function does not
set the SCK signal to LOW at the end of a word
transfer. This causes communications errors with
certain devices (e.g. the PCF2123 RTC chip).

The patch ensures that the SCK signal will be LOW.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index e025282..d4b8e12 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -200,6 +200,8 @@ static u32 ath79_spi_txrx_mode0(struct spi_device *spi, unsigned nsecs,
 		ath79_spi_delay(sp, nsecs);
 		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out | AR71XX_SPI_IOC_CLK);
 		ath79_spi_delay(sp, nsecs);
+		if (bits == 1)
+			ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out);
 
 		word <<= 1;
 	}
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 3/6] spi/ath79: remove superfluous chip select code

[Gabor Juhos]

The spi_bitbang driver calls the chipselect function
of the driver from spi_bitbang_setup in order to
deselect the given SPI chip, so we don't have to
initialize the CS line here.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index d4b8e12..a725e62 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -128,12 +128,6 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
 			gpio_free(cdata->gpio);
 			return status;
 		}
-	} else {
-		if (spi->mode & SPI_CS_HIGH)
-			sp->ioc_base |= AR71XX_SPI_IOC_CS0;
-		else
-			sp->ioc_base &= ~AR71XX_SPI_IOC_CS0;
-		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, sp->ioc_base);
 	}
 
 	return 0;
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 4/6] spi/ath79: use gpio_request_one

[Gabor Juhos]

Use gpio_request_one() instead of multiple gpiolib calls.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index a725e62..19d539e 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -100,6 +100,7 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
 {
 	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
 	struct ath79_spi_controller_data *cdata;
+	int status;
 
 	cdata = spi->controller_data;
 	if (spi->chip_select && !cdata)
@@ -115,22 +116,21 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
 	/* TODO: setup speed? */
 	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, 0x43);
 
+	status = 0;
 	if (spi->chip_select) {
-		int status = 0;
+		unsigned long flags;
 
-		status = gpio_request(cdata->gpio, dev_name(&spi->dev));
-		if (status)
-			return status;
+		flags = GPIOF_DIR_OUT;
+		if (spi->mode & SPI_CS_HIGH)
+			flags |= GPIOF_INIT_HIGH;
+		else
+			flags |= GPIOF_INIT_LOW;
 
-		status = gpio_direction_output(cdata->gpio,
-					       spi->mode & SPI_CS_HIGH);
-		if (status) {
-			gpio_free(cdata->gpio);
-			return status;
-		}
+		status = gpio_request_one(cdata->gpio, flags,
+					  dev_name(&spi->dev));
 	}
 
-	return 0;
+	return status;
 }
 
 static void ath79_spi_cleanup_cs(struct spi_device *spi)
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 5/6] spi/ath79: avoid multiple initialization of the SPI controller

[Gabor Juhos]

Currently we are initializing the SPI controller in
the chip select line function, and that function is
called once for each SPI device on the bus. If a
board has multiple SPI devices, the controller will
be initialized multiple times.

Introduce ath79_spi_{en,dis}able helper functions,
and call those from probe/response in order to avoid
the mutliple initialization of the controller.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |   41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index 19d539e..842acd8 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -96,16 +96,8 @@ static void ath79_spi_chipselect(struct spi_device *spi, int is_active)
 
 }
 
-static int ath79_spi_setup_cs(struct spi_device *spi)
+static void ath79_spi_enable(struct ath79_spi *sp)
 {
-	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
-	struct ath79_spi_controller_data *cdata;
-	int status;
-
-	cdata = spi->controller_data;
-	if (spi->chip_select && !cdata)
-		return -EINVAL;
-
 	/* enable GPIO mode */
 	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, AR71XX_SPI_FS_GPIO);
 
@@ -115,6 +107,24 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
 
 	/* TODO: setup speed? */
 	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, 0x43);
+}
+
+static void ath79_spi_disable(struct ath79_spi *sp)
+{
+	/* restore CTRL register */
+	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, sp->reg_ctrl);
+	/* disable GPIO mode */
+	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, 0);
+}
+
+static int ath79_spi_setup_cs(struct spi_device *spi)
+{
+	struct ath79_spi_controller_data *cdata;
+	int status;
+
+	cdata = spi->controller_data;
+	if (spi->chip_select && !cdata)
+		return -EINVAL;
 
 	status = 0;
 	if (spi->chip_select) {
@@ -135,17 +145,10 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
 
 static void ath79_spi_cleanup_cs(struct spi_device *spi)
 {
-	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
-
 	if (spi->chip_select) {
 		struct ath79_spi_controller_data *cdata = spi->controller_data;
 		gpio_free(cdata->gpio);
 	}
-
-	/* restore CTRL register */
-	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, sp->reg_ctrl);
-	/* disable GPIO mode */
-	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, 0);
 }
 
 static int ath79_spi_setup(struct spi_device *spi)
@@ -268,12 +271,15 @@ static int ath79_spi_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "register read/write delay is %u nsecs\n",
 		sp->rrw_delay);
 
+	ath79_spi_enable(sp);
 	ret = spi_bitbang_start(&sp->bitbang);
 	if (ret)
-		goto err_clk_disable;
+		goto err_disable;
 
 	return 0;
 
+err_disable:
+	ath79_spi_disable(sp);
 err_clk_disable:
 	clk_disable(sp->clk);
 err_clk_put:
@@ -292,6 +298,7 @@ static int ath79_spi_remove(struct platform_device *pdev)
 	struct ath79_spi *sp = platform_get_drvdata(pdev);
 
 	spi_bitbang_stop(&sp->bitbang);
+	ath79_spi_disable(sp);
 	clk_disable(sp->clk);
 	clk_put(sp->clk);
 	iounmap(sp->base);
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

[PATCH 6/6] spi/ath79: add shutdown handler

[Gabor Juhos]

The SPI controller of the AR7xxx/AR9xxx SoCs
have a special mode which allows the SoC to
directly read data from SPI flash chips. In
this mode, the content of the SPI flash chip
can be accessed via a memory mapped region.

During early init time, the kernel expects
that the flash chip is accessible through
that memory region because it reads board
specific values (e.g. MAC address, WiFi
calibration data) from the flash on various
boards.

This is working if the kernel is loaded
directly by the bootloader because that
leaves the SPI controller in the special
mode. However it is not working in a kexec'd
kernel because the SPI driver does not restore
the special mode during shutdown.

The patch adds a shutdown handler to fix this
issue.

Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
 drivers/spi/spi-ath79.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index 842acd8..73e491e 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -293,7 +293,7 @@ err_put_master:
 	return ret;
 }
 
-static int ath79_spi_remove(struct platform_device *pdev)
+static void __ath79_spi_remove(struct platform_device *pdev)
 {
 	struct ath79_spi *sp = platform_get_drvdata(pdev);
 
@@ -304,13 +304,23 @@ static int ath79_spi_remove(struct platform_device *pdev)
 	iounmap(sp->base);
 	platform_set_drvdata(pdev, NULL);
 	spi_master_put(sp->bitbang.master);
+}
 
+static int ath79_spi_remove(struct platform_device *pdev)
+{
+	__ath79_spi_remove(pdev);
 	return 0;
 }
 
+static void ath79_spi_shutdown(struct platform_device *pdev)
+{
+	__ath79_spi_remove(pdev);
+}
+
 static struct platform_driver ath79_spi_driver = {
 	.probe		= ath79_spi_probe,
 	.remove		= ath79_spi_remove,
+	.shutdown	= ath79_spi_shutdown,
 	.driver		= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
-- 
1.7.10


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122712

Re: [PATCH 1/6] spi/ath79: add delay between SCK changes

[Grant Likely]

On Thu, 27 Dec 2012 10:42:24 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> The driver uses the "as fast as it can" approach
> to drive the SCK signal. However this does not
> work with certain low speed SPI chips (e.g. the
> PCF2123 RTC chip).
> 
> The patch adds per-bit slowdowns in order to be
> able to use the driver with such chips as well.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>

I've applied this, but please take a second look to make sure you're not
doing something unintended. The ndelay call will spin until it
completes. If the current context is interrupted or scheduled out then
it will still spin when it gets back. You may be wasting more time than
is necessary. It would be better to check the wall time over a loop
iteration. And if the delay required is large, then it should sleep.

g.


------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 2/6] spi/ath79: add missing HIGH->LOW SCK transition

[Grant Likely]

On Thu, 27 Dec 2012 10:42:25 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> The 'ath79_spi_txrx_mode0' function does not
> set the SCK signal to LOW at the end of a word
> transfer. This causes communications errors with
> certain devices (e.g. the PCF2123 RTC chip).
> 
> The patch ensures that the SCK signal will be LOW.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-ath79.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
> index e025282..d4b8e12 100644
> --- a/drivers/spi/spi-ath79.c
> +++ b/drivers/spi/spi-ath79.c
> @@ -200,6 +200,8 @@ static u32 ath79_spi_txrx_mode0(struct spi_device *spi, unsigned nsecs,
>  		ath79_spi_delay(sp, nsecs);
>  		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out | AR71XX_SPI_IOC_CLK);
>  		ath79_spi_delay(sp, nsecs);
> +		if (bits == 1)
> +			ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, out);
>  
>  		word <<= 1;
>  	}
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 3/6] spi/ath79: remove superfluous chip select code

[Grant Likely]

On Thu, 27 Dec 2012 10:42:26 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> The spi_bitbang driver calls the chipselect function
> of the driver from spi_bitbang_setup in order to
> deselect the given SPI chip, so we don't have to
> initialize the CS line here.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>

Applied, thanks.

g.
> ---
>  drivers/spi/spi-ath79.c |    6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
> index d4b8e12..a725e62 100644
> --- a/drivers/spi/spi-ath79.c
> +++ b/drivers/spi/spi-ath79.c
> @@ -128,12 +128,6 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
>  			gpio_free(cdata->gpio);
>  			return status;
>  		}
> -	} else {
> -		if (spi->mode & SPI_CS_HIGH)
> -			sp->ioc_base |= AR71XX_SPI_IOC_CS0;
> -		else
> -			sp->ioc_base &= ~AR71XX_SPI_IOC_CS0;
> -		ath79_spi_wr(sp, AR71XX_SPI_REG_IOC, sp->ioc_base);
>  	}
>  
>  	return 0;
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 4/6] spi/ath79: use gpio_request_one

[Grant Likely]

On Thu, 27 Dec 2012 10:42:27 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> Use gpio_request_one() instead of multiple gpiolib calls.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-ath79.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
> index a725e62..19d539e 100644
> --- a/drivers/spi/spi-ath79.c
> +++ b/drivers/spi/spi-ath79.c
> @@ -100,6 +100,7 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
>  {
>  	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
>  	struct ath79_spi_controller_data *cdata;
> +	int status;
>  
>  	cdata = spi->controller_data;
>  	if (spi->chip_select && !cdata)
> @@ -115,22 +116,21 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
>  	/* TODO: setup speed? */
>  	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, 0x43);
>  
> +	status = 0;
>  	if (spi->chip_select) {
> -		int status = 0;
> +		unsigned long flags;
>  
> -		status = gpio_request(cdata->gpio, dev_name(&spi->dev));
> -		if (status)
> -			return status;
> +		flags = GPIOF_DIR_OUT;
> +		if (spi->mode & SPI_CS_HIGH)
> +			flags |= GPIOF_INIT_HIGH;
> +		else
> +			flags |= GPIOF_INIT_LOW;
>  
> -		status = gpio_direction_output(cdata->gpio,
> -					       spi->mode & SPI_CS_HIGH);
> -		if (status) {
> -			gpio_free(cdata->gpio);
> -			return status;
> -		}
> +		status = gpio_request_one(cdata->gpio, flags,
> +					  dev_name(&spi->dev));
>  	}
>  
> -	return 0;
> +	return status;
>  }
>  
>  static void ath79_spi_cleanup_cs(struct spi_device *spi)
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 5/6] spi/ath79: avoid multiple initialization of the SPI controller

[Grant Likely]

On Thu, 27 Dec 2012 10:42:28 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> Currently we are initializing the SPI controller in
> the chip select line function, and that function is
> called once for each SPI device on the bus. If a
> board has multiple SPI devices, the controller will
> be initialized multiple times.
> 
> Introduce ath79_spi_{en,dis}able helper functions,
> and call those from probe/response in order to avoid
> the mutliple initialization of the controller.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/spi-ath79.c |   41 ++++++++++++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
> index 19d539e..842acd8 100644
> --- a/drivers/spi/spi-ath79.c
> +++ b/drivers/spi/spi-ath79.c
> @@ -96,16 +96,8 @@ static void ath79_spi_chipselect(struct spi_device *spi, int is_active)
>  
>  }
>  
> -static int ath79_spi_setup_cs(struct spi_device *spi)
> +static void ath79_spi_enable(struct ath79_spi *sp)
>  {
> -	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
> -	struct ath79_spi_controller_data *cdata;
> -	int status;
> -
> -	cdata = spi->controller_data;
> -	if (spi->chip_select && !cdata)
> -		return -EINVAL;
> -
>  	/* enable GPIO mode */
>  	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, AR71XX_SPI_FS_GPIO);
>  
> @@ -115,6 +107,24 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
>  
>  	/* TODO: setup speed? */
>  	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, 0x43);
> +}
> +
> +static void ath79_spi_disable(struct ath79_spi *sp)
> +{
> +	/* restore CTRL register */
> +	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, sp->reg_ctrl);
> +	/* disable GPIO mode */
> +	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, 0);
> +}
> +
> +static int ath79_spi_setup_cs(struct spi_device *spi)
> +{
> +	struct ath79_spi_controller_data *cdata;
> +	int status;
> +
> +	cdata = spi->controller_data;
> +	if (spi->chip_select && !cdata)
> +		return -EINVAL;
>  
>  	status = 0;
>  	if (spi->chip_select) {
> @@ -135,17 +145,10 @@ static int ath79_spi_setup_cs(struct spi_device *spi)
>  
>  static void ath79_spi_cleanup_cs(struct spi_device *spi)
>  {
> -	struct ath79_spi *sp = ath79_spidev_to_sp(spi);
> -
>  	if (spi->chip_select) {
>  		struct ath79_spi_controller_data *cdata = spi->controller_data;
>  		gpio_free(cdata->gpio);
>  	}
> -
> -	/* restore CTRL register */
> -	ath79_spi_wr(sp, AR71XX_SPI_REG_CTRL, sp->reg_ctrl);
> -	/* disable GPIO mode */
> -	ath79_spi_wr(sp, AR71XX_SPI_REG_FS, 0);
>  }
>  
>  static int ath79_spi_setup(struct spi_device *spi)
> @@ -268,12 +271,15 @@ static int ath79_spi_probe(struct platform_device *pdev)
>  	dev_dbg(&pdev->dev, "register read/write delay is %u nsecs\n",
>  		sp->rrw_delay);
>  
> +	ath79_spi_enable(sp);
>  	ret = spi_bitbang_start(&sp->bitbang);
>  	if (ret)
> -		goto err_clk_disable;
> +		goto err_disable;
>  
>  	return 0;
>  
> +err_disable:
> +	ath79_spi_disable(sp);
>  err_clk_disable:
>  	clk_disable(sp->clk);
>  err_clk_put:
> @@ -292,6 +298,7 @@ static int ath79_spi_remove(struct platform_device *pdev)
>  	struct ath79_spi *sp = platform_get_drvdata(pdev);
>  
>  	spi_bitbang_stop(&sp->bitbang);
> +	ath79_spi_disable(sp);
>  	clk_disable(sp->clk);
>  	clk_put(sp->clk);
>  	iounmap(sp->base);
> -- 
> 1.7.10
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 6/6] spi/ath79: add shutdown handler

[Grant Likely]

On Thu, 27 Dec 2012 10:42:29 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
> The SPI controller of the AR7xxx/AR9xxx SoCs
> have a special mode which allows the SoC to
> directly read data from SPI flash chips. In
> this mode, the content of the SPI flash chip
> can be accessed via a memory mapped region.
> 
> During early init time, the kernel expects
> that the flash chip is accessible through
> that memory region because it reads board
> specific values (e.g. MAC address, WiFi
> calibration data) from the flash on various
> boards.
> 
> This is working if the kernel is loaded
> directly by the bootloader because that
> leaves the SPI controller in the special
> mode. However it is not working in a kexec'd
> kernel because the SPI driver does not restore
> the special mode during shutdown.
> 
> The patch adds a shutdown handler to fix this
> issue.
> 
> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> ---
>  drivers/spi/spi-ath79.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
> index 842acd8..73e491e 100644
> --- a/drivers/spi/spi-ath79.c
> +++ b/drivers/spi/spi-ath79.c
> @@ -293,7 +293,7 @@ err_put_master:
>  	return ret;
>  }
>  
> -static int ath79_spi_remove(struct platform_device *pdev)
> +static void __ath79_spi_remove(struct platform_device *pdev)
>  {
>  	struct ath79_spi *sp = platform_get_drvdata(pdev);
>  
> @@ -304,13 +304,23 @@ static int ath79_spi_remove(struct platform_device *pdev)
>  	iounmap(sp->base);
>  	platform_set_drvdata(pdev, NULL);
>  	spi_master_put(sp->bitbang.master);
> +}
>  
> +static int ath79_spi_remove(struct platform_device *pdev)
> +{
> +	__ath79_spi_remove(pdev);
>  	return 0;
>  }
>  
> +static void ath79_spi_shutdown(struct platform_device *pdev)
> +{
> +	__ath79_spi_remove(pdev);
> +}

Just call ath79_spi_remote(pdev) directly from ath79_spi_shutdown(). No
need for the extra hook and __ version of the original function.

g.


------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 1/6] spi/ath79: add delay between SCK changes

[Gabor Juhos]

Hi Grant,

> On Thu, 27 Dec 2012 10:42:24 +0100, Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org> wrote:
>> The driver uses the "as fast as it can" approach
>> to drive the SCK signal. However this does not
>> work with certain low speed SPI chips (e.g. the
>> PCF2123 RTC chip).
>>
>> The patch adds per-bit slowdowns in order to be
>> able to use the driver with such chips as well.
>>
>> Signed-off-by: Gabor Juhos <juhosg-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> 
> I've applied this, but please take a second look to make sure you're not
> doing something unintended. The ndelay call will spin until it
> completes. If the current context is interrupted or scheduled out then
> it will still spin when it gets back. You may be wasting more time than
> is necessary. It would be better to check the wall time over a loop
> iteration. And if the delay required is large, then it should sleep.

Thank for the review!

Unfortunately, at the moment I don't have access to the board which uses that
PCF2123 chip but I will try to improve the code.

-Gabor


------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH 6/6] spi/ath79: add shutdown handler

[Gabor Juhos]

2013.02.05. 14:02 keltezéssel, Grant Likely írta:

>> -static int ath79_spi_remove(struct platform_device *pdev)
>> +static void __ath79_spi_remove(struct platform_device *pdev)
>>  {
>>  	struct ath79_spi *sp = platform_get_drvdata(pdev);
>>  
>> @@ -304,13 +304,23 @@ static int ath79_spi_remove(struct platform_device *pdev)
>>  	iounmap(sp->base);
>>  	platform_set_drvdata(pdev, NULL);
>>  	spi_master_put(sp->bitbang.master);
>> +}
>>  
>> +static int ath79_spi_remove(struct platform_device *pdev)
>> +{
>> +	__ath79_spi_remove(pdev);
>>  	return 0;
>>  }
>>  
>> +static void ath79_spi_shutdown(struct platform_device *pdev)
>> +{
>> +	__ath79_spi_remove(pdev);
>> +}
> 
> Just call ath79_spi_remote(pdev) directly from ath79_spi_shutdown(). No
> need for the extra hook and __ version of the original function.

Ok, I will send a modified patch.

Thanks,
Gabor

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb