diff mbox

[U-Boot,2/7] spi: cadence_qspi: Fix baud rate calculation

Message ID 1478099749-6395-3-git-send-email-phil.edworthy@renesas.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Phil Edworthy Nov. 2, 2016, 3:15 p.m. UTC
With the existing code, when the requested SPI clock rate is near
to the lowerest that can be achieved by the hardware (max divider
of the ref clock is 32), the generated clock rate is wrong.
For example, with a 50MHz ref clock, when asked for anything less
than a 1.5MHz SPI clock, the code sets up the divider to generate
25MHz.

This change fixes the calculation.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

Comments

Marek Vasut Nov. 2, 2016, 7:39 p.m. UTC | #1
On 11/02/2016 04:15 PM, Phil Edworthy wrote:
> With the existing code, when the requested SPI clock rate is near
> to the lowerest that can be achieved by the hardware (max divider
> of the ref clock is 32), the generated clock rate is wrong.
> For example, with a 50MHz ref clock, when asked for anything less
> than a 1.5MHz SPI clock, the code sets up the divider to generate
> 25MHz.
> 
> This change fixes the calculation.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 2403e71..71bde1f 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -273,22 +273,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
>  	reg = readl(reg_base + CQSPI_REG_CONFIG);
>  	reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
>  
> -	div = ref_clk_hz / sclk_hz;
> -
> -	if (div > 32)
> -		div = 32;
> -
> -	/* Check if even number. */
> -	if ((div & 1)) {
> -		div = (div / 2);
> -	} else {
> -		if (ref_clk_hz % sclk_hz)
> -			/* ensure generated SCLK doesn't exceed user
> -			specified sclk_hz */
> -			div = (div / 2);
> -		else
> -			div = (div / 2) - 1;
> -	}
> +	/*
> +	 * The baud_div field in the config reg is 4 bits, and the ref clock is
> +	 * divided by 2 * (baud_div + 1). Round up the divider to ensure the
> +	 * SPI clock rate is less than or equal to the requested clock rate.
> +	 */
> +	div = (ref_clk_hz + sclk_hz - 1) / sclk_hz;

DIV_ROUND_UP()

> +	div = ((div + 1) / 2) - 1;

Is this roundup() ?

>  	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
>  	      ref_clk_hz, sclk_hz, div);
>
Phil Edworthy Nov. 3, 2016, 7:36 a.m. UTC | #2
Hi Marek,

On 02 November 2016 19:39, Marek Vasut wrote:
> On 11/02/2016 04:15 PM, Phil Edworthy wrote:
> > With the existing code, when the requested SPI clock rate is near
> > to the lowerest that can be achieved by the hardware (max divider
> > of the ref clock is 32), the generated clock rate is wrong.
> > For example, with a 50MHz ref clock, when asked for anything less
> > than a 1.5MHz SPI clock, the code sets up the divider to generate
> > 25MHz.
> >
> > This change fixes the calculation.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 2403e71..71bde1f 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -273,22 +273,13 @@ void cadence_qspi_apb_config_baudrate_div(void
> *reg_base,
> >  	reg = readl(reg_base + CQSPI_REG_CONFIG);
> >  	reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK <<
> CQSPI_REG_CONFIG_BAUD_LSB);
> >
> > -	div = ref_clk_hz / sclk_hz;
> > -
> > -	if (div > 32)
> > -		div = 32;
> > -
> > -	/* Check if even number. */
> > -	if ((div & 1)) {
> > -		div = (div / 2);
> > -	} else {
> > -		if (ref_clk_hz % sclk_hz)
> > -			/* ensure generated SCLK doesn't exceed user
> > -			specified sclk_hz */
> > -			div = (div / 2);
> > -		else
> > -			div = (div / 2) - 1;
> > -	}
> > +	/*
> > +	 * The baud_div field in the config reg is 4 bits, and the ref clock is
> > +	 * divided by 2 * (baud_div + 1). Round up the divider to ensure the
> > +	 * SPI clock rate is less than or equal to the requested clock rate.
> > +	 */
> > +	div = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
> 
> DIV_ROUND_UP()
Ah yes, that would be clearer!

> > +	div = ((div + 1) / 2) - 1;
> Is this roundup() ?
It's DIV_ROUND_UP() - 1

> 
> >  	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
> >  	      ref_clk_hz, sclk_hz, div);
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Thanks
Phil
diff mbox

Patch

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2403e71..71bde1f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -273,22 +273,13 @@  void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
 
-	div = ref_clk_hz / sclk_hz;
-
-	if (div > 32)
-		div = 32;
-
-	/* Check if even number. */
-	if ((div & 1)) {
-		div = (div / 2);
-	} else {
-		if (ref_clk_hz % sclk_hz)
-			/* ensure generated SCLK doesn't exceed user
-			specified sclk_hz */
-			div = (div / 2);
-		else
-			div = (div / 2) - 1;
-	}
+	/*
+	 * The baud_div field in the config reg is 4 bits, and the ref clock is
+	 * divided by 2 * (baud_div + 1). Round up the divider to ensure the
+	 * SPI clock rate is less than or equal to the requested clock rate.
+	 */
+	div = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
+	div = ((div + 1) / 2) - 1;
 
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);