From 6cc2d1a6536788f6334217440a2956c7e73f87f6 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Sun, 9 Jan 2022 10:27:28 +1300 Subject: staging: pi433: move get version func to where all other functions are As a convention for the pi433 driver, all routines that deals with the rf69 chip are defined in the rf69.c file. There was an exception to the rule in which the uC version verification was being done directly elsewhere. While at it, the Version Register hardcoded value was replaced with a pre-existing constant in the driver. This patch adds rf69_get_version function to rf69.c Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/20220108212728.GA7784@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 68c09fa016ed..051c9052aeeb 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -1115,8 +1115,8 @@ static int pi433_probe(struct spi_device *spi) "spi interface setup: mode 0x%2x, %d bits per word, %dhz max speed", spi->mode, spi->bits_per_word, spi->max_speed_hz); - /* Ping the chip by reading the version register */ - retval = spi_w8r8(spi, 0x10); + /* read chip version */ + retval = rf69_get_version(spi); if (retval < 0) return retval; -- cgit v1.2.3 From 14dbdad1f1a142f2c5d5dde6f671c18ac6763e30 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Wed, 19 Jan 2022 12:05:02 +1300 Subject: staging: pi433: change order in which driver config the rf69 chip There is an explicit dependency between modulation and bit rate configurations. To ensure proper validation of input value for the set_bit_rate routine, we must ensure that modulation has been set before. This patch ensures that set_modulation is always called before set_bit_rate for both RX and TX routines Acked-by: Dan Carpenter Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/20220118230502.GA4897@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 051c9052aeeb..523a1bca0c58 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -164,10 +164,10 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg) ret = rf69_set_frequency(dev->spi, rx_cfg->frequency); if (ret < 0) return ret; - ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate); + ret = rf69_set_modulation(dev->spi, rx_cfg->modulation); if (ret < 0) return ret; - ret = rf69_set_modulation(dev->spi, rx_cfg->modulation); + ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate); if (ret < 0) return ret; ret = rf69_set_antenna_impedance(dev->spi, rx_cfg->antenna_impedance); @@ -287,10 +287,10 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg) ret = rf69_set_frequency(dev->spi, tx_cfg->frequency); if (ret < 0) return ret; - ret = rf69_set_bit_rate(dev->spi, tx_cfg->bit_rate); + ret = rf69_set_modulation(dev->spi, tx_cfg->modulation); if (ret < 0) return ret; - ret = rf69_set_modulation(dev->spi, tx_cfg->modulation); + ret = rf69_set_bit_rate(dev->spi, tx_cfg->bit_rate); if (ret < 0) return ret; ret = rf69_set_deviation(dev->spi, tx_cfg->dev_frequency); -- cgit v1.2.3 From ce514dadc61a53ea7a2f5bbd67d31c52654e19b8 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Sat, 15 Jan 2022 11:16:43 +1300 Subject: staging: pi433: enforce tx_cfg to be set before any message can be sent this driver relies on exposing a char device to userspace to tx messages. Every message can be sent using different trasmitter settings such so the tx_cfg must be written before sending any messages. Failing to do so will cause the message to fail silently depending on printk/dynamic_debug settings which makes it hard to troubleshoot. This patch add a control variable that will get initialized once tx_cfg is set for the fd used when interacting with the char device. Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/20220114221643.GA7843@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 523a1bca0c58..17ff51f6a9da 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -108,6 +108,9 @@ struct pi433_device { struct pi433_instance { struct pi433_device *device; struct pi433_tx_cfg tx_cfg; + + /* control flags */ + bool tx_cfg_initialized; }; /*-------------------------------------------------------------------------*/ @@ -823,6 +826,16 @@ pi433_write(struct file *filp, const char __user *buf, if (count > MAX_MSG_SIZE) return -EMSGSIZE; + /* + * check if tx_cfg has been initialized otherwise we won't be able to + * config the RF trasmitter correctly due to invalid settings + */ + if (!instance->tx_cfg_initialized) { + dev_notice_once(device->dev, + "write: failed due to unconfigured tx_cfg (see PI433_IOC_WR_TX_CFG)"); + return -EINVAL; + } + /* * write the following sequence into fifo: * - tx_cfg @@ -897,6 +910,7 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return -EFAULT; mutex_lock(&device->tx_fifo_lock); memcpy(&instance->tx_cfg, &tx_cfg, sizeof(struct pi433_tx_cfg)); + instance->tx_cfg_initialized = true; mutex_unlock(&device->tx_fifo_lock); break; case PI433_IOC_RD_RX_CFG: @@ -949,8 +963,6 @@ static int pi433_open(struct inode *inode, struct file *filp) /* setup instance data*/ instance->device = device; - instance->tx_cfg.bit_rate = 4711; - // TODO: fill instance->tx_cfg; /* instance data as context */ filp->private_data = instance; -- cgit v1.2.3 From 52f11ec9b901ce6dc62705f1a734a1b00bb2735f Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Sun, 30 Jan 2022 18:08:38 +1300 Subject: staging: pi433: remove coding style item from the TODO file After several patches sent by the community along the last couple of years, it's possible to remove the coding style item from the TODO file in the driver's folder. This patch addresses the last code formatting inconsistences and remove the coding style item from the TODO file. Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/YfYdVokxsQ+Adl+T@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/TODO | 1 - drivers/staging/pi433/pi433_if.c | 12 ++++-------- drivers/staging/pi433/rf69.c | 3 ++- 3 files changed, 6 insertions(+), 10 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/TODO b/drivers/staging/pi433/TODO index b9e6c01a02e0..8d0f1b57961a 100644 --- a/drivers/staging/pi433/TODO +++ b/drivers/staging/pi433/TODO @@ -1,4 +1,3 @@ -* coding style does not fully comply with the kernel style guide. * still TODOs, annotated in the code * currently the code introduces new IOCTLs. I'm afraid this is a bad idea. -> Replace this with another interface, hints are welcome! diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 17ff51f6a9da..86ad497417f7 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -362,8 +362,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg) /*-------------------------------------------------------------------------*/ -static int -pi433_start_rx(struct pi433_device *dev) +static int pi433_start_rx(struct pi433_device *dev) { int retval; @@ -403,8 +402,7 @@ pi433_start_rx(struct pi433_device *dev) /*-------------------------------------------------------------------------*/ -static int -pi433_receive(void *data) +static int pi433_receive(void *data) { struct pi433_device *dev = data; struct spi_device *spi = dev->spi; @@ -555,8 +553,7 @@ abort: return bytes_total; } -static int -pi433_tx_thread(void *data) +static int pi433_tx_thread(void *data) { struct pi433_device *device = data; struct spi_device *spi = device->spi; @@ -881,8 +878,7 @@ abort: return -EAGAIN; } -static long -pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) +static long pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct pi433_instance *instance; struct pi433_device *device; diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index d60514a840c2..c68ad4809e91 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -654,7 +654,8 @@ bool rf69_get_flag(struct spi_device *spi, enum flag flag) return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_CRC_OK); case battery_low: return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_LOW_BAT); - default: return false; + default: + return false; } } -- cgit v1.2.3 From 4ef027d5a367131c56ffc4949466a892733a7d60 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Fri, 4 Feb 2022 21:04:32 +1300 Subject: staging: pi433: add debugfs interface This adds debugfs interface that can be used for debugging possible hardware/software issues. It currently exposes the following debugfs entries for each SPI device probed: /sys/kernel/debug/pi433//regs ... The 'regs' file contains all rf69 uC registers values that are useful for troubleshooting misconfigurations between 2 devices. It contains one register per line so it should be easy to use normal filtering tools to find the registers of interest if needed. Reviewed-by: Dan Carpenter Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/YfzeEHJcd+qvYGZ8@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 76 ++++++++++++++++++++++++++++++++++++++++ drivers/staging/pi433/rf69.c | 2 +- drivers/staging/pi433/rf69.h | 1 + 3 files changed, 78 insertions(+), 1 deletion(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 86ad497417f7..4fbac3ccef74 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -41,6 +41,8 @@ #ifdef CONFIG_COMPAT #include #endif +#include +#include #include "pi433_if.h" #include "rf69.h" @@ -1098,12 +1100,76 @@ static const struct file_operations pi433_fops = { .llseek = no_llseek, }; +static int pi433_debugfs_regs_show(struct seq_file *m, void *p) +{ + struct pi433_device *dev; + u8 reg_data[114]; + int i; + char *fmt = "0x%02x, 0x%02x\n"; + int ret; + + dev = m->private; + + mutex_lock(&dev->tx_fifo_lock); + mutex_lock(&dev->rx_lock); + + // wait for on-going operations to finish + ret = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active); + if (ret) + goto out_unlock; + + ret = wait_event_interruptible(dev->tx_wait_queue, !dev->rx_active); + if (ret) + goto out_unlock; + + // skip FIFO register (0x0) otherwise this can affect some of uC ops + for (i = 1; i < 0x50; i++) + reg_data[i] = rf69_read_reg(dev->spi, i); + + reg_data[REG_TESTLNA] = rf69_read_reg(dev->spi, REG_TESTLNA); + reg_data[REG_TESTPA1] = rf69_read_reg(dev->spi, REG_TESTPA1); + reg_data[REG_TESTPA2] = rf69_read_reg(dev->spi, REG_TESTPA2); + reg_data[REG_TESTDAGC] = rf69_read_reg(dev->spi, REG_TESTDAGC); + reg_data[REG_TESTAFC] = rf69_read_reg(dev->spi, REG_TESTAFC); + + seq_puts(m, "# reg, val\n"); + + for (i = 1; i < 0x50; i++) + seq_printf(m, fmt, i, reg_data[i]); + + seq_printf(m, fmt, REG_TESTLNA, reg_data[REG_TESTLNA]); + seq_printf(m, fmt, REG_TESTPA1, reg_data[REG_TESTPA1]); + seq_printf(m, fmt, REG_TESTPA2, reg_data[REG_TESTPA2]); + seq_printf(m, fmt, REG_TESTDAGC, reg_data[REG_TESTDAGC]); + seq_printf(m, fmt, REG_TESTAFC, reg_data[REG_TESTAFC]); + +out_unlock: + mutex_unlock(&dev->rx_lock); + mutex_unlock(&dev->tx_fifo_lock); + + return ret; +} + +static int pi433_debugfs_regs_open(struct inode *inode, struct file *filp) +{ + return single_open(filp, pi433_debugfs_regs_show, inode->i_private); +} + +static const struct file_operations debugfs_fops = { + .llseek = seq_lseek, + .open = pi433_debugfs_regs_open, + .owner = THIS_MODULE, + .read = seq_read, + .release = single_release +}; + /*-------------------------------------------------------------------------*/ static int pi433_probe(struct spi_device *spi) { struct pi433_device *device; int retval; + struct dentry *entry; /* setup spi parameters */ spi->mode = 0x00; @@ -1252,6 +1318,10 @@ static int pi433_probe(struct spi_device *spi) /* spi setup */ spi_set_drvdata(spi, device); + entry = debugfs_create_dir(dev_name(device->dev), + debugfs_lookup(KBUILD_MODNAME, NULL)); + debugfs_create_file("regs", 0400, entry, device, &debugfs_fops); + return 0; del_cdev: @@ -1275,6 +1345,9 @@ RX_failed: static int pi433_remove(struct spi_device *spi) { struct pi433_device *device = spi_get_drvdata(spi); + struct dentry *mod_entry = debugfs_lookup(KBUILD_MODNAME, NULL); + + debugfs_remove(debugfs_lookup(dev_name(device->dev), mod_entry)); /* free GPIOs */ free_gpio(device); @@ -1349,6 +1422,8 @@ static int __init pi433_init(void) return PTR_ERR(pi433_class); } + debugfs_create_dir(KBUILD_MODNAME, NULL); + status = spi_register_driver(&pi433_spi_driver); if (status < 0) { class_destroy(pi433_class); @@ -1366,6 +1441,7 @@ static void __exit pi433_exit(void) spi_unregister_driver(&pi433_spi_driver); class_destroy(pi433_class); unregister_chrdev(MAJOR(pi433_dev), pi433_spi_driver.driver.name); + debugfs_remove_recursive(debugfs_lookup(KBUILD_MODNAME, NULL)); } module_exit(pi433_exit); diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index c68ad4809e91..2ab3bf46e37d 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -24,7 +24,7 @@ /*-------------------------------------------------------------------------*/ -static u8 rf69_read_reg(struct spi_device *spi, u8 addr) +u8 rf69_read_reg(struct spi_device *spi, u8 addr) { int retval; diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h index c25942f142a6..3ff5609ecf2e 100644 --- a/drivers/staging/pi433/rf69.h +++ b/drivers/staging/pi433/rf69.h @@ -17,6 +17,7 @@ #define FIFO_SIZE 66 /* bytes */ #define FIFO_THRESHOLD 15 /* bytes */ +u8 rf69_read_reg(struct spi_device *spi, u8 addr); int rf69_get_version(struct spi_device *spi); int rf69_set_mode(struct spi_device *spi, enum mode mode); int rf69_set_data_mode(struct spi_device *spi, u8 data_mode); -- cgit v1.2.3 From 1b6a6147374eb37f652cd0a4e5b121ef8077c4d3 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Mon, 7 Feb 2022 10:06:36 +1300 Subject: staging: pi433: standardize use of line escape chars in dev_dbg stmts In this driver there are strings ending with of both '\n' and '' (without '\n') when using dev_dbg function. While it doesn't affect drivers functionality, it would be good to keep it consistent across the driver's source code. This patch add '\n' characters where it's missing to make it consistent with the other dev_dbg instances. Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/YgA4XHU4uf6YkOk5@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 54 ++++++++++++++++++++-------------------- drivers/staging/pi433/rf69.c | 51 ++++++++++++++++++------------------- 2 files changed, 53 insertions(+), 52 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 4fbac3ccef74..069255f023c8 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -414,7 +414,7 @@ static int pi433_receive(void *data) dev->interrupt_rx_allowed = false; /* wait for any tx to finish */ - dev_dbg(dev->dev, "rx: going to wait for any tx to finish"); + dev_dbg(dev->dev, "rx: going to wait for any tx to finish\n"); retval = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active); if (retval) { /* wait was interrupted */ @@ -440,7 +440,7 @@ static int pi433_receive(void *data) wake_up_interruptible(&dev->tx_wait_queue); /* wait for RSSI level to become high */ - dev_dbg(dev->dev, "rx: going to wait for high RSSI level"); + dev_dbg(dev->dev, "rx: going to wait for high RSSI level\n"); retval = wait_event_interruptible(dev->rx_wait_queue, rf69_get_flag(dev->spi, rssi_exceeded_threshold)); @@ -467,11 +467,11 @@ static int pi433_receive(void *data) goto abort; } bytes_total = dev->rx_cfg.fixed_message_length; - dev_dbg(dev->dev, "rx: msg len set to %d by fixed length", + dev_dbg(dev->dev, "rx: msg len set to %d by fixed length\n", bytes_total); } else { bytes_total = dev->rx_buffer_size; - dev_dbg(dev->dev, "rx: msg len set to %d as requested by read", + dev_dbg(dev->dev, "rx: msg len set to %d as requested by read\n", bytes_total); } @@ -488,7 +488,7 @@ static int pi433_receive(void *data) goto abort; } dev->free_in_fifo++; - dev_dbg(dev->dev, "rx: msg len reset to %d due to length byte", + dev_dbg(dev->dev, "rx: msg len reset to %d due to length byte\n", bytes_total); } @@ -505,7 +505,7 @@ static int pi433_receive(void *data) rf69_read_fifo(spi, &dummy, 1); dev->free_in_fifo++; - dev_dbg(dev->dev, "rx: address byte stripped off"); + dev_dbg(dev->dev, "rx: address byte stripped off\n"); } /* get payload */ @@ -567,7 +567,7 @@ static int pi433_tx_thread(void *data) while (1) { /* wait for fifo to be populated or for request to terminate*/ - dev_dbg(device->dev, "thread: going to wait for new messages"); + dev_dbg(device->dev, "thread: going to wait for new messages\n"); wait_event_interruptible(device->tx_wait_queue, (!kfifo_is_empty(&device->tx_fifo) || kthread_should_stop())); @@ -583,7 +583,7 @@ static int pi433_tx_thread(void *data) retval = kfifo_out(&device->tx_fifo, &tx_cfg, sizeof(tx_cfg)); if (retval != sizeof(tx_cfg)) { dev_dbg(device->dev, - "reading tx_cfg from fifo failed: got %d byte(s), expected %d", + "reading tx_cfg from fifo failed: got %d byte(s), expected %d\n", retval, (unsigned int)sizeof(tx_cfg)); continue; } @@ -591,7 +591,7 @@ static int pi433_tx_thread(void *data) retval = kfifo_out(&device->tx_fifo, &size, sizeof(size_t)); if (retval != sizeof(size_t)) { dev_dbg(device->dev, - "reading msg size from fifo failed: got %d, expected %d", + "reading msg size from fifo failed: got %d, expected %d\n", retval, (unsigned int)sizeof(size_t)); continue; } @@ -628,7 +628,7 @@ static int pi433_tx_thread(void *data) retval = kfifo_out(&device->tx_fifo, &device->buffer[position], sizeof(device->buffer) - position); dev_dbg(device->dev, - "read %d message byte(s) from fifo queue.", retval); + "read %d message byte(s) from fifo queue.\n", retval); /* * if rx is active, we need to interrupt the waiting for @@ -733,7 +733,7 @@ static int pi433_tx_thread(void *data) /* we are done. Wait for packet to get sent */ dev_dbg(device->dev, - "thread: wait for packet to get sent/fifo to be empty"); + "thread: wait for packet to get sent/fifo to be empty\n"); wait_event_interruptible(device->fifo_wait_queue, device->free_in_fifo == FIFO_SIZE || kthread_should_stop()); @@ -741,7 +741,7 @@ static int pi433_tx_thread(void *data) return 0; /* STOP_TRANSMISSION */ - dev_dbg(device->dev, "thread: Packet sent. Set mode to stby."); + dev_dbg(device->dev, "thread: Packet sent. Set mode to stby.\n"); retval = rf69_set_mode(spi, standby); if (retval < 0) goto abort; @@ -831,7 +831,7 @@ pi433_write(struct file *filp, const char __user *buf, */ if (!instance->tx_cfg_initialized) { dev_notice_once(device->dev, - "write: failed due to unconfigured tx_cfg (see PI433_IOC_WR_TX_CFG)"); + "write: failed due to unconfigured tx_cfg (see PI433_IOC_WR_TX_CFG)\n"); return -EINVAL; } @@ -846,7 +846,7 @@ pi433_write(struct file *filp, const char __user *buf, required = sizeof(instance->tx_cfg) + sizeof(size_t) + count; available = kfifo_avail(&device->tx_fifo); if (required > available) { - dev_dbg(device->dev, "write to fifo failed: %d bytes required but %d available", + dev_dbg(device->dev, "write to fifo failed: %d bytes required but %d available\n", required, available); mutex_unlock(&device->tx_fifo_lock); return -EAGAIN; @@ -869,13 +869,13 @@ pi433_write(struct file *filp, const char __user *buf, /* start transfer */ wake_up_interruptible(&device->tx_wait_queue); - dev_dbg(device->dev, "write: generated new msg with %d bytes.", copied); + dev_dbg(device->dev, "write: generated new msg with %d bytes.\n", copied); return copied; abort: dev_warn(device->dev, - "write to fifo failed, non recoverable: 0x%x", retval); + "write to fifo failed, non recoverable: 0x%x\n", retval); mutex_unlock(&device->tx_fifo_lock); return -EAGAIN; } @@ -1000,12 +1000,12 @@ static int setup_gpio(struct pi433_device *device) if (device->gpiod[i] == ERR_PTR(-ENOENT)) { dev_dbg(&device->spi->dev, - "Could not find entry for %s. Ignoring.", name); + "Could not find entry for %s. Ignoring.\n", name); continue; } if (device->gpiod[i] == ERR_PTR(-EBUSY)) - dev_dbg(&device->spi->dev, "%s is busy.", name); + dev_dbg(&device->spi->dev, "%s is busy.\n", name); if (IS_ERR(device->gpiod[i])) { retval = PTR_ERR(device->gpiod[i]); @@ -1038,7 +1038,7 @@ static int setup_gpio(struct pi433_device *device) if (retval) return retval; - dev_dbg(&device->spi->dev, "%s successfully configured", name); + dev_dbg(&device->spi->dev, "%s successfully configured\n", name); } return 0; @@ -1186,7 +1186,7 @@ static int pi433_probe(struct spi_device *spi) } dev_dbg(&spi->dev, - "spi interface setup: mode 0x%2x, %d bits per word, %dhz max speed", + "spi interface setup: mode 0x%2x, %d bits per word, %dhz max speed\n", spi->mode, spi->bits_per_word, spi->max_speed_hz); /* read chip version */ @@ -1196,10 +1196,10 @@ static int pi433_probe(struct spi_device *spi) switch (retval) { case 0x24: - dev_dbg(&spi->dev, "found pi433 (ver. 0x%x)", retval); + dev_dbg(&spi->dev, "found pi433 (ver. 0x%x)\n", retval); break; default: - dev_dbg(&spi->dev, "unknown chip version: 0x%x", retval); + dev_dbg(&spi->dev, "unknown chip version: 0x%x\n", retval); return -ENODEV; } @@ -1236,7 +1236,7 @@ static int pi433_probe(struct spi_device *spi) /* setup GPIO (including irq_handler) for the different DIOs */ retval = setup_gpio(device); if (retval) { - dev_dbg(&spi->dev, "setup of GPIOs failed"); + dev_dbg(&spi->dev, "setup of GPIOs failed\n"); goto GPIO_failed; } @@ -1266,7 +1266,7 @@ static int pi433_probe(struct spi_device *spi) /* determ minor number */ retval = pi433_get_minor(device); if (retval) { - dev_dbg(&spi->dev, "get of minor number failed"); + dev_dbg(&spi->dev, "get of minor number failed\n"); goto minor_failed; } @@ -1295,7 +1295,7 @@ static int pi433_probe(struct spi_device *spi) "pi433.%d_tx_task", device->minor); if (IS_ERR(device->tx_task_struct)) { - dev_dbg(device->dev, "start of send thread failed"); + dev_dbg(device->dev, "start of send thread failed\n"); retval = PTR_ERR(device->tx_task_struct); goto send_thread_failed; } @@ -1303,7 +1303,7 @@ static int pi433_probe(struct spi_device *spi) /* create cdev */ device->cdev = cdev_alloc(); if (!device->cdev) { - dev_dbg(device->dev, "allocation of cdev failed"); + dev_dbg(device->dev, "allocation of cdev failed\n"); retval = -ENOMEM; goto cdev_failed; } @@ -1311,7 +1311,7 @@ static int pi433_probe(struct spi_device *spi) cdev_init(device->cdev, &pi433_fops); retval = cdev_add(device->cdev, device->devt, 1); if (retval) { - dev_dbg(device->dev, "register of cdev failed"); + dev_dbg(device->dev, "register of cdev failed\n"); goto del_cdev; } diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index bf26aef69daa..a8def70808d6 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -85,7 +85,7 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode) }; if (unlikely(mode >= ARRAY_SIZE(mode_map))) { - dev_dbg(&spi->dev, "set: illegal mode %u", mode); + dev_dbg(&spi->dev, "set: illegal mode %u\n", mode); return -EINVAL; } @@ -115,7 +115,7 @@ int rf69_set_modulation(struct spi_device *spi, enum modulation modulation) }; if (unlikely(modulation >= ARRAY_SIZE(modulation_map))) { - dev_dbg(&spi->dev, "set: illegal modulation %u", modulation); + dev_dbg(&spi->dev, "set: illegal modulation %u\n", modulation); return -EINVAL; } @@ -163,7 +163,7 @@ int rf69_set_modulation_shaping(struct spi_device *spi, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_0_3); default: - dev_dbg(&spi->dev, "set: illegal mod shaping for FSK %u", mod_shaping); + dev_dbg(&spi->dev, "set: illegal mod shaping for FSK %u\n", mod_shaping); return -EINVAL; } case OOK: @@ -181,11 +181,11 @@ int rf69_set_modulation_shaping(struct spi_device *spi, MASK_DATAMODUL_MODULATION_SHAPE, DATAMODUL_MODULATION_SHAPE_2BR); default: - dev_dbg(&spi->dev, "set: illegal mod shaping for OOK %u", mod_shaping); + dev_dbg(&spi->dev, "set: illegal mod shaping for OOK %u\n", mod_shaping); return -EINVAL; } default: - dev_dbg(&spi->dev, "set: modulation undefined"); + dev_dbg(&spi->dev, "set: modulation undefined\n"); return -EINVAL; } } @@ -201,13 +201,13 @@ int rf69_set_bit_rate(struct spi_device *spi, u16 bit_rate) // check if modulation is configured mod = rf69_get_modulation(spi); if (mod == UNDEF) { - dev_dbg(&spi->dev, "setBitRate: modulation is undefined"); + dev_dbg(&spi->dev, "setBitRate: modulation is undefined\n"); return -EINVAL; } // check input value if (bit_rate < 1200 || (mod == OOK && bit_rate > 32768)) { - dev_dbg(&spi->dev, "setBitRate: illegal input param"); + dev_dbg(&spi->dev, "setBitRate: illegal input param\n"); return -EINVAL; } @@ -251,7 +251,7 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation) */ if (deviation < 600 || (deviation + (bit_rate / 2)) > 500000) { dev_dbg(&spi->dev, - "set_deviation: illegal input param: %u", deviation); + "set_deviation: illegal input param: %u\n", deviation); return -EINVAL; } @@ -268,7 +268,7 @@ int rf69_set_deviation(struct spi_device *spi, u32 deviation) // check msb if (msb & ~FDEVMASB_MASK) { - dev_dbg(&spi->dev, "set_deviation: err in calc of msb"); + dev_dbg(&spi->dev, "set_deviation: err in calc of msb\n"); return -EINVAL; } @@ -301,7 +301,7 @@ int rf69_set_frequency(struct spi_device *spi, u32 frequency) // check input value f_max = div_u64(f_step * 8388608, factor); if (frequency > f_max) { - dev_dbg(&spi->dev, "setFrequency: illegal input param"); + dev_dbg(&spi->dev, "setFrequency: illegal input param\n"); return -EINVAL; } @@ -382,7 +382,7 @@ int rf69_set_output_power_level(struct spi_device *spi, u8 power_level) return rf69_read_mod_write(spi, REG_PALEVEL, MASK_PALEVEL_OUTPUT_POWER, power_level); failed: - dev_dbg(&spi->dev, "set: illegal power level %u", power_level); + dev_dbg(&spi->dev, "set: illegal power level %u\n", power_level); return -EINVAL; } @@ -407,7 +407,7 @@ int rf69_set_pa_ramp(struct spi_device *spi, enum pa_ramp pa_ramp) }; if (unlikely(pa_ramp >= ARRAY_SIZE(pa_ramp_map))) { - dev_dbg(&spi->dev, "set: illegal pa_ramp %u", pa_ramp); + dev_dbg(&spi->dev, "set: illegal pa_ramp %u\n", pa_ramp); return -EINVAL; } @@ -423,7 +423,7 @@ int rf69_set_antenna_impedance(struct spi_device *spi, case two_hundred_ohm: return rf69_set_bit(spi, REG_LNA, MASK_LNA_ZIN); default: - dev_dbg(&spi->dev, "set: illegal antenna impedance %u", antenna_impedance); + dev_dbg(&spi->dev, "set: illegal antenna impedance %u\n", antenna_impedance); return -EINVAL; } } @@ -441,7 +441,7 @@ int rf69_set_lna_gain(struct spi_device *spi, enum lna_gain lna_gain) }; if (unlikely(lna_gain >= ARRAY_SIZE(lna_gain_map))) { - dev_dbg(&spi->dev, "set: illegal lna gain %u", lna_gain); + dev_dbg(&spi->dev, "set: illegal lna gain %u\n", lna_gain); return -EINVAL; } @@ -456,14 +456,14 @@ static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, // check value for mantisse and exponent if (exponent > 7) { - dev_dbg(&spi->dev, "set: illegal bandwidth exponent %u", exponent); + dev_dbg(&spi->dev, "set: illegal bandwidth exponent %u\n", exponent); return -EINVAL; } if (mantisse != mantisse16 && mantisse != mantisse20 && mantisse != mantisse24) { - dev_dbg(&spi->dev, "set: illegal bandwidth mantisse %u", mantisse); + dev_dbg(&spi->dev, "set: illegal bandwidth mantisse %u\n", mantisse); return -EINVAL; } @@ -521,7 +521,8 @@ int rf69_set_ook_threshold_dec(struct spi_device *spi, }; if (unlikely(threshold_decrement >= ARRAY_SIZE(td_map))) { - dev_dbg(&spi->dev, "set: illegal OOK threshold decrement %u", threshold_decrement); + dev_dbg(&spi->dev, "set: illegal OOK threshold decrement %u\n", + threshold_decrement); return -EINVAL; } @@ -568,7 +569,7 @@ int rf69_set_dio_mapping(struct spi_device *spi, u8 dio_number, u8 value) dio_addr = REG_DIOMAPPING2; break; default: - dev_dbg(&spi->dev, "set: illegal dio number %u", dio_number); + dev_dbg(&spi->dev, "set: illegal dio number %u\n", dio_number); return -EINVAL; } @@ -672,7 +673,7 @@ int rf69_set_fifo_fill_condition(struct spi_device *spi, return rf69_clear_bit(spi, REG_SYNC_CONFIG, MASK_SYNC_CONFIG_FIFO_FILL_CONDITION); default: - dev_dbg(&spi->dev, "set: illegal fifo fill condition %u", fifo_fill_condition); + dev_dbg(&spi->dev, "set: illegal fifo fill condition %u\n", fifo_fill_condition); return -EINVAL; } } @@ -681,7 +682,7 @@ int rf69_set_sync_size(struct spi_device *spi, u8 sync_size) { // check input value if (sync_size > 0x07) { - dev_dbg(&spi->dev, "set: illegal sync size %u", sync_size); + dev_dbg(&spi->dev, "set: illegal sync size %u\n", sync_size); return -EINVAL; } @@ -718,7 +719,7 @@ int rf69_set_packet_format(struct spi_device *spi, return rf69_clear_bit(spi, REG_PACKETCONFIG1, MASK_PACKETCONFIG1_PACKET_FORMAT_VARIABLE); default: - dev_dbg(&spi->dev, "set: illegal packet format %u", packet_format); + dev_dbg(&spi->dev, "set: illegal packet format %u\n", packet_format); return -EINVAL; } } @@ -744,7 +745,7 @@ int rf69_set_address_filtering(struct spi_device *spi, }; if (unlikely(address_filtering >= ARRAY_SIZE(af_map))) { - dev_dbg(&spi->dev, "set: illegal address filtering %u", address_filtering); + dev_dbg(&spi->dev, "set: illegal address filtering %u\n", address_filtering); return -EINVAL; } @@ -779,7 +780,7 @@ int rf69_set_tx_start_condition(struct spi_device *spi, return rf69_set_bit(spi, REG_FIFO_THRESH, MASK_FIFO_THRESH_TXSTART); default: - dev_dbg(&spi->dev, "set: illegal tx start condition %u", tx_start_condition); + dev_dbg(&spi->dev, "set: illegal tx start condition %u\n", tx_start_condition); return -EINVAL; } } @@ -790,7 +791,7 @@ int rf69_set_fifo_threshold(struct spi_device *spi, u8 threshold) /* check input value */ if (threshold & 0x80) { - dev_dbg(&spi->dev, "set: illegal fifo threshold %u", threshold); + dev_dbg(&spi->dev, "set: illegal fifo threshold %u\n", threshold); return -EINVAL; } @@ -817,7 +818,7 @@ int rf69_set_dagc(struct spi_device *spi, enum dagc dagc) }; if (unlikely(dagc >= ARRAY_SIZE(dagc_map))) { - dev_dbg(&spi->dev, "set: illegal dagc %u", dagc); + dev_dbg(&spi->dev, "set: illegal dagc %u\n", dagc); return -EINVAL; } -- cgit v1.2.3 From 2d19e698e7f1c2453340b515f18adc779c42b11e Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Sat, 26 Feb 2022 11:40:33 +1300 Subject: staging: pi433: remove rf69_get_flag function resolving enum conflict The reason why rf69_get_flag() existed was to provide a high-level way to obtain values out of 1 (of 2) flags registers using bit masking. The idea was to map the possible flag values found in the data sheet like shown in page 70 of the RFM69HCW datasheet. However, due to the fact that enums values in C must be unique, there was a naming conflict on 'fifo_not_empty' which is used by the tx_start_condition enum. So the author decided to create a 'fifo_empty' one which would negate the value that comes from the flag register as the solution to that conflict (which is very confusing). this patch removes rf69_get_flag function which subsequently solves the enum redeclaration problem so kernel developers can follow the data sheet more easily. Reviewed-by: Dan Carpenter Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/Yhla4a1Clpguoo2h@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 8 +++---- drivers/staging/pi433/rf69.c | 44 --------------------------------------- drivers/staging/pi433/rf69.h | 1 - drivers/staging/pi433/rf69_enum.h | 20 ------------------ 4 files changed, 4 insertions(+), 69 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 069255f023c8..3f3e863e6cc8 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -434,7 +434,7 @@ static int pi433_receive(void *data) return retval; /* now check RSSI, if low wait for getting high (RSSI interrupt) */ - while (!rf69_get_flag(dev->spi, rssi_exceeded_threshold)) { + while (!(rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_RSSI)) { /* allow tx to interrupt us while waiting for high RSSI */ dev->interrupt_rx_allowed = true; wake_up_interruptible(&dev->tx_wait_queue); @@ -442,8 +442,8 @@ static int pi433_receive(void *data) /* wait for RSSI level to become high */ dev_dbg(dev->dev, "rx: going to wait for high RSSI level\n"); retval = wait_event_interruptible(dev->rx_wait_queue, - rf69_get_flag(dev->spi, - rssi_exceeded_threshold)); + rf69_read_reg(spi, REG_IRQFLAGS1) + & MASK_IRQFLAGS1_RSSI); if (retval) /* wait was interrupted */ goto abort; dev->interrupt_rx_allowed = false; @@ -510,7 +510,7 @@ static int pi433_receive(void *data) /* get payload */ while (dev->rx_position < bytes_total) { - if (!rf69_get_flag(dev->spi, payload_ready)) { + if (!(rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_PAYLOAD_READY)) { retval = wait_event_interruptible(dev->fifo_wait_queue, dev->free_in_fifo < FIFO_SIZE); if (retval) /* wait was interrupted */ diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index 1a0081ebf63c..e5b23ab39c69 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -578,50 +578,6 @@ int rf69_set_dio_mapping(struct spi_device *spi, u8 dio_number, u8 value) return rf69_write_reg(spi, dio_addr, dio_value); } -bool rf69_get_flag(struct spi_device *spi, enum flag flag) -{ - switch (flag) { - case mode_switch_completed: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_MODE_READY); - case ready_to_receive: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_RX_READY); - case ready_to_send: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_TX_READY); - case pll_locked: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_PLL_LOCK); - case rssi_exceeded_threshold: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_RSSI); - case timeout: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_TIMEOUT); - case automode: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_AUTOMODE); - case sync_address_match: - return (rf69_read_reg(spi, REG_IRQFLAGS1) & MASK_IRQFLAGS1_SYNC_ADDRESS_MATCH); - case fifo_full: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_FULL); -/* - * case fifo_not_empty: - * return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY); - */ - case fifo_empty: - return !(rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_NOT_EMPTY); - case fifo_level_below_threshold: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_LEVEL); - case fifo_overrun: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_FIFO_OVERRUN); - case packet_sent: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_PACKET_SENT); - case payload_ready: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_PAYLOAD_READY); - case crc_ok: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_CRC_OK); - case battery_low: - return (rf69_read_reg(spi, REG_IRQFLAGS2) & MASK_IRQFLAGS2_LOW_BAT); - default: - return false; - } -} - int rf69_set_rssi_threshold(struct spi_device *spi, u8 threshold) { /* no value check needed - u8 exactly matches register size */ diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h index 3ff5609ecf2e..78fa0b8bab8b 100644 --- a/drivers/staging/pi433/rf69.h +++ b/drivers/staging/pi433/rf69.h @@ -42,7 +42,6 @@ int rf69_set_bandwidth_during_afc(struct spi_device *spi, int rf69_set_ook_threshold_dec(struct spi_device *spi, enum threshold_decrement threshold_decrement); int rf69_set_dio_mapping(struct spi_device *spi, u8 dio_number, u8 value); -bool rf69_get_flag(struct spi_device *spi, enum flag flag); int rf69_set_rssi_threshold(struct spi_device *spi, u8 threshold); int rf69_set_preamble_length(struct spi_device *spi, u16 preamble_length); int rf69_enable_sync(struct spi_device *spi); diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h index b33a33a85d3b..9dc906124e98 100644 --- a/drivers/staging/pi433/rf69_enum.h +++ b/drivers/staging/pi433/rf69_enum.h @@ -84,26 +84,6 @@ enum threshold_decrement { dec_16times }; -enum flag { - mode_switch_completed, - ready_to_receive, - ready_to_send, - pll_locked, - rssi_exceeded_threshold, - timeout, - automode, - sync_address_match, - fifo_full, -// fifo_not_empty, collision with next enum; replaced by following enum... - fifo_empty, - fifo_level_below_threshold, - fifo_overrun, - packet_sent, - payload_ready, - crc_ok, - battery_low -}; - enum fifo_fill_condition { after_sync_interrupt, always -- cgit v1.2.3 From d7e2d1e88823c93f98ce241050a9cfc9b591d33a Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Tue, 8 Mar 2022 21:19:50 +1300 Subject: staging: pi433: formatting improvement for multi-line bitwise statement Move bitwise & operator to the end of previous line for better readability Signed-off-by: Paulo Miguel Almeida Link: https://lore.kernel.org/r/YicRpmT2xMupVp4h@mail.google.com Signed-off-by: Greg Kroah-Hartman --- drivers/staging/pi433/pi433_if.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/staging/pi433/pi433_if.c') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 3f3e863e6cc8..9a55fb29bd54 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -442,8 +442,8 @@ static int pi433_receive(void *data) /* wait for RSSI level to become high */ dev_dbg(dev->dev, "rx: going to wait for high RSSI level\n"); retval = wait_event_interruptible(dev->rx_wait_queue, - rf69_read_reg(spi, REG_IRQFLAGS1) - & MASK_IRQFLAGS1_RSSI); + rf69_read_reg(spi, REG_IRQFLAGS1) & + MASK_IRQFLAGS1_RSSI); if (retval) /* wait was interrupted */ goto abort; dev->interrupt_rx_allowed = false; -- cgit v1.2.3