[U-Boot] [PATCH] kw_spi: fix clock prescaler computation

[U-Boot] [PATCH] kw_spi: fix clock prescaler computation

[Valentin Longchamp]

The computation was not correct with low clock values: setting a 1MHz
clock would result in an overlap that would then configure a 25Mhz
clock.

This patch implements a correct computation method according to the
kirkwood functionnal spec. table 629 (Serial Memory Interface
Configuration Register).

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
 arch/arm/include/asm/arch-kirkwood/spi.h |    1 +
 drivers/spi/kirkwood_spi.c               |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h b/arch/arm/include/asm/arch-kirkwood/spi.h
index 1d5043f..5a38dc5 100644
--- a/arch/arm/include/asm/arch-kirkwood/spi.h
+++ b/arch/arm/include/asm/arch-kirkwood/spi.h
@@ -38,6 +38,7 @@ struct kwspi_registers {
 };
 
 #define KWSPI_CLKPRESCL_MASK	0x1f
+#define KWSPI_CLKPRESCL_MIN	0x12
 #define KWSPI_CSN_ACT		1 /* Activates serial memory interface */
 #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
 #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index ee14669..9b3f6a4 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -57,8 +57,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
 
 	/* calculate spi clock prescaller using max_hz */
-	data = ((CONFIG_SYS_TCLK / 2) / max_hz) & KWSPI_CLKPRESCL_MASK;
-	data |= 0x10;
+	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
+	data = data < KWSPI_CLKPRESCL_MIN ? KWSPI_CLKPRESCL_MIN : data;
+	data = data > KWSPI_CLKPRESCL_MASK ? KWSPI_CLKPRESCL_MASK : data;
 
 	/* program spi clock prescaller using max_hz */
 	writel(KWSPI_ADRLEN_3BYTE | data, &spireg->cfg);
-- 
1.7.1

[U-Boot] [PATCH] kw_spi: fix clock prescaler computation

[Valentin Longchamp]

Hi Prafulla,

I haven't received feedback about this patch from you. Can I please kindly
remind you about it since I am about to send patches that deal with SPI clk setting.

Thank you.

On 06/29/2012 02:17 PM, Valentin Longchamp wrote:
> The computation was not correct with low clock values: setting a 1MHz
> clock would result in an overlap that would then configure a 25Mhz
> clock.
> 
> This patch implements a correct computation method according to the
> kirkwood functionnal spec. table 629 (Serial Memory Interface
> Configuration Register).
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  arch/arm/include/asm/arch-kirkwood/spi.h |    1 +
>  drivers/spi/kirkwood_spi.c               |    5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h b/arch/arm/include/asm/arch-kirkwood/spi.h
> index 1d5043f..5a38dc5 100644
> --- a/arch/arm/include/asm/arch-kirkwood/spi.h
> +++ b/arch/arm/include/asm/arch-kirkwood/spi.h
> @@ -38,6 +38,7 @@ struct kwspi_registers {
>  };
>  
>  #define KWSPI_CLKPRESCL_MASK	0x1f
> +#define KWSPI_CLKPRESCL_MIN	0x12
>  #define KWSPI_CSN_ACT		1 /* Activates serial memory interface */
>  #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
>  #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index ee14669..9b3f6a4 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -57,8 +57,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>  	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
>  
>  	/* calculate spi clock prescaller using max_hz */
> -	data = ((CONFIG_SYS_TCLK / 2) / max_hz) & KWSPI_CLKPRESCL_MASK;
> -	data |= 0x10;
> +	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
> +	data = data < KWSPI_CLKPRESCL_MIN ? KWSPI_CLKPRESCL_MIN : data;
> +	data = data > KWSPI_CLKPRESCL_MASK ? KWSPI_CLKPRESCL_MASK : data;
>  
>  	/* program spi clock prescaller using max_hz */
>  	writel(KWSPI_ADRLEN_3BYTE | data, &spireg->cfg);
> 

[U-Boot] [PATCH] kw_spi: fix clock prescaler computation

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 14 August 2012 15:02
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Holger Brunck
> Subject: Re: [PATCH] kw_spi: fix clock prescaler computation
> 
> Hi Prafulla,
> 
> I haven't received feedback about this patch from you. Can I please
> kindly
> remind you about it since I am about to send patches that deal with
> SPI clk setting.

Dear Valentin
My bad :-( I am really sorry about this.
I missed this patch.
Please find my comments in lined.

> 
> Thank you.
> 
> On 06/29/2012 02:17 PM, Valentin Longchamp wrote:
> > The computation was not correct with low clock values: setting a
> 1MHz
> > clock would result in an overlap that would then configure a 25Mhz
> > clock.
> >
> > This patch implements a correct computation method according to the
> > kirkwood functionnal spec. table 629 (Serial Memory Interface
> > Configuration Register).

I think this is table 600 (ref: MV-S104860-00 Rev. E specs available on extranet)
This is a cosmetic change.

Otherwise I ack for this patch.

Please kindly let me know, should I pull this patch or you will resend it with above change?

Regards...
Prafulla . . .

[U-Boot] [PATCH] kw_spi: fix clock prescaler computation

[Valentin Longchamp]

Hi Prafulla,

On 08/14/2012 03:15 PM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 14 August 2012 15:02
>> To: Prafulla Wadaskar
>> Cc: u-boot at lists.denx.de; Holger Brunck
>> Subject: Re: [PATCH] kw_spi: fix clock prescaler computation
>>
>> Hi Prafulla,
>>
>> I haven't received feedback about this patch from you. Can I please
>> kindly
>> remind you about it since I am about to send patches that deal with
>> SPI clk setting.
> 
> Dear Valentin
> My bad :-( I am really sorry about this.
> I missed this patch.
> Please find my comments in lined.
> 
>>
>> Thank you.
>>
>> On 06/29/2012 02:17 PM, Valentin Longchamp wrote:
>>> The computation was not correct with low clock values: setting a
>> 1MHz
>>> clock would result in an overlap that would then configure a 25Mhz
>>> clock.
>>>
>>> This patch implements a correct computation method according to the
>>> kirkwood functionnal spec. table 629 (Serial Memory Interface
>>> Configuration Register).
> 
> I think this is table 600 (ref: MV-S104860-00 Rev. E specs available on extranet)
> This is a cosmetic change.
> 
> Otherwise I ack for this patch.
> 
> Please kindly let me know, should I pull this patch or you will resend it with above change?

Ok, I had taken rev. C of the document as a reference, I will resend the patch
with the updated commit message.

Valentin

[U-Boot] [PATCH v2] kw_spi: fix clock prescaler computation

[Valentin Longchamp]

The computation was not correct with low clock values: setting a 1MHz
clock would result in an overlap that would then configure a 25Mhz
clock.

This patch implements a correct computation method according to the
kirkwood functionnal spec. table 600 (Serial Memory Interface
Configuration Register).

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Holger Brunck <holger.brunck@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
---
changes for v2:
 - table nb in commit message


 arch/arm/include/asm/arch-kirkwood/spi.h |    1 +
 drivers/spi/kirkwood_spi.c               |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h b/arch/arm/include/asm/arch-kirkwood/spi.h
index 1d5043f..5a38dc5 100644
--- a/arch/arm/include/asm/arch-kirkwood/spi.h
+++ b/arch/arm/include/asm/arch-kirkwood/spi.h
@@ -38,6 +38,7 @@ struct kwspi_registers {
 };
 
 #define KWSPI_CLKPRESCL_MASK	0x1f
+#define KWSPI_CLKPRESCL_MIN	0x12
 #define KWSPI_CSN_ACT		1 /* Activates serial memory interface */
 #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
 #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
index ee14669..9b3f6a4 100644
--- a/drivers/spi/kirkwood_spi.c
+++ b/drivers/spi/kirkwood_spi.c
@@ -57,8 +57,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
 
 	/* calculate spi clock prescaller using max_hz */
-	data = ((CONFIG_SYS_TCLK / 2) / max_hz) & KWSPI_CLKPRESCL_MASK;
-	data |= 0x10;
+	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
+	data = data < KWSPI_CLKPRESCL_MIN ? KWSPI_CLKPRESCL_MIN : data;
+	data = data > KWSPI_CLKPRESCL_MASK ? KWSPI_CLKPRESCL_MASK : data;
 
 	/* program spi clock prescaller using max_hz */
 	writel(KWSPI_ADRLEN_3BYTE | data, &spireg->cfg);
-- 
1.7.1

[U-Boot] [PATCH v2] kw_spi: fix clock prescaler computation

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 15 August 2012 21:02
> To: u-boot at lists.denx.de
> Cc: Valentin Longchamp; Holger Brunck; Prafulla Wadaskar
> Subject: [PATCH v2] kw_spi: fix clock prescaler computation
> 
> The computation was not correct with low clock values: setting a 1MHz
> clock would result in an overlap that would then configure a 25Mhz
> clock.
> 
> This patch implements a correct computation method according to the
> kirkwood functionnal spec. table 600 (Serial Memory Interface
> Configuration Register).
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Holger Brunck <holger.brunck@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> ---
> changes for v2:
>  - table nb in commit message
> 
> 
>  arch/arm/include/asm/arch-kirkwood/spi.h |    1 +
>  drivers/spi/kirkwood_spi.c               |    5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h
> b/arch/arm/include/asm/arch-kirkwood/spi.h
> index 1d5043f..5a38dc5 100644
> --- a/arch/arm/include/asm/arch-kirkwood/spi.h
> +++ b/arch/arm/include/asm/arch-kirkwood/spi.h
> @@ -38,6 +38,7 @@ struct kwspi_registers {
>  };
> 
>  #define KWSPI_CLKPRESCL_MASK	0x1f
> +#define KWSPI_CLKPRESCL_MIN	0x12
>  #define KWSPI_CSN_ACT		1 /* Activates serial memory interface
> */
>  #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
>  #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> index ee14669..9b3f6a4 100644
> --- a/drivers/spi/kirkwood_spi.c
> +++ b/drivers/spi/kirkwood_spi.c
> @@ -57,8 +57,9 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
> unsigned int cs,
>  	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
> 
>  	/* calculate spi clock prescaller using max_hz */
> -	data = ((CONFIG_SYS_TCLK / 2) / max_hz) & KWSPI_CLKPRESCL_MASK;
> -	data |= 0x10;
> +	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
> +	data = data < KWSPI_CLKPRESCL_MIN ? KWSPI_CLKPRESCL_MIN : data;
> +	data = data > KWSPI_CLKPRESCL_MASK ? KWSPI_CLKPRESCL_MASK : data;
> 
>  	/* program spi clock prescaller using max_hz */
>  	writel(KWSPI_ADRLEN_3BYTE | data, &spireg->cfg);
> --

Acked-by: Prafulla Wadaskar <Prafulla@marvell.com>
Will pull this soon.

Regards...
Prafulla . . .

[U-Boot] [PATCH v2] kw_spi: fix clock prescaler computation

[Prafulla Wadaskar]

> -----Original Message-----
> From: Prafulla Wadaskar
> Sent: 16 August 2012 09:44
> To: 'Valentin Longchamp'; u-boot at lists.denx.de
> Cc: Holger Brunck
> Subject: RE: [PATCH v2] kw_spi: fix clock prescaler computation
> 
> 
> 
> > -----Original Message-----
> > From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> > Sent: 15 August 2012 21:02
> > To: u-boot at lists.denx.de
> > Cc: Valentin Longchamp; Holger Brunck; Prafulla Wadaskar
> > Subject: [PATCH v2] kw_spi: fix clock prescaler computation
> >
> > The computation was not correct with low clock values: setting a
> 1MHz
> > clock would result in an overlap that would then configure a 25Mhz
> > clock.
> >
> > This patch implements a correct computation method according to the
> > kirkwood functionnal spec. table 600 (Serial Memory Interface
> > Configuration Register).
> >
> > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> > cc: Holger Brunck <holger.brunck@keymile.com>
> > cc: Prafulla Wadaskar <prafulla@marvell.com>
> > ---
> > changes for v2:
> >  - table nb in commit message
> >
> >
> >  arch/arm/include/asm/arch-kirkwood/spi.h |    1 +
> >  drivers/spi/kirkwood_spi.c               |    5 +++--
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h
> > b/arch/arm/include/asm/arch-kirkwood/spi.h
> > index 1d5043f..5a38dc5 100644
> > --- a/arch/arm/include/asm/arch-kirkwood/spi.h
> > +++ b/arch/arm/include/asm/arch-kirkwood/spi.h
> > @@ -38,6 +38,7 @@ struct kwspi_registers {
> >  };
> >
> >  #define KWSPI_CLKPRESCL_MASK	0x1f
> > +#define KWSPI_CLKPRESCL_MIN	0x12
> >  #define KWSPI_CSN_ACT		1 /* Activates serial memory interface
> > */
> >  #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
> >  #define KWSPI_IRQUNMASK		1 /* unmask SPI interrupt */
> > diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
> > index ee14669..9b3f6a4 100644
> > --- a/drivers/spi/kirkwood_spi.c
> > +++ b/drivers/spi/kirkwood_spi.c
> > @@ -57,8 +57,9 @@ struct spi_slave *spi_setup_slave(unsigned int
> bus,
> > unsigned int cs,
> >  	writel(~KWSPI_CSN_ACT | KWSPI_SMEMRDY, &spireg->ctrl);
> >
> >  	/* calculate spi clock prescaller using max_hz */
> > -	data = ((CONFIG_SYS_TCLK / 2) / max_hz) & KWSPI_CLKPRESCL_MASK;
> > -	data |= 0x10;
> > +	data = ((CONFIG_SYS_TCLK / 2) / max_hz) + 0x10;
> > +	data = data < KWSPI_CLKPRESCL_MIN ? KWSPI_CLKPRESCL_MIN : data;
> > +	data = data > KWSPI_CLKPRESCL_MASK ? KWSPI_CLKPRESCL_MASK : data;
> >
> >  	/* program spi clock prescaller using max_hz */
> >  	writel(KWSPI_ADRLEN_3BYTE | data, &spireg->cfg);
> > --
> 
> Acked-by: Prafulla Wadaskar <Prafulla@marvell.com>
> Will pull this soon.

Applied to u-boot-marvell.git to master branch by resolving merge conflict

Regards...
Prafulla . . .