dm: spi: Move slave details to child platdata

At present we go through various contortions to store the SPI slave's chip
select in its private data. This only exists when the slave is active so
must be set up when it is probed. Until the device is probed we don't
actually know what chip select it will appear on.

However, now that we can support per-child platform data, we can use that
instead. This allows us to set up the chip select when the child is bound,
and avoid the messy contortions.

Unfortunately this is a fairly large change and it seems to be difficult to
break it down further.

Signed-off-by: Simon Glass <sjg@chromium.org>

diff --git a/drivers/misc/cros_ec_spi.c b/drivers/misc/cros_ec_spi.c
index e6dba298b1e9f2a37b31b4511cf9c8902c9ff227..25a5a0468e6bb52944fbb0e71fb16a8e3d96c280 100644 (file)
--- a/drivers/misc/cros_ec_spi.c
+++ b/drivers/misc/cros_ec_spi.c
@@ -202,25 +202,6 @@ int cros_ec_spi_init(struct cros_ec_dev *dev, const void *blob)
 #ifdef CONFIG_DM_CROS_EC
 int cros_ec_probe(struct udevice *dev)
 {
-       struct spi_slave *slave = dev_get_parentdata(dev);
-       int ret;
-
-       /*
-        * TODO(sjg@chromium.org)
-        *
-        * This is really horrible at present. It is an artifact of removing
-        * the child_pre_probe() method for SPI. Everything here could go in
-        * an automatic function, except that spi_get_bus_and_cs() wants to
-        * set it up manually and call device_probe_child().
-        *
-        * The solution may be to re-enable the child_pre_probe() method for
-        * SPI and have it do nothing if the child is already passed in via
-        * device_probe_child().
-        */
-       slave->dev = dev;
-       ret = spi_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, slave);
-       if (ret)
-               return ret;
        return cros_ec_register(dev);
 }
 

diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
index 106dda9ccc52b8850f84d0575b774537dab0c6e9..d576d31243a7a90a7213466d7b3ce600a2d1d84f 100644 (file)
--- a/drivers/mtd/spi/sandbox.c
+++ b/drivers/mtd/spi/sandbox.c
@@ -590,6 +590,11 @@ int sandbox_sf_bind_emul(struct sandbox_state *state, int busnum, int cs,
 
 void sandbox_sf_unbind_emul(struct sandbox_state *state, int busnum, int cs)
 {
+       struct udevice *dev;
+
+       dev = state->spi[busnum][cs].emul;
+       device_remove(dev);
+       device_unbind(dev);
        state->spi[busnum][cs].emul = NULL;
 }
 

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index ce9987fd1a8770aeb8d948030a792553b511ed80..41037238590b650cfe12cfe59c2edd960170db27 100644 (file)
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -481,11 +481,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
 int spi_flash_std_probe(struct udevice *dev)
 {
        struct spi_slave *slave = dev_get_parentdata(dev);
+       struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
        struct spi_flash *flash;
 
        flash = dev->uclass_priv;
        flash->dev = dev;
-       debug("%s: slave=%p, cs=%d\n", __func__, slave, slave->cs);
+       debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs);
        return spi_flash_probe_slave(slave, flash);
 }
 

diff --git a/drivers/spi/soft_spi.c b/drivers/spi/soft_spi.c
index 9f7d80ee4dada9c46be3251aa690b1a47a0c0e30..6ae45f5377ab89ba647387029fdfa771dc1ee8ed 100644 (file)
--- a/drivers/spi/soft_spi.c
+++ b/drivers/spi/soft_spi.c
@@ -179,14 +179,6 @@ static int soft_spi_set_mode(struct udevice *dev, unsigned int mode)
        return 0;
 }
 
-static int soft_spi_child_pre_probe(struct udevice *dev)
-{
-       struct spi_slave *slave = dev_get_parentdata(dev);
-
-       slave->dev = dev;
-       return spi_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, slave);
-}
-
 static const struct dm_spi_ops soft_spi_ops = {
        .claim_bus      = soft_spi_claim_bus,
        .release_bus    = soft_spi_release_bus,
@@ -241,5 +233,4 @@ U_BOOT_DRIVER(soft_spi) = {
        .platdata_auto_alloc_size = sizeof(struct soft_spi_platdata),
        .priv_auto_alloc_size = sizeof(struct soft_spi_priv),
        .probe  = soft_spi_probe,
-       .child_pre_probe        = soft_spi_child_pre_probe,
 };

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index 2c134eb39f77ce20d57668de249d532177b64c48..63a6217cc62d0085b6c4ba259415fae514651ba9 100644 (file)
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -98,11 +98,21 @@ int spi_post_bind(struct udevice *dev)
        return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false);
 }
 
-int spi_post_probe(struct udevice *dev)
+int spi_child_post_bind(struct udevice *dev)
 {
-       struct dm_spi_bus *spi = dev->uclass_priv;
+       struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
 
-       spi->max_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+       if (dev->of_offset == -1)
+               return 0;
+
+       return spi_slave_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat);
+}
+
+int spi_post_probe(struct udevice *bus)
+{
+       struct dm_spi_bus *spi = bus->uclass_priv;
+
+       spi->max_hz = fdtdec_get_int(gd->fdt_blob, bus->of_offset,
                                     "spi-max-frequency", 0);
 
        return 0;
@@ -110,18 +120,29 @@ int spi_post_probe(struct udevice *dev)
 
 int spi_child_pre_probe(struct udevice *dev)
 {
+       struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
        struct spi_slave *slave = dev_get_parentdata(dev);
 
+       /*
+        * This is needed because we pass struct spi_slave around the place
+        * instead slave->dev (a struct udevice). So we have to have some
+        * way to access the slave udevice given struct spi_slave. Once we
+        * change the SPI API to use udevice instead of spi_slave, we can
+        * drop this.
+        */
        slave->dev = dev;
 
+       slave->max_hz = plat->max_hz;
+       slave->mode = plat->mode;
+
        return 0;
 }
 
 int spi_chip_select(struct udevice *dev)
 {
-       struct spi_slave *slave = dev_get_parentdata(dev);
+       struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
 
-       return slave ? slave->cs : -ENOENT;
+       return plat ? plat->cs : -ENOENT;
 }
 
 int spi_find_chip_select(struct udevice *bus, int cs, struct udevice **devp)
@@ -130,17 +151,11 @@ int spi_find_chip_select(struct udevice *bus, int cs, struct udevice **devp)
 
        for (device_find_first_child(bus, &dev); dev;
             device_find_next_child(&dev)) {
-               struct spi_slave store;
-               struct spi_slave *slave = dev_get_parentdata(dev);
+               struct dm_spi_slave_platdata *plat;
 
-               if (!slave)  {
-                       slave = &store;
-                       spi_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
-                                              slave);
-               }
-               debug("%s: slave=%p, cs=%d\n", __func__, slave,
-                     slave ? slave->cs : -1);
-               if (slave && slave->cs == cs) {
+               plat = dev_get_parent_platdata(dev);
+               debug("%s: plat=%p, cs=%d\n", __func__, plat, plat->cs);
+               if (plat->cs == cs) {
                        *devp = dev;
                        return 0;
                }
@@ -224,7 +239,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
                       struct udevice **busp, struct spi_slave **devp)
 {
        struct udevice *bus, *dev;
-       struct spi_slave *slave;
        bool created = false;
        int ret;
 
@@ -241,11 +255,17 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
         * SPI flash chip - we will bind to the correct driver.
         */
        if (ret == -ENODEV && drv_name) {
+               struct dm_spi_slave_platdata *plat;
+
                debug("%s: Binding new device '%s', busnum=%d, cs=%d, driver=%s\n",
                      __func__, dev_name, busnum, cs, drv_name);
                ret = device_bind_driver(bus, drv_name, dev_name, &dev);
                if (ret)
                        return ret;
+               plat = dev_get_parent_platdata(dev);
+               plat->cs = cs;
+               plat->max_hz = speed;
+               plat->mode = mode;
                created = true;
        } else if (ret) {
                printf("Invalid chip select %d:%d (err=%d)\n", busnum, cs,
@@ -254,23 +274,13 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
        }
 
        if (!device_active(dev)) {
-               slave = (struct spi_slave *)calloc(1,
-                                                  sizeof(struct spi_slave));
-               if (!slave) {
-                       ret = -ENOMEM;
-                       goto err;
-               }
+               struct spi_slave *slave;
 
-               ret = spi_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
-                                            slave);
+               ret = device_probe(dev);
                if (ret)
                        goto err;
-               slave->cs = cs;
+               slave = dev_get_parentdata(dev);
                slave->dev = dev;
-               ret = device_probe_child(dev, slave);
-               free(slave);
-               if (ret)
-                       goto err;
        }
 
        ret = spi_set_speed_mode(bus, speed, mode);
@@ -284,6 +294,8 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
        return 0;
 
 err:
+       debug("%s: Error path, credted=%d, device '%s'\n", __func__,
+             created, dev->name);
        if (created) {
                device_remove(dev);
                device_unbind(dev);
@@ -330,13 +342,13 @@ void spi_free_slave(struct spi_slave *slave)
        slave->dev = NULL;
 }
 
-int spi_ofdata_to_platdata(const void *blob, int node,
-                          struct spi_slave *spi)
+int spi_slave_ofdata_to_platdata(const void *blob, int node,
+                                struct dm_spi_slave_platdata *plat)
 {
        int mode = 0;
 
-       spi->cs = fdtdec_get_int(blob, node, "reg", -1);
-       spi->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", 0);
+       plat->cs = fdtdec_get_int(blob, node, "reg", -1);
+       plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", 0);
        if (fdtdec_get_bool(blob, node, "spi-cpol"))
                mode |= SPI_CPOL;
        if (fdtdec_get_bool(blob, node, "spi-cpha"))
@@ -345,7 +357,7 @@ int spi_ofdata_to_platdata(const void *blob, int node,
                mode |= SPI_CS_HIGH;
        if (fdtdec_get_bool(blob, node, "spi-half-duplex"))
                mode |= SPI_PREAMBLE;
-       spi->mode = mode;
+       plat->mode = mode;
 
        return 0;
 }
@@ -359,6 +371,9 @@ UCLASS_DRIVER(spi) = {
        .child_pre_probe = spi_child_pre_probe,
        .per_device_auto_alloc_size = sizeof(struct dm_spi_bus),
        .per_child_auto_alloc_size = sizeof(struct spi_slave),
+       .per_child_platdata_auto_alloc_size =
+                       sizeof(struct dm_spi_slave_platdata),
+       .child_post_bind = spi_child_post_bind,
 };
 
 UCLASS_DRIVER(spi_generic) = {

diff --git a/include/spi.h b/include/spi.h
index ec17bd0bcc8963e4c4e402e57c0003d3b839b568..c58e453559616eb1cbbca36ee20704afcd4ebecc 100644 (file)
--- a/include/spi.h
+++ b/include/spi.h
@@ -56,20 +56,42 @@
 #define SPI_DEFAULT_WORDLEN 8
 
 #ifdef CONFIG_DM_SPI
+/* TODO(sjg@chromium.org): Remove this and use max_hz from struct spi_slave */
 struct dm_spi_bus {
        uint max_hz;
 };
 
+/**
+ * struct dm_spi_platdata - platform data for all SPI slaves
+ *
+ * This describes a SPI slave, a child device of the SPI bus. To obtain this
+ * struct from a spi_slave, use dev_get_parent_platdata(dev) or
+ * dev_get_parent_platdata(slave->dev).
+ *
+ * This data is immuatable. Each time the device is probed, @max_hz and @mode
+ * will be copied to struct spi_slave.
+ *
+ * @cs:                Chip select number (0..n-1)
+ * @max_hz:    Maximum bus speed that this slave can tolerate
+ * @mode:      SPI mode to use for this device (see SPI mode flags)
+ */
+struct dm_spi_slave_platdata {
+       unsigned int cs;
+       uint max_hz;
+       uint mode;
+};
+
 #endif /* CONFIG_DM_SPI */
 
 /**
  * struct spi_slave - Representation of a SPI slave
  *
  * For driver model this is the per-child data used by the SPI bus. It can
- * be accessed using dev_get_parentdata() on the slave device. Each SPI
- * driver should define this child data in its U_BOOT_DRIVER() definition:
- *
- *     .per_child_auto_alloc_size      = sizeof(struct spi_slave),
+ * be accessed using dev_get_parentdata() on the slave device. The SPI uclass
+ * sets uip per_child_auto_alloc_size to sizeof(struct spi_slave), and the
+ * driver should not override it. Two platform data fields (max_hz and mode)
+ * are copied into this structure to provide an initial value. This allows
+ * them to be changed, since we should never change platform data in drivers.
  *
  * If not using driver model, drivers are expected to extend this with
  * controller-specific data.
@@ -97,8 +119,8 @@ struct spi_slave {
        uint mode;
 #else
        unsigned int bus;
-#endif
        unsigned int cs;
+#endif
        u8 op_mode_rx;
        u8 op_mode_tx;
        unsigned int wordlen;
@@ -545,16 +567,16 @@ int spi_chip_select(struct udevice *slave);
 int spi_find_chip_select(struct udevice *bus, int cs, struct udevice **devp);
 
 /**
- * spi_ofdata_to_platdata() - decode standard SPI platform data
+ * spi_slave_ofdata_to_platdata() - decode standard SPI platform data
  *
- * This decodes the speed and mode from a device tree node and puts it into
- * the spi_slave structure.
+ * This decodes the speed and mode for a slave from a device tree node
  *
  * @blob:      Device tree blob
  * @node:      Node offset to read from
- * @spi:       Place to put the decoded information
+ * @plat:      Place to put the decoded information
  */
-int spi_ofdata_to_platdata(const void *blob, int node, struct spi_slave *spi);
+int spi_slave_ofdata_to_platdata(const void *blob, int node,
+                                struct dm_spi_slave_platdata *plat);
 
 /**
  * spi_cs_info() - Check information on a chip select

diff --git a/test/dm/spi.c b/test/dm/spi.c
index 61b5b2548c42e4e9895ef95345b5e9e99bc9c52a..c7ee65207b0472d3d360e26a7a87629849544c29 100644 (file)
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -36,7 +36,6 @@ static int dm_test_spi_find(struct dm_test_state *dms)
        ut_asserteq(0, uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus));
        ut_assertok(spi_cs_info(bus, cs, &info));
        of_offset = info.dev->of_offset;
-       sandbox_sf_unbind_emul(state_get_current(), busnum, cs);
        device_remove(info.dev);
        device_unbind(info.dev);
 
@@ -45,7 +44,7 @@ static int dm_test_spi_find(struct dm_test_state *dms)
         * reports that CS 0 is present
         */
        ut_assertok(spi_cs_info(bus, cs, &info));
-       ut_asserteq_ptr(info.dev, NULL);
+       ut_asserteq_ptr(NULL, info.dev);
 
        /* This finds nothing because we removed the device */
        ut_asserteq(-ENODEV, spi_find_bus_and_cs(busnum, cs, &bus, &dev));
@@ -62,8 +61,9 @@ static int dm_test_spi_find(struct dm_test_state *dms)
        ut_asserteq(-ENOENT, spi_get_bus_and_cs(busnum, cs, speed, mode,
                                                "spi_flash_std", "name", &bus,
                                                &slave));
+       sandbox_sf_unbind_emul(state_get_current(), busnum, cs);
        ut_assertok(spi_cs_info(bus, cs, &info));
-       ut_asserteq_ptr(info.dev, NULL);
+       ut_asserteq_ptr(NULL, info.dev);
 
        /* Add the emulation and try again */
        ut_assertok(sandbox_sf_bind_emul(state, busnum, cs, bus, of_offset,