[U-Boot] [PATCH 1/2] ARM: dts: dra7xx: Update spi-max-frequency for qspi slave node

[U-Boot] [PATCH 1/2] ARM: dts: dra7xx: Update spi-max-frequency for qspi slave node

[Vignesh R]

Update the spi-max-frequency property of m25p80 flash slave to match
that of TI QSPI controller node, so that QSPI operations happen at
maximum supported frequency of 76.8MHz.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 arch/arm/dts/dra7-evm.dts          | 2 +-
 arch/arm/dts/dra72-evm-common.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/dra7-evm.dts b/arch/arm/dts/dra7-evm.dts
index fe755c05841d..be36d456206d 100644
--- a/arch/arm/dts/dra7-evm.dts
+++ b/arch/arm/dts/dra7-evm.dts
@@ -505,7 +505,7 @@
 	spi-max-frequency = <76800000>;
 	m25p80 at 0 {
 		compatible = "s25fl256s1","spi-flash";
-		spi-max-frequency = <64000000>;
+		spi-max-frequency = <76800000>;
 		reg = <0>;
 		spi-tx-bus-width = <1>;
 		spi-rx-bus-width = <4>;
diff --git a/arch/arm/dts/dra72-evm-common.dtsi b/arch/arm/dts/dra72-evm-common.dtsi
index b0993e5bf7e0..1e1ca725577f 100644
--- a/arch/arm/dts/dra72-evm-common.dtsi
+++ b/arch/arm/dts/dra72-evm-common.dtsi
@@ -441,7 +441,7 @@
 	spi-max-frequency = <76800000>;
 	m25p80 at 0 {
 		compatible = "s25fl256s1", "spi-flash";
-		spi-max-frequency = <64000000>;
+		spi-max-frequency = <76800000>;
 		reg = <0>;
 		spi-tx-bus-width = <1>;
 		spi-rx-bus-width = <4>;
-- 
2.10.1

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Vignesh R]

Fix the divider calculation logic to choose a value so that the
resulting baudrate is either equal to or closest possible baudrate less
than the requested value.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/ti_qspi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index 52520dff6325..d97e2479d1b3 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -16,6 +16,7 @@
 #include <asm/omap_gpio.h>
 #include <asm/omap_common.h>
 #include <asm/ti-common/ti-edma3.h>
+#include <linux/kernel.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
 	if (!hz)
 		clk_div = 0;
 	else
-		clk_div = (priv->fclk / hz) - 1;
+		clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
 
 	debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);
 
-- 
2.10.1

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Jagan Teki]

On Wed, Oct 12, 2016 at 9:46 AM, Vignesh R <vigneshr@ti.com> wrote:
> Fix the divider calculation logic to choose a value so that the
> resulting baudrate is either equal to or closest possible baudrate less
> than the requested value.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/spi/ti_qspi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index 52520dff6325..d97e2479d1b3 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -16,6 +16,7 @@
>  #include <asm/omap_gpio.h>
>  #include <asm/omap_common.h>
>  #include <asm/ti-common/ti-edma3.h>
> +#include <linux/kernel.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>         if (!hz)
>                 clk_div = 0;
>         else
> -               clk_div = (priv->fclk / hz) - 1;
> +               clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;

Better to have a checks for min and max divider values or mask.

>
>         debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 1/2] ARM: dts: dra7xx: Update spi-max-frequency for qspi slave node

[Jagan Teki]

On Wed, Oct 12, 2016 at 9:46 AM, Vignesh R <vigneshr@ti.com> wrote:
> Update the spi-max-frequency property of m25p80 flash slave to match
> that of TI QSPI controller node, so that QSPI operations happen at
> maximum supported frequency of 76.8MHz.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>

Reviewed-by: Jagan Teki <jteki@openedev.com>

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Vignesh R]

Hi,

On Thursday 13 October 2016 05:41 PM, Jagan Teki wrote:
> On Wed, Oct 12, 2016 at 9:46 AM, Vignesh R <vigneshr@ti.com> wrote:
>> Fix the divider calculation logic to choose a value so that the
>> resulting baudrate is either equal to or closest possible baudrate less
>> than the requested value.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>  drivers/spi/ti_qspi.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
>> index 52520dff6325..d97e2479d1b3 100644
>> --- a/drivers/spi/ti_qspi.c
>> +++ b/drivers/spi/ti_qspi.c
>> @@ -16,6 +16,7 @@
>>  #include <asm/omap_gpio.h>
>>  #include <asm/omap_common.h>
>>  #include <asm/ti-common/ti-edma3.h>
>> +#include <linux/kernel.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>>         if (!hz)
>>                 clk_div = 0;
>>         else
>> -               clk_div = (priv->fclk / hz) - 1;
>> +               clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
> 
> Better to have a checks for min and max divider values or mask.

That code already exists in this function.


-- 
Regards
Vignesh

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Jagan Teki]

On Fri, Oct 14, 2016 at 10:54 AM, Vignesh R <vigneshr@ti.com> wrote:
> Hi,
>
> On Thursday 13 October 2016 05:41 PM, Jagan Teki wrote:
>> On Wed, Oct 12, 2016 at 9:46 AM, Vignesh R <vigneshr@ti.com> wrote:
>>> Fix the divider calculation logic to choose a value so that the
>>> resulting baudrate is either equal to or closest possible baudrate less
>>> than the requested value.
>>>
>>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>>> ---
>>>  drivers/spi/ti_qspi.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
>>> index 52520dff6325..d97e2479d1b3 100644
>>> --- a/drivers/spi/ti_qspi.c
>>> +++ b/drivers/spi/ti_qspi.c
>>> @@ -16,6 +16,7 @@
>>>  #include <asm/omap_gpio.h>
>>>  #include <asm/omap_common.h>
>>>  #include <asm/ti-common/ti-edma3.h>
>>> +#include <linux/kernel.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>>>         if (!hz)
>>>                 clk_div = 0;
>>>         else
>>> -               clk_div = (priv->fclk / hz) - 1;
>>> +               clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
>>
>> Better to have a checks for min and max divider values or mask.
>
> That code already exists in this function.

True but it's unnecessary to print the wrong baud prior to checking.
Do the check, then print/debug and finally write reg.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v2 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Vignesh R]

Fix the divider calculation logic to choose a value so that the
resulting baudrate is either equal to or closest possible baudrate less
than the requested value. While at that, cleanup ti_spi_set_speed().

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v2: cleanup ti_spi_set_speed() a bit.

 drivers/spi/ti_qspi.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
index 52520dff6325..b5de70bf40e3 100644
--- a/drivers/spi/ti_qspi.c
+++ b/drivers/spi/ti_qspi.c
@@ -16,6 +16,7 @@
 #include <asm/omap_gpio.h>
 #include <asm/omap_common.h>
 #include <asm/ti-common/ti-edma3.h>
+#include <linux/kernel.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -118,21 +119,19 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
 	if (!hz)
 		clk_div = 0;
 	else
-		clk_div = (priv->fclk / hz) - 1;
-
-	debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);
+		clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
 
 	/* disable SCLK */
 	writel(readl(&priv->base->clk_ctrl) & ~QSPI_CLK_EN,
 	       &priv->base->clk_ctrl);
 
-	/* assign clk_div values */
-	if (clk_div < 0)
-		clk_div = 0;
-	else if (clk_div > QSPI_CLK_DIV_MAX)
+	/* truncate clk_div value to QSPI_CLK_DIV_MAX */
+	if (clk_div > QSPI_CLK_DIV_MAX)
 		clk_div = QSPI_CLK_DIV_MAX;
 
-	/* enable SCLK */
+	debug("ti_spi_set_speed: hz: %d, clock divider %d\n", hz, clk_div);
+
+	/* enable SCLK and program the clk divider */
 	writel(QSPI_CLK_EN | clk_div, &priv->base->clk_ctrl);
 }
 
-- 
2.10.1

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[R, Vignesh]

On 10/14/2016 12:27 PM, Jagan Teki wrote:
> On Fri, Oct 14, 2016 at 10:54 AM, Vignesh R <vigneshr@ti.com> wrote:
...
>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>> @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>>>>         if (!hz)
>>>>                 clk_div = 0;
>>>>         else
>>>> -               clk_div = (priv->fclk / hz) - 1;
>>>> +               clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
>>>
>>> Better to have a checks for min and max divider values or mask.
>>
>> That code already exists in this function.
> 
> True but it's unnecessary to print the wrong baud prior to checking.
> Do the check, then print/debug and finally write reg.
> 

Posted a v2 in reply to the patch. Thanks for the review!

Regards
Vignesh

[U-Boot] [PATCH 2/2] spi: ti_qspi: Fix baudrate divider calculation

[Jagan Teki]

On Fri, Oct 14, 2016 at 4:18 PM, R, Vignesh <vigneshr@ti.com> wrote:
>
>
> On 10/14/2016 12:27 PM, Jagan Teki wrote:
>> On Fri, Oct 14, 2016 at 10:54 AM, Vignesh R <vigneshr@ti.com> wrote:
> ...
>>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>>
>>>>> @@ -118,7 +119,7 @@ static void ti_spi_set_speed(struct ti_qspi_priv *priv, uint hz)
>>>>>         if (!hz)
>>>>>                 clk_div = 0;
>>>>>         else
>>>>> -               clk_div = (priv->fclk / hz) - 1;
>>>>> +               clk_div = DIV_ROUND_UP(priv->fclk, hz) - 1;
>>>>
>>>> Better to have a checks for min and max divider values or mask.
>>>
>>> That code already exists in this function.
>>
>> True but it's unnecessary to print the wrong baud prior to checking.
>> Do the check, then print/debug and finally write reg.
>>
>
> Posted a v2 in reply to the patch. Thanks for the review!

Please re-post the two patches again, so how I missed it.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.