| Message ID | 1409561218-30707-3-git-send-email-list-09_u-boot@tqsc.de |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
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!
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!
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);