[U-Boot] [PATCH 2/3] spi/kirkwood: support spi_claim/release_bus functions

[Prafulla Wadaskar]

> -----Original Message-----
> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> Sent: 29 May 2012 14:02
> To: Prafulla Wadaskar
> Cc: holger.brunck at keymile.com; u-boot at lists.denx.de
> Subject: Re: [PATCH 2/3] spi/kirkwood: support spi_claim/release_bus
> functions
> 
> On 05/24/2012 10:35 AM, Prafulla Wadaskar wrote:
> >> -----Original Message-----
> >> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
> >> Sent: 16 May 2012 16:24
> >> To: Prafulla Wadaskar; holger.brunck at keymile.com
> >> Cc: Valentin Longchamp; u-boot at lists.denx.de; Holger Brunck;
> Prafulla
> >> Wadaskar
> >> Subject: [PATCH 2/3] spi/kirkwood: support spi_claim/release_bus
> >> functions
> >>
> >> These two function nows ensure that the MPP is configured correctly
> >> for
> >> the SPI controller before any SPI access, and restore the initial
> >> configuration when the access is over.
> >>
> >> Since the used pins for the SPI controller can differ (2
> possibilities
> >> for each signal), the used pins are configured with
> >> CONFIG_SYS_KW_SPI_MPP.
> >>
> >> Signed-off-by: Valentin Longchamp <valentin.longchamp at keymile.com>
> >> cc: Holger Brunck <holger.brunck at keymile.com>
> >> cc: Prafulla Wadaskar <prafulla at marvell.com>
> >> ---
> >>  arch/arm/include/asm/arch-kirkwood/spi.h |    9 ++++++++
> >>  drivers/spi/kirkwood_spi.c               |   34
> >> ++++++++++++++++++++++++++++++
> >>  2 files changed, 43 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-kirkwood/spi.h
> >> b/arch/arm/include/asm/arch-kirkwood/spi.h
> >> index 1d5043f..305c573 100644
> >> --- a/arch/arm/include/asm/arch-kirkwood/spi.h
> >> +++ b/arch/arm/include/asm/arch-kirkwood/spi.h
> >> @@ -37,6 +37,15 @@ struct kwspi_registers {
> >>  	u32 irq_mask;	/* 0x10614 */
> >>  };
> >>
> >> +#define CSn_MPP7	0x1
> >> +#define MOSI_MPP6	0x2
> >> +#define SCK_MPP10	0x4
> >> +#define MISO_MPP11	0x8
> >
> > Let's define above as (1 << x) to make it more readable.
> 
> OK
> 
> >
> >> +
> >> +#ifndef CONFIG_SYS_KW_SPI_MPP
> >> +#define CONFIG_SYS_KW_SPI_MPP	0x0
> >
> > Some more documentation is needed, you need to explain how each bit
> we are using to configure the SPI-MPPs
> 
> Not sure I understand what you mean here. But I think that you mean
> that I would
> have to document that bit 1 is for CSn signal (MPP0 or MPP7), bit 2
> for MOSI
> signal (MPP1 or MPP6) and so on ... OK will do it.

Exactly, 

> 
> Would you want me to define CSn_MPP0 as 0x0 (or (0 << 0) ) and
> MOSI_MPP1 as 0x0
>  and so on as well ?

Use any four bits for four configuration, I would suggest to use bit0 to bit-3.

Regards..
Prafulla . . .

> 
> >
> >> +#endif
> >> +
> >>  #define KWSPI_CLKPRESCL_MASK	0x1f
> >>  #define KWSPI_CSN_ACT		1 /* Activates serial memory interface
> >> */
> >>  #define KWSPI_SMEMRDY		(1 << 1) /* SerMem Data xfer ready */
> >> diff --git a/drivers/spi/kirkwood_spi.c
> b/drivers/spi/kirkwood_spi.c
> >> index db8ba8b..0877915 100644
> >> --- a/drivers/spi/kirkwood_spi.c
> >> +++ b/drivers/spi/kirkwood_spi.c
> >> @@ -88,11 +88,45 @@ void spi_free_slave(struct spi_slave *slave)
> >>
> >>  int spi_claim_bus(struct spi_slave *slave)
> >>  {
> >
> > Instead define here
> > #ifdef CONFIG_SYS_KW_SPI_MPP, otherwise build with default.
> 
> OK, if you prefer it this way, it is fine for me. This implies that I
> have to
> remove the above #ifndef CONFIG_SYS_KW_SPI_MPP and that the boards
> that want to
> use this will have to #define CONFIG_SYS_KW_SPI_MPP in their config.
> 
> >
> >> +	u32 config;
> >> +	u32 spi_mpp_config[5];
> >> +
> >> +	config = CONFIG_SYS_KW_SPI_MPP;
> >> +
> >> +	if (config & CSn_MPP7)
> >> +		spi_mpp_config[0] = MPP7_SPI_SCn;
> >> +	else
> >> +		spi_mpp_config[0] = MPP0_SPI_SCn;
> >> +
> >> +	if (config & MOSI_MPP6)
> >> +		spi_mpp_config[1] = MPP6_SPI_MOSI;
> >> +	else
> >> +		spi_mpp_config[1] = MPP1_SPI_MOSI;
> >> +
> >> +	if (config & SCK_MPP10)
> >> +		spi_mpp_config[2] = MPP10_SPI_SCK;
> >> +	else
> >> +		spi_mpp_config[2] = MPP2_SPI_SCK;
> >> +
> >> +	if (config & MISO_MPP11)
> >> +		spi_mpp_config[3] = MPP11_SPI_MISO;
> >> +	else
> >> +		spi_mpp_config[3] = MPP3_SPI_MISO;
> >> +
> >> +	spi_mpp_config[4] = 0;
> >> +
> >> +	/* save current mpp configuration */
> >> +	kirkwood_mpp_save();
> >> +
> >> +	/* finally set chosen mpp spi configuration */
> >> +	kirkwood_mpp_conf(spi_mpp_config);
> >> +
> >>  	return 0;
> >>  }
> >>
> >>  void spi_release_bus(struct spi_slave *slave)
> >>  {
> >> +	kirkwood_mpp_restore();
> >>  }
> >>
> >>  #ifndef CONFIG_SPI_CS_IS_VALID
> >> --
> >
> > Regards..
> > Prafulla . . .