[PATCH 0/3] spi: bitbang fixes and passive serial driver

[PATCH 0/3] spi: bitbang fixes and passive serial driver

[Michael Grzeschik]

This series includes a bugfix and adds LSB mode for the bitbang driver.
It also includes a new character device driver to passively clock a
firmware into an FPGA.

It is an resend after the cc to linux-arm-kernel list got fixed.

Michael Grzeschik (3):
  spi: bitbang: fix shift for getmosi
  spi: bitbang: add lsb first support
  spi: psdev: add passive serial driver

 drivers/spi/Kconfig            |   9 +
 drivers/spi/Makefile           |   1 +
 drivers/spi/spi-bitbang-txrx.h |  98 +++++---
 drivers/spi/spi-bitbang.c      |   3 +-
 drivers/spi/spipsdev.c         | 520 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 601 insertions(+), 30 deletions(-)
 create mode 100644 drivers/spi/spipsdev.c

-- 
1.9.0

[PATCH 1/3] spi: bitbang: fix shift for getmosi

[Michael Grzeschik]

The driver needs to shift the word bit after reading the mosi bit.
Otherwise the return word will have an Off-by-one bit value.

Cc: <stable@vger.kernel.org>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/spi/spi-bitbang-txrx.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index c616e41..b6e348d 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
 		spidelay(nsecs);
 
 		/* sample MSB (from slave) on leading edge */
-		word <<= 1;
 		if ((flags & SPI_MASTER_NO_RX) == 0)
 			word |= getmiso(spi);
 		setsck(spi, cpol);
+		word <<= 1;
 	}
 	return word;
 }
@@ -89,9 +89,9 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
 		spidelay(nsecs);
 
 		/* sample MSB (from slave) on trailing edge */
-		word <<= 1;
 		if ((flags & SPI_MASTER_NO_RX) == 0)
 			word |= getmiso(spi);
+		word <<= 1;
 	}
 	return word;
 }
-- 
1.9.0

[PATCH 1/3] spi: bitbang: fix shift for getmosi

[Mark Brown]

On Wed, Mar 12, 2014 at 04:53:35PM +0100, Michael Grzeschik wrote:
> The driver needs to shift the word bit after reading the mosi bit.
> Otherwise the return word will have an Off-by-one bit value.

This isn't exactly new code...  do we understand why nobody has noticed
this before?

> @@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
>  		spidelay(nsecs);
>  
>  		/* sample MSB (from slave) on leading edge */
> -		word <<= 1;
>  		if ((flags & SPI_MASTER_NO_RX) == 0)
>  			word |= getmiso(spi);
>  		setsck(spi, cpol);
> +		word <<= 1;
>  	}

Just looking at the context here it's not obvious to me that this is
helping - it means that the last bit we read is going to be shifted
which seems wrong, we ought to be reading into LSB.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140312/7bbc3eaf/attachment.sig>

[PATCH 1/3] spi: bitbang: fix shift for getmosi

[Gerhard Sittig]

On Wed, Mar 12, 2014 at 16:24 +0000, Mark Brown wrote:
> 
> On Wed, Mar 12, 2014 at 04:53:35PM +0100, Michael Grzeschik wrote:
> > The driver needs to shift the word bit after reading the mosi bit.
> > Otherwise the return word will have an Off-by-one bit value.
> 
> This isn't exactly new code...  do we understand why nobody has noticed
> this before?

I too suspect that the bug (if there is any) is elsewhere.

> > @@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
> >  		spidelay(nsecs);
> >  
> >  		/* sample MSB (from slave) on leading edge */
> > -		word <<= 1;
> >  		if ((flags & SPI_MASTER_NO_RX) == 0)
> >  			word |= getmiso(spi);
> >  		setsck(spi, cpol);
> > +		word <<= 1;
> >  	}
> 
> Just looking at the context here it's not obvious to me that this is
> helping - it means that the last bit we read is going to be shifted
> which seems wrong, we ought to be reading into LSB.

It might be more robust to 

  if (!(flags & SPI_MASTER_NO_RX) && getmiso(spi))
    word |= 1;
    
instead.  This decouples the construction of the received bits
buffer from whatever the getmiso() implementation might look
like.  That's just a thought after the recent GPIO discussion
about whether 1/0 is given or should not be assumed (and I still
suspect that "!!x" need not result in exactly 1/0 values).

And I agree with Mark that the "late shift" is most probably
wrong.  Just noticed the resend too late and responded in the
other thread.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[PATCH 2/3] spi: bitbang: add lsb first support

[Michael Grzeschik]

The bitbang spi driver currently only supports the MSB mode. This patch
adds the possibility to clock the data in LSB mode.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/spi/spi-bitbang-txrx.h | 98 +++++++++++++++++++++++++++++-------------
 drivers/spi/spi-bitbang.c      |  3 +-
 2 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index b6e348d..7f9c020 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -49,22 +49,42 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
 {
 	/* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */
 
-	/* clock starts at inactive polarity */
-	for (word <<= (32 - bits); likely(bits); bits--) {
-
-		/* setup MSB (to slave) on trailing edge */
-		if ((flags & SPI_MASTER_NO_TX) == 0)
-			setmosi(spi, word & (1 << 31));
-		spidelay(nsecs);	/* T(setup) */
-
-		setsck(spi, !cpol);
-		spidelay(nsecs);
-
-		/* sample MSB (from slave) on leading edge */
-		if ((flags & SPI_MASTER_NO_RX) == 0)
-			word |= getmiso(spi);
-		setsck(spi, cpol);
-		word <<= 1;
+	if (spi->mode & SPI_LSB_FIRST) {
+		/* clock starts at inactive polarity */
+		for (; likely(bits); bits--) {
+
+			/* setup MSB (to slave) on trailing edge */
+			if ((flags & SPI_MASTER_NO_TX) == 0)
+				setmosi(spi, word & 1);
+			spidelay(nsecs);	/* T(setup) */
+
+			setsck(spi, !cpol);
+			spidelay(nsecs);
+
+			/* sample LSB (from slave) on leading edge */
+			if ((flags & SPI_MASTER_NO_RX) == 0)
+				word |= getmiso(spi);
+			setsck(spi, cpol);
+			word >>= 1;
+		}
+	} else {
+		/* clock starts at inactive polarity */
+		for (word <<= (32 - bits); likely(bits); bits--) {
+
+			/* setup MSB (to slave) on trailing edge */
+			if ((flags & SPI_MASTER_NO_TX) == 0)
+				setmosi(spi, word & (1 << 31));
+			spidelay(nsecs);        /* T(setup) */
+
+			setsck(spi, !cpol);
+			spidelay(nsecs);
+
+			/* sample MSB (from slave) on leading edge */
+			if ((flags & SPI_MASTER_NO_RX) == 0)
+				word |= getmiso(spi);
+			setsck(spi, cpol);
+			word <<= 1;
+		}
 	}
 	return word;
 }
@@ -76,22 +96,42 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
 {
 	/* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */
 
-	/* clock starts@inactive polarity */
-	for (word <<= (32 - bits); likely(bits); bits--) {
+	if (spi->mode & SPI_LSB_FIRST) {
+		/* clock starts at inactive polarity */
+		for (; likely(bits); bits--) {
+
+			/* setup MSB (to slave) on leading edge */
+			setsck(spi, !cpol);
+			if ((flags & SPI_MASTER_NO_TX) == 0)
+				setmosi(spi, word & 1);
+			spidelay(nsecs); /* T(setup) */
+
+			setsck(spi, cpol);
+			spidelay(nsecs);
+
+			/* sample MSB (from slave) on trailing edge */
+			if ((flags & SPI_MASTER_NO_RX) == 0)
+				word |= getmiso(spi);
+			word >>= 1;
+		}
+	} else {
+		/* clock starts at inactive polarity */
+		for (word <<= (32 - bits); likely(bits); bits--) {
 
-		/* setup MSB (to slave) on leading edge */
-		setsck(spi, !cpol);
-		if ((flags & SPI_MASTER_NO_TX) == 0)
-			setmosi(spi, word & (1 << 31));
-		spidelay(nsecs); /* T(setup) */
+			/* setup MSB (to slave) on leading edge */
+			setsck(spi, !cpol);
+			if ((flags & SPI_MASTER_NO_TX) == 0)
+				setmosi(spi, word & (1 << 31));
+			spidelay(nsecs); /* T(setup) */
 
-		setsck(spi, cpol);
-		spidelay(nsecs);
+			setsck(spi, cpol);
+			spidelay(nsecs);
 
-		/* sample MSB (from slave) on trailing edge */
-		if ((flags & SPI_MASTER_NO_RX) == 0)
-			word |= getmiso(spi);
-		word <<= 1;
+			/* sample MSB (from slave) on trailing edge */
+			if ((flags & SPI_MASTER_NO_RX) == 0)
+				word |= getmiso(spi);
+			word <<= 1;
+		}
 	}
 	return word;
 }
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index bd222f6..3624f96 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -432,7 +432,8 @@ int spi_bitbang_start(struct spi_bitbang *bitbang)
 	spin_lock_init(&bitbang->lock);
 
 	if (!master->mode_bits)
-		master->mode_bits = SPI_CPOL | SPI_CPHA | bitbang->flags;
+		master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST
+			| bitbang->flags;
 
 	if (master->transfer || master->transfer_one_message)
 		return -EINVAL;
-- 
1.9.0

[PATCH 3/3] spi: psdev: add passive serial driver

[Michael Grzeschik]

This patch introduces the psdev driver. It is used to communicate to an
Altera passive serial connected FPGA by writing into an character
device.

It simply initates the protocol on open and clocks the data into the
FPGA by using the underlying SPI bus on write. The used SPI bus can also
be an bitbang SPI bus. On close it will check for the response of the
FPGA and tells if the write process was successful.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/spi/Kconfig    |   9 +
 drivers/spi/Makefile   |   1 +
 drivers/spi/spipsdev.c | 520 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 530 insertions(+)
 create mode 100644 drivers/spi/spipsdev.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 581ee2a..ab6d438 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -208,6 +208,15 @@ config SPI_GPIO
 	  GPIO operations, you should be able to leverage that for better
 	  speed with a custom version of this driver; see the source code.
 
+config SPI_PASSIVE_SERIAL
+	tristate "GPIO-based bitbanging SPI Altera Passive Serial"
+	depends on GPIOLIB
+	select SPI_BITBANG
+	help
+	  This driver uses GPIOs to communicate by alteras passive serial
+	  protocol. It creates an character device, that can be used to
+	  simply cat RBF Firmware images into the FPGA.
+
 config SPI_IMX
 	tristate "Freescale i.MX SPI controllers"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 95af48d..296b1dd 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -8,6 +8,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG
 # config declarations into driver model code
 obj-$(CONFIG_SPI_MASTER)		+= spi.o
 obj-$(CONFIG_SPI_SPIDEV)		+= spidev.o
+obj-$(CONFIG_SPI_PASSIVE_SERIAL)	+= spipsdev.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
diff --git a/drivers/spi/spipsdev.c b/drivers/spi/spipsdev.c
new file mode 100644
index 0000000..bda89b2
--- /dev/null
+++ b/drivers/spi/spipsdev.c
@@ -0,0 +1,520 @@
+/*
+ * Passive Serial Programming Driver
+ *
+ * Copyright (C) 2014 Michael Grzeschik <mgr@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/ioctl.h>
+#include <linux/fs.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_device.h>
+
+#include <linux/spi/spi.h>
+
+#include <linux/uaccess.h>
+
+#define N_PS_MINORS		32
+
+unsigned int major;
+static DECLARE_BITMAP(minors, N_PS_MINORS);
+
+struct psdev_data {
+	dev_t			devt;
+	spinlock_t		spi_lock;
+	struct spi_device	*spi;
+	struct list_head	device_entry;
+	bool			open;
+
+	struct mutex		buf_lock;
+	u8			*buffer;
+
+	/* gpios for ps protocol */
+	unsigned		nconfig_gpio;
+	unsigned		confd_gpio;
+	unsigned		nstat_gpio;
+};
+
+static LIST_HEAD(device_list);
+static DEFINE_MUTEX(minor_lock);
+
+static unsigned bufsiz = 4096;
+module_param(bufsiz, uint, S_IRUGO);
+MODULE_PARM_DESC(bufsiz, "data bytes in biggest supported SPI message");
+
+/*
+ * We can't use the standard synchronous wrappers for file I/O; we
+ * need to protect against async removal of the underlying spi_device.
+ */
+static void psdev_complete(void *arg)
+{
+	if (arg)
+		complete(arg);
+}
+
+static ssize_t psdev_sync(struct psdev_data *psdev, struct spi_message *message)
+{
+	DECLARE_COMPLETION_ONSTACK(done);
+	int status;
+
+	message->complete = psdev_complete;
+	message->context = &done;
+
+	spin_lock_irq(&psdev->spi_lock);
+	if (psdev->spi == NULL)
+		status = -ESHUTDOWN;
+	else
+		status = spi_async(psdev->spi, message);
+	spin_unlock_irq(&psdev->spi_lock);
+
+	if (status == 0) {
+		unsigned long time_left = msecs_to_jiffies(100);
+
+		time_left = wait_for_completion_timeout(&done, time_left);
+		if (!time_left) {
+			message->context = NULL;
+			return -ETIMEDOUT;
+		}
+		status = message->status;
+		if (status == 0)
+			status = message->actual_length;
+	}
+	return status;
+}
+
+static inline ssize_t psdev_sync_write(struct psdev_data *psdev, size_t len)
+{
+	struct spi_transfer t = {
+		.tx_buf		= psdev->buffer,
+		.len		= len,
+	};
+	struct spi_message m;
+
+	spi_message_init(&m);
+	spi_message_add_tail(&t, &m);
+	return psdev_sync(psdev, &m);
+}
+
+/* Write-only message with current device setup */
+static ssize_t psdev_write(struct file *filp, const char __user *buf,
+		size_t count, loff_t *f_pos)
+{
+	struct psdev_data *psdev;
+	ssize_t	status = 0;
+	unsigned long missing;
+
+	if (count > bufsiz)
+		return -EMSGSIZE;
+
+	psdev = filp->private_data;
+
+	mutex_lock(&psdev->buf_lock);
+	missing = copy_from_user(psdev->buffer, buf, count);
+	if (missing == 0)
+		status = psdev_sync_write(psdev, count);
+	else
+		status = -EFAULT;
+	mutex_unlock(&psdev->buf_lock);
+
+	return status;
+}
+
+static int psdev_start(struct psdev_data *psdev)
+{
+	struct device *dev = &psdev->spi->dev;
+	unsigned long timeout;
+	int ret = -ENXIO;
+
+	dev_dbg(dev, "Initiating programming\n");
+
+	/* initiate an FPGA programming */
+	gpio_set_value(psdev->nconfig_gpio, 0);
+
+	/*
+	 * after about 2 ?s the FPGA must acknowledge with
+	 * STATUS and CONFIG DONE lines at low level
+	 */
+	timeout = jiffies + usecs_to_jiffies(2);
+	do {
+		if ((gpio_get_value(psdev->nstat_gpio) == 0) &&
+		    (gpio_get_value(psdev->confd_gpio) == 0)) {
+			ret = 0;
+			break;
+		}
+		ndelay(100);
+
+	} while (time_before(jiffies, timeout));
+
+	if (ret != 0) {
+		dev_err(dev, "FPGA does not acknowledge the programming initiation\n");
+		if (gpio_get_value(psdev->nstat_gpio))
+			dev_err(dev, "STATUS is still high!\n");
+		if (gpio_get_value(psdev->confd_gpio))
+			dev_err(dev, "CONFIG DONE is still high!\n");
+
+		return ret;
+	}
+
+	/* arm the FPGA to await its new firmware */
+	gpio_set_value(psdev->nconfig_gpio, 1);
+
+	ret = -ENXIO;
+	timeout = jiffies + usecs_to_jiffies(1600);
+	do {
+		if (gpio_get_value(psdev->nstat_gpio) == 0) {
+			ret = 0;
+			break;
+		}
+		udelay(10);
+
+	} while (time_before(jiffies, timeout));
+
+	if (ret != 0) {
+		dev_err(dev, "FPGA does not acknowledge the programming start\n");
+		return ret;
+	}
+
+	dev_dbg(dev, "Initiating passed\n");
+
+	/* at the end, wait at least 2 ?s prior beginning writing data */
+	udelay(2);
+
+	return 0;
+}
+
+static int psdev_open(struct inode *inode, struct file *filp)
+{
+	struct psdev_data *psdev, *tmp;
+	struct device *dev;
+	int status = -ENXIO;
+
+	list_for_each_entry_safe(psdev, tmp, &device_list, device_entry) {
+		if (psdev->devt == inode->i_rdev) {
+			status = 0;
+			break;
+		}
+	}
+
+	if (psdev->open) {
+		status = -EBUSY;
+		goto out;
+	}
+
+	dev = &psdev->spi->dev;
+
+	if (status == 0) {
+		if (!psdev->buffer) {
+			psdev->buffer = kmalloc(bufsiz, GFP_KERNEL);
+			if (!psdev->buffer) {
+				dev_dbg(dev, "open/ENOMEM\n");
+				status = -ENOMEM;
+			}
+		}
+		if (status == 0) {
+			filp->private_data = psdev;
+			nonseekable_open(inode, filp);
+		}
+	} else
+		pr_debug("psdev: nothing for minor %d\n", iminor(inode));
+
+	psdev_start(psdev);
+
+	psdev->open = 1;
+
+ out:
+
+	return status;
+}
+
+static int psdev_release(struct inode *inode, struct file *filp)
+{
+	struct psdev_data *psdev;
+	unsigned long timeout;
+	struct device *dev;
+	int status = -ENXIO;
+
+	psdev = filp->private_data;
+	filp->private_data = NULL;
+
+	dev = &psdev->spi->dev;
+
+	/*
+	 * when programming was successfully,
+	 * both status lines should be at high level
+	 */
+	timeout = jiffies + usecs_to_jiffies(1000);
+	do {
+		if ((gpio_get_value(psdev->nstat_gpio)) &&
+		    (gpio_get_value(psdev->confd_gpio))) {
+			status = 0;
+			break;
+		}
+		udelay(10);
+
+	} while (time_before(jiffies, timeout));
+
+	if (status != 0) {
+		dev_err(dev, "FPGA does not acknowledge the programming initiation\n");
+		if (gpio_get_value(psdev->nstat_gpio) == 0)
+			dev_err(dev, "STATUS is still low!\n");
+		if (gpio_get_value(psdev->confd_gpio) == 0)
+			dev_err(dev, "CONFIG DONE is still low!\n");
+	}
+
+	kfree(psdev->buffer);
+	psdev->buffer = NULL;
+
+	psdev->open = 0;
+
+	return status;
+}
+
+static const struct file_operations psdev_fops = {
+	.owner =	THIS_MODULE,
+	.write =	psdev_write,
+	.open =		psdev_open,
+	.release =	psdev_release,
+	.llseek =	no_llseek,
+};
+
+static struct class *psdev_class;
+
+/*-------------------------------------------------------------------------*/
+#ifdef CONFIG_OF
+
+static const struct of_device_id psdev_dt_ids[] = {
+	{ .compatible = "altr,passive-serial" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, psdev_dt_ids);
+
+static int psdev_probe_dt(struct psdev_data *psdev)
+{
+	int ret = 0;
+	struct device *dev = &psdev->spi->dev;
+	struct device_node *np = dev->of_node;
+	const struct of_device_id *of_id =
+			of_match_device(psdev_dt_ids, &psdev->spi->dev);
+
+	if (!of_id)
+		return -EINVAL;
+
+	ret = of_get_named_gpio(np, "gpio-nstatus", 0);
+	if (ret < 0) {
+		dev_err(dev, "gpio-nstatus property not found\n");
+		goto error;
+	}
+	psdev->nstat_gpio = ret;
+
+	ret = of_get_named_gpio(np, "gpio-confdone", 0);
+	if (ret < 0) {
+		dev_err(dev, "gpio-confdone property not found\n");
+		goto error;
+	}
+	psdev->confd_gpio = ret;
+
+	ret = of_get_named_gpio(np, "gpio-nconfig", 0);
+	if (ret < 0) {
+		dev_err(dev, "gpio-nconfig property not found\n");
+		goto error;
+	}
+	psdev->nconfig_gpio = ret;
+
+	ret = devm_gpio_request_one(dev, psdev->nconfig_gpio,
+			GPIOF_OUT_INIT_HIGH, "nconfig");
+	if (ret) {
+		dev_err(dev, "unable to get nconfig gpio\n");
+		goto error;
+	}
+
+	ret = devm_gpio_request_one(dev, psdev->nstat_gpio,
+			GPIOF_DIR_IN,  "nstat");
+	if (ret) {
+		dev_err(dev, "unable to get nstat gpio\n");
+		goto error;
+	}
+
+	ret = devm_gpio_request_one(dev, psdev->confd_gpio,
+			GPIOF_DIR_IN, "confd");
+	if (ret) {
+		dev_err(dev, "unable to get confd gpio\n");
+		goto error;
+	}
+
+ error:
+	return ret;
+}
+#else
+static inline int spi_gpio_probe_dt(struct platform_device *pdev)
+{
+	return -EINVAL;
+}
+#endif
+
+static int psdev_probe(struct spi_device *spi)
+{
+	struct psdev_data *psdev;
+	unsigned long minor;
+	int status = 0;
+
+	/* Allocate driver data */
+	psdev = devm_kzalloc(&spi->dev, sizeof(*psdev), GFP_KERNEL);
+	if (!psdev)
+		return -ENOMEM;
+
+	/* Initialize the driver data */
+	psdev->spi = spi;
+	spin_lock_init(&psdev->spi_lock);
+	mutex_init(&psdev->buf_lock);
+
+	INIT_LIST_HEAD(&psdev->device_entry);
+	/* If we can allocate a minor number, hook up this device.
+	 * Reusing minors is fine so long as udev or mdev is working.
+	 */
+	mutex_lock(&minor_lock);
+	minor = find_first_zero_bit(minors, N_PS_MINORS);
+	if (minor < N_PS_MINORS) {
+		struct device *dev;
+
+		psdev->devt = MKDEV(major, minor);
+		dev = device_create(psdev_class, &spi->dev, psdev->devt,
+				    psdev, "psdev%lu", minor);
+		status = PTR_ERR_OR_ZERO(dev);
+		if (status) {
+			mutex_unlock(&minor_lock);
+			return -ENODEV;
+		}
+	} else {
+		mutex_unlock(&minor_lock);
+		dev_dbg(&spi->dev, "no minor number available!\n");
+		return -ENODEV;
+	}
+	set_bit(minor, minors);
+	mutex_unlock(&minor_lock);
+
+	list_add(&psdev->device_entry, &device_list);
+
+	spi->mode |= SPI_LSB_FIRST | SPI_NO_CS;
+	spi_setup(spi);
+
+	status = psdev_probe_dt(psdev);
+	if (status)
+		goto error;
+
+	spi_set_drvdata(spi, psdev);
+
+	return status;
+
+ error:
+	mutex_lock(&minor_lock);
+	clear_bit(MINOR(psdev->devt), minors);
+	mutex_unlock(&minor_lock);
+
+	list_del(&psdev->device_entry);
+	device_destroy(psdev_class, psdev->devt);
+
+	return status;
+}
+
+static int psdev_remove(struct spi_device *spi)
+{
+	struct psdev_data *psdev = spi_get_drvdata(spi);
+
+	/* make sure ops on existing fds can abort cleanly */
+	spin_lock_irq(&psdev->spi_lock);
+	psdev->spi = NULL;
+	spin_unlock_irq(&psdev->spi_lock);
+
+	/* prevent new opens */
+	list_del(&psdev->device_entry);
+	device_destroy(psdev_class, psdev->devt);
+	mutex_lock(&minor_lock);
+	clear_bit(MINOR(psdev->devt), minors);
+	mutex_unlock(&minor_lock);
+
+	return 0;
+}
+
+static struct spi_driver psdev_spi_driver = {
+	.driver = {
+		.name =		"psdev",
+		.owner =	THIS_MODULE,
+		.of_match_table = of_match_ptr(psdev_dt_ids),
+	},
+	.probe =	psdev_probe,
+	.remove =	psdev_remove,
+
+	/* NOTE:  suspend/resume methods are not necessary here.
+	 * We don't do anything except pass the requests to/from
+	 * the underlying controller.  The refrigerator handles
+	 * most issues; the controller driver handles the rest.
+	 */
+};
+
+/*-------------------------------------------------------------------------*/
+
+static int __init psdev_init(void)
+{
+	int status;
+
+	/* Claim our 256 reserved device numbers.  Then register a class
+	 * that will key udev/mdev to add/remove /dev nodes.  Last, register
+	 * the driver which manages those device numbers.
+	 */
+	BUILD_BUG_ON(N_PS_MINORS > 256);
+	status = register_chrdev(0, "psdev", &psdev_fops);
+	if (status < 0)
+		return status;
+
+	major = status;
+
+	psdev_class = class_create(THIS_MODULE, "psdev");
+	if (IS_ERR(psdev_class)) {
+		unregister_chrdev(major, psdev_spi_driver.driver.name);
+		return PTR_ERR(psdev_class);
+	}
+
+	status = spi_register_driver(&psdev_spi_driver);
+	if (status < 0) {
+		class_destroy(psdev_class);
+		unregister_chrdev(major, psdev_spi_driver.driver.name);
+	}
+	return status;
+}
+module_init(psdev_init);
+
+static void __exit psdev_exit(void)
+{
+	spi_unregister_driver(&psdev_spi_driver);
+	class_destroy(psdev_class);
+	unregister_chrdev(major, psdev_spi_driver.driver.name);
+}
+module_exit(psdev_exit);
+
+MODULE_AUTHOR("Michael Grzeschik, <mgrzeschik@gmx.net>");
+MODULE_DESCRIPTION("User mode SPI Passive Serial interface");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:psdev");
-- 
1.9.0

[PATCH 3/3] spi: psdev: add passive serial driver

[Mark Brown]

On Wed, Mar 12, 2014 at 04:53:37PM +0100, Michael Grzeschik wrote:

> This patch introduces the psdev driver. It is used to communicate to an
> Altera passive serial connected FPGA by writing into an character
> device.
> 
> It simply initates the protocol on open and clocks the data into the
> FPGA by using the underlying SPI bus on write. The used SPI bus can also
> be an bitbang SPI bus. On close it will check for the response of the
> FPGA and tells if the write process was successful.

Having read the above I'm still not entirely sure what this is supposed
to do.  It sounds like it might be something to do with programming
FPGAs but that doesn't seem like something for the SPI subsystem, it
sounds like something for a FPGA handling subsystem.  People keep
talking about that, there's a bunch of other stuff to integrate with
there like loading DTs for the downloeaded bitstream.  I'm not sure what
the status is there.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140312/47cf4251/attachment.sig>

[PATCH 3/3] spi: psdev: add passive serial driver

[Michael Grzeschik]

On Wed, Mar 12, 2014 at 04:34:27PM +0000, Mark Brown wrote:
> On Wed, Mar 12, 2014 at 04:53:37PM +0100, Michael Grzeschik wrote:
> 
> > This patch introduces the psdev driver. It is used to communicate to an
> > Altera passive serial connected FPGA by writing into an character
> > device.
> > 
> > It simply initates the protocol on open and clocks the data into the
> > FPGA by using the underlying SPI bus on write. The used SPI bus can also
> > be an bitbang SPI bus. On close it will check for the response of the
> > FPGA and tells if the write process was successful.
> 
> Having read the above I'm still not entirely sure what this is supposed
> to do.  It sounds like it might be something to do with programming
> FPGAs but that doesn't seem like something for the SPI subsystem, it
> sounds like something for a FPGA handling subsystem.  People keep
> talking about that, there's a bunch of other stuff to integrate with
> there like loading DTs for the downloeaded bitstream.  I'm not sure what
> the status is there.

Ok, so first of all it needs better documentation, also for the
bindings. I will add some.

Second, the driver has "per se" nothing to do with SPI, beside it needs
a clock and data line to transfer an image to the FPGA. (One way,
therefor: passive). It needs to use some SPI controller to do that. Even
the SPI Bitbang driver. Because of this, it depends on the SPI
framework.

Beside the SCLK and MOSI, the protocol needs to toggle and check some
GPIOs for protocol start and end. For that I chose the posix open and
close, so that in userland it can be used as simple as $(cat
firmware.rbf > /dev/psdev0). This protocol also needs no SS line.

It is actually not more than that. I took the spidev character device
driver as reference implementation. Any Idea where to put this driver
instead.

Regards,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/3] spi: psdev: add passive serial driver

[Mark Brown]

On Fri, Mar 14, 2014 at 11:19:07AM +0100, Michael Grzeschik wrote:

> Second, the driver has "per se" nothing to do with SPI, beside it needs
> a clock and data line to transfer an image to the FPGA. (One way,
> therefor: passive). It needs to use some SPI controller to do that. Even
> the SPI Bitbang driver. Because of this, it depends on the SPI
> framework.

Lots of drivers use SPI but they sit in the relevant frameworks rather
than in the SPI framework.

> It is actually not more than that. I took the spidev character device
> driver as reference implementation. Any Idea where to put this driver
> instead.

Like I say somewhere where it sits with other FPGA programming stuff (if
that's what it does) would be my first thought.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140314/57c1f7d4/attachment-0001.sig>

[PATCH 3/3] spi: psdev: add passive serial driver

[Linus Walleij]

On Fri, Mar 14, 2014 at 11:22 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Mar 14, 2014 at 11:19:07AM +0100, Michael Grzeschik wrote:
>
>> It is actually not more than that. I took the spidev character device
>> driver as reference implementation. Any Idea where to put this driver
>> instead.
>
> Like I say somewhere where it sits with other FPGA programming stuff (if
> that's what it does) would be my first thought.

I'd suggest you try to use drivers/firmware.

That subsystem is a bit x86-centrix at the moment but looking at the
stuff that is in there (like supporting BIOS updates through sysfs and
other low-level firmware communication) this stuff is close enough.

Yours,
Linus Walleij

[PATCH 3/3] spi: psdev: add passive serial driver

[Michael Grzeschik]

On Fri, Mar 14, 2014 at 02:59:47PM +0100, Linus Walleij wrote:
> On Fri, Mar 14, 2014 at 11:22 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Mar 14, 2014 at 11:19:07AM +0100, Michael Grzeschik wrote:
> >
> >> It is actually not more than that. I took the spidev character device
> >> driver as reference implementation. Any Idea where to put this driver
> >> instead.
> >
> > Like I say somewhere where it sits with other FPGA programming stuff (if
> > that's what it does) would be my first thought.
> 
> I'd suggest you try to use drivers/firmware.
> 
> That subsystem is a bit x86-centrix at the moment but looking at the
> stuff that is in there (like supporting BIOS updates through sysfs and
> other low-level firmware communication) this stuff is close enough.

Indeed, the directory could need some refurbishing. It is also not even
sourced by drivers/Kconfig but instead by the architectures that depend
on it. Beside that, it seems to be the place to be.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/3] spi: psdev: add passive serial driver

[Alexander Shiyan]

?????, 12 ????? 2014, 16:53 +01:00 ?? Michael Grzeschik <m.grzeschik@pengutronix.de>:
> This patch introduces the psdev driver. It is used to communicate to an
> Altera passive serial connected FPGA by writing into an character
> device.
> 
> It simply initates the protocol on open and clocks the data into the
> FPGA by using the underlying SPI bus on write. The used SPI bus can also
> be an bitbang SPI bus. On close it will check for the response of the
> FPGA and tells if the write process was successful.

As a suggestion, I want to advise you to use GPIO functions based on
descriptors (xx_gpiod_xx()) for the new drivers.
This will simplify code a bit, allows you to use the driver for non-DT boards
without creating any new global headers for platform_data and eliminate
the possible confusion with active levels for these GPIOs in the future.

---

[PATCH 3/3] spi: psdev: add passive serial driver

[Mark Rutland]

On Wed, Mar 12, 2014 at 03:53:37PM +0000, Michael Grzeschik wrote:
> This patch introduces the psdev driver. It is used to communicate to an
> Altera passive serial connected FPGA by writing into an character
> device.
> 
> It simply initates the protocol on open and clocks the data into the
> FPGA by using the underlying SPI bus on write. The used SPI bus can also
> be an bitbang SPI bus. On close it will check for the response of the
> FPGA and tells if the write process was successful.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/spi/Kconfig    |   9 +
>  drivers/spi/Makefile   |   1 +
>  drivers/spi/spipsdev.c | 520 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 530 insertions(+)
>  create mode 100644 drivers/spi/spipsdev.c

[...]

> +static const struct of_device_id psdev_dt_ids[] = {
> +       { .compatible = "altr,passive-serial" },
> +       {},
> +};

There should be a binding document for this as part of the series.

Thanks,
Mark.

[PATCH 3/3] spi: psdev: add passive serial driver

[Lothar Waßmann]

Hi,

Michael Grzeschik wrote:
> This patch introduces the psdev driver. It is used to communicate to an
> Altera passive serial connected FPGA by writing into an character
> device.
> 
> It simply initates the protocol on open and clocks the data into the
> FPGA by using the underlying SPI bus on write. The used SPI bus can also
> be an bitbang SPI bus. On close it will check for the response of the
> FPGA and tells if the write process was successful.
> 
[...]
+/*-------------------------------------------------------------------------*/
> +#ifdef CONFIG_OF
> +
> +static const struct of_device_id psdev_dt_ids[] = {
> +	{ .compatible = "altr,passive-serial" },
> +	{},
          ^
<nit>
Since '{}' always must be the last entry in the initializer list,
the comma doesn't make sense here.
Omitting it would cause entries added after it (e.g. due to a bad merge
conflict resolution) to generate a compile error.
</nit>


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________