mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/5] sandbox: add gpio support with libftdi1
@ 2017-10-10 12:26 Antony Pavlov
  2017-10-10 12:26 ` [PATCH 1/5] sandbox: avoid symbol conflict for {open,read,close}dir Antony Pavlov
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

This patch series makes it possible to use FT2232H ACBUS[7:0]
pins as gpio pins from sandbox barebox.

The main goal of adding gpio functionality to sandbox barebox
is using it for connecting real i2c and spi devices to sandbox barebox.

Sample dts-file for at24 i2c eeprom and at25 spi flash is included.

Alas bit-bang i2c via libftdi1 works too slow and ds1307 i2c rtc driver
does not work correctly.

Changes since RFC v1 patch series (http://lists.infradead.org/pipermail/barebox/2017-February/029106.html):

  * rebase on top barebox v2017.10.0;
  * libftdi cmdline options are added;
  * device tree support is added.

Antony Pavlov (5):
  sandbox: avoid symbol conflict for {open,read,close}dir
  sandbox: add gpio support with libftdi1
  sandbox: parse libftdi options
  sandbox_defconfig: enable gpio, spi, i2c and led stuff
  sandbox: dts: add i2c and spi libftdi1 bit-bang example

 arch/sandbox/Kconfig                           |   1 +
 arch/sandbox/Makefile                          |  10 +-
 arch/sandbox/configs/sandbox_defconfig         |  24 ++-
 arch/sandbox/dts/sandbox.dts                   |  47 +++++
 arch/sandbox/mach-sandbox/include/mach/linux.h |  11 ++
 arch/sandbox/os/Makefile                       |   3 +
 arch/sandbox/os/common.c                       |  12 +-
 arch/sandbox/os/ftdi.c                         | 250 +++++++++++++++++++++++++
 drivers/gpio/Kconfig                           |   4 +
 drivers/gpio/Makefile                          |   1 +
 drivers/gpio/gpio-libftdi1.c                   | 125 +++++++++++++
 11 files changed, 483 insertions(+), 5 deletions(-)
 create mode 100644 arch/sandbox/os/ftdi.c
 create mode 100644 drivers/gpio/gpio-libftdi1.c

-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] sandbox: avoid symbol conflict for {open,read,close}dir
  2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
@ 2017-10-10 12:26 ` Antony Pavlov
  2017-10-10 12:26 ` [PATCH 2/5] sandbox: add gpio support with libftdi1 Antony Pavlov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

This fixes libusb's /dev/bus/usb directory scan.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/sandbox/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/Makefile b/arch/sandbox/Makefile
index 8155a790eb..9d545c3b71 100644
--- a/arch/sandbox/Makefile
+++ b/arch/sandbox/Makefile
@@ -21,7 +21,9 @@ CFLAGS += -Dmalloc=barebox_malloc -Dcalloc=barebox_calloc \
 		-Dfputs=barebox_fputs -Dsetenv=barebox_setenv \
 		-Dgetenv=barebox_getenv -Dprintf=barebox_printf \
 		-Dglob=barebox_glob -Dglobfree=barebox_globfree \
-		-Dioctl=barebox_ioctl -Dfstat=barebox_fstat
+		-Dioctl=barebox_ioctl -Dfstat=barebox_fstat \
+		-Dopendir=barebox_opendir -Dreaddir=barebox_readdir \
+		-Dclosedir=barebox_closedir
 
 machdirs := $(patsubst %,arch/sandbox/mach-%/,$(machine-y))
 
-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] sandbox: add gpio support with libftdi1
  2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
  2017-10-10 12:26 ` [PATCH 1/5] sandbox: avoid symbol conflict for {open,read,close}dir Antony Pavlov
@ 2017-10-10 12:26 ` Antony Pavlov
  2017-10-16  7:56   ` Sascha Hauer
  2017-10-10 12:26 ` [PATCH 3/5] sandbox: parse libftdi options Antony Pavlov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/sandbox/Kconfig                           |   1 +
 arch/sandbox/Makefile                          |   6 +-
 arch/sandbox/mach-sandbox/include/mach/linux.h |  11 ++
 arch/sandbox/os/Makefile                       |   3 +
 arch/sandbox/os/ftdi.c                         | 173 +++++++++++++++++++++++++
 drivers/gpio/Kconfig                           |   4 +
 drivers/gpio/Makefile                          |   1 +
 drivers/gpio/gpio-libftdi1.c                   | 125 ++++++++++++++++++
 8 files changed, 323 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index 9c72673bde..3f1cefb837 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -1,6 +1,7 @@
 config SANDBOX
 	bool
 	select OFTREE
+	select GPIOLIB
 	default y
 
 config ARCH_TEXT_BASE
diff --git a/arch/sandbox/Makefile b/arch/sandbox/Makefile
index 9d545c3b71..85c70b5e80 100644
--- a/arch/sandbox/Makefile
+++ b/arch/sandbox/Makefile
@@ -41,9 +41,13 @@ ifeq ($(CONFIG_DRIVER_VIDEO_SDL),y)
 SDL_LIBS := $(shell pkg-config sdl --libs)
 endif
 
+ifeq ($(CONFIG_GPIO_LIBFTDI1),y)
+FTDI1_LIBS := $(shell pkg-config libftdi1 --libs)
+endif
+
 cmd_barebox__ = $(CC) -o $@ -Wl,-T,$(barebox-lds) \
 	-Wl,--start-group $(barebox-common) -Wl,--end-group \
-	-lrt -lpthread $(SDL_LIBS)
+	-lrt -lpthread $(SDL_LIBS) $(FTDI1_LIBS)
 
 common-y += $(BOARD) arch/sandbox/os/
 
diff --git a/arch/sandbox/mach-sandbox/include/mach/linux.h b/arch/sandbox/mach-sandbox/include/mach/linux.h
index 1327a56cab..9abb52e93c 100644
--- a/arch/sandbox/mach-sandbox/include/mach/linux.h
+++ b/arch/sandbox/mach-sandbox/include/mach/linux.h
@@ -38,4 +38,15 @@ void sdl_get_bitfield_rgba(struct fb_bitfield *r, struct fb_bitfield *g,
 			    struct fb_bitfield *b, struct fb_bitfield *a);
 void sdl_setpixel(int x, int y, uint8_t r, uint8_t g, uint8_t b, uint8_t a);
 
+struct ft2232_bitbang;
+struct ft2232_bitbang *barebox_libftdi1_open(void);
+void barebox_libftdi1_gpio_direction(struct ft2232_bitbang *ftbb,
+						unsigned off, unsigned dir);
+int barebox_libftdi1_gpio_get_value(struct ft2232_bitbang *ftbb,
+						unsigned off);
+void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
+						unsigned off, unsigned val);
+int barebox_libftdi1_update(struct ft2232_bitbang *ftbb);
+void barebox_libftdi1_close(void);
+
 #endif /* __ASM_ARCH_LINUX_H */
diff --git a/arch/sandbox/os/Makefile b/arch/sandbox/os/Makefile
index 537f848e06..75baa34a83 100644
--- a/arch/sandbox/os/Makefile
+++ b/arch/sandbox/os/Makefile
@@ -17,3 +17,6 @@ obj-y = common.o tap.o
 
 CFLAGS_sdl.o = $(shell pkg-config sdl --cflags)
 obj-$(CONFIG_DRIVER_VIDEO_SDL) += sdl.o
+
+CFLAGS_ftdi.o = $(shell pkg-config libftdi1 --cflags)
+obj-$(CONFIG_GPIO_LIBFTDI1) += ftdi.o
diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
new file mode 100644
index 0000000000..34e9165787
--- /dev/null
+++ b/arch/sandbox/os/ftdi.c
@@ -0,0 +1,173 @@
+/*
+ * sandbox barebox libftdi1 support
+ *
+ * Copyright (C) 2016, 2017 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * 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 <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <ftdi.h>
+#include <errno.h>
+#include <mach/linux.h>
+
+#define FTDI_VID		0x0403	/* Vendor Id */
+#define FTDI_8U2232C_PID	0x6010	/* Dual channel device */
+
+#define GPIOF_DIR_OUT	(0 << 0)
+#define GPIOF_DIR_IN	(1 << 0)
+
+#define BIT(nr)			(1UL << (nr))
+
+struct ft2232_bitbang {
+	struct ftdi_context *ftdi;
+	uint8_t odata;
+	uint8_t dir;
+};
+
+static struct ft2232_bitbang ftbb;
+
+static inline int ftdi_flush(struct ftdi_context *ftdi)
+{
+	uint8_t buf[1];
+	int ret;
+
+	buf[0] = SEND_IMMEDIATE;
+
+	ret = ftdi_write_data(ftdi, buf, 1);
+
+	return ret;
+}
+
+static int ftdi_get_high_byte_data(struct ftdi_context *ftdi, uint8_t *data)
+{
+	uint8_t obuf;
+	int ret;
+
+	obuf = GET_BITS_HIGH;
+	ret = ftdi_write_data(ftdi, &obuf, 1);
+
+	ret = ftdi_read_data(ftdi, data, 1);
+
+	return ret;
+}
+
+static int ftdi_set_high_byte_data_dir(struct ft2232_bitbang *ftbb)
+{
+	uint8_t buf[3];
+	int ret;
+
+	buf[0] = SET_BITS_HIGH;
+	buf[1] = ftbb->odata;
+	buf[2] = ftbb->dir;
+
+	ret = ftdi_write_data(ftbb->ftdi, buf, 3);
+	ftdi_flush(ftbb->ftdi);
+
+	return ret;
+}
+
+int barebox_libftdi1_update(struct ft2232_bitbang *ftbb)
+{
+	return ftdi_set_high_byte_data_dir(ftbb);
+}
+
+void barebox_libftdi1_gpio_direction(struct ft2232_bitbang *ftbb,
+					unsigned off, unsigned dir)
+{
+	switch (dir) {
+	case GPIOF_DIR_IN:
+		ftbb->dir &= ~BIT(off);
+		break;
+	case GPIOF_DIR_OUT:
+		ftbb->dir |= BIT(off);
+		break;
+	default:
+		printf("%s:%d: invalid dir argument\n", __FILE__, __LINE__);
+	}
+}
+
+int barebox_libftdi1_gpio_get_value(struct ft2232_bitbang *ftbb, unsigned off)
+{
+	uint8_t data;
+
+	ftdi_get_high_byte_data(ftbb->ftdi, &data);
+
+	return !!(data & BIT(off));
+}
+
+void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
+					unsigned off, unsigned val)
+{
+	if (val)
+		ftbb->odata |= BIT(off);
+	else
+		ftbb->odata &= ~BIT(off);
+}
+
+int barebox_libftdi1_init(void)
+{
+	struct ftdi_context *ftdi;
+	int ret;
+	/* default FT2232 values */
+	uint16_t vendor_id = FTDI_VID;
+	uint16_t device_id = FTDI_8U2232C_PID;
+
+	ftdi = ftdi_new();
+	if (!ftdi) {
+		fprintf(stderr, "ftdi_new failed\n");
+		goto error;
+	}
+
+	ret = ftdi_usb_open(ftdi, vendor_id, device_id);
+	if (ret < 0 && ret != -5) {
+		fprintf(stderr, "unable to open ftdi device: %d (%s)\n",
+			ret, ftdi_get_error_string(ftdi));
+		goto error;
+	}
+
+	ftdi_set_interface(ftdi, INTERFACE_A);
+	ftdi_set_bitmode(ftdi, 0x00, BITMODE_MPSSE);
+
+	ftbb.ftdi = ftdi;
+
+	/* reset pins to default neutral state */
+	ftbb.dir = 0;
+	ftbb.odata = 0;
+	ftdi_set_high_byte_data_dir(&ftbb);
+
+	return 0;
+
+error:
+	return -1;
+}
+
+struct ft2232_bitbang *barebox_libftdi1_open(void)
+{
+	if (barebox_libftdi1_init() < 0) {
+		printf("Could not initialize ftdi\n");
+		return NULL;
+	}
+
+	return &ftbb;
+}
+
+void barebox_libftdi1_close(void)
+{
+	struct ftdi_context *ftdi = ftbb.ftdi;
+
+	ftdi_set_interface(ftdi, INTERFACE_ANY);
+	ftdi_usb_close(ftdi);
+	ftdi_free(ftdi);
+}
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 434c5688b4..ed93e868ae 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -132,6 +132,10 @@ config GPIO_SX150X
 	  Say Y here to build support for the Semtec Sx150x I2C GPIO
 	  expander chip.
 
+config GPIO_LIBFTDI1
+	bool "libftdi1 driver"
+	depends on SANDBOX
+
 endmenu
 
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index f37dd08f1a..f5ed876d5e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_DIGIC)	+= gpio-digic.o
 obj-$(CONFIG_GPIO_GENERIC)	+= gpio-generic.o
 obj-$(CONFIG_GPIO_IMX)		+= gpio-imx.o
+obj-$(CONFIG_GPIO_LIBFTDI1)	+= gpio-libftdi1.o
 obj-$(CONFIG_GPIO_MXS)		+= gpio-mxs.o
 obj-$(CONFIG_GPIO_JZ4740)	+= gpio-jz4740.o
 obj-$(CONFIG_GPIO_MALTA_FPGA_I2C) += gpio-malta-fpga-i2c.o
diff --git a/drivers/gpio/gpio-libftdi1.c b/drivers/gpio/gpio-libftdi1.c
new file mode 100644
index 0000000000..21fc8cf034
--- /dev/null
+++ b/drivers/gpio/gpio-libftdi1.c
@@ -0,0 +1,125 @@
+/*
+ * libftdi1 sandbox barebox GPIO driver
+ *
+ * Copyright (C) 2016, 2017 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * 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 <common.h>
+#include <errno.h>
+#include <gpio.h>
+#include <init.h>
+#include <malloc.h>
+#include <mach/linux.h>
+
+struct libftdi1_gpio_chip {
+	struct gpio_chip chip;
+	struct ft2232_bitbang *ftbb;
+};
+
+static int libftdi1_gpio_direction_input(struct gpio_chip *chip, unsigned off)
+{
+	struct libftdi1_gpio_chip *gpio =
+		container_of(chip, struct libftdi1_gpio_chip, chip);
+
+	barebox_libftdi1_gpio_direction(gpio->ftbb, off, GPIOF_DIR_IN);
+	barebox_libftdi1_update(gpio->ftbb);
+
+	return 0;
+}
+
+static int libftdi1_gpio_direction_output(
+	struct gpio_chip *chip, unsigned off, int value)
+{
+	struct libftdi1_gpio_chip *gpio =
+		container_of(chip, struct libftdi1_gpio_chip, chip);
+
+	barebox_libftdi1_gpio_set_value(gpio->ftbb, off, value);
+	barebox_libftdi1_gpio_direction(gpio->ftbb, off, GPIOF_DIR_OUT);
+	barebox_libftdi1_update(gpio->ftbb);
+
+	return 0;
+}
+
+static int libftdi1_gpio_get_value(struct gpio_chip *chip, unsigned off)
+{
+	struct libftdi1_gpio_chip *gpio =
+		container_of(chip, struct libftdi1_gpio_chip, chip);
+
+	return barebox_libftdi1_gpio_get_value(gpio->ftbb, off);
+}
+
+static void libftdi1_gpio_set_value(
+	struct gpio_chip *chip, unsigned off, int value)
+{
+	struct libftdi1_gpio_chip *gpio =
+		container_of(chip, struct libftdi1_gpio_chip, chip);
+
+	barebox_libftdi1_gpio_set_value(gpio->ftbb, off, value);
+	barebox_libftdi1_update(gpio->ftbb);
+}
+
+static struct gpio_ops libftdi1_gpio_ops = {
+	.direction_input = libftdi1_gpio_direction_input,
+	.direction_output = libftdi1_gpio_direction_output,
+	.get = libftdi1_gpio_get_value,
+	.set = libftdi1_gpio_set_value,
+};
+
+static int libftdi1_gpio_probe(struct device_d *dev)
+{
+	struct libftdi1_gpio_chip *gpio;
+	struct ft2232_bitbang *ftbb;
+	int ret;
+
+	ftbb = barebox_libftdi1_open();
+	if (!ftbb)
+		return -EIO;
+
+	gpio = xzalloc(sizeof(*gpio));
+
+	gpio->ftbb = ftbb;
+
+	gpio->chip.dev = dev;
+	gpio->chip.ops = &libftdi1_gpio_ops;
+	gpio->chip.base = 0;
+	gpio->chip.ngpio = 8;
+
+	ret = gpiochip_add(&gpio->chip);
+
+	dev_dbg(dev, "%d: probed gpio%d with base %d\n",
+			ret, dev->id, gpio->chip.base);
+
+	return 0;
+}
+
+static __maybe_unused const struct of_device_id libftdi1_gpio_dt_ids[] = {
+	{
+		.compatible = "barebox,libftdi1-gpio",
+	}, {
+		/* sentinel */
+	},
+};
+
+static void libftdi1_gpio_remove(struct device_d *dev)
+{
+	barebox_libftdi1_close();
+}
+
+static struct driver_d libftdi1_gpio_driver = {
+	.name = "libftdi1-gpio",
+	.probe = libftdi1_gpio_probe,
+	.remove = libftdi1_gpio_remove,
+	.of_compatible = DRV_OF_COMPAT(libftdi1_gpio_dt_ids),
+};
+device_platform_driver(libftdi1_gpio_driver);
-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/5] sandbox: parse libftdi options
  2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
  2017-10-10 12:26 ` [PATCH 1/5] sandbox: avoid symbol conflict for {open,read,close}dir Antony Pavlov
  2017-10-10 12:26 ` [PATCH 2/5] sandbox: add gpio support with libftdi1 Antony Pavlov
@ 2017-10-10 12:26 ` Antony Pavlov
  2017-10-16  8:21   ` Sascha Hauer
  2017-10-10 12:26 ` [PATCH 4/5] sandbox_defconfig: enable gpio, spi, i2c and led stuff Antony Pavlov
  2017-10-10 12:26 ` [PATCH 5/5] sandbox: dts: add i2c and spi libftdi1 bit-bang example Antony Pavlov
  4 siblings, 1 reply; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/sandbox/Makefile    |  2 +-
 arch/sandbox/os/common.c | 12 ++++++--
 arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/Makefile b/arch/sandbox/Makefile
index 85c70b5e80..95ef7c84fb 100644
--- a/arch/sandbox/Makefile
+++ b/arch/sandbox/Makefile
@@ -23,7 +23,7 @@ CFLAGS += -Dmalloc=barebox_malloc -Dcalloc=barebox_calloc \
 		-Dglob=barebox_glob -Dglobfree=barebox_globfree \
 		-Dioctl=barebox_ioctl -Dfstat=barebox_fstat \
 		-Dopendir=barebox_opendir -Dreaddir=barebox_readdir \
-		-Dclosedir=barebox_closedir
+		-Dclosedir=barebox_closedir -Dstrdup=barebox_strdup
 
 machdirs := $(patsubst %,arch/sandbox/mach-%/,$(machine-y))
 
diff --git a/arch/sandbox/os/common.c b/arch/sandbox/os/common.c
index 665e8194ef..161ea5c849 100644
--- a/arch/sandbox/os/common.c
+++ b/arch/sandbox/os/common.c
@@ -51,6 +51,8 @@ int sdl_yres;
 static struct termios term_orig, term_vi;
 static char erase_char;	/* the users erase character */
 
+const char *libftdi_options;
+
 static void rawmode(void)
 {
 	tcgetattr(0, &term_orig);
@@ -322,10 +324,11 @@ static struct option long_options[] = {
 	{"stdinout", 1, 0, 'B'},
 	{"xres",     1, 0, 'x'},
 	{"yres",     1, 0, 'y'},
+	{"libftdi",  1, 0, 'f'},
 	{0, 0, 0, 0},
 };
 
-static const char optstring[] = "hm:i:e:d:O:I:B:x:y:";
+static const char optstring[] = "hm:i:e:d:O:I:B:x:y:f:";
 
 int main(int argc, char *argv[])
 {
@@ -367,6 +370,9 @@ int main(int argc, char *argv[])
 		case 'y':
 			sdl_yres = strtoul(optarg, NULL, 0);
 			break;
+		case 'f':
+			libftdi_options = strdup(optarg);
+			break;
 		default:
 			break;
 		}
@@ -495,7 +501,9 @@ static void print_usage(const char *prgname)
 "                       stdin and stdout. <filein> and <fileout> can be regular\n"
 "                       files or FIFOs.\n"
 "  -x, --xres=<res>     SDL width.\n"
-"  -y, --yres=<res>     SDL height.\n",
+"  -y, --yres=<res>     SDL height.\n"
+"  -f, --libftdi=<opts> libftdi options, e.g.\n"
+"                       --libftdi=vendor_id=0x1234,device_id=0x5678,serial=AB0055\n",
 	prgname
 	);
 }
diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
index 34e9165787..e3e46ed12d 100644
--- a/arch/sandbox/os/ftdi.c
+++ b/arch/sandbox/os/ftdi.c
@@ -20,6 +20,7 @@
 #include <unistd.h>
 #include <ftdi.h>
 #include <errno.h>
+#include <string.h>
 #include <mach/linux.h>
 
 #define FTDI_VID		0x0403	/* Vendor Id */
@@ -38,6 +39,8 @@ struct ft2232_bitbang {
 
 static struct ft2232_bitbang ftbb;
 
+extern const char *libftdi_options;
+
 static inline int ftdi_flush(struct ftdi_context *ftdi)
 {
 	uint8_t buf[1];
@@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
 		ftbb->odata &= ~BIT(off);
 }
 
+/* This is a somewhat hacked function similar in some ways to strtok().
+ * It will look for needle with a subsequent '=' in haystack, return a copy of
+ * needle and remove everything from the first occurrence of needle to the next
+ * delimiter from haystack.
+ */
+static char *extract_param(const char *const *haystack, const char *needle,
+			const char *delim)
+{
+	char *param_pos, *opt_pos, *rest;
+	char *opt = NULL;
+	int optlen;
+	int needlelen;
+
+	needlelen = strlen(needle);
+	if (!needlelen) {
+		printf("%s: empty needle!\n", __func__);
+		return NULL;
+	}
+	if (*haystack == NULL)
+		return NULL;
+	param_pos = strstr(*haystack, needle);
+	do {
+		if (!param_pos)
+			return NULL;
+		/* Needle followed by '='? */
+		if (param_pos[needlelen] == '=') {
+
+			/* Beginning of the string? */
+			if (param_pos == *haystack)
+				break;
+			/* After a delimiter? */
+			if (strchr(delim, *(param_pos - 1)))
+				break;
+		}
+		/* Continue searching. */
+		param_pos++;
+		param_pos = strstr(param_pos, needle);
+	} while (1);
+
+	if (param_pos) {
+		/* Get the string after needle and '='. */
+		opt_pos = param_pos + needlelen + 1;
+		optlen = strcspn(opt_pos, delim);
+		/* Return an empty string if the parameter was empty. */
+		opt = malloc(optlen + 1);
+		if (!opt) {
+			printf("Out of memory!\n");
+			exit(1);
+		}
+		strncpy(opt, opt_pos, optlen);
+		opt[optlen] = '\0';
+		rest = opt_pos + optlen;
+		/* Skip all delimiters after the current parameter. */
+		rest += strspn(rest, delim);
+		memmove(param_pos, rest, strlen(rest) + 1);
+		/* We could shrink haystack, but the effort is not worth it. */
+	}
+
+	return opt;
+}
+
 int barebox_libftdi1_init(void)
 {
 	struct ftdi_context *ftdi;
@@ -123,6 +187,9 @@ int barebox_libftdi1_init(void)
 	/* default FT2232 values */
 	uint16_t vendor_id = FTDI_VID;
 	uint16_t device_id = FTDI_8U2232C_PID;
+	char *serial;
+
+	char *arg;
 
 	ftdi = ftdi_new();
 	if (!ftdi) {
@@ -130,7 +197,17 @@ int barebox_libftdi1_init(void)
 		goto error;
 	}
 
-	ret = ftdi_usb_open(ftdi, vendor_id, device_id);
+	arg = extract_param(&libftdi_options, "device_id", ",");
+	if (arg)
+		device_id = strtoul(arg, NULL, 0);
+
+	arg = extract_param(&libftdi_options, "vendor_id", ",");
+	if (arg)
+		vendor_id = strtoul(arg, NULL, 0);
+
+	serial = extract_param(&libftdi_options, "serial", ",");
+
+	ret = ftdi_usb_open_desc(ftdi, vendor_id, device_id, NULL, serial);
 	if (ret < 0 && ret != -5) {
 		fprintf(stderr, "unable to open ftdi device: %d (%s)\n",
 			ret, ftdi_get_error_string(ftdi));
-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/5] sandbox_defconfig: enable gpio, spi, i2c and led stuff
  2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
                   ` (2 preceding siblings ...)
  2017-10-10 12:26 ` [PATCH 3/5] sandbox: parse libftdi options Antony Pavlov
@ 2017-10-10 12:26 ` Antony Pavlov
  2017-10-10 12:26 ` [PATCH 5/5] sandbox: dts: add i2c and spi libftdi1 bit-bang example Antony Pavlov
  4 siblings, 0 replies; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/sandbox/configs/sandbox_defconfig | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/configs/sandbox_defconfig b/arch/sandbox/configs/sandbox_defconfig
index dbaff12bfb..2c9b59d15e 100644
--- a/arch/sandbox/configs/sandbox_defconfig
+++ b/arch/sandbox/configs/sandbox_defconfig
@@ -53,6 +53,11 @@ CONFIG_CMD_CRC_CMP=y
 CONFIG_CMD_MM=y
 CONFIG_CMD_DETECT=y
 CONFIG_CMD_FLASH=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_LED=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_LED_TRIGGER=y
 CONFIG_CMD_2048=y
 CONFIG_CMD_OF_NODE=y
 CONFIG_CMD_OF_PROPERTY=y
@@ -66,9 +71,26 @@ CONFIG_NET_NETCONSOLE=y
 CONFIG_OFDEVICE=y
 CONFIG_OF_BAREBOX_DRIVERS=y
 CONFIG_DRIVER_NET_TAP=y
-# CONFIG_SPI is not set
+CONFIG_DRIVER_SPI_GPIO=y
+CONFIG_I2C=y
+CONFIG_I2C_GPIO=y
+CONFIG_I2C_MUX=y
+CONFIG_I2C_MUX_PCA954x=y
+CONFIG_MTD=y
+CONFIG_MTD_M25P80=y
 CONFIG_VIDEO=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
+CONFIG_LED=y
+CONFIG_LED_GPIO=y
+CONFIG_LED_GPIO_OF=y
+CONFIG_LED_GPIO_RGB=y
+CONFIG_LED_GPIO_BICOLOR=y
+CONFIG_LED_TRIGGERS=y
+CONFIG_EEPROM_AT25=y
+CONFIG_EEPROM_AT24=y
+CONFIG_GPIO_74164=y
+CONFIG_GPIO_PCA953X=y
+CONFIG_GPIO_SX150X=y
 # CONFIG_PINCTRL is not set
 CONFIG_FS_CRAMFS=y
 CONFIG_FS_EXT4=y
-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/5] sandbox: dts: add i2c and spi libftdi1 bit-bang example
  2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
                   ` (3 preceding siblings ...)
  2017-10-10 12:26 ` [PATCH 4/5] sandbox_defconfig: enable gpio, spi, i2c and led stuff Antony Pavlov
@ 2017-10-10 12:26 ` Antony Pavlov
  4 siblings, 0 replies; 12+ messages in thread
From: Antony Pavlov @ 2017-10-10 12:26 UTC (permalink / raw)
  To: barebox

Usage:

  barebox$ make sandbox_defconfig
  barebox$ sed -i "s/# CONFIG_GPIO_LIBFTDI1.*$/CONFIG_GPIO_LIBFTDI1=y/" .config

  # edit arch/sandbox/dts/sandbox.dts if necessary

  barebox$ make
  barebox$ sudo ./barebox -d arch/sandbox/dts/sandbox.dtb

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 arch/sandbox/dts/sandbox.dts | 47 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 2595aa13fa..44c7f92237 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -1,7 +1,54 @@
 /dts-v1/;
 
 #include "skeleton.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
+	gpio0: gpio@0 {
+		compatible = "barebox,libftdi1-gpio";
+		/* use ACBUS[7:0] */
+		gpio-controller;
+		#gpio-cells = <2>;
+		status = "okay";
+	};
 
+	spi0 {
+		compatible = "spi-gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpio-sck  = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+		gpio-mosi = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+		gpio-miso = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+		cs-gpios  = <&gpio0 3 GPIO_ACTIVE_HIGH>;
+
+		num-chipselects = <1>;
+
+		status = "disabled";
+
+		m25p128@0 {
+			compatible = "m25p128", "jedec,spi-nor";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0>;
+			spi-max-frequency = <1000000>;
+		};
+	};
+
+	i2c0: i2c0 {
+		compatible = "i2c-gpio";
+		gpios = <&gpio0 4 GPIO_ACTIVE_HIGH /* sda */
+			&gpio0 5 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		status = "disabled";
+
+		eeprom: eeprom@50 {
+			compatible = "atmel,24c32";
+			reg = <0x50>;
+		};
+	};
 };
-- 
2.14.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] sandbox: add gpio support with libftdi1
  2017-10-10 12:26 ` [PATCH 2/5] sandbox: add gpio support with libftdi1 Antony Pavlov
@ 2017-10-16  7:56   ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2017-10-16  7:56 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

Hi Antony,

On Tue, Oct 10, 2017 at 03:26:28PM +0300, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  arch/sandbox/Kconfig                           |   1 +
>  arch/sandbox/Makefile                          |   6 +-
>  arch/sandbox/mach-sandbox/include/mach/linux.h |  11 ++
>  arch/sandbox/os/Makefile                       |   3 +
>  arch/sandbox/os/ftdi.c                         | 173 +++++++++++++++++++++++++
>  drivers/gpio/Kconfig                           |   4 +
>  drivers/gpio/Makefile                          |   1 +
>  drivers/gpio/gpio-libftdi1.c                   | 125 ++++++++++++++++++
>  8 files changed, 323 insertions(+), 1 deletion(-)
> 

...

> +static struct ft2232_bitbang ftbb;
> +

...

> +
> +int barebox_libftdi1_init(void)
> +{
> +	struct ftdi_context *ftdi;
> +	int ret;
> +	/* default FT2232 values */
> +	uint16_t vendor_id = FTDI_VID;
> +	uint16_t device_id = FTDI_8U2232C_PID;
> +
> +	ftdi = ftdi_new();
> +	if (!ftdi) {
> +		fprintf(stderr, "ftdi_new failed\n");
> +		goto error;
> +	}
> +
> +	ret = ftdi_usb_open(ftdi, vendor_id, device_id);
> +	if (ret < 0 && ret != -5) {
> +		fprintf(stderr, "unable to open ftdi device: %d (%s)\n",
> +			ret, ftdi_get_error_string(ftdi));
> +		goto error;
> +	}

What does a return value of -5 mean? Isn't that an error?

> +
> +	ftdi_set_interface(ftdi, INTERFACE_A);
> +	ftdi_set_bitmode(ftdi, 0x00, BITMODE_MPSSE);
> +
> +	ftbb.ftdi = ftdi;
> +
> +	/* reset pins to default neutral state */
> +	ftbb.dir = 0;
> +	ftbb.odata = 0;
> +	ftdi_set_high_byte_data_dir(&ftbb);
> +
> +	return 0;
> +
> +error:
> +	return -1;
> +}
> +
> +struct ft2232_bitbang *barebox_libftdi1_open(void)
> +{
> +	if (barebox_libftdi1_init() < 0) {
> +		printf("Could not initialize ftdi\n");
> +		return NULL;
> +	}
> +
> +	return &ftbb;
> +}

Somethings fishy here. Do you want to create a new struct ft2232_bitbang
instance for each caller or do you want to return the same instance for
every call to barebox_libftdi1_open()? If you want to do the former you
shouldn't create a static struct ft2232_bitbang, but instead allocate it
dynamically. If you want to do the latter then you should do a "if
(initialized) return existing_instance".

> +	gpio->chip.dev = dev;
> +	gpio->chip.ops = &libftdi1_gpio_ops;
> +	gpio->chip.base = 0;
> +	gpio->chip.ngpio = 8;
> +
> +	ret = gpiochip_add(&gpio->chip);
> +
> +	dev_dbg(dev, "%d: probed gpio%d with base %d\n",
> +			ret, dev->id, gpio->chip.base);
> +
> +	return 0;

Would be good to actually check 'ret' for errors.

Sascha


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] sandbox: parse libftdi options
  2017-10-10 12:26 ` [PATCH 3/5] sandbox: parse libftdi options Antony Pavlov
@ 2017-10-16  8:21   ` Sascha Hauer
  2017-10-16 14:10     ` Antony Pavlov
  0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2017-10-16  8:21 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

On Tue, Oct 10, 2017 at 03:26:29PM +0300, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  arch/sandbox/Makefile    |  2 +-
>  arch/sandbox/os/common.c | 12 ++++++--
>  arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 89 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
> index 34e9165787..e3e46ed12d 100644
> --- a/arch/sandbox/os/ftdi.c
> +++ b/arch/sandbox/os/ftdi.c
> @@ -20,6 +20,7 @@
>  #include <unistd.h>
>  #include <ftdi.h>
>  #include <errno.h>
> +#include <string.h>
>  #include <mach/linux.h>
>  
>  #define FTDI_VID		0x0403	/* Vendor Id */
> @@ -38,6 +39,8 @@ struct ft2232_bitbang {
>  
>  static struct ft2232_bitbang ftbb;
>  
> +extern const char *libftdi_options;
> +
>  static inline int ftdi_flush(struct ftdi_context *ftdi)
>  {
>  	uint8_t buf[1];
> @@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
>  		ftbb->odata &= ~BIT(off);
>  }
>  
> +/* This is a somewhat hacked function similar in some ways to strtok().
> + * It will look for needle with a subsequent '=' in haystack, return a copy of
> + * needle and remove everything from the first occurrence of needle to the next
> + * delimiter from haystack.
> + */
> +static char *extract_param(const char *const *haystack, const char *needle,
> +			const char *delim)
> +{

Parsing comma separated option lists is something we do more than once.
Right now we already have parseopt_b and parseopt_hu. Would be nice to
have this function alongside with the existing functions. Also
parseopt_* look simpler to follow, it may be worth adopting the code for
this function.

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] sandbox: parse libftdi options
  2017-10-16 14:10     ` Antony Pavlov
@ 2017-10-16 14:06       ` Sascha Hauer
  2017-10-19 11:55         ` Antony Pavlov
  0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2017-10-16 14:06 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

On Mon, Oct 16, 2017 at 05:10:11PM +0300, Antony Pavlov wrote:
> On Mon, 16 Oct 2017 10:21:58 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Tue, Oct 10, 2017 at 03:26:29PM +0300, Antony Pavlov wrote:
> > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > ---
> > >  arch/sandbox/Makefile    |  2 +-
> > >  arch/sandbox/os/common.c | 12 ++++++--
> > >  arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
> > >  3 files changed, 89 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
> > > index 34e9165787..e3e46ed12d 100644
> > > --- a/arch/sandbox/os/ftdi.c
> > > +++ b/arch/sandbox/os/ftdi.c
> > > @@ -20,6 +20,7 @@
> > >  #include <unistd.h>
> > >  #include <ftdi.h>
> > >  #include <errno.h>
> > > +#include <string.h>
> > >  #include <mach/linux.h>
> > >  
> > >  #define FTDI_VID		0x0403	/* Vendor Id */
> > > @@ -38,6 +39,8 @@ struct ft2232_bitbang {
> > >  
> > >  static struct ft2232_bitbang ftbb;
> > >  
> > > +extern const char *libftdi_options;
> > > +
> > >  static inline int ftdi_flush(struct ftdi_context *ftdi)
> > >  {
> > >  	uint8_t buf[1];
> > > @@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
> > >  		ftbb->odata &= ~BIT(off);
> > >  }
> > >  
> > > +/* This is a somewhat hacked function similar in some ways to strtok().
> > > + * It will look for needle with a subsequent '=' in haystack, return a copy of
> > > + * needle and remove everything from the first occurrence of needle to the next
> > > + * delimiter from haystack.
> > > + */
> > > +static char *extract_param(const char *const *haystack, const char *needle,
> > > +			const char *delim)
> > > +{
> > 
> > Parsing comma separated option lists is something we do more than once.
> > Right now we already have parseopt_b and parseopt_hu. Would be nice to
> > have this function alongside with the existing functions. Also
> > parseopt_* look simpler to follow, it may be worth adopting the code for
> > this function.
> 
> At the moment the parseopt_* functions are in the fs/parseopt.c file.
> Adding the parseopt_ul() for parsing u32 option value in arch/sandbox/os/ftdi.c
> will lead to moving fs/parseopt.c file to common code, e.g. to lib/.
> 
> Is it ok to add 'obj-y += parseopt.o' to the lib/Makefile file?

obj-y should be fine. ATM fs/parseopt.c is always compiled aswell.

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] sandbox: parse libftdi options
  2017-10-16  8:21   ` Sascha Hauer
@ 2017-10-16 14:10     ` Antony Pavlov
  2017-10-16 14:06       ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Antony Pavlov @ 2017-10-16 14:10 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Mon, 16 Oct 2017 10:21:58 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Tue, Oct 10, 2017 at 03:26:29PM +0300, Antony Pavlov wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  arch/sandbox/Makefile    |  2 +-
> >  arch/sandbox/os/common.c | 12 ++++++--
> >  arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 89 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
> > index 34e9165787..e3e46ed12d 100644
> > --- a/arch/sandbox/os/ftdi.c
> > +++ b/arch/sandbox/os/ftdi.c
> > @@ -20,6 +20,7 @@
> >  #include <unistd.h>
> >  #include <ftdi.h>
> >  #include <errno.h>
> > +#include <string.h>
> >  #include <mach/linux.h>
> >  
> >  #define FTDI_VID		0x0403	/* Vendor Id */
> > @@ -38,6 +39,8 @@ struct ft2232_bitbang {
> >  
> >  static struct ft2232_bitbang ftbb;
> >  
> > +extern const char *libftdi_options;
> > +
> >  static inline int ftdi_flush(struct ftdi_context *ftdi)
> >  {
> >  	uint8_t buf[1];
> > @@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
> >  		ftbb->odata &= ~BIT(off);
> >  }
> >  
> > +/* This is a somewhat hacked function similar in some ways to strtok().
> > + * It will look for needle with a subsequent '=' in haystack, return a copy of
> > + * needle and remove everything from the first occurrence of needle to the next
> > + * delimiter from haystack.
> > + */
> > +static char *extract_param(const char *const *haystack, const char *needle,
> > +			const char *delim)
> > +{
> 
> Parsing comma separated option lists is something we do more than once.
> Right now we already have parseopt_b and parseopt_hu. Would be nice to
> have this function alongside with the existing functions. Also
> parseopt_* look simpler to follow, it may be worth adopting the code for
> this function.

At the moment the parseopt_* functions are in the fs/parseopt.c file.
Adding the parseopt_ul() for parsing u32 option value in arch/sandbox/os/ftdi.c
will lead to moving fs/parseopt.c file to common code, e.g. to lib/.

Is it ok to add 'obj-y += parseopt.o' to the lib/Makefile file?

May be you prefere to introduce special Kconfig option for parseopt.o conditional
compilation?

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] sandbox: parse libftdi options
  2017-10-19 11:55         ` Antony Pavlov
@ 2017-10-19 11:52           ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2017-10-19 11:52 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox

On Thu, Oct 19, 2017 at 02:55:40PM +0300, Antony Pavlov wrote:
> On Mon, 16 Oct 2017 16:06:18 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Mon, Oct 16, 2017 at 05:10:11PM +0300, Antony Pavlov wrote:
> > > On Mon, 16 Oct 2017 10:21:58 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 
> > > > On Tue, Oct 10, 2017 at 03:26:29PM +0300, Antony Pavlov wrote:
> > > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > > > ---
> > > > >  arch/sandbox/Makefile    |  2 +-
> > > > >  arch/sandbox/os/common.c | 12 ++++++--
> > > > >  arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
> > > > >  3 files changed, 89 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
> > > > > index 34e9165787..e3e46ed12d 100644
> > > > > --- a/arch/sandbox/os/ftdi.c
> > > > > +++ b/arch/sandbox/os/ftdi.c
> > > > > @@ -20,6 +20,7 @@
> > > > >  #include <unistd.h>
> > > > >  #include <ftdi.h>
> > > > >  #include <errno.h>
> > > > > +#include <string.h>
> > > > >  #include <mach/linux.h>
> > > > >  
> > > > >  #define FTDI_VID		0x0403	/* Vendor Id */
> > > > > @@ -38,6 +39,8 @@ struct ft2232_bitbang {
> > > > >  
> > > > >  static struct ft2232_bitbang ftbb;
> > > > >  
> > > > > +extern const char *libftdi_options;
> > > > > +
> > > > >  static inline int ftdi_flush(struct ftdi_context *ftdi)
> > > > >  {
> > > > >  	uint8_t buf[1];
> > > > > @@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
> > > > >  		ftbb->odata &= ~BIT(off);
> > > > >  }
> > > > >  
> > > > > +/* This is a somewhat hacked function similar in some ways to strtok().
> > > > > + * It will look for needle with a subsequent '=' in haystack, return a copy of
> > > > > + * needle and remove everything from the first occurrence of needle to the next
> > > > > + * delimiter from haystack.
> > > > > + */
> > > > > +static char *extract_param(const char *const *haystack, const char *needle,
> > > > > +			const char *delim)
> > > > > +{
> > > > 
> > > > Parsing comma separated option lists is something we do more than once.
> > > > Right now we already have parseopt_b and parseopt_hu. Would be nice to
> > > > have this function alongside with the existing functions. Also
> > > > parseopt_* look simpler to follow, it may be worth adopting the code for
> > > > this function.
> > > 
> > > At the moment the parseopt_* functions are in the fs/parseopt.c file.
> > > Adding the parseopt_ul() for parsing u32 option value in arch/sandbox/os/ftdi.c
> > > will lead to moving fs/parseopt.c file to common code, e.g. to lib/.
> > > 
> > > Is it ok to add 'obj-y += parseopt.o' to the lib/Makefile file?
> > 
> > obj-y should be fine. ATM fs/parseopt.c is always compiled aswell.
> 
> Currently parseopt_hu() returns void and has no explicit flag for parsing error
> situation. I think that it is reasonable to use explicit error flag in new
> parseopt_ul() for parsing u32 option value, e.g. use function return value
> as an error flag.
> 
> Any suggestion?

Yes, returning an error is a good idea and given that the return value
is currently unused this is a good place.

Sascha

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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/5] sandbox: parse libftdi options
  2017-10-16 14:06       ` Sascha Hauer
@ 2017-10-19 11:55         ` Antony Pavlov
  2017-10-19 11:52           ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Antony Pavlov @ 2017-10-19 11:55 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Mon, 16 Oct 2017 16:06:18 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Mon, Oct 16, 2017 at 05:10:11PM +0300, Antony Pavlov wrote:
> > On Mon, 16 Oct 2017 10:21:58 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > On Tue, Oct 10, 2017 at 03:26:29PM +0300, Antony Pavlov wrote:
> > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > > > ---
> > > >  arch/sandbox/Makefile    |  2 +-
> > > >  arch/sandbox/os/common.c | 12 ++++++--
> > > >  arch/sandbox/os/ftdi.c   | 79 +++++++++++++++++++++++++++++++++++++++++++++++-
> > > >  3 files changed, 89 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/arch/sandbox/os/ftdi.c b/arch/sandbox/os/ftdi.c
> > > > index 34e9165787..e3e46ed12d 100644
> > > > --- a/arch/sandbox/os/ftdi.c
> > > > +++ b/arch/sandbox/os/ftdi.c
> > > > @@ -20,6 +20,7 @@
> > > >  #include <unistd.h>
> > > >  #include <ftdi.h>
> > > >  #include <errno.h>
> > > > +#include <string.h>
> > > >  #include <mach/linux.h>
> > > >  
> > > >  #define FTDI_VID		0x0403	/* Vendor Id */
> > > > @@ -38,6 +39,8 @@ struct ft2232_bitbang {
> > > >  
> > > >  static struct ft2232_bitbang ftbb;
> > > >  
> > > > +extern const char *libftdi_options;
> > > > +
> > > >  static inline int ftdi_flush(struct ftdi_context *ftdi)
> > > >  {
> > > >  	uint8_t buf[1];
> > > > @@ -116,6 +119,67 @@ void barebox_libftdi1_gpio_set_value(struct ft2232_bitbang *ftbb,
> > > >  		ftbb->odata &= ~BIT(off);
> > > >  }
> > > >  
> > > > +/* This is a somewhat hacked function similar in some ways to strtok().
> > > > + * It will look for needle with a subsequent '=' in haystack, return a copy of
> > > > + * needle and remove everything from the first occurrence of needle to the next
> > > > + * delimiter from haystack.
> > > > + */
> > > > +static char *extract_param(const char *const *haystack, const char *needle,
> > > > +			const char *delim)
> > > > +{
> > > 
> > > Parsing comma separated option lists is something we do more than once.
> > > Right now we already have parseopt_b and parseopt_hu. Would be nice to
> > > have this function alongside with the existing functions. Also
> > > parseopt_* look simpler to follow, it may be worth adopting the code for
> > > this function.
> > 
> > At the moment the parseopt_* functions are in the fs/parseopt.c file.
> > Adding the parseopt_ul() for parsing u32 option value in arch/sandbox/os/ftdi.c
> > will lead to moving fs/parseopt.c file to common code, e.g. to lib/.
> > 
> > Is it ok to add 'obj-y += parseopt.o' to the lib/Makefile file?
> 
> obj-y should be fine. ATM fs/parseopt.c is always compiled aswell.

Currently parseopt_hu() returns void and has no explicit flag for parsing error
situation. I think that it is reasonable to use explicit error flag in new
parseopt_ul() for parsing u32 option value, e.g. use function return value
as an error flag.

Any suggestion?

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-19 11:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 12:26 [PATCH 0/5] sandbox: add gpio support with libftdi1 Antony Pavlov
2017-10-10 12:26 ` [PATCH 1/5] sandbox: avoid symbol conflict for {open,read,close}dir Antony Pavlov
2017-10-10 12:26 ` [PATCH 2/5] sandbox: add gpio support with libftdi1 Antony Pavlov
2017-10-16  7:56   ` Sascha Hauer
2017-10-10 12:26 ` [PATCH 3/5] sandbox: parse libftdi options Antony Pavlov
2017-10-16  8:21   ` Sascha Hauer
2017-10-16 14:10     ` Antony Pavlov
2017-10-16 14:06       ` Sascha Hauer
2017-10-19 11:55         ` Antony Pavlov
2017-10-19 11:52           ` Sascha Hauer
2017-10-10 12:26 ` [PATCH 4/5] sandbox_defconfig: enable gpio, spi, i2c and led stuff Antony Pavlov
2017-10-10 12:26 ` [PATCH 5/5] sandbox: dts: add i2c and spi libftdi1 bit-bang example Antony Pavlov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox