lkml.org 
[lkml]   [2019]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH lora-next 3/5] net: lora: sx130x: Add PicoCell serdev driver
From
Date
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

\
 
 \ /
  Last update: 2019-01-05 10:30    [W:0.125 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site