| Message ID | 20190104112131.14451-4-afaerber@suse.de |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: lora: sx130x: USB CDC Picocell Gateway serdev driver via fake DT nodes | expand |
Am 04.01.19 um 12:21 schrieb Andreas Färber: > diff --git a/drivers/net/lora/sx130x_picogw.c b/drivers/net/lora/sx130x_picogw.c > new file mode 100644 > index 000000000000..577f9fb71d46 > --- /dev/null > +++ b/drivers/net/lora/sx130x_picogw.c [...] > + serdev_device_set_baudrate(sdev, 115200); > + serdev_device_set_parity(sdev, SERDEV_PARITY_NONE); > + serdev_device_set_flow_control(sdev, true); This should probably be false? https://github.com/Lora-net/picoGW_hal/blob/master/libloragw/src/loragw_com_linux.c#L65 tty.c_iflag &= ~(IXON | IXOFF | IXANY | ICRNL); ... tty.c_oflag &= ~(IXON | IXOFF | IXANY | ICRNL); Nothing that looks like RTS/CTS anywhere, which appears to be what the serdev implementation is switching with the above function. However, both true and false appeared to work equally so far. With one particular USB device I get the following warning/error, seemingly independent of whether I enable flow control above or not: [68640.481454] lora-picogw-usb 4-1:1.0: failed to set dtr/rts (imagine s/lora-picogw-usb/cdc_acm/ - cf. cdc-acm.c:acm_port_dtr_rts()) Looks like a firmware/hardware issue to me, since it appeared with the original cdc_acm driver on reboot, plus across reset, ports and hosts. Regards, Andreas
Am 04.01.19 um 12:21 schrieb Andreas Färber: > Currently there's still some bugs to be investigated, with communication > stalling on one device and another reading the radio versions wrong > (07 / 1f instead of 21, also observed on spi once). Reproducable 100% on SPI by setting REGCACHE_RBTREE in sx130x.c. Since this serdev driver was using REGCACHE_NONE still and I don't spot a register missing as volatile either, I guess it may be a timing issue? My earlier locking patch is applied, to rule out any non-determinism in the register access order due to radio vs. concentrator interactions. Regards, Andreas
Hi, On 04/01/2019 20:21, Andreas Färber wrote: > The picoGW reference MCU firmware implements a USB CDC or UART interface > with a set of serial commands. It can be found on multiple mPCIe cards > as well as USB adapters. > > https://github.com/Lora-net/picoGW_mcu > > That MCU design superseded earlier attempts of using FTDI chips (MPSSE) > for controlling the SPI based chip directly from the host. > > This commit adds a serdev driver implementing another regmap_bus to > abstract the register access and reuses our existing regmap driver on top. > Thereby we have an early proof of concept that we can drive both types > of modules/cards with a single driver core! > > It assumes there is only one SX130x (with its radios) accessible, whereas > some new cards reportedly also have an SX127x on-board. So the DT/driver > design may need to be reconsidered once such a card or documentation > becomes available. > It also assumes SX1255/1258 are fully compatible with "semtech,sx1257". > > Currently there's still some bugs to be investigated, with communication > stalling on one device and another reading the radio versions wrong > (07 / 1f instead of 21, also observed on spi once). > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > drivers/net/lora/Makefile | 2 + > drivers/net/lora/sx130x_picogw.c | 466 +++++++++++++++++++++++++++++++++++++++ We are missing a Kconfig select or depend, compilation fails if CONFIG_SERIAL_DEV_BUS is not selected. > 2 files changed, 468 insertions(+) > create mode 100644 drivers/net/lora/sx130x_picogw.c > > diff --git a/drivers/net/lora/Makefile b/drivers/net/lora/Makefile > index c6a0410f80ce..5fef38abf5ed 100644 > --- a/drivers/net/lora/Makefile > +++ b/drivers/net/lora/Makefile > @@ -25,6 +25,8 @@ lora-sx127x-y := sx127x.o > obj-$(CONFIG_LORA_SX130X) += lora-sx130x.o > lora-sx130x-y := sx130x.o > lora-sx130x-y += sx130x_radio.o > +obj-$(CONFIG_LORA_SX130X) += lora-sx130x-picogw.o > +lora-sx130x-picogw-y := sx130x_picogw.o > > obj-$(CONFIG_LORA_USI) += lora-usi.o > lora-usi-y := usi.o > diff --git a/drivers/net/lora/sx130x_picogw.c b/drivers/net/lora/sx130x_picogw.c > new file mode 100644 > index 000000000000..577f9fb71d46 > --- /dev/null > +++ b/drivers/net/lora/sx130x_picogw.c > @@ -0,0 +1,466 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Semtech SX1301/1308 PicoCell gateway serial MCU interface > + * > + * Copyright (c) 2018-2019 Andreas Färber > + */ > +#include <linux/completion.h> > +#include <linux/lora/sx130x.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > +#include <linux/serdev.h> > +#include <linux/serial.h> > +#include <linux/slab.h> > + > +struct picogw_device { > + struct serdev_device *serdev; > + > + u8 rx_buf[1024]; > + int rx_len; > + > + struct completion answer_comp; > + struct completion answer_read_comp; > +}; > + > +static inline struct picogw_device *picogw_get_drvdata(struct serdev_device *sdev) > +{ > + return sx130x_get_drvdata(&sdev->dev); > +} > + > +static bool picogw_valid_cmd(char ch) > +{ > + switch (ch) { > + case 'k': /* invalid command error */ > + case 'r': > + case 'w': > + case 'l': > + return true; > + default: > + return false; > + } > +} > + > +static int picogw_send_cmd(struct picogw_device *picodev, char cmd, > + u8 addr, const void *data, u16 data_len) > +{ > + struct serdev_device *sdev = picodev->serdev; > + u8 buf[4]; > + int i, ret; > + > + buf[0] = cmd; > + buf[1] = (data_len >> 8) & 0xff; > + buf[2] = (data_len >> 0) & 0xff; > + buf[3] = addr; > + > + dev_dbg(&sdev->dev, "%s: %c %02x %02x %02x\n", __func__, buf[0], > + (unsigned int)buf[1], (unsigned int)buf[2], (unsigned int)buf[3]); > + for (i = 0; i < data_len; i++) { > + dev_dbg(&sdev->dev, "%s: data %02x\n", __func__, (unsigned int)((const u8*)data)[i]); > + } > + > + ret = serdev_device_write_buf(sdev, buf, 4); > + if (ret < 0) > + return ret; > + if (ret != 4) > + return -EIO; > + > + if (data_len) { > + ret = serdev_device_write_buf(sdev, data, data_len); > + if (ret < 0) > + return ret; > + if (ret != data_len) > + return -EIO; > + } > + > + return 0; > +} > + > +static int picogw_recv_answer(struct picogw_device *picodev, > + char *cmd, bool *ack, void *buf, int buf_len, > + unsigned long timeout) > +{ > + int len; > + > + timeout = wait_for_completion_timeout(&picodev->answer_comp, timeout); > + if (!timeout) > + return -ETIMEDOUT; > + > + if (cmd) > + *cmd = picodev->rx_buf[0]; > + > + if (ack) > + *ack = (picodev->rx_buf[3] == 1); > + > + len = min(picodev->rx_len - 4, buf_len); > + if (buf) > + memcpy(buf, picodev->rx_buf + 4, len); > + > + reinit_completion(&picodev->answer_comp); > + complete(&picodev->answer_read_comp); > + > + return len; > +} > + > +static int picogw_reg_read(struct picogw_device *picodev, u8 addr, u8 *val, unsigned long timeout) > +{ > + const u8 dummy = 0; > + char cmd; > + bool ack; > + int ret; > + > + ret = picogw_send_cmd(picodev, 'r', addr, &dummy, false ? 1 : 0); > + if (ret) > + return ret; > + > + ret = picogw_recv_answer(picodev, &cmd, &ack, val, 1, timeout); > + if (ret < 0) > + return ret; > + if (cmd != 'r') > + return -EIO; > + if (!ack || ret != 1) > + return -EIO; > + > + return 0; > +} > + > +static int picogw_reg_write(struct picogw_device *picodev, u8 addr, u8 val, unsigned long timeout) > +{ > + char cmd; > + bool ack; > + int ret; > + > + ret = picogw_send_cmd(picodev, 'w', addr, &val, 1); > + if (ret) > + return ret; > + > + ret = picogw_recv_answer(picodev, &cmd, &ack, NULL, 0, timeout); > + if (ret < 0) > + return ret; > + if (cmd != 'w') > + return -EIO; > + if (!ack || ret != 0) > + return -EIO; > + > + return 0; > +} > + > +static int picogw_mcu_fw_check(struct picogw_device *picodev, > + u32 fw_version, u8 *id, unsigned long timeout) > +{ > + char cmd; > + bool ack; > + int ret; > + > + fw_version = cpu_to_be32(fw_version); > + ret = picogw_send_cmd(picodev, 'l', 0, &fw_version, sizeof(fw_version)); > + if (ret) > + return ret; > + > + ret = picogw_recv_answer(picodev, &cmd, &ack, id, id ? 8 : 0, timeout); > + if (ret < 0) > + return ret; > + if (cmd != 'l') > + return -EIO; > + if (id && ret != 8) > + return -EIO; > + > + return ack ? 0 : -ENOTSUPP; > +} > + > +static int picogw_regmap_gather_write(void *context, > + const void *reg_buf, size_t reg_size, const void *val_buf, size_t val_size) > +{ > + struct picogw_device *picodev = context; > + const u8 *addr_buf = reg_buf; > + const u8 *val = val_buf; > + u8 addr; > + int ret; > + > + //dev_dbg(&picodev->serdev->dev, "%s: 0x%x (reg_size %zu) 0x%x (val_size %zu)\n", > + // __func__, (unsigned int)addr_buf[0], reg_size, (unsigned int)val[0], val_size); > + > + if (reg_size != 1 || val_size > 0xffff) > + return -EINVAL; > + > + addr = addr_buf[0] & ~BIT(7); > + if (val_size == 1) { > + ret = picogw_reg_write(picodev, addr, val[0], HZ); > + if (ret) > + return ret; > + return 0; > + } else { > + /* TODO burst mode */ > + dev_err(&picodev->serdev->dev, "burst mode write not yet implemented\n"); > + return -ENOTSUPP; > + } > +} > + > +static int picogw_regmap_write(void *context, > + const void *data_buf, size_t count) > +{ > + const u8 *data = data_buf; > + if (count < 1) > + return -EINVAL; > + > + return picogw_regmap_gather_write(context, data, 1, data + 1, count - 1); > +} > + > +static int picogw_regmap_read(void *context, > + const void *reg_buf, size_t reg_size, void *val_buf, size_t val_size) > +{ > + struct picogw_device *picodev = context; > + const u8 *addr_buf = reg_buf; > + u8 addr; > + int ret; > + > + //dev_dbg(&picodev->serdev->dev, "%s: 0x%x (reg_size %zu) (val_size %zu)\n", __func__, (unsigned int)addr_buf[0], reg_size, val_size); > + > + if (reg_size != 1 || val_size > 0xffff) > + return -EINVAL; > + > + addr = addr_buf[0] & ~BIT(7); > + if (val_size == 1) { > + ret = picogw_reg_read(picodev, addr, val_buf, HZ); > + if (ret) > + return ret; > + return 0; > + } else { > + /* TODO burst mode */ > + dev_err(&picodev->serdev->dev, "burst mode read not yet implemented\n"); > + return -ENOTSUPP; > + } > +} > + > +static const struct regmap_bus picogw_regmap_bus = { > + .write = picogw_regmap_write, > + .gather_write = picogw_regmap_gather_write, > + .read = picogw_regmap_read, > + > + .max_raw_write = 0xffff, > + .max_raw_read = 0xffff, > +}; > + > +static int picogw_handle_answer(struct picogw_device *picodev) > +{ > + struct device *dev = &picodev->serdev->dev; > + unsigned int data_len = ((u16)picodev->rx_buf[1] << 8) | picodev->rx_buf[2]; > + int cmd_len = 4 + data_len; > + int i, ret; > + > + if (picodev->rx_len < 4) > + return 0; > + > + if (cmd_len > sizeof(picodev->rx_buf)) { > + dev_warn(dev, "answer too long (%u)\n", data_len); > + return 0; > + } > + > + if (picodev->rx_len < cmd_len) { > + dev_dbg(dev, "got %u, need %u bytes\n", picodev->rx_len, cmd_len); > + return 0; > + } > + > + dev_dbg(dev, "Answer %c =%u %s (%u)\n", picodev->rx_buf[0], > + (unsigned int)picodev->rx_buf[3], > + (picodev->rx_buf[3] == 1) ? "OK" : "K0", > + data_len); > + for (i = 0; i < data_len; i++) { > + //dev_dbg(dev, "%s: %02x\n", __func__, (unsigned int)picodev->rx_buf[4 + i]); > + } > + > + complete(&picodev->answer_comp); > + ret = wait_for_completion_interruptible_timeout(&picodev->answer_read_comp, HZ / 2); > + if (ret < 0) > + return 0; > + > + reinit_completion(&picodev->answer_read_comp); > + > + return cmd_len; > +} > + > +static int picogw_receive_buf(struct serdev_device *sdev, const u8 *data, size_t count) > +{ > + struct picogw_device *picodev = picogw_get_drvdata(sdev); > + size_t i; > + int len = 0; > + > + dev_dbg(&sdev->dev, "Receive (%zu)\n", count); > + > + if (completion_done(&picodev->answer_comp)) { > + dev_info(&sdev->dev, "RX waiting on completion\n"); > + return 0; > + } > + if (picodev->rx_len == sizeof(picodev->rx_buf)) { > + dev_warn(&sdev->dev, "RX buffer full\n"); > + return 0; > + } > + > + i = min(count, sizeof(picodev->rx_buf) - picodev->rx_len); > + if (i > 0) { > + memcpy(&picodev->rx_buf[picodev->rx_len], data, i); > + picodev->rx_len += i; > + len += i; > + } > + > + while (picodev->rx_len > 0) { > + /* search for valid beginning */ > + for (i = 0; i < picodev->rx_len; i++) { > + if (picogw_valid_cmd(picodev->rx_buf[i])) > + break; > + } > + if (i > 0) { > + dev_dbg(&sdev->dev, "skipping %zu bytes of garbage\n", i); > + if (i < picodev->rx_len) { > + memmove(picodev->rx_buf, &picodev->rx_buf[i], picodev->rx_len - i); > + picodev->rx_len -= i; > + } else > + picodev->rx_len = 0; > + } > + > + i = picogw_handle_answer(picodev); > + if (i == 0) > + break; > + > + if (i % 64 == 0) { > + dev_info(&sdev->dev, "skipping padding byte\n"); > + i++; > + } > + if (picodev->rx_len > i) > + memmove(picodev->rx_buf, &picodev->rx_buf[i], picodev->rx_len - i); > + if (picodev->rx_len >= i) > + picodev->rx_len -= i; > + } > + > + return len; > +} > + > +static const struct serdev_device_ops picogw_serdev_client_ops = { > + .receive_buf = picogw_receive_buf, > + .write_wakeup = serdev_device_write_wakeup, > +}; > + > +static int picogw_serdev_probe(struct serdev_device *sdev) > +{ > + struct picogw_device *picodev; > + struct regmap *regmap; > + u32 fw_version = 0x010a0006; Worth moving the version to a define? > + u8 mac[8];here > + int ret; > + > + //dev_info(&sdev->dev, "Probing\n"); > + > + picodev = devm_kzalloc(&sdev->dev, sizeof(*picodev), GFP_KERNEL); > + if (!picodev) > + return -ENOMEM; > + > + picodev->serdev = sdev; > + init_completion(&picodev->answer_comp); > + init_completion(&picodev->answer_read_comp); > + > + ret = serdev_device_open(sdev); > + if (ret) { > + dev_err(&sdev->dev, "Failed to open (%d)\n", ret); > + return ret; > + } > + > + serdev_device_set_baudrate(sdev, 115200); > + serdev_device_set_parity(sdev, SERDEV_PARITY_NONE); > + serdev_device_set_flow_control(sdev, true); > + > + regmap = devm_regmap_init(&sdev->dev, &picogw_regmap_bus, picodev, &sx130x_regmap_config); > + if (IS_ERR(regmap)) { > + ret = PTR_ERR(regmap); > + dev_err(&sdev->dev, "error initializing regmap (%d)\n", ret); > + serdev_device_close(sdev); > + return ret; > + } > + > + ret = sx130x_early_probe(regmap, NULL); > + if (ret) { > + serdev_device_close(sdev); > + return ret; > + } > + > + sx130x_set_drvdata(&sdev->dev, picodev); > + serdev_device_set_client_ops(sdev, &picogw_serdev_client_ops); > + > + //msleep(1000); > + ret = picogw_mcu_fw_check(picodev, fw_version, mac, HZ); > + if (!ret || ret == -ENOTSUPP) > + dev_info(&sdev->dev, "ID = %02x%02x%02x%02x%02x%02x%02x%02x\n", > + (unsigned int)mac[0], > + (unsigned int)mac[1], > + (unsigned int)mac[2], > + (unsigned int)mac[3], > + (unsigned int)mac[4], > + (unsigned int)mac[5], > + (unsigned int)mac[6], > + (unsigned int)mac[7]); > + while (ret == -ENOTSUPP && ((fw_version & 0xff) > 4)) { > + ret = picogw_mcu_fw_check(picodev, --fw_version, NULL, HZ); > + } > + if (ret == -ENOTSUPP) { > + dev_warn(&sdev->dev, "firmware check failed (%08x)\n", fw_version); > + } else { > + dev_err(&sdev->dev, "ID retrieval failed (%d)\n", ret); > + serdev_device_close(sdev); > + return ret; > + } > + > + ret = sx130x_probe(&sdev->dev); > + if (ret) { > + serdev_device_close(sdev); > + return ret; > + } > + > + //dev_info(&sdev->dev, "Done.\n"); > + > + return 0; > +} > + > +static void picogw_serdev_remove(struct serdev_device *sdev) > +{ > + sx130x_remove(&sdev->dev); > + > + serdev_device_close(sdev); > + > + //dev_info(&sdev->dev, "Removed\n"); > +} > + > +static const struct of_device_id picogw_serdev_of_match[] = { > + { .compatible = "semtech,lora-picocell" }, lora-picocell or lora-picogw... picocell has mobile connotations, should we match the module name? > + {} > +}; > +MODULE_DEVICE_TABLE(of, picogw_serdev_of_match); > + > +static struct serdev_device_driver picogw_serdev_driver = { > + .probe = picogw_serdev_probe, > + .remove = picogw_serdev_remove, > + .driver = { > + .name = "lora-picogw", > + .of_match_table = picogw_serdev_of_match, > + }, > +}; > + > +static int __init picogw_serdev_init(void) > +{ > + int ret; > + > + ret = serdev_device_driver_register(&picogw_serdev_driver); > + if (ret) { > + pr_err("serdev_device_driver_register failed (%d)", ret); > + return ret; > + } > + > + return 0; > +} > +module_init(picogw_serdev_init); > + > +static void __exit picogw_serdev_exit(void) > +{ > + serdev_device_driver_unregister(&picogw_serdev_driver); > +} > +module_exit(picogw_serdev_exit); > + > +MODULE_LICENSE("GPL"); > Thanks, Ben Whitten
On Fr, 2019-01-04 at 12:21 +0100, Andreas Färber wrote: > > +struct picogw_device { > + struct serdev_device *serdev; > + > + u8 rx_buf[1024]; No, you cannot do that. AFAICT this buffer can be used for DMA. Thus putting it into another data structure violates the rules of DMA coherency. You must allocate it separately. > +static int picogw_send_cmd(struct picogw_device *picodev, char cmd, > + u8 addr, const void *data, u16 data_len) > +{ > + struct serdev_device *sdev = picodev->serdev; > + u8 buf[4]; > + int i, ret; > + > + buf[0] = cmd; > + buf[1] = (data_len >> 8) & 0xff; > + buf[2] = (data_len >> 0) & 0xff; We have macros for converting endianness and unaligned access. > + buf[3] = addr; > + > + dev_dbg(&sdev->dev, "%s: %c %02x %02x %02x\n", __func__, buf[0], > + (unsigned int)buf[1], (unsigned int)buf[2], (unsigned int)buf[3]); > + for (i = 0; i < data_len; i++) { > + dev_dbg(&sdev->dev, "%s: data %02x\n", __func__, (unsigned int)((const u8*)data)[i]); > + } > + > + ret = serdev_device_write_buf(sdev, buf, 4); > + if (ret < 0) > + return ret; > + if (ret != 4) > + return -EIO; > + > + if (data_len) { > + ret = serdev_device_write_buf(sdev, data, data_len); > + if (ret < 0) > + return ret; > + if (ret != data_len) > + return -EIO; > + } > + > + return 0; > +} > + > +static int picogw_recv_answer(struct picogw_device *picodev, > + char *cmd, bool *ack, void *buf, int buf_len, > + unsigned long timeout) > +{ > + int len; > + > + timeout = wait_for_completion_timeout(&picodev->answer_comp, timeout); > + if (!timeout) > + return -ETIMEDOUT; And? The IO is still scheduled. Simply erroring out is problematic. If you see a timeout you need to kill the scheduled IO. > +static int picogw_handle_answer(struct picogw_device *picodev) > +{ > + struct device *dev = &picodev->serdev->dev; > + unsigned int data_len = ((u16)picodev->rx_buf[1] << 8) | picodev->rx_buf[2]; > + int cmd_len = 4 + data_len; > + int i, ret; > + > + if (picodev->rx_len < 4) > + return 0; > + > + if (cmd_len > sizeof(picodev->rx_buf)) { > + dev_warn(dev, "answer too long (%u)\n", data_len); > + return 0; > + } > + > + if (picodev->rx_len < cmd_len) { > + dev_dbg(dev, "got %u, need %u bytes\n", picodev->rx_len, cmd_len); > + return 0; > + } > + > + dev_dbg(dev, "Answer %c =%u %s (%u)\n", picodev->rx_buf[0], > + (unsigned int)picodev->rx_buf[3], > + (picodev->rx_buf[3] == 1) ? "OK" : "K0", > + data_len); > + for (i = 0; i < data_len; i++) { > + //dev_dbg(dev, "%s: %02x\n", __func__, (unsigned int)picodev->rx_buf[4 + i]); > + } > + > + complete(&picodev->answer_comp); > + ret = wait_for_completion_interruptible_timeout(&picodev->answer_read_comp, HZ / 2); Problematic. You have no idea when that complete() will have an effect. You are operating with an undefined timeout here. Theoretically it could be negative. Regards Oliver
diff --git a/drivers/net/lora/Makefile b/drivers/net/lora/Makefile index c6a0410f80ce..5fef38abf5ed 100644 --- a/drivers/net/lora/Makefile +++ b/drivers/net/lora/Makefile @@ -25,6 +25,8 @@ lora-sx127x-y := sx127x.o obj-$(CONFIG_LORA_SX130X) += lora-sx130x.o lora-sx130x-y := sx130x.o lora-sx130x-y += sx130x_radio.o +obj-$(CONFIG_LORA_SX130X) += lora-sx130x-picogw.o +lora-sx130x-picogw-y := sx130x_picogw.o obj-$(CONFIG_LORA_USI) += lora-usi.o lora-usi-y := usi.o diff --git a/drivers/net/lora/sx130x_picogw.c b/drivers/net/lora/sx130x_picogw.c new file mode 100644 index 000000000000..577f9fb71d46 --- /dev/null +++ b/drivers/net/lora/sx130x_picogw.c @@ -0,0 +1,466 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Semtech SX1301/1308 PicoCell gateway serial MCU interface + * + * Copyright (c) 2018-2019 Andreas Färber + */ +#include <linux/completion.h> +#include <linux/lora/sx130x.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regmap.h> +#include <linux/serdev.h> +#include <linux/serial.h> +#include <linux/slab.h> + +struct picogw_device { + struct serdev_device *serdev; + + u8 rx_buf[1024]; + int rx_len; + + struct completion answer_comp; + struct completion answer_read_comp; +}; + +static inline struct picogw_device *picogw_get_drvdata(struct serdev_device *sdev) +{ + return sx130x_get_drvdata(&sdev->dev); +} + +static bool picogw_valid_cmd(char ch) +{ + switch (ch) { + case 'k': /* invalid command error */ + case 'r': + case 'w': + case 'l': + return true; + default: + return false; + } +} + +static int picogw_send_cmd(struct picogw_device *picodev, char cmd, + u8 addr, const void *data, u16 data_len) +{ + struct serdev_device *sdev = picodev->serdev; + u8 buf[4]; + int i, ret; + + buf[0] = cmd; + buf[1] = (data_len >> 8) & 0xff; + buf[2] = (data_len >> 0) & 0xff; + buf[3] = addr; + + dev_dbg(&sdev->dev, "%s: %c %02x %02x %02x\n", __func__, buf[0], + (unsigned int)buf[1], (unsigned int)buf[2], (unsigned int)buf[3]); + for (i = 0; i < data_len; i++) { + dev_dbg(&sdev->dev, "%s: data %02x\n", __func__, (unsigned int)((const u8*)data)[i]); + } + + ret = serdev_device_write_buf(sdev, buf, 4); + if (ret < 0) + return ret; + if (ret != 4) + return -EIO; + + if (data_len) { + ret = serdev_device_write_buf(sdev, data, data_len); + if (ret < 0) + return ret; + if (ret != data_len) + return -EIO; + } + + return 0; +} + +static int picogw_recv_answer(struct picogw_device *picodev, + char *cmd, bool *ack, void *buf, int buf_len, + unsigned long timeout) +{ + int len; + + timeout = wait_for_completion_timeout(&picodev->answer_comp, timeout); + if (!timeout) + return -ETIMEDOUT; + + if (cmd) + *cmd = picodev->rx_buf[0]; + + if (ack) + *ack = (picodev->rx_buf[3] == 1); + + len = min(picodev->rx_len - 4, buf_len); + if (buf) + memcpy(buf, picodev->rx_buf + 4, len); + + reinit_completion(&picodev->answer_comp); + complete(&picodev->answer_read_comp); + + return len; +} + +static int picogw_reg_read(struct picogw_device *picodev, u8 addr, u8 *val, unsigned long timeout) +{ + const u8 dummy = 0; + char cmd; + bool ack; + int ret; + + ret = picogw_send_cmd(picodev, 'r', addr, &dummy, false ? 1 : 0); + if (ret) + return ret; + + ret = picogw_recv_answer(picodev, &cmd, &ack, val, 1, timeout); + if (ret < 0) + return ret; + if (cmd != 'r') + return -EIO; + if (!ack || ret != 1) + return -EIO; + + return 0; +} + +static int picogw_reg_write(struct picogw_device *picodev, u8 addr, u8 val, unsigned long timeout) +{ + char cmd; + bool ack; + int ret; + + ret = picogw_send_cmd(picodev, 'w', addr, &val, 1); + if (ret) + return ret; + + ret = picogw_recv_answer(picodev, &cmd, &ack, NULL, 0, timeout); + if (ret < 0) + return ret; + if (cmd != 'w') + return -EIO; + if (!ack || ret != 0) + return -EIO; + + return 0; +} + +static int picogw_mcu_fw_check(struct picogw_device *picodev, + u32 fw_version, u8 *id, unsigned long timeout) +{ + char cmd; + bool ack; + int ret; + + fw_version = cpu_to_be32(fw_version); + ret = picogw_send_cmd(picodev, 'l', 0, &fw_version, sizeof(fw_version)); + if (ret) + return ret; + + ret = picogw_recv_answer(picodev, &cmd, &ack, id, id ? 8 : 0, timeout); + if (ret < 0) + return ret; + if (cmd != 'l') + return -EIO; + if (id && ret != 8) + return -EIO; + + return ack ? 0 : -ENOTSUPP; +} + +static int picogw_regmap_gather_write(void *context, + const void *reg_buf, size_t reg_size, const void *val_buf, size_t val_size) +{ + struct picogw_device *picodev = context; + const u8 *addr_buf = reg_buf; + const u8 *val = val_buf; + u8 addr; + int ret; + + //dev_dbg(&picodev->serdev->dev, "%s: 0x%x (reg_size %zu) 0x%x (val_size %zu)\n", + // __func__, (unsigned int)addr_buf[0], reg_size, (unsigned int)val[0], val_size); + + if (reg_size != 1 || val_size > 0xffff) + return -EINVAL; + + addr = addr_buf[0] & ~BIT(7); + if (val_size == 1) { + ret = picogw_reg_write(picodev, addr, val[0], HZ); + if (ret) + return ret; + return 0; + } else { + /* TODO burst mode */ + dev_err(&picodev->serdev->dev, "burst mode write not yet implemented\n"); + return -ENOTSUPP; + } +} + +static int picogw_regmap_write(void *context, + const void *data_buf, size_t count) +{ + const u8 *data = data_buf; + if (count < 1) + return -EINVAL; + + return picogw_regmap_gather_write(context, data, 1, data + 1, count - 1); +} + +static int picogw_regmap_read(void *context, + const void *reg_buf, size_t reg_size, void *val_buf, size_t val_size) +{ + struct picogw_device *picodev = context; + const u8 *addr_buf = reg_buf; + u8 addr; + int ret; + + //dev_dbg(&picodev->serdev->dev, "%s: 0x%x (reg_size %zu) (val_size %zu)\n", __func__, (unsigned int)addr_buf[0], reg_size, val_size); + + if (reg_size != 1 || val_size > 0xffff) + return -EINVAL; + + addr = addr_buf[0] & ~BIT(7); + if (val_size == 1) { + ret = picogw_reg_read(picodev, addr, val_buf, HZ); + if (ret) + return ret; + return 0; + } else { + /* TODO burst mode */ + dev_err(&picodev->serdev->dev, "burst mode read not yet implemented\n"); + return -ENOTSUPP; + } +} + +static const struct regmap_bus picogw_regmap_bus = { + .write = picogw_regmap_write, + .gather_write = picogw_regmap_gather_write, + .read = picogw_regmap_read, + + .max_raw_write = 0xffff, + .max_raw_read = 0xffff, +}; + +static int picogw_handle_answer(struct picogw_device *picodev) +{ + struct device *dev = &picodev->serdev->dev; + unsigned int data_len = ((u16)picodev->rx_buf[1] << 8) | picodev->rx_buf[2]; + int cmd_len = 4 + data_len; + int i, ret; + + if (picodev->rx_len < 4) + return 0; + + if (cmd_len > sizeof(picodev->rx_buf)) { + dev_warn(dev, "answer too long (%u)\n", data_len); + return 0; + } + + if (picodev->rx_len < cmd_len) { + dev_dbg(dev, "got %u, need %u bytes\n", picodev->rx_len, cmd_len); + return 0; + } + + dev_dbg(dev, "Answer %c =%u %s (%u)\n", picodev->rx_buf[0], + (unsigned int)picodev->rx_buf[3], + (picodev->rx_buf[3] == 1) ? "OK" : "K0", + data_len); + for (i = 0; i < data_len; i++) { + //dev_dbg(dev, "%s: %02x\n", __func__, (unsigned int)picodev->rx_buf[4 + i]); + } + + complete(&picodev->answer_comp); + ret = wait_for_completion_interruptible_timeout(&picodev->answer_read_comp, HZ / 2); + if (ret < 0) + return 0; + + reinit_completion(&picodev->answer_read_comp); + + return cmd_len; +} + +static int picogw_receive_buf(struct serdev_device *sdev, const u8 *data, size_t count) +{ + struct picogw_device *picodev = picogw_get_drvdata(sdev); + size_t i; + int len = 0; + + dev_dbg(&sdev->dev, "Receive (%zu)\n", count); + + if (completion_done(&picodev->answer_comp)) { + dev_info(&sdev->dev, "RX waiting on completion\n"); + return 0; + } + if (picodev->rx_len == sizeof(picodev->rx_buf)) { + dev_warn(&sdev->dev, "RX buffer full\n"); + return 0; + } + + i = min(count, sizeof(picodev->rx_buf) - picodev->rx_len); + if (i > 0) { + memcpy(&picodev->rx_buf[picodev->rx_len], data, i); + picodev->rx_len += i; + len += i; + } + + while (picodev->rx_len > 0) { + /* search for valid beginning */ + for (i = 0; i < picodev->rx_len; i++) { + if (picogw_valid_cmd(picodev->rx_buf[i])) + break; + } + if (i > 0) { + dev_dbg(&sdev->dev, "skipping %zu bytes of garbage\n", i); + if (i < picodev->rx_len) { + memmove(picodev->rx_buf, &picodev->rx_buf[i], picodev->rx_len - i); + picodev->rx_len -= i; + } else + picodev->rx_len = 0; + } + + i = picogw_handle_answer(picodev); + if (i == 0) + break; + + if (i % 64 == 0) { + dev_info(&sdev->dev, "skipping padding byte\n"); + i++; + } + if (picodev->rx_len > i) + memmove(picodev->rx_buf, &picodev->rx_buf[i], picodev->rx_len - i); + if (picodev->rx_len >= i) + picodev->rx_len -= i; + } + + return len; +} + +static const struct serdev_device_ops picogw_serdev_client_ops = { + .receive_buf = picogw_receive_buf, + .write_wakeup = serdev_device_write_wakeup, +}; + +static int picogw_serdev_probe(struct serdev_device *sdev) +{ + struct picogw_device *picodev; + struct regmap *regmap; + u32 fw_version = 0x010a0006; + u8 mac[8]; + int ret; + + //dev_info(&sdev->dev, "Probing\n"); + + picodev = devm_kzalloc(&sdev->dev, sizeof(*picodev), GFP_KERNEL); + if (!picodev) + return -ENOMEM; + + picodev->serdev = sdev; + init_completion(&picodev->answer_comp); + init_completion(&picodev->answer_read_comp); + + ret = serdev_device_open(sdev); + if (ret) { + dev_err(&sdev->dev, "Failed to open (%d)\n", ret); + return ret; + } + + serdev_device_set_baudrate(sdev, 115200); + serdev_device_set_parity(sdev, SERDEV_PARITY_NONE); + serdev_device_set_flow_control(sdev, true); + + regmap = devm_regmap_init(&sdev->dev, &picogw_regmap_bus, picodev, &sx130x_regmap_config); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(&sdev->dev, "error initializing regmap (%d)\n", ret); + serdev_device_close(sdev); + return ret; + } + + ret = sx130x_early_probe(regmap, NULL); + if (ret) { + serdev_device_close(sdev); + return ret; + } + + sx130x_set_drvdata(&sdev->dev, picodev); + serdev_device_set_client_ops(sdev, &picogw_serdev_client_ops); + + //msleep(1000); + ret = picogw_mcu_fw_check(picodev, fw_version, mac, HZ); + if (!ret || ret == -ENOTSUPP) + dev_info(&sdev->dev, "ID = %02x%02x%02x%02x%02x%02x%02x%02x\n", + (unsigned int)mac[0], + (unsigned int)mac[1], + (unsigned int)mac[2], + (unsigned int)mac[3], + (unsigned int)mac[4], + (unsigned int)mac[5], + (unsigned int)mac[6], + (unsigned int)mac[7]); + while (ret == -ENOTSUPP && ((fw_version & 0xff) > 4)) { + ret = picogw_mcu_fw_check(picodev, --fw_version, NULL, HZ); + } + if (ret == -ENOTSUPP) { + dev_warn(&sdev->dev, "firmware check failed (%08x)\n", fw_version); + } else { + dev_err(&sdev->dev, "ID retrieval failed (%d)\n", ret); + serdev_device_close(sdev); + return ret; + } + + ret = sx130x_probe(&sdev->dev); + if (ret) { + serdev_device_close(sdev); + return ret; + } + + //dev_info(&sdev->dev, "Done.\n"); + + return 0; +} + +static void picogw_serdev_remove(struct serdev_device *sdev) +{ + sx130x_remove(&sdev->dev); + + serdev_device_close(sdev); + + //dev_info(&sdev->dev, "Removed\n"); +} + +static const struct of_device_id picogw_serdev_of_match[] = { + { .compatible = "semtech,lora-picocell" }, + {} +}; +MODULE_DEVICE_TABLE(of, picogw_serdev_of_match); + +static struct serdev_device_driver picogw_serdev_driver = { + .probe = picogw_serdev_probe, + .remove = picogw_serdev_remove, + .driver = { + .name = "lora-picogw", + .of_match_table = picogw_serdev_of_match, + }, +}; + +static int __init picogw_serdev_init(void) +{ + int ret; + + ret = serdev_device_driver_register(&picogw_serdev_driver); + if (ret) { + pr_err("serdev_device_driver_register failed (%d)", ret); + return ret; + } + + return 0; +} +module_init(picogw_serdev_init); + +static void __exit picogw_serdev_exit(void) +{ + serdev_device_driver_unregister(&picogw_serdev_driver); +} +module_exit(picogw_serdev_exit); + +MODULE_LICENSE("GPL");
The picoGW reference MCU firmware implements a USB CDC or UART interface with a set of serial commands. It can be found on multiple mPCIe cards as well as USB adapters. https://github.com/Lora-net/picoGW_mcu That MCU design superseded earlier attempts of using FTDI chips (MPSSE) for controlling the SPI based chip directly from the host. This commit adds a serdev driver implementing another regmap_bus to abstract the register access and reuses our existing regmap driver on top. Thereby we have an early proof of concept that we can drive both types of modules/cards with a single driver core! It assumes there is only one SX130x (with its radios) accessible, whereas some new cards reportedly also have an SX127x on-board. So the DT/driver design may need to be reconsidered once such a card or documentation becomes available. It also assumes SX1255/1258 are fully compatible with "semtech,sx1257". Currently there's still some bugs to be investigated, with communication stalling on one device and another reading the radio versions wrong (07 / 1f instead of 21, also observed on spi once). Signed-off-by: Andreas Färber <afaerber@suse.de> --- drivers/net/lora/Makefile | 2 + drivers/net/lora/sx130x_picogw.c | 466 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 drivers/net/lora/sx130x_picogw.c