Skip to contentSkip to conversation

spi: mxc_spi: Update pre and post divider algorithm

PATCH New Dirk Behme 4 participants 7 messages May 11, 2013 – Jun 26, 2013

Subsystems (2)
Arm i.MX and Layerscape SPI
lore
Share & tools
lore

Dirk Behme

author
#0 lore raw

PATCH v2 1/1 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 <[email protected]>
Signed-off-by Dirk Behme <[email protected]>

---

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(-)
drivers/spi/mxc_spi.c +12 -18
index 3e903b3..e87b899 100644
@@ -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(&regs->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);
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

Quoted 8 lines · Dirk Behme

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 <[email protected]>
Signed-off-by Dirk Behme <[email protected]>

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

Quoted 74 lines

---

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(-)
drivers/spi/mxc_spi.c +12 -18
index 3e903b3..e87b899 100644
@@ -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(&regs->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);
#2 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

Hi Dirk,

Quoted 10 lines · Dirk Behme
Quoted 9 lines · Dirk Behme

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.

Quoted 8 lines
Quoted 2 lines
Signed-off-by Troy Kisky <[email protected]>
Signed-off-by Dirk Behme <[email protected]>

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

Quoted 89 lines

applied?

Thanks

Dirk

Quoted 79 lines

---

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(-)
drivers/spi/mxc_spi.c +2 -2
index 3e903b3..e87b899 100644
@@ -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) {
drivers/spi/mxc_spi.c +0 -2
@@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave
 *mxcs, unsigned int cs,
      reg_ctrl |=  MXC_CSPICTRL_EN;
      reg_write(&regs->ctrl, reg_ctrl);
 
-    /*
-     * The following computation is taken directly from Freescale's

code.

drivers/spi/mxc_spi.c +10 -14
-     */
      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);
Signature · 5 lines
=====================================================================
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
=====================================================================
#3 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

Quoted 123 lines · Stefano Babic

Hi Dirk,

Quoted 10 lines · Dirk Behme
Quoted 9 lines · Dirk Behme

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.

Quoted 8 lines
Quoted 2 lines
Signed-off-by Troy Kisky <[email protected]>
Signed-off-by Dirk Behme <[email protected]>

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

Quoted 75 lines

applied?

Thanks

Dirk

Quoted 68 lines

---

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(-)
drivers/spi/mxc_spi.c +2 -2
index 3e903b3..e87b899 100644
@@ -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) {
drivers/spi/mxc_spi.c +0 -2
@@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave
 *mxcs, unsigned int cs,
      reg_ctrl |=  MXC_CSPICTRL_EN;
      reg_write(&regs->ctrl, reg_ctrl);
 
-    /*
-     * The following computation is taken directly from Freescale's

code.

drivers/spi/mxc_spi.c +9 -13
-     */
      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;
          }
      }
Quoted 14 lines

drivers/spi/mxc_spi.c static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, · line 174

Quoted 11 lines
drivers/spi/mxc_spi.c +1 -1
@@ -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);
Quoted 10 lines

--
=====================================================================
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
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot at 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.

#4 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

Hi,

Quoted 3 lines · Jagan Teki

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

Signature · 5 lines
=====================================================================
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
=====================================================================
#5 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

Quoted 6 lines · Stefano Babic

Hi,

Quoted 1 line · Jagan Teki
Quoted 7 lines
Quoted 2 lines

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.

#6 lore raw

PATCH v2 1/1 spi: mxc_spi: Update pre and post divider algorithm

In reply to Dirk Behme
Quoted 9 lines · Dirk Behme

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 <[email protected]>
Signed-off-by Dirk Behme <[email protected]>

---

Applied to u-boot-imx as a fix, thanks !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
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
=====================================================================