On Mon, Apr 11, 2016 at 02:08:56AM +0200, Marek Vasut wrote:
+#define GXBB_GPIO_0_EN GXBB_PERIPHS_ADDR(0x0c) +#define GXBB_GPIO_0_OUT GXBB_PERIPHS_ADDR(0x0d) +#define GXBB_GPIO_0_IN GXBB_PERIPHS_ADDR(0x0e)
You can also define this as GXBB_GPIO_EN(n) (0xc + 3 * (n) + 0) GXBB_GPIO_OUT(n) (0xc + 3 * (n) + 1) GXBB_GPIO_IN(n) (0xc + 3 * (n) + 2)
This would work well if GPIO_6 didn't exist :)
+#define GXBB_GPIO_1_EN GXBB_PERIPHS_ADDR(0x0f) +#define GXBB_GPIO_1_OUT GXBB_PERIPHS_ADDR(0x10) +#define GXBB_GPIO_1_IN GXBB_PERIPHS_ADDR(0x11) +#define GXBB_GPIO_2_EN GXBB_PERIPHS_ADDR(0x12) +#define GXBB_GPIO_2_OUT GXBB_PERIPHS_ADDR(0x13) +#define GXBB_GPIO_2_IN GXBB_PERIPHS_ADDR(0x14) +#define GXBB_GPIO_3_EN GXBB_PERIPHS_ADDR(0x15) +#define GXBB_GPIO_3_OUT GXBB_PERIPHS_ADDR(0x16) +#define GXBB_GPIO_3_IN GXBB_PERIPHS_ADDR(0x17) +#define GXBB_GPIO_4_EN GXBB_PERIPHS_ADDR(0x18) +#define GXBB_GPIO_4_OUT GXBB_PERIPHS_ADDR(0x19) +#define GXBB_GPIO_4_IN GXBB_PERIPHS_ADDR(0x1a) +#define GXBB_GPIO_5_EN GXBB_PERIPHS_ADDR(0x1b) +#define GXBB_GPIO_5_OUT GXBB_PERIPHS_ADDR(0x1c) +#define GXBB_GPIO_5_IN GXBB_PERIPHS_ADDR(0x1d) +#define GXBB_GPIO_6_EN GXBB_PERIPHS_ADDR(0x08) +#define GXBB_GPIO_6_OUT GXBB_PERIPHS_ADDR(0x09) +#define GXBB_GPIO_6_IN GXBB_PERIPHS_ADDR(0x0a)
It'd be nice to have base addresses somewhere at the beginning instead of having them mixed with the bit macros, but that's a matter of taste.
I agree, I'll change this.
- val = fdt_getprop(gd->fdt_blob, offset, "reg", &len);
- if (len < sizeof(*val) * 4)
return -EINVAL;- /* Don't use fdt64_t to avoid unaligned access */
This looks iffy, can you elaborate on this issue ?
I was getting a "Synchronous Abort handler, esr 0x96000021" which seemed to indicate a alignment fault, but thinking again about it I'm not sure anymore of the real cause. fdt64_t and fdt64_to_cpu() don't work here, I will try to investigate better why. Suggestions are welcome :)
- /* Reserve first 16 MiB of RAM */
Why ?
I'll add a comment, first 16 MiB seems to be reserved for firmware.
+void reset_cpu(ulong addr) +{
How does the system reboot then ?
The system reboots through a call to secure monitor, which is not implemented in this submission.
+struct mm_region *mem_map = gxbb_mem_map;
This looks super-iffy, I wouldn't be surprised if this started being optimized away at some point.
I don't understand, why that should happen?
- /* Reset PHY on GPIOZ_14 */
- clrbits_le32(GXBB_GPIO_3_EN, BIT(14));
- clrbits_le32(GXBB_GPIO_3_OUT, BIT(14));
- udelay(100000);
mdelay(100); , though that is quite some wait.
Will change and decrease the timeout.
diff --git a/drivers/serial/serial_meson.c b/drivers/serial/serial_meson.c
This should go in a separate patch.
I originally submitted the driver as separate patch, then Tom suggested that the initial platform patch should contain everything needed to perform a boot, so UART, DDR, board file and DTS.
Thanks for the review,
Beniamino