[U-Boot] [PATCH v6] dm: at91: Add driver model support for the spi driver

Wenyou.Yang at microchip.com Wenyou.Yang at microchip.com
Fri Jul 29 02:38:19 CEST 2016


Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: 2016年7月26日 14:21
> To: Wenyou Yang <wenyou.yang at atmel.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>
> Subject: Re: [U-Boot] [PATCH v6] dm: at91: Add driver model support for the spi
> driver
> 
> On 25 July 2016 at 15:27, Wenyou Yang <wenyou.yang at atmel.com> wrote:
> > Add driver model support while retaining the existing legacy code.
> > This allows the driver to support boards that have converted to driver
> > model as well as those that have not.
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang at atmel.com>
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v6:
> >  - Remove the two flash related options.
> >
> > Changes in v5:
> >  - Change clk_client.h -> clk.h to adapt to clk API conversion.
> >
> > Changes in v4:
> >  - Collect Reviewed-by tag.
> >  - Update the clk API based on [PATCH] clk: convert API to match
> >    reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
> >  - Remove check on dev_get_parent() return.
> >  - Fixed the return value, -ENODEV->-EINVAL.
> >  - Retain #include <asm/arch/clk.h> line.
> >
> > Changes in v3:
> >  - Remove redundant log print.
> >
> > Changes in v2:
> >  - Add clock support.
> >
> >  drivers/spi/Kconfig     |   7 ++
> >  drivers/spi/atmel_spi.c | 303
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 310 insertions(+)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > aca385d..16ed231 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -32,6 +32,13 @@ config ATH79_SPI
> >           uses driver model and requires a device tree binding to operate.
> >           please refer to doc/device-tree-bindings/spi/spi-ath79.txt.
> >
> > +config ATMEL_SPI
> > +       bool "Atmel SPI driver"
> > +       depends on ARCH_AT91
> > +       help
> > +         Enable the Atmel SPI driver. This driver can be used to access
> > +         the SPI Flash, such as AT25DF321.
> > +
> >  config CADENCE_QSPI
> >         bool "Cadence QSPI driver"
> >         help
> > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c index
> > ed6278b..98bee35 100644
> > --- a/drivers/spi/atmel_spi.c
> > +++ b/drivers/spi/atmel_spi.c
> > @@ -4,6 +4,9 @@
> >   * SPDX-License-Identifier:    GPL-2.0+
> >   */
> >  #include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <fdtdec.h>
> >  #include <spi.h>
> >  #include <malloc.h>
> >
> > @@ -11,9 +14,15 @@
> >
> >  #include <asm/arch/clk.h>
> >  #include <asm/arch/hardware.h>
> > +#include <asm/arch/at91_spi.h>
> > +#include <asm/gpio.h>
> >
> >  #include "atmel_spi.h"
> >
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#ifndef CONFIG_DM_SPI
> > +
> >  static int spi_has_wdrbt(struct atmel_spi_slave *slave)  {
> >         unsigned int ver;
> > @@ -209,3 +218,297 @@ out:
> >
> >         return 0;
> >  }
> > +
> > +#else
> > +
> > +#define MAX_CS_COUNT   4
> > +
> > +struct atmel_spi_platdata {
> > +       struct at91_spi *regs;
> > +};
> > +
> > +struct atmel_spi_priv {
> > +       unsigned int freq;              /* Default frequency */
> > +       unsigned int mode;
> > +       ulong bus_clk_rate;
> > +       struct gpio_desc cs_gpios[MAX_CS_COUNT]; };
> > +
> > +static int atmel_spi_claim_bus(struct udevice *dev) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct atmel_spi_platdata *bus_plat = dev_get_platdata(bus);
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> > +       struct at91_spi *reg_base = bus_plat->regs;
> > +       u32 cs = slave_plat->cs;
> > +       u32 freq = priv->freq;
> > +       u32 scbr, csrx, mode;
> > +
> > +       scbr = (priv->bus_clk_rate + freq - 1) / freq;
> > +       if (scbr > ATMEL_SPI_CSRx_SCBR_MAX)
> > +               return -EINVAL;
> > +
> > +       if (scbr < 1)
> > +               scbr = 1;
> > +
> > +       csrx = ATMEL_SPI_CSRx_SCBR(scbr);
> > +       csrx |= ATMEL_SPI_CSRx_BITS(ATMEL_SPI_BITS_8);
> > +
> > +       if (!(priv->mode & SPI_CPHA))
> > +               csrx |= ATMEL_SPI_CSRx_NCPHA;
> > +       if (priv->mode & SPI_CPOL)
> > +               csrx |= ATMEL_SPI_CSRx_CPOL;
> > +
> > +       writel(csrx, &reg_base->csr[cs]);
> > +
> > +       mode = ATMEL_SPI_MR_MSTR |
> > +              ATMEL_SPI_MR_MODFDIS |
> > +              ATMEL_SPI_MR_WDRBT |
> > +              ATMEL_SPI_MR_PCS(~(1 << cs));
> > +
> > +       writel(mode, &reg_base->mr);
> > +
> > +       writel(ATMEL_SPI_CR_SPIEN, &reg_base->cr);
> > +
> > +       return 0;
> > +}
> > +
> > +static int atmel_spi_release_bus(struct udevice *dev) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct atmel_spi_platdata *bus_plat = dev_get_platdata(bus);
> > +
> > +       writel(ATMEL_SPI_CR_SPIDIS, &bus_plat->regs->cr);
> > +
> > +       return 0;
> > +}
> > +
> > +static void atmel_spi_cs_activate(struct udevice *dev) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> > +       u32 cs = slave_plat->cs;
> > +
> > +       dm_gpio_set_value(&priv->cs_gpios[cs], 0); }
> > +
> > +static void atmel_spi_cs_deactivate(struct udevice *dev) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +       struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> > +       u32 cs = slave_plat->cs;
> > +
> > +       dm_gpio_set_value(&priv->cs_gpios[cs], 1); }
> > +
> > +static int atmel_spi_xfer(struct udevice *dev, unsigned int bitlen,
> > +                         const void *dout, void *din, unsigned long
> > +flags) {
> > +       struct udevice *bus = dev_get_parent(dev);
> > +       struct atmel_spi_platdata *bus_plat = dev_get_platdata(bus);
> > +       struct at91_spi *reg_base = bus_plat->regs;
> > +
> > +       u32 len_tx, len_rx, len;
> > +       u32 status;
> > +       const u8 *txp = dout;
> > +       u8 *rxp = din;
> > +       u8 value;
> > +
> > +       if (bitlen == 0)
> > +               goto out;
> > +
> > +       /*
> > +        * The controller can do non-multiple-of-8 bit
> > +        * transfers, but this driver currently doesn't support it.
> > +        *
> > +        * It's also not clear how such transfers are supposed to be
> > +        * represented as a stream of bytes...this is a limitation of
> > +        * the current SPI interface.
> > +        */
> > +       if (bitlen % 8) {
> > +               /* Errors always terminate an ongoing transfer */
> > +               flags |= SPI_XFER_END;
> > +               goto out;
> > +       }
> > +
> > +       len = bitlen / 8;
> > +
> > +       /*
> > +        * The controller can do automatic CS control, but it is
> > +        * somewhat quirky, and it doesn't really buy us much anyway
> > +        * in the context of U-Boot.
> > +        */
> > +       if (flags & SPI_XFER_BEGIN) {
> > +               atmel_spi_cs_activate(dev);
> > +
> > +               /*
> > +                * sometimes the RDR is not empty when we get here,
> > +                * in theory that should not happen, but it DOES happen.
> > +                * Read it here to be on the safe side.
> > +                * That also clears the OVRES flag. Required if the
> > +                * following loop exits due to OVRES!
> > +                */
> > +               readl(&reg_base->rdr);
> > +       }
> > +
> > +       for (len_tx = 0, len_rx = 0; len_rx < len; ) {
> > +               status = readl(&reg_base->sr);
> > +
> > +               if (status & ATMEL_SPI_SR_OVRES)
> > +                       return -1;
> > +
> > +               if ((len_tx < len) && (status & ATMEL_SPI_SR_TDRE)) {
> > +                       if (txp)
> > +                               value = *txp++;
> > +                       else
> > +                               value = 0;
> > +                       writel(value, &reg_base->tdr);
> > +                       len_tx++;
> > +               }
> > +
> > +               if (status & ATMEL_SPI_SR_RDRF) {
> > +                       value = readl(&reg_base->rdr);
> > +                       if (rxp)
> > +                               *rxp++ = value;
> > +                       len_rx++;
> > +               }
> > +       }
> > +
> > +out:
> > +       if (flags & SPI_XFER_END) {
> > +               /*
> > +                * Wait until the transfer is completely done before
> > +                * we deactivate CS.
> > +                */
> > +               do {
> > +                       status = readl(&reg_base->sr);
> > +               } while (!(status & ATMEL_SPI_SR_TXEMPTY));
> > +
> > +               atmel_spi_cs_deactivate(dev);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int atmel_spi_set_speed(struct udevice *bus, uint speed) {
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +
> > +       priv->freq = speed;
> > +
> > +       return 0;
> > +}
> > +
> > +static int atmel_spi_set_mode(struct udevice *bus, uint mode) {
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +
> > +       priv->mode = mode;
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct dm_spi_ops atmel_spi_ops = {
> > +       .claim_bus      = atmel_spi_claim_bus,
> > +       .release_bus    = atmel_spi_release_bus,
> > +       .xfer           = atmel_spi_xfer,
> > +       .set_speed      = atmel_spi_set_speed,
> > +       .set_mode       = atmel_spi_set_mode,
> > +       /*
> > +        * cs_info is not needed, since we require all chip selects to be
> > +        * in the device tree explicitly
> > +        */
> > +};
> > +
> > +static int atmel_spi_ofdata_to_platdata(struct udevice *bus) {
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +       int ret;
> > +
> > +       ret = gpio_request_list_by_name(bus, "cs-gpios", priv->cs_gpios,
> > +                                       ARRAY_SIZE(priv->cs_gpios), 0);
> > +       if (ret < 0) {
> > +               error("Can't get %s gpios! Error: %d", bus->name, ret);
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> 
> Better to use plat->cs_gpio or do this gpio_request in probe because
> *_ofdata_to_platdata usually assign/get the plat data.

Accepted.

Move this gpio_request to probe.

Thank you.

> 
> > +
> > +static int atmel_spi_enable_clk(struct udevice *bus) {
> > +       struct atmel_spi_priv *priv = dev_get_priv(bus);
> > +       struct udevice *dev_clk;
> > +       struct clk clk;
> > +       ulong clk_rate;
> > +       int periph;
> > +       int ret;
> > +
> > +       ret = clk_get_by_index(bus, 0, &clk);
> > +       if (ret)
> > +               return -EINVAL;
> > +
> > +       periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
> > +       if (periph < 0)
> > +               return -EINVAL;
> > +
> > +       dev_clk = dev_get_parent(clk.dev);
> > +       ret = clk_request(dev_clk, &clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       clk.id = periph;
> > +       ret = clk_enable(&clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = clk_get_by_index(dev_clk, 0, &clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       clk_rate = clk_get_rate(&clk);
> > +       if (!clk_rate)
> > +               return -EINVAL;
> > +
> > +       priv->bus_clk_rate = clk_rate;
> > +
> > +       clk_free(&clk);
> > +
> > +       return 0;
> > +}
> > +
> > +static int atmel_spi_probe(struct udevice *bus) {
> > +       struct atmel_spi_platdata *bus_plat = dev_get_platdata(bus);
> > +       int ret;
> > +
> > +       ret = atmel_spi_enable_clk(bus);
> > +       if (ret)
> > +               return ret;
> > +
> > +       bus_plat->regs = (struct at91_spi *)dev_get_addr(bus);
> > +
> > +       writel(ATMEL_SPI_CR_SWRST, &bus_plat->regs->cr);
> > +
> > +       return 0;
> > +}
> 
> Same comment as above, doing opposite.
> 
> thanks!
> --
> Jagan.


Best Regards,
Wenyou Yang


More information about the U-Boot mailing list