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

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

[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 <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(&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);
 
-- 
1.7.10.4

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

[Dirk Behme]

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(&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);
>
>

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

[Stefano Babic]

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(&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);
>>
>>
> 
> 
> 


-- 
=====================================================================
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] [PATCH v2] spi: mxc_spi: Update pre and post divider algorithm

[Jagan Teki]

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(&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);
>>>
>>>
>>
>>
>>
>
>
> --
> =====================================================================
> 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.

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

[Stefano Babic]

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


-- 
=====================================================================
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] [PATCH v2] spi: mxc_spi: Update pre and post divider algorithm

[Jagan Teki]

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.

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

[Stefano Babic]

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


-- 
=====================================================================
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
=====================================================================