diff mbox

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

Message ID 1444427680-10423-3-git-send-email-george.mccollister@gmail.com
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

George McCollister Oct. 9, 2015, 9:54 p.m. UTC 
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@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

Comments

Bin Meng Oct. 10, 2015, 3:31 a.m. UTC | #1 
Hi George,

On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
<george.mccollister@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@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.

> +               #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?

> +                       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@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@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.

> +
> +#endif /* __CONFIG_H */
> --

Regards,
Bin
Bin Meng Oct. 10, 2015, 3:33 a.m. UTC | #2 
Hi George,

Sorry, one more nits below.

On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
<george.mccollister@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@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";
> +               #address-cells = <3>;
> +               #size-cells = <2>;
> +               u-boot,dm-pre-reloc;
> +               ranges = <0x02000000 0x0 0xe0000000 0xe0000000 0 0x10000000
> +                       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@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@gmail.com>

Please remove these unnecessary spaces before the email address.

> + *
> + * 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
> +
> +#endif /* __CONFIG_H */
> --

Regards,
Bin
Simon Glass Oct. 10, 2015, 7:31 a.m. UTC | #3 
+Bin

Hi George,

On 9 October 2015 at 22:54, George McCollister
<george.mccollister@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@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

I'm starting to think we need to name these better. ARM uses the
<soc>_<board>, so how about broadwell_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";
> +               #address-cells = <3>;
> +               #size-cells = <2>;
> +               u-boot,dm-pre-reloc;
> +               ranges = <0x02000000 0x0 0xe0000000 0xe0000000 0 0x10000000
> +                       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@0 {
> +                       reg = <0>;
> +                       compatible = "winbond,w25q128", "spi-flash";
> +                       memory-map = <0xff800000 0x00800000>;

If it is 16MB it might be mapped at 0xff000000 and have size 0x01000000.

> +               };
> +       };
> +};
> 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@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
> +
> +#endif /* __CONFIG_H */
> --
> 2.5.0
>

Regards,
Simon
Bin Meng Oct. 10, 2015, 9:08 a.m. UTC | #4 
Hi Simon,

On Sat, Oct 10, 2015 at 3:31 PM, Simon Glass <sjg@chromium.org> wrote:
> +Bin
>
> Hi George,
>
> On 9 October 2015 at 22:54, George McCollister
> <george.mccollister@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@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
>
> I'm starting to think we need to name these better. ARM uses the
> <soc>_<board>, so how about broadwell_som-6896.dtb?

I am good with the new name. And should we change defconfig and
config.h to use the same name too?

[snip]

Regards,
Bin
George McCollister Oct. 12, 2015, 1:34 p.m. UTC | #5 
On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi George,
>
> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
> <george.mccollister@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@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.
>
>> +                       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@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@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
George McCollister Oct. 12, 2015, 6:30 p.m. UTC | #6 
On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
<george.mccollister@gmail.com> wrote:
> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi George,
>>
>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>> <george.mccollister@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@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.

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>;

>>
>>> +                       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@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@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
Bin Meng Oct. 13, 2015, 12:48 a.m. UTC | #7 
Hi George,

On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
<george.mccollister@gmail.com> wrote:
> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
> <george.mccollister@gmail.com> wrote:
>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi George,
>>>
>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>>> <george.mccollister@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@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.

>>>
>>>> +                       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@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@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
George McCollister Oct. 13, 2015, 2:52 a.m. UTC | #8 
On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi George,
>
> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
> <george.mccollister@gmail.com> wrote:
>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
>> <george.mccollister@gmail.com> wrote:
>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi George,
>>>>
>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>>>> <george.mccollister@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@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@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@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
Bin Meng Oct. 13, 2015, 3:25 a.m. UTC | #9 
Hi George,

On Tue, Oct 13, 2015 at 10:52 AM, George McCollister
<george.mccollister@gmail.com> wrote:
> On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi George,
>>
>> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
>> <george.mccollister@gmail.com> wrote:
>>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
>>> <george.mccollister@gmail.com> wrote:
>>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> Hi George,
>>>>>
>>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>>>>> <george.mccollister@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@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.
>

Ah, yes! Sorry I was wrongly saying 0x1000, but I should really say
0x2000 as the I/O start address for U-Boot. 0x2000 is the I/O start
address which is used by other x86 boards. No need to change coreboot.

> 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.
>

I think currently this name (som-6896) is fine. I think a future
single patch to rename all x86 boards would be better. Also we may
need change config.h and defconfig file names as well.

Regards,
Bin
Simon Glass Oct. 15, 2015, 1:25 p.m. UTC | #10 
Hi Bin,

On Monday, 12 October 2015, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi George,
>
> On Tue, Oct 13, 2015 at 10:52 AM, George McCollister
> <george.mccollister@gmail.com> wrote:
> > On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >> Hi George,
> >>
> >> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
> >> <george.mccollister@gmail.com> wrote:
> >>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
> >>> <george.mccollister@gmail.com> wrote:
> >>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >>>>> Hi George,
> >>>>>
> >>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
> >>>>> <george.mccollister@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@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.
> >
>
> Ah, yes! Sorry I was wrongly saying 0x1000, but I should really say
> 0x2000 as the I/O start address for U-Boot. 0x2000 is the I/O start
> address which is used by other x86 boards. No need to change coreboot.
>
> > 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.
> >
>
> I think currently this name (som-6896) is fine. I think a future
> single patch to rename all x86 boards would be better. Also we may
> need change config.h and defconfig file names as well.

We tend to allow any config/board name, it is just the device tree
that has the convention I mentioned.

Regards,
Simon
George McCollister Oct. 19, 2015, 2:29 p.m. UTC | #11 
Simon,

On Thu, Oct 15, 2015 at 8:25 AM, Simon Glass <sjg@google.com> wrote:
> Hi Bin,
>
> On Monday, 12 October 2015, Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> Hi George,
>>
>> On Tue, Oct 13, 2015 at 10:52 AM, George McCollister
>> <george.mccollister@gmail.com> wrote:
>> > On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >> Hi George,
>> >>
>> >> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
>> >> <george.mccollister@gmail.com> wrote:
>> >>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
>> >>> <george.mccollister@gmail.com> wrote:
>> >>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>>>> Hi George,
>> >>>>>
>> >>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>> >>>>> <george.mccollister@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@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.
>> >
>>
>> Ah, yes! Sorry I was wrongly saying 0x1000, but I should really say
>> 0x2000 as the I/O start address for U-Boot. 0x2000 is the I/O start
>> address which is used by other x86 boards. No need to change coreboot.
>>
>> > 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.
>> >
>>
>> I think currently this name (som-6896) is fine. I think a future
>> single patch to rename all x86 boards would be better. Also we may
>> need change config.h and defconfig file names as well.
>
> We tend to allow any config/board name, it is just the device tree
> that has the convention I mentioned.
Do you want me to resend the patch with the DT renamed to
broadwell_som-6896.dts?
Do you want me to change the I/O start address to 0x2000 from 0x1900
(0x1900 seems to be working fine)?

Is there anything else I'm missing before it'll be acceptable to merge?
>
> Regards,
> Simon

Thanks,
George
Simon Glass Oct. 19, 2015, 2:44 p.m. UTC | #12 
Hi George,

On 19 October 2015 at 08:29, George McCollister
<george.mccollister@gmail.com> wrote:
> Simon,
>
> On Thu, Oct 15, 2015 at 8:25 AM, Simon Glass <sjg@google.com> wrote:
>> Hi Bin,
>>
>> On Monday, 12 October 2015, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>
>>> Hi George,
>>>
>>> On Tue, Oct 13, 2015 at 10:52 AM, George McCollister
>>> <george.mccollister@gmail.com> wrote:
>>> > On Mon, Oct 12, 2015 at 7:48 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> >> Hi George,
>>> >>
>>> >> On Tue, Oct 13, 2015 at 2:30 AM, George McCollister
>>> >> <george.mccollister@gmail.com> wrote:
>>> >>> On Mon, Oct 12, 2015 at 8:34 AM, George McCollister
>>> >>> <george.mccollister@gmail.com> wrote:
>>> >>>> On Fri, Oct 9, 2015 at 10:31 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> >>>>> Hi George,
>>> >>>>>
>>> >>>>> On Sat, Oct 10, 2015 at 5:54 AM, George McCollister
>>> >>>>> <george.mccollister@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@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.
>>> >
>>>
>>> Ah, yes! Sorry I was wrongly saying 0x1000, but I should really say
>>> 0x2000 as the I/O start address for U-Boot. 0x2000 is the I/O start
>>> address which is used by other x86 boards. No need to change coreboot.
>>>
>>> > 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.
>>> >
>>>
>>> I think currently this name (som-6896) is fine. I think a future
>>> single patch to rename all x86 boards would be better. Also we may
>>> need change config.h and defconfig file names as well.
>>
>> We tend to allow any config/board name, it is just the device tree
>> that has the convention I mentioned.
> Do you want me to resend the patch with the DT renamed to
> broadwell_som-6896.dts?

Yes please

> Do you want me to change the I/O start address to 0x2000 from 0x1900
> (0x1900 seems to be working fine)?

I think Bin requested this - yes let's go with 0x2000.

>
> Is there anything else I'm missing before it'll be acceptable to merge?

Not that I know of.

Regards,
Simon
diff mbox

Patch

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";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		u-boot,dm-pre-reloc;
+		ranges = <0x02000000 0x0 0xe0000000 0xe0000000 0 0x10000000
+			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@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@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
+
+#endif	/* __CONFIG_H */