Skip to contentSkip to conversation

spi: enable spi_board_info to be registered after spi_master

PATCH Superseded Feng Tang 2 participants 2 messages Aug 2, 2010 SPI

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Feng Tang

author
#0 lore raw

PATCH v4 1/1 spi: enable spi_board_info to be registered after spi_master

Hi All,

Pls review this patch. thanks,

-Feng

Changelog:
v4:
* Abstract a spi_match_master_to_boardinfo() which will be called in scanning master/boardinfo list, suggested by Grant also.
v3:
* As suggested by Grant, use one lock to protect the match between spi_master and spi board info
v2:
* fix a typo in looping scan_masterlist()
* fix one race condition found by Grant

Quoted 1 line

From 799ff94edc90117afdbba223fb35cb36a1d2b4b4 Mon Sep 17 00:00:00 2001

From: Feng Tang <[email protected]>
Date: Tue, 27 Jul 2010 17:15:05 +0800
Subject: [PATCH] spi: enable spi_board_info to be registered after spi_master

Currently spi_register_board_info() has to be called before its related spi_master be registered, otherwise these board info will be just ignored.

This patch will remove this order limit, it adds a global spi master list like the existing global board info listr. Whenever a board info or a spi_master is registered, the spi master list or board info list will be scanned, and a new spi device will be created if there is a master-board info match.

Cc David Brownell <[email protected]>
Cc Grant Likely <grant.likely-s3s/[email protected]>
---
 drivers/spi/spi.c       |   86 +++++++++++++++++++++++++++++++----------------
 include/linux/spi/spi.h |    3 ++
 2 files changed, 60 insertions(+), 29 deletions(-)
drivers/spi/spi.c +57 -29
index b3a1f92..70fb141 100644
@@ -196,11 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
 
 struct boardinfo {
 	struct list_head	list;
-	unsigned		n_board_info;
-	struct spi_board_info	board_info[0];
+	struct spi_board_info	board_info;
 };
 
 static LIST_HEAD(board_list);
+static LIST_HEAD(spi_master_list);
+
+/*
+ * Used to protect add/del opertion for board_info list and
+ * spi_master list, and their matching process
+ */
 static DEFINE_MUTEX(board_lock);
 
 /**
@@ -365,6 +370,30 @@ struct spi_device *spi_new_device(struct spi_master *master,
 }
 EXPORT_SYMBOL_GPL(spi_new_device);
 
+static void spi_match_master_to_boardinfo(struct spi_master *master,
+				struct spi_board_info *bi)
+{
+	struct spi_device *dev;
+
+	if (master->bus_num != bi->bus_num)
+		return;
+
+	dev = spi_new_device(master, bi);
+	if (!dev)
+		dev_err(master->dev.parent, "can't create new device for %s\n",
+			bi->modalias);
+}
+
+
+/* Has to be called when board_lock is acquired */
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+	struct spi_master *master;
+
+	list_for_each_entry(master, &spi_master_list, list)
+		spi_match_master_to_boardinfo(master, bi);
+}
+
 /**
  * spi_register_board_info - register SPI devices for a given board
  * @info: array of chip descriptors
@@ -387,43 +416,36 @@ EXPORT_SYMBOL_GPL(spi_new_device);
 int __init
 spi_register_board_info(struct spi_board_info const *info, unsigned n)
 {
-	struct boardinfo	*bi;
+	struct boardinfo *bi;
+	int i;
 
-	bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+	bi = kzalloc(n * sizeof(*bi), GFP_KERNEL);
 	if (!bi)
 		return -ENOMEM;
-	bi->n_board_info = n;
-	memcpy(bi->board_info, info, n * sizeof *info);
 
-	mutex_lock(&board_lock);
-	list_add_tail(&bi->list, &board_list);
-	mutex_unlock(&board_lock);
+	for (i = 0; i < n; i++, bi++, info++) {
+		memcpy(&bi->board_info, info, sizeof(*info));
+		mutex_lock(&board_lock);
+		list_add_tail(&bi->list, &board_list);
+		spi_scan_masterlist(&bi->board_info);
+		mutex_unlock(&board_lock);
+	}
+
 	return 0;
 }
 
-/* FIXME someone should add support for a __setup("spi", ...) that
+/*
+ * FIXME someone should add support for a __setup("spi", ...) that
  * creates board info from kernel command lines
+ *
+ * Should be called when board_lock is acquired
  */
-
 static void scan_boardinfo(struct spi_master *master)
 {
-	struct boardinfo	*bi;
+	struct boardinfo *bi;
 
-	mutex_lock(&board_lock);
-	list_for_each_entry(bi, &board_list, list) {
-		struct spi_board_info	*chip = bi->board_info;
-		unsigned		n;
-
-		for (n = bi->n_board_info; n > 0; n--, chip++) {
-			if (chip->bus_num != master->bus_num)
-				continue;
-			/* NOTE: this relies on spi_new_device to
-			 * issue diagnostics when given bogus inputs
-			 */
-			(void) spi_new_device(master, chip);
-		}
-	}
-	mutex_unlock(&board_lock);
+	list_for_each_entry(bi, &board_list, list)
+		spi_match_master_to_boardinfo(master, &bi->board_info);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -537,15 +559,17 @@ int spi_register_master(struct spi_master *master)
 	dev_dbg(dev, "registered master %s%s\n", dev_name(&master->dev),
 			dynamic ? " (dynamic)" : "");
 
-	/* populate children from any spi device tables */
+	mutex_lock(&board_lock);
+	list_add_tail(&master->list, &spi_master_list);
 	scan_boardinfo(master);
+	mutex_unlock(&board_lock);
+
 	status = 0;
 done:
 	return status;
 }
 EXPORT_SYMBOL_GPL(spi_register_master);
 
-
 static int __unregister(struct device *dev, void *master_dev)
 {
 	/* note: before about 2.6.14-rc1 this would corrupt memory: */
@@ -568,6 +592,10 @@ void spi_unregister_master(struct spi_master *master)
 {
 	int dummy;
 
+	mutex_lock(&board_lock);
+	list_del(&master->list);
+	mutex_unlock(&board_lock);
+
 	dummy = device_for_each_child(master->dev.parent, &master->dev,
 					__unregister);
 	device_unregister(&master->dev);
include/linux/spi/spi.h +3 -0
index af56071..f4a29b6 100644
@@ -204,6 +204,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
 /**
  * struct spi_master - interface to SPI master controller
  * @dev: device interface to this driver
+ * @list: link with the global spi_master list
  * @bus_num: board-specific (and often SOC-specific) identifier for a
  *	given SPI controller.
  * @num_chipselect: chipselects are used to distinguish individual
@@ -235,6 +236,8 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
 struct spi_master {
 	struct device	dev;
 
+	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,
Signature · 7 lines
1.7.0.4

------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://p.sf.net/sfu/dev2dev-palm
#1 lore raw
Quoted 82 lines · Feng Tang

Hi All,

Pls review this patch. thanks,

-Feng

Changelog:
       v4:
          * Abstract a spi_match_master_to_boardinfo() which will be called in scanning master/boardinfo list, suggested by Grant also.
       v3:
          * As suggested by Grant, use one lock to protect the match between spi_master and spi board info
       v2:
          * fix a typo in looping scan_masterlist()
          * fix one race condition found by Grant

From 799ff94edc90117afdbba223fb35cb36a1d2b4b4 Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Tue, 27 Jul 2010 17:15:05 +0800
Subject: [PATCH] spi: enable spi_board_info to be registered after spi_master

Currently spi_register_board_info() has to be called before its related spi_master be registered, otherwise these board info will be just ignored.

This patch will remove this order limit, it adds a global spi master list like the existing global board info listr. Whenever a board info or a spi_master is registered, the spi master list or board info list will be scanned, and a new spi device will be created if there is a master-board info match.

Cc David Brownell <[email protected]>
Cc Grant Likely <grant.likely-s3s/[email protected]>

---
 drivers/spi/spi.c       |   86 +++++++++++++++++++++++++++++++---------------- include/linux/spi/spi.h |    3 ++

 2 files changed, 60 insertions(+), 29 deletions(-)
drivers/spi/spi.c +20 -2
index b3a1f92..70fb141 100644
@@ -196,11 +196,16 @@ EXPORT_SYMBOL_GPL(spi_register_driver);
 
  struct boardinfo {
        struct list_head        list;
-       unsigned                n_board_info;
-       struct spi_board_info   board_info[0];
+       struct spi_board_info   board_info;
  };
 
  static LIST_HEAD(board_list);
+static LIST_HEAD(spi_master_list);
+
+/*
+ * Used to protect add/del opertion for board_info list and
+ * spi_master list, and their matching process
+ */
  static DEFINE_MUTEX(board_lock);
 
  /**
@@ -365,6 +370,30 @@ struct spi_device *spi_new_device(struct spi_master *master,
  }
  EXPORT_SYMBOL_GPL(spi_new_device);
 
+static void spi_match_master_to_boardinfo(struct spi_master *master,
+                               struct spi_board_info *bi)
+{
+       struct spi_device *dev;
+
+       if (master->bus_num != bi->bus_num)
+               return;
+
+       dev = spi_new_device(master, bi);
+       if (!dev)
+               dev_err(master->dev.parent, "can't create new device for %s\n",
+                       bi->modalias);
+}

drivers/spi/spi.c struct spi_device *spi_new_device(struct spi_master *master, · line 395

Quoted 10 lines
drivers/spi/spi.c +10 -0
+
+
+/* Has to be called when board_lock is acquired */
+static void spi_scan_masterlist(struct spi_board_info *bi)
+{
+       struct spi_master *master;
+
+       list_for_each_entry(master, &spi_master_list, list)
+               spi_match_master_to_boardinfo(master, bi);
+}

Missed the point! You don't need this function anymore. Just roll the list_for_each_entry() directly into spi_register_board_info()!

and...

Quoted 56 lines
drivers/spi/spi.c +1 -0
+
  /**
  * spi_register_board_info - register SPI devices for a given board
  * @info: array of chip descriptors
@@ -387,43 +416,36 @@ EXPORT_SYMBOL_GPL(spi_new_device);

 int __init
 spi_register_board_info(struct spi_board_info const *info, unsigned n) {
-       struct boardinfo        *bi;
+       struct boardinfo *bi;
+       int i;

-       bi = kmalloc(sizeof(*bi) + n * sizeof *info, GFP_KERNEL);
+       bi = kzalloc(n * sizeof(*bi), GFP_KERNEL);
       if (!bi)
               return -ENOMEM;
-       bi->n_board_info = n;
-       memcpy(bi->board_info, info, n * sizeof *info);

-       mutex_lock(&board_lock);
-       list_add_tail(&bi->list, &board_list);
-       mutex_unlock(&board_lock);
+       for (i = 0; i < n; i++, bi++, info++) {
+               memcpy(&bi->board_info, info, sizeof(*info));
+               mutex_lock(&board_lock);
+               list_add_tail(&bi->list, &board_list);
+               spi_scan_masterlist(&bi->board_info);
+               mutex_unlock(&board_lock);
+       }
+
       return 0;
 }

drivers/spi/spi.c +2 -1
-/* FIXME someone should add support for a __setup("spi", ...) that
+/*
+ * FIXME someone should add support for a __setup("spi", ...) that

 * creates board info from kernel command lines
+ *
+ * Should be called when board_lock is acquired */
-
 static void scan_boardinfo(struct spi_master *master) {
-       struct boardinfo        *bi;
+       struct boardinfo *bi;

-       mutex_lock(&board_lock);
-       list_for_each_entry(bi, &board_list, list) {
-               struct spi_board_info   *chip = bi->board_info;
-               unsigned                n;
-
-               for (n = bi->n_board_info; n > 0; n--, chip++) {
-                       if (chip->bus_num != master->bus_num)
-                               continue;
-                       /* NOTE: this relies on spi_new_device to
-                        * issue diagnostics when given bogus inputs

Quoted 8 lines

-                        */
-                       (void) spi_new_device(master, chip);
-               }
-       }
-       mutex_unlock(&board_lock);
+       list_for_each_entry(bi, &board_list, list)
+               spi_match_master_to_boardinfo(master, &bi->board_info); }

This function isn't needed either! Again roll spi_match_master_to_boardinfo() into spi_register_master(). No need to have separate functions for 2 lines of code that aren't called from anywhere else.

g.

Quoted 57 lines

 /*-------------------------------------------------------------------------*/

drivers/spi/spi.c +4 -1
@@ -537,15 +559,17 @@ int spi_register_master(struct spi_master *master)
        dev_dbg(dev, "registered master %s%s\n", dev_name(&master->dev),
                        dynamic ? " (dynamic)" : "");
 
-       /* populate children from any spi device tables */
+       mutex_lock(&board_lock);
+       list_add_tail(&master->list, &spi_master_list);
        scan_boardinfo(master);
+       mutex_unlock(&board_lock);
+
        status = 0;

 done:
       return status;
 }
 EXPORT_SYMBOL_GPL(spi_register_master);

-
 static int __unregister(struct device *dev, void *master_dev) {
       /* note: before about 2.6.14-rc1 this would corrupt memory: */

drivers/spi/spi.c +4 -0
@@ -568,6 +592,10 @@ void spi_unregister_master(struct spi_master *master)
  {
        int dummy;
 
+       mutex_lock(&board_lock);
+       list_del(&master->list);
+       mutex_unlock(&board_lock);
+
        dummy = device_for_each_child(master->dev.parent, &master->dev,
                                        __unregister);
        device_unregister(&master->dev);
include/linux/spi/spi.h +3 -1
index af56071..f4a29b6 100644
@@ -204,6 +204,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  /**
  * struct spi_master - interface to SPI master controller
  * @dev: device interface to this driver
+ * @list: link with the global spi_master list
  * @bus_num: board-specific (and often SOC-specific) identifier for a
  *     given SPI controller.
  * @num_chipselect: chipselects are used to distinguish individual
@@ -235,6 +236,8 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  struct spi_master {
        struct device   dev;
 
+       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,
--

1.7.0.4

Signature · 8 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://p.sf.net/sfu/dev2dev-palm