Libertas with if_spi on pxa270

[Mike Rapoport]

Colin McCabe wrote:
> On Tue, Jan 27, 2009 at 3:51 AM, Mike Rapoport <mike at compulab.co.il> wrote:
>> Dan,
>>
>> Dan Williams wrote:
>>> On Mon, 2009-01-26 at 18:24 +0200, Mike Rapoport wrote:
>>>> Dan Williams wrote:
>>>>> On Mon, 2009-01-26 at 16:58 +0200, Mike Rapoport wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Have anybody tried to use 8686 with G-SPI on PXA270?
>>>>>> I've merged libertas-related changes from wireless-testing tree on top of
>>>>>> 2.6.29-rc2, added a "libertas_spi" device to my platform code, but all I could
>>>>>> get from the if_spi was "libertas: Can't read bus mode register." :(
>>>>> Have you set up the Chip Select stuff correctly for your board (see
>>>>> gpio_cs in the platform data)?  That seems to be board-dependent and
>>>>> there's not a good generic way of doing this in the current kernel
>>>>> sources.
>>>>>
>>>>> That error is the first error that will get reported when the driver
>>>>> actually tries to talk to the card, which requires that the Chip Select
>>>>> stuff be set up correctly.  If you think you've got that already set up,
>>>>> can you manually inspect whether or not the libertas device has actually
>>>>> been selected on the board?
>>>> The first thing I've done was to recheck GPIO setup and platform_data for all
>>>> SPI related stuff.
>>>> I've even inserted GPIO setup function at the very beginning of if_spi_probe.
>>>> The Marvell proprietary driver works well on the same board and exactly same
>>>> GPIOs setup.
>>> Ok, Colin McCabe will probably need to get involved here then since he
>>> wrote those bits :)
>> You were right about possible problems with my board setup. The 8686 requires
>> SPI clock pin to be high at power up to start with G-SPI rather than SDIO. That
>> part was missing from my board setup.
> 
> Good catch. Our board had pull-up resistors on SCLK, MISO, and CS, so
> we never experienced this.
> 
>> Still I needed to alter the if_spi.c to make it work. First of all, I think it
>> makes sence to add
>>
>>        spi->bits_per_word = 16;
>>        spi_setup(spi);
>>
>> at the beginning of if_spi_probe.
> 
> I would be more in favor of something like
> BUG_ON(spi->bits_per_word != 16).
> rather than silently overriding the configuration given in the
> board-specific files.

Currently the only way to set bits_per_word for PXA SPI implementation is using
spi_setup. If we introduce platform-specific init, I can call spi_setup from the
platform-specific init function, at least until this issue with PXA SPI
controller driver is resolved.

> regards,
> Colin
> 
>> Another thing, it's possible worth adding ability to call platform specific
>> init/exit functions supplied with libertas_spi_platform_data.
>>
>> After the driver begun to work I've noted that it's performance is not as good
>> as I expected. The reason for it is that PXA SPI requires buffers to be 8-byte
>> aligned to use DMA. And, indeed, changing alignment from 4 to 8 throughout the
>> if_spi.c gave me x2 boost in large file transfer rate. I've thought about adding
>> a LIBERTAS_SPI_DMA_ALIGN option to drivers/net/wireless/Kconfig to allow
>> platforms to select required DMA alignment.
>>
>> If it sounds reasonable, I can send a patch with the proposed changes.
>>
>>> Dan
>>>
>>>
>> --
>> Sincerely yours,
>> Mike.
>>
>>
> 

-- 
Sincerely yours,
Mike.