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 ---
drivers/misc/cros_ec_i2c.c | 19 ----------- drivers/misc/cros_ec_spi.c | 19 ----------- drivers/mtd/spi/sandbox.c | 5 +++ drivers/mtd/spi/sf_probe.c | 3 +- drivers/spi/soft_spi.c | 9 ------ drivers/spi/spi-uclass.c | 79 ++++++++++++++++++++++++++-------------------- include/spi.h | 29 +++++++++++++---- test/dm/spi.c | 6 ++-- 8 files changed, 78 insertions(+), 91 deletions(-)
diff --git a/drivers/misc/cros_ec_i2c.c b/drivers/misc/cros_ec_i2c.c index 13017e1..3da8556 100644 --- a/drivers/misc/cros_ec_i2c.c +++ b/drivers/misc/cros_ec_i2c.c @@ -180,25 +180,6 @@ int cros_ec_i2c_init(struct cros_ec_dev *dev, const void *blob) #ifdef CONFIG_DM_CROS_EC static 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/misc/cros_ec_spi.c b/drivers/misc/cros_ec_spi.c index 92eace2..7f696d9 100644 --- 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 static 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 106dda9..d576d31 100644 --- 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 ce9987f..4103723 100644 --- 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 9f7d80e..6ae45f5 100644 --- 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 2c134eb..14e9337 100644 --- 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; @@ -112,6 +122,13 @@ int spi_child_pre_probe(struct udevice *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;
return 0; @@ -119,9 +136,9 @@ int spi_child_pre_probe(struct udevice *dev)
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 +147,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 +235,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 +251,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 +270,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 +290,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 +338,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 +353,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 +367,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 ec17bd0..5537a13 100644 --- a/include/spi.h +++ b/include/spi.h @@ -60,6 +60,23 @@ 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). + * + * @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 */
/** @@ -97,8 +114,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 +562,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 61b5b25..c7ee652 100644 --- 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,