[U-Boot,u-boot-spi,1/1] spi: mvebu_a3700_spi: Fix clock prescale computation
diff mbox series

Message ID 20190717145834.24335-1-marek.behun@nic.cz
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series
  • [U-Boot,u-boot-spi,1/1] spi: mvebu_a3700_spi: Fix clock prescale computation
Related show

Commit Message

Marek Behún July 17, 2019, 2:58 p.m. UTC
The prescaler value computation can yield wrong result if given 0x1f at
the beginning: the value is computed to be 0x20, but the maximum value
the register can hold 0x1f, so the actual stored value in this case is
0, which is obviously wrong. The first condition should also take care
of the 0x1f value.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/spi/mvebu_a3700_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Roese July 17, 2019, 4:21 p.m. UTC | #1
On 17.07.19 16:58, Marek Behún wrote:
> The prescaler value computation can yield wrong result if given 0x1f at
> the beginning: the value is computed to be 0x20, but the maximum value
> the register can hold 0x1f, so the actual stored value in this case is
> 0, which is obviously wrong. The first condition should also take care
> of the 0x1f value.

Wouldn't it be better to max the value after the calculation (see
below)...
  
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>   drivers/spi/mvebu_a3700_spi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/mvebu_a3700_spi.c b/drivers/spi/mvebu_a3700_spi.c
> index feeafdceaa..cc73ff3fee 100644
> --- a/drivers/spi/mvebu_a3700_spi.c
> +++ b/drivers/spi/mvebu_a3700_spi.c
> @@ -181,7 +181,7 @@ static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
>   	data = readl(&reg->cfg);
>   
>   	prescale = DIV_ROUND_UP(clk_get_rate(&plat->clk), hz);
> -	if (prescale > 0x1f)
> +	if (prescale >= 0x1f)
>   		prescale = 0x1f;
>   	else if (prescale > 0xf)
>   		prescale = 0x10 + (prescale + 1) / 2;

Something like this:

	if (prescale > 0xf)
   		prescale = 0x10 + (prescale + 1) / 2;
	prescale = min(prescale, 0x1f);

?

Thanks,
Stefan
Marek Behún July 18, 2019, 9:34 a.m. UTC | #2
:) probably yes

On Wed, 17 Jul 2019 18:21:35 +0200
Stefan Roese <sr@denx.de> wrote:

> On 17.07.19 16:58, Marek Behún wrote:
> > The prescaler value computation can yield wrong result if given
> > 0x1f at the beginning: the value is computed to be 0x20, but the
> > maximum value the register can hold 0x1f, so the actual stored
> > value in this case is 0, which is obviously wrong. The first
> > condition should also take care of the 0x1f value.  
> 
> Wouldn't it be better to max the value after the calculation (see
> below)...
>   
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >   drivers/spi/mvebu_a3700_spi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/mvebu_a3700_spi.c
> > b/drivers/spi/mvebu_a3700_spi.c index feeafdceaa..cc73ff3fee 100644
> > --- a/drivers/spi/mvebu_a3700_spi.c
> > +++ b/drivers/spi/mvebu_a3700_spi.c
> > @@ -181,7 +181,7 @@ static int mvebu_spi_set_speed(struct udevice
> > *bus, uint hz) data = readl(&reg->cfg);
> >   
> >   	prescale = DIV_ROUND_UP(clk_get_rate(&plat->clk), hz);
> > -	if (prescale > 0x1f)
> > +	if (prescale >= 0x1f)
> >   		prescale = 0x1f;
> >   	else if (prescale > 0xf)
> >   		prescale = 0x10 + (prescale + 1) / 2;  
> 
> Something like this:
> 
> 	if (prescale > 0xf)
>    		prescale = 0x10 + (prescale + 1) / 2;
> 	prescale = min(prescale, 0x1f);
> 
> ?
> 
> Thanks,
> Stefan

Patch
diff mbox series

diff --git a/drivers/spi/mvebu_a3700_spi.c b/drivers/spi/mvebu_a3700_spi.c
index feeafdceaa..cc73ff3fee 100644
--- a/drivers/spi/mvebu_a3700_spi.c
+++ b/drivers/spi/mvebu_a3700_spi.c
@@ -181,7 +181,7 @@  static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
 	data = readl(&reg->cfg);
 
 	prescale = DIV_ROUND_UP(clk_get_rate(&plat->clk), hz);
-	if (prescale > 0x1f)
+	if (prescale >= 0x1f)
 		prescale = 0x1f;
 	else if (prescale > 0xf)
 		prescale = 0x10 + (prescale + 1) / 2;