[U-Boot] [PATCH] spi: mxc_spi: support driver model
Lothar Waßmann
LW at KARO-electronics.de
Tue Aug 8 10:20:28 UTC 2017
Hi,
On Tue, 8 Aug 2017 18:00:01 +0800 Peng Fan wrote:
> Add driver model support for mxc spi driver.
> Most functions are restructured to be reused by DM and non-DM.
> Tested on mx6slevk/mx6qsabresd board.
>
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> Cc: Jagan Teki <jagan at openedev.com>
> cc: Stefano Babic <sbabic at denx.de>
> ---
> drivers/spi/mxc_spi.c | 183 +++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 150 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index e1562c3..44fed94 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -5,6 +5,7 @@
> */
>
> #include <common.h>
> +#include <dm.h>
> #include <malloc.h>
> #include <spi.h>
> #include <linux/errno.h>
> @@ -14,6 +15,8 @@
> #include <asm/arch/clock.h>
> #include <asm/mach-imx/spi.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> #ifdef CONFIG_MX27
> /* i.MX27 has a completely wrong register layout and register definitions in the
> * datasheet, the correct one is in the Freescale's Linux driver */
> @@ -22,10 +25,6 @@
> "See linux mxc_spi driver from Freescale for details."
> #endif
>
> -static unsigned long spi_bases[] = {
> - MXC_SPI_BASE_ADDRESSES
> -};
> -
> __weak int board_spi_cs_gpio(unsigned bus, unsigned cs)
> {
> return -1;
> @@ -51,6 +50,7 @@ struct mxc_spi_slave {
> int ss_pol;
> unsigned int max_hz;
> unsigned int mode;
> + struct gpio_desc ss;
> };
>
> static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave)
> @@ -58,19 +58,24 @@ static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave)
> return container_of(slave, struct mxc_spi_slave, slave);
> }
>
> -void spi_cs_activate(struct spi_slave *slave)
> +static void mxc_spi_cs_activate(struct mxc_spi_slave *mxcs)
> {
> - struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> - if (mxcs->gpio > 0)
> - gpio_set_value(mxcs->gpio, mxcs->ss_pol);
> + if (CONFIG_IS_ENABLED(DM_SPI)) {
>
Shouldn't this be DM_GPIO rather than DM_SPI?
> + dm_gpio_set_value(&mxcs->ss, mxcs->ss_pol);
> + } else {
> + if (mxcs->gpio > 0)
> + gpio_set_value(mxcs->gpio, mxcs->ss_pol);
> + }
> }
>
> -void spi_cs_deactivate(struct spi_slave *slave)
> +static void mxc_spi_cs_deactivate(struct mxc_spi_slave *mxcs)
> {
> - struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> - if (mxcs->gpio > 0)
> - gpio_set_value(mxcs->gpio,
> - !(mxcs->ss_pol));
> + if (CONFIG_IS_ENABLED(DM_SPI)) {
dto.
> + dm_gpio_set_value(&mxcs->ss, !(mxcs->ss_pol));
> + } else {
> + if (mxcs->gpio > 0)
> + gpio_set_value(mxcs->gpio, !(mxcs->ss_pol));
> + }
> }
>
> u32 get_cspi_div(u32 div)
> @@ -211,10 +216,9 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs)
> }
> #endif
>
> -int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
> +int spi_xchg_single(struct mxc_spi_slave *mxcs, unsigned int bitlen,
> const u8 *dout, u8 *din, unsigned long flags)
> {
> - struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> int nbytes = DIV_ROUND_UP(bitlen, 8);
> u32 data, cnt, i;
> struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
> @@ -327,8 +331,9 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
>
> }
>
> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> - void *din, unsigned long flags)
> +static int mxc_spi_xfer_internal(struct mxc_spi_slave *mxcs,
> + unsigned int bitlen, const void *dout,
> + void *din, unsigned long flags)
> {
> int n_bytes = DIV_ROUND_UP(bitlen, 8);
> int n_bits;
> @@ -337,11 +342,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> u8 *p_outbuf = (u8 *)dout;
> u8 *p_inbuf = (u8 *)din;
>
> - if (!slave)
> - return -1;
> + if (!mxcs)
> + return -EINVAL;
>
> if (flags & SPI_XFER_BEGIN)
> - spi_cs_activate(slave);
> + mxc_spi_cs_activate(mxcs);
>
> while (n_bytes > 0) {
> if (n_bytes < MAX_SPI_BYTES)
> @@ -351,7 +356,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>
> n_bits = blk_size * 8;
>
> - ret = spi_xchg_single(slave, n_bits, p_outbuf, p_inbuf, 0);
> + ret = spi_xchg_single(mxcs, n_bits, p_outbuf, p_inbuf, 0);
>
> if (ret)
> return ret;
> @@ -363,12 +368,39 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> }
>
> if (flags & SPI_XFER_END) {
> - spi_cs_deactivate(slave);
> + mxc_spi_cs_deactivate(mxcs);
> + }
> +
> + return 0;
> +}
> +
> +static int mxc_spi_claim_bus_internal(struct mxc_spi_slave *mxcs, int cs)
> +{
> + struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
> + int ret;
> +
> + reg_write(®s->rxdata, 1);
> + udelay(1);
> + ret = spi_cfg_mxc(mxcs, cs);
> + if (ret) {
> + printf("mxc_spi: cannot setup SPI controller\n");
> + return ret;
> }
> + reg_write(®s->period, MXC_CSPIPERIOD_32KHZ);
> + reg_write(®s->intr, 0);
>
> return 0;
> }
>
> +#ifndef CONFIG_DM_SPI
> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> + void *din, unsigned long flags)
> +{
> + struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> +
> + return mxc_spi_xfer_internal(mxcs, bitlen, dout, din, flags);
> +}
> +
> void spi_init(void)
> {
> }
> @@ -390,6 +422,7 @@ static int setup_cs_gpio(struct mxc_spi_slave *mxcs,
> if (mxcs->gpio == -1)
> return 0;
>
> + gpio_request(mxcs->gpio, "spi-cs");
> ret = gpio_direction_output(mxcs->gpio, !(mxcs->ss_pol));
> if (ret) {
> printf("mxc_spi: cannot setup gpio %d\n", mxcs->gpio);
> @@ -399,6 +432,10 @@ static int setup_cs_gpio(struct mxc_spi_slave *mxcs,
> return 0;
> }
>
> +static unsigned long spi_bases[] = {
> + MXC_SPI_BASE_ADDRESSES
> +};
> +
> struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> unsigned int max_hz, unsigned int mode)
> {
> @@ -443,24 +480,104 @@ void spi_free_slave(struct spi_slave *slave)
>
> int spi_claim_bus(struct spi_slave *slave)
> {
> - int ret;
> struct mxc_spi_slave *mxcs = to_mxc_spi_slave(slave);
> - struct cspi_regs *regs = (struct cspi_regs *)mxcs->base;
>
> - reg_write(®s->rxdata, 1);
> - udelay(1);
> - ret = spi_cfg_mxc(mxcs, slave->cs);
> + return mxc_spi_claim_bus_internal(mxcs, slave->cs);
> +}
> +
> +void spi_release_bus(struct spi_slave *slave)
> +{
> + /* TODO: Shut the controller down */
> +}
> +#else
> +
> +static int mxc_spi_probe(struct udevice *bus)
> +{
> + struct mxc_spi_slave *plat = bus->platdata;
> + struct mxc_spi_slave *mxcs = dev_get_platdata(bus);
> + int node = dev_of_offset(bus);
> + const void *blob = gd->fdt_blob;
> + int ret;
> +
> + if (gpio_request_by_name(bus, "cs-gpios", 0, &plat->ss,
> + GPIOD_IS_OUT)) {
> + dev_err(bus, "No cs-gpios property\n");
> + return -EINVAL;
> + }
> +
> + plat->base = dev_get_addr(bus);
> + if (plat->base == FDT_ADDR_T_NONE)
> + return -EINVAL;
>
IMO -ENODEV is more appropriate here.
> + ret = dm_gpio_set_value(&plat->ss, !(mxcs->ss_pol));
>
No conditional on CONFIG_IS_ENABLED() here?
> if (ret) {
> - printf("mxc_spi: cannot setup SPI controller\n");
> - return ret;
> + dev_err(bus, "Setting cs error\n");
> + return -EBUSY;
>
dm_gpio_set_value() returned an appropriate error code which should be
passed to the caller rather than inventing a new error code.
Lothar Waßmann
More information about the U-Boot
mailing list