[U-Boot] [PATCH] spi: kirkwood_spi: Add support for multiple chip-selects on MVEBU

[Stefan Roese <sr@denx.de>]

Hi Jagan,

On 23.03.2016 15:36, Stefan Roese wrote:
> Hi Jagan,
>
> please excuse the long delay here. Some comments below.
>
> On 12.02.2016 21:27, Jagan Teki wrote:
>> On 12 February 2016 at 18:15, Stefan Roese <sr@denx.de> wrote:
>>> Currently only chip-select 0 is supported by the kirkwood SPI driver.
>>> The Armada XP / 38x SoCs also use this driver and support multiple chip
>>> selects. This patch adds support for multiple CS on MVEBU.
>>>
>>> The register definitions are restructured a bit with this patch.
>>> Grouping
>>> them to the corresponding registers.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Cc: Luka Perkov <luka.perkov@sartura.hr>
>>> Cc: Jagan Teki <jteki@openedev.com>
>>> ---
>>>   arch/arm/include/asm/arch-mvebu/spi.h | 17 ++++++++++++-----
>>>   drivers/spi/kirkwood_spi.c            |  6 ++++++
>>>   2 files changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-mvebu/spi.h
>>> b/arch/arm/include/asm/arch-mvebu/spi.h
>>> index e512dce..4b66499 100644
>>> --- a/arch/arm/include/asm/arch-mvebu/spi.h
>>> +++ b/arch/arm/include/asm/arch-mvebu/spi.h
>>> @@ -32,13 +32,15 @@ struct kwspi_registers {
>>>   #define SCK_MPP10      (1 << 1)
>>>   #define MISO_MPP11     (1 << 2)
>>>
>>> +/* Control Register */
>>> +#define KWSPI_CSN_ACT          (1 << 0) /* Activates serial memory
>>> interface */
>>> +#define KWSPI_SMEMRDY          (1 << 1) /* SerMem Data xfer ready */
>>> +#define KWSPI_CS_SHIFT         2       /* chip select shift */
>>> +#define KWSPI_CS_MASK          0x7     /* chip select mask */
>>> +
>>> +/* Configuration Register */
>>>   #define KWSPI_CLKPRESCL_MASK   0x1f
>>>   #define KWSPI_CLKPRESCL_MIN    0x12
>>> -#define KWSPI_CSN_ACT          1 /* Activates serial memory
>>> interface */
>>> -#define KWSPI_SMEMRDY          (1 << 1) /* SerMem Data xfer ready */
>>> -#define KWSPI_IRQUNMASK                1 /* unmask SPI interrupt */
>>> -#define KWSPI_IRQMASK          0 /* mask SPI interrupt */
>>> -#define KWSPI_SMEMRDIRQ                1 /* SerMem data xfer ready
>>> irq */
>>>   #define KWSPI_XFERLEN_1BYTE    0
>>>   #define KWSPI_XFERLEN_2BYTE    (1 << 5)
>>>   #define KWSPI_XFERLEN_MASK     (1 << 5)
>>> @@ -47,6 +49,11 @@ struct kwspi_registers {
>>>   #define KWSPI_ADRLEN_3BYTE     (2 << 8)
>>>   #define KWSPI_ADRLEN_4BYTE     (3 << 8)
>>>   #define KWSPI_ADRLEN_MASK      (3 << 8)
>>> +
>>> +#define KWSPI_IRQUNMASK                1 /* unmask SPI interrupt */
>>> +#define KWSPI_IRQMASK          0 /* mask SPI interrupt */
>>> +#define KWSPI_SMEMRDIRQ                1 /* SerMem data xfer ready
>>> irq */
>>> +
>>>   #define KWSPI_TIMEOUT          10000
>>>
>>>   #endif /* __KW_SPI_H__ */
>>> diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c
>>> index 7890796..fa30be4 100644
>>> --- a/drivers/spi/kirkwood_spi.c
>>> +++ b/drivers/spi/kirkwood_spi.c
>>> @@ -280,6 +280,12 @@ static int mvebu_spi_xfer(struct udevice *dev,
>>> unsigned int bitlen,
>>>          struct udevice *bus = dev->parent;
>>>          struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
>>>
>>> +#ifdef CONFIG_ARCH_MVEBU
>>> +       clrsetbits_le32(&plat->spireg->ctrl,
>>> +                       KWSPI_CS_MASK << KWSPI_CS_SHIFT,
>>> +                       spi_chip_select(dev) << KWSPI_CS_SHIFT);
>>> +#endif
>>
>> This looks clearing a desired cs from 'sf probe' why is this need
>> here?
>
> Its not clearing a CS but configuring the CS number in the CTRL
> register. This configured CS number will get enabled or disabled
> exactly the same way as when its zero (without this patch) in
> the existing spi_cs_activate/deactivate functions
>
>> I think we can do the desired cs activate or deactivate through
>> existing spi_cs_activate/deactivate functions itself.
>
> This is exactly what this patch does. The CS number is
> configured with the clrsetbits_le32() call above. And activated /
> deactivated in the existing functions.
>
> Or do you mean something else?

Any updates on this? I would like to have this multiple CS support
added in this release. I can push it via the marvell git repository
if you have no objections.

Thanks,
Stefan