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

[Valentin Longchamp]

On 05/29/2012 12:29 PM, Prafulla Wadaskar wrote:
>> -----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.

That's already how it is implemented ...:

bit 0: selects pin for CSn (MPP0 if 0, MPP7 if 1)
bit 1: selects pin for MOSI (MPP1 if 0, MPP6 if 1)
bit 2: selects pin for SCK (MPP2 if 0, MPP10 if 1)
bit 3: selects pin for MISO (MPP3 if 0, MPP11 if 1)

> 
> 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 . . .