[U-Boot] [PATCH 12/25] dm: spi: Add documentation on how to convert over SPI drivers
Jagan Teki
jagannadh.teki at gmail.com
Thu Aug 28 13:32:47 CEST 2014
On 15 July 2014 06:26, Simon Glass <sjg at chromium.org> wrote:
> This README is intended to help maintainers move their SPI drivers over to
> driver model. It works through the required steps with an example.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> doc/driver-model/spi-howto.txt | 570 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 570 insertions(+)
> create mode 100644 doc/driver-model/spi-howto.txt
>
> diff --git a/doc/driver-model/spi-howto.txt b/doc/driver-model/spi-howto.txt
> new file mode 100644
> index 0000000..bb64735
> --- /dev/null
> +++ b/doc/driver-model/spi-howto.txt
> @@ -0,0 +1,570 @@
> +How to port a SPI driver to driver model
> +========================================
> +
> +Here is a rough step-by-step guide. It is based around converting the
> +exynos SPI driver to driver model (DM) and the example code is based
> +around U-Boot v2014.04 (commit dda0dbf).
> +
> +It is quite long since it includes actual code examples.
> +
> +Before driver model, SPI drivers have their own private structure which
> +contains 'struct spi_slave'. With driver model, 'struct spi_slave' still
> +exists, but now it is 'per-child data' for the SPI bus. Each child of the
> +SPI bus is a SPI slave. The information that was stored in the
> +driver-specific slave structure can now be put in private data for the
> +SPI bus.
Do you think spi_slave still require, I guess It's needed as some slave members
to cs, bus are used to control the driver.
But any specific reason for removing spi_slave from exynos_spi.c?
> +
> +For example, struct tegra_spi_slave looks like this:
> +
> +struct tegra_spi_slave {
> + struct spi_slave slave;
> + struct tegra_spi_ctrl *ctrl;
> +};
> +
> +In this case 'slave' will be in per-child data, and 'ctrl' will be in the
> +SPI bus' private data.
> +
> +
> +0. How long does this take?
> +
> +Around 2.5 hours, including some allowance for figuring out the driver
> +model bits.
> +
> +
> +1. Enable driver mode for SPI and SPI flash
> +
> +Add these to your board config:
> +
> +#define CONFIG_DM_SPI
> +#define CONFIG_DM_SPI_FLASH
> +
> +
> +2. Add the skeleton
> +
> +Put this code at the bottom of your existing driver file:
> +
> +struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs,
> + unsigned int max_hz, unsigned int mode)
> +{
> + return NULL;
> +}
> +
> +struct spi_slave *spi_setup_slave_fdt(const void *blob, int slave_node,
> + int spi_node)
> +{
> + return NULL;
> +}
> +
> +static int exynos_spi_ofdata_to_platdata(struct udevice *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_probe(struct udevice *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_remove(struct udevice *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_claim_bus(struct udevice *dev)
> +{
> +
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_release_bus(struct udevice *dev)
> +{
> +
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_xfer(struct udevice *dev, unsigned int bitlen,
> + const void *dout, void *din, unsigned long flags)
> +{
> +
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_set_speed(struct udevice *dev, uint speed)
> +{
> + return -ENODEV;
> +}
> +
> +static int exynos_spi_set_mode(struct udevice *dev, uint mode)
> +{
> + return -ENODEV;
> +}
> +
> +static const struct dm_spi_ops exynos_spi_ops = {
> + .claim_bus = exynos_spi_claim_bus,
> + .release_bus = exynos_spi_release_bus,
> + .xfer = exynos_spi_xfer,
> + .set_speed = exynos_spi_set_speed,
> + .set_mode = exynos_spi_set_mode,
> +};
> +
> +static const struct udevice_id exynos_spi_ids[] = {
> + { .compatible = "samsung,exynos-spi" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(exynos_spi) = {
> + .name = "exynos_spi",
> + .id = UCLASS_SPI,
> + .of_match = exynos_spi_ids,
> + .ops = &exynos_spi_ops,
> + .ofdata_to_platdata = exynos_spi_ofdata_to_platdata,
> + .probe = exynos_spi_probe,
> + .remove = exynos_spi_remove
comma missing - remove is necessary?
> +};
> +
> +
> +3. Replace 'exynos' in the above code with your driver name
> +
> +
> +4. #ifdef out all of the code in your driver except for the above
> +
> +This will allow you to get it building, which means you can work
> +incrementally. Since all the methods return an error initially, there is
> +less chance that you will accidentally leave something in.
> +
> +Also, even though your conversion is basically a rewrite, it might help
> +reviewers if you leave functions in the same place in the file,
> +particularly for large drivers.
> +
> +
> +5. Add some includes
> +
> +Add these includes to your driver:
> +
> +#include <dm.h>
> +#include <errno.h>
> +
> +
> +6. Build
> +
> +At this point you should be able to build U-Boot for your board with the
> +empty SPI driver. You still have empty methods in yur driver, but we will
typo your instead of yur
> +write these one by one.
> +
> +If you have spi_init() functions or the like that are called from your
> +board then the build will fail. Remove these calls and make a note of the
> +init that needs to be done.
> +
> +
> +7. Set up your platform data structure
> +
> +This will hold the information your driver needs to operate, like its
> +hardware address or maximum frequency.
> +
> +You may already have a struct like this, or you may need to create one
> +from some of the #defines or global variables in the driver.
> +
> +Note that this information is not the run-time information. It should not
> +include state that changes. It should be fixed throughout the life of
> +U-Boot. Run-time information comes later.
> +
> +Here is what was in the exynos spi driver:
> +
> +struct spi_bus {
> + enum periph_id periph_id;
> + s32 frequency; /* Default clock frequency, -1 for none */
> + struct exynos_spi *regs;
> + int inited; /* 1 if this bus is ready for use */
> + int node;
> + uint deactivate_delay_us; /* Delay to wait after deactivate */
> +};
> +
> +Of these, inited is handled by DM and node is the device tree node, which
> +DM tells you. The name is not quite right. So in this case we would use:
> +
> +struct exynos_spi_platdata {
> + enum periph_id periph_id;
> + s32 frequency; /* Default clock frequency, -1 for none */
> + struct exynos_spi *regs;
> + uint deactivate_delay_us; /* Delay to wait after deactivate */
> +};
> +
> +
> +8a. Write ofdata_to_platdata() [for device tree only]
> +
> +This method will convert information in the device tree node into a C
> +structure in your driver (called platform data). If you are not using
> +device tree, go to 8b.
> +
> +DM will automatically allocate the struct for us when we are using device
> +tree, but we need to tell it the size:
> +
> +U_BOOT_DRIVER(spi_exynos) = {
> +...
> + .platdata_auto_alloc_size = sizeof(struct exynos_spi_platdata),
> +
> +
> +Here is a sample function. It gets a pointer to the platform data and
> +fills in the fields from device tree.
> +
> +static int exynos_spi_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct exynos_spi_platdata *plat = dev->platdata;
> + const void *blob = gd->fdt_blob;
> + int node = dev->of_offset;
> +
> + plat->regs = (struct exynos_spi *)fdtdec_get_addr(blob, node, "reg");
> + plat->periph_id = pinmux_decode_periph_id(blob, node);
> +
> + if (plat->periph_id == PERIPH_ID_NONE) {
> + debug("%s: Invalid peripheral ID %d\n", __func__,
> + plat->periph_id);
> + return -FDT_ERR_NOTFOUND;
> + }
> +
> + /* Use 500KHz as a suitable default */
> + plat->frequency = fdtdec_get_int(blob, node, "spi-max-frequency",
> + 500000);
> + plat->deactivate_delay_us = fdtdec_get_int(blob, node,
> + "spi-deactivate-delay", 0);
> +
> + return 0;
> +}
> +
> +
> +8b. Add the platform data [non-device-tree only]
> +
> +Specify this data in a U_BOOT_DEVICE() declaration in your board file:
> +
> +struct exynos_spi_platdata platdata_spi0 = {
> + .periph_id = ...
> + .frequency = ...
> + .regs = ...
> + .deactivate_delay_us = ...
> +};
> +
> +U_BOOT_DEVICE(board_spi0) = {
> + .name = "exynos_spi",
> + .platdata = &platdata_spi0,
> +};
> +
> +You will unfortunately need to put the struct into a header file in this
> +case so that your board file can use it.
> +
> +
> +9. Add the device private data
> +
> +Most devices have some private data which they use to keep track of things
> +while active. This is the run-time information and needs to be stored in
> +a structure. There is probably a structure in the driver that includes a
> +'struct spi_slave', so you can use that.
> +
> +struct exynos_spi_slave {
> + struct spi_slave slave;
> + struct exynos_spi *regs;
> + unsigned int freq; /* Default frequency */
> + unsigned int mode;
> + enum periph_id periph_id; /* Peripheral ID for this device */
> + unsigned int fifo_size;
> + int skip_preamble;
> + struct spi_bus *bus; /* Pointer to our SPI bus info */
> + ulong last_transaction_us; /* Time of last transaction end */
> +};
> +
> +
> +We should rename this to make its purpose more obvious, and get rid of
> +the slave structure, so we have:
> +
> +struct exynos_spi_priv {
> + struct exynos_spi *regs;
> + unsigned int freq; /* Default frequency */
> + unsigned int mode;
> + enum periph_id periph_id; /* Peripheral ID for this device */
> + unsigned int fifo_size;
> + int skip_preamble;
> + ulong last_transaction_us; /* Time of last transaction end */
> +};
> +
> +
> +DM can auto-allocate this also:
> +
> +U_BOOT_DRIVER(spi_exynos) = {
> +...
> + .priv_auto_alloc_size = sizeof(struct exynos_spi_priv),
> +
> +
> +Note that this is created before the probe() method is called, and destroyed
> +after the remove() method is called. It will be zeroed when the probe()
> +method is called.
> +
> +
> +10. Add the probe() and remove() methods
> +
> +Note: It's a good idea to build repeatedly as you are working, to avoid a
> +huge amount of work getting things compiling at the end.
> +
> +The probe() method is supposed to set up the hardware. U-Boot used to use
> +spi_setup_slave() to do this. So take a look at this function and see
> +what you can copy out to set things up.
> +
> +
> +static int exynos_spi_probe(struct udevice *dev)
> +{
> + struct exynos_spi_platdata *pdata = dev_get_platdata(dev);
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> +
> + if (pdata->periph_id == PERIPH_ID_SPI1 ||
> + pdata->periph_id == PERIPH_ID_SPI2)
> + priv->fifo_size = 64;
> + else
> + priv->fifo_size = 256;
> +
> + priv->skip_preamble = 0;
> + priv->last_transaction_us = timer_get_us();
> + priv->freq = pdata->frequency;
> + priv->periph_id = pdata->periph_id;
> +
> + return 0;
> +}
> +
> +This implementation doesn't actually touch the hardware, which is somewhat
> +unusual for a driver. In this case we will do that when the device is
> +claimed by something that wants to use the SPI bus.
> +
> +For remove() we could shut down the clocks, but in this case there is
> +nothing to do. DM frees any memory that it allocated, so we can just
> +remove exynos_spi_remove() and its reference in U_BOOT_DRIVER.
> +
> +
> +11. Implement set_speed()
> +
> +This should set up clocks so that the SPI bus is running at the right
> +speed. With the old API spi_claim_bus() would normally do this and several
> +of the following functions, so let's look at that function:
> +
> +int spi_claim_bus(struct spi_slave *slave)
> +{
> + struct exynos_spi_slave *spi_slave = to_exynos_spi(slave);
> + struct exynos_spi *regs = spi_slave->regs;
> + u32 reg = 0;
> + int ret;
> +
> + ret = set_spi_clk(spi_slave->periph_id,
> + spi_slave->freq);
> + if (ret < 0) {
> + debug("%s: Failed to setup spi clock\n", __func__);
> + return ret;
> + }
> +
> + exynos_pinmux_config(spi_slave->periph_id, PINMUX_FLAG_NONE);
> +
> + spi_flush_fifo(slave);
> +
> + reg = readl(®s->ch_cfg);
> + reg &= ~(SPI_CH_CPHA_B | SPI_CH_CPOL_L);
> +
> + if (spi_slave->mode & SPI_CPHA)
> + reg |= SPI_CH_CPHA_B;
> +
> + if (spi_slave->mode & SPI_CPOL)
> + reg |= SPI_CH_CPOL_L;
> +
> + writel(reg, ®s->ch_cfg);
> + writel(SPI_FB_DELAY_180, ®s->fb_clk);
> +
> + return 0;
> +}
> +
> +
> +It sets up the speed, mode, pinmux, feedback delay and clears the FIFOs.
> +With DM these will happen in separate methods.
> +
> +
> +Here is an example for the speed part:
> +
> +static int exynos_spi_set_speed(struct udevice *dev, uint speed)
> +{
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + ret = set_spi_clk(priv->periph_id, speed);
> + if (ret)
> + return ret;
> + priv->freq = speed;
> +
> + return 0;
> +}
> +
> +
> +12. Implement set_mode()
> +
> +This should adjust the SPI mode (polarity, etc.). Again this code probably
> +comes from the old spi_claim_bus(). Here is an example:
> +
> +
> +static int exynos_spi_set_mode(struct udevice *dev, uint mode)
> +{
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> + uint32_t reg;
> +
> + reg = readl(&priv->regs->ch_cfg);
> + reg &= ~(SPI_CH_CPHA_B | SPI_CH_CPOL_L);
> +
> + if (mode & SPI_CPHA)
> + reg |= SPI_CH_CPHA_B;
> +
> + if (mode & SPI_CPOL)
> + reg |= SPI_CH_CPOL_L;
> +
> + writel(reg, &priv->regs->ch_cfg);
> +
> + return 0;
> +}
> +
> +
> +13. Implement claim_bus()
> +
> +This is where a client wants to make use of the bus, so claims it first.
> +At this point we need to make sure every is set up ready for data transfer.
> +
> +Here again we look at the old claim function and see some code that is
> +needed. It is anything unrelated to speed and mode:
> +
> +static int exynos_spi_claim_bus(struct udevice *dev)
> +{
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> +
> + exynos_pinmux_config(priv->periph_id, PINMUX_FLAG_NONE);
> + spi_flush_fifo(priv->regs);
> +
> + writel(SPI_FB_DELAY_180, &priv->regs->fb_clk);
> +
> + return 0;
> +}
> +
> +The spi_flush_fifo() function is in the removed part of the code, so we
> +need to expose it again (perhaps with an #endif before it and '#if 0'
> +after it). It only needs accesss to priv->regs which is why we have
> +passed that in:
> +
> +/**
> + * Flush spi tx, rx fifos and reset the SPI controller
> + *
> + * @param regs Pointer to SPI registers
> + */
> +static void spi_flush_fifo(struct exynos_spi *regs)
> +{
> + clrsetbits_le32(®s->ch_cfg, SPI_CH_HS_EN, SPI_CH_RST);
> + clrbits_le32(®s->ch_cfg, SPI_CH_RST);
> + setbits_le32(®s->ch_cfg, SPI_TX_CH_ON | SPI_RX_CH_ON);
> +}
> +
> +
> +14. Implement release_bus()
> +
> +This releases the bus - in our example the old code in spi_release_bus()
> +is a call to spi_flush_fifo, so we add:
> +
> +static int exynos_spi_release_bus(struct udevice *dev)
> +{
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> +
> + spi_flush_fifo(priv->regs);
> +
> + return 0;
> +}
> +
> +
> +15. Implement xfer()
> +
> +This is the final method that we need to create, and it is where all the
> +work happens. The method parameters are the same as the old spi_xfer() with
> +the addition of a 'struct udevice' so conversion is pretty easy. Start
> +by copying the contents of spi_xfer() to your new xfer() mthod and proceed
typo - method
> +from there.
> +
> +If (flags & SPI_XFER_BEGIN) is non-zero then xfer() normally calls an
> +activate function, something like this:
> +
> +void spi_cs_activate(struct spi_slave *slave)
> +{
> + struct exynos_spi_slave *spi_slave = to_exynos_spi(slave);
> +
> + /* If it's too soon to do another transaction, wait */
> + if (spi_slave->bus->deactivate_delay_us &&
> + spi_slave->last_transaction_us) {
> + ulong delay_us; /* The delay completed so far */
> + delay_us = timer_get_us() - spi_slave->last_transaction_us;
> + if (delay_us < spi_slave->bus->deactivate_delay_us)
> + udelay(spi_slave->bus->deactivate_delay_us - delay_us);
> + }
> +
> + clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT);
> + debug("Activate CS, bus %d\n", spi_slave->slave.bus);
> + spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE;
> +}
> +
> +The new version looks like this:
> +
> +static void spi_cs_activate(struct udevice *dev)
> +{
> + struct exynos_spi_platdata *pdata = dev_get_platdata(dev);
> + struct exynos_spi_priv *priv = dev_get_priv(dev);
> +
> + /* If it's too soon to do another transaction, wait */
> + if (pdata->deactivate_delay_us &&
> + priv->last_transaction_us) {
> + ulong delay_us; /* The delay completed so far */
> + delay_us = timer_get_us() - priv->last_transaction_us;
> + if (delay_us < pdata->deactivate_delay_us)
> + udelay(pdata->deactivate_delay_us - delay_us);
> + }
> +
> + clrbits_le32(&priv->regs->cs_reg, SPI_SLAVE_SIG_INACT);
> + debug("Activate CS, bus '%s'\n", dev->name);
> + priv->skip_preamble = priv->mode & SPI_PREAMBLE;
> +}
> +
> +All we have really done here is change the pointers and print the device name
> +instead of the bus number. Other local static functions can be treated in
> +the same way.
> +
> +
> +16. Set up the per-child data and child_pre_probe() method
> +
> +To minimise the pain and complexity of the SPI subsystem while the driver
typo - minimize
> +model change-over is in place, struct spi_slave is used to reference a
> +SPI bus slave, even though that slave is actually a struct udevice. In fact
> +struct spi_slave is the device's child data. We need to make sure this is
> +set up. It is possible to allocate more space that struct spi_slave needs,
> +but this is the minimum.
> +
> +U_BOOT_DRIVER(exynos_spi) = {
> +...
> + .per_child_auto_alloc_size = sizeof(struct spi_slave),
> + .child_pre_probe = exynos_spi_child_pre_probe,
> +}
> +
> +int exynos_spi_child_pre_probe(struct udevice *dev)
> +{
> + struct spi_slave *slave = dev_get_parentdata(dev);
> +
> + return spi_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, slave);
> +}
So this routine will fill spi_slave members - is it?
Why cs and bus are not part of DM?
> +
> +
> +Here our child_pre_probe() method merely calls a standard method to convert
> +device tree data to that used in the slave.
> +
> +
> +17. Test it
> +
> +Now that you have the code written and it compiles, try testing it using
> +the 'sf test' command. You may need to enable CONFIG_CMD_SF_TEST for your
> +board.
> +
> +
> +18. Prepare patches and send them to the mailing lists
> +
> +You can use 'tools/patman/patman' to prepare, check and send patches for
> +your work. See the README for details.
> --
> 2.0.0.526.g5318336
>
thanks!
--
Jagan.
More information about the U-Boot
mailing list