Re: [PATCH] ARM: Add Advantech imx6 board support

[Christoph Fritz]

Hi Andrey,

 thanks for your input, I'll integrate your suggestions and send a v2
patch.

Thanks
  -- Christoph


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] ARM: Add Advantech imx6 board support

[Christoph Fritz]

Hi Sascha,

On Fri, 2018-04-06 at 21:21 +0200, Sascha Hauer wrote:
> Hi Christoph,
> 
> On Thu, Apr 05, 2018 at 03:15:49PM +0200, Christoph Fritz wrote:
> > +   phy_register_fixup_for_uid(0x004dd072, 0xffef, ar8035_phy_fixup);
> > +
> > +   switch (bootsource_get()) {
> > +   case BOOTSOURCE_MMC:
> > +   environment_path = basprintf("/chosen/environment-sd%d",
> > +bootsource_get_instance() + 1);
> > +   envdev = "MMC";
> > +   break;
> > +   case BOOTSOURCE_SPI:
> > +   default:
> > +   environment_path = basprintf("/chosen/environment-sd4");
> > +   envdev = "MMC";
> > +   break;
> > +   }
> > +
> > +   if (environment_path) {
> > +   ret = of_device_enable_path(environment_path);
> > +   if (ret < 0)
> > +   pr_warn("Failed to enable env partition '%s' (%d)\n",
> > +   environment_path, ret);
> > +   free(environment_path);
> > +   }
> > +
> > +   pr_notice("Using environment in %s\n", envdev);
> 
> This is always "MMC", not very informative. Maybe "eMMC" and "external
> SD"?

I'll fix that.

> 
> > +
> > +   imx6_bbu_internal_mmc_register_handler("mmc3", "/dev/mmc3",
> > +   BBU_HANDLER_FLAG_DEFAULT);
> 
> That would be the eMMC, right? For this I can recommend putting barebox
> into the boot partitions of the eMMC. Normally there are two of them
> and with imx6_bbu_internal_mmcboot_register_handler() you even get a
> robust barebox A/B update mechanism.

I'm currently using here a hybrid u-boot into barebox boot and can't use
the eMMC SLC boot-partitions without changes on the u-boot side. That's
why I kept this config.

If you really want me to go for the boot0 partition, I can adapt this in
this mainline barebox setup?

> > +
> > +   return 0;
> > +}
> > +device_initcall(advantech_mx6_devices_init);
> > diff --git 
> > a/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg 
> > b/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
> > new file mode 100644
> > index 000..611e06b
> > --- /dev/null
> > +++ b/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
> > @@ -0,0 +1,73 @@
> > +soc imx6
> > +loadaddr 0x1000
> > +dcdofs 0x400
> > +
> > +wm 32 0x020e0774 0x000C
> 
> [...]
> 
> > +wm 32 0x021b0004 0x0002556D
> > +wm 32 0x021b0404 0x00011006
> > +wm 32 0x021b001c 0x
> 
> > +wm 32 0x020c4068 0x00C03F3F
> > +wm 32 0x020c406c 0x0030FC03
> > +wm 32 0x020c4070 0x0FFFC000
> > +wm 32 0x020c4074 0x3FF0
> > +wm 32 0x020c4078 0x00FFF300
> > +wm 32 0x020c407c 0x0FC3
> > +wm 32 0x020c4080 0x03FF
> 
> Please remove writing these clock registers here. The registers get
> overwritten in a few moments by barebox anyway. Often enough these gate
> settings disable the USB clocks and then booting this image from USB is
> no longer possible.

I'll fix that.

> 
> > +wm 32 0x020e0010 0xF0CF
> > +wm 32 0x020e0018 0x007F007F
> > +wm 32 0x020e001c 0x007F007F
> > diff --git a/arch/arm/boards/advantech-mx6/lowlevel.c 
> > b/arch/arm/boards/advantech-mx6/lowlevel.c
> > new file mode 100644
> > index 000..5efb91a
> > --- /dev/null
> > +++ b/arch/arm/boards/advantech-mx6/lowlevel.c
> > @@ -0,0 +1,58 @@
> > +/*
> > + * Copyright (C) 2018 Christoph Fritz 
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static inline void setup_uart(void)
> > +{
> > +   void __iomem *iomuxbase = (void *)MX6_IOMUXC_BASE_ADDR;
> > +
> > +   writel(0x3, iomuxbase + 0x4c);
> > +
> > +   imx6_ungate_all_peripherals();
> > +   imx6_uart_setup_ll();
> > +
> > +   putc_ll('>');
> > +}
> > +
> > +extern char __dtb_imx6dl_advantech_rom_7421_start[];
> > +
> > +BAREBOX_IMD_TAG_STRING(advantech_imx6dl_memsize_512M, IMD_TYPE_PARAMETER,
> > +  "memsize=512", 0);
> > +
> > +ENTRY_FUNCTION(start_advantech_imx6dl_rom_7421, r0, r1, r2)
> > +{
> > +   void *fdt;
> > +
> > +   imx6_cpu_lowlevel_init();
> > +
> > +   arm_setup_stack(0x0092 - 8);
> > +
> > +   IMD_USED(advantech_imx6dl_memsize_512M);
> > +
> > +   if (IS_ENABLED(CONFIG_DEBUG_LL))
> > +   setup_uart();
> > +
> > +   fdt = __dtb_imx6dl_advantech_rom_7421_start - get_runtime_offset();
> 
> Since "a43e2bbc46 ARM: return 

Re: [PATCH] ARM: Add Advantech imx6 board support

[Andrey Smirnov]

On Thu, Apr 5, 2018 at 6:15 AM, Christoph Fritz
 wrote:
> Add support for Advantech i.MX6 SOM named ROM-7421.
>

I don't have any serious comments about this, just a bunch of nits below.

> Signed-off-by: Christoph Fritz 
> ---
>  arch/arm/boards/Makefile   |   1 +
>  arch/arm/boards/advantech-mx6/Makefile |   2 +
>  arch/arm/boards/advantech-mx6/board.c  |  93 +
>  .../flash-header-advantech-rom-7421.imxcfg |  73 +++
>  arch/arm/boards/advantech-mx6/lowlevel.c   |  58 ++
>  arch/arm/configs/imx_v7_defconfig  |   1 +
>  arch/arm/dts/Makefile  |   1 +
>  arch/arm/dts/imx6dl-advantech-rom-7421.dts | 227 
> +
>  arch/arm/mach-imx/Kconfig  |   6 +
>  images/Makefile.imx|   5 +
>  10 files changed, 467 insertions(+)
>  create mode 100644 arch/arm/boards/advantech-mx6/Makefile
>  create mode 100644 arch/arm/boards/advantech-mx6/board.c
>  create mode 100644 
> arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
>  create mode 100644 arch/arm/boards/advantech-mx6/lowlevel.c
>  create mode 100755 arch/arm/dts/imx6dl-advantech-rom-7421.dts
>
> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
> index ca187cc..521f37d 100644
> --- a/arch/arm/boards/Makefile
> +++ b/arch/arm/boards/Makefile
> @@ -1,4 +1,5 @@
>  # keep sorted by CONFIG_* macro name.
> +obj-$(CONFIG_MACH_ADVANTECH_ROM_742X)  += advantech-mx6/
>  obj-$(CONFIG_MACH_AFI_GF)  += afi-gf/
>  obj-$(CONFIG_MACH_ANIMEO_IP)   += animeo_ip/
>  obj-$(CONFIG_MACH_ARCHOSG9)+= archosg9/
> diff --git a/arch/arm/boards/advantech-mx6/Makefile 
> b/arch/arm/boards/advantech-mx6/Makefile
> new file mode 100644
> index 000..01c7a25
> --- /dev/null
> +++ b/arch/arm/boards/advantech-mx6/Makefile
> @@ -0,0 +1,2 @@
> +obj-y += board.o
> +lwl-y += lowlevel.o
> diff --git a/arch/arm/boards/advantech-mx6/board.c 
> b/arch/arm/boards/advantech-mx6/board.c
> new file mode 100644
> index 000..8686a5e
> --- /dev/null
> +++ b/arch/arm/boards/advantech-mx6/board.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright (C) 2018 Christoph Fritz 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int ar8035_phy_fixup(struct phy_device *dev)
> +{
> +   u16 val;
> +
> +   /* Ar803x phy SmartEEE feature cause link status generates glitch,
> +* which cause ethernet link down/up issue, so disable SmartEEE
> +*/
> +   phy_write(dev, 0xd, 0x3);
> +   phy_write(dev, 0xe, 0x805d);
> +   phy_write(dev, 0xd, 0x4003);
> +
> +   val = phy_read(dev, 0xe);
> +   phy_write(dev, 0xe, val & ~(1 << 8));
> +

BIT(8)?

> +   /* To enable AR8031 ouput a 125MHz clk from CLK_25M */
> +   phy_write(dev, 0xd, 0x7);
> +   phy_write(dev, 0xe, 0x8016);
> +   phy_write(dev, 0xd, 0x4007);
> +
> +   val = phy_read(dev, 0xe);
> +   val &= 0xffe3;
> +   val |= 0x18;
> +   phy_write(dev, 0xe, val);
> +
> +   /* introduce tx clock delay */
> +   phy_write(dev, 0x1d, 0x5);
> +   val = phy_read(dev, 0x1e);
> +   val |= 0x0100;
> +   phy_write(dev, 0x1e, val);
> +
> +   return 0;
> +}
> +
> +static int advantech_mx6_devices_init(void)
> +{
> +   int ret;
> +   char *environment_path, *envdev;
> +
> +   if (!of_machine_is_compatible("advantech,imx6dl-rom-7421"))
> +   return 0;
> +
> +   phy_register_fixup_for_uid(0x004dd072, 0xffef, ar8035_phy_fixup);
> +
> +   switch (bootsource_get()) {
> +   case BOOTSOURCE_MMC:
> +   environment_path = basprintf("/chosen/environment-sd%d",
> +bootsource_get_instance() + 1);
> +   envdev = "MMC";
> +   break;
> +   case BOOTSOURCE_SPI:
> +   default:
> +   environment_path = basprintf("/chosen/environment-sd4");
> +   envdev = "MMC";
> +   break;
> +   }
> +
> +   if (environment_path) {
> +   ret = of_device_enable_path(environment_path);
> +   if (ret < 0)
> +   pr_warn("Failed to enable env partition '%s' (%d)\n",
> +

Re: [PATCH] ARM: Add Advantech imx6 board support

[Sascha Hauer]

Hi Christoph,

On Thu, Apr 05, 2018 at 03:15:49PM +0200, Christoph Fritz wrote:
> + phy_register_fixup_for_uid(0x004dd072, 0xffef, ar8035_phy_fixup);
> +
> + switch (bootsource_get()) {
> + case BOOTSOURCE_MMC:
> + environment_path = basprintf("/chosen/environment-sd%d",
> +  bootsource_get_instance() + 1);
> + envdev = "MMC";
> + break;
> + case BOOTSOURCE_SPI:
> + default:
> + environment_path = basprintf("/chosen/environment-sd4");
> + envdev = "MMC";
> + break;
> + }
> +
> + if (environment_path) {
> + ret = of_device_enable_path(environment_path);
> + if (ret < 0)
> + pr_warn("Failed to enable env partition '%s' (%d)\n",
> + environment_path, ret);
> + free(environment_path);
> + }
> +
> + pr_notice("Using environment in %s\n", envdev);

This is always "MMC", not very informative. Maybe "eMMC" and "external
SD"?

> +
> + imx6_bbu_internal_mmc_register_handler("mmc3", "/dev/mmc3",
> + BBU_HANDLER_FLAG_DEFAULT);

That would be the eMMC, right? For this I can recommend putting barebox
into the boot partitions of the eMMC. Normally there are two of them
and with imx6_bbu_internal_mmcboot_register_handler() you even get a
robust barebox A/B update mechanism.

> +
> + return 0;
> +}
> +device_initcall(advantech_mx6_devices_init);
> diff --git 
> a/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg 
> b/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
> new file mode 100644
> index 000..611e06b
> --- /dev/null
> +++ b/arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
> @@ -0,0 +1,73 @@
> +soc imx6
> +loadaddr 0x1000
> +dcdofs 0x400
> +
> +wm 32 0x020e0774 0x000C

[...]

> +wm 32 0x021b0004 0x0002556D
> +wm 32 0x021b0404 0x00011006
> +wm 32 0x021b001c 0x

> +wm 32 0x020c4068 0x00C03F3F
> +wm 32 0x020c406c 0x0030FC03
> +wm 32 0x020c4070 0x0FFFC000
> +wm 32 0x020c4074 0x3FF0
> +wm 32 0x020c4078 0x00FFF300
> +wm 32 0x020c407c 0x0FC3
> +wm 32 0x020c4080 0x03FF

Please remove writing these clock registers here. The registers get
overwritten in a few moments by barebox anyway. Often enough these gate
settings disable the USB clocks and then booting this image from USB is
no longer possible.

> +wm 32 0x020e0010 0xF0CF
> +wm 32 0x020e0018 0x007F007F
> +wm 32 0x020e001c 0x007F007F
> diff --git a/arch/arm/boards/advantech-mx6/lowlevel.c 
> b/arch/arm/boards/advantech-mx6/lowlevel.c
> new file mode 100644
> index 000..5efb91a
> --- /dev/null
> +++ b/arch/arm/boards/advantech-mx6/lowlevel.c
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (C) 2018 Christoph Fritz 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static inline void setup_uart(void)
> +{
> + void __iomem *iomuxbase = (void *)MX6_IOMUXC_BASE_ADDR;
> +
> + writel(0x3, iomuxbase + 0x4c);
> +
> + imx6_ungate_all_peripherals();
> + imx6_uart_setup_ll();
> +
> + putc_ll('>');
> +}
> +
> +extern char __dtb_imx6dl_advantech_rom_7421_start[];
> +
> +BAREBOX_IMD_TAG_STRING(advantech_imx6dl_memsize_512M, IMD_TYPE_PARAMETER,
> +"memsize=512", 0);
> +
> +ENTRY_FUNCTION(start_advantech_imx6dl_rom_7421, r0, r1, r2)
> +{
> + void *fdt;
> +
> + imx6_cpu_lowlevel_init();
> +
> + arm_setup_stack(0x0092 - 8);
> +
> + IMD_USED(advantech_imx6dl_memsize_512M);
> +
> + if (IS_ENABLED(CONFIG_DEBUG_LL))
> + setup_uart();
> +
> + fdt = __dtb_imx6dl_advantech_rom_7421_start - get_runtime_offset();

Since "a43e2bbc46 ARM: return positive offset in get_runtime_offset()"
this must be '+' get_runtime_offset();

> +#include 
> +#include 
> +#include "imx6dl.dtsi"
> +
> +/ {
> + model = "Advantech i.MX6 ROM-7421";
> + compatible = "advantech,imx6dl-rom-7421", "fsl,imx6dl";
> +
> + chosen {
> + linux,stdout-path = 

just stdout-path. linux,stdout-path is the old binding.

> +
> + environment-sd2 {   /* Micro SD */
> + compatible = "barebox,environment";
> + device-path = , "partname:barebox-environment";
> + 

[PATCH] ARM: Add Advantech imx6 board support

[Christoph Fritz]

Add support for Advantech i.MX6 SOM named ROM-7421.

Signed-off-by: Christoph Fritz 
---
 arch/arm/boards/Makefile   |   1 +
 arch/arm/boards/advantech-mx6/Makefile |   2 +
 arch/arm/boards/advantech-mx6/board.c  |  93 +
 .../flash-header-advantech-rom-7421.imxcfg |  73 +++
 arch/arm/boards/advantech-mx6/lowlevel.c   |  58 ++
 arch/arm/configs/imx_v7_defconfig  |   1 +
 arch/arm/dts/Makefile  |   1 +
 arch/arm/dts/imx6dl-advantech-rom-7421.dts | 227 +
 arch/arm/mach-imx/Kconfig  |   6 +
 images/Makefile.imx|   5 +
 10 files changed, 467 insertions(+)
 create mode 100644 arch/arm/boards/advantech-mx6/Makefile
 create mode 100644 arch/arm/boards/advantech-mx6/board.c
 create mode 100644 
arch/arm/boards/advantech-mx6/flash-header-advantech-rom-7421.imxcfg
 create mode 100644 arch/arm/boards/advantech-mx6/lowlevel.c
 create mode 100755 arch/arm/dts/imx6dl-advantech-rom-7421.dts

diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
index ca187cc..521f37d 100644
--- a/arch/arm/boards/Makefile
+++ b/arch/arm/boards/Makefile
@@ -1,4 +1,5 @@
 # keep sorted by CONFIG_* macro name.
+obj-$(CONFIG_MACH_ADVANTECH_ROM_742X)  += advantech-mx6/
 obj-$(CONFIG_MACH_AFI_GF)  += afi-gf/
 obj-$(CONFIG_MACH_ANIMEO_IP)   += animeo_ip/
 obj-$(CONFIG_MACH_ARCHOSG9)+= archosg9/
diff --git a/arch/arm/boards/advantech-mx6/Makefile 
b/arch/arm/boards/advantech-mx6/Makefile
new file mode 100644
index 000..01c7a25
--- /dev/null
+++ b/arch/arm/boards/advantech-mx6/Makefile
@@ -0,0 +1,2 @@
+obj-y += board.o
+lwl-y += lowlevel.o
diff --git a/arch/arm/boards/advantech-mx6/board.c 
b/arch/arm/boards/advantech-mx6/board.c
new file mode 100644
index 000..8686a5e
--- /dev/null
+++ b/arch/arm/boards/advantech-mx6/board.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2018 Christoph Fritz 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int ar8035_phy_fixup(struct phy_device *dev)
+{
+   u16 val;
+
+   /* Ar803x phy SmartEEE feature cause link status generates glitch,
+* which cause ethernet link down/up issue, so disable SmartEEE
+*/
+   phy_write(dev, 0xd, 0x3);
+   phy_write(dev, 0xe, 0x805d);
+   phy_write(dev, 0xd, 0x4003);
+
+   val = phy_read(dev, 0xe);
+   phy_write(dev, 0xe, val & ~(1 << 8));
+
+   /* To enable AR8031 ouput a 125MHz clk from CLK_25M */
+   phy_write(dev, 0xd, 0x7);
+   phy_write(dev, 0xe, 0x8016);
+   phy_write(dev, 0xd, 0x4007);
+
+   val = phy_read(dev, 0xe);
+   val &= 0xffe3;
+   val |= 0x18;
+   phy_write(dev, 0xe, val);
+
+   /* introduce tx clock delay */
+   phy_write(dev, 0x1d, 0x5);
+   val = phy_read(dev, 0x1e);
+   val |= 0x0100;
+   phy_write(dev, 0x1e, val);
+
+   return 0;
+}
+
+static int advantech_mx6_devices_init(void)
+{
+   int ret;
+   char *environment_path, *envdev;
+
+   if (!of_machine_is_compatible("advantech,imx6dl-rom-7421"))
+   return 0;
+
+   phy_register_fixup_for_uid(0x004dd072, 0xffef, ar8035_phy_fixup);
+
+   switch (bootsource_get()) {
+   case BOOTSOURCE_MMC:
+   environment_path = basprintf("/chosen/environment-sd%d",
+bootsource_get_instance() + 1);
+   envdev = "MMC";
+   break;
+   case BOOTSOURCE_SPI:
+   default:
+   environment_path = basprintf("/chosen/environment-sd4");
+   envdev = "MMC";
+   break;
+   }
+
+   if (environment_path) {
+   ret = of_device_enable_path(environment_path);
+   if (ret < 0)
+   pr_warn("Failed to enable env partition '%s' (%d)\n",
+   environment_path, ret);
+   free(environment_path);
+   }
+
+   pr_notice("Using environment in %s\n", envdev);
+
+   imx6_bbu_internal_mmc_register_handler("mmc3", "/dev/mmc3",
+   BBU_HANDLER_FLAG_DEFAULT);
+
+   return 0;
+}
+device_initcall(advantech_mx6_devices_init);
diff --git