Libertas with if_spi on pxa270

[Dan Williams]

On Tue, 2009-01-27 at 13:51 +0200, Mike Rapoport 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.
> 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.

Is that a restriction of/optimization for the SPI controller?  See below
if it is...  It seems like the SPI layer in the kernel has a bit of
maturation to do.

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

The alignment issue is a controller specific issue and should be quirked
in the SPI controller code (where the restriction really lies),
hopefully not in the libertas code itself.

Colin must/may have had a reason to only use 4-byte alignment; perhaps
his controller requires it.  But that re-inforces the point that if the
controller has the quirks, they should be handled in the controller code
as much as possible.

Dan