On 15 July 2014 06:26, Simon Glass sjg@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@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;elsepriv->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!