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

26 Jul 2016

On 25 July 2016 at 13:34,  Wenyou.Yang@microchip.com wrote:
...
Hi Jagan,
...
-----Original Message-----
From: Jagan Teki [mailto:jagannadh.teki@gmail.com]
Sent: 2016年7月21日 15:10
To: Wenyou Yang wenyou.yang@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.yang@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@atmel.com
Reviewed-by: Simon Glass sjg@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, &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);




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.

    