Re: [PATCH lora-next 3/5] net: lora: sx130x: Add PicoCell serdev driver
From: Ben Whitten
Date: Sat Jan 05 2019 - 04:30:43 EST
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@xxxxxxx>
---
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