[U-Boot] [PATCH v2] spi: mxc_spi: Update pre and post divider algorithm

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 ---
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 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);
reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div);MXC_CSPICTRL_PREDIV(pre_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
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
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
participants (4)
-
Dirk Behme
-
Dirk Behme
-
Jagan Teki
-
Stefano Babic