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

Andreas Bießmann andreas at biessmann.org
Thu Aug 18 11:41:34 CEST 2016


Hi Wenyou,

On 2016-08-18 03:16, Wenyou.Yang at microchip.com wrote:
> Hi Andreas,
> 
>> -----Original Message-----
>> From: Andreas Bießmann [mailto:andreas at biessmann.org]
>> Sent: 2016年8月16日 5:15
>> To: Wenyou Yang <wenyou.yang at atmel.com>; U-Boot Mailing List <u-
>> boot at lists.denx.de>
>> Cc: Simon Glass <sjg at chromium.org>; Jagan Teki <jteki at 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.yang at atmel.com>
>> > Reviewed-by: Simon Glass <sjg at 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


More information about the U-Boot mailing list