[U-Boot] [PATCH 3/3] x86: Added support for Advantech SOM-6896

[George McCollister]

On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi George,
>
> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
> <george.mccollister at gmail.com> wrote:
>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
>> <george.mccollister at gmail.com> wrote:
>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Hi George,
>>>>
>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>>>> <george.mccollister at gmail.com> wrote:
>>>>> Advantech SOM-6896 is a Broadwell U based COM Express Compact Module
>>>>> Type 6. This patch adds support for it as a coreboot payload.
>>>>>
>>>>> On board SATA and SPI are functional. On board Ethernet isn't functional
>>>>> but since it's optional and ties up a PCIe x4 that is otherwise brought
>>>>> out, this isn't a concern at the moment. USB doesn't work since the
>>>>> xHCI driver appears to be broken.
>>>>>
>>>>> Signed-off-by: George McCollister <george.mccollister at gmail.com>
>>>>> ---
>>>>>  arch/x86/dts/Makefile      |  3 ++-
>>>>>  arch/x86/dts/som-6896.dts  | 43 +++++++++++++++++++++++++++++++++++++++++++
>>>>>  include/configs/som-6896.h | 38 ++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 83 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 arch/x86/dts/som-6896.dts
>>>>>  create mode 100644 include/configs/som-6896.h
>>>>>
>>>>> diff --git a/arch/x86/dts/Makefile b/arch/x86/dts/Makefile
>>>>> index 71595c7..9fa2e21 100644
>>>>> --- a/arch/x86/dts/Makefile
>>>>> +++ b/arch/x86/dts/Makefile
>>>>> @@ -6,7 +6,8 @@ dtb-y += bayleybay.dtb \
>>>>>         galileo.dtb \
>>>>>         minnowmax.dtb \
>>>>>         qemu-x86_i440fx.dtb \
>>>>> -       qemu-x86_q35.dtb
>>>>> +       qemu-x86_q35.dtb \
>>>>> +       som-6896.dtb
>>>>>
>>>>>  targets += $(dtb-y)
>>>>>
>>>>> diff --git a/arch/x86/dts/som-6896.dts b/arch/x86/dts/som-6896.dts
>>>>> new file mode 100644
>>>>> index 0000000..ad904c9
>>>>> --- /dev/null
>>>>> +++ b/arch/x86/dts/som-6896.dts
>>>>> @@ -0,0 +1,43 @@
>>>>> +/dts-v1/;
>>>>> +
>>>>> +/include/ "skeleton.dtsi"
>>>>> +/include/ "serial.dtsi"
>>>>> +/include/ "rtc.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +       model = "Advantech SOM-6896";
>>>>> +       compatible = "advantech,som-6896", "intel,broadwell";
>>>>> +
>>>>> +       aliases {
>>>>> +               spi0 = "/spi";
>>>>> +       };
>>>>> +
>>>>> +       config {
>>>>> +              silent_console = <0>;
>>>>> +       };
>>>>> +
>>>>> +       chosen {
>>>>> +               stdout-path = "/serial";
>>>>> +       };
>>>>> +
>>>>> +       pci {
>>>>> +               compatible = "intel,pci-broadwell", "pci-x86";
>>>>
>>>> I would just describe it as "pci-x86" as Intel chipset PCI is compatible.
>>> Okay
>>>>
>>>>> +               #address-cells = <3>;
>>>>> +               #size-cells = <2>;
>>>>> +               u-boot,dm-pre-reloc;
>>>>> +               ranges = <0x02000000 0x0 0xe0000000 0xe0000000 0 0x10000000
>>>>
>>>> Can you verify 0xe0000000 is not configured by coreboot as the PCIe
>>>> ECAM base address?
>>> I'll try to verify these, it's quite possible they are incorrect.
>>
>> CONFIG_MMCONF_BASE_ADDRESS is set to 0xf0000000 in coreboot.
>>
>> For prefmem/mem coreboot is showing:
>> DOMAIN: 0000 mem: base:d0000000 size:1111f120 align:28 gran:0 limit:efffffff
>> PCI: 00:02.0 18 *  [0xd0000000 - 0xdfffffff] prefmem
>> PCI: 00:02.0 10 *  [0xe0000000 - 0xe0ffffff] mem
>> PCI: 00:1c.0 20 *  [0xe1000000 - 0xe10fffff] mem
>> PCI: 00:14.0 10 *  [0xe1100000 - 0xe110ffff] mem
>> PCI: 00:1f.2 24 *  [0xe1110000 - 0xe1117fff] mem
>> PCI: 00:1b.0 10 *  [0xe1118000 - 0xe111bfff] mem
>> PCI: 00:15.0 10 *  [0xe111c000 - 0xe111cfff] mem
>> PCI: 00:15.0 14 *  [0xe111d000 - 0xe111dfff] mem
>> PCI: 00:1f.6 10 *  [0xe111e000 - 0xe111efff] mem
>> PCI: 00:1f.3 10 *  [0xe111f000 - 0xe111f0ff] mem
>> PCI: 00:16.0 10 *  [0xe111f100 - 0xe111f11f] mem
>> DOMAIN: 0000 mem: next_base: e111f120 size: 1111f120 align: 28 gran: 0 done
>> PCI: 00:1c.0 prefmem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.0 prefmem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.0 mem: base:e1000000 size:100000 align:20 gran:20 limit:e10fffff
>> PCI: 01:00.0 14 *  [0xe1000000 - 0xe107ffff] mem
>> PCI: 01:00.0 30 *  [0xe1080000 - 0xe10bffff] mem
>> PCI: 01:00.0 10 *  [0xe10c0000 - 0xe10dffff] mem
>> PCI: 01:00.0 1c *  [0xe10e0000 - 0xe10e3fff] mem
>> PCI: 00:1c.0 mem: next_base: e10e4000 size: 100000 align: 20 gran: 20 done
>> PCI: 00:1c.1 prefmem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.1 prefmem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.1 mem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.1 mem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.2 prefmem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.2 prefmem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.2 mem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.2 mem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.3 prefmem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.3 prefmem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.3 mem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.3 mem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.4 prefmem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.4 prefmem: next_base: efffffff size: 0 align: 20 gran: 20 done
>> PCI: 00:1c.4 mem: base:efffffff size:0 align:20 gran:20 limit:efffffff
>> PCI: 00:1c.4 mem: next_base: efffffff size: 0 align: 20 gran: 20 done
>>
>> Based on that I think the prefetchable mem region and the mem region
>> are correct.
>
> Thanks for checking this
>
>>
>> For io coreboot is showing:
>> DOMAIN: 0000 io: base:1900 size:1078 align:12 gran:0 limit:ffff
>> PCI: 00:1c.0 1c *  [0x2000 - 0x2fff] io
>> PCI: 00:02.0 20 *  [0x3000 - 0x303f] io
>> PCI: 00:1f.2 20 *  [0x3040 - 0x305f] io
>> PCI: 00:1f.2 10 *  [0x3060 - 0x3067] io
>> PCI: 00:1f.2 18 *  [0x3068 - 0x306f] io
>> PCI: 00:1f.2 14 *  [0x3070 - 0x3073] io
>> PCI: 00:1f.2 1c *  [0x3074 - 0x3077] io
>> DOMAIN: 0000 io: next_base: 3078 size: 1078 align: 12 gran: 0 done
>> PCI: 00:1c.0 io: base:2000 size:1000 align:12 gran:12 limit:2fff
>> PCI: 01:00.0 18 *  [0x2000 - 0x201f] io
>> PCI: 00:1c.0 io: next_base: 2020 size: 1000 align: 12 gran: 12 done
>> PCI: 00:1c.1 io: base:ffff size:0 align:12 gran:12 limit:ffff
>> PCI: 00:1c.1 io: next_base: ffff size: 0 align: 12 gran: 12 done
>> PCI: 00:1c.2 io: base:ffff size:0 align:12 gran:12 limit:ffff
>> PCI: 00:1c.2 io: next_base: ffff size: 0 align: 12 gran: 12 done
>> PCI: 00:1c.3 io: base:ffff size:0 align:12 gran:12 limit:ffff
>> PCI: 00:1c.3 io: next_base: ffff size: 0 align: 12 gran: 12 done
>> PCI: 00:1c.4 io: base:ffff size:0 align:12 gran:12 limit:ffff
>> PCI: 00:1c.4 io: next_base: ffff size: 0 align: 12 gran: 12 done
>>
>> Based on that I'm thinking this:
>> 0x01000000 0x0 0x1000 0x1000 0 0xefff>
>> Should actually be:
>> 0x01000000 0x0 0x1900 0x1900 0 0xe700>;
>>
>
> I think having I/O address start from 0x1000 does not have a problem.
> All the common known legacy I/O port addresses are below 0x1000. Are
> there any special I/O chipset that occupies 0x1000 - 0x1900? Anyway,
> setting it to 0x1900 in U-Boot is not a big deal.

Before changing this I had to move the ACPI base address configured by
coreboot (default 0x1000) to a different address for Linux to work
with ACPI. One of the reviewers to my coreboot submission asked me to
look into why this was required and I found that I was able to use the
default ACPI base address of 0x1000 when I moved this from 0x1000 to
0x1900 in u-boot. Since changing the ACPI base address in coreboot
required a patch that touched the chipset headers and required me to
add a configuration setting they'd like me to leave it out since it's
not necessary.

On a separate note I just noticed I forgot to rename som-6896 to
broadwell_som-6896 as suggested. I'll send an updated patch series in
the morning with that change.

>
>>>>
>>>>> +                       0x42000000 0x0 0xd0000000 0xd0000000 0 0x10000000
>>>>> +                       0x01000000 0x0 0x1000 0x1000 0 0xefff>;
>>>>> +       };
>>>>> +
>>>>> +       spi {
>>>>> +               #address-cells = <1>;
>>>>> +               #size-cells = <0>;
>>>>> +               compatible = "intel,ich-spi";
>>>>> +               spi-flash at 0 {
>>>>> +                       reg = <0>;
>>>>> +                       compatible = "winbond,w25q128", "spi-flash";
>>>>> +                       memory-map = <0xff800000 0x00800000>;
>>>>> +               };
>>>>> +       };
>>>>> +};
>>>>> diff --git a/include/configs/som-6896.h b/include/configs/som-6896.h
>>>>> new file mode 100644
>>>>> index 0000000..518bf11
>>>>> --- /dev/null
>>>>> +++ b/include/configs/som-6896.h
>>>>> @@ -0,0 +1,38 @@
>>>>> +/*
>>>>> + * Configuration settings for the SOM-6896
>>>>> + *
>>>>> + * Copyright (C) 2015 NovaTech LLC
>>>>> + *                   George McCollister <george.mccollister at gmail.com>
>>>>> + *
>>>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#ifndef __CONFIG_H
>>>>> +#define __CONFIG_H
>>>>> +
>>>>> +#include <configs/x86-common.h>
>>>>> +
>>>>> +#define CONFIG_SYS_MONITOR_LEN         (1 << 20)
>>>>> +
>>>>> +#define CONFIG_BOARD_EARLY_INIT_F
>>>>> +#define CONFIG_MISC_INIT_R
>>>>> +
>>>>> +#define CONFIG_SCSI_DEV_LIST   \
>>>>> +       {PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_WILDCATPOINT_AHCI}
>>>>> +
>>>>> +#define CONFIG_SYS_EARLY_PCI_INIT
>>>>> +#define CONFIG_PCI_PNP
>>>>> +
>>>>> +#define VIDEO_IO_OFFSET                        0
>>>>> +#define CONFIG_X86EMU_RAW_IO
>>>>> +
>>>>> +#define CONFIG_ARCH_EARLY_INIT_R
>>>>> +
>>>>> +#define CONFIG_STD_DEVICES_SETTINGS     "stdin=serial,vga,usbkbd\0" \
>>>>> +                                       "stdout=serial,vga\0" \
>>>>> +                                       "stderr=serial,vga\0"
>>>>> +
>>>>> +#define CONFIG_ENV_SECT_SIZE           0x1000
>>>>> +#define CONFIG_ENV_OFFSET              0x00ff0000
>>>>
>>>> This address looks odd to me. As in the dts file, the SPI flash is
>>>> described as a 8MB chip, but here the offset is below 16MB, which is
>>>> outside the flash address range.
>>> Yeah it's a 16MB chip, I screwed that up, I'll fix it. Thanks for catching that.
>>>>
>>>>> +
>>>>> +#endif /* __CONFIG_H */
>>>>> --
>>>>
>
> Regards,
> Bin

Regards,
George