diff options
author | David Brownell <david-b@pacbell.net> | 2007-07-31 07:39:45 (GMT) |
---|---|---|
committer | Linus Torvalds <torvalds@woody.linux-foundation.org> | 2007-07-31 22:39:44 (GMT) |
commit | 082c8cb4e5e68c0fd29cc10c59db94d2284fd2b0 (patch) | |
tree | 5f4ec8042b29272f0fd0c28b2cc2c0bb63f38dc0 /drivers/spi/spi.c | |
parent | 2604288f45605d1b2844f001dc3141149667b3d1 (diff) | |
download | linux-082c8cb4e5e68c0fd29cc10c59db94d2284fd2b0.tar.xz |
spi device setup gets better error checking
This updates some error reporting paths in SPI device setup:
- Move validation logic for SPI chipselects to spi_new_device(),
which is where it should always have been.
- In spi_new_device(), emit error messages if the device can't
be created. This is LOTS better than a silent failure; though
eventually, the calling convention should probably change to
use the <linux/err.h> conventions.
- Includes one previously-missing check: SPI masters must always
have at least one chipselect, even for dedicated busses which
always keep it selected!
It also adds a FIXME (IDR for dynamic ID allocation) so the issue doesn't live
purely in my mailbox.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/spi/spi.c')
-rw-r--r-- | drivers/spi/spi.c | 45 |
1 files changed, 30 insertions, 15 deletions
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index b05de30..e84d215 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -200,6 +200,8 @@ static DEFINE_MUTEX(board_lock); * platforms may not be able to use spi_register_board_info though, and * this is exported so that for example a USB or parport based adapter * driver could add devices (which it would learn about out-of-band). + * + * Returns the new device, or NULL. */ struct spi_device *spi_new_device(struct spi_master *master, struct spi_board_info *chip) @@ -208,7 +210,20 @@ struct spi_device *spi_new_device(struct spi_master *master, struct device *dev = master->cdev.dev; int status; - /* NOTE: caller did any chip->bus_num checks necessary */ + /* NOTE: caller did any chip->bus_num checks necessary. + * + * Also, unless we change the return value convention to use + * error-or-pointer (not NULL-or-pointer), troubleshootability + * suggests syslogged diagnostics are best here (ugh). + */ + + /* Chipselects are numbered 0..max; validate. */ + if (chip->chip_select >= master->num_chipselect) { + dev_err(dev, "cs%d > max %d\n", + chip->chip_select, + master->num_chipselect); + return NULL; + } if (!spi_master_get(master)) return NULL; @@ -236,10 +251,10 @@ struct spi_device *spi_new_device(struct spi_master *master, proxy->controller_state = NULL; proxy->dev.release = spidev_release; - /* drivers may modify this default i/o setup */ + /* drivers may modify this initial i/o setup */ status = master->setup(proxy); if (status < 0) { - dev_dbg(dev, "can't %s %s, status %d\n", + dev_err(dev, "can't %s %s, status %d\n", "setup", proxy->dev.bus_id, status); goto fail; } @@ -249,7 +264,7 @@ struct spi_device *spi_new_device(struct spi_master *master, */ status = device_register(&proxy->dev); if (status < 0) { - dev_dbg(dev, "can't %s %s, status %d\n", + dev_err(dev, "can't %s %s, status %d\n", "add", proxy->dev.bus_id, status); goto fail; } @@ -306,7 +321,6 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n) static void scan_boardinfo(struct spi_master *master) { struct boardinfo *bi; - struct device *dev = master->cdev.dev; mutex_lock(&board_lock); list_for_each_entry(bi, &board_list, list) { @@ -316,17 +330,9 @@ static void scan_boardinfo(struct spi_master *master) for (n = bi->n_board_info; n > 0; n--, chip++) { if (chip->bus_num != master->bus_num) continue; - /* some controllers only have one chip, so they - * might not use chipselects. otherwise, the - * chipselects are numbered 0..max. + /* NOTE: this relies on spi_new_device to + * issue diagnostics when given bogus inputs */ - if (chip->chip_select >= master->num_chipselect - && master->num_chipselect) { - dev_dbg(dev, "cs%d > max %d\n", - chip->chip_select, - master->num_chipselect); - continue; - } (void) spi_new_device(master, chip); } } @@ -419,8 +425,17 @@ int spi_register_master(struct spi_master *master) if (!dev) return -ENODEV; + /* even if it's just one always-selected device, there must + * be at least one chipselect + */ + if (master->num_chipselect == 0) + return -EINVAL; + /* convention: dynamically assigned bus IDs count down from the max */ if (master->bus_num < 0) { + /* FIXME switch to an IDR based scheme, something like + * I2C now uses, so we can't run out of "dynamic" IDs + */ master->bus_num = atomic_dec_return(&dyn_bus_id); dynamic = 1; } |