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