| Message ID | 20180701110804.32415-16-afaerber@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@lairdtech.com> > Cc: Steve deRosier <derosier@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Michael Röder <michael.roeder@avnet.eu> > Cc: Ken Yu (禹凯) <ken.yu@rakwireless.com> > Cc: linux-spi@vger.kernel.org > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > 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@suse.de>"); > +MODULE_LICENSE("GPL"); > -- > 2.16.4
On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: > +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); > +} So we never disable chip select? > + if (tx_buf) { > + ret = sx1301_write(ssx->parent, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0); This looks confused. We're in an if (tx_buf) block but there's a use of the ternery operator that appears to be checking if we have a tx_buf? > + 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; > + } This looks awfully like you're coming in at the wrong abstraction layer and the hardware actually implements a register abstraction rather than a SPI one so you should be using regmap as the abstraction. > + 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; > + } > + } For a read we never set an address? > +static void sx1301_radio_setup(struct spi_controller *ctrl) > +{ > + ctrl->mode_bits = SPI_CS_HIGH | SPI_NO_CS; This controller has no chip select but we provided a set_cs operation?
Hi Mark, This driver is still evolving, there's newer code on my lora-next branch already: https://github.com/afaerber/linux/commits/lora-next The reason you're in CC on this RFC is two-fold: 1) You applied Ben's patch to associate "semtech,sx1301" with spidev, whereas I am now preparing a new driver for the same compatible. 2) This SPI device is in turn exposing the two SPI masters that you already found below, and I didn't see a sane way to split that code out into drivers/spi/, so it's in drivers/net/lora/ here - has there been any precedence either way? More inline ... Am 02.07.2018 um 18:12 schrieb Mark Brown: > On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: > >> +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); >> +} > > So we never disable chip select? Not here, I instead did that in transfer_one below. Unfortunately there seems to be no documentation, only reference code: https://github.com/Lora-net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L121 https://github.com/Lora-net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L165 It sets CS to 0 before writing to address and data registers, then immediately sets CS to 1 and back to 0 before reading or ending the write transaction. I've tried to force the same behavior in this driver. My guess was that CS is high-active during the short 1-0 cycle, because if it's low-active during the register writes then why the heck is it set to 0 again in the end instead of keeping at 1... confusing. Maybe the Semtech folks CC'ed can comment how these registers work? >> + if (tx_buf) { >> + ret = sx1301_write(ssx->parent, ssx->regs + REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0); > > This looks confused. We're in an if (tx_buf) block but there's a use of > the ternery operator that appears to be checking if we have a tx_buf? Yeah, as mentioned this RFC is not ready for merging - checkpatch.pl will complain about lines too long, and TODOs are sprinkled all over or not even mentioned. It's a Proof of Concept that a net_device could work for a wide range of spi and serdev based drivers, and on top this device has more than one channel, which may influence network-level design discussions. That said, I'll happily drop the second check. Thanks for spotting! >> + 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; >> + } > > This looks awfully like you're coming in at the wrong abstraction layer > and the hardware actually implements a register abstraction rather than > a SPI one so you should be using regmap as the abstraction. I don't understand. Ben has suggested using regmap for the SPI _device_ that we're talking to, which may be a good idea. But this SX1301 device in turn has two SPI _masters_ talking to an SX125x slave each. I don't see how using regmap instead of my wrappers avoids this spi_controller? The whole point of this spi_controller is to abstract and separate the SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver, instead of mixing it into the SX1301 driver - to me that looks cleaner and more extensible. It also has the side-effect that we could configure the two radios via DT (frequencies, clk output, etc.). You will find a datasheet with some diagrams mentioning "SPI" at: https://www.semtech.com/products/wireless-rf/lora-gateways/SX1301 >> + 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; >> + } >> + } > > For a read we never set an address? To read, you first write the address via tx_buf, then either in the same transfer in the third byte or in a subsequent one-byte transfer as first byte you get the data. If you have better ideas how to structure this, do let me know. >> +static void sx1301_radio_setup(struct spi_controller *ctrl) >> +{ >> + ctrl->mode_bits = SPI_CS_HIGH | SPI_NO_CS; > > This controller has no chip select but we provided a set_cs operation? Oops, I played around with those two options and was hoping SPI_NO_CS would avoid the undesired set_cs invocations, but it didn't work as expected and so I added the "if (enabled)" check above. Thanks for your review, Andreas
> Subject: Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301 > > Hi Mark, > > This driver is still evolving, there's newer code on my lora-next branch > already: https://github.com/afaerber/linux/commits/lora-next > > The reason you're in CC on this RFC is two-fold: > > 1) You applied Ben's patch to associate "semtech,sx1301" with spidev, > whereas I am now preparing a new driver for the same compatible. > > 2) This SPI device is in turn exposing the two SPI masters that you > already found below, and I didn't see a sane way to split that code out > into drivers/spi/, so it's in drivers/net/lora/ here - has there been > any precedence either way? In my work in progress driver I just register one controller for the sx1301 with two chip selects and use the chip select information to choose the correct radio to send to, this is based on the DT reg information. No need to register two separate masters. > More inline ... > > Am 02.07.2018 um 18:12 schrieb Mark Brown: > > On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: > > > >> +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); > >> +} > > > > So we never disable chip select? > > Not here, I instead did that in transfer_one below. > > Unfortunately there seems to be no documentation, only reference code: > > https://github.com/Lora- > net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L121 > https://github.com/Lora- > net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L165 > > It sets CS to 0 before writing to address and data registers, then > immediately sets CS to 1 and back to 0 before reading or ending the > write transaction. I've tried to force the same behavior in this driver. > My guess was that CS is high-active during the short 1-0 cycle, because > if it's low-active during the register writes then why the heck is it > set to 0 again in the end instead of keeping at 1... confusing. > > Maybe the Semtech folks CC'ed can comment how these registers work? > > >> + if (tx_buf) { > >> + ret = sx1301_write(ssx->parent, ssx->regs + > REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0); > > > > This looks confused. We're in an if (tx_buf) block but there's a use of > > the ternery operator that appears to be checking if we have a tx_buf? > > Yeah, as mentioned this RFC is not ready for merging - checkpatch.pl > will complain about lines too long, and TODOs are sprinkled all over or > not even mentioned. It's a Proof of Concept that a net_device could work > for a wide range of spi and serdev based drivers, and on top this device > has more than one channel, which may influence network-level design > discussions. > > That said, I'll happily drop the second check. Thanks for spotting! > > >> + 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; > >> + } > > > > This looks awfully like you're coming in at the wrong abstraction layer > > and the hardware actually implements a register abstraction rather than > > a SPI one so you should be using regmap as the abstraction. > > I don't understand. Ben has suggested using regmap for the SPI _device_ > that we're talking to, which may be a good idea. But this SX1301 device > in turn has two SPI _masters_ talking to an SX125x slave each. I don't > see how using regmap instead of my wrappers avoids this spi_controller? > The whole point of this spi_controller is to abstract and separate the > SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver, > instead of mixing it into the SX1301 driver - to me that looks cleaner > and more extensible. It also has the side-effect that we could configure > the two radios via DT (frequencies, clk output, etc.). You want an SPI controller in the SX1301 as the down stream radios are SPI and could be attached directly to a host SPI bus, makes sense to have one radio driver and talk through the SX1301. But you should use the regmap to access the SX1301 master controller registers. Example I use with one SPI master and some clock info: eg: sx1301: sx1301@0 { compatible = "semtech,sx1301"; reg = <0>; #address-cells = <1>; #size-cells = <0>; spi-max-frequency = <8000000>; gpios-reset = <&pioA 26 GPIO_ACTIVE_HIGH>; clocks = <&radio1 0>, <&clkhs 0>; clock-names = "clk32m", "clkhs"; radio0: sx1257@0 { compatible = "semtech,sx125x"; reg = <0>; spi-max-frequency = <8000000>; tx; clocks = <&tcxo 0>; clock-names = "tcxo"; }; radio1: sx1257@1 { compatible = "semtech,sx125x"; reg = <1>; spi-max-frequency = <8000000>; #clock-cells = <0>; clocks = <&tcxo 0>; clock-names = "tcxo"; clock-output-names = "clk32m"; }; }; > You will find a datasheet with some diagrams mentioning "SPI" at: > https://www.semtech.com/products/wireless-rf/lora-gateways/SX1301 > > >> + 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; > >> + } > >> + } > > > > For a read we never set an address? > > To read, you first write the address via tx_buf, then either in the same > transfer in the third byte or in a subsequent one-byte transfer as first > byte you get the data. > > If you have better ideas how to structure this, do let me know. > > >> +static void sx1301_radio_setup(struct spi_controller *ctrl) > >> +{ > >> + ctrl->mode_bits = SPI_CS_HIGH | SPI_NO_CS; > > > > This controller has no chip select but we provided a set_cs operation? > > Oops, I played around with those two options and was hoping SPI_NO_CS > would avoid the undesired set_cs invocations, but it didn't work as > expected and so I added the "if (enabled)" check above. > > Thanks for your review, > > Andreas > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ben, Am 02.07.2018 um 13:51 schrieb Ben Whitten: > Excellent work on doing this I have also been working on and off > this personally for some time. Thanks. Colliding work is always unfortunate, I can relate... > 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. Please note that my lora-next branch already has bug fixes and cleanups over this patch. The probe error handling was broken, and I implemented wrappers for paged reads and writes as well as burst modes, plus the firmware loading. https://github.com/afaerber/linux/commits/lora-next I took a quick look at your sx1257 and noticed you licensed it as GPLv2. Is there any particular reason for that? Since I wrote my driver without copying from GPLv2 code, I prefer the less restrictive GPLv2+. So far a work day has passed with no maintainer objecting to or commenting on the underlying PF_LORA network layer design. Meanwhile there's already three of us with code and more people have inquired about testing and contributing, so I'm thinking about setting up a staging tree on kernel.org to collaborate on... Would you be willing to contribute your regmap ideas to my driver as a patch to squash? Needs a Signed-off-by of course, which your GitHub commits are lacking, so I can't merge them on my own. > I have also been trying to use the clk framework to capture the various > routing that the cards have. I thought about clk too, but won't that cause name conflicts when probing multiple concentrators? Would be nice to use that for configuring the SX1257 clock output instead of my current hack. Another thought I haven't investigated yet is whether we could use remoteproc for ARB and AGC. I would at least prefer to have the firmware as a binary loaded via the usual request_firmware(), not as byte array. But then again the AGC gets firmware loaded twice, so maybe too complex for remoteproc. BTW do you have any insights on what MCU is in there? Would be nice to understand in form of source code what the firmware is doing, to avoid the hard dependency on a specific firmware version (imagine user updating kernel-firmware - containing versions X,Y,Z - and kernel and booting two different kernel versions, the older one stops working). https://www.thethingsnetwork.org/forum/t/secret-price-of-a-lora-gateway/5730/74 Regards, Andreas > I will dig into this series this evening. > > [1] https://github.com/BWhitten/linux-stable/tree/971aadc8fdfe842020d912449bdd71b33d576fe3/drivers/net/lora [...] >> 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 [snip]
Hi Ben, Am 02.07.2018 um 22:43 schrieb Ben Whitten: >> 2) This SPI device is in turn exposing the two SPI masters that you >> already found below, and I didn't see a sane way to split that code out >> into drivers/spi/, so it's in drivers/net/lora/ here - has there been >> any precedence either way? > > In my work in progress driver I just register one controller for the sx1301 with two chip selects and use the chip select information to choose the correct radio to send to, this is based on the DT reg information. No need to register two separate masters. I had considered that and discarded it. The SX1301 has not just two CS registers though but also two pairs of addr, data registers. That speaks for two masters with a single chip-select each, unless I'm misunderstanding the meaning of the registers. >> Am 02.07.2018 um 18:12 schrieb Mark Brown: >>> On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: >>> >>>> +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); >>>> +} >>> >>> So we never disable chip select? >> >> Not here, I instead did that in transfer_one below. >> >> Unfortunately there seems to be no documentation, only reference code: >> >> https://github.com/Lora- >> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L121 >> https://github.com/Lora- >> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L165 >> >> It sets CS to 0 before writing to address and data registers, then >> immediately sets CS to 1 and back to 0 before reading or ending the >> write transaction. I've tried to force the same behavior in this driver. >> My guess was that CS is high-active during the short 1-0 cycle, because >> if it's low-active during the register writes then why the heck is it >> set to 0 again in the end instead of keeping at 1... confusing. >> >> Maybe the Semtech folks CC'ed can comment how these registers work? >> >>>> + if (tx_buf) { >>>> + ret = sx1301_write(ssx->parent, ssx->regs + >> REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0); >>> >>> This looks confused. We're in an if (tx_buf) block but there's a use of >>> the ternery operator that appears to be checking if we have a tx_buf? >> >> Yeah, as mentioned this RFC is not ready for merging - checkpatch.pl >> will complain about lines too long, and TODOs are sprinkled all over or >> not even mentioned. It's a Proof of Concept that a net_device could work >> for a wide range of spi and serdev based drivers, and on top this device >> has more than one channel, which may influence network-level design >> discussions. >> >> That said, I'll happily drop the second check. Thanks for spotting! >> >>>> + 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; >>>> + } >>> >>> This looks awfully like you're coming in at the wrong abstraction layer >>> and the hardware actually implements a register abstraction rather than >>> a SPI one so you should be using regmap as the abstraction. >> >> I don't understand. Ben has suggested using regmap for the SPI _device_ >> that we're talking to, which may be a good idea. But this SX1301 device >> in turn has two SPI _masters_ talking to an SX125x slave each. I don't >> see how using regmap instead of my wrappers avoids this spi_controller? >> The whole point of this spi_controller is to abstract and separate the >> SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver, >> instead of mixing it into the SX1301 driver - to me that looks cleaner >> and more extensible. It also has the side-effect that we could configure >> the two radios via DT (frequencies, clk output, etc.). > > You want an SPI controller in the SX1301 as the down stream radios are SPI and could be attached directly to a host SPI bus, makes sense to have one radio driver and talk through the SX1301. > But you should use the regmap to access the SX1301 master controller registers. > Example I use with one SPI master and some clock info: > eg: > sx1301: sx1301@0 { Node names should not repeat the chipset, that goes into compatible. lora-concentrator@0? > compatible = "semtech,sx1301"; > reg = <0>; > #address-cells = <1>; > #size-cells = <0>; I would still find it cleaner to have (a) sub-node(s) for the radios. > spi-max-frequency = <8000000>; Datasheet says 10 MHz, why 8 MHz? > gpios-reset = <&pioA 26 GPIO_ACTIVE_HIGH>; reset-gpios? > clocks = <&radio1 0>, <&clkhs 0>; > clock-names = "clk32m", "clkhs"; > > radio0: sx1257@0 { lora@0? > compatible = "semtech,sx125x"; No wildcards in bindings please, use concrete "semtech,sx1257". > reg = <0>; > spi-max-frequency = <8000000>; Datasheet says 10 ns - I reported to Semtech that it should probably say 10 MHz, too. > tx; Might we configure that on the sx1301 instead? > clocks = <&tcxo 0>; > clock-names = "tcxo"; > }; > > radio1: sx1257@1 { > compatible = "semtech,sx125x"; > reg = <1>; > spi-max-frequency = <8000000>; > #clock-cells = <0>; > clocks = <&tcxo 0>; > clock-names = "tcxo"; > clock-output-names = "clk32m"; > }; > }; [snip] Regards, Andreas
On Mon, Jul 02, 2018 at 07:34:21PM +0200, Andreas Färber wrote: > Hi Mark, > > This driver is still evolving, there's newer code on my lora-next branch > already: https://github.com/afaerber/linux/commits/lora-next Please don't top post, reply in line with needed context. This allows readers to readily follow the flow of conversation and understand what you are talking about and also helps ensure that everything in the discussion is being addressed. > 2) This SPI device is in turn exposing the two SPI masters that you > already found below, and I didn't see a sane way to split that code out > into drivers/spi/, so it's in drivers/net/lora/ here - has there been > any precedence either way? A MFD? > Am 02.07.2018 um 18:12 schrieb Mark Brown: > > On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: > >> +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; > > So we never disable chip select? > Not here, I instead did that in transfer_one below. That's obviously at best going to be fragile, you're implementing half the operation here and half somewhere else which is most likely going to break at some point when the framework changes. > >> + 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; > >> + } > > This looks awfully like you're coming in at the wrong abstraction layer > > and the hardware actually implements a register abstraction rather than > > a SPI one so you should be using regmap as the abstraction. > I don't understand. Ben has suggested using regmap for the SPI _device_ > that we're talking to, which may be a good idea. But this SX1301 device > in turn has two SPI _masters_ talking to an SX125x slave each. I don't > see how using regmap instead of my wrappers avoids this spi_controller? It seems obvious from the code that this isn't actually interacting with a SPI controller, you're writing an address and a value to a register map rather than dealing with a byte stream. There may be a SPI bus somewhere behind some other hardware but you don't seem to be interacting with it as such. > The whole point of this spi_controller is to abstract and separate the > SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver, > instead of mixing it into the SX1301 driver - to me that looks cleaner > and more extensible. It also has the side-effect that we could configure > the two radios via DT (frequencies, clk output, etc.). A register map would work just as well here, we already have plenty of devices that abstract at this level (most obviously the I2C/SPI devices that use it to offer both interfaces with a single core driver).
Am 03.07.2018 um 16:50 schrieb Mark Brown: > On Mon, Jul 02, 2018 at 07:34:21PM +0200, Andreas Färber wrote: >> Hi Mark, >> >> This driver is still evolving, there's newer code on my lora-next branch >> already: https://github.com/afaerber/linux/commits/lora-next > > Please don't top post, reply in line with needed context. This allows > readers to readily follow the flow of conversation and understand what > you are talking about and also helps ensure that everything in the > discussion is being addressed. I did reply inline, if you cared to read on. ;) >> 2) This SPI device is in turn exposing the two SPI masters that you >> already found below, and I didn't see a sane way to split that code out >> into drivers/spi/, so it's in drivers/net/lora/ here - has there been >> any precedence either way? > > A MFD? I know of mfd, but how would the the the net vs. spi pieces interact then? Some functions would need to be exported then or is there an easier way without needing to set a cross-module API in stone? >> Am 02.07.2018 um 18:12 schrieb Mark Brown: >>> On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote: >>>> +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; > >>> So we never disable chip select? > >> Not here, I instead did that in transfer_one below. > > That's obviously at best going to be fragile, you're implementing half > the operation here and half somewhere else which is most likely going to > break at some point when the framework changes. > >>>> + 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; >>>> + } > >>> This looks awfully like you're coming in at the wrong abstraction layer >>> and the hardware actually implements a register abstraction rather than >>> a SPI one so you should be using regmap as the abstraction. > >> I don't understand. Ben has suggested using regmap for the SPI _device_ >> that we're talking to, which may be a good idea. But this SX1301 device >> in turn has two SPI _masters_ talking to an SX125x slave each. I don't >> see how using regmap instead of my wrappers avoids this spi_controller? > > It seems obvious from the code that this isn't actually interacting with > a SPI controller, you're writing an address and a value to a register > map rather than dealing with a byte stream. There may be a SPI bus > somewhere behind some other hardware but you don't seem to be > interacting with it as such. > >> The whole point of this spi_controller is to abstract and separate the >> SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver, >> instead of mixing it into the SX1301 driver - to me that looks cleaner >> and more extensible. It also has the side-effect that we could configure >> the two radios via DT (frequencies, clk output, etc.). > > A register map would work just as well here, we already have plenty of > devices that abstract at this level (most obviously the I2C/SPI devices > that use it to offer both interfaces with a single core driver). The address and data registers together form a two-byte SPI message! It is transmitted by writing to the CS register. The received data is afterwards available in another register. HTE, Andreas
On Tue, Jul 03, 2018 at 05:09:38PM +0200, Andreas Färber wrote: > Am 03.07.2018 um 16:50 schrieb Mark Brown: > >> 2) This SPI device is in turn exposing the two SPI masters that you > >> already found below, and I didn't see a sane way to split that code out > >> into drivers/spi/, so it's in drivers/net/lora/ here - has there been > >> any precedence either way? > > A MFD? > I know of mfd, but how would the the the net vs. spi pieces interact > then? Some functions would need to be exported then or is there an > easier way without needing to set a cross-module API in stone? It's an in-kernel ABI it's not exactly set in stone but yeah, you'll need some interface. A lot of devices work by having the children know that they're part of a MFD and fish things out of the parent device, either the pdata or (in the common case where the MFD bit mostly just instantiates subdevices and holds a regmap) with dev_get_regmap(). > > A register map would work just as well here, we already have plenty of > > devices that abstract at this level (most obviously the I2C/SPI devices > > that use it to offer both interfaces with a single core driver). > The address and data registers together form a two-byte SPI message! > It is transmitted by writing to the CS register. > The received data is afterwards available in another register. Right, but it seems from the code that the hardware understands that it's formatting register I/O and not just shifting in and out a byte stream which is what a SPI controller does. I'd not be surprised to learn that the register you're calling a chip select register is a strobe that initiates the transfer (and that this may be some of the difficulty you're having with handling it in the way the framework expects), the pattern with writing 1 followed immediately by 0 is a bit of a flag here. I've seen such before hardware where I know it was intentionally designed that way so it wouldn't be totally surprising.
Am 03.07.2018 um 17:31 schrieb Mark Brown: > On Tue, Jul 03, 2018 at 05:09:38PM +0200, Andreas Färber wrote: >> Am 03.07.2018 um 16:50 schrieb Mark Brown: >>> A register map would work just as well here, we already have plenty of >>> devices that abstract at this level (most obviously the I2C/SPI devices >>> that use it to offer both interfaces with a single core driver). > >> The address and data registers together form a two-byte SPI message! > >> It is transmitted by writing to the CS register. > >> The received data is afterwards available in another register. > > Right, but it seems from the code that the hardware understands that > it's formatting register I/O and not just shifting in and out a byte > stream which is what a SPI controller does. I'd not be surprised to > learn that the register you're calling a chip select register is a > strobe that initiates the transfer (and that this may be some of the > difficulty you're having with handling it in the way the framework > expects), the pattern with writing 1 followed immediately by 0 is a bit > of a flag here. Yeah, the current implementation assumes exactly that. :) > I've seen such before hardware where I know it was intentionally > designed that way so it wouldn't be totally surprising. If we don't implement a spi_controller here, then IIUC we can't have multiple spi_device implementations for the devices on the receiving end, as they rely on a spi_controller for their APIs. Do you have an alternative solution for abstraction? A regmap would seem to require putting everything into a monolithic SX1301 driver despite those connected chipsets actually being regular, external SPI chips that could also be attached to non-SX1301 SPI masters. Regards, Andreas
On Tue, Jul 03, 2018 at 06:40:41PM +0200, Andreas Färber wrote: > Do you have an alternative solution for abstraction? A regmap would seem > to require putting everything into a monolithic SX1301 driver despite > those connected chipsets actually being regular, external SPI chips that > could also be attached to non-SX1301 SPI masters. I'm suggesting a regmap for those external SPI chips. It doesn't matter what the host uses.
> Subject: Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301 > > On Tue, Jul 03, 2018 at 06:40:41PM +0200, Andreas Färber wrote: > > > Do you have an alternative solution for abstraction? A regmap would seem > > to require putting everything into a monolithic SX1301 driver despite > > those connected chipsets actually being regular, external SPI chips that > > could also be attached to non-SX1301 SPI masters. > > I'm suggesting a regmap for those external SPI chips. It doesn't matter > what the host uses. Currently in my testing drivers I have used regmap for both the SX1301 and the downstream SX1257. In my SX1257 driver currently the regmap is backed via SPI with devm_regmap_init_spi, please correct me if I am wrong but if I understand correctly this could be split out to regmap backed by SPI in the case of a direct host connection, and a regmap backed by the SX1301's regmapped strobing register interface, regmap_bus? Is there a precedent to this I can examine? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 04, 2018 at 01:41:42PM +0000, Ben Whitten wrote: Please fix your mail client to word wrap within paragraphs at something substantially less than 80 columns. Doing this makes your messages much easier to read and reply to. > In my SX1257 driver currently the regmap is backed via SPI with > devm_regmap_init_spi, please correct me if I am wrong but if I > understand correctly this could be split out to regmap backed by SPI > in the case of a direct host connection, and a regmap backed by the > SX1301's regmapped strobing register interface, regmap_bus? Yes. > Is there a precedent to this I can examine? There's lots of devices that provide both SPI and I2C, you'd just be using a regmap_bus instead of the I2C one. I can't recall an example of the specific combination you're looking for though.
PiBTdWJqZWN0OiBSZTogW1JGQyBuZXQtbmV4dCAxNS8xNV0gbmV0OiBsb3JhOiBBZGQgU2VtdGVj aCBTWDEzMDENCj4gDQo+IEhpIEJlbiwNCj4gDQo+IEFtIDAyLjA3LjIwMTggdW0gMjI6NDMgc2No cmllYiBCZW4gV2hpdHRlbjoNCj4gPj4gMikgVGhpcyBTUEkgZGV2aWNlIGlzIGluIHR1cm4gZXhw b3NpbmcgdGhlIHR3byBTUEkgbWFzdGVycyB0aGF0IHlvdQ0KPiA+PiBhbHJlYWR5IGZvdW5kIGJl bG93LCBhbmQgSSBkaWRuJ3Qgc2VlIGEgc2FuZSB3YXkgdG8gc3BsaXQgdGhhdCBjb2RlIG91dA0K PiA+PiBpbnRvIGRyaXZlcnMvc3BpLywgc28gaXQncyBpbiBkcml2ZXJzL25ldC9sb3JhLyBoZXJl IC0gaGFzIHRoZXJlIGJlZW4NCj4gPj4gYW55IHByZWNlZGVuY2UgZWl0aGVyIHdheT8NCj4gPg0K PiA+IEluIG15IHdvcmsgaW4gcHJvZ3Jlc3MgZHJpdmVyIEkganVzdCByZWdpc3RlciBvbmUgY29u dHJvbGxlciBmb3IgdGhlIHN4MTMwMQ0KPiB3aXRoIHR3byBjaGlwIHNlbGVjdHMgYW5kIHVzZSB0 aGUgY2hpcCBzZWxlY3QgaW5mb3JtYXRpb24gdG8gY2hvb3NlIHRoZQ0KPiBjb3JyZWN0IHJhZGlv IHRvIHNlbmQgdG8sIHRoaXMgaXMgYmFzZWQgb24gdGhlIERUIHJlZyBpbmZvcm1hdGlvbi4gTm8g bmVlZCB0bw0KPiByZWdpc3RlciB0d28gc2VwYXJhdGUgbWFzdGVycy4NCj4gDQo+IEkgaGFkIGNv bnNpZGVyZWQgdGhhdCBhbmQgZGlzY2FyZGVkIGl0LiBUaGUgU1gxMzAxIGhhcyBub3QganVzdCB0 d28gQ1MNCj4gcmVnaXN0ZXJzIHRob3VnaCBidXQgYWxzbyB0d28gcGFpcnMgb2YgYWRkciwgZGF0 YSByZWdpc3RlcnMuIFRoYXQgc3BlYWtzDQo+IGZvciB0d28gbWFzdGVycyB3aXRoIGEgc2luZ2xl IGNoaXAtc2VsZWN0IGVhY2gsIHVubGVzcyBJJ20NCj4gbWlzdW5kZXJzdGFuZGluZyB0aGUgbWVh bmluZyBvZiB0aGUgcmVnaXN0ZXJzLg0KDQpCYXNlZCBvbiBNYXJrcyBzdWdnZXN0aW9uIEkgYW0g ZXhwZXJpbWVudGluZyB3aXRoIHVzaW5nIHRoZSBTWDEzMDEgdG8gZXhwb3NlIGEgcmVnbWFwX2J1 cyB0aGF0IHRoZSB1bmRlcmx5aW5nIFNYMTI1NyBjYW4gYXR0YWNoIHRvLCBzbyB0aGUgcmFkaW8g aGFzIGEgY29yZSBjb21wb25lbnQsIGEgU1BJIGNvbXBvbmVudCBmb3IgZGlyZWN0IGNvbm5lY3Rp b24gYW5kIHRoaXMgY29uY2VudHJhdG9yIGNvbm5lY3Rpb24gY29tcG9uZW50LiANCg0KPiA+PiBB bSAwMi4wNy4yMDE4IHVtIDE4OjEyIHNjaHJpZWIgTWFyayBCcm93bjoNCj4gPj4+IE9uIFN1biwg SnVsIDAxLCAyMDE4IGF0IDAxOjA4OjA0UE0gKzAyMDAsIEFuZHJlYXMgRsOkcmJlciB3cm90ZToN Cj4gPj4+DQo+ID4+Pj4gK3N0YXRpYyB2b2lkIHN4MTMwMV9yYWRpb19zcGlfc2V0X2NzKHN0cnVj dCBzcGlfZGV2aWNlICpzcGksIGJvb2wNCj4gZW5hYmxlKQ0KPiA+Pj4+ICt7DQo+ID4+Pj4gKwlp bnQgcmV0Ow0KPiA+Pj4+ICsNCj4gPj4+PiArCWRldl9kYmcoJnNwaS0+ZGV2LCAic2V0dGluZyBT UEkgQ1MgdG8gJXNcbiIsIGVuYWJsZSA/ICIxIiA6ICIwIik7DQo+ID4+Pj4gKw0KPiA+Pj4+ICsJ aWYgKGVuYWJsZSkNCj4gPj4+PiArCQlyZXR1cm47DQo+ID4+Pj4gKw0KPiA+Pj4+ICsJcmV0ID0g c3gxMzAxX3JhZGlvX3NldF9jcyhzcGktPmNvbnRyb2xsZXIsIGVuYWJsZSk7DQo+ID4+Pj4gKwlp ZiAocmV0KQ0KPiA+Pj4+ICsJCWRldl93YXJuKCZzcGktPmRldiwgImZhaWxlZCB0byB3cml0ZSBD UyAoJWQpXG4iLCByZXQpOw0KPiA+Pj4+ICt9DQo+ID4+Pg0KPiA+Pj4gU28gd2UgbmV2ZXIgZGlz YWJsZSBjaGlwIHNlbGVjdD8NCj4gPj4NCj4gPj4gTm90IGhlcmUsIEkgaW5zdGVhZCBkaWQgdGhh dCBpbiB0cmFuc2Zlcl9vbmUgYmVsb3cuDQo+ID4+DQo+ID4+IFVuZm9ydHVuYXRlbHkgdGhlcmUg c2VlbXMgdG8gYmUgbm8gZG9jdW1lbnRhdGlvbiwgb25seSByZWZlcmVuY2UNCj4gY29kZToNCj4g Pj4NCj4gPj4gaHR0cHM6Ly9naXRodWIuY29tL0xvcmEtDQo+ID4+IG5ldC9sb3JhX2dhdGV3YXkv YmxvYi9tYXN0ZXIvbGlibG9yYWd3L3NyYy9sb3JhZ3dfcmFkaW8uYyNMMTIxDQo+ID4+IGh0dHBz Oi8vZ2l0aHViLmNvbS9Mb3JhLQ0KPiA+PiBuZXQvbG9yYV9nYXRld2F5L2Jsb2IvbWFzdGVyL2xp YmxvcmFndy9zcmMvbG9yYWd3X3JhZGlvLmMjTDE2NQ0KPiA+Pg0KPiA+PiBJdCBzZXRzIENTIHRv IDAgYmVmb3JlIHdyaXRpbmcgdG8gYWRkcmVzcyBhbmQgZGF0YSByZWdpc3RlcnMsIHRoZW4NCj4g Pj4gaW1tZWRpYXRlbHkgc2V0cyBDUyB0byAxIGFuZCBiYWNrIHRvIDAgYmVmb3JlIHJlYWRpbmcg b3IgZW5kaW5nIHRoZQ0KPiA+PiB3cml0ZSB0cmFuc2FjdGlvbi4gSSd2ZSB0cmllZCB0byBmb3Jj ZSB0aGUgc2FtZSBiZWhhdmlvciBpbiB0aGlzIGRyaXZlci4NCj4gPj4gTXkgZ3Vlc3Mgd2FzIHRo YXQgQ1MgaXMgaGlnaC1hY3RpdmUgZHVyaW5nIHRoZSBzaG9ydCAxLTAgY3ljbGUsIGJlY2F1c2UN Cj4gPj4gaWYgaXQncyBsb3ctYWN0aXZlIGR1cmluZyB0aGUgcmVnaXN0ZXIgd3JpdGVzIHRoZW4g d2h5IHRoZSBoZWNrIGlzIGl0DQo+ID4+IHNldCB0byAwIGFnYWluIGluIHRoZSBlbmQgaW5zdGVh ZCBvZiBrZWVwaW5nIGF0IDEuLi4gY29uZnVzaW5nLg0KPiA+Pg0KPiA+PiBNYXliZSB0aGUgU2Vt dGVjaCBmb2xrcyBDQydlZCBjYW4gY29tbWVudCBob3cgdGhlc2UgcmVnaXN0ZXJzIHdvcms/DQo+ ID4+DQo+ID4+Pj4gKwlpZiAodHhfYnVmKSB7DQo+ID4+Pj4gKwkJcmV0ID0gc3gxMzAxX3dyaXRl KHNzeC0+cGFyZW50LCBzc3gtPnJlZ3MgKw0KPiA+PiBSRUdfUkFESU9fWF9BRERSLCB0eF9idWYg PyB0eF9idWZbMF0gOiAwKTsNCj4gPj4+DQo+ID4+PiBUaGlzIGxvb2tzIGNvbmZ1c2VkLiAgV2Un cmUgaW4gYW4gaWYgKHR4X2J1ZikgYmxvY2sgYnV0IHRoZXJlJ3MgYSB1c2Ugb2YNCj4gPj4+IHRo ZSB0ZXJuZXJ5IG9wZXJhdG9yIHRoYXQgYXBwZWFycyB0byBiZSBjaGVja2luZyBpZiB3ZSBoYXZl IGEgdHhfYnVmPw0KPiA+Pg0KPiA+PiBZZWFoLCBhcyBtZW50aW9uZWQgdGhpcyBSRkMgaXMgbm90 IHJlYWR5IGZvciBtZXJnaW5nIC0gY2hlY2twYXRjaC5wbA0KPiA+PiB3aWxsIGNvbXBsYWluIGFi b3V0IGxpbmVzIHRvbyBsb25nLCBhbmQgVE9ET3MgYXJlIHNwcmlua2xlZCBhbGwgb3ZlciBvcg0K PiA+PiBub3QgZXZlbiBtZW50aW9uZWQuIEl0J3MgYSBQcm9vZiBvZiBDb25jZXB0IHRoYXQgYSBu ZXRfZGV2aWNlIGNvdWxkIHdvcmsNCj4gPj4gZm9yIGEgd2lkZSByYW5nZSBvZiBzcGkgYW5kIHNl cmRldiBiYXNlZCBkcml2ZXJzLCBhbmQgb24gdG9wIHRoaXMgZGV2aWNlDQo+ID4+IGhhcyBtb3Jl IHRoYW4gb25lIGNoYW5uZWwsIHdoaWNoIG1heSBpbmZsdWVuY2UgbmV0d29yay1sZXZlbCBkZXNp Z24NCj4gPj4gZGlzY3Vzc2lvbnMuDQo+ID4+DQo+ID4+IFRoYXQgc2FpZCwgSSdsbCBoYXBwaWx5 IGRyb3AgdGhlIHNlY29uZCBjaGVjay4gVGhhbmtzIGZvciBzcG90dGluZyENCj4gPj4NCj4gPj4+ PiArCQlpZiAocmV0KSB7DQo+ID4+Pj4gKwkJCWRldl9lcnIoJnNwaS0+ZGV2LCAiU1BJIHJhZGlv IGFkZHJlc3Mgd3JpdGUNCj4gPj4gZmFpbGVkXG4iKTsNCj4gPj4+PiArCQkJcmV0dXJuIHJldDsN Cj4gPj4+PiArCQl9DQo+ID4+Pj4gKw0KPiA+Pj4+ICsJCXJldCA9IHN4MTMwMV93cml0ZShzc3gt PnBhcmVudCwgc3N4LT5yZWdzICsNCj4gPj4gUkVHX1JBRElPX1hfREFUQSwgKHR4X2J1ZiAmJiB4 ZnItPmxlbiA+PSAyKSA/IHR4X2J1ZlsxXSA6IDApOw0KPiA+Pj4+ICsJCWlmIChyZXQpIHsNCj4g Pj4+PiArCQkJZGV2X2Vycigmc3BpLT5kZXYsICJTUEkgcmFkaW8gZGF0YSB3cml0ZSBmYWlsZWRc biIpOw0KPiA+Pj4+ICsJCQlyZXR1cm4gcmV0Ow0KPiA+Pj4+ICsJCX0NCj4gPj4+DQo+ID4+PiBU aGlzIGxvb2tzIGF3ZnVsbHkgbGlrZSB5b3UncmUgY29taW5nIGluIGF0IHRoZSB3cm9uZyBhYnN0 cmFjdGlvbiBsYXllcg0KPiA+Pj4gYW5kIHRoZSBoYXJkd2FyZSBhY3R1YWxseSBpbXBsZW1lbnRz IGEgcmVnaXN0ZXIgYWJzdHJhY3Rpb24gcmF0aGVyIHRoYW4NCj4gPj4+IGEgU1BJIG9uZSBzbyB5 b3Ugc2hvdWxkIGJlIHVzaW5nIHJlZ21hcCBhcyB0aGUgYWJzdHJhY3Rpb24uDQo+ID4+DQo+ID4+ IEkgZG9uJ3QgdW5kZXJzdGFuZC4gQmVuIGhhcyBzdWdnZXN0ZWQgdXNpbmcgcmVnbWFwIGZvciB0 aGUgU1BJIF9kZXZpY2VfDQo+ID4+IHRoYXQgd2UncmUgdGFsa2luZyB0bywgd2hpY2ggbWF5IGJl IGEgZ29vZCBpZGVhLiBCdXQgdGhpcyBTWDEzMDEgZGV2aWNlDQo+ID4+IGluIHR1cm4gaGFzIHR3 byBTUEkgX21hc3RlcnNfIHRhbGtpbmcgdG8gYW4gU1gxMjV4IHNsYXZlIGVhY2guIEkgZG9uJ3QN Cj4gPj4gc2VlIGhvdyB1c2luZyByZWdtYXAgaW5zdGVhZCBvZiBteSB3cmFwcGVycyBhdm9pZHMg dGhpcyBzcGlfY29udHJvbGxlcj8NCj4gPj4gVGhlIHdob2xlIHBvaW50IG9mIHRoaXMgc3BpX2Nv bnRyb2xsZXIgaXMgdG8gYWJzdHJhY3QgYW5kIHNlcGFyYXRlIHRoZQ0KPiA+PiBTWDEyNTUgdnMu IFNYMTI1NyB2cy4gd2hhdGV2ZXItcmFkaW8tYXR0YWNoZWQgaW50byBhIHNlcGFyYXRlIGRyaXZl ciwNCj4gPj4gaW5zdGVhZCBvZiBtaXhpbmcgaXQgaW50byB0aGUgU1gxMzAxIGRyaXZlciAtIHRv IG1lIHRoYXQgbG9va3MgY2xlYW5lcg0KPiA+PiBhbmQgbW9yZSBleHRlbnNpYmxlLiBJdCBhbHNv IGhhcyB0aGUgc2lkZS1lZmZlY3QgdGhhdCB3ZSBjb3VsZCBjb25maWd1cmUNCj4gPj4gdGhlIHR3 byByYWRpb3MgdmlhIERUIChmcmVxdWVuY2llcywgY2xrIG91dHB1dCwgZXRjLikuDQo+ID4NCj4g PiBZb3Ugd2FudCBhbiBTUEkgY29udHJvbGxlciBpbiB0aGUgU1gxMzAxIGFzIHRoZSBkb3duIHN0 cmVhbSByYWRpb3MgYXJlIFNQSQ0KPiBhbmQgY291bGQgYmUgYXR0YWNoZWQgZGlyZWN0bHkgdG8g YSBob3N0IFNQSSBidXMsIG1ha2VzIHNlbnNlIHRvIGhhdmUgb25lDQo+IHJhZGlvIGRyaXZlciBh bmQgdGFsayB0aHJvdWdoIHRoZSBTWDEzMDEuDQo+ID4gQnV0IHlvdSBzaG91bGQgdXNlIHRoZSBy ZWdtYXAgdG8gYWNjZXNzIHRoZSBTWDEzMDEgbWFzdGVyIGNvbnRyb2xsZXINCj4gcmVnaXN0ZXJz Lg0KPiA+IEV4YW1wbGUgSSB1c2Ugd2l0aCBvbmUgU1BJIG1hc3RlciBhbmQgc29tZSBjbG9jayBp bmZvOg0KPiA+IGVnOg0KPiA+IAlzeDEzMDE6IHN4MTMwMUAwIHsNCj4gDQo+IE5vZGUgbmFtZXMg c2hvdWxkIG5vdCByZXBlYXQgdGhlIGNoaXBzZXQsIHRoYXQgZ29lcyBpbnRvIGNvbXBhdGlibGUu DQo+IA0KPiBsb3JhLWNvbmNlbnRyYXRvckAwPw0KPiANClN1cmUNCg0KPiA+IAkJY29tcGF0aWJs ZSA9ICJzZW10ZWNoLHN4MTMwMSI7DQo+ID4gCQlyZWcgPSA8MD47DQo+ID4gCQkjYWRkcmVzcy1j ZWxscyA9IDwxPjsNCj4gPiAJCSNzaXplLWNlbGxzID0gPDA+Ow0KPiANCj4gSSB3b3VsZCBzdGls bCBmaW5kIGl0IGNsZWFuZXIgdG8gaGF2ZSAoYSkgc3ViLW5vZGUocykgZm9yIHRoZSByYWRpb3Mu DQpIb3cgZG8geW91IG1lYW4/DQoNCj4gPiAJCXNwaS1tYXgtZnJlcXVlbmN5ID0gPDgwMDAwMDA+ Ow0KPiANCj4gRGF0YXNoZWV0IHNheXMgMTAgTUh6LCB3aHkgOCBNSHo/DQo+IA0KPiA+IAkJZ3Bp b3MtcmVzZXQgPSA8JnBpb0EgMjYgR1BJT19BQ1RJVkVfSElHSD47DQo+IA0KPiByZXNldC1ncGlv cz8NCkFncmVlZCwgdGhpcyBzZWVtcyBtb3JlIGNvbW1vbi4NCg0KPiA+IAkJY2xvY2tzID0gPCZy YWRpbzEgMD4sIDwmY2xraHMgMD47DQo+ID4gCQljbG9jay1uYW1lcyA9ICJjbGszMm0iLCAiY2xr aHMiOw0KPiA+DQo+ID4gCQlyYWRpbzA6IHN4MTI1N0AwIHsNCj4gDQo+IGxvcmFAMD8NCj4gDQo+ ID4gCQkJY29tcGF0aWJsZSA9ICJzZW10ZWNoLHN4MTI1eCI7DQo+IA0KPiBObyB3aWxkY2FyZHMg aW4gYmluZGluZ3MgcGxlYXNlLCB1c2UgY29uY3JldGUgInNlbXRlY2gsc3gxMjU3Ii4NClN1cmUN Cg0KPiA+IAkJCXJlZyA9IDwwPjsNCj4gPiAJCQlzcGktbWF4LWZyZXF1ZW5jeSA9IDw4MDAwMDAw PjsNCj4gDQo+IERhdGFzaGVldCBzYXlzIDEwIG5zIC0gSSByZXBvcnRlZCB0byBTZW10ZWNoIHRo YXQgaXQgc2hvdWxkIHByb2JhYmx5IHNheQ0KPiAxMCBNSHosIHRvby4NCj4gDQo+ID4gCQkJdHg7 DQo+IA0KPiBNaWdodCB3ZSBjb25maWd1cmUgdGhhdCBvbiB0aGUgc3gxMzAxIGluc3RlYWQ/DQoN CldlbGwgdGhlIGFiaWxpdHkgZm9yIGEgcmFkaW8gdG8gVFggaXMgYSByYWRpbyBwcm9wZXJ0eSBy ZWFsbHkuIEl0IGRlcGVuZHMgb24gdGhlIGJvYXJkIHdoaWNoIGNoYWluIGhhcyB0aGUgUEFzIG9u LiBJIGRvbuKAmXQgdGhpbmsgdGhhdCBpdHMgYXBwcm9wcmlhdGUgdG8gY29uZmlndXJlIHRoaXMg YXQgdGhlIGNvbmNlbnRyYXRvciwgaXQgY2FuIGluc3RlYWQgZGlzY292ZXIgdGhpcyBmcm9tIHRo ZSByYWRpb3MuDQoNCj4gPiAJCQljbG9ja3MgPSA8JnRjeG8gMD47DQo+ID4gCQkJY2xvY2stbmFt ZXMgPSAidGN4byI7DQo+ID4gCQl9Ow0KPiA+DQo+ID4gCQlyYWRpbzE6IHN4MTI1N0AxIHsNCj4g PiAJCQljb21wYXRpYmxlID0gInNlbXRlY2gsc3gxMjV4IjsNCj4gPiAJCQlyZWcgPSA8MT47DQo+ ID4gCQkJc3BpLW1heC1mcmVxdWVuY3kgPSA8ODAwMDAwMD47DQo+ID4gCQkJI2Nsb2NrLWNlbGxz ID0gPDA+Ow0KPiA+IAkJCWNsb2NrcyA9IDwmdGN4byAwPjsNCj4gPiAJCQljbG9jay1uYW1lcyA9 ICJ0Y3hvIjsNCj4gPiAJCQljbG9jay1vdXRwdXQtbmFtZXMgPSAiY2xrMzJtIjsNCj4gPiAJCX07 DQo+ID4gfTsNCj4gW3NuaXBdDQo+IA0KPiBSZWdhcmRzLA0KPiBBbmRyZWFzDQo+IA0KPiAtLQ0K PiBTVVNFIExpbnV4IEdtYkgsIE1heGZlbGRzdHIuIDUsIDkwNDA5IE7DvHJuYmVyZywgR2VybWFu eQ0KPiBHRjogRmVsaXggSW1lbmTDtnJmZmVyLCBKYW5lIFNtaXRoYXJkLCBHcmFoYW0gTm9ydG9u DQo+IEhSQiAyMTI4NCAoQUcgTsO8cm5iZXJnKQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiBTdWJqZWN0OiBSZTogW1JGQyBuZXQtbmV4dCAxNS8xNV0gbmV0OiBsb3JhOiBBZGQgU2VtdGVj aCBTWDEzMDENCj4gDQo+IEhpIEJlbiwNCj4gDQo+IEFtIDAyLjA3LjIwMTggdW0gMTM6NTEgc2No cmllYiBCZW4gV2hpdHRlbjoNCj4gPiBFeGNlbGxlbnQgd29yayBvbiBkb2luZyB0aGlzIEkgaGF2 ZSBhbHNvIGJlZW4gd29ya2luZyBvbiBhbmQgb2ZmDQo+ID4gdGhpcyBwZXJzb25hbGx5IGZvciBz b21lIHRpbWUuDQo+IA0KPiBUaGFua3MuIENvbGxpZGluZyB3b3JrIGlzIGFsd2F5cyB1bmZvcnR1 bmF0ZSwgSSBjYW4gcmVsYXRlLi4uDQo+IA0KPiA+IEhhdmUgYSBsb29rIGF0IG15IHJlcG9zaXRv cnkgWzFdIGZvciBzeDEzMDEgYW5kIHN4MTI1NyBkcml2ZXJzLA0KPiA+IEkgdXNlIHJlZ21hcHMg Y2FwYWJpbGl0eSBvZiBzd2l0Y2hpbmcgcGFnZXMgd2hpY2ggc2hvdWxkIHNpbXBsaWZ5DQo+ID4g eW91ciBkcml2ZXIgY29uc2lkZXJhYmx5LCBJIGFsc28gaGF2ZSBhIGZ1bGwgcmVnaXN0ZXIgbWFw IGFuZCBiaXQgZmllbGQuDQo+IA0KPiBQbGVhc2Ugbm90ZSB0aGF0IG15IGxvcmEtbmV4dCBicmFu Y2ggYWxyZWFkeSBoYXMgYnVnIGZpeGVzIGFuZCBjbGVhbnVwcw0KPiBvdmVyIHRoaXMgcGF0Y2gu IFRoZSBwcm9iZSBlcnJvciBoYW5kbGluZyB3YXMgYnJva2VuLCBhbmQgSSBpbXBsZW1lbnRlZA0K PiB3cmFwcGVycyBmb3IgcGFnZWQgcmVhZHMgYW5kIHdyaXRlcyBhcyB3ZWxsIGFzIGJ1cnN0IG1v ZGVzLCBwbHVzIHRoZQ0KPiBmaXJtd2FyZSBsb2FkaW5nLg0KPiANCj4gaHR0cHM6Ly9naXRodWIu Y29tL2FmYWVyYmVyL2xpbnV4L2NvbW1pdHMvbG9yYS1uZXh0DQo+IA0KPiBJIHRvb2sgYSBxdWlj ayBsb29rIGF0IHlvdXIgc3gxMjU3IGFuZCBub3RpY2VkIHlvdSBsaWNlbnNlZCBpdCBhcyBHUEx2 Mi4NCj4gSXMgdGhlcmUgYW55IHBhcnRpY3VsYXIgcmVhc29uIGZvciB0aGF0PyBTaW5jZSBJIHdy b3RlIG15IGRyaXZlciB3aXRob3V0DQo+IGNvcHlpbmcgZnJvbSBHUEx2MiBjb2RlLCBJIHByZWZl ciB0aGUgbGVzcyByZXN0cmljdGl2ZSBHUEx2MisuDQoNCkFzIEkgd2FzIGxlYXJuaW5nIGFib3V0 IHJlZ21hcCB1c2FnZSBhbmQgaG93IFNQSSBkZXZpY2VzIGNvbm5lY3QgdG8gdGhlIGJ1cyBJIGFk b3B0ZWQgdGhlIG1vc3QgY29tbW9uIGxpY2VuY2UgZW1wbG95ZWQgaW4gdGhlc2Uga2VybmVsIGZp bGVzLCBzZWVtZWQgYXBwcm9wcmlhdGUuDQpVbHRpbWF0ZWx5IEkgZG9u4oCZdCB3YW50IGEgbGlj ZW5jZSBjaG9pY2UgdG8gaGFtcGVyIGluY2x1c2lvbiB0byBtYWlubGluZSwgbm90IGEgbGF3eWVy IEkgZG9u4oCZdCBmaW5kIHRoYXQgZnVuLg0KDQo+IA0KPiBTbyBmYXIgYSB3b3JrIGRheSBoYXMg cGFzc2VkIHdpdGggbm8gbWFpbnRhaW5lciBvYmplY3RpbmcgdG8gb3INCj4gY29tbWVudGluZyBv biB0aGUgdW5kZXJseWluZyBQRl9MT1JBIG5ldHdvcmsgbGF5ZXIgZGVzaWduLiBNZWFud2hpbGUN Cj4gdGhlcmUncyBhbHJlYWR5IHRocmVlIG9mIHVzIHdpdGggY29kZSBhbmQgbW9yZSBwZW9wbGUg aGF2ZSBpbnF1aXJlZA0KPiBhYm91dCB0ZXN0aW5nIGFuZCBjb250cmlidXRpbmcsIHNvIEknbSB0 aGlua2luZyBhYm91dCBzZXR0aW5nIHVwIGENCj4gc3RhZ2luZyB0cmVlIG9uIGtlcm5lbC5vcmcg dG8gY29sbGFib3JhdGUgb24uLi4NCj4gDQo+IFdvdWxkIHlvdSBiZSB3aWxsaW5nIHRvIGNvbnRy aWJ1dGUgeW91ciByZWdtYXAgaWRlYXMgdG8gbXkgZHJpdmVyIGFzIGENCj4gcGF0Y2ggdG8gc3F1 YXNoPyBOZWVkcyBhIFNpZ25lZC1vZmYtYnkgb2YgY291cnNlLCB3aGljaCB5b3VyIEdpdEh1Yg0K PiBjb21taXRzIGFyZSBsYWNraW5nLCBzbyBJIGNhbid0IG1lcmdlIHRoZW0gb24gbXkgb3duLg0K DQpJJ20gc3VyZSB3aGljaGV2ZXIgd2F5IHdlIG1lcmdlIGl0LCB5b3VycyB0byBtaW5lIC8gbWlu ZSB0byB5b3Vycywgd2UgY2FuIHdvcmsgdG9nZXRoZXIgb24gaXQuDQpBcyBJIGFscmVhZHkgaGF2 ZSByZWdtYXAgcnVubmluZyBhbmQgbW9zdGx5IHNwbGl0IG91dCBTWDEyNTcgZHJpdmVycyB0byBN YXJrcyBzdWdnZXN0aW9ucyBhdm9pZGluZyB0aGUgZXh0cmEgU1BJIGxheWVyIGl0IG1heSBieSBl YXNpZXIgdG8gcG9ydCB0aGUgdmFyaW91cyBmdW5jdGlvbnMgeW91IGhhdmUgdG8gbWluZSBhbmQg c2hhcmUgYXV0aG9yc2hpcC4NCg0KPiA+IEkgaGF2ZSBhbHNvIGJlZW4gdHJ5aW5nIHRvIHVzZSB0 aGUgY2xrIGZyYW1ld29yayB0byBjYXB0dXJlIHRoZSB2YXJpb3VzDQo+ID4gcm91dGluZyB0aGF0 IHRoZSBjYXJkcyBoYXZlLg0KPiANCj4gSSB0aG91Z2h0IGFib3V0IGNsayB0b28sIGJ1dCB3b24n dCB0aGF0IGNhdXNlIG5hbWUgY29uZmxpY3RzIHdoZW4NCj4gcHJvYmluZyBtdWx0aXBsZSBjb25j ZW50cmF0b3JzPyBXb3VsZCBiZSBuaWNlIHRvIHVzZSB0aGF0IGZvcg0KPiBjb25maWd1cmluZyB0 aGUgU1gxMjU3IGNsb2NrIG91dHB1dCBpbnN0ZWFkIG9mIG15IGN1cnJlbnQgaGFjay4NCg0KUG90 ZW50aWFsbHksIGl0IG1heWJlIHRoYXQgd2UgbmVlZCB0byBhdWdtZW50IHRoZSBuYW1lcyBvZiB0 aGUgY2xrIHdpdGggdGhlIENTIG9mIHRoZSBjb25jZW50cmF0b3IgaW4gdXNlLg0KT3IgbWF5YmUg dGhlIHJhZGlvIG9ubHkgc2VhcmNoZXMgdGhlIGNsaydzIG9mIGl0cyBwYXJlbnQgZGV2aWNlLi4u DQpJJ3ZlIG5vdCBleHBsb3JlZCB0aGlzIGFyZWEgdG8gZmFyIHlldCBidXQgdGhlcmUgd2lsbCBi ZSBhIHNvbHV0aW9uIEknbSBzdXJlLg0KDQo+IEFub3RoZXIgdGhvdWdodCBJIGhhdmVuJ3QgaW52 ZXN0aWdhdGVkIHlldCBpcyB3aGV0aGVyIHdlIGNvdWxkIHVzZQ0KPiByZW1vdGVwcm9jIGZvciBB UkIgYW5kIEFHQy4gSSB3b3VsZCBhdCBsZWFzdCBwcmVmZXIgdG8gaGF2ZSB0aGUgZmlybXdhcmUN Cj4gYXMgYSBiaW5hcnkgbG9hZGVkIHZpYSB0aGUgdXN1YWwgcmVxdWVzdF9maXJtd2FyZSgpLCBu b3QgYXMgYnl0ZSBhcnJheS4NCj4gQnV0IHRoZW4gYWdhaW4gdGhlIEFHQyBnZXRzIGZpcm13YXJl IGxvYWRlZCB0d2ljZSwgc28gbWF5YmUgdG9vIGNvbXBsZXgNCj4gZm9yIHJlbW90ZXByb2MuDQo+ IA0KPiBCVFcgZG8geW91IGhhdmUgYW55IGluc2lnaHRzIG9uIHdoYXQgTUNVIGlzIGluIHRoZXJl PyBXb3VsZCBiZSBuaWNlIHRvDQo+IHVuZGVyc3RhbmQgaW4gZm9ybSBvZiBzb3VyY2UgY29kZSB3 aGF0IHRoZSBmaXJtd2FyZSBpcyBkb2luZywgdG8gYXZvaWQNCj4gdGhlIGhhcmQgZGVwZW5kZW5j eSBvbiBhIHNwZWNpZmljIGZpcm13YXJlIHZlcnNpb24gKGltYWdpbmUgdXNlcg0KPiB1cGRhdGlu ZyBrZXJuZWwtZmlybXdhcmUgLSBjb250YWluaW5nIHZlcnNpb25zIFgsWSxaIC0gYW5kIGtlcm5l bCBhbmQNCj4gYm9vdGluZyB0d28gZGlmZmVyZW50IGtlcm5lbCB2ZXJzaW9ucywgdGhlIG9sZGVy IG9uZSBzdG9wcyB3b3JraW5nKS4NCg0KSSdtIGFmcmFpZCBubyBpbnNpZ2h0IGludG8gdGhlIGlu dGVybmFscyBvZiB0aGlzIGNoaXAsIHNvbWUgZ29vZCBndWVzc3dvcmsgYnV0IEkgdGhpbmsgYSBm aXJtd2FyZSBibG9iIGlzIHNvbWV0aGluZyB3ZSBhcmUgZ29pbmcgdG8gaGF2ZSB0byBsaXZlIHdp dGggZm9yIHRoZSB0aW1lIGJlaW5nLg0KDQo+IGh0dHBzOi8vd3d3LnRoZXRoaW5nc25ldHdvcmsu b3JnL2ZvcnVtL3Qvc2VjcmV0LXByaWNlLW9mLWEtbG9yYS0NCj4gZ2F0ZXdheS81NzMwLzc0DQo+ IA0KPiBSZWdhcmRzLA0KPiBBbmRyZWFzDQo+IA0KPiA+IEkgd2lsbCBkaWcgaW50byB0aGlzIHNl cmllcyB0aGlzIGV2ZW5pbmcuDQo+ID4NCj4gPiBbMV0gaHR0cHM6Ly9naXRodWIuY29tL0JXaGl0 dGVuL2xpbnV4LQ0KPiBzdGFibGUvdHJlZS85NzFhYWRjOGZkZmU4NDIwMjBkOTEyNDQ5YmRkNzFi MzNkNTc2ZmUzL2RyaXZlcnMvbmV0L2xvcmENCj4gWy4uLl0NCj4gPj4gZGlmZiAtLWdpdCBhL2Ry aXZlcnMvbmV0L2xvcmEvc3gxMzAxLmMgYi9kcml2ZXJzL25ldC9sb3JhL3N4MTMwMS5jDQo+ID4+ IG5ldyBmaWxlIG1vZGUgMTAwNjQ0DQo+ID4+IGluZGV4IDAwMDAwMDAwMDAwMC4uNWM5MzZjMTEx NmQxDQo+ID4+IC0tLSAvZGV2L251bGwNCj4gPj4gKysrIGIvZHJpdmVycy9uZXQvbG9yYS9zeDEz MDEuYw0KPiA+PiBAQCAtMCwwICsxLDQ0NiBAQA0KPiA+PiArLy8gU1BEWC1MaWNlbnNlLUlkZW50 aWZpZXI6IEdQTC0yLjAtb3ItbGF0ZXINCj4gW3NuaXBdDQoNCkJlbg0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@suse.de>"); +MODULE_LICENSE("GPL");
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@lairdtech.com> Cc: Steve deRosier <derosier@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Michael Röder <michael.roeder@avnet.eu> Cc: Ken Yu (禹凯) <ken.yu@rakwireless.com> Cc: linux-spi@vger.kernel.org Signed-off-by: Andreas Färber <afaerber@suse.de> --- 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