Hi Jagan,
> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.t...@gmail.com]
> Sent: 2016年7月21日 15:10
> To: Wenyou Yang <wenyou.y...@atmel.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>
> Subject: Re: [U-Boot] [PATCH v5] dm: at91: Add driver model support for the
> spi
> driver
>
> On 20 July 2016 at 15:05, Wenyou Yang <wenyou.y...@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.y...@atmel.com>
> > Reviewed-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > 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 | 9 ++
> > drivers/spi/atmel_spi.c | 303
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 312 insertions(+)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > aca385d..e015d1d 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -32,6 +32,15 @@ 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
> > + select SPI_FLASH
> > + select SPI_FLASH_ATMEL
>
> These two configs's are flash related don't add these in spi side, and
Accepted.
> > + 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, ®_base->csr[cs]);
> > +
> > + mode = ATMEL_SPI_MR_MSTR |
> > + ATMEL_SPI_MR_MODFDIS |
> > + ATMEL_SPI_MR_WDRBT |
> > + ATMEL_SPI_MR_PCS(~(1 << cs));
> > +
> > + writel(mode, ®_base->mr);
> > +
>
> claim_bus means have to enable spi not doing mode and freq stuff because this
> claim is calling for every transactions from flash so, doing mode and freq
> stuff
> every time little overhead. skip these and add mode stuff in
> atmel_spi_set_mode
> freq in atmel_spi_set_speed.
I totally agreed your opinions.
But for Atmel SPI, for different cs, the frequency is set by different the
SPI_CSRx registers.
That is to say, cs = 0, it is set by SPI_CSR0, cs =1 it is by SPI_CSR1, and so
on.
And ops callback (*set_speed)(struct udevice *bus, uint hz) doesn't provide the
cs or slave device information.
Similar reason for mode set.
Maybe we should improve these callbacks. Do you agree?
>
> > + writel(ATMEL_SPI_CR_SPIEN, ®_base->cr);
> > +
> > + return 0;
> > +}
>
> --
> Jagan.
Best Regards,
Wenyou Yang
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
