Re: [PATCH 1/7] spi: Add support for device table matching

2009-08-04 Thread David Brownell
On Wednesday 29 July 2009, Ben Dooks wrote:
   struct spi_driver {
  + const struct spi_device_id *id_table;
  + int (*probe_id)(struct spi_device *spi,
  +     const struct spi_device_id *id);
 
 how about leaving it at just probe and have either a call or a field
 in the device that you can look at to see if this was a new style of
 call?
 
    int (*probe)(struct spi_device *spi);

For the record, if this is going to happen I think the
appropriate long-term solution is to have probe() take
the device_id just as it does with other busses.

Of course that involves changing *every* SPI driver...
and I'd rather not do that quite yet.




___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] spi: Add support for device table matching

2009-07-29 Thread Ben Dooks
On Wed, Jul 29, 2009 at 09:04:57PM +0400, Anton Vorontsov wrote:
 With this patch spi drivers can use standard spi_driver.id_table and
 MODULE_DEVICE_TABLE() mechanisms to bind against the devices. Just
 like we do with I2C drivers.
 
 This is useful when a single driver supports several variants of
 devices but it is not possible to detect them in run-time (like
 non-JEDEC chips probing in drivers/mtd/devices/m25p80.c), and
 when platform_data usage is overkill.
 
 This patch also makes life a lot easier on OpenFirmware platforms,
 since with OF we extensively use proper device IDs in modaliases.
 
 Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
 ---
  drivers/spi/spi.c   |   26 +-
  include/linux/mod_devicetable.h |   13 +
  include/linux/spi/spi.h |   10 --
  scripts/mod/file2alias.c|   13 +
  4 files changed, 59 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
 index 70845cc..1431bf2 100644
 --- a/drivers/spi/spi.c
 +++ b/drivers/spi/spi.c
 @@ -59,9 +59,24 @@ static struct device_attribute spi_dev_attrs[] = {
   * and the sysfs version makes coldplug work too.
   */
  
 +static const struct spi_device_id *spi_match_id(const struct spi_device_id 
 *id,
 + const struct spi_device *sdev)
 +{
 + while (id-name[0]) {
 + if (!strcmp(sdev-modalias, id-name))
 + return id;
 + id++;
 + }
 + return NULL;
 +}
 +
  static int spi_match_device(struct device *dev, struct device_driver *drv)
  {
   const struct spi_device *spi = to_spi_device(dev);
 + const struct spi_driver *sdrv = to_spi_driver(drv);
 +
 + if (sdrv-id_table)
 + return !!spi_match_id(sdrv-id_table, spi);
  
   return strcmp(spi-modalias, drv-name) == 0;
  }
 @@ -121,6 +136,13 @@ struct bus_type spi_bus_type = {
  };
  EXPORT_SYMBOL_GPL(spi_bus_type);
  
 +static int spi_drv_probe_id(struct device *dev)
 +{
 + const struct spi_driver *sdrv = to_spi_driver(dev-driver);
 + struct spi_device   *sdev = to_spi_device(dev);
 +
 + return sdrv-probe_id(sdev, spi_match_id(sdrv-id_table, sdev));
 +}
  
  static int spi_drv_probe(struct device *dev)
  {
 @@ -151,7 +173,9 @@ static void spi_drv_shutdown(struct device *dev)
  int spi_register_driver(struct spi_driver *sdrv)
  {
   sdrv-driver.bus = spi_bus_type;
 - if (sdrv-probe)
 + if (sdrv-probe_id)
 + sdrv-driver.probe = spi_drv_probe_id;
 + else if (sdrv-probe)
   sdrv-driver.probe = spi_drv_probe;
   if (sdrv-remove)
   sdrv-driver.remove = spi_drv_remove;
 diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
 index 1bf5900..9660dca 100644
 --- a/include/linux/mod_devicetable.h
 +++ b/include/linux/mod_devicetable.h
 @@ -399,6 +399,19 @@ struct i2c_device_id {
   __attribute__((aligned(sizeof(kernel_ulong_t;
  };
  
 +/* spi */
 +
 +#define SPI_NAME_SIZE20
 +
 +struct spi_device_id {
 + char name[SPI_NAME_SIZE];
 +#ifdef __KERNEL__
 + void *data;
 +#else
 + kernel_ulong_t data;
 +#endif
 +};
 +
  /* dmi */
  enum dmi_field {
   DMI_NONE,
 diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
 index c47c4b4..c8d92a1 100644
 --- a/include/linux/spi/spi.h
 +++ b/include/linux/spi/spi.h
 @@ -20,6 +20,7 @@
  #define __LINUX_SPI_H
  
  #include linux/device.h
 +#include linux/mod_devicetable.h
  
  /*
   * INTERFACES between SPI master-side drivers and SPI infrastructure.
 @@ -86,7 +87,7 @@ struct spi_device {
   int irq;
   void*controller_state;
   void*controller_data;
 - charmodalias[32];
 + charmodalias[SPI_NAME_SIZE];
  
   /*
* likely need more hooks for more protocol options affecting how
 @@ -145,6 +146,8 @@ struct spi_message;
  
  /**
   * struct spi_driver - Host side protocol driver
 + * @id_table: List of SPI devices supported by this driver
 + * @probe_id: Binds this driver to the spi device via id_table matching.
   * @probe: Binds this driver to the spi device.  Drivers can verify
   *   that the device is actually present, and may need to configure
   *   characteristics (such as bits_per_word) which weren't needed for
 @@ -170,6 +173,9 @@ struct spi_message;
   * MMC, RTC, filesystem character device nodes, and hardware monitoring.
   */
  struct spi_driver {
 + const struct spi_device_id *id_table;
 + int (*probe_id)(struct spi_device *spi,
 + const struct spi_device_id *id);

how about leaving it at just probe and have either a call or a field
in the device that you can look at to see if this was a new style of
call?

   int (*probe)(struct spi_device 

Re: [PATCH 1/7] spi: Add support for device table matching

2009-07-29 Thread Anton Vorontsov
On Wed, Jul 29, 2009 at 10:44:46PM +0100, Ben Dooks wrote:
[...]
  +   const struct spi_device_id *id_table;
  +   int (*probe_id)(struct spi_device *spi,
  +   const struct spi_device_id *id);
 
 how about leaving it at just probe and have either a call or a field
 in the device that you can look at to see if this was a new style of
 call?

There are no technical difficulties with that, but it would be
inconsitent wrt other device table-aware buses (i2c, pci, of).

Note that I'm getting rid of probe_id function in patch 5/7, as a
cleanup step. I want to keep new features and api cleanups
separate. That way it's easier to review the changes.

Thanks!

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] spi: Add support for device table matching

2009-07-29 Thread Anton Vorontsov
On Thu, Jul 30, 2009 at 02:32:23AM +0400, Anton Vorontsov wrote:
 On Wed, Jul 29, 2009 at 10:44:46PM +0100, Ben Dooks wrote:
 [...]
   + const struct spi_device_id *id_table;
   + int (*probe_id)(struct spi_device *spi,
   + const struct spi_device_id *id);
  
  how about leaving it at just probe and have either a call or a field
  in the device that you can look at to see if this was a new style of
  call?
 
 There are no technical difficulties with that, but it would be
 inconsitent wrt other device table-aware buses (i2c, pci, of).

Btw, I guess there are few reasons why other buses pass id via
probe() call:

- You'll have to store the id in device struct forever, while
  in most cases you only need it during probe(), then you don't
  need it at all;

- If you don't store id in the device struct, you'll have
  to look up the device table twice (at first during bus-match(),
  and second time in drivers' probe() hook, i.e.
  probe(struct bus_dev *dev) {
id = bus_get_devid(dev); /* here */
  }

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/7] spi: Add support for device table matching

2009-07-29 Thread Anton Vorontsov
On Thu, Jul 30, 2009 at 02:40:50AM +0400, Anton Vorontsov wrote:
[...]
 - If you don't store id in the device struct, you'll have
   to look up the device table twice (at first during bus-match(),
   and second time in drivers' probe() hook, i.e.
   probe(struct bus_dev *dev) {
   id = bus_get_devid(dev); /* here */
   }

Hm... actually, we're doing this anyway, but in spi core.

So, doing something like spi_get_device_id() might be a good
idea.

Thanks,

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev