Skip to content

Commit

Permalink
spi: bcm2835: reduce the abuse of the GPIO API
Browse files Browse the repository at this point in the history
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 <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20230911161553.24313-1-brgl@bgdev.pl
Signed-off-by: Mark Brown <broonie@kernel.org>
  • Loading branch information
Bartosz Golaszewski authored and Mark Brown committed Sep 18, 2023
1 parent 9386c95 commit 21f252c
Showing 1 changed file with 34 additions and 24 deletions.
58 changes: 34 additions & 24 deletions drivers/spi/spi-bcm2835.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* spi-atmel.c, Copyright (C) 2006 Atmel Corporation
*/

#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/debugfs.h>
Expand All @@ -26,9 +27,10 @@
#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h> /* FIXME: using chip internals */
#include <linux/gpio/driver.h> /* FIXME: using chip internals */
#include <linux/gpio/machine.h> /* FIXME: using GPIO lookup tables */
#include <linux/of_irq.h>
#include <linux/overflow.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>

/* SPI register offsets */
Expand Down Expand Up @@ -83,6 +85,7 @@ MODULE_PARM_DESC(polling_limit_us,
* struct bcm2835_spi - BCM2835 SPI controller
* @regs: base address of register map
* @clk: core clock, divided to calculate serial clock
* @cs_gpio: chip-select GPIO descriptor
* @clk_hz: core clock cached speed
* @irq: interrupt, signals TX FIFO empty or RX FIFO ¾ full
* @tfr: SPI transfer currently processed
Expand Down Expand Up @@ -117,6 +120,7 @@ MODULE_PARM_DESC(polling_limit_us,
struct bcm2835_spi {
void __iomem *regs;
struct clk *clk;
struct gpio_desc *cs_gpio;
unsigned long clk_hz;
int irq;
struct spi_transfer *tfr;
Expand Down Expand Up @@ -1156,15 +1160,11 @@ static void bcm2835_spi_handle_err(struct spi_controller *ctlr,
bcm2835_spi_reset_hw(bs);
}

static int chip_match_name(struct gpio_chip *chip, void *data)
{
return !strcmp(chip->label, data);
}

static void bcm2835_spi_cleanup(struct spi_device *spi)
{
struct bcm2835_spidev *target = spi_get_ctldata(spi);
struct spi_controller *ctlr = spi->controller;
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);

if (target->clear_rx_desc)
dmaengine_desc_free(target->clear_rx_desc);
Expand All @@ -1175,6 +1175,9 @@ static void bcm2835_spi_cleanup(struct spi_device *spi)
sizeof(u32),
DMA_TO_DEVICE);

gpiod_put(bs->cs_gpio);
spi_set_csgpiod(spi, 0, NULL);

kfree(target);
}

Expand Down Expand Up @@ -1221,7 +1224,7 @@ static int bcm2835_spi_setup(struct spi_device *spi)
struct spi_controller *ctlr = spi->controller;
struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
struct bcm2835_spidev *target = spi_get_ctldata(spi);
struct gpio_chip *chip;
struct gpiod_lookup_table *lookup __free(kfree) = NULL;
int ret;
u32 cs;

Expand Down Expand Up @@ -1288,29 +1291,36 @@ static int bcm2835_spi_setup(struct spi_device *spi)
}

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

/* get the gpio chip for the base */
chip = gpiochip_find("pinctrl-bcm2835", chip_match_name);
if (!chip)
return 0;
lookup->dev_id = dev_name(&spi->dev);
lookup->table[0] = GPIO_LOOKUP("pinctrl-bcm2835",
8 - (spi_get_chipselect(spi, 0)),
"cs", GPIO_LOOKUP_FLAGS_DEFAULT);

spi_set_csgpiod(spi, 0, gpiochip_request_own_desc(chip,
8 - (spi_get_chipselect(spi, 0)),
DRV_NAME,
GPIO_LOOKUP_FLAGS_DEFAULT,
GPIOD_OUT_LOW));
if (IS_ERR(spi_get_csgpiod(spi, 0))) {
ret = PTR_ERR(spi_get_csgpiod(spi, 0));
gpiod_add_lookup_table(lookup);

bs->cs_gpio = gpiod_get(&spi->dev, "cs", GPIOD_OUT_LOW);
gpiod_remove_lookup_table(lookup);
if (IS_ERR(bs->cs_gpio)) {
ret = PTR_ERR(bs->cs_gpio);
goto err_cleanup;
}

spi_set_csgpiod(spi, 0, bs->cs_gpio);

/* and set up the "mode" and level */
dev_info(&spi->dev, "setting up native-CS%i to use GPIO\n",
spi_get_chipselect(spi, 0));
Expand Down

0 comments on commit 21f252c

Please sign in to comment.