diff mbox

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

Message ID 1356601349-23617-2-git-send-email-juhosg@openwrt.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Gabor Juhos Dec. 27, 2012, 9:42 a.m. UTC
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@openwrt.org>
---
 drivers/spi/spi-ath79.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

Comments

Grant Likely Feb. 5, 2013, 12:57 p.m. UTC | #1
On Thu, 27 Dec 2012 10:42:24 +0100, Gabor Juhos <juhosg@openwrt.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@openwrt.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
Gabor Juhos Feb. 5, 2013, 7:52 p.m. UTC | #2
Hi Grant,

> On Thu, 27 Dec 2012 10:42:24 +0100, Gabor Juhos <juhosg@openwrt.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@openwrt.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
diff mbox

Patch

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