| Message ID | 1368249954-8790-1-git-send-email-dirk.behme@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
On 11.05.2013 07:25, Dirk Behme wrote: > The spi clock divisor is of the form x * (2**y), or x << y, where x is > 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. > Convert the desired divisor to the smallest number which is >= desired divisor, > and can be represented in this form. The previous algorithm chose a divisor > which could be almost twice as large as needed. > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > Signed-off-by: Dirk Behme <dirk.behme@gmail.com> While we are talking about a first -rc now, could we get these two patches http://patchwork.ozlabs.org/patch/242709/ http://patchwork.ozlabs.org/patch/243113/ applied? Thanks Dirk > --- > > Notes: > > - Changes in v2: Make the alogrithm simpler by removing the -1 as proposed > by Troy. Make the pre_div and post_div u32. > > - This replaces v1 of this patch and depends on the previous sent patch > http://patchwork.ozlabs.org/patch/242709/ > > drivers/spi/mxc_spi.c | 30 ++++++++++++------------------ > 1 file changed, 12 insertions(+), 18 deletions(-) > > diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c > index 3e903b3..e87b899 100644 > --- a/drivers/spi/mxc_spi.c > +++ b/drivers/spi/mxc_spi.c > @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > unsigned int max_hz, unsigned int mode) > { > u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); > - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; > - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; > + s32 reg_ctrl, reg_config; > + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; > struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; > > if (max_hz == 0) { > @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > reg_ctrl |= MXC_CSPICTRL_EN; > reg_write(®s->ctrl, reg_ctrl); > > - /* > - * The following computation is taken directly from Freescale's code. > - */ > if (clk_src > max_hz) { > - pre_div = DIV_ROUND_UP(clk_src, max_hz); > - if (pre_div > 16) { > - post_div = pre_div / 16; > - pre_div = 16; > - } > - if (post_div != 0) { > - for (i = 0; i < 16; i++) { > - if ((1 << i) >= post_div) > - break; > - } > - if (i == 16) { > + pre_div = (clk_src - 1) / max_hz; > + /* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */ > + post_div = fls(pre_div); > + if (post_div > 4) { > + post_div -= 4; > + if (post_div >= 16) { > printf("Error: no divider for the freq: %d\n", > max_hz); > return -1; > } > - post_div = i; > + pre_div >>= post_div; > + } else { > + post_div = 0; > } > } > > @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, > reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) | > MXC_CSPICTRL_SELCHAN(cs); > reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) | > - MXC_CSPICTRL_PREDIV(pre_div - 1); > + MXC_CSPICTRL_PREDIV(pre_div); > reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | > MXC_CSPICTRL_POSTDIV(post_div); > >
Hi Dirk, On 12/06/2013 07:28, Dirk Behme wrote: > On 11.05.2013 07:25, Dirk Behme wrote: >> The spi clock divisor is of the form x * (2**y), or x << y, where x is >> 1 to 16, and y is 0 to 15. Note the similarity with floating point >> numbers. >> Convert the desired divisor to the smallest number which is >= desired >> divisor, >> and can be represented in this form. The previous algorithm chose a >> divisor >> which could be almost twice as large as needed. >> >> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> >> Signed-off-by: Dirk Behme <dirk.behme@gmail.com> > > While we are talking about a first -rc now, could we get these two patches > > http://patchwork.ozlabs.org/patch/242709/ > > http://patchwork.ozlabs.org/patch/243113/ Sorry to come late with these two patches - after a first glance when you post then, I put the patches at the bottom of my queue and I never checked again. I will work them before end of the week. Best regards, Stefano Babic > > applied? > > Thanks > > Dirk > >> --- >> >> Notes: >> >> - Changes in v2: Make the alogrithm simpler by removing the -1 as >> proposed >> by Troy. Make the pre_div and post_div u32. >> >> - This replaces v1 of this patch and depends on the previous sent patch >> http://patchwork.ozlabs.org/patch/242709/ >> >> drivers/spi/mxc_spi.c | 30 ++++++++++++------------------ >> 1 file changed, 12 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >> index 3e903b3..e87b899 100644 >> --- a/drivers/spi/mxc_spi.c >> +++ b/drivers/spi/mxc_spi.c >> @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, >> unsigned int cs, >> unsigned int max_hz, unsigned int mode) >> { >> u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); >> - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; >> - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; >> + s32 reg_ctrl, reg_config; >> + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; >> struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; >> >> if (max_hz == 0) { >> @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave >> *mxcs, unsigned int cs, >> reg_ctrl |= MXC_CSPICTRL_EN; >> reg_write(®s->ctrl, reg_ctrl); >> >> - /* >> - * The following computation is taken directly from Freescale's >> code. >> - */ >> if (clk_src > max_hz) { >> - pre_div = DIV_ROUND_UP(clk_src, max_hz); >> - if (pre_div > 16) { >> - post_div = pre_div / 16; >> - pre_div = 16; >> - } >> - if (post_div != 0) { >> - for (i = 0; i < 16; i++) { >> - if ((1 << i) >= post_div) >> - break; >> - } >> - if (i == 16) { >> + pre_div = (clk_src - 1) / max_hz; >> + /* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */ >> + post_div = fls(pre_div); >> + if (post_div > 4) { >> + post_div -= 4; >> + if (post_div >= 16) { >> printf("Error: no divider for the freq: %d\n", >> max_hz); >> return -1; >> } >> - post_div = i; >> + pre_div >>= post_div; >> + } else { >> + post_div = 0; >> } >> } >> >> @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, >> unsigned int cs, >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) | >> MXC_CSPICTRL_SELCHAN(cs); >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) | >> - MXC_CSPICTRL_PREDIV(pre_div - 1); >> + MXC_CSPICTRL_PREDIV(pre_div); >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | >> MXC_CSPICTRL_POSTDIV(post_div); >> >> > > >
On Wed, Jun 12, 2013 at 9:12 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi Dirk, > > On 12/06/2013 07:28, Dirk Behme wrote: >> On 11.05.2013 07:25, Dirk Behme wrote: >>> The spi clock divisor is of the form x * (2**y), or x << y, where x is >>> 1 to 16, and y is 0 to 15. Note the similarity with floating point >>> numbers. >>> Convert the desired divisor to the smallest number which is >= desired >>> divisor, >>> and can be represented in this form. The previous algorithm chose a >>> divisor >>> which could be almost twice as large as needed. >>> >>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> >>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com> >> >> While we are talking about a first -rc now, could we get these two patches >> >> http://patchwork.ozlabs.org/patch/242709/ >> >> http://patchwork.ozlabs.org/patch/243113/ > > Sorry to come late with these two patches - after a first glance when > you post then, I put the patches at the bottom of my queue and I never > checked again. > > I will work them before end of the week. > > Best regards, > Stefano Babic > > >> >> applied? >> >> Thanks >> >> Dirk >> >>> --- >>> >>> Notes: >>> >>> - Changes in v2: Make the alogrithm simpler by removing the -1 as >>> proposed >>> by Troy. Make the pre_div and post_div u32. >>> >>> - This replaces v1 of this patch and depends on the previous sent patch >>> http://patchwork.ozlabs.org/patch/242709/ >>> >>> drivers/spi/mxc_spi.c | 30 ++++++++++++------------------ >>> 1 file changed, 12 insertions(+), 18 deletions(-) >>> >>> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >>> index 3e903b3..e87b899 100644 >>> --- a/drivers/spi/mxc_spi.c >>> +++ b/drivers/spi/mxc_spi.c >>> @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, >>> unsigned int cs, >>> unsigned int max_hz, unsigned int mode) >>> { >>> u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); >>> - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; >>> - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; >>> + s32 reg_ctrl, reg_config; >>> + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; >>> struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; >>> >>> if (max_hz == 0) { >>> @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave >>> *mxcs, unsigned int cs, >>> reg_ctrl |= MXC_CSPICTRL_EN; >>> reg_write(®s->ctrl, reg_ctrl); >>> >>> - /* >>> - * The following computation is taken directly from Freescale's >>> code. >>> - */ >>> if (clk_src > max_hz) { >>> - pre_div = DIV_ROUND_UP(clk_src, max_hz); >>> - if (pre_div > 16) { >>> - post_div = pre_div / 16; >>> - pre_div = 16; >>> - } >>> - if (post_div != 0) { >>> - for (i = 0; i < 16; i++) { >>> - if ((1 << i) >= post_div) >>> - break; >>> - } >>> - if (i == 16) { >>> + pre_div = (clk_src - 1) / max_hz; >>> + /* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */ >>> + post_div = fls(pre_div); >>> + if (post_div > 4) { >>> + post_div -= 4; >>> + if (post_div >= 16) { >>> printf("Error: no divider for the freq: %d\n", >>> max_hz); >>> return -1; >>> } >>> - post_div = i; >>> + pre_div >>= post_div; >>> + } else { >>> + post_div = 0; >>> } >>> } >>> >>> @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, >>> unsigned int cs, >>> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) | >>> MXC_CSPICTRL_SELCHAN(cs); >>> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) | >>> - MXC_CSPICTRL_PREDIV(pre_div - 1); >>> + MXC_CSPICTRL_PREDIV(pre_div); >>> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | >>> MXC_CSPICTRL_POSTDIV(post_div); >>> >>> >> >> >> > > > -- > ===================================================================== > 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@denx.de > ===================================================================== > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Sorry, i didn't understand the conversation here, was this fix applied? Could you please explain. -- Thanks, Jagan.
Hi, On 12/06/2013 17:47, Jagan Teki wrote: > > Sorry, i didn't understand the conversation here, was this fix applied? > Could you please explain. Patches are not applied and are currently assigned to me. As they concerned the SPI subsystem (really, it is the SPI driver for iMX), they could be applied by you as SPI custodian or by me as IMX custodian. Should I assign them to you ? Regards, Stefano
On Wed, Jun 12, 2013 at 9:30 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi, > > > On 12/06/2013 17:47, Jagan Teki wrote: > >> >> Sorry, i didn't understand the conversation here, was this fix applied? >> Could you please explain. > > Patches are not applied and are currently assigned to me. As they > concerned the SPI subsystem (really, it is the SPI driver for iMX), they > could be applied by you as SPI custodian or by me as IMX custodian. > Should I assign them to you ? np, thanks for your information. if your are in middle of this conversation/review please go ahead, but please let me know once the review has done so-that i will apply this on my custodian tree. Is it ok for you? -- Thanks, Jagan.
On 11/05/2013 07:25, Dirk Behme wrote: > The spi clock divisor is of the form x * (2**y), or x << y, where x is > 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. > Convert the desired divisor to the smallest number which is >= desired divisor, > and can be represented in this form. The previous algorithm chose a divisor > which could be almost twice as large as needed. > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > Signed-off-by: Dirk Behme <dirk.behme@gmail.com> > --- Applied to u-boot-imx as a fix, thanks ! Best regards, Stefano Babic
diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..e87b899 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; + s32 reg_ctrl, reg_config; + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs->base; if (max_hz == 0) { @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(®s->ctrl, reg_ctrl); - /* - * The following computation is taken directly from Freescale's code. - */ if (clk_src > max_hz) { - pre_div = DIV_ROUND_UP(clk_src, max_hz); - if (pre_div > 16) { - post_div = pre_div / 16; - pre_div = 16; - } - if (post_div != 0) { - for (i = 0; i < 16; i++) { - if ((1 << i) >= post_div) - break; - } - if (i == 16) { + pre_div = (clk_src - 1) / max_hz; + /* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */ + post_div = fls(pre_div); + if (post_div > 4) { + post_div -= 4; + if (post_div >= 16) { printf("Error: no divider for the freq: %d\n", max_hz); return -1; } - post_div = i; + pre_div >>= post_div; + } else { + post_div = 0; } } @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div - 1); + MXC_CSPICTRL_PREDIV(pre_div); reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div);