Hi

Sorry for delay

13.11.2017, 00:05, "Hans-Frieder Vogt" <hfv...@gmx.net>:
> attached is the revised version of a driver that supports the serial OneWire 
> masters from iButtonLink(TM) in the w1 kernel driver. In order to be usable 
> it needs an updated userland tool (patch included in documentation file). The 
> kernel patch is against linux-next. Please try and comment.
>
> v2 of the patch set changes the following:
> - based on review input from Evgeniy (thanks!), introduced a helper function 
> to re-use more code and generally improved the style of the code (couldn't 
> get rid of all problems checkpatch identified, but many of them)
> - introduced a version check to make sure the hardware belongs to the 
> supported devices
> - removed a dependency on little endian system
> - further testing revealed re-entry problems. The protection of critical code 
> areas has therefore been changed from local spinlocks to a more toplevel 
> mutex-based system to enforce serialisation. I have not run into any problems 
> with unexpected feedback from the link device since.

This looks good to me, thank you!
Greg, please pull it into your tree

> Signed-off by: Hans-Frieder Vogt <hfv...@gmx.net>

Acked-by: Evgeniy Polyakov <z...@ioremap.net>

>  Documentation/w1/masters/00-INDEX   |    2
>  Documentation/w1/masters/linkserial |   44 ++
>  drivers/w1/masters/Kconfig          |   10
>  drivers/w1/masters/Makefile         |    1
>  drivers/w1/masters/linkserial.c     |  538 
> ++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serio.h          |    1
>  6 files changed, 596 insertions(+)
>
> diff --git a/Documentation/w1/masters/00-INDEX 
> b/Documentation/w1/masters/00-INDEX
> index 8330cf9325f0..33c522854126 100644
> --- a/Documentation/w1/masters/00-INDEX
> +++ b/Documentation/w1/masters/00-INDEX
> @@ -4,6 +4,8 @@ ds2482
>          - The Maxim/Dallas Semiconductor DS2482 provides 1-wire busses.
>  ds2490
>          - The Maxim/Dallas Semiconductor DS2490 builds USB <-> W1 bridges.
> +linkserial
> + - Serial to W1 bridge provided by iButtonLink devices.
>  mxc-w1
>          - W1 master controller driver found on Freescale MX2/MX3 SoCs
>  omap-hdq
> diff --git a/Documentation/w1/masters/linkserial 
> b/Documentation/w1/masters/linkserial
> new file mode 100644
> index 000000000000..d8b0a209ce4a
> --- /dev/null
> +++ b/Documentation/w1/masters/linkserial
> @@ -0,0 +1,44 @@
> +Kernel driver linkserial
> +========================
> +
> +Supported devices
> + * LinkUSB(TM)
> + * LinkOEM(TM)
> + * LINK(TM) untested
> +
> +Author: Hans-Frieder Vogt <hfv...@gmx.net>
> +
> +
> +Description
> +-----------
> +
> +1-wire bus master driver for iButtonLink LLC devices. These devices can 
> operate
> +in 2 modes: DS9097U Emulation and ASCII mode. The driver linkserial uses the
> +ASCII command interface only.
> +
> +This driver needs an adapted userspace program inputattach to be used. The
> +following patch modifies inputattach as needed:
> +
> +--- a/inputattach.c 2016-08-09 13:04:05.000000000 +0200
> ++++ b/inputattach.c 2017-10-14 23:49:08.594165130 +0200
> +@@ -49,6 +49,9 @@
> + #ifdef SYSTEMD_SUPPORT
> + #include <systemd/sd-daemon.h>
> + #endif
> ++#ifndef SERIO_ILINK
> ++#define SERIO_ILINK 0x42
> ++#endif
> +
> + static int readchar(int fd, unsigned char *c, int timeout)
> + {
> +@@ -867,6 +870,9 @@ static struct input_types input_types[]
> + { "--pulse8-cec", "-pulse8-cec", "Pulse Eight HDMI CEC dongle",
> + B9600, CS8,
> + SERIO_PULSE8_CEC, 0x00, 0x00, 0, NULL },
> ++{ "--linkserial", "-linkserial", "Link Onewire master",
> ++ B9600, CS8,
> ++ SERIO_ILINK, 0x00, 0x00, 0, NULL },
> + { NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, NULL }
> + };
> +
> +
> diff --git a/drivers/w1/masters/Kconfig b/drivers/w1/masters/Kconfig
> index 1708b2300c7a..eba57c2f0751 100644
> --- a/drivers/w1/masters/Kconfig
> +++ b/drivers/w1/masters/Kconfig
> @@ -34,6 +34,16 @@ config W1_MASTER_DS2482
>            This driver can also be built as a module. If so, the module
>            will be called ds2482.
>
> +config W1_MASTER_ILINK
> + tristate "iButtonLink(TM) serial to 1-Wire bridge"
> + depends on SERIO
> + help
> + Say Y here to get support for the iButtonLink(TM) serial to 1-Wire
> + bridges like the LINK(TM), the LinkUSB(TM) and the LinkOEM(TM).
> +
> + This driver can also be built as a module. If so, the module
> + will be called linkserial.
> +
>  config W1_MASTER_MXC
>          tristate "Freescale MXC 1-wire busmaster"
>          depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/w1/masters/Makefile b/drivers/w1/masters/Makefile
> index 18954cae4256..3fe656187c17 100644
> --- a/drivers/w1/masters/Makefile
> +++ b/drivers/w1/masters/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_W1_MASTER_MATROX) += matrox_w1.o
>  obj-$(CONFIG_W1_MASTER_DS2490) += ds2490.o
>  obj-$(CONFIG_W1_MASTER_DS2482) += ds2482.o
> +obj-$(CONFIG_W1_MASTER_ILINK) += linkserial.o
>  obj-$(CONFIG_W1_MASTER_MXC) += mxc_w1.o
>
>  obj-$(CONFIG_W1_MASTER_DS1WM) += ds1wm.o
> diff --git a/drivers/w1/masters/linkserial.c b/drivers/w1/masters/linkserial.c
> new file mode 100644
> index 000000000000..69aceacda6c2
> --- /dev/null
> +++ b/drivers/w1/masters/linkserial.c
> @@ -0,0 +1,538 @@
> +/*
> + * driver for W1 master based on iButtonLink(TM) Link devices
> + *
> + * Copyright (C) 2017 Hans-Frieder Vogt <hfv...@gmx.net>
> + * 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; version 2 of the License.
> + *
> + * 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/delay.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/serio.h>
> +#include <linux/string.h>
> +#include <linux/mutex.h>
> +
> +#include <linux/w1.h>
> +
> +#define BUFFER_SIZE 133
> +
> +#define LINK_STATE_IDLE 0
> +#define LINK_STATE_BUSY -1
> +
> +#define MODE_ASCII 0
> +#define MODE_BYTE 1
> +
> +#define DRV_VERSION "0.1"
> +#define MAX_LINK_VERSION_LENGTH 36
> +
> +#define TIMEOUT 100
> +
> +static DECLARE_WAIT_QUEUE_HEAD(wq);
> +static DEFINE_MUTEX(link_mutex);
> +
> +struct link_data {
> + struct serio *serio;
> + struct w1_bus_master master;
> +
> + int state;
> + int mode;
> + unsigned char buffer[BUFFER_SIZE];
> + unsigned int pos;
> + int pull_up;
> + int expected;
> +};
> +
> +static char val2char[] = {'0', '1', '2', '3', '4', '5', '6', '7',
> + '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
> +
> +static u8 char2val[] = {/* 0x30 */ 0, 1, 2, 3, 4, 5, 6, 7,
> + /* 0x38 */ 8, 9, 0x0f, 0x0f, 0x0f, 0x0f, 0x0f, 0x0f,
> + /* 0x40 */ 0x0f, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f};
> +
> +static inline int conv_hex(char c)
> +{
> + int ret;
> +
> + if ((c >= 0x30) && (c <= 0x46))
> + ret = char2val[c-0x30];
> + else
> + ret = 0x0f;
> + return ret;
> +}
> +
> +static int link_read(struct link_data *link)
> +{
> + wait_event_interruptible_timeout(wq, link->state == LINK_STATE_IDLE,
> + msecs_to_jiffies(TIMEOUT));
> +
> + if (link->state != LINK_STATE_IDLE) {
> + dev_err(&link->serio->dev, "%s: Timeout (pos=%d)!\n", __func__,
> + link->pos);
> + link->pos = 0;
> + return 0;
> + }
> + if (link->mode == MODE_ASCII) {
> + /* remove <CR><LF> */
> + if ((link->pos >= 2) && (link->buffer[link->pos-2] == 0x0d) &&
> + (link->buffer[link->pos-1] == 0x0a)) {
> + link->pos -= 2;
> + link->buffer[link->pos] = '\0';
> + }
> + }
> +
> + return link->pos;
> +}
> +
> +static void link_write_raw(struct link_data *link, u8 val)
> +{
> + struct serio *serio = link->serio;
> +
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->mode = MODE_ASCII;
> + serio_write(serio, val);
> +}
> +
> +/* change to ASCII mode
> + * => send <CR>
> + * <= <CR><LF>
> + */
> +static int link_mode_ascii(struct link_data *link)
> +{
> + int len;
> +
> + if (link->mode != MODE_ASCII) {
> + /* back to ASCII mode */
> + link_write_raw(link, 0x0d); /* cr */
> + /* ignore return value */
> + len = link_read(link);
> + }
> + return 0;
> +}
> +
> +/* change to BYTE mode
> + * => send 'p' or 'b'
> + * <= none
> + */
> +static int link_mode_byte(struct link_data *link)
> +{
> + struct serio *serio = link->serio;
> +
> + if (link->mode != MODE_BYTE) {
> + link->mode = MODE_BYTE;
> + /* change to byte mode */
> + if (link->pull_up)
> + serio_write(serio, 'p');
> + else
> + serio_write(serio, 'b');
> + }
> + return 0;
> +}
> +
> +static void link_write_hex_bytes(struct link_data *link, const u8 *buf, int 
> len)
> +{
> + struct serio *serio = link->serio;
> + const u8 *p = buf;
> + int i;
> +
> + /* make sure we are in hex mode */
> + link_mode_byte(link);
> +
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + link->expected = 2*len;
> + for (i = 0; i < len; i++, p++) {
> + serio_write(serio, val2char[*p >> 4]);
> + serio_write(serio, val2char[*p & 0x0f]);
> + }
> +}
> +
> +static void link_w1_write_byte(void *data, u8 byte)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int len, err;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return;
> + link_write_hex_bytes(link, &byte, 1);
> +
> + /* get back byte as confirmation */
> + len = link_read(link);
> + if (len != 2)
> + dev_err(&serio->dev, "%s: Read %d bytes, expected 2!\n",
> + __func__, len);
> + mutex_unlock(&link_mutex);
> +}
> +
> +static u8 link_w1_read_byte(void *data)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + const u8 byte = 0xff;
> + u8 ret;
> + int len, err;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return 0xff;
> + link_write_hex_bytes(link, &byte, 1);
> +
> + len = link_read(link);
> + if (len == 2)
> + ret = (conv_hex(link->buffer[0]) << 4) +
> + conv_hex(link->buffer[1]);
> + else {
> + dev_err(&serio->dev, "%s: Read %d bytes, expected 2!\n",
> + __func__, len);
> + ret = 0xff;
> + }
> + mutex_unlock(&link_mutex);
> +
> + return ret;
> +}
> +
> +static void link_w1_write_block(void *data, const u8 *buf, int len)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int l, err;
> +
> + if (len <= 0)
> + return;
> + if (len*2 > BUFFER_SIZE)
> + return;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return;
> + link_write_hex_bytes(link, buf, len);
> +
> + l = link_read(link);
> + if (l != 2*len)
> + dev_err(&serio->dev, "%s: Read %d bytes, expected %d!\n",
> + __func__, l, 2*len);
> + mutex_unlock(&link_mutex);
> +}
> +
> +static u8 link_w1_read_block(void *data, u8 *buf, int len)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + u8 ret;
> + int l, i, err;
> + u8 *wbuf;
> + u8 *p = buf;
> +
> + if (len <= 0)
> + return 0;
> + if (len*2 > BUFFER_SIZE)
> + return -E2BIG;
> +
> + wbuf = kzalloc(len, GFP_KERNEL);
> + if (!wbuf)
> + return -ENOMEM;
> + memset(wbuf, 0xff, len);
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return 0;
> + link_write_hex_bytes(link, wbuf, len);
> +
> + l = link_read(link);
> + if (l == 2*len) {
> + for (i = 0; i < l; i += 2, p++) {
> + *p = (conv_hex(link->buffer[i]) << 4) +
> + conv_hex(link->buffer[i+1]);
> + }
> + ret = len;
> + } else {
> + dev_err(&serio->dev, "%s: Read %d bytes, expected %d!\n",
> + __func__, l, len);
> + ret = 0;
> + }
> + mutex_unlock(&link_mutex);
> +
> + kfree(wbuf);
> + return ret;
> +}
> +
> +static void link_w1_search(void *data, struct w1_master *master,
> + u8 search_type, w1_slave_found_callback callback)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + int len, err;
> + u64 id, idle;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return;
> + link_mode_ascii(link);
> +
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + serio_write(serio, 'f');
> + while (1) {
> + len = link_read(link);
> + if (len != 18) {
> + dev_err(&serio->dev, "%s: Read %d bytes, expected 18!\n",
> + __func__, len);
> + break;
> + }
> + /* buffer should contain now something like
> + * "+,1234567890123456"
> + */
> + err = kstrtou64(&link->buffer[2], 16, &idle);
> + if (err)
> + break;
> + id = le64_to_cpu(idle);
> + callback(master, id);
> + if (link->buffer[0] == '+') {
> + link->pos = 0;
> + link->state = LINK_STATE_BUSY;
> + serio_write(serio, 'n');
> + } else
> + break;
> + }
> +
> + master->search_id = 0;
> + mutex_unlock(&link_mutex);
> +}
> +
> +static u8 link_w1_reset_bus(void *data)
> +{
> + struct link_data *link = data;
> + struct serio *serio = link->serio;
> + u8 ret = 0, len, status;
> + int err;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return 0xff;
> + link_mode_ascii(link);
> + link_write_raw(link, 'r');
> +
> + len = link_read(link);
> + if (len <= 0) {
> + dev_err(&serio->dev, "%s: Zero read!\n", __func__);
> + goto common_out;
> + }
> + if (len > 1)
> + dev_err(&serio->dev, "%s: Read %d bytes, expected 1!\n",
> + __func__, len);
> + status = link->buffer[0];
> + switch (status) {
> + case 'P':
> + break;
> + case 'N':
> + dev_warn(&serio->dev, "%s: No devices on bus\n", __func__);
> + ret = 1;
> + break;
> + case 'S':
> + dev_err(&serio->dev, "%s: Shortened bus!\n", __func__);
> + ret = 0xff;
> + break;
> + default:
> + dev_err(&serio->dev, "%s: Unknown link response '%c'(0x%02x)!\n",
> + __func__, status, status);
> + ret = 0xff;
> + }
> +
> +common_out:
> + mutex_unlock(&link_mutex);
> + return ret;
> +}
> +
> +static u8 link_w1_set_pullup(void *data, int delay)
> +{
> + struct link_data *link = data;
> + int err;
> +
> + err = mutex_lock_interruptible(&link_mutex);
> + if (err)
> + return 0xff;
> + link->pull_up = delay;
> + mutex_unlock(&link_mutex);
> +
> + return 0;
> +}
> +
> +static void linkserial_disconnect(struct serio *serio)
> +{
> + struct link_data *link = serio_get_drvdata(serio);
> +
> + w1_remove_master_device(&link->master);
> + serio_close(serio);
> + kfree(link);
> +
> + dev_info(&serio->dev, "Disconnected from device\n");
> +}
> +
> +static int link_version(struct link_data *link)
> +{
> + struct serio *serio = link->serio;
> + int len, err;
> +
> + /* version command */
> + link_write_raw(link, ' ');
> +
> + len = link_read(link);
> + if (len >= MAX_LINK_VERSION_LENGTH) {
> + err = -ENODEV;
> + dev_err(&serio->dev, "%s: Too long version string read (i=%d)\n",
> + __func__, len);
> + goto err_close;
> + }
> + if (strncmp(link->buffer, "LINK v1", 7) &&
> + strncmp(link->buffer, "LINK VM1", 8) &&
> + strncmp(link->buffer, "LinkUSB V1", 10)) {
> + err = -ENODEV;
> + dev_err(&serio->dev, "Unknown version of adapter \"%s\"\n",
> + link->buffer);
> + goto err_close;
> + }
> + dev_info(&serio->dev, "Version: \"%s\" (%d bytes)\n", link->buffer,
> + len);
> +
> + return 0;
> +
> +err_close:
> + return err;
> +}
> +
> +static int link_init(struct link_data *link)
> +{
> + struct serio *serio = link->serio;
> + int len;
> +
> + dev_info(&serio->dev, "%s: Ignore pot. following timeout message\n",
> + __func__);
> +
> + /* send break, reset with 9600 baud */
> + link_write_raw(link, 0);
> +
> + /* slurp in initial bytes */
> + len = link_read(link);
> + if (!len)
> + return -1;
> + else
> + return 0;
> +}
> +
> +static irqreturn_t linkserial_interrupt(struct serio *serio, unsigned char 
> data,
> + unsigned int flags)
> +{
> + struct link_data *link = serio_get_drvdata(serio);
> +
> + link->buffer[link->pos++] = data;
> +
> + switch (link->mode) {
> + case MODE_BYTE:
> + if (link->pos >= link->expected) {
> + link->buffer[link->pos] = '\0';
> + link->state = LINK_STATE_IDLE;
> + wake_up_interruptible(&wq);
> + }
> + break;
> + case MODE_ASCII:
> + if ((data == 0x0a) || (link->pos == BUFFER_SIZE-1)) {
> + link->buffer[link->pos] = '\0';
> + link->state = LINK_STATE_IDLE;
> + wake_up_interruptible(&wq);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int linkserial_connect(struct serio *serio, struct serio_driver *drv)
> +{
> + struct link_data *link;
> + int err;
> +
> + link = kzalloc(sizeof(struct link_data), GFP_KERNEL);
> + if (!link) {
> + err = -ENOMEM;
> + goto exit;
> + }
> + serio_set_drvdata(serio, link);
> +
> + err = serio_open(serio, drv);
> + if (err)
> + goto exit_free;
> +
> + link->serio = serio;
> +
> + err = link_init(link);
> + usleep_range(10000, 50000);
> + err = link_version(link);
> + if (err)
> + goto exit_close;
> +
> + link->master.data = link;
> + link->master.read_byte = link_w1_read_byte;
> + link->master.write_byte = link_w1_write_byte;
> + link->master.read_block = link_w1_read_block;
> + link->master.write_block = link_w1_write_block;
> + link->master.reset_bus = link_w1_reset_bus;
> + link->master.set_pullup = link_w1_set_pullup;
> + link->master.search = link_w1_search;
> +
> + err = w1_add_master_device(&link->master);
> + if (err)
> + goto exit_close;
> +
> + dev_info(&serio->dev, "Connected to device\n");
> + return 0;
> +
> +exit_close:
> + serio_close(serio);
> +exit_free:
> + kfree(link);
> +exit:
> + return err;
> +}
> +
> +static struct serio_device_id linkserial_serio_ids[] = {
> + {
> + .type = SERIO_RS232,
> + .proto = SERIO_ILINK,
> + .id = SERIO_ANY,
> + .extra = SERIO_ANY,
> + },
> + { 0 }
> +};
> +MODULE_DEVICE_TABLE(serio, linkserial_serio_ids);
> +
> +static struct serio_driver linkserial_drv = {
> + .driver = {
> + .name = "linkserial",
> + },
> + .description = "W1 bus master driver for iButtonLink(TM) devices",
> + .id_table = linkserial_serio_ids,
> + .connect = linkserial_connect,
> + .disconnect = linkserial_disconnect,
> + .interrupt = linkserial_interrupt,
> +};
> +
> +module_serio_driver(linkserial_drv);
> +
> +MODULE_AUTHOR("Hans-Frieder Vogt <hfv...@gmx.net>");
> +MODULE_DESCRIPTION("W1 bus master driver for iButtonLink(TM) devices");
> +MODULE_VERSION(DRV_VERSION);
> +MODULE_LICENSE("GPL");
> +
> diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h
> index a0cac1d8670d..14482bf6d789 100644
> --- a/include/uapi/linux/serio.h
> +++ b/include/uapi/linux/serio.h
> @@ -82,5 +82,6 @@
>  #define SERIO_EGALAX 0x3f
>  #define SERIO_PULSE8_CEC 0x40
>  #define SERIO_RAINSHADOW_CEC 0x41
> +#define SERIO_ILINK 0x42
>
>  #endif /* _UAPI_SERIO_H */

Reply via email to