Hi Jagan, On 28 August 2014 02:58, Jagan Teki <jagannadh.t...@gmail.com> wrote: > On 15 July 2014 06:26, Simon Glass <s...@chromium.org> wrote: >> Add a uclass which provides access to SPI buses and includes operations >> required by SPI. >> >> For a time driver model will need to co-exist with the legacy SPI interface >> so some parts of the header file are changed depending on which is in use. >> The exports are adjusted also since some functions are not available with >> driver model. >> >> Boards must define CONFIG_DM_SPI to use driver model for SPI. >> >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> common/exports.c | 4 +- >> drivers/spi/Makefile | 4 + >> drivers/spi/spi-uclass.c | 253 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> include/dm/uclass-id.h | 1 + >> include/spi.h | 140 ++++++++++++++++++++++++++ >> 5 files changed, 401 insertions(+), 1 deletion(-) >> create mode 100644 drivers/spi/spi-uclass.c >> >> diff --git a/common/exports.c b/common/exports.c >> index b97ca48..88fcfc8 100644 >> --- a/common/exports.c >> +++ b/common/exports.c >> @@ -27,10 +27,12 @@ unsigned long get_version(void) >> # define i2c_write dummy >> # define i2c_read dummy >> #endif >> -#ifndef CONFIG_CMD_SPI >> +#if !defined(CONFIG_CMD_SPI) || defined(CONFIG_DM_SPI) >> # define spi_init dummy >> # define spi_setup_slave dummy >> # define spi_free_slave dummy >> +#endif >> +#ifndef CONFIG_CMD_SPI >> # define spi_claim_bus dummy >> # define spi_release_bus dummy >> # define spi_xfer dummy >> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile >> index f02c35a..d1f1dd0 100644 >> --- a/drivers/spi/Makefile >> +++ b/drivers/spi/Makefile >> @@ -6,7 +6,11 @@ >> # >> >> # There are many options which enable SPI, so make this library available >> +ifdef CONFIG_DM_SPI >> +obj-y += spi-uclass.o >> +else >> obj-y += spi.o >> +endif >> >> obj-$(CONFIG_EP93XX_SPI) += ep93xx_spi.o >> obj-$(CONFIG_ALTERA_SPI) += altera_spi.o >> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c >> new file mode 100644 >> index 0000000..4057bce >> --- /dev/null >> +++ b/drivers/spi/spi-uclass.c >> @@ -0,0 +1,253 @@ >> +/* >> + * Copyright (c) 2014 Google, Inc >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <dm.h> >> +#include <errno.h> >> +#include <fdtdec.h> >> +#include <spi.h> >> +#include <dm/device-internal.h> >> +#include <dm/uclass-internal.h> >> +#include <dm/root.h> >> +#include <dm/lists.h> >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +static int spi_set_speed_mode(struct udevice *bus, int speed, int mode) >> +{ >> + struct dm_spi_ops *ops; >> + int ret; >> + >> + ops = spi_get_ops(bus); >> + if (ops->set_speed) >> + ret = (*ops->set_speed)(bus, speed); >> + else >> + ret = -EINVAL; >> + if (ret) { >> + printf("Cannot set speed (err=%d)\n", ret); >> + return ret; >> + } >> + >> + ops = spi_get_ops(bus); >> + if (ops->set_mode) >> + ret = (*ops->set_mode)(bus, mode); >> + else >> + ret = -EINVAL; >> + if (ret) { >> + printf("Cannot set mode (err=%d)\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +int spi_claim_bus(struct spi_slave *slave) >> +{ >> + struct udevice *dev = slave->dev; >> + struct udevice *bus = dev->parent; >> + struct dm_spi_ops *ops = spi_get_ops(bus); >> + struct dm_spi_bus *spi = bus->uclass_priv; >> + int speed; >> + int ret; >> + >> + speed = slave->max_hz; >> + if (spi->max_hz) { >> + if (speed) >> + speed = min(speed, spi->max_hz); >> + else >> + speed = spi->max_hz; >> + } >> + if (!speed) >> + speed = 100000; >> + ret = spi_set_speed_mode(bus, speed, slave->mode); >> + if (ret) >> + return ret; >> + >> + return ops->claim_bus ? ops->claim_bus(bus) : 0; >> +} >> + >> +void spi_release_bus(struct spi_slave *slave) >> +{ >> + struct udevice *dev = slave->dev; >> + struct udevice *bus = dev->parent; >> + struct dm_spi_ops *ops = spi_get_ops(bus); >> + >> + if (ops->release_bus) >> + spi_get_ops(bus)->release_bus(bus); >> +} >> + >> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, >> + const void *dout, void *din, unsigned long flags) >> +{ >> + struct udevice *dev = slave->dev; >> + struct udevice *bus = dev->parent; >> + >> + if (bus->uclass->uc_drv->id != UCLASS_SPI) >> + return -EOPNOTSUPP; >> + >> + return spi_get_ops(bus)->xfer(bus, dev, bitlen, dout, din, flags); >> +} > > Cleared all these calls, as individual drivers/spi* will setup their > ops and fills > the priv data. So this uclass will call accordingly - correct? add if > I'm missing anything.
Yes so far as I understand you, that is correct. > >> + >> +int spi_post_bind(struct udevice *dev) >> +{ >> + /* Scan the bus for devices */ >> + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); >> +} >> + >> +int spi_post_probe(struct udevice *dev) >> +{ >> + struct dm_spi_bus *spi = dev->uclass_priv; >> + >> + spi->max_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "spi-max-frequency", 0); >> + >> + return 0; >> +} > > Anyway each spi driver will fills the data from dtb - like > spi-max-frequency, why the spi > core (uclass) will do the same. Because the overall SPI peripheral has a maximum speed too. We should allow it to run at a nominal speed, but slow down for particular peripherals if needed. > > Can't we reuse the spi_slave, why dm_spi_bus? > >> + >> +int spi_bind_device(struct udevice *bus, int cs, const char *drv_name, >> + const char *dev_name, struct udevice **slavep) >> +{ >> + struct driver *drv; >> + int ret; >> + >> + drv = lists_driver_lookup_name(drv_name); >> + if (!drv) { >> + puts("Cannot find spi_flash_std driver\n"); >> + return -ENOENT; >> + } >> + ret = device_bind(bus, drv, dev_name, NULL, -1, slavep); >> + if (ret) { >> + printf("Cannot create device named '%s' (err=%d)\n", >> + dev_name, ret); >> + return ret; >> + } >> + (*slavep)->req_seq = cs; >> + >> + return 0; >> +} >> + >> +int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp, >> + struct udevice **devp) >> +{ >> + struct udevice *bus, *dev; >> + int ret; >> + >> + ret = uclass_find_device_by_seq(UCLASS_SPI, busnum, false, &bus); >> + if (ret) >> + return ret; >> + ret = device_find_child_by_seq(bus, cs, false, &dev); >> + if (ret) >> + return ret; >> + *busp = bus; >> + *devp = dev; >> + >> + return ret; >> +} >> + >> +int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, >> + const char *drv_name, const char *dev_name, >> + struct udevice **devp, struct spi_slave **slavep) >> +{ >> + struct udevice *bus, *dev; >> + int ret; >> + >> + ret = uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus); >> + if (ret) { >> + printf("Invalid bus %d (err=%d)\n", busnum, ret); >> + return ret; >> + } >> + ret = device_get_child_by_seq(bus, cs, &dev); >> + >> + /** >> + * If there is no such device, create one automatically. This means >> + * that we don't need a device tree node or platform data for the >> + * SPI flash chip - we will bind to the correct driver. >> + */ >> + if (ret == -ENODEV && drv_name) { >> + ret = spi_bind_device(bus, cs, drv_name, dev_name, &dev); >> + if (ret) >> + return ret; >> + } >> + if (ret) { >> + printf("Invalid chip select %d:%d (err=%d)\n", busnum, cs, >> + ret); >> + return ret; >> + } >> + >> + ret = spi_set_speed_mode(bus, speed, mode); >> + if (ret) >> + return ret; >> + >> + *devp = bus; >> + *slavep = dev_get_parentdata(dev); >> + >> + return 0; >> +} > > I guess this is one of the replacement for spi_setup_slave() in spi drivers, > so > how bus and cs member will populate to drivers. > > Usually bus and cs members from spi_slave will populate to drivers through > "sf probe" so spi_cs_activate will set the respective chip-select and > along with > reg_base in driver select through bus value. > > spi_setup_slave() { > ... > zslave->base = get_zynq_spi_base(bus); > ... > } > > How these will handling with dm? The base should be in the device tree, or in platform data. The bus number should not be visible to the driver. > >> + >> +/* Compatibility function - to be removed */ >> +struct spi_slave *spi_setup_slave_fdt(const void *blob, int node, >> + int bus_node) >> +{ >> + struct udevice *bus, *dev; >> + int ret; >> + >> + ret = uclass_get_device_by_of_offset(UCLASS_SPI, bus_node, &bus); >> + if (ret) >> + return NULL; >> + ret = device_get_child_by_of_offset(bus, node, &dev); >> + if (ret) >> + return NULL; >> + return dev_get_parentdata(dev); >> +} >> + >> +/* Compatibility function - to be removed */ >> +struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, >> + unsigned int speed, unsigned int mode) >> +{ >> + struct spi_slave *slave; >> + struct udevice *dev; >> + int ret; >> + >> + ret = spi_get_bus_and_cs(busnum, cs, speed, mode, NULL, 0, &dev, >> + &slave); >> + if (ret) >> + return NULL; >> + >> + return slave; >> +} >> + >> +void spi_free_slave(struct spi_slave *slave) >> +{ >> + device_remove(slave->dev); >> + slave->dev = NULL; >> +} >> + >> +int spi_ofdata_to_platdata(const void *blob, int node, >> + struct spi_slave *spi) >> +{ >> + int mode = 0; >> + >> + spi->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")) >> + mode |= SPI_CPHA; >> + if (fdtdec_get_bool(blob, node, "spi-cs-high")) >> + mode |= SPI_CS_HIGH; >> + if (fdtdec_get_bool(blob, node, "spi-half-duplex")) >> + mode |= SPI_PREAMBLE; >> + spi->mode = mode; >> + >> + return 0; >> +} >> + >> +UCLASS_DRIVER(spi) = { >> + .id = UCLASS_SPI, >> + .name = "spi", >> + .post_bind = spi_post_bind, >> + .post_probe = spi_post_probe, >> + .per_device_auto_alloc_size = sizeof(struct dm_spi_bus), >> +}; >> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h >> index 7f0e37b..8207483 100644 >> --- a/include/dm/uclass-id.h >> +++ b/include/dm/uclass-id.h >> @@ -22,6 +22,7 @@ enum uclass_id { >> /* U-Boot uclasses start here */ >> UCLASS_GPIO, /* Bank of general-purpose I/O pins */ >> UCLASS_SERIAL, /* Serial UART */ >> + UCLASS_SPI, /* SPI bus */ >> >> UCLASS_COUNT, >> UCLASS_INVALID = -1, >> diff --git a/include/spi.h b/include/spi.h >> index b673be2..b5e9347 100644 >> --- a/include/spi.h >> +++ b/include/spi.h >> @@ -54,11 +54,19 @@ >> >> #define SPI_DEFAULT_WORDLEN 8 >> >> +#ifdef CONFIG_DM_SPI >> +struct dm_spi_bus { >> + uint max_hz; >> +}; >> + >> +#endif /* CONFIG_DM_SPI */ >> + > > No idea why this should require, max_hz is already a part of spi_slave > and handling of these like getting from dtb and process and fill on priv > data should be part of individual drivers and filling spi_slave member should > be part of uclass - based on my understanding. > > Any comments? See above - the bus has its own maximum speed in many cases. > > >> /** >> * struct spi_slave - Representation of a SPI slave >> * >> * Drivers are expected to extend this with controller-specific data. >> * >> + * @dev: SPI slave device > > Missing comments for max_hz and mode > >> * @bus: ID of the bus that the slave is attached to. >> * @cs: ID of the chip select connected to the slave. >> * @op_mode_rx: SPI RX operation mode. >> @@ -71,8 +79,14 @@ >> * @flags: Indication of SPI flags. >> */ >> struct spi_slave { >> +#ifdef CONFIG_DM_SPI >> + struct udevice *dev; /* struct spi_slave is dev->parentdata */ >> + uint max_hz; >> + uint mode; >> +#else >> unsigned int bus; >> unsigned int cs; >> +#endif >> u8 op_mode_rx; >> u8 op_mode_tx; >> unsigned int wordlen; >> @@ -220,6 +234,8 @@ int spi_set_wordlen(struct spi_slave *slave, unsigned >> int wordlen); >> int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void >> *dout, >> void *din, unsigned long flags); >> >> +#ifndef CONFIG_DM_SPI >> + >> /** >> * Determine if a SPI chipselect is valid. >> * This function is provided by the board if the low-level SPI driver >> @@ -255,6 +271,7 @@ void spi_cs_deactivate(struct spi_slave *slave); >> * @hz: The transfer speed >> */ >> void spi_set_speed(struct spi_slave *slave, uint hz); >> +#endif >> >> /** >> * Write 8 bits, then read 8 bits. >> @@ -305,4 +322,127 @@ struct spi_slave *spi_setup_slave_fdt(const void >> *blob, int slave_node, >> struct spi_slave *spi_base_setup_slave_fdt(const void *blob, int busnum, >> int node); >> >> +#ifdef CONFIG_DM_SPI >> + >> +/** >> + * struct struct dm_spi_ops - Driver model SPI operations >> + * >> + * The uclass interface is implemented by all SPI devices which use >> + * driver model. >> + */ >> +struct dm_spi_ops { >> + /** >> + * Claim the bus and prepare it for communication. >> + * >> + * The device privided is the slave device. It's parent controller >> + * will be used to provide the communication. >> + * >> + * This must be called before doing any transfers with a SPI slave. >> It >> + * will enable and initialize any SPI hardware as necessary, and make >> + * sure that the SCK line is in the correct idle state. It is not >> + * allowed to claim the same bus for several slaves without releasing >> + * the bus in between. >> + * >> + * @device: The SPI slave >> + * >> + * Returns: 0 if the bus was claimed successfully, or a negative >> value >> + * if it wasn't. >> + */ >> + int (*claim_bus)(struct udevice *device); >> + >> + /** >> + * Release the SPI bus >> + * >> + * This must be called once for every call to spi_claim_bus() after >> + * all transfers have finished. It may disable any SPI hardware as >> + * appropriate. >> + * >> + * @device: The SPI slave >> + */ >> + int (*release_bus)(struct udevice *device); >> + >> + /** >> + * Set the word length for SPI transactions >> + * >> + * Set the word length (number of bits per word) for SPI >> transactions. >> + * >> + * @device: The SPI slave >> + * @wordlen: The number of bits in a word >> + * >> + * Returns: 0 on success, -ve on failure. >> + */ >> + int (*set_wordlen)(struct udevice *deiuce, unsigned int wordlen); >> + >> + /** >> + * SPI transfer >> + * >> + * This writes "bitlen" bits out the SPI MOSI port and simultaneously >> + * clocks "bitlen" bits in the SPI MISO port. That's just the way >> SPI >> + * works. >> + * >> + * The source of the outgoing bits is the "dout" parameter and the >> + * destination of the input bits is the "din" parameter. Note that >> + * "dout" and "din" can point to the same memory location, in which >> + * case the input data overwrites the output data (since both are >> + * buffered by temporary variables, this is OK). >> + * >> + * spi_xfer() interface: >> + * @device: The SPI bus which will be sending/receiving the data. >> + * @slave: The slave device to communicate with >> + * @bitlen: How many bits to write and read. >> + * @dout: Pointer to a string of bits to send out. The bits >> are >> + * held in a byte array and are sent MSB first. >> + * @din: Pointer to a string of bits that will be filled in. >> + * @flags: A bitwise combination of SPI_XFER_* flags. >> + * >> + * Returns: 0 on success, not -1 on failure >> + */ >> + int (*xfer)(struct udevice *device, struct udevice *slave, >> + unsigned int bitlen, const void *dout, void *din, >> + unsigned long flags); >> + >> + /** >> + * Set transfer speed. >> + * This sets a new speed to be applied for next spi_xfer(). >> + * @slave: The SPI slave >> + * @hz: The transfer speed >> + * @return 0 if OK, -ve on error >> + */ >> + int (*set_speed)(struct udevice *device, uint hz); >> + >> + /** >> + * Set the SPI mode/flags >> + * >> + * It is unclear if we want to set speed and mode together instead >> + * of separately. >> + * >> + * @slave: The SPI slave >> + * @mode: Requested SPI mode (SPI_... flags) >> + * @return 0 if OK, -ve on error >> + */ >> + int (*set_mode)(struct udevice *device, uint mode); >> +}; >> + >> +int spi_find_bus_and_cs(int busnum, int cs, struct udevice **busp, >> + struct udevice **devp); >> + >> +int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode, >> + const char *drv_name, const char *dev_name, >> + struct udevice **devp, struct spi_slave **slavep); >> + >> +int spi_bind_device(struct udevice *bus, int cs, const char *drv_name, >> + const char *dev_name, struct udevice **slavep); >> + >> +int spi_ofdata_to_platdata(const void *blob, int node, >> + struct spi_slave *spi); >> + >> +struct sandbox_state; >> +int sandbox_spi_get_emul(struct sandbox_state *state, >> + struct udevice *bus, struct udevice *slave, >> + struct udevice **emulp); >> + >> +/* Access the serial operations for a device */ >> +#define spi_get_ops(dev) ((struct dm_spi_ops *)(dev)->driver->ops) >> +#endif /* CONFIG_DM_SPI */ >> + >> #endif /* _SPI_H_ */ >> -- >> 2.0.0.526.g5318336 >> > > thanks! > -- > Jagan. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot