Amlogic Meson GXBaby and ODROID-C2 support

All members of the DMA descriptor must be 32-bit, even on 64-bit
architectures: change the type to u32 to ensure this. Also, fix other warnings.

Signed-off-by Beniamino Galvani <[email protected]>

---
 drivers/net/designware.c | 59 ++++++++++++++++++++++++++----------------------
 drivers/net/designware.h |  4 ++--
 2 files changed, 34 insertions(+), 29 deletions(-)

drivers/net/designware.c +0 -0

index ca58f34..2eda461 100644

drivers/net/designware.h +2 -2

index ed6344c..d48df7b 100644
@@ -110,8 +110,8 @@ struct eth_dma_regs {
 struct dmamacdescr {
 	u32 txrx_status;
 	u32 dmamac_cntl;
-	void *dmamac_addr;
-	struct dmamacdescr *dmamac_next;
+	u32 dmamac_addr;
+	u32 dmamac_next;
 } __aligned(ARCH_DMA_MINALIGN);
 
 /*

drivers/net/designware.c static void tx_descs_init(struct dw_eth_dev *priv) · line 102

Quoted 4 lines

drivers/net/designware.c +2 -2

-		desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
-		desc_p->dmamac_next = &desc_table_p[idx + 1];
+		desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
+		desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];

Why don't you use u32 instead of ulong ? The u32 is well defined. DTTO all over the place.

btw just curious, but what will happen if the descriptors get allocated in area above 4GiB ? Will the code silently corrupt memory by discarding the top bits in the descriptor pointer?

Quoted 7 lines · Marek Vasut

drivers/net/designware.c static void tx_descs_init(struct dw_eth_dev *priv) · line 102

Quoted 4 lines

drivers/net/designware.c +2 -2

-		desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
-		desc_p->dmamac_next = &desc_table_p[idx + 1];
+		desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
+		desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];

Why don't you use u32 instead of ulong ? The u32 is well defined. DTTO all over the place.

&txbuffs[idx * CONFIG_ETH_BUFSIZE] is a pointer (and hence has the size of a ulong) and casting it to u32 would give a warning on 64 bit archs ("cast from pointer to integer of different size").

Quoted 3 lines

btw just curious, but what will happen if the descriptors get allocated in area above 4GiB ? Will the code silently corrupt memory by discarding the top bits in the descriptor pointer?

No, if the driver private structure (which contains buffers and descriptors) is above 4GiB, designware_initialize() will complain and return an error.

Beniamino

Quoted 9 lines · Beniamino Galvani

Quoted 7 lines · Marek Vasut

drivers/net/designware.c static void tx_descs_init(struct dw_eth_dev *priv) · line 102

Quoted 4 lines

drivers/net/designware.c +2 -2

-		desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
-		desc_p->dmamac_next = &desc_table_p[idx + 1];
+		desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
+		desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];

Why don't you use u32 instead of ulong ? The u32 is well defined. DTTO all over the place.

Quoted 3 lines

&txbuffs[idx * CONFIG_ETH_BUFSIZE] is a pointer (and hence has the size of a ulong) and casting it to u32 would give a warning on 64 bit archs ("cast from pointer to integer of different size").

Will cast to uintptr_t and then to u32 help ?

It's just a feeling, but casting to ulong just to circumvent compiler warning does not sound right.

Quoted 7 lines

Quoted 3 lines

btw just curious, but what will happen if the descriptors get allocated in area above 4GiB ? Will the code silently corrupt memory by discarding the top bits in the descriptor pointer?

No, if the driver private structure (which contains buffers and descriptors) is above 4GiB, designware_initialize() will complain and return an error.

Which code checks that ?

Quoted 8 lines · Marek Vasut

Quoted 7 lines · Beniamino Galvani

Quoted 6 lines · Marek Vasut

drivers/net/designware.c static void tx_descs_init(struct dw_eth_dev *priv) · line 102

Quoted 4 lines

drivers/net/designware.c +2 -2

-		desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
-		desc_p->dmamac_next = &desc_table_p[idx + 1];
+		desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
+		desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];

Why don't you use u32 instead of ulong ? The u32 is well defined. DTTO all over the place.

Quoted 4 lines

Quoted 3 lines

&txbuffs[idx * CONFIG_ETH_BUFSIZE] is a pointer (and hence has the size of a ulong) and casting it to u32 would give a warning on 64 bit archs ("cast from pointer to integer of different size").

Will cast to uintptr_t and then to u32 help ?

Note that uintptr_t is defined as ulong and the second cast to u32 is not needed because C does not require casts between arithmetic types. So I don't see much difference.

Quoted 2 lines

It's just a feeling, but casting to ulong just to circumvent compiler warning does not sound right.

It seems fine to me, the (ulong) is needed to cast the pointer to an arithmetic type of equivalent size which then can be assigned to an u32 variable.

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

btw just curious, but what will happen if the descriptors get allocated in area above 4GiB ? Will the code silently corrupt memory by discarding the top bits in the descriptor pointer?

No, if the driver private structure (which contains buffers and descriptors) is above 4GiB, designware_initialize() will complain and return an error.

Quoted 1 line

Which code checks that ?

+ if ((unsigned long long)priv + sizeof(*priv) > (1ULL << 32)) {
+ printf("designware: buffers are outside DMA memory\n");
+ return -EINVAL;
+ }
+

Hmm, this is going to get very interesting with efi_loader support. By default we allocate memory at the highest possible free address, so payloads will probably (unless they specify limits) have their buffers above 32bit on this platform. If we now deny any DMA to them, we basically break I/O access.

Could you by any chance just use a bounce buffer?

Quoted 5 lines · Alexander Graf

Hmm, this is going to get very interesting with efi_loader support. By default we allocate memory at the highest possible free address, so payloads will probably (unless they specify limits) have their buffers above 32bit on this platform. If we now deny any DMA to them, we basically break I/O access.

I'm not familiar with efi_loader, but on this platform the physical RAM is within the 32bit memory range, so I don't think a workaround is needed. And I guess probably it's the same for the other 64bit ARM SoC using this driver.

BTW, I see that another driver (sunxi_mmc) also truncates the upper 32 bits of addresses on 64bit platforms. Maybe this issue should be addresses in a generic way?

Quoted 1 line

Could you by any chance just use a bounce buffer?

Do you have any suggestions on how to do it? Are there any primitives in u-boot to request memory from low addresses?

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Beniamino Galvani

Quoted 5 lines · Marek Vasut

Quoted 4 lines

drivers/net/designware.c +2 -2

-         desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
-         desc_p->dmamac_next = &desc_table_p[idx + 1];
+         desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
+         desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Why don't you use u32 instead of ulong ? The u32 is well defined. DTTO all over the place.

&txbuffs[idx * CONFIG_ETH_BUFSIZE] is a pointer (and hence has the size of a ulong) and casting it to u32 would give a warning on 64 bit archs ("cast from pointer to integer of different size").

Will cast to uintptr_t and then to u32 help ?

Note that uintptr_t is defined as ulong and the second cast to u32 is not needed because C does not require casts between arithmetic types. So I don't see much difference.

Quoted 2 lines

It's just a feeling, but casting to ulong just to circumvent compiler warning does not sound right.

It seems fine to me, the (ulong) is needed to cast the pointer to an arithmetic type of equivalent size which then can be assigned to an u32 variable.

Quoted 9 lines

Quoted 7 lines

Quoted 3 lines

btw just curious, but what will happen if the descriptors get allocated in area above 4GiB ? Will the code silently corrupt memory by discarding the top bits in the descriptor pointer?

No, if the driver private structure (which contains buffers and descriptors) is above 4GiB, designware_initialize() will complain and return an error.

Which code checks that ?

+ if ((unsigned long long)priv + sizeof(*priv) > (1ULL << 32)) {
+ printf("designware: buffers are outside DMA memory\n");
+ return -EINVAL;
+ }
+

Beniamino
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

All members of the DMA descriptor must be 32-bit, even on 64-bit
architectures: change the type to u32 to ensure this. Also, fix other warnings.

Signed-off-by Beniamino Galvani <[email protected]>

---
 drivers/net/designware.c | 59 ++++++++++++++++++++++++++----------------------
 drivers/net/designware.h |  4 ++--
 2 files changed, 34 insertions(+), 29 deletions(-)

drivers/net/designware.c +0 -0

index ca58f34..2eda461 100644

drivers/net/designware.c +1 -0

@@ -488,6 +486,11 @@ int designware_initialize(ulong base_addr, u32 interface)
 		return -ENOMEM;
 	}
 
+	if ((unsigned long long)priv + sizeof(*priv) > (1ULL << 32)) {
 

=?

drivers/net/designware.c +4 -0

+		printf("designware: buffers are outside DMA memory\n");
+		return -EINVAL;
+	}
+
 	memset(dev, 0, sizeof(struct eth_device));
 	memset(priv, 0, sizeof(struct dw_eth_dev));

Quoted 3 lines

drivers/net/designware.c +1 -0

+	if ((unsigned long long)priv + sizeof(*priv) > (1ULL << 32)) {

=?

Am 17.04.2016 um 09:48 schrieb Beniamino Galvani <[email protected]>:

Implement calls to the secure monitor to reset the board and read the MAC address from e-fuse.

Signed-off-by Beniamino Galvani <[email protected]>

---

arch/arm/include/asm/arch-meson/sm.h   | 13 +++++++
arch/arm/mach-meson/Makefile           |  2 +-
arch/arm/mach-meson/board.c            |  3 +-
arch/arm/mach-meson/sm.c               | 71 ++++++++++++++++++++++++++++++++++
board/hardkernel/odroid-c2/odroid-c2.c | 16 ++++++++
5 files changed, 103 insertions(+), 2 deletions(-)

create mode 100644 arch/arm/include/asm/arch-meson/sm.h create mode 100644 arch/arm/mach-meson/sm.c

arch/arm/include/asm/arch-meson/sm.h new file +13 -0

new file mode 100644
index 0000000..87134c6
@@ -0,0 +1,13 @@
+/*
+ * (C) Copyright 2016 - Beniamino Galvani <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef __MESON_SM_H__
+#define __MESON_SM_H__
+
+void meson_sm_reboot(void);
+ssize_t meson_sm_read_efuse(uintptr_t offset, void *buffer, size_t size);
+
+#endif /* __MESON_SM_H__ */

arch/arm/mach-meson/Makefile +1 -1

index 44e3d63..bf49b8b 100644
@@ -4,4 +4,4 @@
 # SPDX-License-Identifier:    GPL-2.0+
 #
 
-obj-y += board.o
+obj-y += board.o sm.o

arch/arm/mach-meson/board.c +1 -0

index 05f4c79..6272f2f 100644
@@ -8,6 +8,7 @@
 #include <libfdt.h>
 #include <linux/err.h>
 #include <asm/arch/gxbb.h>
+#include <asm/arch/sm.h>
 #include <asm/armv8/mmu.h>
 #include <asm/unaligned.h>
 

Shouldn't reboot be available as psci call? We could then have a generic reset function for systems with working psci in atf.

Implement calls to the secure monitor to reset the board and read the MAC address from e-fuse.

Signed-off-by Beniamino Galvani <[email protected]>

arch/arm/mach-meson/sm.c new file +2 -0

new file mode 100644
index 0000000..3065bd2
@@ -0,0 +1,71 @@
+/*
+ * (C) Copyright 2016 Beniamino Galvani <[email protected]>

arch/arm/mach-meson/sm.c new file +8 -0

+#define FN_GET_SHARE_MEM_INPUT_BASE	0x82000020
+#define FN_GET_SHARE_MEM_OUTPUT_BASE	0x82000021
+#define FN_EFUSE_READ			0x82000030
+#define FN_EFUSE_WRITE			0x82000031
+#define FN_REBOOT			0x84000009
+
+static void *shmem_input;
+static void *shmem_output;

Quoted 1 line

+#define MIN(a, b) ((a) < (b) ? (a) : (b))

See include/linux/kernel.h for min macro