Hi Wenyou,

On 2016-08-18 03:16, wenyou.y...@microchip.com wrote:
Hi Andreas,

-----Original Message-----
From: Andreas Bießmann [mailto:andr...@biessmann.org]
Sent: 2016年8月16日 5:15
To: Wenyou Yang <wenyou.y...@atmel.com>; U-Boot Mailing List <u-
b...@lists.denx.de>
Cc: Simon Glass <s...@chromium.org>; Jagan Teki <jt...@openedev.com>
Subject: Re: [PATCH v7] dm: at91: Add driver model support for the spi driver

Hi Wenyou,

On 29.07.16 02:38, Wenyou Yang 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.y...@atmel.com>
> Reviewed-by: Simon Glass <s...@chromium.org>

This one breaks avr32 boards:

---8<---
02: dm: at91: Add driver model support for the spi driver
     avr32:  +   atngw100 atngw100mkii
+../drivers/spi/atmel_spi.c:17:31: error: asm/arch/at91_spi.h: No such
file or directory
+../drivers/spi/atmel_spi.c:18:22: error: asm/gpio.h: No such file or
directory
+make[2]: *** [drivers/spi/atmel_spi.o] Error 1
+make[1]: *** [drivers/spi] Error 2
+make: *** [sub-make] Error 2
--->8---

It seems this error is not introduced by this patch.

Well, I think it is introduced by this patch. This patch chenges the atmel_cpi driver in such a way that it breaks for avr32 architecture which has a slight different structure in files.

Can we apply this patch first?

No, not if it breaks AVR32.

Andreas



please fix this

Andreas

> ---
>
> Changes in v7:
>  - Move gpio_request_list_by_name() to _probe(), remove
>    *_ofdata_to_platdata().
>
> 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 | 295
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 302 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..db40631 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,289 @@ 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_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);
> +  struct atmel_spi_priv *priv = dev_get_priv(bus);
> +  int ret;
> +
> +  ret = atmel_spi_enable_clk(bus);
> +  if (ret)
> +          return ret;
> +
> +  bus_plat->regs = (struct at91_spi *)dev_get_addr(bus);
> +
> +  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;
> +  }
> +
> +  writel(ATMEL_SPI_CR_SWRST, &bus_plat->regs->cr);
> +
> +  return 0;
> +}
> +
> +static const struct udevice_id atmel_spi_ids[] = {
> +  { .compatible = "atmel,at91rm9200-spi" },
> +  { }
> +};
> +
> +U_BOOT_DRIVER(atmel_spi) = {
> +  .name   = "atmel_spi",
> +  .id     = UCLASS_SPI,
> +  .of_match = atmel_spi_ids,
> +  .ops    = &atmel_spi_ops,
> +  .platdata_auto_alloc_size = sizeof(struct atmel_spi_platdata),
> +  .priv_auto_alloc_size = sizeof(struct atmel_spi_priv),
> +  .probe  = atmel_spi_probe,
> +};
> +#endif
>


Best Regards,
Wenyou Yang
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to