On 25 July 2016 at 13:34, <wenyou.y...@microchip.com> wrote: > 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?
Clear, yes we need to fix these callbacks. thanks! -- Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
