[U-Boot] [PATCH v2] spi/kirkwood: add weak functions board_spi_bus_claim/release

[Valentin Longchamp <valentin.longchamp@keymile.com>]

Dear Prafulla,

On 03/30/2012 02:58 PM, Prafulla Wadaskar wrote:
>> -----Original Message-----
>> From: Valentin Longchamp [mailto:valentin.longchamp at keymile.com]
>> Sent: 30 March 2012 17:45
>> To: Prafulla Wadaskar
>> Cc: u-boot at lists.denx.de; Gerlando Falauto; Holger Brunck
>> Subject: Re: [PATCH v2] spi/kirkwood: add weak functions
>> board_spi_bus_claim/release
>>
>> Hi Prafulla,
>>
>> For the simplicity of the discussion, I have removed everything in the
>> discussion that is not relevant for the current open point.
>>
>> On 03/30/2012 01:34 PM, Prafulla Wadaskar wrote:
>>> In Kirkwood specific claim_bus API, you will backup default
>> configuration (which is NF in your case) for these particular pins
>> (SPI_SI, SPI_SCK, SPI_CSn, MOSI, MISO).
>>
>> But which MPP are these particular pins ?
>>
>> Let's have a look at a single signal, SPI_SCK for instance. From the
>> 88F6281
>> Hardware Spec [1], page 53, SPI_SCK can be MPP[2], MPP[10]. How can
>> the generic
>> driver know which one actually is wired to the SPI device SCK pin on
>> the
>> currently running hardware (when none is configured as then, since by
>> default
>> for us MPP[2] is NF_IO[4] and MPP[10] is UA0_TXD ) ? This is a board
>> specific !
>>
>> If you tell me how I easily can find this out in the kirkwood driver,
> 
> Dear Valentin
> 
> Please make a use of CONFIG_SF_DEFAULT_CS for this.

Is this really the correct #define to use ? From the documentation, this is
supposed to set the Serial Flash CS. OK our SPI controller is used for serial
flash access, but this may be used for something else.

So I would not use this #define, nor any that is related to Serial Flash support.

> By default this is defined to 0, lets map it to our default use case i.e. using MPP0-3 for default SPI signals
> 
> One may pre-define this in his board config as per specific need, and we can use this effectively in Kirkwood_spi driver.
> i.e.
>  bit0 to be used to configure SPI_CSn
>  bit1 to be used to configure SPI_MOSI... and so on
> 
> so if my needs are to use
> 1. MPP7 as SPI_CSn I will define CONFIG_SF_DEFAULT_CS to 0x01
> 2. MPP6 as SPI_MOSI I will define CONFIG_SF_DEFAULT_CS to 0x02
> 2. MPP6 as SPI_MOSI and MPP7 as SPI_CSn, I will define CONFIG_SF_DEFAULT_CS to 0x03
> .. and so on.
> 

Yeah I see what you mean and that's what I had figured out. But I don't think
this is a simple solution:

1) We would have to add another CONFIG_SYS_XY for this purpose (as explained above)

2) The two spi_claim_bus and spi_release_bus functions should implement some bit
masking on the new CONFIG_SYS_XY to determine 4 pins that have to be configured.
The number of code lines would not be huge, but that would still be a lot more
than the 10 lines that my board specific functions would require. 10 sequencial
c line codes/board are from my point of view acceptable (and it would be only
for our board, since I see no other board using this).

OK genericity is nice, but I guess we would be the only ones using this code,
and it would not be worth it in this case. Additionnaly, I like the fact that
anything that has to do with the mpp configurations stays in one single board
specific .c file.

Regards

-- 
Valentin Longchamp
Embedded Software Engineer
Hardware and Chip Integration
______________________________________
KEYMILE AG
Schwarzenburgstr. 73
CH-3097 Liebefeld
Phone +41 31 377 1318
Fax   +41 31 377 1212
valentin.longchamp at keymile.com
www.keymile.com
______________________________________
KEYMILE: A Specialist as a Partner