[U-Boot] [PATCH 06/25] dm: spi: Add a uclass for SPI
Simon Glass
sjg at chromium.org
Sat Aug 30 01:38:48 CEST 2014
Hi Jagan,
On 28 August 2014 02:58, Jagan Teki <jagannadh.teki at gmail.com> wrote:
> On 15 July 2014 06:26, Simon Glass <sjg at 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 <sjg at 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
More information about the U-Boot
mailing list