[U-Boot] [PATCH v0 0/5] Some improvements for mvebu/a38x
From: Dirk Eibach dirk.eibach@gdsys.cc
Dirk Eibach (5): pci: mvebu: Fix Armada 38x support arm: mvebu: Add gpio support arm: mvebu: Fix SAR1_CPU_CORE_MASK arm: mvebu: Fix ddr3_init() cpu config spi: Add support for Armada 38x second controller
arch/arm/mach-mvebu/include/mach/gpio.h | 41 ++++++++++++++++- arch/arm/mach-mvebu/include/mach/soc.h | 1 + .../ddr/marvell/a38x/ddr3_hws_hw_training_def.h | 7 +-- drivers/ddr/marvell/a38x/ddr3_init.c | 2 - drivers/pci/pci_mvebu.c | 22 ++++----- drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++--- 6 files changed, 98 insertions(+), 27 deletions(-)
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has 4 pci ports, not 3. The optimization in pci_init_board() seems to assume, that every port has 3 lanes. This is obviously wrong and breaks support for Armada 38x.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc ---
arch/arm/mach-mvebu/include/mach/soc.h | 1 + drivers/pci/pci_mvebu.c | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h index 02c21bc..a1014b3 100644 --- a/arch/arm/mach-mvebu/include/mach/soc.h +++ b/arch/arm/mach-mvebu/include/mach/soc.h @@ -67,6 +67,7 @@ #define MVEBU_USB20_BASE (MVEBU_REGISTER(0x58000)) #define MVEBU_EGIGA0_BASE (MVEBU_REGISTER(0x70000)) #define MVEBU_EGIGA1_BASE (MVEBU_REGISTER(0x74000)) +#define MVEBU_REG_PCIE0_BASE (MVEBU_REGISTER(0x80000)) #define MVEBU_AXP_SATA_BASE (MVEBU_REGISTER(0xa0000)) #define MVEBU_SATA0_BASE (MVEBU_REGISTER(0xa8000)) #define MVEBU_NAND_BASE (MVEBU_REGISTER(0xd0000)) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index fd2744d..50e6419 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -90,26 +90,27 @@ static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE;
#if defined(CONFIG_ARMADA_38X) #define PCIE_BASE(if) \ - ((if) == 0 ? \ - MVEBU_REG_PCIE_BASE + 0x40000 : \ - MVEBU_REG_PCIE_BASE + 0x4000 * (if)) + ((if) == 0 ? \ + MVEBU_REG_PCIE0_BASE : \ + (MVEBU_REG_PCIE_BASE + 0x4000 * (if - 1)))
/* * On A38x MV6820 these PEX ports are supported: * 0 - Port 0.0 - * 1 - Port 0.1 - * 2 - Port 0.2 + * 1 - Port 1.0 + * 2 - Port 2.0 + * 3 - Port 3.0 */ -#define MAX_PEX 3 +#define MAX_PEX 4 static struct mvebu_pcie pcie_bus[MAX_PEX];
static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx, int *mem_target, int *mem_attr) { - u8 port[] = { 0, 1, 2 }; - u8 lane[] = { 0, 0, 0 }; - u8 target[] = { 8, 4, 4 }; - u8 attr[] = { 0xe8, 0xe8, 0xd8 }; + u8 port[] = { 0, 1, 2, 3 }; + u8 lane[] = { 0, 0, 0, 0 }; + u8 target[] = { 8, 4, 4, 4 }; + u8 attr[] = { 0xe8, 0xe8, 0xd8, 0xb8 };
pcie->port = port[pex_idx]; pcie->lane = lane[pex_idx]; @@ -344,7 +345,6 @@ void pci_init_board(void)
/* Don't read at all from pci registers if port power is down */ if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0) { - i += 3; debug("%s: skipping port %d\n", __func__, pcie->port); continue; }
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has 4 pci ports, not 3. The optimization in pci_init_board() seems to assume, that every port has 3 lanes. This is obviously wrong and breaks support for Armada 38x.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
Looks good, so:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Hi Dirk,
2015-10-28 16:44 GMT+01:00 dirk.eibach@gdsys.cc:
From: Dirk Eibach dirk.eibach@gdsys.cc
@@ -344,7 +345,6 @@ void pci_init_board(void)
/* Don't read at all from pci registers if port power isdown */ if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0) {
i += 3; debug("%s: skipping port %d\n", __func__,pcie->port); continue; }
Is there a specific reason why you removed this line or was it just by mistake? Because I think doing so would break Armada XP in certain Serdes Configurations, as it doesn't like it's PCI registers being read if the port is off.
Kind Regards, Anton
Hi Anton,
2015-11-17 13:55 GMT+01:00 Anton Schubert anton.schubert@gmx.de:
Hi Dirk,
2015-10-28 16:44 GMT+01:00 dirk.eibach@gdsys.cc:
From: Dirk Eibach dirk.eibach@gdsys.cc
@@ -344,7 +345,6 @@ void pci_init_board(void)
/* Don't read at all from pci registers if port power isdown */ if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0) {
i += 3; debug("%s: skipping port %d\n", __func__,pcie->port); continue; }
Is there a specific reason why you removed this line or was it just by mistake? Because I think doing so would break Armada XP in certain Serdes Configurations, as it doesn't like it's PCI registers being read if the port is off.
I assume the idea is to go to the next port if the current port is disabled. But adding 3 to the index does not seem to be the right thing to do, since Armada XP has ports with 4 lanes, but also with ports with one lane. I assume that iterating over all lanes would not be a problem, but by mistake the pcie->lane == 0 was left in the condition. So this should perform better:
/* Don't read at all from pci registers if port power is down */ if (SELECT(soc_ctrl, pcie->port) == 0) { if (pcie->lane == 0) debug("%s: skipping port %d\n", __func__, pcie->port); continue; }
What do you think?
Cheers Dirk
Am 18.11.2015 um 13:48 schrieb Dirk Eibach:
I assume the idea is to go to the next port if the current port is disabled. But adding 3 to the index does not seem to be the right thing to do, since Armada XP has ports with 4 lanes, but also with ports with one lane. I assume that iterating over all lanes would not be a problem, but by mistake the pcie->lane == 0 was left in the condition.
Yeah you are right. The additional condition was superfluous in the original version.
So this should perform better:
/* Don't read at all from pci registers if port power is down */ if (SELECT(soc_ctrl, pcie->port) == 0) { if (pcie->lane == 0) debug("%s: skipping port %d\n", __func__, pcie->port); continue; }
I agree.
Regards, Anton
2015-11-18 14:23 GMT+01:00 Anton Schubert anton.schubert@gmx.de:
Am 18.11.2015 um 13:48 schrieb Dirk Eibach:
I assume the idea is to go to the next port if the current port is disabled. But adding 3 to the index does not seem to be the right thing to do, since Armada XP has ports with 4 lanes, but also with ports with one lane. I assume that iterating over all lanes would not be a problem, but by mistake the pcie->lane == 0 was left in the condition.
Yeah you are right. The additional condition was superfluous in the original version.
So this should perform better:
/* Don't read at all from pci registers if port power is down */ if (SELECT(soc_ctrl, pcie->port) == 0) { if (pcie->lane == 0) debug("%s: skipping port %d\n", __func__, pcie->port); continue; }
I agree.
Fine. I will add this to the v1 series.
Cheers Dirk
From: Dirk Eibach dirk.eibach@gdsys.cc
mvebu gpio is based on kirkwood.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc ---
arch/arm/mach-mvebu/include/mach/gpio.h | 41 +++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mvebu/include/mach/gpio.h b/arch/arm/mach-mvebu/include/mach/gpio.h index 09e3c50..cd869ee 100644 --- a/arch/arm/mach-mvebu/include/mach/gpio.h +++ b/arch/arm/mach-mvebu/include/mach/gpio.h @@ -1,10 +1,47 @@ /* - * SPDX-License-Identifier: GPL-2.0+ + * SPDX-License-Identifier: GPL-2.0+ + */ + +/* + * Based on (mostly copied from) plat-orion based Linux 2.6 kernel driver. + * Removed kernel level irq handling. Took some macros from kernel to + * allow build. + * + * Dieter Kiermaier dk-arm-linux@gmx.de */
#ifndef __MACH_MVEBU_GPIO_H #define __MACH_MVEBU_GPIO_H
-/* Empty file - sdhci requires this. */ +/* got from kernel include/linux/bitops.h */ +#define BITS_PER_BYTE 8 +#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) + +#define GPIO_MAX 59 +#define GPIO_OFF(pin) (((pin) >> 5) ? 0x0040 : 0x0000) +#define GPIO_OUT(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x00) +#define GPIO_IO_CONF(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x04) +#define GPIO_BLINK_EN(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x08) +#define GPIO_IN_POL(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x0c) +#define GPIO_DATA_IN(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x10) +#define GPIO_EDGE_CAUSE(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x14) +#define GPIO_EDGE_MASK(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x18) +#define GPIO_LEVEL_MASK(pin) (MVEBU_GPIO0_BASE + GPIO_OFF(pin) + 0x1c) + +/* + * Kirkwood-specific GPIO API + */ + +void kw_gpio_set_valid(unsigned pin, int mode); +int kw_gpio_is_valid(unsigned pin, int mode); +int kw_gpio_direction_input(unsigned pin); +int kw_gpio_direction_output(unsigned pin, int value); +int kw_gpio_get_value(unsigned pin); +void kw_gpio_set_value(unsigned pin, int value); +void kw_gpio_set_blink(unsigned pin, int blink); +void kw_gpio_set_unused(unsigned pin); + +#define GPIO_INPUT_OK (1 << 0) +#define GPIO_OUTPUT_OK (1 << 1)
#endif
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
mvebu gpio is based on kirkwood.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
Again, I would really like to see a DM based driver for this. It shouldn't be too much work. GPIO is perhaps the simplest driver here. Perhaps you can spare some cycles for such a task...
Thanks, Stefan
From: Dirk Eibach dirk.eibach@gdsys.cc
SAR1_CPU_CORE_MASK was wrong, probably copy/paste from another architecture.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc ---
drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h b/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h index 7500a72..06d0ab1 100644 --- a/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h +++ b/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h @@ -23,8 +23,8 @@
#define CPU_CONFIGURATION_REG(id) (0x21800 + (id * 0x100)) #define CPU_MRVL_ID_OFFSET 0x10 -#define SAR1_CPU_CORE_MASK 0x00000018 -#define SAR1_CPU_CORE_OFFSET 3 +#define SAR1_CPU_CORE_MASK 0x38000000 +#define SAR1_CPU_CORE_OFFSET 27
#define NEW_FABRIC_TWSI_ADDR 0x4e #ifdef DB_784MP_GP @@ -461,7 +461,4 @@ #define CLK_CPU_2200 13 #define CLK_CPU_2400 14
-#define SAR1_CPU_CORE_MASK 0x00000018 -#define SAR1_CPU_CORE_OFFSET 3 - #endif /* _DDR3_HWS_HW_TRAINING_DEF_H */
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
SAR1_CPU_CORE_MASK was wrong, probably copy/paste from another architecture.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h b/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h index 7500a72..06d0ab1 100644 --- a/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h +++ b/drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h @@ -23,8 +23,8 @@
#define CPU_CONFIGURATION_REG(id) (0x21800 + (id * 0x100)) #define CPU_MRVL_ID_OFFSET 0x10 -#define SAR1_CPU_CORE_MASK 0x00000018 -#define SAR1_CPU_CORE_OFFSET 3 +#define SAR1_CPU_CORE_MASK 0x38000000 +#define SAR1_CPU_CORE_OFFSET 27
#define NEW_FABRIC_TWSI_ADDR 0x4e #ifdef DB_784MP_GP @@ -461,7 +461,4 @@ #define CLK_CPU_2200 13 #define CLK_CPU_2400 14
-#define SAR1_CPU_CORE_MASK 0x00000018 -#define SAR1_CPU_CORE_OFFSET 3
- #endif /* _DDR3_HWS_HW_TRAINING_DEF_H */
Thanks for spotting. Seems to be correct from the datasheet. How did you find this problem? What exactly did happen on your board?
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Hi Stefan,
2015-10-28 17:34 GMT+01:00 Stefan Roese sr@denx.de:
Thanks for spotting. Seems to be correct from the datasheet. How did you find this problem? What exactly did happen on your board?
I did a code review when we had some DDR3 problems on initial board bringup. This issue had no visible consequences.
Cheers Dirk
On Wed, Oct 28, 2015 at 04:44:14PM +0100, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
SAR1_CPU_CORE_MASK was wrong, probably copy/paste from another architecture.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/ddr/marvell/a38x/ddr3_hws_hw_training_def.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Applied in marvell/master. Thank you!
Luka
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has a maximum of two cores. Probably copy/paste bug from Armada XP.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc ---
drivers/ddr/marvell/a38x/ddr3_init.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c index d6ed8e0..cbfc58c 100644 --- a/drivers/ddr/marvell/a38x/ddr3_init.c +++ b/drivers/ddr/marvell/a38x/ddr3_init.c @@ -306,8 +306,6 @@ int ddr3_init(void) SAR1_CPU_CORE_OFFSET; switch (soc_num) { case 0x3: - reg_bit_set(CPU_CONFIGURATION_REG(3), CPU_MRVL_ID_OFFSET); - reg_bit_set(CPU_CONFIGURATION_REG(2), CPU_MRVL_ID_OFFSET); case 0x1: reg_bit_set(CPU_CONFIGURATION_REG(1), CPU_MRVL_ID_OFFSET); case 0x0:
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has a maximum of two cores. Probably copy/paste bug from Armada XP.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/ddr/marvell/a38x/ddr3_init.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c index d6ed8e0..cbfc58c 100644 --- a/drivers/ddr/marvell/a38x/ddr3_init.c +++ b/drivers/ddr/marvell/a38x/ddr3_init.c @@ -306,8 +306,6 @@ int ddr3_init(void) SAR1_CPU_CORE_OFFSET; switch (soc_num) { case 0x3:
reg_bit_set(CPU_CONFIGURATION_REG(3), CPU_MRVL_ID_OFFSET);reg_bit_set(CPU_CONFIGURATION_REG(2), CPU_MRVL_ID_OFFSET);
Shouldn't you remove the "case 0x3:" line as well?
Thanks, Stefan
Hi Stefan,
2015-10-28 17:35 GMT+01:00 Stefan Roese sr@denx.de:
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has a maximum of two cores. Probably copy/paste bug from Armada XP.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/ddr/marvell/a38x/ddr3_init.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c index d6ed8e0..cbfc58c 100644 --- a/drivers/ddr/marvell/a38x/ddr3_init.c +++ b/drivers/ddr/marvell/a38x/ddr3_init.c @@ -306,8 +306,6 @@ int ddr3_init(void) SAR1_CPU_CORE_OFFSET; switch (soc_num) { case 0x3:
reg_bit_set(CPU_CONFIGURATION_REG(3), CPU_MRVL_ID_OFFSET);reg_bit_set(CPU_CONFIGURATION_REG(2), CPU_MRVL_ID_OFFSET); case 0x1: reg_bit_set(CPU_CONFIGURATION_REG(1), CPU_MRVL_ID_OFFSET); case 0x0:Shouldn't you remove the "case 0x3:" line as well?
Nope, according to Reset Configuration Pins table in the hardware spec 0 means Armada 380 (singlecore), 1 means Armada 385 (dualcore) and 3 means Armada 388 (dualcore). So handling soc_num 1 and 3 the same way is perfectly allright.
Cheers Dirk
Hi Dirk,
On 29.10.2015 10:51, Dirk Eibach wrote:
2015-10-28 17:35 GMT+01:00 Stefan Roese sr@denx.de:
Hi Dirk,
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has a maximum of two cores. Probably copy/paste bug from Armada XP.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/ddr/marvell/a38x/ddr3_init.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/ddr/marvell/a38x/ddr3_init.c b/drivers/ddr/marvell/a38x/ddr3_init.c index d6ed8e0..cbfc58c 100644 --- a/drivers/ddr/marvell/a38x/ddr3_init.c +++ b/drivers/ddr/marvell/a38x/ddr3_init.c @@ -306,8 +306,6 @@ int ddr3_init(void) SAR1_CPU_CORE_OFFSET; switch (soc_num) { case 0x3:
reg_bit_set(CPU_CONFIGURATION_REG(3), CPU_MRVL_ID_OFFSET);reg_bit_set(CPU_CONFIGURATION_REG(2), CPU_MRVL_ID_OFFSET); case 0x1: reg_bit_set(CPU_CONFIGURATION_REG(1), CPU_MRVL_ID_OFFSET); case 0x0:Shouldn't you remove the "case 0x3:" line as well?
Nope, according to Reset Configuration Pins table in the hardware spec 0 means Armada 380 (singlecore), 1 means Armada 385 (dualcore) and 3 means Armada 388 (dualcore). So handling soc_num 1 and 3 the same way is perfectly allright.
Thanks for the explanation:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
On 28.10.2015 16:44, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has a maximum of two cores. Probably copy/paste bug from Armada XP.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
Applied to u-boot-marvell/master.
Thanks, Stefan
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has two spi controllers.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc ---
drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c index e7b0982..200c391 100644 --- a/drivers/spi/kirkwood_spi.c +++ b/drivers/spi/kirkwood_spi.c @@ -18,17 +18,25 @@ #endif #include <asm/arch-mvebu/spi.h>
-static struct kwspi_registers *spireg = - (struct kwspi_registers *)MVEBU_SPI_BASE; - #ifdef CONFIG_KIRKWOOD static u32 cs_spi_mpp_back[2]; #endif
+struct kwspi_slave { + struct spi_slave slave; + struct kwspi_registers *spireg; +}; + +static inline struct kwspi_slave *to_kwspi(struct spi_slave *slave) +{ + return container_of(slave, struct kwspi_slave, slave); +} + struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, unsigned int max_hz, unsigned int mode) { - struct spi_slave *slave; + struct kwspi_slave *kwspi_slave; + struct kwspi_registers *spireg; u32 data; #ifdef CONFIG_KIRKWOOD static const u32 kwspi_mpp_config[2][2] = { @@ -40,10 +48,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, if (!spi_cs_is_valid(bus, cs)) return NULL;
- slave = spi_alloc_slave_base(bus, cs); - if (!slave) + kwspi_slave = spi_alloc_slave(struct kwspi_slave, bus, cs); + if (!kwspi_slave) return NULL;
+ switch (bus) { + case 0: + kwspi_slave->spireg = (struct kwspi_registers *)MVEBU_SPI_BASE; + break; +#ifdef CONFIG_ARMADA_38X + /* Armada 38x has two SPI controllers */ + case 1: + kwspi_slave->spireg = + (struct kwspi_registers *)(MVEBU_SPI_BASE + 0x80); + break; +#endif + default: + return NULL; + } + + spireg = kwspi_slave->spireg; + writel(KWSPI_SMEMRDY, &spireg->ctrl);
/* calculate spi clock prescaller using max_hz */ @@ -63,7 +88,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, kirkwood_mpp_conf(kwspi_mpp_config[cs ? 1 : 0], cs_spi_mpp_back); #endif
- return slave; + return &kwspi_slave->slave; }
void spi_free_slave(struct spi_slave *slave) @@ -137,7 +162,12 @@ void spi_release_bus(struct spi_slave *slave) */ int spi_cs_is_valid(unsigned int bus, unsigned int cs) { +#ifdef CONFIG_ARMADA_38X + /* Armada 38x has two SPI controllers */ + return (bus < 2) && (cs < 3); +#else return bus == 0 && (cs == 0 || cs == 1); +#endif } #endif
@@ -147,11 +177,17 @@ void spi_init(void)
void spi_cs_activate(struct spi_slave *slave) { + struct kwspi_slave *kwspi_slave = to_kwspi(slave); + struct kwspi_registers *spireg = kwspi_slave->spireg; + setbits_le32(&spireg->ctrl, KWSPI_CSN_ACT); }
void spi_cs_deactivate(struct spi_slave *slave) { + struct kwspi_slave *kwspi_slave = to_kwspi(slave); + struct kwspi_registers *spireg = kwspi_slave->spireg; + clrbits_le32(&spireg->ctrl, KWSPI_CSN_ACT); }
@@ -160,6 +196,8 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, { unsigned int tmpdout, tmpdin; int tm, isread = 0; + struct kwspi_slave *kwspi_slave = to_kwspi(slave); + struct kwspi_registers *spireg = kwspi_slave->spireg;
debug("spi_xfer: slave %u:%u dout %p din %p bitlen %u\n", slave->bus, slave->cs, dout, din, bitlen);
On 28 October 2015 at 21:14, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has two spi controllers.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c index e7b0982..200c391 100644 --- a/drivers/spi/kirkwood_spi.c +++ b/drivers/spi/kirkwood_spi.c @@ -18,17 +18,25 @@ #endif #include <asm/arch-mvebu/spi.h>
-static struct kwspi_registers *spireg =
(struct kwspi_registers *)MVEBU_SPI_BASE;#ifdef CONFIG_KIRKWOOD static u32 cs_spi_mpp_back[2]; #endif
+struct kwspi_slave {
struct spi_slave slave;struct kwspi_registers *spireg;+};
+static inline struct kwspi_slave *to_kwspi(struct spi_slave *slave) +{
return container_of(slave, struct kwspi_slave, slave);+}
struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, unsigned int max_hz, unsigned int mode) {
struct spi_slave *slave;
struct kwspi_slave *kwspi_slave;struct kwspi_registers *spireg; u32 data;#ifdef CONFIG_KIRKWOOD static const u32 kwspi_mpp_config[2][2] = { @@ -40,10 +48,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, if (!spi_cs_is_valid(bus, cs)) return NULL;
slave = spi_alloc_slave_base(bus, cs);if (!slave)
kwspi_slave = spi_alloc_slave(struct kwspi_slave, bus, cs);if (!kwspi_slave) return NULL;switch (bus) {case 0:kwspi_slave->spireg = (struct kwspi_registers *)MVEBU_SPI_BASE;break;+#ifdef CONFIG_ARMADA_38X
/* Armada 38x has two SPI controllers */
Can you please add this through driver-model, I understand it's bit big task but I can help you at any moment.
case 1:kwspi_slave->spireg =(struct kwspi_registers *)(MVEBU_SPI_BASE + 0x80);break;+#endif
default:return NULL;}spireg = kwspi_slave->spireg;writel(KWSPI_SMEMRDY, &spireg->ctrl); /* calculate spi clock prescaller using max_hz */@@ -63,7 +88,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, kirkwood_mpp_conf(kwspi_mpp_config[cs ? 1 : 0], cs_spi_mpp_back); #endif
return slave;
return &kwspi_slave->slave;}
void spi_free_slave(struct spi_slave *slave) @@ -137,7 +162,12 @@ void spi_release_bus(struct spi_slave *slave) */ int spi_cs_is_valid(unsigned int bus, unsigned int cs) { +#ifdef CONFIG_ARMADA_38X
/* Armada 38x has two SPI controllers */return (bus < 2) && (cs < 3);+#else return bus == 0 && (cs == 0 || cs == 1); +#endif } #endif
@@ -147,11 +177,17 @@ void spi_init(void)
void spi_cs_activate(struct spi_slave *slave) {
struct kwspi_slave *kwspi_slave = to_kwspi(slave);struct kwspi_registers *spireg = kwspi_slave->spireg;setbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);}
void spi_cs_deactivate(struct spi_slave *slave) {
struct kwspi_slave *kwspi_slave = to_kwspi(slave);struct kwspi_registers *spireg = kwspi_slave->spireg;clrbits_le32(&spireg->ctrl, KWSPI_CSN_ACT);}
@@ -160,6 +196,8 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, { unsigned int tmpdout, tmpdin; int tm, isread = 0;
struct kwspi_slave *kwspi_slave = to_kwspi(slave);struct kwspi_registers *spireg = kwspi_slave->spireg; debug("spi_xfer: slave %u:%u dout %p din %p bitlen %u\n", slave->bus, slave->cs, dout, din, bitlen);-- 2.1.3
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Dirk,
On 28.10.2015 17:29, Jagan Teki wrote:
On 28 October 2015 at 21:14, dirk.eibach@gdsys.cc wrote:
From: Dirk Eibach dirk.eibach@gdsys.cc
Armada 38x has two spi controllers.
Signed-off-by: Dirk Eibach dirk.eibach@gdsys.cc
drivers/spi/kirkwood_spi.c | 52 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/kirkwood_spi.c b/drivers/spi/kirkwood_spi.c index e7b0982..200c391 100644 --- a/drivers/spi/kirkwood_spi.c +++ b/drivers/spi/kirkwood_spi.c @@ -18,17 +18,25 @@ #endif #include <asm/arch-mvebu/spi.h>
-static struct kwspi_registers *spireg =
(struct kwspi_registers *)MVEBU_SPI_BASE;- #ifdef CONFIG_KIRKWOOD static u32 cs_spi_mpp_back[2]; #endif
+struct kwspi_slave {
struct spi_slave slave;struct kwspi_registers *spireg;+};
+static inline struct kwspi_slave *to_kwspi(struct spi_slave *slave) +{
return container_of(slave, struct kwspi_slave, slave);+}
- struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, unsigned int max_hz, unsigned int mode) {
struct spi_slave *slave;
struct kwspi_slave *kwspi_slave;struct kwspi_registers *spireg; u32 data;@@ -40,10 +48,27 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, if (!spi_cs_is_valid(bus, cs)) return NULL;
slave = spi_alloc_slave_base(bus, cs);if (!slave)
kwspi_slave = spi_alloc_slave(struct kwspi_slave, bus, cs);if (!kwspi_slave) return NULL;switch (bus) {case 0:kwspi_slave->spireg = (struct kwspi_registers *)MVEBU_SPI_BASE;break;+#ifdef CONFIG_ARMADA_38X
/* Armada 38x has two SPI controllers */Can you please add this through driver-model, I understand it's bit big task but I can help you at any moment.
Yes, please do. I know this is additional work. But we really need to get there at some time. And please note that you can use the runtime SoC detection for this:
if (mvebu_soc_family() == MVEBU_SOC_A38X)
So no new #idefs are needed in such places.
Thanks, Stefan
Hi Stefan,
2015-10-28 17:39 GMT+01:00 Stefan Roese sr@denx.de:
... And please note that you can use the runtime SoC detection for this:
if (mvebu_soc_family() == MVEBU_SOC_A38X)So no new #idefs are needed in such places.
Just give me a quick update please. Why is runtime detection better? Is it about code coverage? What about binary footprint?
Cheers Dirk
Hi Dirk,
On 29.10.2015 10:54, Dirk Eibach wrote:
2015-10-28 17:39 GMT+01:00 Stefan Roese sr@denx.de:
... And please note that you can use the runtime SoC detection for this:
if (mvebu_soc_family() == MVEBU_SOC_A38X)So no new #idefs are needed in such places.
Just give me a quick update please. Why is runtime detection better? Is it about code coverage? What about binary footprint?
We try hard not to add more #idef's to the U-Boot source code if possible. This is definitely one of the reasons. Another is, that this runtime detection will enable support for multiple SoC's in one binary image. This is currently not the case, but we should try to work this way if its not too hard. And these places for the SoC detection are pretty easy to achieve.
The image size will of course be affected. But this drawback is outweighed by the pros noted above. At least from my point of view.
Thanks, Stefan
Hi Stefan,
2015-10-29 11:02 GMT+01:00 Stefan Roese sr@denx.de:
Hi Dirk,
On 29.10.2015 10:54, Dirk Eibach wrote:
2015-10-28 17:39 GMT+01:00 Stefan Roese sr@denx.de:
... And please note that you can use the runtime SoC detection for this:
if (mvebu_soc_family() == MVEBU_SOC_A38X)So no new #idefs are needed in such places.
Just give me a quick update please. Why is runtime detection better? Is it about code coverage? What about binary footprint?
We try hard not to add more #idef's to the U-Boot source code if possible. This is definitely one of the reasons. Another is, that this runtime detection will enable support for multiple SoC's in one binary image. This is currently not the case, but we should try to work this way if its not too hard. And these places for the SoC detection are pretty easy to achieve.
The image size will of course be affected. But this drawback is outweighed by the pros noted above. At least from my point of view.
Ok, that's the information I needed. I will adapt this to runtime detection.
Cheers Dirk
participants (6)
-
Anton Schubert -
Dirk Eibach -
dirk.eibach＠gdsys.cc
-
Jagan Teki -
Luka Perkov -
Stefan Roese