Re: [PATCH 9/9] ARM: i.MX: Add support for NXP i.MX7 SABRESD board

[Andrey Smirnov <andrew.smirnov@gmail.com>]

On Mon, Jul 24, 2017 at 12:03 PM, Stefan Lengfeld
<contact@stefanchrist.eu> wrote:
> Hi Andrey,
>
> On Mon, Jul 24, 2017 at 07:54:00AM -0700, Andrey Smirnov wrote:
>> Add minimal code to support NXP i.MX7 SABRESD board. Tested to have
>> working SD card and first Ethernet port as well as being able to boot
>> upstream Linux kernel (4.12+).
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  arch/arm/boards/Makefile                           |  1 +
>>  arch/arm/boards/freescale-mx7-sabresd/Makefile     |  3 +
>>  arch/arm/boards/freescale-mx7-sabresd/board.c      | 59 ++++++++++++++++
>>  .../flash-header-mx7-sabresd.imxcfg                | 79 ++++++++++++++++++++++
>>  arch/arm/boards/freescale-mx7-sabresd/lowlevel.c   | 46 +++++++++++++
>>  arch/arm/dts/Makefile                              |  2 +-
>>  arch/arm/dts/imx7d-sdb.dts                         | 70 +++++++++++++++++++
>>  arch/arm/mach-imx/Kconfig                          |  7 ++
>>  images/Makefile.imx                                |  5 ++
>>  9 files changed, 271 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm/boards/freescale-mx7-sabresd/Makefile
>>  create mode 100644 arch/arm/boards/freescale-mx7-sabresd/board.c
>>  create mode 100644 arch/arm/boards/freescale-mx7-sabresd/flash-header-mx7-sabresd.imxcfg
>>  create mode 100644 arch/arm/boards/freescale-mx7-sabresd/lowlevel.c
>>  create mode 100644 arch/arm/dts/imx7d-sdb.dts
>>
>> diff --git a/arch/arm/boards/Makefile b/arch/arm/boards/Makefile
>> index 9bbdd684f..295a362bd 100644
>> --- a/arch/arm/boards/Makefile
>> +++ b/arch/arm/boards/Makefile
>> @@ -148,3 +148,4 @@ obj-$(CONFIG_MACH_WARP7)                  += element14-warp7/
>>  obj-$(CONFIG_MACH_VF610_TWR)                 += freescale-vf610-twr/
>>  obj-$(CONFIG_MACH_ZII_RDU2)                  += zii-imx6q-rdu2/
>>  obj-$(CONFIG_MACH_ZII_VF610_DEV)             += zii-vf610-dev/
>> +obj-$(CONFIG_MACH_FREESCALE_MX7_SABRESD)     += freescale-mx7-sabresd/
>> diff --git a/arch/arm/boards/freescale-mx7-sabresd/Makefile b/arch/arm/boards/freescale-mx7-sabresd/Makefile
>> new file mode 100644
>> index 000000000..9c3707a01
>> --- /dev/null
>> +++ b/arch/arm/boards/freescale-mx7-sabresd/Makefile
>> @@ -0,0 +1,3 @@
>> +obj-y += board.o
>> +lwl-y += lowlevel.o
>> +bbenv-y += env
>
> You are not adding any default environment files. So the above line is
> not needed.
>

Oops, that's a leftover from my custom code I used for testing but
missed and didn't remove. Will fix in v2.

>> diff --git a/arch/arm/boards/freescale-mx7-sabresd/board.c b/arch/arm/boards/freescale-mx7-sabresd/board.c
>> new file mode 100644
>> index 000000000..9c30707b2
>> --- /dev/null
>> +++ b/arch/arm/boards/freescale-mx7-sabresd/board.c
>> @@ -0,0 +1,59 @@
>> +/*
>> + * Copyright (C) 2016 Zodiac Inflight Innovation
>> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
>> + *
>> + * 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 <common.h>
>> +#include <init.h>
>> +#include <io.h>
>> +#include <mach/imx7-regs.h>
>> +#include <linux/phy.h>
>> +#include <mfd/imx7-iomuxc-gpr.h>
>> +
>> +#define PHY_ID_BCM54220 0x600d8589
>> +
>> +static int bcm54220_phy_fixup(struct phy_device *dev)
>> +{
>> +     phy_write(dev, 0x1e, 0x21);
>> +     phy_write(dev, 0x1f, 0x7ea8);
>> +     phy_write(dev, 0x1e, 0x2f);
>> +     phy_write(dev, 0x1f, 0x71b7);
>> +
>> +     return 0;
>> +}
>> +
>> +static int mx7_sabresd_init_fec(void)
>> +{
>> +     void __iomem *gpr = IOMEM(MX7_IOMUXC_GPR_BASE_ADDR);
>> +     uint32_t gpr1;
>> +
>> +     gpr1 = readl(gpr + IOMUXC_GPR1);
>> +     gpr1 &= ~(IMX7D_GPR1_ENET1_TX_CLK_SEL_MASK |
>> +               IMX7D_GPR1_ENET1_CLK_DIR_MASK);
>> +     writel(gpr1, gpr + IOMUXC_GPR1);
>> +
>> +     return 0;
>> +}
>> +coredevice_initcall(mx7_sabresd_init_fec);
>
> As Sam as already pointed out, the fec init should not be done a
> initcall. For multi image support just call the function in the body of
> mx7_sabresd_coredevices_init(), so it's protected by the device tree
> compatible.
>

Yep, as I already mentioned to Sam, that's a mistake on my part and
the intention was to have that guarded by of_machine_is_compatible as
well. Will fix in v2.

>> +
>> +static int mx7_sabresd_coredevices_init(void)
>> +{
>> +     if (!of_machine_is_compatible("fsl,imx7d-sdb"))
>> +             return 0;
>> +
>> +     phy_register_fixup_for_uid(PHY_ID_BCM54220, 0xffffffff,
>> +                                bcm54220_phy_fixup);
>> +
>> +     return 0;
>> +}
>> +coredevice_initcall(mx7_sabresd_coredevices_init);
>> diff --git a/arch/arm/boards/freescale-mx7-sabresd/flash-header-mx7-sabresd.imxcfg b/arch/arm/boards/freescale-mx7-sabresd/flash-header-mx7-sabresd.imxcfg
>> new file mode 100644
>> index 000000000..c5e17f2a0
>> --- /dev/null
>> +++ b/arch/arm/boards/freescale-mx7-sabresd/flash-header-mx7-sabresd.imxcfg
>> @@ -0,0 +1,79 @@
>> +/*
>> + * Copyright (C) 2016 NXP Semiconductors
>> + *
>> + * SPDX-License-Identifier:  GPL-2.0
>> + *
>> + * Refer docs/README.imxmage for more details about how-to configure
>> + * and create imximage boot image
>> + *
>> + * The syntax is taken as close as possible with the kwbimage
>> + */
>> +
>> +soc imx7
>> +loadaddr 0x80000000
>> +dcdofs 0x400
>> +
>
> Maybe add a short comment here, where you have found the DCD values. It
> seems that they are copied from the u-boot fork of Freescale/NXP. So
> just adding a URL and commit SHA1 would be fine.
>
> It's quite hard to verify and track down the origin these magics values
> any time later. Usually they pop up from somewhere and are never touched
> again.
>

Came from upstream U-Boot, actually. The ones that were in NXP tree
didn't work for me at all. Will add a reference to the source in v2.

Thanks,
Andrey Smirnov

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