[U-Boot] [PATCH v3] arm: imx: add i.MX53 Beckhoff CX9020 Embedded PC

[linux-kernel-dev]

Hi Jagan,
>From: Jagan Teki [mailto:jagan at openedev.com]
>Sent: Donnerstag, 20. Oktober 2016 12:52
>Subject: Re: [U-Boot] [PATCH v3] arm: imx: add i.MX53 Beckhoff CX9020
>Embedded PC
>
>On Mon, Oct 17, 2016 at 5:57 PM,  <linux-kernel-dev at beckhoff.com> wrote:
>> From: Patrick Bruenn <p.bruenn at beckhoff.com>
>>...
>> diff --git a/board/beckhoff/mx53cx9020/imximage.cfg
>b/board/beckhoff/mx53cx9020/imximage.cfg
>> new file mode 100644
>> index 0000000..c6bdc72
>> --- /dev/null
>> +++ b/board/beckhoff/mx53cx9020/imximage.cfg
>> @@ -0,0 +1,82 @@
>> +/*
>> + * Copyright (C) 2015  Beckhoff Automation GmbH
>> + * Patrick Bruenn <p.bruenn at beckhoff.com>
>> + *
>> + * Based on Freescale's Linux i.MX mx53loco/imximage.cfg file:
>
>Is this Linux refenace file? I saw similar one at ./board/freescale/mx53loco.
It's from <u-boot>/board/freescale/mx53loco, I will make that more clear in the comment
>>...
>> diff --git a/board/beckhoff/mx53cx9020/mx53cx9020.c
>b/board/beckhoff/mx53cx9020/mx53cx9020.c
>> new file mode 100644
>> index 0000000..c92ec15
>> --- /dev/null
>> +++ b/board/beckhoff/mx53cx9020/mx53cx9020.c
>> @@ -0,0 +1,563 @@
>> +/*
>> + * Copyright (C) 2015  Beckhoff Automation GmbH & Co. KG
>> + * Patrick Bruenn <p.bruenn at beckhoff.com>
>> + *
>> + * Based on Freescale's Linux i.MX mx53loco.c file:
>
>Same as above
>
yes, same here.
>>...
>> +static void clock_1GHz(void)
>> +{
>> +       int ret;
>> +       u32 ref_clk = MXC_HCLK;
>> +       /*
>> +        * After increasing voltage to 1.25V, we can switch
>> +        * CPU clock to 1GHz and DDR to 400MHz safely
>> +        */
>> +       ret = mxc_set_clock(ref_clk, 1000, MXC_ARM_CLK);
>> +       if (ret)
>> +               printf("CPU:   Switch CPU clock to 1GHZ failed\n");
>> +
>> +       ret = mxc_set_clock(ref_clk, 400, MXC_PERIPH_CLK);
>> +       ret |= mxc_set_clock(ref_clk, 400, MXC_DDR_CLK);
>
>Why we need to | the previous ret?
>
To keep it the same as in mx53loco.c. Besides it sounds reasonable to me the check both functions for success.
>>...
>> +U_BOOT_CMD(loadccat, 5, 1, do_load_ccat,
>> +          "loads ccat firmware",
>> +          "<interface> <dev[:part]> <addr> <filename>\n"
>> +          "    - write FPGA firmware from 'dev' on 'interface' in 'filename' to
>CCAT using 'addr' as a buffer");
>
>Can't we achieve this through cmd/fpga* ?
>
Maybe, but I didn't find a working solution. From your experience should the cyclone2 driver work for cyclon3 as well?
>> ...
>> diff --git a/include/configs/mx53cx9020.h b/include/configs/mx53cx9020.h
>> new file mode 100644
>> index 0000000..b90e076
>> --- /dev/null
>> +++ b/include/configs/mx53cx9020.h
>> @@ -0,0 +1,206 @@
>> +/*
>> + * Copyright (C) 2015  Beckhoff Automation GmbH & Co. KG
>> + * Patrick Bruenn <p.bruenn at beckhoff.com>
>> + *
>> + * Configuration settings for Beckhoff CX9020.
>> + *
>> + * Based on Freescale's Linux i.MX mx53loco.h file:
>> + * Copyright (C) 2010-2011 Freescale Semiconductor.
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef __CONFIG_H
>> +#define __CONFIG_H
>> +
>> +#define CONFIG_MX53
>
>Move to defconfig
>
If I move that, mx53cx9020 would be different from all other mx53 boards.
Despite from that I tried to, but compiling will fail, because CPU_TYPE, required by imx-regs.h, is not set.
>> +
>> +#define CONFIG_MACH_TYPE       MACH_TYPE_MX53_LOCO
>
>What is the need for this?
>
I will remove it.
>> +
>> +#include <asm/arch/imx-regs.h>
>> +
>> +#define CONFIG_CMDLINE_TAG
>> +#define CONFIG_SETUP_MEMORY_TAGS
>> +#define CONFIG_INITRD_TAG
>> +
>> +#define CONFIG_SYS_FSL_CLK
>> +
>> +/* Size of malloc() pool */
>> +#define CONFIG_SYS_MALLOC_LEN          (10 * 1024 * 1024)
>> +
>> +#define CONFIG_BOARD_EARLY_INIT_F
>> +#define CONFIG_BOARD_LATE_INIT
>> +#define CONFIG_MXC_GPIO
>> +#define CONFIG_REVISION_TAG
>> +
>> +#define CONFIG_MXC_UART
>
>Move to defconfig
>
Again, mx53cx9020 would be different form all remaining mx53 boards. But at least this one seems to work.
>> +
>> +/* MMC Configs */
>> +#define CONFIG_FSL_ESDHC
>
>Move to defconfig
>
Again, mx53cx9020 would be different form all remaining mx53 boards. This one seems to work, too.
>> +#define CONFIG_SYS_FSL_ESDHC_ADDR      0
>> +#define CONFIG_SYS_FSL_ESDHC_NUM       2
>> +
>> +#define CONFIG_MMC
>> +#define CONFIG_GENERIC_MMC
>> +
>> +/* bootz: zImage/initrd.img support */
>> +#define CONFIG_DOS_PARTITION
>> +
>> +/* Eth Configs */
>> +#define CONFIG_MII
>> +
>> +#define CONFIG_FEC_MXC
>
>Move to defconfig
>
Again, mx53cx9020 would be different form all remaining mx53 boards. This one seems to work, too.
>> +#define IMX_FEC_BASE   FEC_BASE_ADDR
>> +#define CONFIG_FEC_MXC_PHYADDR 0x1F
>> +
>> +/* USB Configs */
>> +#define CONFIG_USB_EHCI
>> +#define CONFIG_USB_EHCI_MX5
>> +#define CONFIG_USB_STORAGE
>> +#define CONFIG_USB_HOST_ETHER
>> +#define CONFIG_USB_ETHER_ASIX
>> +#define CONFIG_USB_ETHER_MCS7830
>> +#define CONFIG_USB_ETHER_SMSC95XX
>> +#define CONFIG_MXC_USB_PORT    1
>> +#define CONFIG_MXC_USB_PORTSC  (PORT_PTS_UTMI |
>PORT_PTS_PTW)
>> +#define CONFIG_MXC_USB_FLAGS   0
>> +
>> +/* allow to overwrite serial and ethaddr */
>> +#define CONFIG_ENV_OVERWRITE
>> +#define CONFIG_CONS_INDEX              1
>> +#define CONFIG_BAUDRATE                        115200
>> +
>> +/* Command definition */
>> +#define CONFIG_SUPPORT_RAW_INITRD
>> +
>> +#define CONFIG_ETHPRIME                "FEC0"
>
>Garb all eth configs at one place.
>
I will move it up.
>> ...
>
>thanks!
thanks for your feedback. 

Regards,
Patrick
---