* [U-Boot] [PATCH 0/2] SPI: mxc_spi: slave initialisation fixes @ 2014-09-01 8:46 Markus Niebel 2014-09-01 8:46 ` [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler Markus Niebel 2014-09-01 8:46 ` [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus Markus Niebel 0 siblings, 2 replies; 7+ messages in thread From: Markus Niebel @ 2014-09-01 8:46 UTC (permalink / raw) To: u-boot From: Markus Niebel <Markus.Niebel@tq-group.com> current implementation of the mxc_spi host driver gives issues, if using more than one slave on the same bus. These patches try to improve this use case. They were tested on a TQMa6S (i.MC6S) with a custom mainboard using two slave devices in SPI MODE 0 and 3, on of the devices uses gpio based chip select the other hardware base chip select. Markus Niebel (2): SPI: mxc_spi: remove second reset from ECSPI config handler SPI: mxc_spi: delay initialisation until claim bus drivers/spi/mxc_spi.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler 2014-09-01 8:46 [U-Boot] [PATCH 0/2] SPI: mxc_spi: slave initialisation fixes Markus Niebel @ 2014-09-01 8:46 ` Markus Niebel 2014-09-12 8:50 ` Stefano Babic 2014-09-01 8:46 ` [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus Markus Niebel 1 sibling, 1 reply; 7+ messages in thread From: Markus Niebel @ 2014-09-01 8:46 UTC (permalink / raw) To: u-boot From: Markus Niebel <Markus.Niebel@tq-group.com> the second reset prevents other registers to be written. This will prevent to have the correct signal levels for SCLK before writing to the config reg in spi_xchg_single. Tested with GPIO based chipselect and SPI_MODE_3 on i.MX6S Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> --- drivers/spi/mxc_spi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 2d5f385..6a05d15 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -163,9 +163,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); - /* We need to disable SPI before changing registers */ - reg_ctrl &= ~MXC_CSPICTRL_EN; - if (mode & SPI_CS_HIGH) ss_pol = 1; -- 2.1.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler 2014-09-01 8:46 ` [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler Markus Niebel @ 2014-09-12 8:50 ` Stefano Babic 2014-09-12 9:10 ` Jagan Teki 0 siblings, 1 reply; 7+ messages in thread From: Stefano Babic @ 2014-09-12 8:50 UTC (permalink / raw) To: u-boot Hi Markus, On 01/09/2014 10:46, Markus Niebel wrote: > From: Markus Niebel <Markus.Niebel@tq-group.com> > > the second reset prevents other registers to be written. > This will prevent to have the correct signal levels for > SCLK before writing to the config reg in spi_xchg_single. > > Tested with GPIO based chipselect and SPI_MODE_3 on i.MX6S > > Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> > --- > drivers/spi/mxc_spi.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c > index 2d5f385..6a05d15 100644 > --- a/drivers/spi/mxc_spi.c > +++ b/drivers/spi/mxc_spi.c > @@ -163,9 +163,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | > MXC_CSPICTRL_POSTDIV(post_div); > > - /* We need to disable SPI before changing registers */ > - reg_ctrl &= ~MXC_CSPICTRL_EN; > - > if (mode & SPI_CS_HIGH) > ss_pol = 1; > > Acked-by: Stefano Babic <sbabic@denx.de> Jagannadha, this series is currently assigned to me in patchwork. Should I merge into u-boot-imx or do you prefer to merge the two patches into u-boot-spi ? Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de ===================================================================== ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler 2014-09-12 8:50 ` Stefano Babic @ 2014-09-12 9:10 ` Jagan Teki 0 siblings, 0 replies; 7+ messages in thread From: Jagan Teki @ 2014-09-12 9:10 UTC (permalink / raw) To: u-boot I will pick! On 12 September 2014 14:20, Stefano Babic <sbabic@denx.de> wrote: > Hi Markus, > > On 01/09/2014 10:46, Markus Niebel wrote: >> From: Markus Niebel <Markus.Niebel@tq-group.com> >> >> the second reset prevents other registers to be written. >> This will prevent to have the correct signal levels for >> SCLK before writing to the config reg in spi_xchg_single. >> >> Tested with GPIO based chipselect and SPI_MODE_3 on i.MX6S >> >> Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> >> --- >> drivers/spi/mxc_spi.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >> index 2d5f385..6a05d15 100644 >> --- a/drivers/spi/mxc_spi.c >> +++ b/drivers/spi/mxc_spi.c >> @@ -163,9 +163,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | >> MXC_CSPICTRL_POSTDIV(post_div); >> >> - /* We need to disable SPI before changing registers */ >> - reg_ctrl &= ~MXC_CSPICTRL_EN; >> - >> if (mode & SPI_CS_HIGH) >> ss_pol = 1; >> >> > > Acked-by: Stefano Babic <sbabic@denx.de> > > Jagannadha, this series is currently assigned to me in patchwork. Should > I merge into u-boot-imx or do you prefer to merge the two patches into > u-boot-spi ? > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de > ===================================================================== -- Jagan. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus 2014-09-01 8:46 [U-Boot] [PATCH 0/2] SPI: mxc_spi: slave initialisation fixes Markus Niebel 2014-09-01 8:46 ` [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler Markus Niebel @ 2014-09-01 8:46 ` Markus Niebel 2014-09-24 18:23 ` Jagan Teki 1 sibling, 1 reply; 7+ messages in thread From: Markus Niebel @ 2014-09-01 8:46 UTC (permalink / raw) To: u-boot From: Markus Niebel <Markus.Niebel@tq-group.com> it is not correct to init for a specific slave in spi_setup_slave. instead buffer the values and delay init until spi_claim_bus. Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> --- drivers/spi/mxc_spi.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 6a05d15..c741738 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -43,6 +43,8 @@ struct mxc_spi_slave { #endif int gpio; int ss_pol; + unsigned int max_hz; + unsigned int mode; }; static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave) @@ -77,12 +79,13 @@ u32 get_cspi_div(u32 div) } #ifdef MXC_CSPI -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, - unsigned int max_hz, unsigned int mode) +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) { unsigned int ctrl_reg; u32 clk_src; u32 div; + unsigned int max_hz = mxcs->max_hz; + unsigned int mode = mxcs->mode; clk_src = mxc_get_clock(MXC_CSPI_CLK); @@ -114,19 +117,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, #endif #ifdef MXC_ECSPI -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, - unsigned int max_hz, unsigned int mode) +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); s32 reg_ctrl, reg_config; u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, sclkctl = 0; u32 pre_div = 0, post_div = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; - - if (max_hz == 0) { - printf("Error: desired clock is 0\n"); - return -1; - } + unsigned int max_hz = mxcs->max_hz; + unsigned int mode = mxcs->mode; /* * Reset SPI and set all CSs to master mode, if toggling @@ -404,6 +403,11 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, if (bus >= ARRAY_SIZE(spi_bases)) return NULL; + if (max_hz == 0) { + printf("Error: desired clock is 0\n"); + return NULL; + } + mxcs = spi_alloc_slave(struct mxc_spi_slave, bus, cs); if (!mxcs) { puts("mxc_spi: SPI Slave not allocated !\n"); @@ -421,13 +425,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, cs = ret; mxcs->base = spi_bases[bus]; + mxcs->max_hz = max_hz; + mxcs->mode = mode; - ret = spi_cfg_mxc(mxcs, cs, max_hz, mode); - if (ret) { - printf("mxc_spi: cannot setup SPI controller\n"); - free(mxcs); - return NULL; - } return &mxcs->slave; } @@ -440,12 +440,17 @@ 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); - reg_write(®s->ctrl, mxcs->ctrl_reg); + ret = spi_cfg_mxc(mxcs, slave->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); -- 2.1.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus 2014-09-01 8:46 ` [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus Markus Niebel @ 2014-09-24 18:23 ` Jagan Teki 2014-09-24 18:42 ` Jagan Teki 0 siblings, 1 reply; 7+ messages in thread From: Jagan Teki @ 2014-09-24 18:23 UTC (permalink / raw) To: u-boot On 1 September 2014 14:16, Markus Niebel <list-09_u-boot@tqsc.de> wrote: > From: Markus Niebel <Markus.Niebel@tq-group.com> > > it is not correct to init for a specific slave in spi_setup_slave. > instead buffer the values and delay init until spi_claim_bus. > > Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> > --- > drivers/spi/mxc_spi.c | 37 +++++++++++++++++++++---------------- > 1 file changed, 21 insertions(+), 16 deletions(-) > > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c > index 6a05d15..c741738 100644 > --- a/drivers/spi/mxc_spi.c > +++ b/drivers/spi/mxc_spi.c > @@ -43,6 +43,8 @@ struct mxc_spi_slave { > #endif > int gpio; > int ss_pol; > + unsigned int max_hz; > + unsigned int mode; > }; > > static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave) > @@ -77,12 +79,13 @@ u32 get_cspi_div(u32 div) > } > > #ifdef MXC_CSPI > -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > - unsigned int max_hz, unsigned int mode) > +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) > { > unsigned int ctrl_reg; > u32 clk_src; > u32 div; > + unsigned int max_hz = mxcs->max_hz; > + unsigned int mode = mxcs->mode; > > clk_src = mxc_get_clock(MXC_CSPI_CLK); > > @@ -114,19 +117,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > #endif > > #ifdef MXC_ECSPI > -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > - unsigned int max_hz, unsigned int mode) > +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) > { > u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); > s32 reg_ctrl, reg_config; > u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, sclkctl = 0; > u32 pre_div = 0, post_div = 0; > struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; > - > - if (max_hz == 0) { > - printf("Error: desired clock is 0\n"); > - return -1; > - } > + unsigned int max_hz = mxcs->max_hz; > + unsigned int mode = mxcs->mode; > > /* > * Reset SPI and set all CSs to master mode, if toggling > @@ -404,6 +403,11 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > if (bus >= ARRAY_SIZE(spi_bases)) > return NULL; > > + if (max_hz == 0) { > + printf("Error: desired clock is 0\n"); > + return NULL; > + } > + > mxcs = spi_alloc_slave(struct mxc_spi_slave, bus, cs); > if (!mxcs) { > puts("mxc_spi: SPI Slave not allocated !\n"); > @@ -421,13 +425,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > cs = ret; > > mxcs->base = spi_bases[bus]; > + mxcs->max_hz = max_hz; > + mxcs->mode = mode; > > - ret = spi_cfg_mxc(mxcs, cs, max_hz, mode); > - if (ret) { > - printf("mxc_spi: cannot setup SPI controller\n"); > - free(mxcs); > - return NULL; > - } > return &mxcs->slave; > } > > @@ -440,12 +440,17 @@ 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); > - reg_write(®s->ctrl, mxcs->ctrl_reg); > + ret = spi_cfg_mxc(mxcs, slave->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); > > -- > 2.1.0 > In fact this driver is using spi_cfg_mxc() for configuring SPI clock,polarities and frequency in spi_setup_slave() time, but usually spi_setup_slave() will require only basic controller reg initialization. So while in spi_claim_bus() clock, polarities and frequencies will handle. Please think on that direction and will be good if you fix those and send the patches for next time. thanks! -- Jagan. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus 2014-09-24 18:23 ` Jagan Teki @ 2014-09-24 18:42 ` Jagan Teki 0 siblings, 0 replies; 7+ messages in thread From: Jagan Teki @ 2014-09-24 18:42 UTC (permalink / raw) To: u-boot On 24 September 2014 23:53, Jagan Teki <jagannadh.teki@gmail.com> wrote: > On 1 September 2014 14:16, Markus Niebel <list-09_u-boot@tqsc.de> wrote: >> From: Markus Niebel <Markus.Niebel@tq-group.com> >> >> it is not correct to init for a specific slave in spi_setup_slave. >> instead buffer the values and delay init until spi_claim_bus. >> >> Signed-off-by: Markus Niebel <Markus.Niebel@tq-group.com> >> --- >> drivers/spi/mxc_spi.c | 37 +++++++++++++++++++++---------------- >> 1 file changed, 21 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >> index 6a05d15..c741738 100644 >> --- a/drivers/spi/mxc_spi.c >> +++ b/drivers/spi/mxc_spi.c >> @@ -43,6 +43,8 @@ struct mxc_spi_slave { >> #endif >> int gpio; >> int ss_pol; >> + unsigned int max_hz; >> + unsigned int mode; >> }; >> >> static inline struct mxc_spi_slave *to_mxc_spi_slave(struct spi_slave *slave) >> @@ -77,12 +79,13 @@ u32 get_cspi_div(u32 div) >> } >> >> #ifdef MXC_CSPI >> -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> - unsigned int max_hz, unsigned int mode) >> +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) >> { >> unsigned int ctrl_reg; >> u32 clk_src; >> u32 div; >> + unsigned int max_hz = mxcs->max_hz; >> + unsigned int mode = mxcs->mode; >> >> clk_src = mxc_get_clock(MXC_CSPI_CLK); >> >> @@ -114,19 +117,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> #endif >> >> #ifdef MXC_ECSPI >> -static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, >> - unsigned int max_hz, unsigned int mode) >> +static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs) >> { >> u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); >> s32 reg_ctrl, reg_config; >> u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, sclkctl = 0; >> u32 pre_div = 0, post_div = 0; >> struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; >> - >> - if (max_hz == 0) { >> - printf("Error: desired clock is 0\n"); >> - return -1; >> - } >> + unsigned int max_hz = mxcs->max_hz; >> + unsigned int mode = mxcs->mode; >> >> /* >> * Reset SPI and set all CSs to master mode, if toggling >> @@ -404,6 +403,11 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> if (bus >= ARRAY_SIZE(spi_bases)) >> return NULL; >> >> + if (max_hz == 0) { >> + printf("Error: desired clock is 0\n"); >> + return NULL; >> + } >> + >> mxcs = spi_alloc_slave(struct mxc_spi_slave, bus, cs); >> if (!mxcs) { >> puts("mxc_spi: SPI Slave not allocated !\n"); >> @@ -421,13 +425,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> cs = ret; >> >> mxcs->base = spi_bases[bus]; >> + mxcs->max_hz = max_hz; >> + mxcs->mode = mode; >> >> - ret = spi_cfg_mxc(mxcs, cs, max_hz, mode); >> - if (ret) { >> - printf("mxc_spi: cannot setup SPI controller\n"); >> - free(mxcs); >> - return NULL; >> - } >> return &mxcs->slave; >> } >> >> @@ -440,12 +440,17 @@ 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); >> - reg_write(®s->ctrl, mxcs->ctrl_reg); >> + ret = spi_cfg_mxc(mxcs, slave->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); >> >> -- >> 2.1.0 >> > > In fact this driver is using spi_cfg_mxc() for configuring SPI > clock,polarities and frequency > in spi_setup_slave() time, but usually spi_setup_slave() will require > only basic controller reg > initialization. So while in spi_claim_bus() clock, polarities and > frequencies will handle. > > Please think on that direction and will be good if you fix those and > send the patches for next time. I think you did the same, what I mentioned above, could you rebase this patch on master, I have seen patch failed error while applying. thanks! -- Jagan. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-24 18:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-01 8:46 [U-Boot] [PATCH 0/2] SPI: mxc_spi: slave initialisation fixes Markus Niebel 2014-09-01 8:46 ` [U-Boot] [PATCH 1/2] SPI: mxc_spi: remove second reset from ECSPI config handler Markus Niebel 2014-09-12 8:50 ` Stefano Babic 2014-09-12 9:10 ` Jagan Teki 2014-09-01 8:46 ` [U-Boot] [PATCH 2/2] SPI: mxc_spi: delay initialisation until claim bus Markus Niebel 2014-09-24 18:23 ` Jagan Teki 2014-09-24 18:42 ` Jagan Teki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox