* [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver @ 2008-05-16 19:35 Grant Likely 2008-05-16 19:36 ` [PATCH 1/4] spi: Change modalias from a pointer to a character array Grant Likely ` (4 more replies) 0 siblings, 5 replies; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:35 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell Cc: fabrizio.garetto, jonsmirl This series is a set of changes to allow the slaves on an SPI bus to be described in the OF device tree (useful in arch/powerpc) and adds a driver that uses it (the Freescale MPC5200 SoC's SPI device). Please review and comment. David, I've included in this series my earlier patch to change modalias from a pointer to a string as one of the later patches depends on it. Cheers, g. -- Grant Likely, B.Sc. P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 1/4] spi: Change modalias from a pointer to a character array 2008-05-16 19:35 [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Grant Likely @ 2008-05-16 19:36 ` Grant Likely 2008-05-16 19:36 ` [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration Grant Likely ` (3 subsequent siblings) 4 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:36 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell Cc: fabrizio.garetto, jonsmirl From: Grant Likely <grant.likely@secretlab.ca> Currently, 'modalias' in the spi_device structure is a 'const char *'. The spi_new_device() function fills in the modalias value from a passed in spi_board_info data block. Since it is a pointer copy, the new spi_device remains dependent on the spi_board_info structure after the new spi_device is registered (no other fields in spi_device directly depend on the spi_board_info structure; all of the other data is copied). This causes a problem when dynamically propulating the list of attached SPI devices. For example, in arch/powerpc, the list of SPI devices can be populated from data in the device tree. With the current code, the device tree adapter must kmalloc() a new spi_board_info structure for each new SPI device it finds in the device tree, and there is no simple mechanism in place for keeping track of these allocations. This patch changes modalias from a 'const char *' to a fixed char array. By copying the modalias string instead of referencing it, the dependency on the spi_board_info structure is eliminated and an outside caller does not need to maintain a separate spi_board_info allocation for each device. If searched through the code to the best of my ability for any references to modalias which may be affected by this change and haven't found anything. It has been tested with the lite5200b platform in arch/powerpc. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> --- drivers/spi/spi.c | 2 +- include/linux/spi/spi.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 1ad12af..bdf1b70 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -229,7 +229,7 @@ struct spi_device *spi_new_device(struct spi_master *master, proxy->max_speed_hz = chip->max_speed_hz; proxy->mode = chip->mode; proxy->irq = chip->irq; - proxy->modalias = chip->modalias; + strncpy(proxy->modalias, chip->modalias, KOBJ_NAME_LEN); snprintf(proxy->dev.bus_id, sizeof proxy->dev.bus_id, "%s.%u", master->dev.bus_id, diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 387e428..38a080b 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -82,7 +82,7 @@ struct spi_device { int irq; void *controller_state; void *controller_data; - const char *modalias; + char modalias[KOBJ_NAME_LEN]; /* * likely need more hooks for more protocol options affecting how ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. 2008-05-16 19:35 [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Grant Likely 2008-05-16 19:36 ` [PATCH 1/4] spi: Change modalias from a pointer to a character array Grant Likely @ 2008-05-16 19:36 ` Grant Likely [not found] ` <20080516193608.28030.34968.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-16 19:36 ` [PATCH 3/4] spi: Add OF binding support for SPI busses Grant Likely ` (2 subsequent siblings) 4 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:36 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell Cc: fabrizio.garetto, jonsmirl From: Grant Likely <grant.likely@secretlab.ca> spi_new_device() allocates and registers an spi device all in one swoop. If the driver needs to add extra data to the spi_device before it is registered, then this causes problems. This patch splits the allocation and registration portions of code out of spi_new_device() and creates three new functions; spi_alloc_device(), spi_register_device(), and spi_device_release(). spi_new_device() is modified to use the new functions for allocation and registration. None of the existing users of spi_new_device() should be affected by this change. Drivers using the new API can forego the use of an spi_board_info structure to describe the device layout and populate data into the spi_device structure directly. This change is in preparation for adding an OF device tree parser to generate spi_devices based on data in the device tree. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> --- drivers/spi/spi.c | 150 +++++++++++++++++++++++++++++++++-------------- include/linux/spi/spi.h | 13 ++++ 2 files changed, 119 insertions(+), 44 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index bdf1b70..9c7a84d 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -178,6 +178,107 @@ struct boardinfo { static LIST_HEAD(board_list); static DEFINE_MUTEX(board_lock); +/** + * spi_alloc_device - Allocate a new SPI device + * @master: Controller to which device is connected + * Context: can sleep + * + * Allows a driver to allocate and initialize and spi_device without + * registering it immediately. This allows a driver to directly + * fill the spi_device with device parameters before calling + * spi_register_device() on it. + * + * Caller is responsible to call spi_register_device on the returned + * spi_device structure. + * + * Returns a pointer to the new device, or NULL. + */ +struct spi_device *spi_alloc_device(struct spi_master *master) +{ + struct spi_device *spi; + struct device *dev = master->dev.parent; + + if (!spi_master_get(master)) + return NULL; + + spi = kzalloc(sizeof *spi, GFP_KERNEL); + if (!spi) { + dev_err(dev, "cannot alloc spi_device\n"); + spi_master_put(master); + return NULL; + } + + spi->master = master; + spi->dev.parent = dev; + spi->dev.bus = &spi_bus_type; + spi->dev.release = spidev_release; + return spi; +} +EXPORT_SYMBOL_GPL(spi_alloc_device); + +/** + * spi_register_device - Register an spi_device allocated with spi_alloc_device + * @spi: spi_device to register + * + * Companion function to spi_alloc_device. Devices allocated with + * spi_alloc_device can be registerd onto the spi bus with this function. + * + * Returns 0 on success; non-zero on failure + */ +int spi_register_device(struct spi_device *spi) +{ + struct device *dev = spi->master->dev.parent; + int status; + + /* Chipselects are numbered 0..max; validate. */ + if (spi->chip_select >= spi->master->num_chipselect) { + dev_err(dev, "cs%d > max %d\n", + spi->chip_select, + spi->master->num_chipselect); + return -EINVAL; + } + + /* Set the bus ID string */ + snprintf(spi->dev.bus_id, sizeof spi->dev.bus_id, + "%s.%u", spi->master->dev.bus_id, + spi->chip_select); + + /* drivers may modify this initial i/o setup */ + status = spi->master->setup(spi); + if (status < 0) { + dev_err(dev, "can't %s %s, status %d\n", + "setup", spi->dev.bus_id, status); + return status; + } + + /* driver core catches callers that misbehave by defining + * devices that already exist. + */ + status = device_register(&spi->dev); + if (status < 0) { + dev_err(dev, "can't %s %s, status %d\n", + "add", spi->dev.bus_id, status); + return status; + } + + dev_dbg(dev, "registered child %s\n", spi->dev.bus_id); + return 0; +} +EXPORT_SYMBOL_GPL(spi_register_device); + +/** + * spi_device_release - Free an allocated spi_device structure + * @spi: spi device to free + * + * Call this to free an spi_device allocated with spi_alloc_device(). Caller + * is responsible to ensure that the device has been unregistered first. + */ +void spi_device_release(struct spi_device *spi) +{ + spi_master_put(spi->master); + kfree(spi); +} +EXPORT_SYMBOL_GPL(spi_device_release); /** * spi_new_device - instantiate one new SPI device @@ -197,7 +298,6 @@ struct spi_device *spi_new_device(struct spi_master *master, struct spi_board_info *chip) { struct spi_device *proxy; - struct device *dev = master->dev.parent; int status; /* NOTE: caller did any chip->bus_num checks necessary. @@ -207,64 +307,26 @@ struct spi_device *spi_new_device(struct spi_master *master, * 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)) + proxy = spi_alloc_device(master); + if (!proxy) return NULL; - proxy = kzalloc(sizeof *proxy, GFP_KERNEL); - if (!proxy) { - dev_err(dev, "can't alloc dev for cs%d\n", - chip->chip_select); - goto fail; - } - proxy->master = master; proxy->chip_select = chip->chip_select; proxy->max_speed_hz = chip->max_speed_hz; proxy->mode = chip->mode; proxy->irq = chip->irq; strncpy(proxy->modalias, chip->modalias, KOBJ_NAME_LEN); - - snprintf(proxy->dev.bus_id, sizeof proxy->dev.bus_id, - "%s.%u", master->dev.bus_id, - chip->chip_select); - proxy->dev.parent = dev; - proxy->dev.bus = &spi_bus_type; proxy->dev.platform_data = (void *) chip->platform_data; proxy->controller_data = chip->controller_data; proxy->controller_state = NULL; - proxy->dev.release = spidev_release; - /* drivers may modify this initial i/o setup */ - status = master->setup(proxy); + status = spi_register_device(proxy); if (status < 0) { - dev_err(dev, "can't %s %s, status %d\n", - "setup", proxy->dev.bus_id, status); - goto fail; + spi_device_release(proxy); + return NULL; } - /* driver core catches callers that misbehave by defining - * devices that already exist. - */ - status = device_register(&proxy->dev); - if (status < 0) { - dev_err(dev, "can't %s %s, status %d\n", - "add", proxy->dev.bus_id, status); - goto fail; - } - dev_dbg(dev, "registered child %s\n", proxy->dev.bus_id); return proxy; - -fail: - spi_master_put(master); - kfree(proxy); - return NULL; } EXPORT_SYMBOL_GPL(spi_new_device); diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 38a080b..ca7c933 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -778,8 +778,21 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n) * use spi_new_device() to describe each device. You can also call * spi_unregister_device() to start making that device vanish, but * normally that would be handled by spi_unregister_master(). + * + * You can also use spi_alloc_device() and spi_register_device() to + * for a two stage registration of an SPI device. This gives the caller + * some more control over the spi_device structure before it is registered */ extern struct spi_device * +spi_alloc_device(struct spi_master *master); + +extern int +spi_register_device(struct spi_device *spi); + +extern void +spi_device_release(struct spi_device *spi); + +extern struct spi_device * spi_new_device(struct spi_master *, struct spi_board_info *); static inline void ^ permalink raw reply related [flat|nested] 50+ messages in thread
[parent not found: <20080516193608.28030.34968.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org>]
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <20080516193608.28030.34968.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> @ 2008-05-22 0:17 ` David Brownell [not found] ` <200805211717.13206.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-22 0:17 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Friday 16 May 2008, Grant Likely wrote: > > This patch splits the allocation and registration portions of code out > of spi_new_device() and creates three new functions; spi_alloc_device(), > spi_register_device(), and spi_device_release(). I have no problem with the first two, but why the last? If the devices are always allocated by spi_alloc_device() as they should be -- probably through an intermediary -- the only public function necessary for that cleanup should be the existing spi_dev_put(). - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805211717.13206.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <200805211717.13206.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 6:43 ` Grant Likely [not found] ` <fa686aa40805232343x20031560j5659d203e25f494-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:43 UTC (permalink / raw) To: David Brownell Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Wed, May 21, 2008 at 6:17 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Friday 16 May 2008, Grant Likely wrote: >> >> This patch splits the allocation and registration portions of code out >> of spi_new_device() and creates three new functions; spi_alloc_device(), >> spi_register_device(), and spi_device_release(). > > I have no problem with the first two, but why the last? > > If the devices are always allocated by spi_alloc_device() as > they should be -- probably through an intermediary -- the > only public function necessary for that cleanup should be > the existing spi_dev_put(). Ah, okay. I'm still a bit fuzzy on the device model conventions. I'll remove that then. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805232343x20031560j5659d203e25f494-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <fa686aa40805232343x20031560j5659d203e25f494-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 6:54 ` Grant Likely [not found] ` <fa686aa40805232354g147acfcdx4753fce1a448ceb7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-06-17 7:28 ` Grant Likely 1 sibling, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:54 UTC (permalink / raw) To: David Brownell Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 24, 2008 at 12:43 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Wed, May 21, 2008 at 6:17 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: >> On Friday 16 May 2008, Grant Likely wrote: >>> >>> This patch splits the allocation and registration portions of code out >>> of spi_new_device() and creates three new functions; spi_alloc_device(), >>> spi_register_device(), and spi_device_release(). >> >> I have no problem with the first two, but why the last? >> >> If the devices are always allocated by spi_alloc_device() as >> they should be -- probably through an intermediary -- the >> only public function necessary for that cleanup should be >> the existing spi_dev_put(). > > Ah, okay. I'm still a bit fuzzy on the device model conventions. > I'll remove that then. Question: spi_alloc_device() (and the original code) does a spi_master_get() on the spi_master device. Doesn't spi_master_put() need to be called when the device is discarded? spi_dev_put() doesn't do that explicitly; is it an implicit operation after a device has been deregistered from the spi_master? Thanks, g. > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805232354g147acfcdx4753fce1a448ceb7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <fa686aa40805232354g147acfcdx4753fce1a448ceb7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-06-30 4:08 ` David Brownell 0 siblings, 0 replies; 50+ messages in thread From: David Brownell @ 2008-06-30 4:08 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Friday 23 May 2008, Grant Likely wrote: > Question: spi_alloc_device() (and the original code) does a > spi_master_get() on the spi_master device. Doesn't spi_master_put() > need to be called when the device is discarded? spi_dev_put() doesn't > do that explicitly; is it an implicit operation after a device has > been deregistered from the spi_master? Depends whether or not the add() has been done to hook things into the driver model tree, as I recall. The add() presumes things are properly refcounted. When you make a driver model tree node vanish, its associated refcounts get updated too. - Dave ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <fa686aa40805232343x20031560j5659d203e25f494-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 6:54 ` Grant Likely @ 2008-06-17 7:28 ` Grant Likely [not found] ` <fa686aa40806170028t2ccb679k22d2d3cea793ebc1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-06-17 7:28 UTC (permalink / raw) To: David Brownell Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 24, 2008 at 12:43 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Wed, May 21, 2008 at 6:17 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: >> On Friday 16 May 2008, Grant Likely wrote: >>> >>> This patch splits the allocation and registration portions of code out >>> of spi_new_device() and creates three new functions; spi_alloc_device(), >>> spi_register_device(), and spi_device_release(). >> >> I have no problem with the first two, but why the last? >> >> If the devices are always allocated by spi_alloc_device() as >> they should be -- probably through an intermediary -- the >> only public function necessary for that cleanup should be >> the existing spi_dev_put(). > > Ah, okay. I'm still a bit fuzzy on the device model conventions. > I'll remove that then. I've dug into this some more. spi_alloc_device only allocates the memory. It doesn't call device_initialize() to initialize the kref. All of that behaviour is handled within device_register(). Therefore if a driver uses spi_alloc_device() and then if a later part of the initialization fails before spi_register_device() is called, then the alloc'd memory needs to be freed, but spi_dev_put() won't work because the kobj isn't set up so I need another function to handle freeing it in on a failure path. Should I switch things around to do device_initialize() in the alloc function and call device_add() instead of device_register() in the spi_register_device() function? Is that sufficient to make put_device() work? Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40806170028t2ccb679k22d2d3cea793ebc1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration. [not found] ` <fa686aa40806170028t2ccb679k22d2d3cea793ebc1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-06-30 4:10 ` David Brownell 0 siblings, 0 replies; 50+ messages in thread From: David Brownell @ 2008-06-30 4:10 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Tuesday 17 June 2008, Grant Likely wrote: > >>> This patch splits the allocation and registration portions of code out > >>> of spi_new_device() and creates three new functions; spi_alloc_device(), > >>> spi_register_device(), and spi_device_release(). > >> > >> I have no problem with the first two, but why the last? > >> > >> If the devices are always allocated by spi_alloc_device() as > >> they should be -- probably through an intermediary -- the > >> only public function necessary for that cleanup should be > >> the existing spi_dev_put(). > > > > Ah, okay. I'm still a bit fuzzy on the device model conventions. > > I'll remove that then. > > I've dug into this some more. spi_alloc_device only allocates the > memory. It doesn't call device_initialize() to initialize the kref. Well, the driver model idiom is initialize() then add(), with register() calls combining the two. An alloc() is just a bit outside those core idioms ... But one alloc() example is platform_device_alloc(), which does the device_initialize() call ... followed by platform_device_add(). The spi_new_device() call does a bunch of stuff beyond a register(), but it also calls device_register(). > All of that behaviour is handled within device_register(). Therefore > if a driver uses spi_alloc_device() and then if a later part of the > initialization fails before spi_register_device() is called, then the > alloc'd memory needs to be freed, but spi_dev_put() won't work because > the kobj isn't set up so I need another function to handle freeing it > in on a failure path. I see ... > Should I switch things around to do device_initialize() in the alloc > function Yes. > and call device_add() instead of device_register() in the > spi_register_device() function? You should also rename it to spi_add_device(), since register() calls always do the initialize() rather than having it done for them in advance. People rely on those names supporting that pattern (as they should). > Is that sufficient to make put_device() work? Looks like it to me. Calling device_initialize() will do a kobject_init(), which is documented as requiring a kobject_put() to clean up ... that's all put_device() will ever do. - Dave ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-16 19:35 [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Grant Likely 2008-05-16 19:36 ` [PATCH 1/4] spi: Change modalias from a pointer to a character array Grant Likely 2008-05-16 19:36 ` [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration Grant Likely @ 2008-05-16 19:36 ` Grant Likely [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> [not found] ` <200805221915.59878.david-b@pacbell.net> 2008-05-16 19:36 ` [PATCH 4/4] [CSB] Add new mpc5200-spi (non-psc) device driver Grant Likely [not found] ` <20080516193054.28030.35126.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 4 siblings, 2 replies; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:36 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell Cc: fabrizio.garetto, jonsmirl From: Grant Likely <grant.likely@secretlab.ca> This patch adds support for populating an SPI bus based on data in the OF device tree. This is useful for powerpc platforms which use the device tree instead of discrete code for describing platform layout. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> --- Documentation/powerpc/booting-without-of.txt | 61 ++++++++++++++++++ drivers/spi/Kconfig | 4 + drivers/spi/Makefile | 1 drivers/spi/spi_of.c | 86 ++++++++++++++++++++++++++ include/linux/spi/spi_of.h | 18 +++++ 5 files changed, 170 insertions(+), 0 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 1d2a772..452c242 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -58,6 +58,7 @@ Table of Contents o) Xilinx IP cores p) Freescale Synchronous Serial Interface q) USB EHCI controllers + s) SPI busses VII - Marvell Discovery mv64[345]6x System Controller chips 1) The /system-controller node @@ -2870,6 +2871,66 @@ platforms are moved over to use the flattened-device-tree model. reg = <0xe8000000 32>; }; + s) SPI (Serial Peripheral Interface) busses + + SPI busses can be described with a node for the SPI master device + and a set of child nodes for each SPI slave on the bus. For this + discussion, it is assumed that the system's SPI controller is in + SPI master mode. This binding does not describe SPI controllers + in slave mode. + + The SPI master node requires the following properties: + - #address-cells - number of cells required to define a chip select + address on the SPI bus. + - #size-cells - should be zero. + - compatible - name of SPI bus controller following generic names + recommended practice. + No other properties are required in the spi bus node. It is assumed + that a driver for an SPI bus device will understand that it is an SPI bus. + However, the binding does not attempt to define the specific method for + assigning chip select numbers. Since SPI chip select configuration is + flexible and non-standardized, it is left out of this binding with the + assumption that board specific platform code will be used to manage + chip selects. Individual drivers can define additional properties to + support describing the chip select layout. + + SPI slave nodes must be children of the spi master node and can + contain the following properties. + - reg - (required) chip select address of device. + - compatible - (required) name of SPI device following generic names + recommended practice + - max-speed - (optional) Maximum SPI clocking speed of device in Hz + - spi,cpol - (optional) Device requires inverse clock polarity + - spi,cpha - (optional) Device requires shifted clock phase + - linux,modalias - (optional, Linux specific) Force binding of SPI device + to a particular spi_device driver. Useful for changing + driver binding between spidev and a kernel spi driver. + + SPI example for an MPC5200 SPI bus: + spi@f00 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; + reg = <0xf00 0x20>; + interrupts = <2 13 0 2 14 0>; + interrupt-parent = <&mpc5200_pic>; + + ethernet-switch@0 { + compatible = "micrel,ks8995m"; + linux,modalias = "ks8995"; + max-speed = <1000000>; + reg = <0>; + }; + + codec@1 { + compatible = "ti,tlv320aic26"; + max-speed = <100000>; + reg = <1>; + }; + }; + + + VII - Marvell Discovery mv64[345]6x System Controller chips =========================================================== diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 66ec5d8..12c35da 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -49,6 +49,10 @@ config SPI_MASTER controller and the protocol drivers for the SPI slave chips that are connected. +# OpenFirmware device tree support +config SPI_MASTER_OF + bool + comment "SPI Master Controller Drivers" depends on SPI_MASTER diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 7fca043..29c592f 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -9,6 +9,7 @@ endif # small core, mostly translating board-specific # config declarations into driver model code obj-$(CONFIG_SPI_MASTER) += spi.o +obj-$(CONFIG_SPI_MASTER_OF) += spi_of.o # SPI master controller drivers (bus) obj-$(CONFIG_SPI_ATMEL) += atmel_spi.o diff --git a/drivers/spi/spi_of.c b/drivers/spi/spi_of.c new file mode 100644 index 0000000..b5ae434 --- /dev/null +++ b/drivers/spi/spi_of.c @@ -0,0 +1,86 @@ +/* + * SPI OF support routines + * Copyright (C) 2008 Secret Lab Technologies Ltd. + * + * Support routines for deriving SPI device attachments from the device + * tree. + */ + +#include <linux/of.h> +#include <linux/device.h> +#include <linux/spi/spi.h> +#include <linux/spi/spi_of.h> + +/** + * spi_of_register_devices - Register child devices onto the SPI bus + * @master: Pointer to spi_master device + * @np: parent node of SPI device nodes + * + * Registers an spi_device for each child node of 'np' which has a 'reg' + * property. + */ +void spi_of_register_devices(struct spi_master *master, struct device_node *np) +{ + struct spi_device *spi; + struct device_node *nc; + const u32 *prop; + const char *sprop; + int rc; + int len; + + for_each_child_of_node(np, nc) { + /* Alloc an spi_device */ + spi = spi_alloc_device(master); + if (!spi) { + dev_err(&master->dev, "spi_device alloc error for %s\n", + np->full_name); + continue; + } + + /* Device address */ + prop = of_get_property(nc, "reg", &len); + if (!prop || len < sizeof(*prop)) { + dev_err(&master->dev, "%s has no 'reg' property\n", + np->full_name); + continue; + } + spi->chip_select = *prop; + + /* Mode (clock phase/polarity/etc. */ + if (of_find_property(nc, "spi,cpha", NULL)) + spi->mode |= SPI_CPHA; + if (of_find_property(nc, "spi,cpol", NULL)) + spi->mode |= SPI_CPOL; + + /* Device speed */ + prop = of_get_property(nc, "max-speed", &len); + if (prop && len >= sizeof(*prop)) + spi->max_speed_hz = *prop; + else + spi->max_speed_hz = 100000; + + /* IRQ */ + spi->irq = irq_of_parse_and_map(nc, 0); + + /* Select device driver */ + sprop = of_get_property(nc, "linux,modalias", &len); + if (sprop && len > 0) + strncpy(spi->modalias, sprop, KOBJ_NAME_LEN); + else + strncpy(spi->modalias, "spidev", KOBJ_NAME_LEN); + + /* Store a pointer to the node in the device structure */ + of_node_get(nc); + spi->dev.archdata.of_node = nc; + + /* Register the new device */ + rc = spi_register_device(spi); + if (rc) { + dev_err(&master->dev, "spi_device register error %s\n", + np->full_name); + spi_device_release(spi); + } + + } +} +EXPORT_SYMBOL(spi_of_register_devices); diff --git a/include/linux/spi/spi_of.h b/include/linux/spi/spi_of.h new file mode 100644 index 0000000..c943f98 --- /dev/null +++ b/include/linux/spi/spi_of.h @@ -0,0 +1,18 @@ +/* + * SPI OF support routines + * Copyright (C) 2008 Secret Lab Technologies Ltd. + * + * Support routines for deriving SPI device attachments from the device + * tree. + */ + +#ifndef __LINUX_SPI_OF_H +#define __LINUX_SPI_OF_H + +#include <linux/of.h> +#include <linux/spi/spi.h> + +extern void spi_of_register_devices(struct spi_master *master, + struct device_node *np); + +#endif /* __LINUX_SPI_OF */ ^ permalink raw reply related [flat|nested] 50+ messages in thread
[parent not found: <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> @ 2008-05-16 20:47 ` Randy Dunlap 2008-05-16 20:51 ` Grant Likely 2008-05-16 22:03 ` Anton Vorontsov ` (2 subsequent siblings) 3 siblings, 1 reply; 50+ messages in thread From: Randy Dunlap @ 2008-05-16 20:47 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, 16 May 2008 13:36:13 -0600 Grant Likely wrote: > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt > index 1d2a772..452c242 100644 > --- a/Documentation/powerpc/booting-without-of.txt > +++ b/Documentation/powerpc/booting-without-of.txt > @@ -2870,6 +2871,66 @@ platforms are moved over to use the flattened-device-tree model. > reg = <0xe8000000 32>; > }; > > + s) SPI (Serial Peripheral Interface) busses > + > + SPI busses can be described with a node for the SPI master device > + and a set of child nodes for each SPI slave on the bus. For this > + discussion, it is assumed that the system's SPI controller is in > + SPI master mode. This binding does not describe SPI controllers > + in slave mode. > + > + The SPI master node requires the following properties: > + - #address-cells - number of cells required to define a chip select > + address on the SPI bus. > + - #size-cells - should be zero. > + - compatible - name of SPI bus controller following generic names > + recommended practice. > + No other properties are required in the spi bus node. It is assumed ~~~ > + that a driver for an SPI bus device will understand that it is an SPI bus. > + However, the binding does not attempt to define the specific method for > + assigning chip select numbers. Since SPI chip select configuration is > + flexible and non-standardized, it is left out of this binding with the > + assumption that board specific platform code will be used to manage > + chip selects. Individual drivers can define additional properties to > + support describing the chip select layout. > + > + SPI slave nodes must be children of the spi master node and can ~~~ > + contain the following properties. > + - reg - (required) chip select address of device. > + - compatible - (required) name of SPI device following generic names > + recommended practice > + - max-speed - (optional) Maximum SPI clocking speed of device in Hz > + - spi,cpol - (optional) Device requires inverse clock polarity > + - spi,cpha - (optional) Device requires shifted clock phase > + - linux,modalias - (optional, Linux specific) Force binding of SPI device > + to a particular spi_device driver. Useful for changing > + driver binding between spidev and a kernel spi driver. ~~~ Hi, You mostly capitalize "SPI" in sentences (i.e., when it's not part of a function name or OF data), so could the 3 underlined instances of it also be all caps? Thanks, --- ~Randy ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-16 20:47 ` Randy Dunlap @ 2008-05-16 20:51 ` Grant Likely 0 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-16 20:51 UTC (permalink / raw) To: Randy Dunlap Cc: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell, fabrizio.garetto, jonsmirl On Fri, May 16, 2008 at 2:47 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote: > On Fri, 16 May 2008 13:36:13 -0600 Grant Likely wrote: > >> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt >> index 1d2a772..452c242 100644 >> --- a/Documentation/powerpc/booting-without-of.txt >> +++ b/Documentation/powerpc/booting-without-of.txt >> @@ -2870,6 +2871,66 @@ platforms are moved over to use the flattened-device-tree model. >> reg = <0xe8000000 32>; >> }; >> >> + s) SPI (Serial Peripheral Interface) busses >> + >> + SPI busses can be described with a node for the SPI master device >> + and a set of child nodes for each SPI slave on the bus. For this >> + discussion, it is assumed that the system's SPI controller is in >> + SPI master mode. This binding does not describe SPI controllers >> + in slave mode. >> + >> + The SPI master node requires the following properties: >> + - #address-cells - number of cells required to define a chip select >> + address on the SPI bus. >> + - #size-cells - should be zero. >> + - compatible - name of SPI bus controller following generic names >> + recommended practice. >> + No other properties are required in the spi bus node. It is assumed > ~~~ > >> + that a driver for an SPI bus device will understand that it is an SPI bus. >> + However, the binding does not attempt to define the specific method for >> + assigning chip select numbers. Since SPI chip select configuration is >> + flexible and non-standardized, it is left out of this binding with the >> + assumption that board specific platform code will be used to manage >> + chip selects. Individual drivers can define additional properties to >> + support describing the chip select layout. >> + >> + SPI slave nodes must be children of the spi master node and can > ~~~ > >> + contain the following properties. >> + - reg - (required) chip select address of device. >> + - compatible - (required) name of SPI device following generic names >> + recommended practice >> + - max-speed - (optional) Maximum SPI clocking speed of device in Hz >> + - spi,cpol - (optional) Device requires inverse clock polarity >> + - spi,cpha - (optional) Device requires shifted clock phase >> + - linux,modalias - (optional, Linux specific) Force binding of SPI device >> + to a particular spi_device driver. Useful for changing >> + driver binding between spidev and a kernel spi driver. > ~~~ > > Hi, > You mostly capitalize "SPI" in sentences (i.e., when it's not part of > a function name or OF data), so could the 3 underlined instances of it > also be all caps? No problem. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-16 20:47 ` Randy Dunlap @ 2008-05-16 22:03 ` Anton Vorontsov 2008-05-16 22:14 ` Grant Likely 2008-05-19 13:17 ` Guennadi Liakhovetski 2008-05-21 15:19 ` Anton Vorontsov 3 siblings, 1 reply; 50+ messages in thread From: Anton Vorontsov @ 2008-05-16 22:03 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote: > From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> > > This patch adds support for populating an SPI bus based on data in the > OF device tree. This is useful for powerpc platforms which use the > device tree instead of discrete code for describing platform layout. > > Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> > --- [...] > +void spi_of_register_devices(struct spi_master *master, struct device_node *np) > +{ > + struct spi_device *spi; > + struct device_node *nc; > + const u32 *prop; > + const char *sprop; > + int rc; > + int len; > + > + for_each_child_of_node(np, nc) { > + /* Alloc an spi_device */ > + spi = spi_alloc_device(master); > + if (!spi) { > + dev_err(&master->dev, "spi_device alloc error for %s\n", > + np->full_name); > + continue; > + } > + > + /* Device address */ > + prop = of_get_property(nc, "reg", &len); > + if (!prop || len < sizeof(*prop)) { > + dev_err(&master->dev, "%s has no 'reg' property\n", > + np->full_name); > + continue; > + } > + spi->chip_select = *prop; > + > + /* Mode (clock phase/polarity/etc. */ > + if (of_find_property(nc, "spi,cpha", NULL)) > + spi->mode |= SPI_CPHA; > + if (of_find_property(nc, "spi,cpol", NULL)) > + spi->mode |= SPI_CPOL; > + > + /* Device speed */ > + prop = of_get_property(nc, "max-speed", &len); > + if (prop && len >= sizeof(*prop)) > + spi->max_speed_hz = *prop; > + else > + spi->max_speed_hz = 100000; > + > + /* IRQ */ > + spi->irq = irq_of_parse_and_map(nc, 0); > + > + /* Select device driver */ > + sprop = of_get_property(nc, "linux,modalias", &len); > + if (sprop && len > 0) > + strncpy(spi->modalias, sprop, KOBJ_NAME_LEN); > + else > + strncpy(spi->modalias, "spidev", KOBJ_NAME_LEN); > + > + /* Store a pointer to the node in the device structure */ > + of_node_get(nc); > + spi->dev.archdata.of_node = nc; > + > + /* Register the new device */ > + rc = spi_register_device(spi); > + if (rc) { > + dev_err(&master->dev, "spi_device register error %s\n", > + np->full_name); > + spi_device_release(spi); > + } No way to pass platform data... can you suggest any idea to use this for things like "[POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI" I've sent just recently...? Maybe this code could do something like spi->dev.platform_data = nc->data; and board code would fill nc->data at early stages? This needs to be a convention, not just random use though.. Maybe we can expand the struct device_node to explicitly include .platform_data for such cases? Thanks, -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-16 22:03 ` Anton Vorontsov @ 2008-05-16 22:14 ` Grant Likely [not found] ` <fa686aa40805161514r513d0eebt380a76f64abe8434-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-16 22:14 UTC (permalink / raw) To: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 4:03 PM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote: >> + /* Store a pointer to the node in the device structure */ >> + of_node_get(nc); >> + spi->dev.archdata.of_node = nc; >> + >> + /* Register the new device */ >> + rc = spi_register_device(spi); >> + if (rc) { >> + dev_err(&master->dev, "spi_device register error %s\n", >> + np->full_name); >> + spi_device_release(spi); >> + } > > No way to pass platform data... can you suggest any idea to use > this for things like > "[POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI" > I've sent just recently...? That's right. platform_data being a very driver specific thing there is no way to generically extract a pdata structure from the device tree. Instead, I'm storing the device node in archdata.of_node (line immediately above spi_register_device) so that drivers can read the device node themselves to populate a platform_device structure. (Protected by CONFIG_OF of course). > Maybe this code could do something like > spi->dev.platform_data = nc->data; > and board code would fill nc->data at early stages? This needs to be a > convention, not just random use though.. Maybe we can expand the struct > device_node to explicitly include .platform_data for such cases? Hmmm, as you say, this could end up being rather messy. However, by passing the device node pointer, the driver could extract that data on a per case basis. (ie. it would be decided on a per driver basis where to get the platform data). I'm not sure; this bears more thought... Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805161514r513d0eebt380a76f64abe8434-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805161514r513d0eebt380a76f64abe8434-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-16 22:49 ` Anton Vorontsov 2008-05-17 5:02 ` Grant Likely 0 siblings, 1 reply; 50+ messages in thread From: Anton Vorontsov @ 2008-05-16 22:49 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 04:14:23PM -0600, Grant Likely wrote: > On Fri, May 16, 2008 at 4:03 PM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote: > >> + /* Store a pointer to the node in the device structure */ > >> + of_node_get(nc); > >> + spi->dev.archdata.of_node = nc; > >> + > >> + /* Register the new device */ > >> + rc = spi_register_device(spi); > >> + if (rc) { > >> + dev_err(&master->dev, "spi_device register error %s\n", > >> + np->full_name); > >> + spi_device_release(spi); > >> + } > > > > No way to pass platform data... can you suggest any idea to use > > this for things like > > "[POWERPC] 86xx: mpc8610_hpcd: add support for SPI and MMC-over-SPI" > > I've sent just recently...? > > That's right. platform_data being a very driver specific thing there > is no way to generically extract a pdata structure from the device > tree. Instead, I'm storing the device node in archdata.of_node (line > immediately above spi_register_device) so that drivers can read the > device node themselves to populate a platform_device structure. > (Protected by CONFIG_OF of course). > > > Maybe this code could do something like > > spi->dev.platform_data = nc->data; > > and board code would fill nc->data at early stages? This needs to be a > > convention, not just random use though.. Maybe we can expand the struct > > device_node to explicitly include .platform_data for such cases? > > Hmmm, as you say, this could end up being rather messy. However, by > passing the device node pointer, the driver could extract that data on > a per case basis. (ie. it would be decided on a per driver basis > where to get the platform data). I'm not sure; this bears more > thought... Sometimes it's not worth powder and shot adding OF functionality to the drivers, I2C and SPI are major examples. Another [not mmc_spi] example is drivers/input/touchscreen/ads7846.c, which is SPI driver and needs platform data. There is a board that needs this (touchscreen controller on a MPC8360E-RDK). Also there is no way to pass functions via device tree, we're always end up doing board-specific hooks in the generic drivers... Finally, let's call this platform_data and be done with it. Then we can use this for things like drivers/video/fsl-diu-fb.c (see diu_ops, which is global struct, filled by arch/powerpc/platforms/86xx/mpc8610_hpcd.c). -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-16 22:49 ` Anton Vorontsov @ 2008-05-17 5:02 ` Grant Likely [not found] ` <fa686aa40805162202m336aade4qd6cfa5b17d6f3892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-17 5:02 UTC (permalink / raw) To: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 4:49 PM, Anton Vorontsov <cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On Fri, May 16, 2008 at 04:14:23PM -0600, Grant Likely wrote: >> > Maybe this code could do something like >> > spi->dev.platform_data = nc->data; >> > and board code would fill nc->data at early stages? This needs to be a >> > convention, not just random use though.. Maybe we can expand the struct >> > device_node to explicitly include .platform_data for such cases? >> >> Hmmm, as you say, this could end up being rather messy. However, by >> passing the device node pointer, the driver could extract that data on >> a per case basis. (ie. it would be decided on a per driver basis >> where to get the platform data). I'm not sure; this bears more >> thought... > > Sometimes it's not worth powder and shot adding OF functionality to > the drivers, I2C and SPI are major examples. Another [not mmc_spi] > example is drivers/input/touchscreen/ads7846.c, which is SPI driver > and needs platform data. There is a board that needs this (touchscreen > controller on a MPC8360E-RDK). In my mind; platform_data and the device tree are all about the same thing: representation. In other words, how to describe the configuration of the hardware independent of the driver itself. The device tree and platform data structures both solve the same problem. In both cases, the representation data must be translated/decoded/interpreted and stored in the drivers own private data structure so it can be of use. One of the things I find rather interesting is just how frequently drivers using platform data structures have a big block of code which simply copy pdata fields into identically named fields in the device private data... specifically: copied discretely instead of being a verbatim block that can be memcopied, or instead of maintaining a pointer and using the pdata itself. It highlights for me just how much pdata structures are decoupled from the driver itself (just like how the device tree data is decoupled from the driver)... but I digress. The point is that the translation of data from the device tree (and from pdata for that matter) to a form usable by the driver has to live *somewhere*. Does it belong in the platform code? Or should it live with the driver itself? I argue that it really belongs as much as feasibly possible with the driver code. Even if a pdata structure is chosen to be used as an intermediary representation, the code is only relevant to the driver and therefore shouldn't appear anywhere else in the kernel tree. Putting it with the driver also has the added advantage that it can be lumped in with the driver module and therefore will only get compiled into the kernel if the driver is present. Putting driver specific (not platform specific) translation code anywhere other than with the device driver it is intended for is just wrong. In addition, I'd really like to avoid a situation where the same block of translation code (or at least calls to it) is duplicated all over the platform code directories. It's that sort of duplication that the device tree (and similar schemes) is intended to solve. I agree that using platform code is often the best solution, especially when dealing with one-off board ports that won't appear anywhere else. However, I strongly believe that the platform code approach should be the exception, not the rule. If it is a common data property that all instantiations of the device must have, then encode it in the device tree and be done with it. Doing so keeps platform code straight forward and reduces duplication. > Also there is no way to pass functions via device tree, we're > always end up doing board-specific hooks in the generic drivers... > > Finally, let's call this platform_data and be done with it. Then we > can use this for things like drivers/video/fsl-diu-fb.c (see diu_ops, > which is global struct, filled by > arch/powerpc/platforms/86xx/mpc8610_hpcd.c). Yes, I agree, there are always going to be platform/board specific hooks and I'm not saying that we should try to eliminate them. But (as expressed in the argument above) I don't like the idea of making the platform code fill in all the necessary pdata structures. How about this as an alternative: Instead of allowing platform code to fill in platform_data pointers at early platform setup, let it register a driver-specific callback hook instead. If the hook it populated, the driver will call it at an appropriate time to allow the platform code to manipulate the driver configuration. The signature of the hook can be driver dependent (just like how the pdata hook is platform dependent). Doing this ensure that the translation code stays where it belongs: with the driver itself, and it defers execution of the code to a point to driver initialization time instead of earlier in the platform setup which should improve boot times in certain circumstances if the drivers are loaded as modules. As for adding OF support to the drivers "not worth powder and shot"; I must disagree. Adding the device tree support really isn't very complex and the impact on existing drivers quite minimal. All of the code can be restricted to a function called by the drivers probe routine that can be compiled out entirely if CONFIG_OF is not set. I've already done similar stuff with drivers supporting both platform and of_platform busses, and this situation I think should be even less invasive. Thoughts? Cheers, g. --- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805162202m336aade4qd6cfa5b17d6f3892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805162202m336aade4qd6cfa5b17d6f3892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-22 1:16 ` David Brownell [not found] ` <200805211816.10753.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-22 1:16 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Friday 16 May 2008, Grant Likely wrote: > In my mind; platform_data and the device tree are all about the same > thing: representation. In other words, how to describe the > configuration of the hardware independent of the driver itself. Platform_data isn't what I'd call independent of drivers. The reason the data is there in the first place is that the driver needs it ... and chose not to hard-wire it. > One of the things I find rather interesting is just how frequently > drivers using platform data structures have a big block of code which > simply copy pdata fields into identically named fields in the device > private data... ... because platform data was designed as a partial template for that driver, letting it do that. (Sometimes without even doing scale conversions.) As drivers grow functionally, they sometimes end up needing more platform data fields, to expose data that previously didn't matter. Whether that data can usefully be stored in flash (or ROM) and handed out through the bootloader is something of a manufacturing issue. - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805211816.10753.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805211816.10753.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 6:24 ` Grant Likely 0 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:24 UTC (permalink / raw) To: David Brownell Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Wed, May 21, 2008 at 7:16 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Friday 16 May 2008, Grant Likely wrote: >> In my mind; platform_data and the device tree are all about the same >> thing: representation. In other words, how to describe the >> configuration of the hardware independent of the driver itself. > > Platform_data isn't what I'd call independent of drivers. > > The reason the data is there in the first place is that > the driver needs it ... and chose not to hard-wire it. Oh, of course the driver needs it! I'm not claiming otherwise. More what I mean is that the driver doesn't need to be loaded or even configured in for the platform code to make use of pdata. >> One of the things I find rather interesting is just how frequently >> drivers using platform data structures have a big block of code which >> simply copy pdata fields into identically named fields in the device >> private data... > > ... because platform data was designed as a partial template > for that driver, letting it do that. (Sometimes without even > doing scale conversions.) As drivers grow functionally, they > sometimes end up needing more platform data fields, to expose > data that previously didn't matter. > > Whether that data can usefully be stored in flash (or ROM) > and handed out through the bootloader is something of a > manufacturing issue. I do not dispute any of that. My point, however, is that pdata is typically used simply as a representation that is convenient for platform code to pass that data into the driver and that often drivers don't use that representation directly. Instead, the data is explicitly copied explicitly field by field into the driver at probe time and is not touched again. That says to me that driver developers view pdata as somewhat decoupled from the internal workings of the driver and in the case of many powerpc devices a different representation is more convenient; namely a device tree node. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-16 20:47 ` Randy Dunlap 2008-05-16 22:03 ` Anton Vorontsov @ 2008-05-19 13:17 ` Guennadi Liakhovetski 2008-05-19 15:57 ` Grant Likely 2008-05-21 15:19 ` Anton Vorontsov 3 siblings, 1 reply; 50+ messages in thread From: Guennadi Liakhovetski @ 2008-05-19 13:17 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, 16 May 2008, Grant Likely wrote: > + However, the binding does not attempt to define the specific method for > + assigning chip select numbers. Since SPI chip select configuration is > + flexible and non-standardized, it is left out of this binding with the > + assumption that board specific platform code will be used to manage > + chip selects. Individual drivers can define additional properties to > + support describing the chip select layout. Yes, this looks like a problem to me. This means, SPI devices will need two bindings - OF and platform?... Maybe define an spi_chipselect OF-binding? Thanks Guennadi --- Guennadi Liakhovetski ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-19 13:17 ` Guennadi Liakhovetski @ 2008-05-19 15:57 ` Grant Likely 2008-05-19 16:30 ` Guennadi Liakhovetski 2008-05-19 17:09 ` Gary Jennejohn 0 siblings, 2 replies; 50+ messages in thread From: Grant Likely @ 2008-05-19 15:57 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: fabrizio.garetto, linux-kernel, linuxppc-dev, dbrownell, spi-devel-general On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 16 May 2008, Grant Likely wrote: > >> + However, the binding does not attempt to define the specific method for >> + assigning chip select numbers. Since SPI chip select configuration is >> + flexible and non-standardized, it is left out of this binding with the >> + assumption that board specific platform code will be used to manage >> + chip selects. Individual drivers can define additional properties to >> + support describing the chip select layout. > > Yes, this looks like a problem to me. This means, SPI devices will need > two bindings - OF and platform?... Maybe define an spi_chipselect > OF-binding? Actually, spi devices have *neither*. :-) They bind to the SPI bus. Not the platform bus or of_platform bus. But that is Linux internal details; this discussion is about device tree bindings. Note that I did say that drivers can define additional properties for supporting chip select changes as needed. I'm just not attempting to encode them into the formal binding. There is simply just too many different ways to manipulate chip select signals and so I don't feel confident trying to define a *common* binding at this moment in time. At some point in the future when we have a number of examples to choose from then we can extend this binding with chip select related properties. As for the Linux internals, the 5200 SPI bus driver that I posted exports a function that allows another driver to call in and manipulated the CS lines before the transfer. It isn't the prettiest solution, but I'm not locked into the approach and that gives some time to consider cleaner interfaces. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-19 15:57 ` Grant Likely @ 2008-05-19 16:30 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.0805191811510.29559-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 2008-05-19 17:09 ` Gary Jennejohn 1 sibling, 1 reply; 50+ messages in thread From: Guennadi Liakhovetski @ 2008-05-19 16:30 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto, linux-kernel, linuxppc-dev, dbrownell, spi-devel-general On Mon, 19 May 2008, Grant Likely wrote: > On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > On Fri, 16 May 2008, Grant Likely wrote: > > > >> + However, the binding does not attempt to define the specific method for > >> + assigning chip select numbers. Since SPI chip select configuration is > >> + flexible and non-standardized, it is left out of this binding with the > >> + assumption that board specific platform code will be used to manage > >> + chip selects. Individual drivers can define additional properties to > >> + support describing the chip select layout. > > > > Yes, this looks like a problem to me. This means, SPI devices will need > > two bindings - OF and platform?... Maybe define an spi_chipselect > > OF-binding? > > Actually, spi devices have *neither*. :-) They bind to the SPI bus. > Not the platform bus or of_platform bus. Right, sorry, your SPI bus driver scans the bus device bindings and registers devices on it using spi_of_register_devices(). > But that is Linux internal > details; this discussion is about device tree bindings. > > Note that I did say that drivers can define additional properties for > supporting chip select changes as needed. I'm just not attempting to > encode them into the formal binding. There is simply just too many > different ways to manipulate chip select signals and so I don't feel > confident trying to define a *common* binding at this moment in time. Yes, I understand, that physically there can be many ways SPI chipselects can be controlled. But I thought there could be a generic way to cover them all by defining a separate entry on your SPI bus. Like + SPI example for an MPC5200 SPI bus: + spi@f00 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; + reg = <0xf00 0x20>; + interrupts = <2 13 0 2 14 0>; + interrupt-parent = <&mpc5200_pic>; + eth-switch-cs@0 { + compatible = "oem,cs-type"; + }; + + ethernet-switch@0 { + compatible = "micrel,ks8995m"; + linux,modalias = "ks8995"; + max-speed = <1000000>; + reg = <0>; + cs-parent = <&/.../spi@f00/eth-switch-cs@0>; + }; ... + }; Then whatever method is used to actually switch the CS, a driver should be registered to handle eth-switch-cs@0, providing the required calls. Without such a driver ethernet-switch@0 will not probe successfully. Wouldn't this cover all possible cases? One could even consider actually putting SPI devices on SPI chipselect busses, but that won't look very elegant:-) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <Pine.LNX.4.64.0805191811510.29559-0199iw4Nj15frtckUFj5Ag@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <Pine.LNX.4.64.0805191811510.29559-0199iw4Nj15frtckUFj5Ag@public.gmane.org> @ 2008-05-20 5:13 ` Grant Likely 2008-05-20 15:26 ` Guennadi Liakhovetski 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-20 5:13 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Mon, May 19, 2008 at 10:30 AM, Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> wrote: > On Mon, 19 May 2008, Grant Likely wrote: >> But that is Linux internal >> details; this discussion is about device tree bindings. >> >> Note that I did say that drivers can define additional properties for >> supporting chip select changes as needed. I'm just not attempting to >> encode them into the formal binding. There is simply just too many >> different ways to manipulate chip select signals and so I don't feel >> confident trying to define a *common* binding at this moment in time. > > Yes, I understand, that physically there can be many ways SPI chipselects > can be controlled. But I thought there could be a generic way to cover > them all by defining a separate entry on your SPI bus. Like > > + SPI example for an MPC5200 SPI bus: > + spi@f00 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; > + reg = <0xf00 0x20>; > + interrupts = <2 13 0 2 14 0>; > + interrupt-parent = <&mpc5200_pic>; > + eth-switch-cs@0 { > + compatible = "oem,cs-type"; > + }; > + > + ethernet-switch@0 { > + compatible = "micrel,ks8995m"; > + linux,modalias = "ks8995"; > + max-speed = <1000000>; > + reg = <0>; > + cs-parent = <&/.../spi@f00/eth-switch-cs@0>; > + }; > ... > + }; > > Then whatever method is used to actually switch the CS, a driver should be > registered to handle eth-switch-cs@0, providing the required calls. > Without such a driver ethernet-switch@0 will not probe successfully. > Wouldn't this cover all possible cases? One could even consider actually > putting SPI devices on SPI chipselect busses, but that won't look very > elegant:-) Hurrmmmm... I'm not so fond of this approach. cs-parent doesn't seem to make much sense to me. It might be better to have a cs-handler property on the SPI bus node instead of on the SPI slave nodes, but even then it leaves a number of questions about what it really means. In some cases it would be overkill. For example, if the SPI node simply had multiple GPIO lines then an extra cs-parent node wouldn't be needed at all. Then there are the complex arrangements. When setting CS requires inserting a special 'set cs' SPI message at the right time. Or worse; when setting CS requires /modifying/ the sent SPI message. Essentially, the binding would need to describe the ability to completely intercept and rewrite all SPI messages going through the CS scheme. I'm not saying it's not possible to do, but I am saying that I'd like to have a better feel for all the use cases before it is defined. I'm not convinced that adding a cs-parent phandle will do that appropriately. That being said, my gut feel is that the solution will be to support spi-bridge nodes that handle the complex CS configuration settings; the spi-bridge would be a child of the spi-master and the parent of the spi devices; and simple CS settings being handled with regular old GPIO bindings. (Much like the last suggestion you make; except that I think that it *does* looks elegant.) :-) example; here's an SPI bus that has 2 GPIOs for two bus CS lines and an SPI bridge that uses both CSes; one address for accessing the bridge's CS register and one CS to access the downstream devices. + SPI example for an MPC5200 SPI bus: + spi@f00 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; + reg = <0xf00 0x20>; + interrupts = <2 13 0 2 14 0>; + interrupt-parent = <&mpc5200_pic>; + gpios = <&gpio1 0 0 &gpio1 1 0>; + spi-bridge@0 { + compatible = "oem,spi-bridge-type"; + reg = < 0 1 >; // note: 2 SPI CS addresses; first one to access bridge registers + + ethernet-switch@0 { + compatible = "micrel,ks8995m"; + linux,modalias = "ks8995"; + max-speed = <1000000>; + reg = <0>; + }; ... // and more SPI child nodes here... + }; + }; But even this doesn't reflect the hardware layout well. What if the SS lines are on SPI GPIO expanders on the same bus? Then does it make sense for them to be layed out as spi bridges? Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-20 5:13 ` Grant Likely @ 2008-05-20 15:26 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.0805201650280.5283-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Guennadi Liakhovetski @ 2008-05-20 15:26 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell, fabrizio.garetto, jonsmirl On Mon, 19 May 2008, Grant Likely wrote: > I'm not so fond of this approach. cs-parent doesn't seem to make much > sense to me. It might be better to have a cs-handler property on the > SPI bus node instead of on the SPI slave nodes, but even then it > leaves a number of questions about what it really means. In some > cases it would be overkill. For example, if the SPI node simply had > multiple GPIO lines then an extra cs-parent node wouldn't be needed at > all. Right, it is optional. > Then there are the complex arrangements. When setting CS > requires inserting a special 'set cs' SPI message at the right time. > Or worse; when setting CS requires /modifying/ the sent SPI message. Hm, are there actually such SPI _controllers_ that use SPI data to toggle chipselects? I.e., you would have to send your SPI client data (for the RTC or whatever) plus some extra bytes or with some modifications, and this extra information would then be intercepted by the SPI _controller_ itself and only client data would be sent out? Isn't what you're describing really a case of an SPI bridge, as you also call it below? In which case, I think, it might make sense to cleanly differentiate these two cases: 1. SPI chipselect. Either controlled by an external (typically a GPIO) signal or by the controller itself, in the latter case the controller has to be configured with the required address 2. SPI bridge. I don't know such configurations, so, I can only guess: the SPI controller has a single SPI client, which acts like a bridge. It receives data from the primary host, and in this data the target client data and its address are encoded. Now, I can also imagine case 2 where the bridge is actually a part of the host controller... Even though this doesn't make any sense to me. > Essentially, the binding would need to describe the ability to > completely intercept and rewrite all SPI messages going through the CS > scheme. > > I'm not saying it's not possible to do, but I am saying that I'd like > to have a better feel for all the use cases before it is defined. I'm > not convinced that adding a cs-parent phandle will do that > appropriately. That being said, my gut feel is that the solution will > be to support spi-bridge nodes that handle the complex CS > configuration settings; the spi-bridge would be a child of the > spi-master and the parent of the spi devices; and simple CS settings > being handled with regular old GPIO bindings. (Much like the last > suggestion you make; except that I think that it *does* looks > elegant.) :-) Ok, elegance apart:-) You can use the SPI-bridge construct to also describe simple SPI-chipselect configurations. But is it really a good idea? Wouldn't it be better to handle these two cases separately? Using "bridge" to describe CS's seems also confusing - imagine someone implementing a new DTS, having to describe a bridge not having one doesn't seem very intuitive:-) > example; here's an SPI bus that has 2 GPIOs for two bus CS lines and > an SPI bridge that uses both CSes; one address for accessing the > bridge's CS register and one CS to access the downstream devices. > > + SPI example for an MPC5200 SPI bus: > + spi@f00 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; > + reg = <0xf00 0x20>; > + interrupts = <2 13 0 2 14 0>; > + interrupt-parent = <&mpc5200_pic>; > + gpios = <&gpio1 0 0 &gpio1 1 0>; > + spi-bridge@0 { > + compatible = "oem,spi-bridge-type"; > + reg = < 0 1 >; // note: 2 SPI CS addresses; first one to access bridge registers > + > + ethernet-switch@0 { > + compatible = "micrel,ks8995m"; > + linux,modalias = "ks8995"; > + max-speed = <1000000>; > + reg = <0>; > + }; > ... // and more SPI child nodes here... > + }; > + }; > > But even this doesn't reflect the hardware layout well. What if the > SS lines are on SPI GPIO expanders on the same bus? Then does it make > sense for them to be layed out as spi bridges? Well, in this case - yes, because addressing clients "behind" the expander and the expander itself is done differently. On the whole, I think, it begins to look good - I think, it is better to implement an imperfect but complete and consistent scheme and modify or extend it in the future, than a perfect, but incomplete, and have to use auxiliary means (platform bindings) to fill in the gaps. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <Pine.LNX.4.64.0805201650280.5283-0199iw4Nj15frtckUFj5Ag@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <Pine.LNX.4.64.0805201650280.5283-0199iw4Nj15frtckUFj5Ag@public.gmane.org> @ 2008-05-20 15:48 ` Grant Likely 2008-05-21 19:11 ` Segher Boessenkool 1 sibling, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-20 15:48 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, jonsmirl-Re5JQEeQqe8AvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Tue, May 20, 2008 at 9:26 AM, Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> wrote: > On Mon, 19 May 2008, Grant Likely wrote: > >> I'm not so fond of this approach. cs-parent doesn't seem to make much >> sense to me. It might be better to have a cs-handler property on the >> SPI bus node instead of on the SPI slave nodes, but even then it >> leaves a number of questions about what it really means. In some >> cases it would be overkill. For example, if the SPI node simply had >> multiple GPIO lines then an extra cs-parent node wouldn't be needed at >> all. > > Right, it is optional. > >> Then there are the complex arrangements. When setting CS >> requires inserting a special 'set cs' SPI message at the right time. >> Or worse; when setting CS requires /modifying/ the sent SPI message. > > Hm, are there actually such SPI _controllers_ that use SPI data to toggle > chipselects? I.e., you would have to send your SPI client data (for the > RTC or whatever) plus some extra bytes or with some modifications, and > this extra information would then be intercepted by the SPI _controller_ > itself and only client data would be sent out? Yes! There really are!!! One case I've been told of is an SPI bridge that uses the first byte of the transfer to obtain the chip select. > Isn't what you're > describing really a case of an SPI bridge, as you also call it below? In > which case, I think, it might make sense to cleanly differentiate these > two cases: > > 1. SPI chipselect. Either controlled by an external (typically a GPIO) > signal or by the controller itself, in the latter case the controller has > to be configured with the required address > > 2. SPI bridge. I don't know such configurations, so, I can only guess: the > SPI controller has a single SPI client, which acts like a bridge. It > receives data from the primary host, and in this data the target client > data and its address are encoded. Yes, this is probably appropriate. > Now, I can also imagine case 2 where the bridge is actually a part of the > host controller... Even though this doesn't make any sense to me. Hmmm, yeah, I suppose it is possible; but if it is internal to the bus controller then it can also be handled internally by the bus controller driver and probably won't need to be reflected in the device tree layout. >> Essentially, the binding would need to describe the ability to >> completely intercept and rewrite all SPI messages going through the CS >> scheme. It also needs to describe layouts which require SPI transfers in a particular order. For example, if you're doing 2 SPI messages (M1 and M2) to 2 different SPI devices (S1 and S2), and the CS lines are on a GPIO expander which is a third SPI device (S3). In which case 5 or 6 SPI messages need to be transmitted: ctrl msg -> S3 // To set the CS to S1 M1 -> S1 ctrl msg -> S3 // To clear the CS to S1 ctrl msg -> S3 // To set the CS to S2 M2 -> S2 ctrl msg -> S3 // To clear the CS to S2 An important subtlety to note here is that the GPIO device (S3) cannot simply enqueue the control message to the SPI device when it is time to send M1 or M2. Normal enqueuing would add the messages to the end of the queue; too late to actually activate the relevant CS line. Granted, the is mostly a driver issue; not a device tree issue; but it has some impact on the layout. It could be handled with the spi_bridge construct, but S1 and S2 aren't really children of S3. On the other hand; the spi_bridge is really more of a board level construct. The spi_bridge could be considered the board wireup and S1, S2 and S3 are all children of the spi_bridge. The spi_bridge would have to be knowledgeable enough to handle control messages to S3 in a special order. >> I'm not saying it's not possible to do, but I am saying that I'd like >> to have a better feel for all the use cases before it is defined. I'm >> not convinced that adding a cs-parent phandle will do that >> appropriately. That being said, my gut feel is that the solution will >> be to support spi-bridge nodes that handle the complex CS >> configuration settings; the spi-bridge would be a child of the >> spi-master and the parent of the spi devices; and simple CS settings >> being handled with regular old GPIO bindings. (Much like the last >> suggestion you make; except that I think that it *does* looks >> elegant.) :-) > > Ok, elegance apart:-) You can use the SPI-bridge construct to also > describe simple SPI-chipselect configurations. But is it really a good > idea? Wouldn't it be better to handle these two cases separately? Using > "bridge" to describe CS's seems also confusing - imagine someone > implementing a new DTS, having to describe a bridge not having one doesn't > seem very intuitive:-) ... and it must be said that I got rather lazy in my example below. I really covered both layouts (simple GPIO and an SPI bridge in the same example without documenting it sufficiently. I'll hash out my thoughts some more and post a series of better examples this afternoon. > >> example; here's an SPI bus that has 2 GPIOs for two bus CS lines and >> an SPI bridge that uses both CSes; one address for accessing the >> bridge's CS register and one CS to access the downstream devices. >> >> + SPI example for an MPC5200 SPI bus: >> + spi@f00 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "fsl,mpc5200b-spi","fsl,mpc5200-spi"; >> + reg = <0xf00 0x20>; >> + interrupts = <2 13 0 2 14 0>; >> + interrupt-parent = <&mpc5200_pic>; >> + gpios = <&gpio1 0 0 &gpio1 1 0>; >> + spi-bridge@0 { >> + compatible = "oem,spi-bridge-type"; >> + reg = < 0 1 >; // note: 2 SPI CS addresses; first one to access bridge registers >> + >> + ethernet-switch@0 { >> + compatible = "micrel,ks8995m"; >> + linux,modalias = "ks8995"; >> + max-speed = <1000000>; >> + reg = <0>; >> + }; >> ... // and more SPI child nodes here... >> + }; >> + }; >> >> But even this doesn't reflect the hardware layout well. What if the >> SS lines are on SPI GPIO expanders on the same bus? Then does it make >> sense for them to be layed out as spi bridges? > > Well, in this case - yes, because addressing clients "behind" the expander > and the expander itself is done differently. > > On the whole, I think, it begins to look good - I think, it is better to > implement an imperfect but complete and consistent scheme and modify or > extend it in the future, than a perfect, but incomplete, and have to use > auxiliary means (platform bindings) to fill in the gaps. I'm not too opposed to using non-standard properties to describe stuff in the short term; but I agree that avoiding hacky platform data stuff is not desired. Thanks for all the feedback. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <Pine.LNX.4.64.0805201650280.5283-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 2008-05-20 15:48 ` Grant Likely @ 2008-05-21 19:11 ` Segher Boessenkool [not found] ` <716a0f1b6c9a544b480c06a329072483-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> 1 sibling, 1 reply; 50+ messages in thread From: Segher Boessenkool @ 2008-05-21 19:11 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f > Ok, elegance apart:-) You can use the SPI-bridge construct to also > describe simple SPI-chipselect configurations. But is it really a good > idea? Wouldn't it be better to handle these two cases separately? It would be best to handle all these things that are specific to a certain SPI controller (like how CSs work) in the binding for that SPI controller, and not try to shoehorn all of this into some artificial generic framework. If you can have identical addresses on the SPI bus going to different devices based on which CS is asserted, you'll have to make the CS part of the "reg". Example: spi-controller { #address-cells = 2; #size-cells = 0; some-device@0,f000 { reg = < 0 f000 >; } // CS 0, SPI address f000 some-device@1,f000 { reg = < 1 f000 >; } // CS 1, SPI address f000 some-device@1,ff00 { reg = < 1 ff00 >; } // CS 1, SPI address ff00 } SPI-to-SPI bridges can (and should!) be handled the same way as anything-to-anything-else bridges are handled as well: either there is a nice simple one-to-one matching (and you can use "ranges") or you need a driver for that bridge that knows how to make it work (or both, "ranges" isn't always enough, the bridge might require some specific handling for some special situations -- error handling, suspend, whatever). Segher ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <716a0f1b6c9a544b480c06a329072483-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <716a0f1b6c9a544b480c06a329072483-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> @ 2008-05-21 19:33 ` Grant Likely [not found] ` <fa686aa40805211233h72a258bpf8c945b9f662d6ee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-21 19:33 UTC (permalink / raw) To: Segher Boessenkool Cc: dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Guennadi Liakhovetski On Wed, May 21, 2008 at 1:11 PM, Segher Boessenkool <segher-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote: >> Ok, elegance apart:-) You can use the SPI-bridge construct to also >> describe simple SPI-chipselect configurations. But is it really a good >> idea? Wouldn't it be better to handle these two cases separately? > > It would be best to handle all these things that are specific to > a certain SPI controller (like how CSs work) in the binding for > that SPI controller, and not try to shoehorn all of this into some > artificial generic framework. > > If you can have identical addresses on the SPI bus going to different > devices based on which CS is asserted, you'll have to make the CS part > of the "reg". Example: > > spi-controller { > #address-cells = 2; > #size-cells = 0; > some-device@0,f000 { reg = < 0 f000 >; } // CS 0, SPI address f000 > some-device@1,f000 { reg = < 1 f000 >; } // CS 1, SPI address f000 > some-device@1,ff00 { reg = < 1 ff00 >; } // CS 1, SPI address ff00 > } For SPI the CS # *is* the address. :-) Unlike I2C, SPI doesn't impose any protocol on the data. It is all anonymous data out, anonymous data in, a clock and a chip select. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805211233h72a258bpf8c945b9f662d6ee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805211233h72a258bpf8c945b9f662d6ee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-23 2:26 ` David Brownell [not found] ` <200805221926.24112.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-23 2:26 UTC (permalink / raw) To: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, Segher Boessenkool, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, Guennadi Liakhovetski On Wednesday 21 May 2008, Grant Likely wrote: > > spi-controller { > > #address-cells = 2; > > #size-cells = 0; > > some-device@0,f000 { reg = < 0 f000 >; } // CS 0, SPI address f000 > > some-device@1,f000 { reg = < 1 f000 >; } // CS 1, SPI address f000 > > some-device@1,ff00 { reg = < 1 ff00 >; } // CS 1, SPI address ff00 > > } > > For SPI the CS # *is* the address. :-) > > Unlike I2C, SPI doesn't impose any protocol on the data. It is all > anonymous data out, anonymous data in, a clock and a chip select. Very true ... but then there are SPI chips which embed addressing. I have in mind the mcp23s08 (and mcp23s17) GPIO expanders, which support up to four chips wired in parallel on a given chipselect. The devices are distinguished by how two address pins are wired; and two bits in the command byte must match them. (I think they just recycled an I2C design into the SPI world.) - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805221926.24112.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805221926.24112.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 6:25 ` Grant Likely [not found] ` <fa686aa40805232325w65d4f706i50798121a8cce263-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:25 UTC (permalink / raw) To: David Brownell Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, Segher Boessenkool, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Guennadi Liakhovetski On Thu, May 22, 2008 at 8:26 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Wednesday 21 May 2008, Grant Likely wrote: >> > spi-controller { >> > #address-cells = 2; >> > #size-cells = 0; >> > some-device@0,f000 { reg = < 0 f000 >; } // CS 0, SPI address f000 >> > some-device@1,f000 { reg = < 1 f000 >; } // CS 1, SPI address f000 >> > some-device@1,ff00 { reg = < 1 ff00 >; } // CS 1, SPI address ff00 >> > } >> >> For SPI the CS # *is* the address. :-) >> >> Unlike I2C, SPI doesn't impose any protocol on the data. It is all >> anonymous data out, anonymous data in, a clock and a chip select. > > Very true ... but then there are SPI chips which embed addressing. > > I have in mind the mcp23s08 (and mcp23s17) GPIO expanders, which > support up to four chips wired in parallel on a given chipselect. > The devices are distinguished by how two address pins are wired; > and two bits in the command byte must match them. (I think they > just recycled an I2C design into the SPI world.) Very good point. Okay, so we cannot assume any correlation between the number of CS lines and the number of child nodes to the SPI bus. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805232325w65d4f706i50798121a8cce263-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805232325w65d4f706i50798121a8cce263-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 7:13 ` David Brownell 0 siblings, 0 replies; 50+ messages in thread From: David Brownell @ 2008-05-24 7:13 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, Segher Boessenkool, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Guennadi Liakhovetski On Friday 23 May 2008, Grant Likely wrote: > Very good point. Okay, so we cannot assume any correlation between > the number of CS lines and the number of child nodes to the SPI bus. That wasn't what I was implying ... all the devices hooked up to a given chipselect should be viewed as a single (albeit composite) child node. Now, the driver for that child node may want to expose lots of substructure. But that's no different from any other complex device, whose protocol happens to embed some notion of component addressing. It's just that in the case I mentioned, that addressing is a bit more externally visible than it is in some other cases. - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses 2008-05-19 15:57 ` Grant Likely 2008-05-19 16:30 ` Guennadi Liakhovetski @ 2008-05-19 17:09 ` Gary Jennejohn [not found] ` <20080519190900.01ec3b2a-f7AvneZ2CE0iXleZOAq1AWD2FQJk+8+b@public.gmane.org> 1 sibling, 1 reply; 50+ messages in thread From: Gary Jennejohn @ 2008-05-19 17:09 UTC (permalink / raw) To: Grant Likely Cc: Guennadi Liakhovetski, fabrizio.garetto, linux-kernel, linuxppc-dev, dbrownell, spi-devel-general On Mon, 19 May 2008 09:57:21 -0600 "Grant Likely" <grant.likely@secretlab.ca> wrote: > On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > On Fri, 16 May 2008, Grant Likely wrote: > > > >> + However, the binding does not attempt to define the specific method for > >> + assigning chip select numbers. Since SPI chip select configuration is > >> + flexible and non-standardized, it is left out of this binding with the > >> + assumption that board specific platform code will be used to manage > >> + chip selects. Individual drivers can define additional properties to > >> + support describing the chip select layout. > > > > Yes, this looks like a problem to me. This means, SPI devices will need > > two bindings - OF and platform?... Maybe define an spi_chipselect > > OF-binding? > > Actually, spi devices have *neither*. :-) They bind to the SPI bus. > Not the platform bus or of_platform bus. But that is Linux internal > details; this discussion is about device tree bindings. > > Note that I did say that drivers can define additional properties for > supporting chip select changes as needed. I'm just not attempting to > encode them into the formal binding. There is simply just too many > different ways to manipulate chip select signals and so I don't feel > confident trying to define a *common* binding at this moment in time. > At some point in the future when we have a number of examples to > choose from then we can extend this binding with chip select related > properties. > > As for the Linux internals, the 5200 SPI bus driver that I posted > exports a function that allows another driver to call in and > manipulated the CS lines before the transfer. It isn't the prettiest > solution, but I'm not locked into the approach and that gives some > time to consider cleaner interfaces. > I sort of hesitate to hijack this thread, but since we're discussing SPI and chip selects... I have a driver for the SPI controller in the 440EPx. This controller is very simple and does not have any internal support for a chip select. The controller seems to also be in the 440GR and 440EP, and may be in other AMCC CPUs too. All chip selects must be done using GPIO. In fact, the board for which I developed this driver, a modified sequoia, actually uses 2 chip selects. My problem was, and is, that there's no generic GPIO support for powerpc. At least, not that I'm aware of. Please tell me if I'm wrong. So the driver has great gobs of GPIO code in it, most of which I took from u-boot. The code is pretty generic, but some 440EPx-specific stuff may have crept in without my being aware of it. My real question is - should this code be in a platform-specific file such as sequoia.c, which could result in lots of duplicated code, or is it better to leave it in the driver for now until some day we hopefully get generic GPIO support for powerpc? I want to get this driver upstream ASAP. --- Gary Jennejohn ********************************************************************* DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ********************************************************************* ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <20080519190900.01ec3b2a-f7AvneZ2CE0iXleZOAq1AWD2FQJk+8+b@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080519190900.01ec3b2a-f7AvneZ2CE0iXleZOAq1AWD2FQJk+8+b@public.gmane.org> @ 2008-05-19 17:19 ` Anton Vorontsov 0 siblings, 0 replies; 50+ messages in thread From: Anton Vorontsov @ 2008-05-19 17:19 UTC (permalink / raw) To: gary.jennejohn-KuiJ5kEpwI6ELgA04lAiVw Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Guennadi Liakhovetski On Mon, May 19, 2008 at 07:09:00PM +0200, Gary Jennejohn wrote: > On Mon, 19 May 2008 09:57:21 -0600 > "Grant Likely" <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > > > On Mon, May 19, 2008 at 7:17 AM, Guennadi Liakhovetski > > <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> wrote: > > > On Fri, 16 May 2008, Grant Likely wrote: > > > > > >> + However, the binding does not attempt to define the specific method for > > >> + assigning chip select numbers. Since SPI chip select configuration is > > >> + flexible and non-standardized, it is left out of this binding with the > > >> + assumption that board specific platform code will be used to manage > > >> + chip selects. Individual drivers can define additional properties to > > >> + support describing the chip select layout. > > > > > > Yes, this looks like a problem to me. This means, SPI devices will need > > > two bindings - OF and platform?... Maybe define an spi_chipselect > > > OF-binding? > > > > Actually, spi devices have *neither*. :-) They bind to the SPI bus. > > Not the platform bus or of_platform bus. But that is Linux internal > > details; this discussion is about device tree bindings. > > > > Note that I did say that drivers can define additional properties for > > supporting chip select changes as needed. I'm just not attempting to > > encode them into the formal binding. There is simply just too many > > different ways to manipulate chip select signals and so I don't feel > > confident trying to define a *common* binding at this moment in time. > > At some point in the future when we have a number of examples to > > choose from then we can extend this binding with chip select related > > properties. > > > > As for the Linux internals, the 5200 SPI bus driver that I posted > > exports a function that allows another driver to call in and > > manipulated the CS lines before the transfer. It isn't the prettiest > > solution, but I'm not locked into the approach and that gives some > > time to consider cleaner interfaces. > > > > I sort of hesitate to hijack this thread, but since we're discussing SPI > and chip selects... > > I have a driver for the SPI controller in the 440EPx. This controller > is very simple and does not have any internal support for a chip select. > The controller seems to also be in the 440GR and 440EP, and may be in > other AMCC CPUs too. > > All chip selects must be done using GPIO. In fact, the board for which > I developed this driver, a modified sequoia, actually uses 2 chip selects. > > My problem was, and is, that there's no generic GPIO support for powerpc. > At least, not that I'm aware of. Please tell me if I'm wrong. Documentation/powerpc/booting-without-of.txt VIII - Specifying GPIO information for devices. And include/linux/of_gpio.h + drivers/of/gpio.c. Soon I'll post some patches for mpc83xx_spi showing how to use GPIOs for the SPI chip selects. -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> ` (2 preceding siblings ...) 2008-05-19 13:17 ` Guennadi Liakhovetski @ 2008-05-21 15:19 ` Anton Vorontsov [not found] ` <20080521151928.GA28857-PHTr8nzUCjejyJ0x5qLZdcN33GVbZNy3@public.gmane.org> 3 siblings, 1 reply; 50+ messages in thread From: Anton Vorontsov @ 2008-05-21 15:19 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 01:36:13PM -0600, Grant Likely wrote: > From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> > > This patch adds support for populating an SPI bus based on data in the > OF device tree. This is useful for powerpc platforms which use the > device tree instead of discrete code for describing platform layout. > > Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> > --- [...] > diff --git a/drivers/spi/spi_of.c b/drivers/spi/spi_of.c > new file mode 100644 > index 0000000..b5ae434 > --- /dev/null > +++ b/drivers/spi/spi_of.c > @@ -0,0 +1,86 @@ > +/* I think better placement for this is drivers/of, no? > + * SPI OF support routines > + * Copyright (C) 2008 Secret Lab Technologies Ltd. > + * > + * Support routines for deriving SPI device attachments from the device > + * tree. > + */ > + > +#include <linux/of.h> > +#include <linux/device.h> > +#include <linux/spi/spi.h> > +#include <linux/spi/spi_of.h> > + > +/** > + * spi_of_register_devices - Register child devices onto the SPI bus > + * @master: Pointer to spi_master device > + * @np: parent node of SPI device nodes > + * > + * Registers an spi_device for each child node of 'np' which has a 'reg' > + * property. > + */ > +void spi_of_register_devices(struct spi_master *master, struct device_node *np) > +{ > + struct spi_device *spi; > + struct device_node *nc; > + const u32 *prop; > + const char *sprop; > + int rc; > + int len; > + > + for_each_child_of_node(np, nc) { > + /* Alloc an spi_device */ > + spi = spi_alloc_device(master); > + if (!spi) { > + dev_err(&master->dev, "spi_device alloc error for %s\n", > + np->full_name); > + continue; > + } > + > + /* Device address */ > + prop = of_get_property(nc, "reg", &len); > + if (!prop || len < sizeof(*prop)) { > + dev_err(&master->dev, "%s has no 'reg' property\n", > + np->full_name); Should be nc->full_name. > + continue; > + } [...] -- Anton Vorontsov email: cbouatmailru-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org irc://irc.freenode.net/bd2 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <20080521151928.GA28857-PHTr8nzUCjejyJ0x5qLZdcN33GVbZNy3@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <20080521151928.GA28857-PHTr8nzUCjejyJ0x5qLZdcN33GVbZNy3@public.gmane.org> @ 2008-05-23 2:05 ` David Brownell [not found] ` <200805221905.32288.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-23 2:05 UTC (permalink / raw) To: avorontsov-hkdhdckH98+B+jHODAdFcQ Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Wednesday 21 May 2008, Anton Vorontsov wrote: > > +++ b/drivers/spi/spi_of.c > > I think better placement for this is drivers/of, no? Yes please. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805221905.32288.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805221905.32288.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 6:26 ` Grant Likely [not found] ` <fa686aa40805232326w35f455d1s274899160d47eccb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:26 UTC (permalink / raw) To: David Brownell Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Thu, May 22, 2008 at 8:05 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Wednesday 21 May 2008, Anton Vorontsov wrote: >> > +++ b/drivers/spi/spi_of.c >> >> I think better placement for this is drivers/of, no? > > Yes please. Okay, I wasn't sure. Will do. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805232326w35f455d1s274899160d47eccb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805232326w35f455d1s274899160d47eccb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 16:50 ` Grant Likely [not found] ` <fa686aa40805240950ocd16b97y308a54c68efa28ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 16:50 UTC (permalink / raw) To: David Brownell Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 24, 2008 at 12:26 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Thu, May 22, 2008 at 8:05 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: >> On Wednesday 21 May 2008, Anton Vorontsov wrote: >>> > +++ b/drivers/spi/spi_of.c >>> >>> I think better placement for this is drivers/of, no? >> >> Yes please. > > Okay, I wasn't sure. Will do. I'm having second thoughts about this. I think this code is more SPI centric than it is OF centric. ie. it is usable by all spi masters in an OF enabled system, but it is not usable by all OF devices in an SPI enabled system. Or, in other words; it adds OF support to SPI, not the other way around. I think drivers/spi is the right place for this to live. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805240950ocd16b97y308a54c68efa28ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805240950ocd16b97y308a54c68efa28ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 17:14 ` Jochen Friedrich [not found] ` <48384D13.6010608-NIgtFMG+Po8@public.gmane.org> 2008-05-24 17:43 ` David Brownell 1 sibling, 1 reply; 50+ messages in thread From: Jochen Friedrich @ 2008-05-24 17:14 UTC (permalink / raw) To: Grant Likely Cc: David Brownell, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Grant Likely schrieb: > On Sat, May 24, 2008 at 12:26 AM, Grant Likely > <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >> On Thu, May 22, 2008 at 8:05 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: >>> On Wednesday 21 May 2008, Anton Vorontsov wrote: >>>>> +++ b/drivers/spi/spi_of.c >>>> I think better placement for this is drivers/of, no? >>> Yes please. >> Okay, I wasn't sure. Will do. > > I'm having second thoughts about this. I think this code is more SPI > centric than it is OF centric. ie. it is usable by all spi masters in > an OF enabled system, but it is not usable by all OF devices in an SPI > enabled system. Or, in other words; it adds OF support to SPI, not > the other way around. I think drivers/spi is the right place for this > to live. Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well? Thanks, Jochen ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <48384D13.6010608-NIgtFMG+Po8@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <48384D13.6010608-NIgtFMG+Po8@public.gmane.org> @ 2008-05-24 17:33 ` Grant Likely [not found] ` <fa686aa40805241033x128c30b0v826717cc879a712e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 17:33 UTC (permalink / raw) To: Jochen Friedrich Cc: David Brownell, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 24, 2008 at 11:14 AM, Jochen Friedrich <jochen-NIgtFMG+Po8@public.gmane.org> wrote: > Grant Likely schrieb: >> On Sat, May 24, 2008 at 12:26 AM, Grant Likely >> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >>> On Thu, May 22, 2008 at 8:05 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: >>>> On Wednesday 21 May 2008, Anton Vorontsov wrote: >>>>>> +++ b/drivers/spi/spi_of.c >>>>> I think better placement for this is drivers/of, no? >>>> Yes please. >>> Okay, I wasn't sure. Will do. >> >> I'm having second thoughts about this. I think this code is more SPI >> centric than it is OF centric. ie. it is usable by all spi masters in >> an OF enabled system, but it is not usable by all OF devices in an SPI >> enabled system. Or, in other words; it adds OF support to SPI, not >> the other way around. I think drivers/spi is the right place for this >> to live. > > Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well? I would argue 'yes!' g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805241033x128c30b0v826717cc879a712e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805241033x128c30b0v826717cc879a712e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 17:45 ` David Brownell [not found] ` <200805241045.47448.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-24 17:45 UTC (permalink / raw) To: Grant Likely Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, Jochen Friedrich, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Saturday 24 May 2008, Grant Likely wrote: > > Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well? > > I would argue 'yes!' ... all the more reason to have the SPI glue go there too, matching the ACPI/PCI precedent as well as those others! ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805241045.47448.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805241045.47448.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-25 4:56 ` Grant Likely 0 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-25 4:56 UTC (permalink / raw) To: David Brownell Cc: linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, Jochen Friedrich, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Sat, May 24, 2008 at 11:45 AM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Saturday 24 May 2008, Grant Likely wrote: >> > Isn't the same true for drivers/of/gpio.c or drivers/of/of_i2c.c, as well? >> >> I would argue 'yes!' > > ... all the more reason to have the SPI glue go there too, > matching the ACPI/PCI precedent as well as those others! Alright; I give! I'll put it in drivers/of. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805240950ocd16b97y308a54c68efa28ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 17:14 ` Jochen Friedrich @ 2008-05-24 17:43 ` David Brownell 1 sibling, 0 replies; 50+ messages in thread From: David Brownell @ 2008-05-24 17:43 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Saturday 24 May 2008, Grant Likely wrote: > >>> > +++ b/drivers/spi/spi_of.c > >>> > >>> I think better placement for this is drivers/of, no? > >> > >> Yes please. > > > > Okay, I wasn't sure. Will do. > > I'm having second thoughts about this. I think this code is more SPI > centric than it is OF centric. ie. it is usable by all spi masters in > an OF enabled system, but it is not usable by all OF devices in an SPI > enabled system. It's not usable by *any* SPI master on a non-OF system though. So in that sense it's far more about OF setup than it is about SPI. > Or, in other words; it adds OF support to SPI, not > the other way around. I think drivers/spi is the right place for this > to live. I'd still rather see such translations in the OF-specific part of the source tree. Like drivers/acpi/pci_*.c code, this has more to do with the firmware interface than with bus (SPI) interface. Arguments could be made both ways here, but for the moment it makes more sense to me to keep this type of platform glue (be it OF, ACPI, arch-specific setup code, or whatever) together in the source tree and apart from the bus-specific code. Where do the proposed patches gluing OF to I2C live, or has that been settled yet? - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805221915.59878.david-b@pacbell.net>]
[parent not found: <200805221915.59878.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805221915.59878.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 6:31 ` Grant Likely [not found] ` <fa686aa40805232331p1bf2c1bcn8c46c21a094ef01e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-24 6:31 UTC (permalink / raw) To: David Brownell, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Thu, May 22, 2008 at 8:15 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Friday 16 May 2008, you wrote: >> + prop = of_get_property(nc, "max-speed", &len); >> + if (prop && len >= sizeof(*prop)) >> + spi->max_speed_hz = *prop; >> + else >> + spi->max_speed_hz = 100000; > > This isn't I2C; I suggest a default more appropriate to SPI! > Maybe 10 MHz, rather than 100 KHz; or if you want folk to use > this *a lot* then maybe 1 MHz. I'd consider it a bug to have > folk rely on this very much, though. Yeah, I thought it a little stinky when I wrote it, but I wanted to put *something* in for the case where the driver sets it's own value for max_speed and it can be omitted from the device tree. Maybe it would just be better to leave it as 0 if the max-speed property is non-existent. What do you think? Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805232331p1bf2c1bcn8c46c21a094ef01e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <fa686aa40805232331p1bf2c1bcn8c46c21a094ef01e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-24 16:50 ` David Brownell [not found] ` <200805240950.43394.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: David Brownell @ 2008-05-24 16:50 UTC (permalink / raw) To: Grant Likely; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Friday 23 May 2008, Grant Likely wrote: > On Thu, May 22, 2008 at 8:15 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > > On Friday 16 May 2008, you wrote: > >> + prop = of_get_property(nc, "max-speed", &len); > >> + if (prop && len >= sizeof(*prop)) > >> + spi->max_speed_hz = *prop; > >> + else > >> + spi->max_speed_hz = 100000; > > > > This isn't I2C; I suggest a default more appropriate to SPI! > > Maybe 10 MHz, rather than 100 KHz; or if you want folk to use > > this *a lot* then maybe 1 MHz. I'd consider it a bug to have > > folk rely on this very much, though. > > Yeah, I thought it a little stinky when I wrote it, but I wanted to > put *something* in for the case where the driver sets it's own value > for max_speed and it can be omitted from the device tree. That is, this is just so the spi_setup() from spi_new_device() doesn't fail? Thing is, drivers playing with max_speed are rare. They just can't know the board-specific factors involved in making some speed fail, even when the chip might handle it on other boards. (Factors like lower voltage and higher capacitance tend to reduce the speeds that will work.) The only driver I know which mucks with max_speed_hz is the MMC-over-SPI driver, and that's because the cards themselves report various ranges that can work ... and even there, the driver remembers the board-specific maximum (which may be less than the card can handle). > Maybe it > would just be better to leave it as 0 if the max-speed property is > non-existent. Which would usually cause spi_setup() to fail, and thus cause the device not to be listed ... > What do you think? Technically, spi_setup() with max_speed set to 0 isn't what I'd call well specified ... the issue rarely came up before. "Max" of zero basically means "off", and there's not really any such state in the spi_device lifecycle. Those are good reasons to avoid such boundary cases; also, it's easy to oops in divider calculations when zero is used. Instead, board init code has set up nonzero speeds, so the spi_new_device() call to spi_setup() hasn't had ar eason to fail because max_speed was illegal/unsupportable. If it's worth avoiding, that may mean it's worth defaulting it in core code (e.g. spi_new_device) not OF platform glue... but I basically think of that speed as a value that board setup code *must* provide. - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <200805240950.43394.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH 3/4] spi: Add OF binding support for SPI busses [not found] ` <200805240950.43394.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-24 16:53 ` Grant Likely 0 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-24 16:53 UTC (permalink / raw) To: David Brownell; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, May 24, 2008 at 10:50 AM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Friday 23 May 2008, Grant Likely wrote: >> Maybe it >> would just be better to leave it as 0 if the max-speed property is >> non-existent. > > If it's worth avoiding, that may mean it's worth defaulting > it in core code (e.g. spi_new_device) not OF platform glue... > but I basically think of that speed as a value that board > setup code *must* provide. Then perhaps I'll just make it a required property. If the property isn't there, then I'll make the SPI glue skip the node. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 4/4] [CSB] Add new mpc5200-spi (non-psc) device driver 2008-05-16 19:35 [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Grant Likely ` (2 preceding siblings ...) 2008-05-16 19:36 ` [PATCH 3/4] spi: Add OF binding support for SPI busses Grant Likely @ 2008-05-16 19:36 ` Grant Likely 2008-05-16 19:42 ` Grant Likely [not found] ` <20080516193054.28030.35126.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 4 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:36 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell Cc: fabrizio.garetto, jonsmirl From: Grant Likely <grant.likely@secretlab.ca> Unlike the old CSB driver, this driver uses the SPI infrastructure. Signed-off-by: Grant Likely <grant.likely@secretlab.ca> --- drivers/spi/Kconfig | 8 + drivers/spi/Makefile | 1 drivers/spi/mpc52xx_spi.c | 561 +++++++++++++++++++++++++++++++++++++++ include/linux/spi/mpc52xx_spi.h | 10 + 4 files changed, 580 insertions(+), 0 deletions(-) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 12c35da..bd07429 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -120,6 +120,14 @@ config SPI_LM70_LLP which interfaces to an LM70 temperature sensor using a parallel port. +config SPI_MPC52xx + tristate "Freescale MPC52xx SPI (non-PSC) controller support" + depends on PPC_MPC52xx && SPI + select SPI_MASTER_OF + help + This drivers supports the MPC52xx SPI controller in master SPI + mode. + config SPI_MPC52xx_PSC tristate "Freescale MPC52xx PSC SPI controller" depends on SPI_MASTER && PPC_MPC52xx && EXPERIMENTAL diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 29c592f..805bef1 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -23,6 +23,7 @@ obj-$(CONFIG_SPI_PXA2XX) += pxa2xx_spi.o obj-$(CONFIG_SPI_OMAP_UWIRE) += omap_uwire.o obj-$(CONFIG_SPI_OMAP24XX) += omap2_mcspi.o obj-$(CONFIG_SPI_MPC52xx_PSC) += mpc52xx_psc_spi.o +obj-$(CONFIG_SPI_MPC52xx) += mpc52xx_spi.o obj-$(CONFIG_SPI_MPC83xx) += spi_mpc83xx.o obj-$(CONFIG_SPI_S3C24XX_GPIO) += spi_s3c24xx_gpio.o obj-$(CONFIG_SPI_S3C24XX) += spi_s3c24xx.o diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c new file mode 100644 index 0000000..39dc6a2 --- /dev/null +++ b/drivers/spi/mpc52xx_spi.c @@ -0,0 +1,561 @@ +/* + * MPC52xx SPI master driver. + * Copyright (C) 2008 Secret Lab Technologies Ltd. + * + * This is the driver for the MPC5200's dedicated SPI device (not for a + * PSC in SPI mode) + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/errno.h> +#include <linux/of_platform.h> +#include <linux/interrupt.h> +#include <linux/delay.h> +#include <linux/spi/spi.h> +#include <linux/spi/spi_of.h> +#include <linux/spi/mpc52xx_spi.h> +#include <linux/io.h> +#include <linux/time.h> +#include <asm/mpc52xx.h> + +MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>"); +MODULE_DESCRIPTION("MPC52xx SPI (non-PSC) Driver"); +MODULE_LICENSE("GPL"); + +/* Register offsets */ +#define SPI_CTRL1 0x00 +#define SPI_CTRL1_SPIE (1 << 7) +#define SPI_CTRL1_SPE (1 << 6) +#define SPI_CTRL1_MSTR (1 << 4) +#define SPI_CTRL1_CPOL (1 << 3) +#define SPI_CTRL1_CPHA (1 << 2) +#define SPI_CTRL1_SSOE (1 << 1) +#define SPI_CTRL1_LSBFE (1 << 0) + +#define SPI_CTRL2 0x01 +#define SPI_BRR 0x04 + +#define SPI_STATUS 0x05 +#define SPI_STATUS_SPIF (1 << 7) +#define SPI_STATUS_WCOL (1 << 6) +#define SPI_STATUS_MODF (1 << 4) + +#define SPI_DATA 0x09 +#define SPI_PORTDATA 0x0d +#define SPI_DATADIR 0x10 + +/* FSM state return values */ +#define FSM_STOP 0 +#define FSM_POLL 1 +#define FSM_CONTINUE 2 + +/* Driver internal data */ +struct mpc52xx_spi { + struct spi_master *master; + u32 sysclk; + void __iomem *regs; + int irq; + int ipb_freq; + + /* Statistics */ + int msg_count; + int wcol_count; + int wcol_ticks; + u32 wcol_tx_timestamp; + int modf_count; + int byte_count; + + /* Hooks for platform modification of behaviour */ + void (*premessage)(struct spi_message *m, void *context); + void *premessage_context; + + struct list_head queue; /* queue of pending messages */ + spinlock_t lock; + struct work_struct work; + + + /* Details of current transfer (length, and buffer pointers) */ + struct spi_message *message; /* current message */ + struct spi_transfer *transfer; /* current transfer */ + int (*state)(struct mpc52xx_spi *ms); + int len; + int timestamp; + u8 *rx_buf; + const u8 *tx_buf; + int cs_change; +}; + +/* + * CS control function + */ +static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value) +{ + if (value) + writeb(0, ms->regs + SPI_PORTDATA); /* Assert SS pin */ + else + writeb(0x08, ms->regs + SPI_PORTDATA); /* Deassert SS pin */ +} + +/* + * Start a new transfer. This is called both by the idle state + * for the first transfer in a message, and by the wait state when the + * previous transfer in a message is complete. + */ +static void mpc52xx_spi_start_transfer(struct mpc52xx_spi *ms) +{ + ms->rx_buf = ms->transfer->rx_buf; + ms->tx_buf = ms->transfer->tx_buf; + ms->len = ms->transfer->len; + + /* Activate the chip select */ + if (ms->cs_change) + mpc52xx_spi_chipsel(ms, 1); + ms->cs_change = ms->transfer->cs_change; + + /* Write out the first byte */ + ms->wcol_tx_timestamp = get_tbl(); + if (ms->tx_buf) + writeb(*ms->tx_buf++, ms->regs + SPI_DATA); + else + writeb(0, ms->regs + SPI_DATA); +} + +/* Forward declaration of state handlers */ +static int mpc52xx_spi_fsmstate_transfer(struct mpc52xx_spi *ms); +static int mpc52xx_spi_fsmstate_wait(struct mpc52xx_spi *ms); + +/* + * IDLE state + * + * No transfers are in progress; if another transfer is pending then retrieve + * it and kick it off. Otherwise, stop processing the state machine + */ +static int mpc52xx_spi_fsmstate_idle(struct mpc52xx_spi *ms) +{ + struct spi_message *m; + struct spi_device *spi; + int spr, sppr; + u8 ctrl1; + + /* Check if there is another transfer waiting */ + if (list_empty(&ms->queue)) + return FSM_STOP; + + /* Get the next message */ + spin_lock(&ms->lock); + + /* Call the pre-message hook with a pointer to the next + * message. The pre-message hook may enqueue a new message for + * changing the chip select value to the head of the queue */ + m = list_first_entry(&ms->queue, struct spi_message, queue); + if (ms->premessage) + ms->premessage(m, ms->premessage_context); + + /* reget the head of the queue (the premessage hook may have enqueued + * something before it.) and drop the spinlock */ + ms->message = list_first_entry(&ms->queue, struct spi_message, queue); + list_del_init(&ms->message->queue); + spin_unlock(&ms->lock); + + /* Setup the controller parameters */ + ctrl1 = SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR; + spi = ms->message->spi; + if (spi->mode & SPI_CPHA) + ctrl1 |= SPI_CTRL1_CPHA; + if (spi->mode & SPI_CPOL) + ctrl1 |= SPI_CTRL1_CPOL; + if (spi->mode & SPI_LSB_FIRST) + ctrl1 |= SPI_CTRL1_LSBFE; + writeb(ctrl1, ms->regs + SPI_CTRL1); + + /* Setup the controller speed */ + /* minimum divider is '2'. Also, add '1' to force rounding up. */ + sppr = ((ms->ipb_freq / ms->message->spi->max_speed_hz) + 1) >> 1; + spr = 0; + if (sppr < 1) + sppr = 1; + while (((sppr - 1) & ~0x7) != 0) { + sppr = (sppr + 1) >> 1; /* add '1' to force rounding up */ + spr++; + } + sppr--; /* sppr quantity in register is offset by 1 */ + if (spr > 7) { + /* Don't overrun limits of SPI baudrate register */ + spr = 7; + sppr = 7; + } + writeb(sppr << 4 | spr, ms->regs + SPI_BRR); /* Set speed */ + + ms->cs_change = 1; + ms->transfer = container_of(ms->message->transfers.next, + struct spi_transfer, transfer_list); + + mpc52xx_spi_start_transfer(ms); + ms->state = mpc52xx_spi_fsmstate_transfer; + +#if defined(VERBOSE_DEBUG) + dev_info(&ms->master->dev, "msg:%p, max_speed:%i, brr:%.2x\n", + ms->message, ms->message->spi->max_speed_hz, + readb(ms->regs + SPI_BRR)); +#endif + + return FSM_CONTINUE; +} + +/* + * TRANSFER state + * + * In the middle of a transfer. If the SPI core has completed processing + * a byte, then read out the received data and write out the next byte + * (unless this transfer is finished; in which case go on to the wait + * state) + */ +static int mpc52xx_spi_fsmstate_transfer(struct mpc52xx_spi *ms) +{ + u8 status = readb(ms->regs + SPI_STATUS); + u8 data; + + /* Interrupt cleared by read of STATUS followed by read of DATA */ + if (!status) + return ms->irq == NO_IRQ ? FSM_POLL : FSM_STOP; + + if (status & SPI_STATUS_WCOL) { + /* The SPI device is stoopid. At slower speeds, it may raise + * the SPIF flag before the state machine is actually finished. + * which causes a collision (internal to the state machine + * only). The manual recommends inserting a delay between + * receving the interrupt and sending the next byte, but + * it can also be worked around simply by retrying the + * transfer which is what we do here. */ + ms->wcol_count++; + ms->wcol_ticks += get_tbl() - ms->wcol_tx_timestamp; + ms->wcol_tx_timestamp = get_tbl(); + readb(ms->regs + SPI_DATA); /* clear status */ + data = 0; + if (ms->tx_buf) + data = *(ms->tx_buf-1); + writeb(data, ms->regs + SPI_DATA); /* try again */ + return FSM_CONTINUE; + } else if (status & SPI_STATUS_MODF) { + ms->modf_count++; + dev_err(&ms->master->dev, "mod fault\n"); + readb(ms->regs + SPI_DATA); /* clear status */ + mpc52xx_spi_chipsel(ms, 0); + ms->message->status = -EIO; + if (ms->message->complete) + ms->message->complete(ms->message->context); + ms->state = mpc52xx_spi_fsmstate_idle; + return FSM_CONTINUE; + } + + /* Read data out of the spi device */ + ms->byte_count++; + data = readb(ms->regs + SPI_DATA); + if (ms->rx_buf) + *ms->rx_buf++ = data; + + /* Is the transfer complete? */ + ms->len--; + if (ms->len == 0) { + ms->timestamp = get_tbl(); + ms->timestamp += ms->transfer->delay_usecs * tb_ticks_per_usec; + ms->state = mpc52xx_spi_fsmstate_wait; + return FSM_CONTINUE; + } + + /* Write out the next byte */ + ms->wcol_tx_timestamp = get_tbl(); + if (ms->tx_buf) + writeb(*ms->tx_buf++, ms->regs + SPI_DATA); + else + writeb(0, ms->regs + SPI_DATA); + + return FSM_CONTINUE; +} + +/* + * WAIT state + * + * A transfer has completed; need to wait for the delay period to complete + * before starting the next transfer + */ +static int mpc52xx_spi_fsmstate_wait(struct mpc52xx_spi *ms) +{ + if (((int)get_tbl()) - ms->timestamp < 0) + return FSM_POLL; + + ms->message->actual_length += ms->transfer->len; + + /* Check if there is another transfer in this message. If there + * aren't then deactivate CS, notify sender, and drop back to idle + * to start the next message. */ + if (ms->transfer->transfer_list.next == &ms->message->transfers) { + ms->msg_count++; + mpc52xx_spi_chipsel(ms, 0); + ms->message->status = 0; + if (ms->message->complete) + ms->message->complete(ms->message->context); + ms->state = mpc52xx_spi_fsmstate_idle; + return FSM_CONTINUE; + } + + /* There is another transfer; kick it off */ + + if (ms->cs_change) + mpc52xx_spi_chipsel(ms, 0); + + ms->transfer = container_of(ms->transfer->transfer_list.next, + struct spi_transfer, transfer_list); + mpc52xx_spi_start_transfer(ms); + ms->state = mpc52xx_spi_fsmstate_transfer; + return FSM_CONTINUE; +} + +/* + * IRQ handler + */ +static irqreturn_t mpc52xx_spi_irq(int irq, void *_ms) +{ + struct mpc52xx_spi *ms = _ms; + int rc = FSM_CONTINUE; + + while (rc == FSM_CONTINUE) + rc = ms->state(ms); + + if (rc == FSM_POLL) + schedule_work(&ms->work); + + return IRQ_HANDLED; +} + +/* + * Workqueue method of running the state machine + */ +static void mpc52xx_spi_wq(struct work_struct *work) +{ + struct mpc52xx_spi *ms = container_of(work, struct mpc52xx_spi, work); + mpc52xx_spi_irq(ms->irq, ms); +} + +/* + * spi_master callbacks + */ + +static int mpc52xx_spi_setup(struct spi_device *spi) +{ + return 0; +} + +static int mpc52xx_spi_transfer(struct spi_device *spi, struct spi_message *m) +{ + struct mpc52xx_spi *ms = spi_master_get_devdata(spi->master); + unsigned long flags; + + m->actual_length = 0; + m->status = -EINPROGRESS; + + spin_lock_irqsave(&ms->lock, flags); + list_add_tail(&m->queue, &ms->queue); + spin_unlock_irqrestore(&ms->lock, flags); + schedule_work(&ms->work); + + return 0; +} + +/* + * Hook to modify premessage hook + */ +void mpc52xx_spi_set_premessage_hook(struct spi_master *master, + void (*hook)(struct spi_message *m, + void *context), + void *hook_context) +{ + struct mpc52xx_spi *ms = spi_master_get_devdata(master); + ms->premessage = hook; + ms->premessage_context = hook_context; +} +EXPORT_SYMBOL(mpc52xx_spi_set_premessage_hook); + +/* + * SysFS files + */ +static int +*mpc52xx_spi_sysfs_get_counter(struct mpc52xx_spi *ms, const char *name) +{ + if (strcmp(name, "msg_count") == 0) + return &ms->msg_count; + if (strcmp(name, "byte_count") == 0) + return &ms->byte_count; + if (strcmp(name, "wcol_count") == 0) + return &ms->wcol_count; + if (strcmp(name, "wcol_ticks") == 0) + return &ms->wcol_ticks; + if (strcmp(name, "modf_count") == 0) + return &ms->modf_count; + return NULL; +} + +static ssize_t mpc52xx_spi_show_count(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct spi_master *master = container_of(dev, struct spi_master, dev); + struct mpc52xx_spi *ms = spi_master_get_devdata(master); + int *counter; + + counter = mpc52xx_spi_sysfs_get_counter(ms, attr->attr.name); + if (!counter) + return sprintf(buf, "error\n"); + return sprintf(buf, "%d\n", *counter); +} + +static ssize_t mpc52xx_spi_set_count(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct spi_master *master = container_of(dev, struct spi_master, dev); + struct mpc52xx_spi *ms = spi_master_get_devdata(master); + int *counter; + int value = strict_strtoul(buf, NULL, 0); + + counter = mpc52xx_spi_sysfs_get_counter(ms, attr->attr.name); + if (counter) + *counter = value; + return count; +} + +DEVICE_ATTR(msg_count, 0644, mpc52xx_spi_show_count, mpc52xx_spi_set_count); +DEVICE_ATTR(byte_count, 0644, mpc52xx_spi_show_count, mpc52xx_spi_set_count); +DEVICE_ATTR(wcol_count, 0644, mpc52xx_spi_show_count, mpc52xx_spi_set_count); +DEVICE_ATTR(wcol_ticks, 0644, mpc52xx_spi_show_count, mpc52xx_spi_set_count); +DEVICE_ATTR(modf_count, 0644, mpc52xx_spi_show_count, mpc52xx_spi_set_count); + +/* + * OF Platform Bus Binding + */ +static int __devinit mpc52xx_spi_of_probe(struct of_device *op, + const struct of_device_id *match) +{ + struct spi_master *master; + struct mpc52xx_spi *ms; + void __iomem *regs; + const u32 *prop; + int irq0, irq1, rc, len; + + /* MMIO registers */ + dev_dbg(&op->dev, "probing mpc5200 SPI device\n"); + regs = of_iomap(op->node, 0); + if (!regs) + return -ENODEV; + + /* irq */ + irq0 = irq_of_parse_and_map(op->node, 0); + irq1 = irq_of_parse_and_map(op->node, 1); + + dev_dbg(&op->dev, "allocating spi_master struct\n"); + master = spi_alloc_master(&op->dev, sizeof *ms); + if (!master) + return -ENOMEM; + master->bus_num = -1; + master->num_chipselect = 1; + prop = of_get_property(op->node, "num-slaves", &len); + if (prop && len >= sizeof(*prop)) + master->num_chipselect = *prop; + + master->setup = mpc52xx_spi_setup; + master->transfer = mpc52xx_spi_transfer; + dev_set_drvdata(&op->dev, master); + + ms = spi_master_get_devdata(master); + ms->master = master; + ms->regs = regs; + ms->irq = irq0; + ms->state = mpc52xx_spi_fsmstate_idle; + ms->ipb_freq = mpc52xx_find_ipb_freq(op->node); + spin_lock_init(&ms->lock); + INIT_LIST_HEAD(&ms->queue); + INIT_WORK(&ms->work, mpc52xx_spi_wq); + + writeb(SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR, + ms->regs + SPI_CTRL1); + writeb(0x0, ms->regs + SPI_CTRL2); + writeb(0xe, ms->regs + SPI_DATADIR); /* Set output pins */ + writeb(0x8, ms->regs + SPI_PORTDATA); /* Deassert /SS signal */ + + dev_dbg(&op->dev, "registering spi_master struct\n"); + rc = spi_register_master(master); + if (rc < 0) + goto err_register; + + rc = request_irq(irq0, mpc52xx_spi_irq, IRQF_SAMPLE_RANDOM, + "mpc5200 SPI MODF", ms); + rc |= request_irq(irq1, mpc52xx_spi_irq, IRQF_SAMPLE_RANDOM, + "mpc5200 SPI SPIF", ms); + if (rc) { + dev_info(&op->dev, "error requesting irq; using polled mode\n"); + ms->irq = NO_IRQ; /* operate in polled mode */ + } + + /* Create SysFS files */ + rc = device_create_file(&ms->master->dev, &dev_attr_msg_count); + rc |= device_create_file(&ms->master->dev, &dev_attr_byte_count); + rc |= device_create_file(&ms->master->dev, &dev_attr_wcol_count); + rc |= device_create_file(&ms->master->dev, &dev_attr_wcol_ticks); + rc |= device_create_file(&ms->master->dev, &dev_attr_modf_count); + if (rc) + dev_info(&ms->master->dev, "error creating sysfs files\n"); + + dev_info(&ms->master->dev, "registered MPC5200 SPI bus\n"); + + spi_of_register_devices(master, op->node); + + return rc; + + err_register: + spi_master_put(master); + return rc; +} + +static void __devexit mpc52xx_spi_of_remove(struct of_device *op) +{ + struct spi_master *master = dev_get_drvdata(&op->dev); + struct mpc52xx_spi *ms = spi_master_get_devdata(master); + + device_remove_file(&ms->master->dev, &dev_attr_msg_count); + device_remove_file(&ms->master->dev, &dev_attr_byte_count); + device_remove_file(&ms->master->dev, &dev_attr_wcol_count); + device_remove_file(&ms->master->dev, &dev_attr_wcol_ticks); + device_remove_file(&ms->master->dev, &dev_attr_modf_count); + + spi_unregister_master(master); + spi_master_put(master); + iounmap(ms->regs); +} + +static struct of_device_id mpc52xx_spi_of_match[] __devinitdata = { + { .compatible = "fsl,mpc5200-spi", }, + {} +}; +MODULE_DEVICE_TABLE(of, mpc52xx_psc_spi_of_match); + +static struct of_platform_driver mpc52xx_spi_of_driver = { + .owner = THIS_MODULE, + .name = "mpc52xx-spi", + .match_table = mpc52xx_spi_of_match, + .probe = mpc52xx_spi_of_probe, + .remove = __exit_p(mpc52xx_spi_of_remove), +}; + +static int __init mpc52xx_spi_init(void) +{ + return of_register_platform_driver(&mpc52xx_spi_of_driver); +} +module_init(mpc52xx_spi_init); + +static void __exit mpc52xx_spi_exit(void) +{ + of_unregister_platform_driver(&mpc52xx_spi_of_driver); +} +module_exit(mpc52xx_spi_exit); + diff --git a/include/linux/spi/mpc52xx_spi.h b/include/linux/spi/mpc52xx_spi.h new file mode 100644 index 0000000..d1004cf --- /dev/null +++ b/include/linux/spi/mpc52xx_spi.h @@ -0,0 +1,10 @@ + +#ifndef INCLUDE_MPC5200_SPI_H +#define INCLUDE_MPC5200_SPI_H + +extern void mpc52xx_spi_set_premessage_hook(struct spi_master *master, + void (*hook)(struct spi_message *m, + void *context), + void *hook_context); + +#endif ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH 4/4] [CSB] Add new mpc5200-spi (non-psc) device driver 2008-05-16 19:36 ` [PATCH 4/4] [CSB] Add new mpc5200-spi (non-psc) device driver Grant Likely @ 2008-05-16 19:42 ` Grant Likely 0 siblings, 0 replies; 50+ messages in thread From: Grant Likely @ 2008-05-16 19:42 UTC (permalink / raw) To: linuxppc-dev, spi-devel-general, linux-kernel, dbrownell On Fri, May 16, 2008 at 1:36 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > From: Grant Likely <grant.likely@secretlab.ca> > > Unlike the old CSB driver, this driver uses the SPI infrastructure. er, this comment is *obviously* wrong. I'll fix it in the next version. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <20080516193054.28030.35126.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org>]
* Re: [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver [not found] ` <20080516193054.28030.35126.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> @ 2008-05-16 20:27 ` Jon Smirl [not found] ` <9e4733910805161327u4c42fd1dg5b09319d89db447c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Jon Smirl @ 2008-05-16 20:27 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > This series is a set of changes to allow the slaves on an SPI bus to be > described in the OF device tree (useful in arch/powerpc) and adds a driver > that uses it (the Freescale MPC5200 SoC's SPI device). Right now we have SPI hooked up to PSC3. Hardware engineer is gone but I'll see if I can get him to alter things to use the SPI controller. I have an old mail from him where he thinks the Phytec board is missing a signal needed to use the SPI controller. Is the current SPI driver working on PSC3? I have a MMC card wired up to it but I've never tried using it. I have the MPC5200 PSC SPI driver enabled and "MMC/SD over SPI" enabled in my kernel. The MMC bus gets created but there aren't any devices on it. Do we need something in the SPI driver so that the MMC layer can find it? Are you going to keeps this as two drivers or merge them? If it is two drivers there should be one entry in Kconfig and two sub choices for the types of drivers. > Please review and comment. David, I've included in this series my earlier > patch to change modalias from a pointer to a string as one of the later > patches depends on it. > > Cheers, > g. > > > -- > Grant Likely, B.Sc. P.Eng. > Secret Lab Technologies Ltd. > -- Jon Smirl jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <9e4733910805161327u4c42fd1dg5b09319d89db447c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver [not found] ` <9e4733910805161327u4c42fd1dg5b09319d89db447c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-16 20:48 ` Grant Likely [not found] ` <fa686aa40805161348t52b94956w112ef6926ff30892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-16 20:48 UTC (permalink / raw) To: Jon Smirl Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 2:27 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >> This series is a set of changes to allow the slaves on an SPI bus to be >> described in the OF device tree (useful in arch/powerpc) and adds a driver >> that uses it (the Freescale MPC5200 SoC's SPI device). > > Right now we have SPI hooked up to PSC3. Hardware engineer is gone but > I'll see if I can get him to alter things to use the SPI controller. I > have an old mail from him where he thinks the Phytec board is missing > a signal needed to use the SPI controller. While I'd appreciate the testing, I suspect that you really don't want to do that. The dedicated SPI controller isn't very good. It only does a byte at a time and so is rather slow. A PSC is SPI mode should be better (but I haven't tried it personally it yet). > > Is the current SPI driver working on PSC3? I have a MMC card wired up > to it but I've never tried using it. It should work. > I have the MPC5200 PSC SPI driver enabled and "MMC/SD over SPI" > enabled in my kernel. The MMC bus gets created but there aren't any > devices on it. Do we need something in the SPI driver so that the MMC > layer can find it? Yes, see patch 3 in my series. the PSC SPI driver needs to do something like this. SPI busses don't really do autodetection (but some sub-protocols, like MMC, do IIRC). > Are you going to keeps this as two drivers or merge them? If it is two > drivers there should be one entry in Kconfig and two sub choices for > the types of drivers. It will remain as two drivers. The devices are entirely separate. I don't think putting them under a single Kconfig is needed or a good idea. They both depend on PPC_MPC52xx anyway so they only show up if you're building for a 5200 platform. Thanks for the comments. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805161348t52b94956w112ef6926ff30892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver [not found] ` <fa686aa40805161348t52b94956w112ef6926ff30892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-16 21:25 ` Jon Smirl [not found] ` <9e4733910805161425i2d6cc034y3377af053a4198b5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Jon Smirl @ 2008-05-16 21:25 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Fri, May 16, 2008 at 2:27 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > >> This series is a set of changes to allow the slaves on an SPI bus to be > >> described in the OF device tree (useful in arch/powerpc) and adds a driver > >> that uses it (the Freescale MPC5200 SoC's SPI device). > > > > Right now we have SPI hooked up to PSC3. Hardware engineer is gone but > > I'll see if I can get him to alter things to use the SPI controller. I > > have an old mail from him where he thinks the Phytec board is missing > > a signal needed to use the SPI controller. > > > While I'd appreciate the testing, I suspect that you really don't want > to do that. The dedicated SPI controller isn't very good. It only > does a byte at a time and so is rather slow. A PSC is SPI mode should > be better (but I haven't tried it personally it yet). What is the device tree node for PSC3 supposed to look like when it has both serial and spi enabled? -- Jon Smirl jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <9e4733910805161425i2d6cc034y3377af053a4198b5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver [not found] ` <9e4733910805161425i2d6cc034y3377af053a4198b5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-16 21:32 ` Grant Likely [not found] ` <fa686aa40805161432w6b5243f9nb0d0c32a87d86d02-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 50+ messages in thread From: Grant Likely @ 2008-05-16 21:32 UTC (permalink / raw) To: Jon Smirl Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, May 16, 2008 at 3:25 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >> On Fri, May 16, 2008 at 2:27 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >> >> This series is a set of changes to allow the slaves on an SPI bus to be >> >> described in the OF device tree (useful in arch/powerpc) and adds a driver >> >> that uses it (the Freescale MPC5200 SoC's SPI device). >> > >> > Right now we have SPI hooked up to PSC3. Hardware engineer is gone but >> > I'll see if I can get him to alter things to use the SPI controller. I >> > have an old mail from him where he thinks the Phytec board is missing >> > a signal needed to use the SPI controller. >> >> >> While I'd appreciate the testing, I suspect that you really don't want >> to do that. The dedicated SPI controller isn't very good. It only >> does a byte at a time and so is rather slow. A PSC is SPI mode should >> be better (but I haven't tried it personally it yet). > > What is the device tree node for PSC3 supposed to look like when it > has both serial and spi enabled? The *PSC3 device* cannot support both serial and SPI at the same time. Only one mode works at a time... However, *PSC3 pin group* has can be configured to route both the *PSC3 device* and the *SPI device* signal out to the board at the same time. Pin routing is not something that is described by the device tree. It's viewed as a board level initialization thing, similar to how DDR RAM initialization is viewed. Ideally, the bootloader will write the correct value into port_config for pin routing and Linux will never need to touch it. If the bootloader cannot be changed, then board-specific platform code can be added to fixup the port_config setting. However, the drivers should never touch or care about pin routing. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <fa686aa40805161432w6b5243f9nb0d0c32a87d86d02-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver [not found] ` <fa686aa40805161432w6b5243f9nb0d0c32a87d86d02-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2008-05-16 21:42 ` Jon Smirl 0 siblings, 0 replies; 50+ messages in thread From: Jon Smirl @ 2008-05-16 21:42 UTC (permalink / raw) To: Grant Likely Cc: fabrizio.garetto-Re5JQEeQqe8AvxtiuMwx3w, linuxppc-dev-mnsaURCQ41sdnm+yROfE0A, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Fri, May 16, 2008 at 3:25 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > >> On Fri, May 16, 2008 at 2:27 PM, Jon Smirl <jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> > On 5/16/08, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > >> >> This series is a set of changes to allow the slaves on an SPI bus to be > >> >> described in the OF device tree (useful in arch/powerpc) and adds a driver > >> >> that uses it (the Freescale MPC5200 SoC's SPI device). > >> > > >> > Right now we have SPI hooked up to PSC3. Hardware engineer is gone but > >> > I'll see if I can get him to alter things to use the SPI controller. I > >> > have an old mail from him where he thinks the Phytec board is missing > >> > a signal needed to use the SPI controller. > >> > >> > >> While I'd appreciate the testing, I suspect that you really don't want > >> to do that. The dedicated SPI controller isn't very good. It only > >> does a byte at a time and so is rather slow. A PSC is SPI mode should > >> be better (but I haven't tried it personally it yet). > > > > What is the device tree node for PSC3 supposed to look like when it > > has both serial and spi enabled? > > > The *PSC3 device* cannot support both serial and SPI at the same time. > Only one mode works at a time... > > However, *PSC3 pin group* has can be configured to route both the > *PSC3 device* and the *SPI device* signal out to the board at the same > time. I need to talk to my hardware guy. He is using PSC3 for the boot console with the assumption that once booted it is ok to retask it to SPI. Serial console is only needed for software debugging. SSH works after boot and can replace the serial console. I'll trying changing my device tree entry from UART to SPI and boot. Hopefully I'll see the console until the SPI driver loads. > > Pin routing is not something that is described by the device tree. > It's viewed as a board level initialization thing, similar to how DDR > RAM initialization is viewed. Ideally, the bootloader will write the > correct value into port_config for pin routing and Linux will never > need to touch it. If the bootloader cannot be changed, then > board-specific platform code can be added to fixup the port_config > setting. However, the drivers should never touch or care about pin > routing. > > > Cheers, > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- Jon Smirl jonsmirl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2008-06-30 4:10 UTC | newest] Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-05-16 19:35 [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Grant Likely 2008-05-16 19:36 ` [PATCH 1/4] spi: Change modalias from a pointer to a character array Grant Likely 2008-05-16 19:36 ` [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration Grant Likely [not found] ` <20080516193608.28030.34968.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-22 0:17 ` David Brownell [not found] ` <200805211717.13206.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 6:43 ` Grant Likely [not found] ` <fa686aa40805232343x20031560j5659d203e25f494-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 6:54 ` Grant Likely [not found] ` <fa686aa40805232354g147acfcdx4753fce1a448ceb7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-06-30 4:08 ` David Brownell 2008-06-17 7:28 ` Grant Likely [not found] ` <fa686aa40806170028t2ccb679k22d2d3cea793ebc1-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-06-30 4:10 ` David Brownell 2008-05-16 19:36 ` [PATCH 3/4] spi: Add OF binding support for SPI busses Grant Likely [not found] ` <20080516193613.28030.13950.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-16 20:47 ` Randy Dunlap 2008-05-16 20:51 ` Grant Likely 2008-05-16 22:03 ` Anton Vorontsov 2008-05-16 22:14 ` Grant Likely [not found] ` <fa686aa40805161514r513d0eebt380a76f64abe8434-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-16 22:49 ` Anton Vorontsov 2008-05-17 5:02 ` Grant Likely [not found] ` <fa686aa40805162202m336aade4qd6cfa5b17d6f3892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-22 1:16 ` David Brownell [not found] ` <200805211816.10753.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 6:24 ` Grant Likely 2008-05-19 13:17 ` Guennadi Liakhovetski 2008-05-19 15:57 ` Grant Likely 2008-05-19 16:30 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.0805191811510.29559-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 2008-05-20 5:13 ` Grant Likely 2008-05-20 15:26 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.0805201650280.5283-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 2008-05-20 15:48 ` Grant Likely 2008-05-21 19:11 ` Segher Boessenkool [not found] ` <716a0f1b6c9a544b480c06a329072483-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> 2008-05-21 19:33 ` Grant Likely [not found] ` <fa686aa40805211233h72a258bpf8c945b9f662d6ee-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-23 2:26 ` David Brownell [not found] ` <200805221926.24112.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 6:25 ` Grant Likely [not found] ` <fa686aa40805232325w65d4f706i50798121a8cce263-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 7:13 ` David Brownell 2008-05-19 17:09 ` Gary Jennejohn [not found] ` <20080519190900.01ec3b2a-f7AvneZ2CE0iXleZOAq1AWD2FQJk+8+b@public.gmane.org> 2008-05-19 17:19 ` Anton Vorontsov 2008-05-21 15:19 ` Anton Vorontsov [not found] ` <20080521151928.GA28857-PHTr8nzUCjejyJ0x5qLZdcN33GVbZNy3@public.gmane.org> 2008-05-23 2:05 ` David Brownell [not found] ` <200805221905.32288.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 6:26 ` Grant Likely [not found] ` <fa686aa40805232326w35f455d1s274899160d47eccb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 16:50 ` Grant Likely [not found] ` <fa686aa40805240950ocd16b97y308a54c68efa28ef-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 17:14 ` Jochen Friedrich [not found] ` <48384D13.6010608-NIgtFMG+Po8@public.gmane.org> 2008-05-24 17:33 ` Grant Likely [not found] ` <fa686aa40805241033x128c30b0v826717cc879a712e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 17:45 ` David Brownell [not found] ` <200805241045.47448.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-25 4:56 ` Grant Likely 2008-05-24 17:43 ` David Brownell [not found] ` <200805221915.59878.david-b@pacbell.net> [not found] ` <200805221915.59878.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 6:31 ` Grant Likely [not found] ` <fa686aa40805232331p1bf2c1bcn8c46c21a094ef01e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-24 16:50 ` David Brownell [not found] ` <200805240950.43394.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-24 16:53 ` Grant Likely 2008-05-16 19:36 ` [PATCH 4/4] [CSB] Add new mpc5200-spi (non-psc) device driver Grant Likely 2008-05-16 19:42 ` Grant Likely [not found] ` <20080516193054.28030.35126.stgit-8FIgwK2HfyId2tlXD8uQ6/kpB+XfMlBf@public.gmane.org> 2008-05-16 20:27 ` [RFC PATCH 0/4] Describe SPI devices in the OF device tree and add mpc5200-spi driver Jon Smirl [not found] ` <9e4733910805161327u4c42fd1dg5b09319d89db447c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-16 20:48 ` Grant Likely [not found] ` <fa686aa40805161348t52b94956w112ef6926ff30892-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-16 21:25 ` Jon Smirl [not found] ` <9e4733910805161425i2d6cc034y3377af053a4198b5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-16 21:32 ` Grant Likely [not found] ` <fa686aa40805161432w6b5243f9nb0d0c32a87d86d02-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2008-05-16 21:42 ` Jon Smirl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).