[MinnowBoard] Multiple SPI CS lines on MinnowBoard Max

[John Hawley]

On 06/24/2015 04:17 PM, Damien Dusha wrote:
> Dear All,
> 
> I have been looking at having multiple chip selects on the SPI
> peripheral of the MinnowBoard Max (Bay Trail). The hardware only
> natively supports one CS line and therefore using GPIO(s)  as chip
> select is necessary.  This topic has previously been discussed in
> generalities on this mailing list [1].
> 
> The spi-pxa2xx.c driver does indeed support multiple GPIO CS - see the
> following snippet:
> 
> static void cs_assert(struct driver_data *drv_data)
> {
>     /* ... */
> 
>     if (gpio_is_valid(chip->gpio_cs)) {
>         gpio_set_value(chip->gpio_cs, chip->gpio_cs_inverted);
>         return;
>     }
> 
>     /* ... */
> }
> 
> This interface is used in at least one (arm) platform driver with no
> less than two GPIO CS lines per bus - see
> arch/arm/mach-pxa/icontrol.c.   There is also support for GPIO CS
> lines being defined via a device tree, but we don't have that option
> on x86.
> 
> This particular board board also has the distinct "advantage" of
> specifying the bus master itself as a platform device and hence is
> able to define the number of CS lines that it has:
> 
> static struct pxa2xx_spi_master pxa_ssp3_spi_master_info = {
>     .clock_enable   = CKEN_SSP3,
>     .num_chipselect = 2,
>     .enable_dma     = 1
> };
> 
> On the bay trail, we're not so "lucky" as to define a platform device
> for this driver. The number of chip selects has been defined as
> precisely 1 when it is enumerated on the PCI bus. This can be seen in
> spi-pxa2xx-pci.c:
> 
> static struct pxa_spi_info spi_info_configs[] = {
>     /* ... */
>     [PORT_BYT] = {
>         .type = LPSS_SSP,
>         .port_id = 0,
>         .num_chipselect = 1,
>         .max_clk_rate = 50000000,
>         .tx_param = &byt_tx_param,
>         .rx_param = &byt_rx_param,
>     },
>     /* ... */
> };
> 
> Now, if I apply a judicious hack at this point to make the
> num_chipselect = 2, then I can happily make a board file* that
> declares my SPI slaves with a GPIO CS for each (not using the native
> one at all), which appear to be working as expected (without yet
> performing any stress testing).
> 
> Clearly, such a hack to the main SPI driver will result in maintaining
> yet another "evil vendor tree".   So, what would the "proper" way to
> introduce support for multiple GPIO CS lines to this particular
> driver.  By proper, I mean something that would result in the patch
> being accepted upstream?
> 
> Best regards,
> Damien.
> 
> 
> [1] http://comments.gmane.org/gmane.comp.hardware.minnowboard.user/1317
> * With apologies: http://thread.gmane.org/gmane.linux.kernel/1515584

So this is fantastic news, and I'm glad that it's working.  Next steps,
as you've surmised, is likely to have a chat with the maintainers, who
look to be:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/MAINTAINERS#n8012

I suspect what we may, actually, want is some sort of default for number
of "physical" CS lines, but with an option to override it with the
number of CS lines, and the GPIOs to use for them.  Do you have your
changed version of the code, might be worth looking at what you did to
get it to work to try and figure out something to discuss with the
maintainers.

- John 'Warthog9' Hawley