Libertas with if_spi on pxa270

[Colin McCabe]

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.

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