Commit 082c8cb4 authored by David Brownell's avatar David Brownell Committed by Linus Torvalds
Browse files

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: default avatarDavid Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 2604288f

drivers/spi/spi.c

+30 −15
Original line number Original line Diff line number Diff line
@@ -200,6 +200,8 @@ static DEFINE_MUTEX(board_lock); 
 * platforms may not be able to use spi_register_board_info though, and

 * 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

 * 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).

 * 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_device *spi_new_device(struct spi_master *master,

				  struct spi_board_info *chip)

				  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;

	struct device		*dev = master->cdev.dev;

	int			status;

	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))

	if (!spi_master_get(master))

		return NULL;

		return NULL;
@@ -236,10 +251,10 @@ struct spi_device *spi_new_device(struct spi_master *master, 
	proxy->controller_state = NULL;

	proxy->controller_state = NULL;

	proxy->dev.release = spidev_release;

	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);

	status = master->setup(proxy);

	if (status < 0) {

	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);

				"setup", proxy->dev.bus_id, status);

		goto fail;

		goto fail;

	}

	}
@@ -249,7 +264,7 @@ struct spi_device *spi_new_device(struct spi_master *master, 
	 */

	 */

	status = device_register(&proxy->dev);

	status = device_register(&proxy->dev);

	if (status < 0) {

	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);

				"add", proxy->dev.bus_id, status);

		goto fail;

		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)

static void scan_boardinfo(struct spi_master *master)

{

{

	struct boardinfo	*bi;

	struct boardinfo	*bi;

	struct device		*dev = master->cdev.dev;





	mutex_lock(&board_lock);

	mutex_lock(&board_lock);

	list_for_each_entry(bi, &board_list, list) {

	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++) {

		for (n = bi->n_board_info; n > 0; n--, chip++) {

			if (chip->bus_num != master->bus_num)

			if (chip->bus_num != master->bus_num)

				continue;

				continue;

			/* some controllers only have one chip, so they

			/* NOTE: this relies on spi_new_device to

			 * might not use chipselects.  otherwise, the

			 * issue diagnostics when given bogus inputs

			 * chipselects are numbered 0..max.

			 */

			 */

			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);

			(void) spi_new_device(master, chip);

		}

		}

	}

	}
@@ -419,8 +425,17 @@ int spi_register_master(struct spi_master *master) 
	if (!dev)

	if (!dev)

		return -ENODEV;

		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 */

	/* convention:  dynamically assigned bus IDs count down from the max */

	if (master->bus_num < 0) {

	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);

		master->bus_num = atomic_dec_return(&dyn_bus_id);

		dynamic = 1;

		dynamic = 1;

	}

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE