From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Wed, 6 Apr 2016 13:32:28 +0200 Subject: [U-Boot] [PATCH] spi: kirkwood_spi: Add support for multiple chip-selects on MVEBU In-Reply-To: References: <1455281149-4783-1-git-send-email-sr@denx.de> <56F2A9FA.8060708@denx.de> <57035509.3030709@denx.de> Message-ID: <5704F3CC.7090706@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jagan, On 06.04.2016 13:21, Jagan Teki wrote: > On 5 April 2016 at 11:32, Stefan Roese wrote: >> 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 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 >>>>> Cc: Luka Perkov >>>>> Cc: Jagan Teki >>>>> --- >>>>> 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 > > OK, if ie the case configure doesn't require for each spi_xfer, isn't > it? Correct. > may be we can put this on to probe or setup_slave area? AFAIK, probe() is only called once per SPI controller. Without any device (chip-select) to use. We need a function that's called per SPI device for this specific chip-select configuration. Checking a bit further, I could use claim_bus() for this. If you are okay, I'll send a v2, using clain_bus() for this CS configuration. Okay? Thanks, Stefan