Re: [U-Boot] [PATCH] spi: mxc_spi: support driver model
> -Original Message- > From: Lothar Waßmann [mailto:l...@karo-electronics.de] > Sent: Tuesday, August 08, 2017 6:20 PM > To: Peng Fan > Cc: ja...@openedev.com; u-boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH] spi: mxc_spi: support driver model > > 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 > > Cc: Jagan Teki > > cc: Stefano Babic > > --- > > 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 > > +#include > > #include > > #include > > #include > > @@ -14,6 +15,8 @@ > > #include > > #include > > > > +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 intmax_hz; > > unsigned intmode; > > + 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? When DM_SPI is enabled, the mxcs->ss will be initialized in the driver probe. So keep DM_SPI here. > > > + 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. Same as above. > > > + 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
Re: [U-Boot] [PATCH] spi: mxc_spi: support driver model
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 > Cc: Jagan Teki > cc: Stefano Babic > --- > 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 > +#include > #include > #include > #include > @@ -14,6 +15,8 @@ > #include > #include > > +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 intmax_hz; > unsigned intmode; > + 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 cs
Re: [U-Boot] [PATCH] spi: mxc_spi: support driver model
On Sat, Mar 4, 2017 at 8:17 AM, 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 board. > > Signed-off-by: Peng Fan > Cc: Jagan Teki > cc: Stefano Babic > --- > drivers/spi/mxc_spi.c | 185 > +- > 1 file changed, 152 insertions(+), 33 deletions(-) > > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c > index fc2786e..431e042 100644 > --- a/drivers/spi/mxc_spi.c > +++ b/drivers/spi/mxc_spi.c > @@ -5,6 +5,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -14,6 +15,8 @@ > #include > #include > > +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; > @@ -40,6 +39,8 @@ __weak int board_spi_cs_gpio(unsigned bus, unsigned cs) > #define CONFIG_SYS_SPI_MXC_WAIT(CONFIG_SYS_HZ/100) /* 10 > ms */ > #endif > > +#define MXC_SPI_MAX_FREQ 2000 This need to get it from dt and manuplutae the freq stuf in .set_speed thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: mxc_spi: support driver model
Ping.. any comments? On Sat, Mar 04, 2017 at 10:47:06AM +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 board. > >Signed-off-by: Peng Fan >Cc: Jagan Teki >cc: Stefano Babic >--- > drivers/spi/mxc_spi.c | 185 +- > 1 file changed, 152 insertions(+), 33 deletions(-) > >diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >index fc2786e..431e042 100644 >--- a/drivers/spi/mxc_spi.c >+++ b/drivers/spi/mxc_spi.c >@@ -5,6 +5,7 @@ > */ > > #include >+#include > #include > #include > #include >@@ -14,6 +15,8 @@ > #include > #include > >+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; >@@ -40,6 +39,8 @@ __weak int board_spi_cs_gpio(unsigned bus, unsigned cs) > #define CONFIG_SYS_SPI_MXC_WAIT (CONFIG_SYS_HZ/100) /* 10 > ms */ > #endif > >+#define MXC_SPI_MAX_FREQ 2000 >+ > struct mxc_spi_slave { > struct spi_slave slave; > unsigned long base; >@@ -51,6 +52,7 @@ struct mxc_spi_slave { > int ss_pol; > unsigned intmax_hz; > unsigned intmode; >+ struct gpio_desc ss; > }; > > static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave) >@@ -58,19 +60,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)) { >+ 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)) { >+ 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 +218,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 +333,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 +344,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 +358,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 +370,39 @@ int spi_xfer(struct spi_slave *slave, unsigned int >bitlen, const void *dout, > } > > if (flags & SPI_XFER_END) { >- spi_cs_deactivate(slave); >+