[PATCH v1 0/8] spi: Header and core clean up and refactoring
Various cleanups and refactorings of the SPI header and core parts united in a single series.
Patches 1 & 2, 5 & 6 & 8 are dependent inside each group.
No functional change intended.
Andy Shevchenko (8): spi: Remove unneeded OF node NULL checks spi: Drop duplicate IDR allocation code in spi_register_controller() spi: Use sysfs_emit() to instead of s*printf() spi: Get rid of old SPI_MASTER_NO_.X and SPI_MASTER_MUST_.X spi: Sort headers alphabetically spi: Clean up headers spi: Fix spelling typos and acronyms capitalization spi: Use struct_size() helper
drivers/spi/spi-at91-usart.c | 2 +- drivers/spi/spi-atmel.c | 2 +- drivers/spi/spi-bitbang-txrx.h | 16 +-- drivers/spi/spi-bitbang.c | 2 +- drivers/spi/spi-davinci.c | 2 +- drivers/spi/spi-fsl-lpspi.c | 2 +- drivers/spi/spi-gpio.c | 8 +- drivers/spi/spi-lp8841-rtc.c | 8 +- drivers/spi/spi-meson-spicc.c | 2 +- drivers/spi/spi-mt65xx.c | 2 +- drivers/spi/spi-pci1xxxx.c | 2 +- drivers/spi/spi-pic32.c | 2 +- drivers/spi/spi-rb4xx.c | 2 +- drivers/spi/spi-slave-mt27xx.c | 2 +- drivers/spi/spi-stm32.c | 2 +- drivers/spi/spi-xtensa-xtfpga.c | 2 +- drivers/spi/spi.c | 102 ++++++++--------- include/linux/spi/spi.h | 188 ++++++++++++++++++-------------- 18 files changed, 183 insertions(+), 165 deletions(-)
In the couple of places the NULL check of OF node is implied by the call that takes it as a parameter. Drop the respective duplicate checks.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/spi/spi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 9291b2a0e887..8f3282a71c63 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2399,9 +2399,6 @@ static void of_register_spi_devices(struct spi_controller *ctlr) struct spi_device *spi; struct device_node *nc;
- if (!ctlr->dev.of_node) - return; - for_each_available_child_of_node(ctlr->dev.of_node, nc) { if (of_node_test_and_set_flag(nc, OF_POPULATED)) continue; @@ -3134,7 +3131,7 @@ int spi_register_controller(struct spi_controller *ctlr) if (WARN(id < 0, "couldn't get idr")) return id == -ENOSPC ? -EBUSY : id; ctlr->bus_num = id; - } else if (ctlr->dev.of_node) { + } else { /* Allocate dynamic bus number using Linux idr */ id = of_alias_get_id(ctlr->dev.of_node, "spi"); if (id >= 0) {
Refactor spi_register_controller() to drop duplicate IDR allocation. Instead of if-else-if branching use two sequential if:s, which allows to re-use the logic of IDR allocation in all cases.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/spi/spi.c | 50 ++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 8f3282a71c63..6d74218cf38e 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3081,6 +3081,20 @@ static int spi_controller_check_ops(struct spi_controller *ctlr) return 0; }
+/* Allocate dynamic bus number using Linux idr */ +static int spi_controller_id_alloc(struct spi_controller *ctlr, int start, int end) +{ + int id; + + mutex_lock(&board_lock); + id = idr_alloc(&spi_master_idr, ctlr, start, end, GFP_KERNEL); + mutex_unlock(&board_lock); + if (WARN(id < 0, "couldn't get idr")) + return id == -ENOSPC ? -EBUSY : id; + ctlr->bus_num = id; + return 0; +} + /** * spi_register_controller - register SPI master or slave controller * @ctlr: initialized master, originally from spi_alloc_master() or @@ -3108,8 +3122,8 @@ int spi_register_controller(struct spi_controller *ctlr) { struct device *dev = ctlr->dev.parent; struct boardinfo *bi; + int first_dynamic; int status; - int id, first_dynamic;
if (!dev) return -ENODEV; @@ -3122,27 +3136,13 @@ int spi_register_controller(struct spi_controller *ctlr) if (status) return status;
+ if (ctlr->bus_num < 0) + ctlr->bus_num = of_alias_get_id(ctlr->dev.of_node, "spi"); if (ctlr->bus_num >= 0) { /* Devices with a fixed bus num must check-in with the num */ - mutex_lock(&board_lock); - id = idr_alloc(&spi_master_idr, ctlr, ctlr->bus_num, - ctlr->bus_num + 1, GFP_KERNEL); - mutex_unlock(&board_lock); - if (WARN(id < 0, "couldn't get idr")) - return id == -ENOSPC ? -EBUSY : id; - ctlr->bus_num = id; - } else { - /* Allocate dynamic bus number using Linux idr */ - id = of_alias_get_id(ctlr->dev.of_node, "spi"); - if (id >= 0) { - ctlr->bus_num = id; - mutex_lock(&board_lock); - id = idr_alloc(&spi_master_idr, ctlr, ctlr->bus_num, - ctlr->bus_num + 1, GFP_KERNEL); - mutex_unlock(&board_lock); - if (WARN(id < 0, "couldn't get idr")) - return id == -ENOSPC ? -EBUSY : id; - } + status = spi_controller_id_alloc(ctlr, ctlr->bus_num, ctlr->bus_num + 1); + if (status) + return status; } if (ctlr->bus_num < 0) { first_dynamic = of_alias_get_highest_id("spi"); @@ -3151,13 +3151,9 @@ int spi_register_controller(struct spi_controller *ctlr) else first_dynamic++;
- mutex_lock(&board_lock); - id = idr_alloc(&spi_master_idr, ctlr, first_dynamic, - 0, GFP_KERNEL); - mutex_unlock(&board_lock); - if (WARN(id < 0, "couldn't get idr")) - return id; - ctlr->bus_num = id; + status = spi_controller_id_alloc(ctlr, first_dynamic, 0); + if (status) + return status; } ctlr->bus_lock_flag = 0; init_completion(&ctlr->xfer_completion);
Follow the advice of the Documentation/filesystems/sysfs.rst and show() should only use sysfs_emit() or sysfs_emit_at() when formatting the value to be returned to user space.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/spi/spi.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 6d74218cf38e..0f05773c277c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -64,7 +64,7 @@ modalias_show(struct device *dev, struct device_attribute *a, char *buf) if (len != -ENODEV) return len;
- return sprintf(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias); + return sysfs_emit(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias); } static DEVICE_ATTR_RO(modalias);
@@ -89,7 +89,7 @@ static ssize_t driver_override_show(struct device *dev, ssize_t len;
device_lock(dev); - len = snprintf(buf, PAGE_SIZE, "%s\n", spi->driver_override ? : ""); + len = sysfs_emit(buf, "%s\n", spi->driver_override ? : ""); device_unlock(dev); return len; } @@ -2814,8 +2814,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr, struct device *child;
child = device_find_any_child(&ctlr->dev); - return sprintf(buf, "%s\n", - child ? to_spi_device(child)->modalias : NULL); + return sysfs_emit(buf, "%s\n", child ? to_spi_device(child)->modalias : NULL); }
static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/spi/spi-at91-usart.c | 2 +- drivers/spi/spi-atmel.c | 2 +- drivers/spi/spi-bitbang-txrx.h | 16 ++++++++-------- drivers/spi/spi-bitbang.c | 2 +- drivers/spi/spi-davinci.c | 2 +- drivers/spi/spi-fsl-lpspi.c | 2 +- drivers/spi/spi-gpio.c | 8 ++++---- drivers/spi/spi-lp8841-rtc.c | 8 ++++---- drivers/spi/spi-meson-spicc.c | 2 +- drivers/spi/spi-mt65xx.c | 2 +- drivers/spi/spi-pci1xxxx.c | 2 +- drivers/spi/spi-pic32.c | 2 +- drivers/spi/spi-rb4xx.c | 2 +- drivers/spi/spi-slave-mt27xx.c | 2 +- drivers/spi/spi-stm32.c | 2 +- drivers/spi/spi-xtensa-xtfpga.c | 2 +- include/linux/spi/spi.h | 4 ---- 17 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c index 7854d9790fe9..7dfe2b6c2990 100644 --- a/drivers/spi/spi-at91-usart.c +++ b/drivers/spi/spi-at91-usart.c @@ -527,7 +527,7 @@ static int at91_usart_spi_probe(struct platform_device *pdev) controller->dev.of_node = pdev->dev.parent->of_node; controller->bits_per_word_mask = SPI_BPW_MASK(8); controller->setup = at91_usart_spi_setup; - controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX; + controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX; controller->transfer_one = at91_usart_spi_transfer_one; controller->prepare_message = at91_usart_spi_prepare_message; controller->unprepare_message = at91_usart_spi_unprepare_message; diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 152cd6773403..0865993005b2 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1475,7 +1475,7 @@ static int atmel_spi_probe(struct platform_device *pdev) host->bus_num = pdev->id; host->num_chipselect = 4; host->setup = atmel_spi_setup; - host->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX | + host->flags = (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX | SPI_MASTER_GPIO_SS); host->transfer_one = atmel_spi_one_transfer; host->set_cs = atmel_spi_set_cs; diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h index 2dcbe166df63..0cab48b7875b 100644 --- a/drivers/spi/spi-bitbang-txrx.h +++ b/drivers/spi/spi-bitbang-txrx.h @@ -57,7 +57,7 @@ bitbang_txrx_be_cpha0(struct spi_device *spi, for (word <<= (32 - bits); likely(bits); bits--) {
/* setup MSB (to slave) on trailing edge */ - if ((flags & SPI_MASTER_NO_TX) == 0) { + if ((flags & SPI_CONTROLLER_NO_TX) == 0) { if ((word & (1 << 31)) != oldbit) { setmosi(spi, word & (1 << 31)); oldbit = word & (1 << 31); @@ -70,7 +70,7 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
/* sample MSB (from slave) on leading edge */ word <<= 1; - if ((flags & SPI_MASTER_NO_RX) == 0) + if ((flags & SPI_CONTROLLER_NO_RX) == 0) word |= getmiso(spi); setsck(spi, cpol); } @@ -90,7 +90,7 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
/* setup MSB (to slave) on leading edge */ setsck(spi, !cpol); - if ((flags & SPI_MASTER_NO_TX) == 0) { + if ((flags & SPI_CONTROLLER_NO_TX) == 0) { if ((word & (1 << 31)) != oldbit) { setmosi(spi, word & (1 << 31)); oldbit = word & (1 << 31); @@ -103,7 +103,7 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
/* sample MSB (from slave) on trailing edge */ word <<= 1; - if ((flags & SPI_MASTER_NO_RX) == 0) + if ((flags & SPI_CONTROLLER_NO_RX) == 0) word |= getmiso(spi); } return word; @@ -122,7 +122,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi, for (; likely(bits); bits--) {
/* setup LSB (to slave) on trailing edge */ - if ((flags & SPI_MASTER_NO_TX) == 0) { + if ((flags & SPI_CONTROLLER_NO_TX) == 0) { if ((word & 1) != oldbit) { setmosi(spi, word & 1); oldbit = word & 1; @@ -135,7 +135,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi,
/* sample LSB (from slave) on leading edge */ word >>= 1; - if ((flags & SPI_MASTER_NO_RX) == 0) + if ((flags & SPI_CONTROLLER_NO_RX) == 0) word |= getmiso(spi) << rxbit; setsck(spi, cpol); } @@ -156,7 +156,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
/* setup LSB (to slave) on leading edge */ setsck(spi, !cpol); - if ((flags & SPI_MASTER_NO_TX) == 0) { + if ((flags & SPI_CONTROLLER_NO_TX) == 0) { if ((word & 1) != oldbit) { setmosi(spi, word & 1); oldbit = word & 1; @@ -169,7 +169,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
/* sample LSB (from slave) on trailing edge */ word >>= 1; - if ((flags & SPI_MASTER_NO_RX) == 0) + if ((flags & SPI_CONTROLLER_NO_RX) == 0) word |= getmiso(spi) << rxbit; } return word; diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c index 27d0087f8688..862f209cada1 100644 --- a/drivers/spi/spi-bitbang.c +++ b/drivers/spi/spi-bitbang.c @@ -248,7 +248,7 @@ static int spi_bitbang_bufs(struct spi_device *spi, struct spi_transfer *t) if (spi->mode & SPI_3WIRE) { unsigned flags;
- flags = t->tx_buf ? SPI_MASTER_NO_RX : SPI_MASTER_NO_TX; + flags = t->tx_buf ? SPI_CONTROLLER_NO_RX : SPI_CONTROLLER_NO_TX; return cs->txrx_bufs(spi, cs->txrx_word, nsecs, t, flags); } return cs->txrx_bufs(spi, cs->txrx_word, nsecs, t, 0); diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c index b04811c911e2..014392459d5e 100644 --- a/drivers/spi/spi-davinci.c +++ b/drivers/spi/spi-davinci.c @@ -939,7 +939,7 @@ static int davinci_spi_probe(struct platform_device *pdev) master->bus_num = pdev->id; master->num_chipselect = pdata->num_chipselect; master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16); - master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_GPIO_SS; + master->flags = SPI_CONTROLLER_MUST_RX | SPI_MASTER_GPIO_SS; master->setup = davinci_spi_setup; master->cleanup = davinci_spi_cleanup; master->can_dma = davinci_spi_can_dma; diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c index fb68c72df171..1c907d5d5bb3 100644 --- a/drivers/spi/spi-fsl-lpspi.c +++ b/drivers/spi/spi-fsl-lpspi.c @@ -856,7 +856,7 @@ static int fsl_lpspi_probe(struct platform_device *pdev) controller->prepare_transfer_hardware = lpspi_prepare_xfer_hardware; controller->unprepare_transfer_hardware = lpspi_unprepare_xfer_hardware; controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH; - controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX; + controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX; controller->dev.of_node = pdev->dev.of_node; controller->bus_num = pdev->id; controller->num_chipselect = fsl_lpspi->num_cs; diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c index 092afc7679d4..85e149ec2910 100644 --- a/drivers/spi/spi-gpio.c +++ b/drivers/spi/spi-gpio.c @@ -170,7 +170,7 @@ static u32 spi_gpio_txrx_word_mode3(struct spi_device *spi,
/* * These functions do not call setmosi or getmiso if respective flag - * (SPI_MASTER_NO_RX or SPI_MASTER_NO_TX) is set, so they are safe to + * (SPI_CONTROLLER_NO_RX or SPI_CONTROLLER_NO_TX) is set, so they are safe to * call when such pin is not present or defined in the controller. * A separate set of callbacks is defined to get highest possible * speed in the generic case (when both MISO and MOSI lines are @@ -416,11 +416,11 @@ static int spi_gpio_probe(struct platform_device *pdev) if (!spi_gpio->mosi) { /* HW configuration without MOSI pin * - * No setting SPI_MASTER_NO_RX here - if there is only + * No setting SPI_CONTROLLER_NO_RX here - if there is only * a MOSI pin connected the host can still do RX by * changing the direction of the line. */ - master->flags = SPI_MASTER_NO_TX; + master->flags = SPI_CONTROLLER_NO_TX; }
master->bus_num = pdev->id; @@ -438,7 +438,7 @@ static int spi_gpio_probe(struct platform_device *pdev) bb->chipselect = spi_gpio_chipselect; bb->set_line_direction = spi_gpio_set_direction;
- if (master->flags & SPI_MASTER_NO_TX) { + if (master->flags & SPI_CONTROLLER_NO_TX) { bb->txrx_word[SPI_MODE_0] = spi_gpio_spec_txrx_word_mode0; bb->txrx_word[SPI_MODE_1] = spi_gpio_spec_txrx_word_mode1; bb->txrx_word[SPI_MODE_2] = spi_gpio_spec_txrx_word_mode2; diff --git a/drivers/spi/spi-lp8841-rtc.c b/drivers/spi/spi-lp8841-rtc.c index 2d436541d6c2..ccaa7a946359 100644 --- a/drivers/spi/spi-lp8841-rtc.c +++ b/drivers/spi/spi-lp8841-rtc.c @@ -75,14 +75,14 @@ bitbang_txrx_be_cpha0_lsb(struct spi_lp8841_rtc *data, for (; likely(bits); bits--) {
/* setup LSB (to slave) on leading edge */ - if ((flags & SPI_MASTER_NO_TX) == 0) + if ((flags & SPI_CONTROLLER_NO_TX) == 0) setmosi(data, (word & 1));
usleep_range(usecs, usecs + 1); /* T(setup) */
/* sample LSB (from slave) on trailing edge */ word >>= 1; - if ((flags & SPI_MASTER_NO_RX) == 0) + if ((flags & SPI_CONTROLLER_NO_RX) == 0) word |= (getmiso(data) << 31);
setsck(data, !cpol); @@ -113,7 +113,7 @@ spi_lp8841_rtc_transfer_one(struct spi_master *master, while (likely(count > 0)) { word = *tx++; bitbang_txrx_be_cpha0_lsb(data, 1, 0, - SPI_MASTER_NO_RX, word, 8); + SPI_CONTROLLER_NO_RX, word, 8); count--; } } else if (rx) { @@ -121,7 +121,7 @@ spi_lp8841_rtc_transfer_one(struct spi_master *master, writeb(data->state, data->iomem); while (likely(count > 0)) { word = bitbang_txrx_be_cpha0_lsb(data, 1, 0, - SPI_MASTER_NO_TX, word, 8); + SPI_CONTROLLER_NO_TX, word, 8); *rx++ = word; count--; } diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c index 141562c882f1..7a9eba35ac38 100644 --- a/drivers/spi/spi-meson-spicc.c +++ b/drivers/spi/spi-meson-spicc.c @@ -864,7 +864,7 @@ static int meson_spicc_probe(struct platform_device *pdev) SPI_BPW_MASK(24) | SPI_BPW_MASK(16) | SPI_BPW_MASK(8); - master->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX); + master->flags = (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX); master->min_speed_hz = spicc->data->min_speed_hz; master->max_speed_hz = spicc->data->max_speed_hz; master->setup = meson_spicc_setup; diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c index 39272ad6641b..0757985947dd 100644 --- a/drivers/spi/spi-mt65xx.c +++ b/drivers/spi/spi-mt65xx.c @@ -1142,7 +1142,7 @@ static int mtk_spi_probe(struct platform_device *pdev) master->mode_bits |= SPI_CS_HIGH;
if (mdata->dev_comp->must_tx) - master->flags = SPI_MASTER_MUST_TX; + master->flags = SPI_CONTROLLER_MUST_TX; if (mdata->dev_comp->ipm_design) master->mode_bits |= SPI_LOOP | SPI_RX_DUAL | SPI_TX_DUAL | SPI_RX_QUAD | SPI_TX_QUAD; diff --git a/drivers/spi/spi-pci1xxxx.c b/drivers/spi/spi-pci1xxxx.c index 4445d82409d6..d23c42839da1 100644 --- a/drivers/spi/spi-pci1xxxx.c +++ b/drivers/spi/spi-pci1xxxx.c @@ -365,7 +365,7 @@ static int pci1xxxx_spi_probe(struct pci_dev *pdev, const struct pci_device_id * spi_host->bits_per_word_mask = SPI_BPW_MASK(8); spi_host->max_speed_hz = PCI1XXXX_SPI_MAX_CLOCK_HZ; spi_host->min_speed_hz = PCI1XXXX_SPI_MIN_CLOCK_HZ; - spi_host->flags = SPI_MASTER_MUST_TX; + spi_host->flags = SPI_CONTROLLER_MUST_TX; spi_master_set_devdata(spi_host, spi_sub_ptr); ret = devm_spi_register_master(dev, spi_host); if (ret) diff --git a/drivers/spi/spi-pic32.c b/drivers/spi/spi-pic32.c index f2af5e653f3d..e9b4c9cb97fb 100644 --- a/drivers/spi/spi-pic32.c +++ b/drivers/spi/spi-pic32.c @@ -773,7 +773,7 @@ static int pic32_spi_probe(struct platform_device *pdev) master->max_speed_hz = clk_get_rate(pic32s->clk); master->setup = pic32_spi_setup; master->cleanup = pic32_spi_cleanup; - master->flags = SPI_MASTER_MUST_TX | SPI_MASTER_MUST_RX; + master->flags = SPI_CONTROLLER_MUST_TX | SPI_CONTROLLER_MUST_RX; master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16) | SPI_BPW_MASK(32); master->transfer_one = pic32_spi_one_transfer; diff --git a/drivers/spi/spi-rb4xx.c b/drivers/spi/spi-rb4xx.c index 5073736d3d1f..c817889a7797 100644 --- a/drivers/spi/spi-rb4xx.c +++ b/drivers/spi/spi-rb4xx.c @@ -156,7 +156,7 @@ static int rb4xx_spi_probe(struct platform_device *pdev) master->num_chipselect = 3; master->mode_bits = SPI_TX_DUAL; master->bits_per_word_mask = SPI_BPW_MASK(8); - master->flags = SPI_MASTER_MUST_TX; + master->flags = SPI_CONTROLLER_MUST_TX; master->transfer_one = rb4xx_transfer_one; master->set_cs = rb4xx_set_cs;
diff --git a/drivers/spi/spi-slave-mt27xx.c b/drivers/spi/spi-slave-mt27xx.c index 4e4d426bfb43..6d6772974783 100644 --- a/drivers/spi/spi-slave-mt27xx.c +++ b/drivers/spi/spi-slave-mt27xx.c @@ -414,7 +414,7 @@ static int mtk_spi_slave_probe(struct platform_device *pdev) mdata->dev_comp = of_id->data;
if (mdata->dev_comp->must_rx) - ctlr->flags = SPI_MASTER_MUST_RX; + ctlr->flags = SPI_CONTROLLER_MUST_RX;
platform_set_drvdata(pdev, ctlr);
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c index 6d10fa4ab783..423212bd0675 100644 --- a/drivers/spi/spi-stm32.c +++ b/drivers/spi/spi-stm32.c @@ -1750,7 +1750,7 @@ static const struct stm32_spi_cfg stm32f4_spi_cfg = { .baud_rate_div_min = STM32F4_SPI_BR_DIV_MIN, .baud_rate_div_max = STM32F4_SPI_BR_DIV_MAX, .has_fifo = false, - .flags = SPI_MASTER_MUST_TX, + .flags = SPI_CONTROLLER_MUST_TX, };
static const struct stm32_spi_cfg stm32h7_spi_cfg = { diff --git a/drivers/spi/spi-xtensa-xtfpga.c b/drivers/spi/spi-xtensa-xtfpga.c index 24dc845b940e..dbd85d7a1526 100644 --- a/drivers/spi/spi-xtensa-xtfpga.c +++ b/drivers/spi/spi-xtensa-xtfpga.c @@ -87,7 +87,7 @@ static int xtfpga_spi_probe(struct platform_device *pdev) if (!master) return -ENOMEM;
- master->flags = SPI_MASTER_NO_RX; + master->flags = SPI_CONTROLLER_NO_RX; master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 16); master->bus_num = pdev->dev.id; master->dev.of_node = pdev->dev.of_node; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 32c94eae8926..44ba6798046c 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -1623,10 +1623,6 @@ spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer) #define spi_master spi_controller
#define SPI_MASTER_HALF_DUPLEX SPI_CONTROLLER_HALF_DUPLEX -#define SPI_MASTER_NO_RX SPI_CONTROLLER_NO_RX -#define SPI_MASTER_NO_TX SPI_CONTROLLER_NO_TX -#define SPI_MASTER_MUST_RX SPI_CONTROLLER_MUST_RX -#define SPI_MASTER_MUST_TX SPI_CONTROLLER_MUST_TX
#define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr) #define spi_master_set_devdata(_ctlr, _data) \
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
On Mon, Jul 10, 2023 at 02:08:00PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
The change here is not the change that is described above.
On Mon, Jul 10, 2023 at 12:10:03PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 02:08:00PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
The change here is not the change that is described above.
Okay, than you for elaboration and review. With the assumed split it should be addressed. Besides that, should I resend the entire series or only this one?
On Mon, Jul 10, 2023 at 02:08:00PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
A wild guess, maybe you meant to split to two changes, one per each macro group?
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
On Mon, Jul 10, 2023 at 02:10:53PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 02:08:00PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
A wild guess, maybe you meant to split to two changes, one per each macro group?
No, doing TX and RX in one commit is fine.
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
What part of the above change is replacing _NO_ with _MUST_?
On Mon, Jul 10, 2023 at 12:22:59PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 02:10:53PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 02:08:00PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:04:35PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 01:27:47PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
The above is not what this change does:
How to improve it? I was sure that the form of "converting to something and something" is clear...
A wild guess, maybe you meant to split to two changes, one per each macro group?
No, doing TX and RX in one commit is fine.
No, I meant one per _NO_ and _MUST_.
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
What part of the above change is replacing _NO_ with _MUST_?
None, that's why assuming the split by name should be fine.
On Mon, Jul 10, 2023 at 03:37:59PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:22:59PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 02:10:53PM +0300, Andy Shevchenko wrote:
Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X and kill the not used anymore definitions.
...
- controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
- controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
What part of the above change is replacing _NO_ with _MUST_?
None, that's why assuming the split by name should be fine.
That's what the above changelog sounds like it's trying to do (I'm not sure the change itself makes sense but the first thing I ran into when reviewing the patch), AFIACT you're missing a "from" in the changelog?
On Mon, Jul 10, 2023 at 02:21:05PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 03:37:59PM +0300, Andy Shevchenko wrote:
On Mon, Jul 10, 2023 at 12:22:59PM +0100, Mark Brown wrote:
On Mon, Jul 10, 2023 at 02:10:53PM +0300, Andy Shevchenko wrote:
...
> Convert the users to SPI_CONTROLLER_NO_?X and SPI_CONTROLLER_MUST_.X > and kill the not used anymore definitions.
...
> - controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX; > + controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
What part of the above change is replacing _NO_ with _MUST_?
None, that's why assuming the split by name should be fine.
That's what the above changelog sounds like it's trying to do (I'm not sure the change itself makes sense but the first thing I ran into when reviewing the patch), AFIACT you're missing a "from" in the changelog?
I see, I will elaborate better in v2. But still, I will split on per macro and add one for GPIO_SS. Seems to me better that way.
Sorting headers alphabetically helps locating duplicates, and make it easier to figure out where to insert new headers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/spi/spi.c | 42 ++++++++++++++++++++--------------------- include/linux/spi/spi.h | 14 +++++++------- 2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 0f05773c277c..695c72ccf93c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -4,36 +4,36 @@ // Copyright (C) 2005 David Brownell // Copyright (C) 2008 Secret Lab Technologies Ltd.
-#include <linux/kernel.h> -#include <linux/device.h> -#include <linux/init.h> +#include <linux/acpi.h> #include <linux/cache.h> -#include <linux/dma-mapping.h> +#include <linux/clk/clk-conf.h> +#include <linux/delay.h> +#include <linux/device.h> #include <linux/dmaengine.h> +#include <linux/dma-mapping.h> +#include <linux/export.h> +#include <linux/gpio/consumer.h> +#include <linux/highmem.h> +#include <linux/idr.h> +#include <linux/init.h> +#include <linux/ioport.h> +#include <linux/kernel.h> +#include <linux/kthread.h> +#include <linux/mod_devicetable.h> #include <linux/mutex.h> #include <linux/of_device.h> #include <linux/of_irq.h> -#include <linux/clk/clk-conf.h> -#include <linux/slab.h> -#include <linux/mod_devicetable.h> -#include <linux/spi/spi.h> -#include <linux/spi/spi-mem.h> -#include <linux/gpio/consumer.h> -#include <linux/pm_runtime.h> +#include <linux/percpu.h> +#include <linux/platform_data/x86/apple.h> #include <linux/pm_domain.h> +#include <linux/pm_runtime.h> #include <linux/property.h> -#include <linux/export.h> +#include <linux/ptp_clock_kernel.h> #include <linux/sched/rt.h> +#include <linux/slab.h> +#include <linux/spi/spi.h> +#include <linux/spi/spi-mem.h> #include <uapi/linux/sched/types.h> -#include <linux/delay.h> -#include <linux/kthread.h> -#include <linux/ioport.h> -#include <linux/acpi.h> -#include <linux/highmem.h> -#include <linux/idr.h> -#include <linux/platform_data/x86/apple.h> -#include <linux/ptp_clock_kernel.h> -#include <linux/percpu.h>
#define CREATE_TRACE_POINTS #include <trace/events/spi.h> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 44ba6798046c..becad31aeea2 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -6,19 +6,19 @@ #ifndef __LINUX_SPI_H #define __LINUX_SPI_H
+#include <linux/acpi.h> #include <linux/bits.h> +#include <linux/completion.h> #include <linux/device.h> -#include <linux/mod_devicetable.h> -#include <linux/slab.h> +#include <linux/gpio/consumer.h> #include <linux/kthread.h> -#include <linux/completion.h> +#include <linux/mod_devicetable.h> #include <linux/scatterlist.h> -#include <linux/gpio/consumer.h> - -#include <uapi/linux/spi/spi.h> -#include <linux/acpi.h> +#include <linux/slab.h> #include <linux/u64_stats_sync.h>
+#include <uapi/linux/spi/spi.h> + struct dma_chan; struct software_node; struct ptp_system_timestamp;
There is a few things done: - include only the headers we are direct user of - when pointer is in use, provide a forward declaration - add missing headers - group generic headers and subsystem headers - sort each group alphabetically
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- include/linux/spi/spi.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index becad31aeea2..8e7fc0f21714 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -6,27 +6,41 @@ #ifndef __LINUX_SPI_H #define __LINUX_SPI_H
-#include <linux/acpi.h> #include <linux/bits.h> #include <linux/completion.h> +#include <linux/container_of.h> #include <linux/device.h> -#include <linux/gpio/consumer.h> +#include <linux/export.h> #include <linux/kthread.h> +#include <linux/limits.h> +#include <linux/list.h> +#include <linux/minmax.h> #include <linux/mod_devicetable.h> +#include <linux/mutex.h> #include <linux/scatterlist.h> #include <linux/slab.h> +#include <linux/smp.h> +#include <linux/spinlock_types.h> +#include <linux/string.h> +#include <linux/types.h> #include <linux/u64_stats_sync.h>
+#include <asm/byteorder.h> + #include <uapi/linux/spi/spi.h>
+struct acpi_device; struct dma_chan; -struct software_node; +struct gpio_desc; struct ptp_system_timestamp; +struct software_node; + struct spi_controller; -struct spi_transfer; struct spi_controller_mem_ops; struct spi_controller_mem_caps; +struct spi_device_id; struct spi_message; +struct spi_transfer;
/* * INTERFACES between SPI master-side drivers and SPI slave protocol handlers,
Fix - spelling typos - capitalization of acronyms in the comments.
While at it, fix the multi-line comment style.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- include/linux/spi/spi.h | 137 ++++++++++++++++++++++------------------ 1 file changed, 75 insertions(+), 62 deletions(-)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 8e7fc0f21714..e9fb96016dc1 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -50,7 +50,7 @@ extern struct bus_type spi_bus_type;
/** * struct spi_statistics - statistics for spi transfers - * @syncp: seqcount to protect members in this struct for per-cpu udate + * @syncp: seqcount to protect members in this struct for per-cpu update * on 32-bit systems * * @messages: number of spi-messages handled @@ -69,7 +69,7 @@ extern struct bus_type spi_bus_type; * @bytes_rx: number of bytes received from device * * @transfer_bytes_histo: - * transfer bytes histogramm + * transfer bytes histogram * * @transfers_split_maxsize: * number of transfers that have been split because of @@ -170,7 +170,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg, * the device will bind to the named driver and only the named driver. * Do not set directly, because core frees it; use driver_set_override() to * set or clear it. - * @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when + * @cs_gpiod: GPIO descriptor of the chipselect line (optional, NULL when * not using a GPIO line) * @word_delay: delay to be inserted between consecutive * words of a transfer @@ -226,7 +226,7 @@ struct spi_device { void *controller_data; char modalias[SPI_NAME_SIZE]; const char *driver_override; - struct gpio_desc *cs_gpiod; /* Chip select gpio desc */ + struct gpio_desc *cs_gpiod; /* Chip select GPIO descriptor */ struct spi_delay word_delay; /* Inter-word delay */ /* CS delays */ struct spi_delay cs_setup; @@ -237,7 +237,7 @@ struct spi_device { struct spi_statistics __percpu *pcpu_statistics;
/* - * likely need more hooks for more protocol options affecting how + * Likely need more hooks for more protocol options affecting how * the controller talks to each chip, like: * - memory packing (12 bit samples into low bits, others zeroed) * - priority @@ -313,11 +313,11 @@ static inline void spi_set_csgpiod(struct spi_device *spi, u8 idx, struct gpio_d /** * struct spi_driver - Host side "protocol" driver * @id_table: List of SPI devices supported by this driver - * @probe: Binds this driver to the spi device. Drivers can verify + * @probe: Binds this driver to the SPI device. Drivers can verify * that the device is actually present, and may need to configure * characteristics (such as bits_per_word) which weren't needed for * the initial configuration done during system setup. - * @remove: Unbinds this driver from the spi device + * @remove: Unbinds this driver from the SPI device * @shutdown: Standard shutdown callback used during system state * transitions such as powerdown/halt and kexec * @driver: SPI device drivers should initialize the name and owner @@ -429,7 +429,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * @queued: whether this controller is providing an internal message queue * @kworker: pointer to thread struct for message pump * @pump_messages: work struct for scheduling work to the message pump - * @queue_lock: spinlock to syncronise access to message queue + * @queue_lock: spinlock to synchronise access to message queue * @queue: message queue * @cur_msg: the currently in-flight message * @cur_msg_completion: a completion for the current in-flight message @@ -487,7 +487,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * @unprepare_message: undo any work done by prepare_message(). * @slave_abort: abort the ongoing transfer request on an SPI slave controller * @target_abort: abort the ongoing transfer request on an SPI target controller - * @cs_gpiods: Array of GPIO descs to use as chip select lines; one per CS + * @cs_gpiods: Array of GPIO descriptors to use as chip select lines; one per CS * number. Any individual value may be NULL for CS lines that * are not GPIOs (driven by the SPI controller itself). * @use_gpio_descriptors: Turns on the code in the SPI core to parse and grab @@ -514,7 +514,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * If the driver does not set this, the SPI core takes the snapshot as * close to the driver hand-over as possible. * @irq_flags: Interrupt enable state during PTP system timestamping - * @fallback: fallback to pio if dma transfer return failure with + * @fallback: fallback to PIO if DMA transfer return failure with * SPI_TRANS_FAIL_NO_START. * @queue_empty: signal green light for opportunistically skipping the queue * for spi_sync transfers. @@ -536,15 +536,17 @@ struct spi_controller {
struct list_head list;
- /* Other than negative (== assign one dynamically), bus_num is fully - * board-specific. usually that simplifies to being SOC-specific. - * example: one SOC has three SPI controllers, numbered 0..2, - * and one board's schematics might show it using SPI-2. software + /* + * Other than negative (== assign one dynamically), bus_num is fully + * board-specific. Usually that simplifies to being SoC-specific. + * example: one SoC has three SPI controllers, numbered 0..2, + * and one board's schematics might show it using SPI-2. Software * would normally use bus_num=2 for that controller. */ s16 bus_num;
- /* chipselects will be integral to many controllers; some others + /* + * Chipselects will be integral to many controllers; some others * might use board-specific GPIOs. */ u16 num_chipselect; @@ -590,8 +592,8 @@ struct spi_controller { };
/* - * on some hardware transfer / message size may be constrained - * the limit may depend on device transfer settings + * On some hardware transfer / message size may be constrained + * the limit may depend on device transfer settings. */ size_t (*max_transfer_size)(struct spi_device *spi); size_t (*max_message_size)(struct spi_device *spi); @@ -609,7 +611,8 @@ struct spi_controller { /* Flag indicating that the SPI bus is locked for exclusive use */ bool bus_lock_flag;
- /* Setup mode and clock, etc (spi driver may call many times). + /* + * Setup mode and clock, etc (SPI driver may call many times). * * IMPORTANT: this may be called when transfers to another * device are active. DO NOT UPDATE SHARED REGISTERS in ways @@ -627,18 +630,19 @@ struct spi_controller { */ int (*set_cs_timing)(struct spi_device *spi);
- /* Bidirectional bulk transfers + /* + * Bidirectional bulk transfers * * + The transfer() method may not sleep; its main role is * just to add the message to the queue. * + For now there's no remove-from-queue operation, or * any other request management - * + To a given spi_device, message queueing is pure fifo + * + To a given spi_device, message queueing is pure FIFO * * + The controller's main job is to process its message queue, * selecting a chip (for masters), then transferring data * + If there are multiple spi_device children, the i/o queue - * arbitration algorithm is unspecified (round robin, fifo, + * arbitration algorithm is unspecified (round robin, FIFO, * priority, reservations, preemption, etc) * * + Chipselect stays active during the entire message @@ -719,7 +723,7 @@ struct spi_controller { const struct spi_controller_mem_ops *mem_ops; const struct spi_controller_mem_caps *mem_caps;
- /* gpio chip select */ + /* GPIO chip select */ struct gpio_desc **cs_gpiods; bool use_gpio_descriptors; s8 unused_native_cs; @@ -803,7 +807,7 @@ void spi_take_timestamp_post(struct spi_controller *ctlr, struct spi_transfer *xfer, size_t progress, bool irqs_off);
-/* The spi driver core manages memory for the spi_controller classdev */ +/* The SPI driver core manages memory for the spi_controller classdev */ extern struct spi_controller *__spi_alloc_controller(struct device *host, unsigned int size, bool slave);
@@ -892,13 +896,13 @@ typedef void (*spi_res_release_t)(struct spi_controller *ctlr, void *res);
/** - * struct spi_res - spi resource management structure + * struct spi_res - SPI resource management structure * @entry: list entry * @release: release code called prior to freeing this resource * @data: extra data allocated for the specific use-case * - * this is based on ideas from devres, but focused on life-cycle - * management during spi_message processing + * This is based on ideas from devres, but focused on life-cycle + * management during spi_message processing. */ struct spi_res { struct list_head entry; @@ -916,7 +920,7 @@ struct spi_res { * * The spi_messages themselves consist of a series of read+write transfer * segments. Those segments always read the same number of bits as they - * write; but one or the other is easily ignored by passing a null buffer + * write; but one or the other is easily ignored by passing a NULL buffer * pointer. (This is unlike most types of I/O API, because SPI hardware * is full duplex.) * @@ -927,8 +931,8 @@ struct spi_res {
/** * struct spi_transfer - a read/write buffer pair - * @tx_buf: data to be written (dma-safe memory), or NULL - * @rx_buf: data to be read (dma-safe memory), or NULL + * @tx_buf: data to be written (DMA-safe memory), or NULL + * @rx_buf: data to be read (DMA-safe memory), or NULL * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped * @tx_nbits: number of bits used for writing. If 0 the default @@ -951,7 +955,7 @@ struct spi_res { * @word_delay: inter word delay to be introduced after each word size * (set by bits_per_word) transmission. * @effective_speed_hz: the effective SCK-speed that was used to - * transfer this transfer. Set to 0 if the spi bus driver does + * transfer this transfer. Set to 0 if the SPI bus driver does * not support it. * @transfer_list: transfers are sequenced through @spi_message.transfers * @tx_sg: Scatterlist for transmit, currently not for client use @@ -980,16 +984,16 @@ struct spi_res { * transmitting the "pre" word, and the "post" timestamp after receiving * transmit confirmation from the controller for the "post" word. * @timestamped: true if the transfer has been timestamped - * @error: Error status logged by spi controller driver. + * @error: Error status logged by SPI controller driver. * * SPI transfers always write the same number of bytes as they read. * Protocol drivers should always provide @rx_buf and/or @tx_buf. * In some cases, they may also want to provide DMA addresses for * the data being transferred; that may reduce overhead, when the - * underlying driver uses dma. + * underlying driver uses DMA. * - * If the transmit buffer is null, zeroes will be shifted out - * while filling @rx_buf. If the receive buffer is null, the data + * If the transmit buffer is NULL, zeroes will be shifted out + * while filling @rx_buf. If the receive buffer is NULL, the data * shifted in will be discarded. Only "len" bytes shift out (or in). * It's an error to try to shift out a partial word. (For example, by * shifting out three bytes with word size of sixteen or twenty bits; @@ -1023,7 +1027,7 @@ struct spi_res { * Some devices need protocol transactions to be built from a series of * spi_message submissions, where the content of one message is determined * by the results of previous messages and where the whole transaction - * ends when the chipselect goes intactive. + * ends when the chipselect goes inactive. * * When SPI can transfer in 1x,2x or 4x. It can get this transfer information * from device through @tx_nbits and @rx_nbits. In Bi-direction, these @@ -1037,10 +1041,11 @@ struct spi_res { * and its transfers, ignore them until its completion callback. */ struct spi_transfer { - /* It's ok if tx_buf == rx_buf (right?) - * for MicroWire, one buffer must be null - * buffers must work with dma_*map_single() calls, unless - * spi_message.is_dma_mapped reports a pre-existing mapping + /* + * It's okay if tx_buf == rx_buf (right?). + * For MicroWire, one buffer must be NULL. + * Buffers must work with dma_*map_single() calls, unless + * spi_message.is_dma_mapped reports a pre-existing mapping. */ const void *tx_buf; void *rx_buf; @@ -1060,9 +1065,9 @@ struct spi_transfer { unsigned tx_nbits:3; unsigned rx_nbits:3; unsigned timestamped:1; -#define SPI_NBITS_SINGLE 0x01 /* 1bit transfer */ -#define SPI_NBITS_DUAL 0x02 /* 2bits transfer */ -#define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ +#define SPI_NBITS_SINGLE 0x01 /* 1-bit transfer */ +#define SPI_NBITS_DUAL 0x02 /* 2-bit transfer */ +#define SPI_NBITS_QUAD 0x04 /* 4-bit transfer */ u8 bits_per_word; struct spi_delay delay; struct spi_delay cs_change_delay; @@ -1083,7 +1088,7 @@ struct spi_transfer { * struct spi_message - one multi-segment SPI transaction * @transfers: list of transfer segments in this transaction * @spi: SPI device to which the transaction is queued - * @is_dma_mapped: if true, the caller provided both dma and cpu virtual + * @is_dma_mapped: if true, the caller provided both DMA and CPU virtual * addresses for each transfer buffer * @complete: called to report transaction completions * @context: the argument to complete() when it's called @@ -1093,7 +1098,7 @@ struct spi_transfer { * @status: zero for success, else negative errno * @queue: for use by whichever driver currently owns the message * @state: for use by whichever driver currently owns the message - * @resources: for resource management when the spi message is processed + * @resources: for resource management when the SPI message is processed * @prepared: spi_prepare_message was called for the this message * * A @spi_message is used to execute an atomic sequence of data transfers, @@ -1120,7 +1125,8 @@ struct spi_message { /* spi_prepare_message() was called for this message */ bool prepared;
- /* REVISIT: we might want a flag affecting the behavior of the + /* + * REVISIT: we might want a flag affecting the behavior of the * last transfer ... allowing things like "read 16 bit length L" * immediately followed by "read L bytes". Basically imposing * a specific message scheduling algorithm. @@ -1138,14 +1144,15 @@ struct spi_message { unsigned frame_length; unsigned actual_length;
- /* For optional use by whatever driver currently owns the + /* + * For optional use by whatever driver currently owns the * spi_message ... between calls to spi_async and then later * complete(), that's the spi_controller controller driver. */ struct list_head queue; void *state;
- /* List of spi_res reources when the spi message is processed */ + /* List of spi_res resources when the SPI message is processed */ struct list_head resources; };
@@ -1182,7 +1189,7 @@ spi_transfer_delay_exec(struct spi_transfer *t) /** * spi_message_init_with_transfers - Initialize spi_message and append transfers * @m: spi_message to be initialized - * @xfers: An array of spi transfers + * @xfers: An array of SPI transfers * @num_xfers: Number of items in the xfer array * * This function initializes the given spi_message and adds each spi_transfer in @@ -1199,10 +1206,10 @@ struct spi_transfer *xfers, unsigned int num_xfers) spi_message_add_tail(&xfers[i], m); }
-/* It's fine to embed message and transaction structures in other data +/* + * It's fine to embed message and transaction structures in other data * structures so long as you don't free them while they're in use. */ - static inline struct spi_message *spi_message_alloc(unsigned ntrans, gfp_t flags) { struct spi_message *m; @@ -1305,7 +1312,7 @@ typedef void (*spi_replaced_release_t)(struct spi_controller *ctlr, * replacements that have occurred * so that they can get reverted * @release: some extra release code to get executed prior to - * relasing this structure + * releasing this structure * @extradata: pointer to some extra data if requested or NULL * @replaced_transfers: transfers that have been replaced and which need * to get restored @@ -1315,9 +1322,9 @@ typedef void (*spi_replaced_release_t)(struct spi_controller *ctlr, * @inserted_transfers: array of spi_transfers of array-size @inserted, * that have been replacing replaced_transfers * - * note: that @extradata will point to @inserted_transfers[@inserted] + * Note: that @extradata will point to @inserted_transfers[@inserted] * if some extra allocation is requested, so alignment will be the same - * as for spi_transfers + * as for spi_transfers. */ struct spi_replaced_transfers { spi_replaced_release_t release; @@ -1343,7 +1350,8 @@ extern int spi_split_transfers_maxwords(struct spi_controller *ctlr,
/*---------------------------------------------------------------------------*/
-/* All these synchronous SPI transfer routines are utilities layered +/* + * All these synchronous SPI transfer routines are utilities layered * over the core async transfer primitive. Here, "synchronous" means * they will sleep uninterruptibly until the async transfer completes. */ @@ -1486,7 +1494,7 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd) * * Callable only from contexts that can sleep. * - * Return: the (unsigned) sixteen bit number returned by the device in cpu + * Return: the (unsigned) sixteen bit number returned by the device in CPU * endianness, or else a negative error code. */ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd) @@ -1514,7 +1522,7 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd) * As a rule, SPI devices can't be probed. Instead, board init code * provides a table listing the devices which are present, with enough * information to bind and set up the device's driver. There's basic - * support for nonstatic configurations too; enough to handle adding + * support for non-static configurations too; enough to handle adding * parport adapters, or microcontrollers acting as USB-to-SPI bridges. */
@@ -1551,12 +1559,13 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd) * are active in some dynamic board configuration models. */ struct spi_board_info { - /* The device name and module name are coupled, like platform_bus; + /* + * The device name and module name are coupled, like platform_bus; * "modalias" is normally the driver name. * * platform_data goes to spi_device.dev.platform_data, * controller_data goes to spi_device.controller_data, - * irq is copied too + * IRQ is copied too. */ char modalias[SPI_NAME_SIZE]; const void *platform_data; @@ -1568,7 +1577,8 @@ struct spi_board_info { u32 max_speed_hz;
- /* bus_num is board specific and matches the bus_num of some + /* + * bus_num is board specific and matches the bus_num of some * spi_controller that will probably be registered later. * * chip_select reflects how this chip is wired to that master; @@ -1577,12 +1587,14 @@ struct spi_board_info { u16 bus_num; u16 chip_select;
- /* mode becomes spi_device.mode, and is essential for chips + /* + * mode becomes spi_device.mode, and is essential for chips * where the default of SPI_CS_HIGH = 0 is wrong. */ u32 mode;
- /* ... may need additional spi_device chip config data here. + /* + * ... may need additional spi_device chip config data here. * avoid stuff protocol drivers can set; but include stuff * needed to behave without being bound to a driver: * - quirks like clock rate mattering when not selected @@ -1599,7 +1611,8 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n) { return 0; } #endif
-/* If you're hotplugging an adapter with devices (parport, usb, etc) +/* + * If you're hotplugging an adapter with devices (parport, USB, etc) * use spi_new_device() to describe each device. You can also call * spi_unregister_device() to start making that device vanish, but * normally that would be handled by spi_unregister_controller().
Prefer struct_size() over open-coded versions.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- include/linux/spi/spi.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index e9fb96016dc1..d4d686af76bd 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -17,6 +17,7 @@ #include <linux/minmax.h> #include <linux/mod_devicetable.h> #include <linux/mutex.h> +#include <linux/overflow.h> #include <linux/scatterlist.h> #include <linux/slab.h> #include <linux/smp.h> @@ -1100,6 +1101,8 @@ struct spi_transfer { * @state: for use by whichever driver currently owns the message * @resources: for resource management when the SPI message is processed * @prepared: spi_prepare_message was called for the this message + * @t: for use with spi_message_alloc() when memory has message and transfers + * together * * A @spi_message is used to execute an atomic sequence of data transfers, * each represented by a struct spi_transfer. The sequence is "atomic" @@ -1154,6 +1157,9 @@ struct spi_message {
/* List of spi_res resources when the SPI message is processed */ struct list_head resources; + + /* For embedding transfers into the memory of the message */ + struct spi_transfer t[]; };
static inline void spi_message_init_no_memset(struct spi_message *m) @@ -1214,16 +1220,13 @@ static inline struct spi_message *spi_message_alloc(unsigned ntrans, gfp_t flags { struct spi_message *m;
- m = kzalloc(sizeof(struct spi_message) - + ntrans * sizeof(struct spi_transfer), - flags); + m = kzalloc(struct_size(m, t, ntrans), flags); if (m) { unsigned i; - struct spi_transfer *t = (struct spi_transfer *)(m + 1);
spi_message_init_no_memset(m); - for (i = 0; i < ntrans; i++, t++) - spi_message_add_tail(t, m); + for (i = 0; i < ntrans; i++) + spi_message_add_tail(&m->t[i], m); } return m; }
On Mon, Jul 10, 2023 at 01:27:43PM +0300, Andy Shevchenko wrote:
Various cleanups and refactorings of the SPI header and core parts united in a single series.
Patches 1 & 2, 5 & 6 & 8 are dependent inside each group.
No functional change intended.
I will resend the entire series.
participants (2)
-
Andy Shevchenko
-
Mark Brown