RE: [RFC net-next 15/15] net: lora: Add Semtech SX1301

[Ben Whitten]

Hi Andreas,

Excellent work on doing this I have also been working on and off
this personally for some time.
Have a look at my repository [1] for sx1301 and sx1257 drivers,
I use regmaps capability of switching pages which should simplify
your driver considerably, I also have a full register map and bit field.

I have also been trying to use the clk framework to capture the various
routing that the cards have.

I will dig into this series this evening.

[1] https://github.com/BWhitten/linux-stable/tree/971aadc8fdfe842020d912449bdd71b33d576fe3/drivers/net/lora


> Subject: [RFC net-next 15/15] net: lora: Add Semtech SX1301
> 
> The Semtech SX1301 was the first multi-channel LoRa "concentrator".
> It uses a SPI interface to the host as well as a dual SPI interface to
> its radios. These two have been implemented as spi_controller, so that
> the Device Tree can specify whether the respective module uses two
> SX1257, two SX1255 or a combination of these or some unforeseen chipset.
> 
> This implementation is the most recent - initialization is not yet
> complete, it will need to load firmware into the two on-chip MCUs.
> 
> Unfortunately there is no full datasheet with register descriptions,
> only a BSD-licensed userspace HAL implementation using spidev devices.
> Therefore some register names are unknown.
> 
> Cc: Ben Whitten <ben.whitten@xxxxxxxxxxxxx>
> Cc: Steve deRosier <derosier@xxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Michael RÃder <michael.roeder@xxxxxxxx>
> Cc: Ken Yu (çå) <ken.yu@xxxxxxxxxxxxxxx>
> Cc: linux-spi@xxxxxxxxxxxxxxx
> Signed-off-by: Andreas FÃrber <afaerber@xxxxxxx>
> ---
>  drivers/net/lora/Kconfig  |   7 +
>  drivers/net/lora/Makefile |   3 +
>  drivers/net/lora/sx1301.c | 446
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 456 insertions(+)
>  create mode 100644 drivers/net/lora/sx1301.c
> 
> diff --git a/drivers/net/lora/Kconfig b/drivers/net/lora/Kconfig
> index 68c7480d7812..950450e353b4 100644
> --- a/drivers/net/lora/Kconfig
> +++ b/drivers/net/lora/Kconfig
> @@ -45,6 +45,13 @@ config LORA_SX1276
>  	help
>  	  Semtech SX1272/1276/1278
> 
> +config LORA_SX1301
> +	tristate "Semtech SX1301 SPI driver"
> +	default y
> +	depends on SPI
> +	help
> +	  Semtech SX1301
> +
>  config LORA_USI
>  	tristate "USI WM-SG-SM-42 driver"
>  	default y
> diff --git a/drivers/net/lora/Makefile b/drivers/net/lora/Makefile
> index 44c578bde7d5..1cc1e3aa189b 100644
> --- a/drivers/net/lora/Makefile
> +++ b/drivers/net/lora/Makefile
> @@ -22,6 +22,9 @@ lora-sx1257-y := sx1257.o
>  obj-$(CONFIG_LORA_SX1276) += lora-sx1276.o
>  lora-sx1276-y := sx1276.o
> 
> +obj-$(CONFIG_LORA_SX1301) += lora-sx1301.o
> +lora-sx1301-y := sx1301.o
> +
>  obj-$(CONFIG_LORA_USI) += lora-usi.o
>  lora-usi-y := usi.o
> 
> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
> new file mode 100644
> index 000000000000..5c936c1116d1
> --- /dev/null
> +++ b/drivers/net/lora/sx1301.c
> @@ -0,0 +1,446 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Semtech SX1301 LoRa concentrator
> + *
> + * Copyright (c) 2018 Andreas FÃrber
> + *
> + * Based on SX1301 HAL code:
> + * Copyright (c) 2013 Semtech-Cycleo
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/lora.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/lora/dev.h>
> +#include <linux/spi/spi.h>
> +
> +#define REG_PAGE_RESET			0
> +#define REG_VERSION			1
> +#define REG_2_SPI_RADIO_A_DATA		33
> +#define REG_2_SPI_RADIO_A_DATA_READBACK	34
> +#define REG_2_SPI_RADIO_A_ADDR		35
> +#define REG_2_SPI_RADIO_A_CS		37
> +#define REG_2_SPI_RADIO_B_DATA		38
> +#define REG_2_SPI_RADIO_B_DATA_READBACK	39
> +#define REG_2_SPI_RADIO_B_ADDR		40
> +#define REG_2_SPI_RADIO_B_CS		42
> +
> +#define REG_PAGE_RESET_SOFT_RESET	BIT(7)
> +
> +#define REG_16_GLOBAL_EN		BIT(3)
> +
> +#define REG_17_CLK32M_EN		BIT(0)
> +
> +#define REG_2_43_RADIO_A_EN		BIT(0)
> +#define REG_2_43_RADIO_B_EN		BIT(1)
> +#define REG_2_43_RADIO_RST		BIT(2)
> +
> +struct spi_sx1301 {
> +	struct spi_device *parent;
> +	u8 page;
> +	u8 regs;
> +};
> +
> +struct sx1301_priv {
> +	struct lora_priv lora;
> +	struct gpio_desc *rst_gpio;
> +	u8 cur_page;
> +	struct spi_controller *radio_a_ctrl, *radio_b_ctrl;
> +};
> +
> +static int sx1301_read(struct spi_device *spi, u8 reg, u8 *val)
> +{
> +	u8 addr = reg & 0x7f;
> +	return spi_write_then_read(spi, &addr, 1, val, 1);
> +}
> +
> +static int sx1301_write(struct spi_device *spi, u8 reg, u8 val)
> +{
> +	u8 buf[2];
> +
> +	buf[0] = reg | BIT(7);
> +	buf[1] = val;
> +	return spi_write(spi, buf, 2);
> +}
> +
> +static int sx1301_page_switch(struct spi_device *spi, u8 page)
> +{
> +	struct sx1301_priv *priv = spi_get_drvdata(spi);
> +	int ret;
> +
> +	if (priv->cur_page == page)
> +		return 0;
> +
> +	dev_dbg(&spi->dev, "switching to page %u\n", (unsigned)page);
> +	ret = sx1301_write(spi, REG_PAGE_RESET, page & 0x3);
> +	if (ret) {
> +		dev_err(&spi->dev, "switching to page %u failed\n",
> (unsigned)page);
> +		return ret;
> +	}
> +
> +	priv->cur_page = page;
> +
> +	return 0;
> +}
> +
> +static int sx1301_soft_reset(struct spi_device *spi)
> +{
> +	return sx1301_write(spi, REG_PAGE_RESET,
> REG_PAGE_RESET_SOFT_RESET);
> +}
> +
> +#define REG_RADIO_X_DATA		0
> +#define REG_RADIO_X_DATA_READBACK	1
> +#define REG_RADIO_X_ADDR		2
> +#define REG_RADIO_X_CS			4
> +
> +static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
> +{
> +	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
> +	u8 cs;
> +	int ret;
> +
> +	dev_dbg(&ctrl->dev, "setting CS to %s\n", enable ? "1" : "0");
> +
> +	ret = sx1301_page_switch(ssx->parent, ssx->page);
> +	if (ret) {
> +		dev_warn(&ctrl->dev, "failed to switch page for CS (%d)\n",
> ret);
> +		return ret;
> +	}
> +
> +	ret = sx1301_read(ssx->parent, ssx->regs + REG_RADIO_X_CS, &cs);
> +	if (ret) {
> +		dev_warn(&ctrl->dev, "failed to read CS (%d)\n", ret);
> +		cs = 0;
> +	}
> +
> +	if (enable)
> +		cs |= BIT(0);
> +	else
> +		cs &= ~BIT(0);
> +
> +	ret = sx1301_write(ssx->parent, ssx->regs + REG_RADIO_X_CS, cs);
> +	if (ret)
> +		dev_warn(&ctrl->dev, "failed to write CS (%d)\n", ret);
> +
> +	return 0;
> +}
> +
> +static void sx1301_radio_spi_set_cs(struct spi_device *spi, bool enable)
> +{
> +	int ret;
> +
> +	dev_dbg(&spi->dev, "setting SPI CS to %s\n", enable ? "1" : "0");
> +
> +	if (enable)
> +		return;
> +
> +	ret = sx1301_radio_set_cs(spi->controller, enable);
> +	if (ret)
> +		dev_warn(&spi->dev, "failed to write CS (%d)\n", ret);
> +}
> +
> +static int sx1301_radio_spi_transfer_one(struct spi_controller *ctrl,
> +	struct spi_device *spi, struct spi_transfer *xfr)
> +{
> +	struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
> +	const u8 *tx_buf = xfr->tx_buf;
> +	u8 *rx_buf = xfr->rx_buf;
> +	int ret;
> +
> +	if (xfr->len == 0 || xfr->len > 3)
> +		return -EINVAL;
> +
> +	dev_dbg(&spi->dev, "transferring one (%u)\n", xfr->len);
> +
> +	ret = sx1301_page_switch(ssx->parent, ssx->page);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to switch page for transfer
> (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	if (tx_buf) {
> +		ret = sx1301_write(ssx->parent, ssx->regs +
> REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
> +		if (ret) {
> +			dev_err(&spi->dev, "SPI radio address write
> failed\n");
> +			return ret;
> +		}
> +
> +		ret = sx1301_write(ssx->parent, ssx->regs +
> REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
> +		if (ret) {
> +			dev_err(&spi->dev, "SPI radio data write failed\n");
> +			return ret;
> +		}
> +
> +		ret = sx1301_radio_set_cs(ctrl, true);
> +		if (ret) {
> +			dev_err(&spi->dev, "SPI radio CS set failed\n");
> +			return ret;
> +		}
> +
> +		ret = sx1301_radio_set_cs(ctrl, false);
> +		if (ret) {
> +			dev_err(&spi->dev, "SPI radio CS unset failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (rx_buf) {
> +		ret = sx1301_read(ssx->parent, ssx->regs +
> REG_RADIO_X_DATA_READBACK, &rx_buf[xfr->len - 1]);
> +		if (ret) {
> +			dev_err(&spi->dev, "SPI radio data read failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void sx1301_radio_setup(struct spi_controller *ctrl)
> +{
> +	ctrl->mode_bits = SPI_CS_HIGH | SPI_NO_CS;
> +	ctrl->bits_per_word_mask = SPI_BPW_MASK(8);
> +	ctrl->num_chipselect = 1;
> +	ctrl->set_cs = sx1301_radio_spi_set_cs;
> +	ctrl->transfer_one = sx1301_radio_spi_transfer_one;
> +}
> +
> +static int sx1301_probe(struct spi_device *spi)
> +{
> +	struct net_device *netdev;
> +	struct sx1301_priv *priv;
> +	struct spi_sx1301 *radio;
> +	struct gpio_desc *rst;
> +	int ret;
> +	u8 val;
> +
> +	rst = devm_gpiod_get_optional(&spi->dev, "reset",
> GPIOD_OUT_LOW);
> +	if (IS_ERR(rst))
> +		return PTR_ERR(rst);
> +
> +	gpiod_set_value_cansleep(rst, 1);
> +	msleep(100);
> +	gpiod_set_value_cansleep(rst, 0);
> +	msleep(100);
> +
> +	spi->bits_per_word = 8;
> +	spi_setup(spi);
> +
> +	ret = sx1301_read(spi, REG_VERSION, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "version read failed\n");
> +		goto err_version;
> +	}
> +
> +	if (val != 103) {
> +		dev_err(&spi->dev, "unexpected version: %u\n", val);
> +		ret = -ENXIO;
> +		goto err_version;
> +	}
> +
> +	netdev = alloc_loradev(sizeof(*priv));
> +	if (!netdev) {
> +		ret = -ENOMEM;
> +		goto err_alloc_loradev;
> +	}
> +
> +	priv = netdev_priv(netdev);
> +	priv->rst_gpio = rst;
> +	priv->cur_page = 0xff;
> +
> +	spi_set_drvdata(spi, netdev);
> +	SET_NETDEV_DEV(netdev, &spi->dev);
> +
> +	ret = sx1301_write(spi, REG_PAGE_RESET, 0);
> +	if (ret) {
> +		dev_err(&spi->dev, "page/reset write failed\n");
> +		return ret;
> +	}
> +
> +	ret = sx1301_soft_reset(spi);
> +	if (ret) {
> +		dev_err(&spi->dev, "soft reset failed\n");
> +		return ret;
> +	}
> +
> +	ret = sx1301_read(spi, 16, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "16 read failed\n");
> +		return ret;
> +	}
> +
> +	val &= ~REG_16_GLOBAL_EN;
> +
> +	ret = sx1301_write(spi, 16, val);
> +	if (ret) {
> +		dev_err(&spi->dev, "16 write failed\n");
> +		return ret;
> +	}
> +
> +	ret = sx1301_read(spi, 17, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "17 read failed\n");
> +		return ret;
> +	}
> +
> +	val &= ~REG_17_CLK32M_EN;
> +
> +	ret = sx1301_write(spi, 17, val);
> +	if (ret) {
> +		dev_err(&spi->dev, "17 write failed\n");
> +		return ret;
> +	}
> +
> +	ret = sx1301_page_switch(spi, 2);
> +	if (ret) {
> +		dev_err(&spi->dev, "page 2 switch failed\n");
> +		return ret;
> +	}
> +
> +	ret = sx1301_read(spi, 43, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 read failed\n");
> +		return ret;
> +	}
> +
> +	val |= REG_2_43_RADIO_B_EN | REG_2_43_RADIO_A_EN;
> +
> +	ret = sx1301_write(spi, 43, val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 write failed\n");
> +		return ret;
> +	}
> +
> +	msleep(500);
> +
> +	ret = sx1301_read(spi, 43, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 read failed\n");
> +		return ret;
> +	}
> +
> +	val |= REG_2_43_RADIO_RST;
> +
> +	ret = sx1301_write(spi, 43, val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 write failed\n");
> +		return ret;
> +	}
> +
> +	msleep(5);
> +
> +	ret = sx1301_read(spi, 43, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 read failed\n");
> +		return ret;
> +	}
> +
> +	val &= ~REG_2_43_RADIO_RST;
> +
> +	ret = sx1301_write(spi, 43, val);
> +	if (ret) {
> +		dev_err(&spi->dev, "2|43 write failed\n");
> +		return ret;
> +	}
> +
> +	/* radio A */
> +
> +	priv->radio_a_ctrl = spi_alloc_master(&spi->dev, sizeof(*radio));
> +	if (!priv->radio_a_ctrl) {
> +		ret = -ENOMEM;
> +		goto err_radio_a_alloc;
> +	}
> +
> +	sx1301_radio_setup(priv->radio_a_ctrl);
> +	priv->radio_a_ctrl->dev.of_node = of_get_child_by_name(spi-
> >dev.of_node, "radio-a");
> +
> +	radio = spi_controller_get_devdata(priv->radio_a_ctrl);
> +	radio->page = 2;
> +	radio->regs = REG_2_SPI_RADIO_A_DATA;
> +	radio->parent = spi;
> +
> +	dev_info(&spi->dev, "registering radio A SPI\n");
> +
> +	ret = devm_spi_register_controller(&spi->dev, priv->radio_a_ctrl);
> +	if (ret) {
> +		dev_err(&spi->dev, "radio A SPI register failed\n");
> +		goto err_radio_a_register;
> +	}
> +
> +	/* radio B */
> +
> +	priv->radio_b_ctrl = spi_alloc_master(&spi->dev, sizeof(*radio));
> +	if (!priv->radio_b_ctrl) {
> +		ret = -ENOMEM;
> +		goto err_radio_b_alloc;
> +	}
> +
> +	sx1301_radio_setup(priv->radio_b_ctrl);
> +	priv->radio_b_ctrl->dev.of_node = of_get_child_by_name(spi-
> >dev.of_node, "radio-b");
> +
> +	radio = spi_controller_get_devdata(priv->radio_b_ctrl);
> +	radio->page = 2;
> +	radio->regs = REG_2_SPI_RADIO_B_DATA;
> +	radio->parent = spi;
> +
> +	dev_info(&spi->dev, "registering radio B SPI\n");
> +
> +	ret = devm_spi_register_controller(&spi->dev, priv->radio_b_ctrl);
> +	if (ret) {
> +		dev_err(&spi->dev, "radio B SPI register failed\n");
> +		goto err_radio_b_register;
> +	}
> +
> +	dev_info(&spi->dev, "SX1301 module probed\n");
> +
> +	return 0;
> +err_radio_b_register:
> +	spi_controller_put(priv->radio_b_ctrl);
> +err_radio_b_alloc:
> +err_radio_a_register:
> +	spi_controller_put(priv->radio_a_ctrl);
> +err_radio_a_alloc:
> +	free_loradev(netdev);
> +err_alloc_loradev:
> +err_version:
> +	return ret;
> +}
> +
> +static int sx1301_remove(struct spi_device *spi)
> +{
> +	struct net_device *netdev = spi_get_drvdata(spi);
> +
> +	//unregister_loradev(netdev);
> +	free_loradev(netdev);
> +
> +	dev_info(&spi->dev, "SX1301 module removed\n");
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sx1301_dt_ids[] = {
> +	{ .compatible = "semtech,sx1301" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sx1301_dt_ids);
> +#endif
> +
> +static struct spi_driver sx1301_spi_driver = {
> +	.driver = {
> +		.name = "sx1301",
> +		.of_match_table = of_match_ptr(sx1301_dt_ids),
> +	},
> +	.probe = sx1301_probe,
> +	.remove = sx1301_remove,
> +};
> +
> +module_spi_driver(sx1301_spi_driver);
> +
> +MODULE_DESCRIPTION("SX1301 SPI driver");
> +MODULE_AUTHOR("Andreas FÃrber <afaerber@xxxxxxx>");
> +MODULE_LICENSE("GPL");
> --
> 2.16.4