Skip to contentSkip to conversation

ARM: highbank: fix Calxeda machines

PATCH New Andre Przywara 6 participants 25 messages Jun 4, 2015 – Jun 25, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover ARM: highbank: fix Calxeda machines

This series aims to upstream fixes that have been in the Calxeda firmware version of U-Boot for ages.
The first two patches fix reset, both for Highbank and Midway. Patches 3-5 add support for larger hard disks, so U-Boot can now access sectors from beyond 2 TB on larger disks.
Patch 6 and 7 teach U-Boot not to touch the FDT memory node. The primary Calxeda firmware detects the amount of memory (it has a DIMM socket!) at boot time and populates the memory node accordingly. U-Boot would overwrite the node with a fixed value, most commonly getting lost of any memory beyond 4 GB on Midway systems. I have been told that this fix has been disliked before, so I am open to a discussion on how to address this problem.
The final patch fixes Highbank's SATA setup, which requires some special care (cf. the sata_highbank.c driver in Linux).

There are actually more patches [1] in the last official firmware, but most of the others only make sense with some special management firmware versions that never made it to the light.
I extracted the most useful and important patches from there and rebased them on top of the current master. With them I can happily use upstream U-Boot on a Midway system - I haven't had time to test it on Highbank, though.

Please review, comment and hopefully apply them!

Cheers,
Andre.

[1] https://git.linaro.org/people/rob.herring/u-boot.git/shortlog/refs/heads/highbank

Mark Langsdorf (5):
ARM: highbank: add reset support for Calxeda Midway machine
cmd_scsi: use lbaint_t for LBA values instead of u32
ahci: support LBA48 data reads for 2+TB drives
ahci: extend data io wait to 10s
highbank: add custom ahci_link_up function

Rob Herring (3):
ARM: highbank: add missing SCU register setup for reset
ARM: bootm: allow skipping fdt memory node fixup
ARM: highbank: remove DRAM bank setup

 arch/arm/lib/board.c       |   2 +
 arch/arm/lib/bootm-fdt.c   |   5 +-
 board/highbank/Makefile    |   2 +-
 board/highbank/ahci.c      | 218 +++++++++++++++++++++++++++++++++++++++++++++
 board/highbank/highbank.c  |  37 ++++++--
 common/cmd_scsi.c          |  58 ++++++++++--
 drivers/block/ahci.c       |  32 +++++--
 include/configs/highbank.h |   2 +-
 include/scsi.h             |   1 +
 9 files changed, 331 insertions(+), 26 deletions(-)
 create mode 100644 board/highbank/ahci.c
Signature · 1 line
1.8.4
#1 lore raw

PATCH 1/8 ARM: highbank: add missing SCU register setup for reset

From: Rob Herring <[email protected]>

Andre: assign names to the magic values

---
 board/highbank/highbank.c | 6 ++++++
 1 file changed, 6 insertions(+)
board/highbank/highbank.c +6 -0
index ba1beb5..ba254b6 100644
@@ -14,6 +14,7 @@
 
 #define HB_AHCI_BASE			0xffe08000
 
+#define HB_SCU_A9_PWR_STATUS		0xfff10008
 #define HB_SREG_A9_PWR_REQ		0xfff3cf00
 #define HB_SREG_A9_BOOT_SRC_STAT	0xfff3cf04
 #define HB_SREG_A9_PWRDOM_STAT		0xfff3cf20
@@ -27,6 +28,10 @@
 #define PWRDOM_STAT_PCI			0x40000000
 #define PWRDOM_STAT_EMMC		0x20000000
 
+#define HB_SCU_A9_PWR_NORMAL		0
+#define HB_SCU_A9_PWR_DORMANT		2
+#define HB_SCU_A9_PWR_OFF		3
+
 DECLARE_GLOBAL_DATA_PTR;
 
 /*
@@ -114,6 +119,7 @@ int ft_board_setup(void *fdt, bd_t *bd)
 void reset_cpu(ulong addr)
 {
 	writel(HB_PWR_HARD_RESET, HB_SREG_A9_PWR_REQ);
+	writeb(HB_SCU_A9_PWR_OFF, HB_SCU_A9_PWR_STATUS);
 
 	wfi();
 }
Signature · 1 line
1.8.4
#2 lore raw

PATCH 1/8 ARM: highbank: add missing SCU register setup for reset

Quoted 6 lines · Andre Przywara

From: Rob Herring <[email protected]>

Andre: assign names to the magic values

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/cbaa3262/attachment.sig>
#3 lore raw

PATCH 2/8 ARM: highbank: add reset support for Calxeda Midway machine

In reply to Andre Przywara

From: Mark Langsdorf <[email protected]>

The Calxeda Midway part has A15 cores, which do not have the Highbank A9's SCU used there for resetting the chip.
Add code to distinguish between the A9 and the A15 and invoke the appropriate register writes to support the newer part.

Andre: rework detection of Highbank vs. Midway
Rob: fix Andre's reworked detection

---
 board/highbank/highbank.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
board/highbank/highbank.c +14 -1
index ba254b6..e8132b8 100644
@@ -18,6 +18,7 @@
 #define HB_SREG_A9_PWR_REQ		0xfff3cf00
 #define HB_SREG_A9_BOOT_SRC_STAT	0xfff3cf04
 #define HB_SREG_A9_PWRDOM_STAT		0xfff3cf20
+#define HB_SREG_A15_PWR_CTRL		0xfff3c200
 
 #define HB_PWR_SUSPEND			0
 #define HB_PWR_SOFT_RESET		1
@@ -116,10 +117,22 @@ int ft_board_setup(void *fdt, bd_t *bd)
 }
 #endif
 
+static int is_highbank(void)
+{
+	uint32_t midr;
+
+	asm volatile ("mrc p15, 0, %0, c0, c0, 0\n" : "=r"(midr));
+
+	return (midr & 0xfff0) == 0xc090;
+}
+
 void reset_cpu(ulong addr)
 {
 	writel(HB_PWR_HARD_RESET, HB_SREG_A9_PWR_REQ);
-	writeb(HB_SCU_A9_PWR_OFF, HB_SCU_A9_PWR_STATUS);
+	if (is_highbank())
+		writeb(HB_SCU_A9_PWR_OFF, HB_SCU_A9_PWR_STATUS);
+	else
+		writel(0x1, HB_SREG_A15_PWR_CTRL);
 
 	wfi();
 }
Signature · 1 line
1.8.4
#4 lore raw

PATCH 2/8 ARM: highbank: add reset support for Calxeda Midway machine

Quoted 7 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

The Calxeda Midway part has A15 cores, which do not have the Highbank A9's SCU used there for resetting the chip.
Add code to distinguish between the A9 and the A15 and invoke the appropriate register writes to support the newer part.

Quoted 6 lines

Andre: rework detection of Highbank vs. Midway
Rob: fix Andre's reworked detection

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/b57799da/attachment.sig>
#5 lore raw

PATCH 3/8 cmd_scsi: use lbaint_t for LBA values instead of u32

In reply to Andre Przywara

From: Mark Langsdorf <[email protected]>

---
 common/cmd_scsi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
common/cmd_scsi.c +7 -7
index f80f549..fe705b6 100644
@@ -54,10 +54,10 @@ static block_dev_desc_t scsi_dev_desc[CONFIG_SYS_SCSI_MAX_DEVICE];
  *  forward declerations of some Setup Routines
  */
 void scsi_setup_test_unit_ready(ccb * pccb);
-void scsi_setup_read6(ccb * pccb, unsigned long start, unsigned short blocks);
-void scsi_setup_read_ext(ccb * pccb, unsigned long start, unsigned short blocks);
-static void scsi_setup_write_ext(ccb *pccb, unsigned long start,
-			  unsigned short blocks);
+void scsi_setup_read6(ccb * pccb, lbaint_t start, unsigned short blocks);
+void scsi_setup_read_ext(ccb * pccb, lbaint_t start, unsigned short blocks);
+static void scsi_setup_write_ext(ccb *pccb, lbaint_t start,
+				unsigned short blocks);
 void scsi_setup_inquiry(ccb * pccb);
 void scsi_ident_cpy (unsigned char *dest, unsigned char *src, unsigned int len);
 
@@ -579,7 +579,7 @@ void scsi_setup_test_unit_ready(ccb * pccb)
 	pccb->msgout[0]=SCSI_IDENTIFY; /* NOT USED */
 }
 
-void scsi_setup_read_ext(ccb * pccb, unsigned long start, unsigned short blocks)
+void scsi_setup_read_ext(ccb * pccb, lbaint_t start, unsigned short blocks)
 {
 	pccb->cmd[0]=SCSI_READ10;
 	pccb->cmd[1]=pccb->lun<<5;
@@ -599,7 +599,7 @@ void scsi_setup_read_ext(ccb * pccb, unsigned long start, unsigned short blocks)
 		pccb->cmd[7],pccb->cmd[8]);
 }
 
-void scsi_setup_write_ext(ccb *pccb, unsigned long start, unsigned short blocks)
+void scsi_setup_write_ext(ccb *pccb, lbaint_t start, unsigned short blocks)
 {
 	pccb->cmd[0] = SCSI_WRITE10;
 	pccb->cmd[1] = pccb->lun << 5;
@@ -620,7 +620,7 @@ void scsi_setup_write_ext(ccb *pccb, unsigned long start, unsigned short blocks)
 	      pccb->cmd[7], pccb->cmd[8]);
 }
 
-void scsi_setup_read6(ccb * pccb, unsigned long start, unsigned short blocks)
+void scsi_setup_read6(ccb * pccb, lbaint_t start, unsigned short blocks)
 {
 	pccb->cmd[0]=SCSI_READ6;
 	pccb->cmd[1]=pccb->lun<<5 | (((unsigned char)(start>>16))&0x1f);
Signature · 1 line
1.8.4
#6 lore raw

PATCH 3/8 cmd_scsi: use lbaint_t for LBA values instead of u32

Quoted 4 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/4e82b67e/attachment.sig>
#7 lore raw

PATCH 4/8 ahci: support LBA48 data reads for 2+TB drives

In reply to Andre Przywara

From: Mark Langsdorf <[email protected]>

Enable full 48-bit LBA48 data reads by passing the upper word of the LBA block pointer in bytes 9 and 10 of the FIS.

This allows uboot to load data from any arbitrary sector on a drive with 2 or more TB of available data connected to an AHCI controller.

---
 common/cmd_scsi.c    | 44 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/block/ahci.c | 30 +++++++++++++++++++++++-------
 include/scsi.h       |  1 +
 3 files changed, 66 insertions(+), 9 deletions(-)
common/cmd_scsi.c +42 -2
index fe705b6..57d6a3f 100644
@@ -56,6 +56,8 @@ static block_dev_desc_t scsi_dev_desc[CONFIG_SYS_SCSI_MAX_DEVICE];
 void scsi_setup_test_unit_ready(ccb * pccb);
 void scsi_setup_read6(ccb * pccb, lbaint_t start, unsigned short blocks);
 void scsi_setup_read_ext(ccb * pccb, lbaint_t start, unsigned short blocks);
+void scsi_setup_read16(ccb * pccb, lbaint_t start, unsigned long blocks);
+
 static void scsi_setup_write_ext(ccb *pccb, lbaint_t start,
 				unsigned short blocks);
 void scsi_setup_inquiry(ccb * pccb);
@@ -357,7 +359,9 @@ int do_scsi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * scsi_read
  */
 
-#define SCSI_MAX_READ_BLK 0xFFFF /* almost the maximum amount of the scsi_ext command.. */
+/* almost the maximum amount of the scsi_ext command.. */
+#define SCSI_MAX_READ_BLK 0xFFFF
+#define SCSI_LBA48_READ	0xFFFFFFF
 
 static ulong scsi_read(int device, lbaint_t blknr, lbaint_t blkcnt,
 		       void *buffer)
@@ -379,7 +383,14 @@ static ulong scsi_read(int device, lbaint_t blknr, lbaint_t blkcnt,
 	      device, start, blks, (unsigned long)buffer);
 	do {
 		pccb->pdata=(unsigned char *)buf_addr;
-		if(blks>SCSI_MAX_READ_BLK) {
+		if (start > SCSI_LBA48_READ) {
+			unsigned long blocks;
+			blocks = min_t(lbaint_t, blks, SCSI_MAX_READ_BLK);
+			pccb->datalen = scsi_dev_desc[device].blksz * blocks;
+			scsi_setup_read16(pccb, start, blocks);
+			start += blocks;
+			blks -= blocks;
+		} else if (blks > SCSI_MAX_READ_BLK) {
 			pccb->datalen=scsi_dev_desc[device].blksz * SCSI_MAX_READ_BLK;
 			smallblks=SCSI_MAX_READ_BLK;
 			scsi_setup_read_ext(pccb,start,smallblks);
@@ -579,6 +590,35 @@ void scsi_setup_test_unit_ready(ccb * pccb)
 	pccb->msgout[0]=SCSI_IDENTIFY; /* NOT USED */
 }
 
+void scsi_setup_read16(ccb * pccb, lbaint_t start, unsigned long blocks)
+{
+	pccb->cmd[0] = SCSI_READ16;
+	pccb->cmd[1] = pccb->lun<<5;
+	pccb->cmd[2] = ((unsigned char) (start >> 56)) & 0xff;
+	pccb->cmd[3] = ((unsigned char) (start >> 48)) & 0xff;
+	pccb->cmd[4] = ((unsigned char) (start >> 40)) & 0xff;
+	pccb->cmd[5] = ((unsigned char) (start >> 32)) & 0xff;
+	pccb->cmd[6] = ((unsigned char) (start >> 24)) & 0xff;
+	pccb->cmd[7] = ((unsigned char) (start >> 16)) & 0xff;
+	pccb->cmd[8] = ((unsigned char) (start >> 8)) & 0xff;
+	pccb->cmd[9] = ((unsigned char) (start)) & 0xff;
+	pccb->cmd[10] = 0;
+	pccb->cmd[11] = ((unsigned char) (blocks >> 24)) & 0xff;
+	pccb->cmd[12] = ((unsigned char) (blocks >> 16)) & 0xff;
+	pccb->cmd[13] = ((unsigned char) (blocks >> 8)) & 0xff;
+	pccb->cmd[14] = (unsigned char) blocks & 0xff;
+	pccb->cmd[15] = 0;
+	pccb->cmdlen = 16;
+	pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
+	debug ("scsi_setup_read16: cmd: %02X %02X "
+	       "startblk %02X%02X%02X%02X%02X%02X%02X%02X "
+	       "blccnt %02X%02X%02X%02X\n",
+		pccb->cmd[0], pccb->cmd[1],
+		pccb->cmd[2], pccb->cmd[3], pccb->cmd[4], pccb->cmd[5],
+		pccb->cmd[6], pccb->cmd[7], pccb->cmd[8], pccb->cmd[9],
+		pccb->cmd[11], pccb->cmd[12], pccb->cmd[13], pccb->cmd[14]);
+}
+
 void scsi_setup_read_ext(ccb * pccb, lbaint_t start, unsigned short blocks)
 {
 	pccb->cmd[0]=SCSI_READ10;
drivers/block/ahci.c +23 -7
index 6508648..28d0b08 100644
@@ -726,18 +726,25 @@ static int ata_scsiop_inquiry(ccb *pccb)
  */
 static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 {
-	u32 lba = 0;
+	lbaint_t lba = 0;
 	u16 blocks = 0;
 	u8 fis[20];
 	u8 *user_buffer = pccb->pdata;
 	u32 user_buffer_size = pccb->datalen;
 
 	/* Retrieve the base LBA number from the ccb structure. */
-	memcpy(&lba, pccb->cmd + 2, sizeof(lba));
-	lba = be32_to_cpu(lba);
+	if (pccb->cmd[0] == SCSI_READ16) {
+		memcpy(&lba, pccb->cmd + 2, 8);
+		lba = be64_to_cpu(lba);
+	} else {
+		u32 temp;
+		memcpy(&temp, pccb->cmd + 2, 4);
+		lba = be32_to_cpu(temp);
+	}
 
 	/*
-	 * And the number of blocks.
+	 * Retrieve the base LBA number and the block count from
+	 * the ccb structure.
 	 *
 	 * For 10-byte and 16-byte SCSI R/W commands, transfer
 	 * length 0 means transfer 0 block of data.
@@ -746,10 +753,13 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 	 *
 	 * WARNING: one or two older ATA drives treat 0 as 0...
 	 */
-	blocks = (((u16)pccb->cmd[7]) << 8) | ((u16) pccb->cmd[8]);
+	if (pccb->cmd[0] == SCSI_READ16)
+		blocks = (((u16)pccb->cmd[13]) << 8) | ((u16) pccb->cmd[14]);
+	else
+		blocks = (((u16)pccb->cmd[7]) << 8) | ((u16) pccb->cmd[8]);
 
-	debug("scsi_ahci: %s %d blocks starting from lba 0x%x\n",
-	      is_write ?  "write" : "read", (unsigned)lba, blocks);
+	debug("scsi_ahci: %s %u blocks starting from lba 0x" LBAFU "\n",
+	      is_write ?  "write" : "read", blocks, lba);
 
 	/* Preset the FIS */
 	memset(fis, 0, sizeof(fis));
@@ -778,6 +788,11 @@ static int ata_scsiop_read_write(ccb *pccb, u8 is_write)
 		fis[6] = (lba >> 16) & 0xff;
 		fis[7] = 1 << 6; /* device reg: set LBA mode */
 		fis[8] = ((lba >> 24) & 0xff);
+		if (pccb->cmd[0] == SCSI_READ16) {
+			fis[9] = ((lba >> 32) & 0xff);
+			fis[10] = ((lba >> 40) & 0xff);
+		}
+
 		fis[3] = 0xe0; /* features */
 
 		/* Block (sector) count */
@@ -883,6 +898,7 @@ int scsi_exec(ccb *pccb)
 	int ret;
 
 	switch (pccb->cmd[0]) {
+	case SCSI_READ16:
 	case SCSI_READ10:
 		ret = ata_scsiop_read_write(pccb, 0);
 		break;
include/scsi.h +1 -0
index 73de7b7..7e37591 100644
@@ -132,6 +132,7 @@ typedef struct SCSI_cmd_block{
 #define SCSI_MED_REMOVL	0x1E		/* Prevent/Allow medium Removal (O) */
 #define SCSI_READ6		0x08		/* Read 6-byte (MANDATORY) */
 #define SCSI_READ10		0x28		/* Read 10-byte (MANDATORY) */
+#define SCSI_READ16	0x48
 #define SCSI_RD_CAPAC	0x25		/* Read Capacity (MANDATORY) */
 #define SCSI_RD_CAPAC10	SCSI_RD_CAPAC	/* Read Capacity (10) */
 #define SCSI_RD_CAPAC16	0x9e		/* Read Capacity (16) */
Signature · 1 line
1.8.4
#8 lore raw

PATCH 4/8 ahci: support LBA48 data reads for 2+TB drives

Quoted 10 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

Enable full 48-bit LBA48 data reads by passing the upper word of the LBA block pointer in bytes 9 and 10 of the FIS.

This allows uboot to load data from any arbitrary sector on a drive with 2 or more TB of available data connected to an AHCI controller.

With some changes to make use of CONFIG_SYS_64BIT_LBA in a few areas to allow the common parts to compile without warning on systems that don't set this:

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/7994466e/attachment.sig>
#9 lore raw

PATCH 4/8 ahci: support LBA48 data reads for 2+TB drives

Hi Andre,

Quoted 10 lines · Tom Rini
Quoted 8 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

Enable full 48-bit LBA48 data reads by passing the upper word of the LBA block pointer in bytes 9 and 10 of the FIS.

This allows uboot to load data from any arbitrary sector on a drive with 2 or more TB of available data connected to an AHCI controller.

Quoted 9 lines
Quoted 2 lines

With some changes to make use of CONFIG_SYS_64BIT_LBA in a few areas to allow the common parts to compile without warning on systems that don't set this:

Applied to u-boot/master, thanks!

This commit creates a warning when CONFIG_SYS_64BIT_LBA is defined. Please check below.

common/cmd_scsi.c: In function ?scsi_read?:
common/cmd_scsi.c:410:3: warning: ?smallblks? may be used uninitialized in this function [-Wmaybe-uninitialized]

Regards,
Bin

#10 lore raw

PATCH 5/8 ahci: extend data io wait to 10s

In reply to Andre Przywara

From: Mark Langsdorf <[email protected]>

The AHCI driver currently waits 5s before timing out when sending a data command to a drive. Some drives take upwards of 8s to respond to the initial data command while they're spinning up. Increase the data io timeout to 10s so that those drives can be found on initial scsi scan.

---
 drivers/block/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/ahci.c +1 -1
index 28d0b08..eb22586 100644
@@ -39,7 +39,7 @@ u16 *ataid[AHCI_MAX_PORTS];
 
 /* Maximum timeouts for each event */
 #define WAIT_MS_SPINUP	20000
-#define WAIT_MS_DATAIO	5000
+#define WAIT_MS_DATAIO	10000
 #define WAIT_MS_FLUSH	5000
 #define WAIT_MS_LINKUP	200
 
Signature · 1 line
1.8.4
#11 lore raw

PATCH 5/8 ahci: extend data io wait to 10s

Quoted 10 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

The AHCI driver currently waits 5s before timing out when sending a data command to a drive. Some drives take upwards of 8s to respond to the initial data command while they're spinning up. Increase the data io timeout to 10s so that those drives can be found on initial scsi scan.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/c5ce5606/attachment.sig>
#12 lore raw

PATCH 6/8 ARM: bootm: allow skipping fdt memory node fixup

In reply to Andre Przywara

From: Rob Herring <[email protected]>

Currently, u-boot will always fixup the DT memory node on ARM. If the dtb has correct memory information, then we don't want or need u-boot to touch the memory node. Allow platforms to skip this by not filling in dram bank information.

---
 arch/arm/lib/board.c     | 2 ++
 arch/arm/lib/bootm-fdt.c | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)
arch/arm/lib/board.c +2 -0
index 37ea6e9..a3c7315 100644
@@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void);
 
 __weak void dram_init_banksize(void)
 {
+#if CONFIG_NR_DRAM_BANKS
 	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
 	gd->bd->bi_dram[0].size =  gd->ram_size;
+#endif
 }
 
 __weak int arch_cpu_init(void)
arch/arm/lib/bootm-fdt.c +4 -1
index 7677358..dbf3d56 100644
@@ -26,8 +26,10 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int arch_fixup_fdt(void *blob)
 {
+	int ret = 0;
+#if CONFIG_NR_DRAM_BANKS
 	bd_t *bd = gd->bd;
-	int bank, ret;
+	int bank;
 	u64 start[CONFIG_NR_DRAM_BANKS];
 	u64 size[CONFIG_NR_DRAM_BANKS];
 
@@ -42,6 +44,7 @@ int arch_fixup_fdt(void *blob)
 	}
 
 	ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
+#endif
 #ifdef CONFIG_ARMV7_NONSEC
 	if (ret)
 		return ret;
Signature · 1 line
1.8.4
#13 lore raw

PATCH 6/8 ARM: bootm: allow skipping fdt memory node fixup

Hi Andre,

arch/arm/lib/board.c

Quoted 18 lines · Andre Przywara

From: Rob Herring <[email protected]>

Currently, u-boot will always fixup the DT memory node on ARM. If the dtb has correct memory information, then we don't want or need u-boot to touch the memory node. Allow platforms to skip this by not filling in dram bank information.

---
 arch/arm/lib/board.c     | 2 ++
 arch/arm/lib/bootm-fdt.c | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)
arch/arm/lib/board.c +0 -0
index 37ea6e9..a3c7315 100644

I don't think we should be patching this file now - everything should be on generic board.

arch/arm/lib/board.c typedef int (init_fnc_t) (void); · line 200

Quoted 11 lines
arch/arm/lib/board.c +2 -0
@@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void);
 
 __weak void dram_init_banksize(void)
 {
+#if CONFIG_NR_DRAM_BANKS
        gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
        gd->bd->bi_dram[0].size =  gd->ram_size;
+#endif
 }
 
 __weak int arch_cpu_init(void)

[snip]

Regards,
Simon

#14 lore raw

PATCH 6/8 ARM: bootm: allow skipping fdt memory node fixup

On Fri, 5 Jun 2015 08:12:25 -0600
Simon Glass <[email protected]> wrote:

Hi Simon,

thanks for taking a look.

Quoted 15 lines · Andre Przywara
Quoted 14 lines

From: Rob Herring <[email protected]>

Currently, u-boot will always fixup the DT memory node on ARM. If the dtb has correct memory information, then we don't want or need u-boot to touch the memory node. Allow platforms to skip this by not filling in dram bank information.

---
 arch/arm/lib/board.c     | 2 ++
 arch/arm/lib/bootm-fdt.c | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)
Quoted 7 lines

arch/arm/lib/board.c

Quoted 4 lines
arch/arm/lib/board.c +0 -0
index 37ea6e9..a3c7315 100644

I don't think we should be patching this file now - everything should be on generic board.

Right, looking closer at this patch much of it is actually not necessary. Having CONFIG_NR_DRAM_BANKS set to 0 automatically skips a lot of the code already, so we don't need any extra gating. Hope that there are no compilers which complain about zero-sized arrays. The only thing that harms us is the final fdt_setprop(), which will _clear_ the reg property in the memory node in this case.
So I replaced this patch by just skipping the reg property setting if the number of banks is 0 in common/fdt_support.c.
From looking at all callers of fdt_fixup_memory_banks() this looks like a safe approach. Any objections?
I will send the replaced patch with the next revision.

Cheers,
Andre.

Quoted 17 lines

arch/arm/lib/board.c typedef int (init_fnc_t) (void); · line 200

Quoted 12 lines
arch/arm/lib/board.c +2 -0
@@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void);
 
 __weak void dram_init_banksize(void)
 {
+#if  
        gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
        gd->bd->bi_dram[0].size =  gd->ram_size;
+#endif
 }
 
 __weak int arch_cpu_init(void)

[snip]

Regards,
Simon

#15 lore raw

PATCH 6/8 ARM: bootm: allow skipping fdt memory node fixup

Hi Andre,

Quoted 30 lines · Andre Przywara

On Fri, 5 Jun 2015 08:12:25 -0600
Simon Glass <[email protected]> wrote:

Hi Simon,

thanks for taking a look.

Quoted 15 lines · Andre Przywara
Quoted 14 lines

From: Rob Herring <[email protected]>

Currently, u-boot will always fixup the DT memory node on ARM. If the dtb has correct memory information, then we don't want or need u-boot to touch the memory node. Allow platforms to skip this by not filling in dram bank information.

---
 arch/arm/lib/board.c     | 2 ++
 arch/arm/lib/bootm-fdt.c | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)
Quoted 7 lines

arch/arm/lib/board.c

Quoted 4 lines
arch/arm/lib/board.c +0 -0
index 37ea6e9..a3c7315 100644

I don't think we should be patching this file now - everything should be on generic board.

Quoted 11 lines

Right, looking closer at this patch much of it is actually not necessary. Having CONFIG_NR_DRAM_BANKS set to 0 automatically skips a lot of the code already, so we don't need any extra gating. Hope that there are no compilers which complain about zero-sized arrays. The only thing that harms us is the final fdt_setprop(), which will _clear_ the reg property in the memory node in this case.
So I replaced this patch by just skipping the reg property setting if the number of banks is 0 in common/fdt_support.c.
From looking at all callers of fdt_fixup_memory_banks() this looks like a safe approach. Any objections?
I will send the replaced patch with the next revision.

Should be OK - can you please add docs for that function to the header file? See fdt_fixup_display() for an example.

Quoted 15 lines

Cheers,
Andre.

Quoted 11 lines
Quoted 11 lines
arch/arm/lib/board.c +2 -0
@@ -191,8 +191,10 @@ typedef int (init_fnc_t) (void);
 
 __weak void dram_init_banksize(void)
 {
+#if
        gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
        gd->bd->bi_dram[0].size =  gd->ram_size;
+#endif
 }
 
Quoted 1 line
Quoted 1 line

arch/arm/lib/board.c typedef int (init_fnc_t) (void); · line 200

Quoted 1 line
arch/arm/lib/board.c +0 -0
 __weak int arch_cpu_init(void)

Regards,
Simon

#16 lore raw

PATCH 1/1 fdt: prevent clearing memory node if there are no banks

Avoid clearing the reg property in the memory DT node if no memory banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0). This allows boards to let U-Boot skip the DT memory tinkering in case other firmware has already setup the node properly before. This should be safe as all callers of fdt_fixup_memory_banks that use a computed <banks> value put at least 1 in there.
Add some documentation comments to the header file.

---
Hi,

this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup" patch, as Simon had some (justified) concerns. If this is too generic now, let me know and I will rework it.

Cheers,
Andre.

 common/fdt_support.c  | 14 ++++++++------
 include/fdt_support.h | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)
common/fdt_support.c +8 -6
index 9e50148..6ad532c 100644
@@ -429,13 +429,15 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
 		return err;
 	}
 
-	len = fdt_pack_reg(blob, tmp, start, size, banks);
+	if (banks) {
+		len = fdt_pack_reg(blob, tmp, start, size, banks);
 
-	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
-	if (err < 0) {
-		printf("WARNING: could not set %s %s.\n",
-				"reg", fdt_strerror(err));
-		return err;
+		err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
+		if (err < 0) {
+			printf("WARNING: could not set %s %s.\n",
+					"reg", fdt_strerror(err));
+			return err;
+		}
 	}
 	return 0;
 }
include/fdt_support.h +26 -0
index 5d4f28d..0296ed3 100644
@@ -40,8 +40,34 @@ void do_fixup_by_compat(void *fdt, const char *compat,
 			const char *prop, const void *val, int len, int create);
 void do_fixup_by_compat_u32(void *fdt, const char *compat,
 			    const char *prop, u32 val, int create);
+/**
+ * Setup the memory node in the DT. Creates one if none was existing before.
+ * Calls fdt_fixup_memory_banks() to populate a single reg pair covering the
+ * whole memory.
+ *
+ * @param blob		FDT blob to update
+ * @param start		Begin of DRAM mapping in physical memory
+ * @param size		Size of the single memory bank
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
 int fdt_fixup_memory(void *blob, u64 start, u64 size);
+
+/**
+ * Fill the DT memory node with multiple memory banks.
+ * Creates the node if none was existing before.
+ * If banks is 0, it will not touch the existing reg property. This allows
+ * boards to not mess with the existing DT setup, which may have been
+ * filled in properly before.
+ *
+ * @param blob		FDT blob to update
+ * @param start		Array of size <banks> to hold the start addresses.
+ * @param size		Array of size <banks> to hold the size of each region.
+ * @param banks		Number of memory banks to create. If 0, the reg
+ *			property will be left untouched.
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
 int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks);
+
 void fdt_fixup_ethernet(void *fdt);
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 			 const void *val, int len, int create);
Signature · 1 line
1.8.4
#17 lore raw

PATCH 1/1 fdt: prevent clearing memory node if there are no banks

Quoted 28 lines · Andre Przywara

Avoid clearing the reg property in the memory DT node if no memory banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0). This allows boards to let U-Boot skip the DT memory tinkering in case other firmware has already setup the node properly before. This should be safe as all callers of fdt_fixup_memory_banks that use a computed <banks> value put at least 1 in there.
Add some documentation comments to the header file.

---
Hi,

this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup" patch, as Simon had some (justified) concerns. If this is too generic now, let me know and I will rework it.

Cheers,
Andre.

 common/fdt_support.c  | 14 ++++++++------
 include/fdt_support.h | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)
common/fdt_support.c +0 -0
index 9e50148..6ad532c 100644

common/fdt_support.c int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks) · line 433

Quoted 6 lines
common/fdt_support.c +2 -1
@@ -429,13 +429,15 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
                return err;
        }
 
-       len = fdt_pack_reg(blob, tmp, start, size, banks);
+       if (banks) {
+               len = fdt_pack_reg(blob, tmp, start, size, banks);

Save some indentation:

if (!banks)
return 0;

Quoted 15 lines
common/fdt_support.c +6 -5
-       err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
-       if (err < 0) {
-               printf("WARNING: could not set %s %s.\n",
-                               "reg", fdt_strerror(err));
-               return err;
+               err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
+               if (err < 0) {
+                       printf("WARNING: could not set %s %s.\n",
+                                       "reg", fdt_strerror(err));
+                       return err;
+               }
        }
        return 0;
 }
#18 lore raw

PATCH 1/1 fdt: prevent clearing memory node if there are no banks

In reply to Andre Przywara

Hi,

Quoted 17 lines · Andre Przywara

Avoid clearing the reg property in the memory DT node if no memory banks have been specified for a board (CONFIG_NR_DRAM_BANKS == 0). This allows boards to let U-Boot skip the DT memory tinkering in case other firmware has already setup the node properly before. This should be safe as all callers of fdt_fixup_memory_banks that use a computed <banks> value put at least 1 in there.
Add some documentation comments to the header file.

---
Hi,

this replaces my former "6/8: ARM: bootm: allow skipping fdt memory node fixup" patch, as Simon had some (justified) concerns. If this is too generic now, let me know and I will rework it.

Quoted 6 lines

Cheers,
Andre.

 common/fdt_support.c  | 14 ++++++++------
 include/fdt_support.h | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

Tom, you can pick this up if you like?

- Simon

#19 lore raw

PATCH 7/8 ARM: highbank: remove DRAM bank setup

In reply to Andre Przywara

From: Rob Herring <[email protected]>

On the highbank platform the SoC's management controller firmware will probe the DRAM modules and populates the initial device tree with the correct values. Therefore the memory sizes in are already correct, so remove U-Boot's DRAM bank setup so the memory node is not "fixed up" by u-boot.

---
 board/highbank/highbank.c  | 6 ------
 include/configs/highbank.h | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)
board/highbank/highbank.c +0 -6
index e8132b8..9ebd895 100644
@@ -93,12 +93,6 @@ int dram_init(void)
 	return 0;
 }
 
-void dram_init_banksize(void)
-{
-	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
-	gd->bd->bi_dram[0].size =  PHYS_SDRAM_1_SIZE;
-}
-
 #if defined(CONFIG_OF_BOARD_SETUP)
 int ft_board_setup(void *fdt, bd_t *bd)
 {
include/configs/highbank.h +1 -1
index da1c837..b1d618b 100644
@@ -83,7 +83,7 @@
 /*-----------------------------------------------------------------------
  * Physical Memory Map
  */
-#define CONFIG_NR_DRAM_BANKS		1
+#define CONFIG_NR_DRAM_BANKS		0
 #define PHYS_SDRAM_1_SIZE		(4089 << 20)
 #define CONFIG_SYS_MEMTEST_START	0x100000
 #define CONFIG_SYS_MEMTEST_END		(PHYS_SDRAM_1_SIZE - 0x100000)
Signature · 1 line
1.8.4
#20 lore raw

PATCH 7/8 ARM: highbank: remove DRAM bank setup

Quoted 10 lines · Andre Przywara

From: Rob Herring <[email protected]>

On the highbank platform the SoC's management controller firmware will probe the DRAM modules and populates the initial device tree with the correct values. Therefore the memory sizes in are already correct, so remove U-Boot's DRAM bank setup so the memory node is not "fixed up" by u-boot.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/e8e485fd/attachment.sig>
#21 lore raw

PATCH 7/8 ARM: highbank: remove DRAM bank setup

Quoted 10 lines · On Fri, 12 Jun 2015 22:10:07 -0400 Tom Rini <[email protected]> wrote:
Quoted 8 lines · Andre Przywara

From: Rob Herring <[email protected]>

On the highbank platform the SoC's management controller firmware will probe the DRAM modules and populates the initial device tree with the correct values. Therefore the memory sizes in are already correct, so remove U-Boot's DRAM bank setup so the memory node is not "fixed up" by u-boot.

Quoted 4 lines
Quoted 2 lines

Applied to u-boot/master, thanks!

Hi Tom,

thanks for applying the patches!
I sent out a reworked version of patch 6/8, which saw a comment from Simon. Please consider applying the new one _or_ revert this very patch here (7/8), as it alone breaks Calxeda machines (which now end up with an empty memory/reg property instead of an untouched one). Sorry for the mess!

Cheers,
Andre.

#22 lore raw

PATCH 7/8 ARM: highbank: remove DRAM bank setup

Quoted 18 lines · Andre Przywara
Quoted 10 lines · On Fri, 12 Jun 2015 22:10:07 -0400 Tom Rini <[email protected]> wrote:
Quoted 8 lines · Andre Przywara

From: Rob Herring <[email protected]>

On the highbank platform the SoC's management controller firmware will probe the DRAM modules and populates the initial device tree with the correct values. Therefore the memory sizes in are already correct, so remove U-Boot's DRAM bank setup so the memory node is not "fixed up" by u-boot.

Quoted 4 lines
Quoted 2 lines

Applied to u-boot/master, thanks!

Quoted 8 lines

Hi Tom,

thanks for applying the patches!
I sent out a reworked version of patch 6/8, which saw a comment from Simon. Please consider applying the new one _or_ revert this very patch here (7/8), as it alone breaks Calxeda machines (which now end up with an empty memory/reg property instead of an untouched one). Sorry for the mess!

"Good" news! I somehow failed to push things when I said I did, so this was all still just local. I've dropped this patch for now as I want Simon to chime in on your updated patch.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150615/f637401c/attachment.sig>
#23 lore raw

PATCH 8/8 highbank: add custom ahci_link_up function

In reply to Andre Przywara

From: Mark Langsdorf <[email protected]>

The Calxeda highbank SOC needs a custom sequence to bring up SATA links, so override ahci_link_up with custom function to handle combophy setup.

Signed-off-by Richard Gibbs
---
 board/highbank/Makefile   |   2 +-
 board/highbank/ahci.c     | 218 ++++++++++++++++++++++++++++++++++++++++++++++
 board/highbank/highbank.c |  12 +++
 3 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 board/highbank/ahci.c
board/highbank/Makefile +1 -1
index d3eb232..ce7ee68 100644
@@ -5,4 +5,4 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-obj-y	:= highbank.o
+obj-y	:= highbank.o ahci.o
board/highbank/ahci.c new file +218 -0

218 lines not on this page · raw message

board/highbank/highbank.c +12 -0
index 9ebd895..55999ed 100644
@@ -35,6 +35,8 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+void cphy_disable_overrides(void);
+
 /*
  * Miscellaneous platform dependent initialisations
  */
@@ -62,6 +64,7 @@ void scsi_init(void)
 {
 	u32 reg = readl(HB_SREG_A9_PWRDOM_STAT);
 
+	cphy_disable_overrides();
 	if (reg & PWRDOM_STAT_SATA) {
 		ahci_init((void __iomem *)HB_AHCI_BASE);
 		scsi_scan(1);
@@ -130,3 +133,12 @@ void reset_cpu(ulong addr)
 
 	wfi();
 }
+
+/*
+ * turn off the override before transferring control to Linux, since Linux
+ * may not support spread spectrum.
+ */
+void arch_preboot_os(void)
+{
+	cphy_disable_overrides();
+}
Signature · 1 line
1.8.4
#24 lore raw

PATCH 8/8 highbank: add custom ahci_link_up function

Quoted 9 lines · Andre Przywara

From: Mark Langsdorf <[email protected]>

The Calxeda highbank SOC needs a custom sequence to bring up SATA links, so override ahci_link_up with custom function to handle combophy setup.

Signed-off-by Richard Gibbs

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150612/39920268/attachment.sig>