Skip to content

Commit 21f252c

Browse files
Bartosz Golaszewskibroonie
authored andcommitted
spi: bcm2835: reduce the abuse of the GPIO API
Currently the bcm2835 SPI driver uses functions that are available exclusively to GPIO providers as a way to handle a platform quirk. Let's use a slightly better alternative that avoids poking around in GPIOLIB's internals and use GPIO lookup tables. Link: https://www.spinics.net/lists/linux-gpio/msg36218.html Signed-off-by: Bartosz Golaszewski <[email protected]> Reviewed-by: Linus Walleij <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Mark Brown <[email protected]>
1 parent 9386c95 commit 21f252c

File tree

1 file changed

+34
-24
lines changed

1 file changed

+34
-24
lines changed

drivers/spi/spi-bcm2835.c

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* spi-atmel.c, Copyright (C) 2006 Atmel Corporation
1212
*/
1313

14+
#include <linux/cleanup.h>
1415
#include <linux/clk.h>
1516
#include <linux/completion.h>
1617
#include <linux/debugfs.h>
@@ -26,9 +27,10 @@
2627
#include <linux/of_address.h>
2728
#include <linux/platform_device.h>
2829
#include <linux/gpio/consumer.h>
29-
#include <linux/gpio/machine.h> /* FIXME: using chip internals */
30-
#include <linux/gpio/driver.h> /* FIXME: using chip internals */
30+
#include <linux/gpio/machine.h> /* FIXME: using GPIO lookup tables */
3131
#include <linux/of_irq.h>
32+
#include <linux/overflow.h>
33+
#include <linux/slab.h>
3234
#include <linux/spi/spi.h>
3335

3436
/* SPI register offsets */
@@ -83,6 +85,7 @@ MODULE_PARM_DESC(polling_limit_us,
8385
* struct bcm2835_spi - BCM2835 SPI controller
8486
* @regs: base address of register map
8587
* @clk: core clock, divided to calculate serial clock
88+
* @cs_gpio: chip-select GPIO descriptor
8689
* @clk_hz: core clock cached speed
8790
* @irq: interrupt, signals TX FIFO empty or RX FIFO ¾ full
8891
* @tfr: SPI transfer currently processed
@@ -117,6 +120,7 @@ MODULE_PARM_DESC(polling_limit_us,
117120
struct bcm2835_spi {
118121
void __iomem *regs;
119122
struct clk *clk;
123+
struct gpio_desc *cs_gpio;
120124
unsigned long clk_hz;
121125
int irq;
122126
struct spi_transfer *tfr;
@@ -1156,15 +1160,11 @@ static void bcm2835_spi_handle_err(struct spi_controller *ctlr,
11561160
bcm2835_spi_reset_hw(bs);
11571161
}
11581162

1159-
static int chip_match_name(struct gpio_chip *chip, void *data)
1160-
{
1161-
return !strcmp(chip->label, data);
1162-
}
1163-
11641163
static void bcm2835_spi_cleanup(struct spi_device *spi)
11651164
{
11661165
struct bcm2835_spidev *target = spi_get_ctldata(spi);
11671166
struct spi_controller *ctlr = spi->controller;
1167+
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
11681168

11691169
if (target->clear_rx_desc)
11701170
dmaengine_desc_free(target->clear_rx_desc);
@@ -1175,6 +1175,9 @@ static void bcm2835_spi_cleanup(struct spi_device *spi)
11751175
sizeof(u32),
11761176
DMA_TO_DEVICE);
11771177

1178+
gpiod_put(bs->cs_gpio);
1179+
spi_set_csgpiod(spi, 0, NULL);
1180+
11781181
kfree(target);
11791182
}
11801183

@@ -1221,7 +1224,7 @@ static int bcm2835_spi_setup(struct spi_device *spi)
12211224
struct spi_controller *ctlr = spi->controller;
12221225
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
12231226
struct bcm2835_spidev *target = spi_get_ctldata(spi);
1224-
struct gpio_chip *chip;
1227+
struct gpiod_lookup_table *lookup __free(kfree) = NULL;
12251228
int ret;
12261229
u32 cs;
12271230

@@ -1288,29 +1291,36 @@ static int bcm2835_spi_setup(struct spi_device *spi)
12881291
}
12891292

12901293
/*
1291-
* Translate native CS to GPIO
1294+
* TODO: The code below is a slightly better alternative to the utter
1295+
* abuse of the GPIO API that I found here before. It creates a
1296+
* temporary lookup table, assigns it to the SPI device, gets the GPIO
1297+
* descriptor and then releases the lookup table.
12921298
*
1293-
* FIXME: poking around in the gpiolib internals like this is
1294-
* not very good practice. Find a way to locate the real problem
1295-
* and fix it. Why is the GPIO descriptor in spi->cs_gpiod
1296-
* sometimes not assigned correctly? Erroneous device trees?
1299+
* More on the problem that it addresses:
1300+
* https://www.spinics.net/lists/linux-gpio/msg36218.html
12971301
*/
1302+
lookup = kzalloc(struct_size(lookup, table, 1), GFP_KERNEL);
1303+
if (!lookup) {
1304+
ret = -ENOMEM;
1305+
goto err_cleanup;
1306+
}
12981307

1299-
/* get the gpio chip for the base */
1300-
chip = gpiochip_find("pinctrl-bcm2835", chip_match_name);
1301-
if (!chip)
1302-
return 0;
1308+
lookup->dev_id = dev_name(&spi->dev);
1309+
lookup->table[0] = GPIO_LOOKUP("pinctrl-bcm2835",
1310+
8 - (spi_get_chipselect(spi, 0)),
1311+
"cs", GPIO_LOOKUP_FLAGS_DEFAULT);
13031312

1304-
spi_set_csgpiod(spi, 0, gpiochip_request_own_desc(chip,
1305-
8 - (spi_get_chipselect(spi, 0)),
1306-
DRV_NAME,
1307-
GPIO_LOOKUP_FLAGS_DEFAULT,
1308-
GPIOD_OUT_LOW));
1309-
if (IS_ERR(spi_get_csgpiod(spi, 0))) {
1310-
ret = PTR_ERR(spi_get_csgpiod(spi, 0));
1313+
gpiod_add_lookup_table(lookup);
1314+
1315+
bs->cs_gpio = gpiod_get(&spi->dev, "cs", GPIOD_OUT_LOW);
1316+
gpiod_remove_lookup_table(lookup);
1317+
if (IS_ERR(bs->cs_gpio)) {
1318+
ret = PTR_ERR(bs->cs_gpio);
13111319
goto err_cleanup;
13121320
}
13131321

1322+
spi_set_csgpiod(spi, 0, bs->cs_gpio);
1323+
13141324
/* and set up the "mode" and level */
13151325
dev_info(&spi->dev, "setting up native-CS%i to use GPIO\n",
13161326
spi_get_chipselect(spi, 0));

0 commit comments

Comments
 (0)