Hi Gabriel,

On 15 February 2015 at 14:55, Gabriel Huau contact@huau-gabriel.fr wrote:

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

Thanks for the patch!

I have mostly nits except for one comment about register access which is different in U-Boot...

---

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

diff --git a/arch/x86/cpu/baytrail/Makefile b/arch/x86/cpu/baytrail/Makefile index 8914e8b..c20a616 100644 --- a/arch/x86/cpu/baytrail/Makefile +++ b/arch/x86/cpu/baytrail/Makefile @@ -8,3 +8,4 @@ obj-y += early_uart.o obj-y += fsp_configs.o obj-y += pci.o obj-y += valleyview.o +obj-y += gpio.o

Please keep in alphabetical order.

diff --git a/arch/x86/cpu/baytrail/gpio.c b/arch/x86/cpu/baytrail/gpio.c new file mode 100644 index 0000000..0ad41cc --- /dev/null +++ b/arch/x86/cpu/baytrail/gpio.c @@ -0,0 +1,206 @@ +/*

• • Copyright (c) 2012 The Chromium OS Authors.

Please add 'From coreboot <filename>' here so people know where it came from.

• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <fdtdec.h> +#include <pci.h> +#include <asm/arch/gpio.h> +#include <asm/arch/irqroute.h> +#include <asm/arch/pmc.h> +#include <asm/gpio.h> +#include <asm/io.h> +#include <asm/pci.h>

+/* GPIO-to-Pad LUTs */ +static const u8 gpncore_gpio_to_pad[GPNCORE_COUNT] = {

•   19, 18, 17, 20, 21, 22, 24, 25, /* [ 0: 7] */
  

•   23, 16, 14, 15, 12, 26, 27,  1, /* [ 8:15] */
  

•   4,  8, 11,  0,  3,  6, 10, 13,  /* [16:23] */
  

•   2,  5,  9                       /* [24:26] */
  

+};

+static const u8 gpscore_gpio_to_pad[GPSCORE_COUNT] = {

•   85,  89, 93,  96, 99, 102,  98, 101, /* [ 0:  7] */
  

•   34,  37, 36,  38, 39,  35,  40,  84, /* [ 8: 15] */
  

•   62,  61, 64,  59, 54,  56,  60,  55, /* [16: 23] */
  

•   63,  57, 51,  50, 53,  47,  52,  49, /* [24: 31] */
  

•   48,  43, 46,  41, 45,  42,  58,  44, /* [32: 39] */
  

•   95, 105, 70,  68, 67,  66,  69,  71, /* [40: 47] */
  

•   65,  72, 86,  90, 88,  92, 103,  77, /* [48: 55] */
  

•   79,  83, 78,  81, 80,  82,  13,  12, /* [56: 63] */
  

•   15,  14, 17,  18, 19,  16,   2,   1, /* [64: 71] */
  

•   0,   4,  6,   7,  9,   8,  33,  32,      /* [72: 79] */
  

•   31,  30, 29,  27, 25,  28,  26,  23, /* [80: 87] */
  

•   21,  20, 24,  22,  5,   3,  10,  11, /* [88: 95] */
  

•   106,  87, 91, 104, 97, 100           /* [96:101] */
  

+};

+static const u8 gpssus_gpio_to_pad[GPSSUS_COUNT] = {

•   29, 33, 30, 31, 32, 34, 36, 35,   /* [ 0: 7] */
  

•     38, 37, 18,  7, 11, 20, 17,  1, /* [ 8:15] */
  

•      8, 10, 19, 12,  0,  2, 23, 39, /* [16:23] */
  

•     28, 27, 22, 21, 24, 25, 26, 51, /* [24:31] */
  

•     56, 54, 49, 55, 48, 57, 50, 58, /* [32:39] */
  

•     52, 53, 59, 40                  /* [40:43] */
  

+};

The above three tables are not quite lined up, but it looks like that was your intention.

+/* GPIO bank descriptions */ +static const struct gpio_bank gpncore_bank = {

•   .gpio_count = GPNCORE_COUNT,
  

•   .gpio_to_pad = gpncore_gpio_to_pad,
  

•   .legacy_base = GP_LEGACY_BASE_NONE,
  

•   .pad_base = GPNCORE_PAD_BASE,
  

•   .has_wake_en = 0,
  

•   .gpio_f1_range_start = GPNCORE_GPIO_F1_RANGE_START,
  

•   .gpio_f1_range_end = GPNCORE_GPIO_F1_RANGE_END,
  

+};

+static const struct gpio_bank gpscore_bank = {

•   .gpio_count = GPSCORE_COUNT,
  

•   .gpio_to_pad = gpscore_gpio_to_pad,
  

•   .legacy_base = GPSCORE_LEGACY_BASE,
  

•   .pad_base = GPSCORE_PAD_BASE,
  

•   .has_wake_en = 0,
  

•   .gpio_f1_range_start = GPSCORE_GPIO_F1_RANGE_START,
  

•   .gpio_f1_range_end = GPSCORE_GPIO_F1_RANGE_END,
  

+};

+static const struct gpio_bank gpssus_bank = {

•   .gpio_count = GPSSUS_COUNT,
  

•   .gpio_to_pad = gpssus_gpio_to_pad,
  

•   .legacy_base = GPSSUS_LEGACY_BASE,
  

•   .pad_base = GPSSUS_PAD_BASE,
  

•   .has_wake_en = 1,
  

•   .gpio_f1_range_start = GPSSUS_GPIO_F1_RANGE_START,
  

•   .gpio_f1_range_end = GPSSUS_GPIO_F1_RANGE_END,
  

+};

+static void setup_gpios(const struct byt_gpio_map *gpios,

•                   const struct gpio_bank *bank)
  

+{

•   const struct byt_gpio_map *config;
  

•   int gpio = 0;
  

•   u32 reg, pad_conf0;
  

•   u8 set, bit;
  

Remove blank line

•   u32 use_sel[4] = {0};
  

•   u32 io_sel[4] = {0};
  

•   u32 gp_lvl[4] = {0};
  

•   u32 tpe[4] = {0};
  

•   u32 tne[4] = {0};
  

•   u32 wake_en[4] = {0};
  

•   if (!gpios)
  

•           return;
  

•   for (config = gpios; config->pad_conf0 != GPIO_LIST_END;
  

•        config++, gpio++) {
  

•           if (gpio > bank->gpio_count)
  

•                   break;
  

•           set = gpio >> 5;
  

•           bit = gpio % 32;
  

•           if (bank->legacy_base != GP_LEGACY_BASE_NONE) {
  

•                   /* Legacy IO configuration */
  

•                   use_sel[set] |= config->use_sel << bit;
  

•                   io_sel[set]  |= config->io_sel  << bit;
  

•                   gp_lvl[set]  |= config->gp_lvl  << bit;
  

•                   tpe[set]     |= config->tpe     << bit;
  

•                   tne[set]     |= config->tne     << bit;
  

•                   /* Some banks do not have wake_en ability */
  

•                   if (bank->has_wake_en)
  

•                           wake_en[set] |= config->wake_en << bit;
  

•           }
  

•           /* Pad configuration registers */
  

•           reg = bank->pad_base + 16 * bank->gpio_to_pad[gpio];
  

Register access in U-Boot should normally be done via a struct. See here:

http://www.denx.de/wiki/U-Boot/CodingStyle

There are exceptions when it really doesn't work, but this doesn't look like one. The LEGACY_USE_SEL_REG, etc. below look like they could become structure members.

•           /* Add correct func to GPIO pad config */
  

•           pad_conf0 = config->pad_conf0;
  

•           if (config->is_gpio) {
  

•                   if (gpio >= bank->gpio_f1_range_start &&
  

•                       gpio <= bank->gpio_f1_range_end)
  

•                           pad_conf0 |= PAD_FUNC1;
  

•                   else
  

•                           pad_conf0 |= PAD_FUNC0;
  

•           }
  

•           writel(reg + PAD_CONF0_REG, pad_conf0);
  

•           writel(reg + PAD_CONF1_REG, config->pad_conf1);
  

•           writel(reg + PAD_VAL_REG, config->pad_val);
  

•   }
  

•   if (bank->legacy_base != GP_LEGACY_BASE_NONE)
  

•           for (set = 0; set <= (bank->gpio_count - 1) / 32; ++set) {
  

•                   reg = bank->legacy_base + 0x20 * set;
  

•                   outl(use_sel[set], reg + LEGACY_USE_SEL_REG);
  

•                   outl(io_sel[set], reg + LEGACY_IO_SEL_REG);
  

•                   outl(gp_lvl[set], reg + LEGACY_GP_LVL_REG);
  

•                   outl(tpe[set], reg + LEGACY_TPE_REG);
  

•                   outl(tne[set], reg + LEGACY_TNE_REG);
  

•                   /* TS registers are WOC  */
  

If you know what this comment means, please spell it out without abbreviations.

•                   outl(0, reg + LEGACY_TS_REG);
  

•                   if (bank->has_wake_en)
  

•                           outl(wake_en[set], reg + LEGACY_WAKE_EN_REG);
  

•           }
  

+}

+static void setup_gpio_route(const struct byt_gpio_map *sus,

•           const struct byt_gpio_map *core)
  

+{

•   uint32_t route_reg = 0;
  

•   int i;
  

•   for (i = 0; i < 8; i++) {
  

•           /* SMI takes precedence and wake_en implies SCI. */
  

•           if (sus[i].smi)
  

•                   route_reg |= ROUTE_SMI << (2 * i);
  

•           else if (sus[i].sci)
  

•                   route_reg |= ROUTE_SCI << (2 * i);
  

•           if (core[i].smi)
  

•                   route_reg |= ROUTE_SMI << (2 * (i + 8));
  

•           else if (core[i].sci)
  

•                   route_reg |= ROUTE_SCI << (2 * (i + 8));
  

•   }
  

What happens to route_reg after this? I don't see it get returned.

+}

+static void setup_dirqs(const u8 dirq[GPIO_MAX_DIRQS],

•           const struct gpio_bank *bank)
  

+{

•   u32 reg = bank->pad_base + PAD_BASE_DIRQ_OFFSET;
  

•   u32 val;
  

•   int i;
  

•   /* Write all four DIRQ registers */
  

•   for (i = 0; i < 4; ++i) {
  

•           val = dirq[i * 4 + 3] << 24 | dirq[i * 4 + 2] << 16 |
  

•                 dirq[i * 4 + 1] << 8  | dirq[i * 4];
  

•           writel(reg + i * 4, val);
  

•   }
  

Can we factor out the * 4, so:

for (i = 0; i < 16; i += 4) { val = dirq[i + 3] << 24 | ...

+}

+void setup_soc_gpios(struct byt_gpio_config *config) +{

•   if (config) {
  

•           setup_gpios(config->ncore, &gpncore_bank);
  

•           setup_gpios(config->score, &gpscore_bank);
  

•           setup_gpios(config->ssus,  &gpssus_bank);
  

•           setup_gpio_route(config->ssus, config->score);
  

•           if (config->core_dirq)
  

•                   setup_dirqs(*config->core_dirq, &gpscore_bank);
  

•           if (config->sus_dirq)
  

•                   setup_dirqs(*config->sus_dirq, &gpssus_bank);
  

•   }
  

+} diff --git a/arch/x86/include/asm/arch-baytrail/gpio.h b/arch/x86/include/asm/arch-baytrail/gpio.h index ab4e059..40b0ffa 100644 --- a/arch/x86/include/asm/arch-baytrail/gpio.h +++ b/arch/x86/include/asm/arch-baytrail/gpio.h @@ -7,7 +7,371 @@ #ifndef _X86_ARCH_GPIO_H_ #define _X86_ARCH_GPIO_H_

+#include <common.h> +#include <asm/arch-baytrail/iomap.h> +#include <asm/io.h>

/* Where in config space is the register that points to the GPIO registers? */ #define PCI_CFG_GPIOBASE 0x44

+/* Pad base, ex. PAD_CONF0[n]= PAD_BASE+16*n */ +#define GPSCORE_PAD_BASE (IO_BASE_ADDRESS + IO_BASE_OFFSET_GPSCORE) +#define GPNCORE_PAD_BASE (IO_BASE_ADDRESS + IO_BASE_OFFSET_GPNCORE) +#define GPSSUS_PAD_BASE (IO_BASE_ADDRESS + IO_BASE_OFFSET_GPSSUS)

+/* DIRQ registers start at pad base + 0x980 */ +#define PAD_BASE_DIRQ_OFFSET 0x980

+/* Pad register offset */ +#define PAD_CONF0_REG 0x0 +#define PAD_CONF1_REG 0x4 +#define PAD_VAL_REG 0x8

+/* Legacy IO register base */ +#define GPSCORE_LEGACY_BASE (GPIO_BASE_ADDRESS + 0x00) +#define GPSSUS_LEGACY_BASE (GPIO_BASE_ADDRESS + 0x80) +/* Some banks have no legacy GPIO interface */ +#define GP_LEGACY_BASE_NONE 0xFFFF

+#define LEGACY_USE_SEL_REG 0x00 +#define LEGACY_IO_SEL_REG 0x04 +#define LEGACY_GP_LVL_REG 0x08 +#define LEGACY_TPE_REG 0x0C +#define LEGACY_TNE_REG 0x10 +#define LEGACY_TS_REG 0x14 +#define LEGACY_WAKE_EN_REG 0x18

Feel like these should be a struct:

struct some_name { u32 use_sel; u32 io_sel; ... };

+/* Number of GPIOs in each bank */ +#define GPNCORE_COUNT 27 +#define GPSCORE_COUNT 102 +#define GPSSUS_COUNT 44

+/* GPIO legacy IO register settings */ +#define GPIO_USE_MMIO 0 +#define GPIO_USE_LEGACY 1

+#define GPIO_DIR_OUTPUT 0 +#define GPIO_DIR_INPUT 1

Can we use the ones in arch/x86/include/asm/gpio.h?

+#define GPIO_LEVEL_LOW 0 +#define GPIO_LEVEL_HIGH 1

+#define GPIO_PEDGE_DISABLE 0 +#define GPIO_PEDGE_ENABLE 1

+#define GPIO_NEDGE_DISABLE 0 +#define GPIO_NEDGE_ENABLE 1

+/* config0[29] - Disable second mask */ +#define PAD_MASK2_DISABLE (1 << 29)

+/* config0[27] - Direct Irq En */ +#define PAD_IRQ_EN (1 << 27)

+/* config0[26] - gd_tne */ +#define PAD_TNE_IRQ (1 << 26)

+/* config0[25] - gd_tpe */ +#define PAD_TPE_IRQ (1 << 25)

+/* config0[24] - Gd Level */ +#define PAD_LEVEL_IRQ (1 << 24) +#define PAD_EDGE_IRQ (0 << 24)

+/* config0[17] - Slow clkgate / glitch filter */ +#define PAD_SLOWGF_ENABLE (1 << 17)

+/* config0[16] - Fast clkgate / glitch filter */ +#define PAD_FASTGF_ENABLE (1 << 16)

+/* config0[15] - Hysteresis enable (inverted) */ +#define PAD_HYST_DISABLE (1 << 15) +#define PAD_HYST_ENABLE (0 << 15)

+/* config0[14:13] - Hysteresis control */ +#define PAD_HYST_CTRL_DEFAULT (2 << 13)

+/* config0[11] - Bypass Flop */ +#define PAD_FLOP_BYPASS (1 << 11) +#define PAD_FLOP_ENABLE (0 << 11)

+/* config0[10:9] - Pull str */ +#define PAD_PU_2K (0 << 9) +#define PAD_PU_10K (1 << 9) +#define PAD_PU_20K (2 << 9) +#define PAD_PU_40K (3 << 9)

+/* config0[8:7] - Pull assign */ +#define PAD_PULL_DISABLE (0 << 7) +#define PAD_PULL_UP (1 << 7) +#define PAD_PULL_DOWN (2 << 7)

+/* config0[2:0] - Func. pin mux */ +#define PAD_FUNC0 0x0 +#define PAD_FUNC1 0x1 +#define PAD_FUNC2 0x2 +#define PAD_FUNC3 0x3 +#define PAD_FUNC4 0x4 +#define PAD_FUNC5 0x5 +#define PAD_FUNC6 0x6

These could be an anonymous enum (optional)

+/* pad config0 power-on values - We will not often want to change these */ +#define PAD_CONFIG0_DEFAULT (PAD_MASK2_DISABLE | PAD_SLOWGF_ENABLE | \

•                            PAD_FASTGF_ENABLE     | PAD_HYST_DISABLE | \
  

•                            PAD_HYST_CTRL_DEFAULT | PAD_FLOP_BYPASS)
  

Then this could be part of the same enum, and you avoid the line continuations.

+/* pad config1 reg power-on values - Shouldn't need to change this */ +#define PAD_CONFIG1_DEFAULT 0x8000

+/* pad_val[2] - Iinenb - active low */ +#define PAD_VAL_INPUT_DISABLE (1 << 2) +#define PAD_VAL_INPUT_ENABLE (0 << 2)

+/* pad_val[1] - Ioutenb - active low */ +#define PAD_VAL_OUTPUT_DISABLE (1 << 1) +#define PAD_VAL_OUTPUT_ENABLE (0 << 1)

+/* Input / Output state should usually be mutually exclusive */ +#define PAD_VAL_INPUT (PAD_VAL_INPUT_ENABLE | PAD_VAL_OUTPUT_DISABLE) +#define PAD_VAL_OUTPUT (PAD_VAL_OUTPUT_ENABLE | PAD_VAL_INPUT_DISABLE)

+/* pad_val[0] - Value */ +#define PAD_VAL_HIGH (1 << 0) +#define PAD_VAL_LOW (0 << 0)

+/* pad_val reg power-on default varies by pad, and apparently can cause issues

• • if not set correctly, even if the pin isn't configured as GPIO. */

+#define PAD_VAL_DEFAULT PAD_VAL_INPUT

+/* Configure GPIOs as MMIO by default */ +#define GPIO_INPUT_PU_10K(_func) \

•   { .pad_conf0 = PAD_FUNC##_func | PAD_PU_10K | \
  

•           PAD_PULL_UP | \
  

•           PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_MMIO, \
  

•     .is_gpio   = 1 }
  

I'm not a big fan of this sort of thing- #defines for structures in header files. It feels pretty ugly?

I wonder if there is another way of doing it?

+#define GPIO_INPUT_PD_10K \

•   { .pad_conf0 = PAD_PU_10K | PAD_PULL_DOWN | PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_MMIO, \
  

•     .is_gpio   = 1 }
  

+#define GPIO_INPUT_NOPU \

•   { .pad_conf0 = PAD_PU_10K | PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_MMIO, \
  

•     .is_gpio   = 1 }
  

+#define GPIO_INPUT_LEGACY_NOPU \

•   { .pad_conf0 = PAD_PU_10K | PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_INPUT, \
  

•     .is_gpio   = 1 }
  

+/* Direct / dedicated IRQ input - pass signal directly to apic */ +#define GPIO_DIRQ \

•   { .pad_conf0 = PAD_PU_10K | PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT \
  

•                | PAD_FUNC0 | PAD_IRQ_EN | PAD_TPE_IRQ | PAD_LEVEL_IRQ, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, }
  

+#define GPIO_OUT_LOW \

•   { .pad_conf0 = PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_OUTPUT | PAD_VAL_LOW, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_OUTPUT, \
  

•     .gp_lvl    = GPIO_LEVEL_LOW, \
  

•     .is_gpio   = 1 }
  

+#define GPIO_OUT_HIGH \

•   { .pad_conf0 = PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_OUTPUT | PAD_VAL_HIGH, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_OUTPUT, \
  

•     .gp_lvl    = GPIO_LEVEL_HIGH, \
  

•     .is_gpio   = 1 }
  

+/* Define no-pull / PU / PD configs for each functional config option */ +#define GPIO_FUNC(_func, _pudir, _str) \

•   { .use_sel   = GPIO_USE_MMIO, \
  

•     .pad_conf0 = PAD_FUNC##_func | PAD_##_pudir | PAD_PU_##_str | \
  

•                  PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_DEFAULT }
  

+/* Default functional configs -- no PU */ +#define GPIO_FUNC0 GPIO_FUNC(0, PULL_DISABLE, 10K) +#define GPIO_FUNC1 GPIO_FUNC(1, PULL_DISABLE, 10K) +#define GPIO_FUNC2 GPIO_FUNC(2, PULL_DISABLE, 10K) +#define GPIO_FUNC3 GPIO_FUNC(3, PULL_DISABLE, 10K) +#define GPIO_FUNC4 GPIO_FUNC(4, PULL_DISABLE, 10K) +#define GPIO_FUNC5 GPIO_FUNC(5, PULL_DISABLE, 10K) +#define GPIO_FUNC6 GPIO_FUNC(6, PULL_DISABLE, 10K)

+/* ACPI GPIO routing. Assume everything is externally pulled and negative edge

• • triggered. SCI implies WAKE, but WAKE doesn't imply SCI. */

+#define GPIO_ACPI_SCI \

•   { .pad_conf0 = PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT | PAD_FUNC0, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_INPUT, \
  

•     .tne       = 1, \
  

•     .sci       = 1, \
  

•     .wake_en   = 1, }
  

+#define GPIO_ACPI_WAKE \

•   { .pad_conf0 = PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT | PAD_FUNC0, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_INPUT, \
  

•     .tne       = 1, \
  

•     .wake_en   = 1, }
  

+#define GPIO_ACPI_SMI \

•   { .pad_conf0 = PAD_PULL_DISABLE | PAD_CONFIG0_DEFAULT | PAD_FUNC0, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_LEGACY, \
  

•     .io_sel    = GPIO_DIR_INPUT, \
  

•     .tne       = 1, \
  

•     .smi       = 1}
  

+/* End marker */ +#define GPIO_LIST_END 0xffffffff

+#define GPIO_END \

•   {  .pad_conf0 = GPIO_LIST_END }
  

+/* Common default GPIO settings */ +#define GPIO_INPUT GPIO_INPUT_NOPU +#define GPIO_INPUT_LEGACY GPIO_INPUT_LEGACY_NOPU +#define GPIO_INPUT_PU GPIO_INPUT_PU_10K(0) +#define GPIO_INPUT_PD GPIO_INPUT_PD_10K +#define GPIO_NC GPIO_INPUT_PU_10K(0) +#define GPIO_NC1 GPIO_INPUT_PU_10K(1) +#define GPIO_DEFAULT GPIO_FUNC0

+/* 16 DirectIRQs per supported bank */ +#define GPIO_MAX_DIRQS 16

+/* Most pins are GPIO function 0. Some banks have a range of pins with GPIO

• • function 1. Indicate first / last GPIOs with function 1. */

Multi-line comment style should always be:

/* * Most pins ... * ... */

Unfortunately this is slightly different from coreboot.

+#define GPIO_NONE 255 +/* All NCORE GPIOs are function 0 */ +#define GPNCORE_GPIO_F1_RANGE_START GPIO_NONE +#define GPNCORE_GPIO_F1_RANGE_END GPIO_NONE +/* SCORE GPIO [92:93] are function 1 */ +#define GPSCORE_GPIO_F1_RANGE_START 92 +#define GPSCORE_GPIO_F1_RANGE_END 93 +/* SSUS GPIO [11:21] are function 1 */ +#define GPSSUS_GPIO_F1_RANGE_START 11 +#define GPSSUS_GPIO_F1_RANGE_END 21

+struct __packed byt_gpio_map {

•   u32 pad_conf0;
  

•   u32 pad_conf1;
  

•   u32 pad_val;
  

•   u32 use_sel:1;
  

•   u32 io_sel:1;
  

•   u32 gp_lvl:1;
  

•   u32 tpe:1;
  

•   u32 tne:1;
  

•   u32 wake_en:1;
  

•   u32 smi:1;
  

•   u32 is_gpio:1;
  

•   u32 sci:1;
  

+};

+struct byt_gpio_config {

•   const struct byt_gpio_map *ncore;
  

•   const struct byt_gpio_map *score;
  

•   const struct byt_gpio_map *ssus;
  

•   const u8 (*core_dirq)[GPIO_MAX_DIRQS];
  

•   const u8 (*sus_dirq)[GPIO_MAX_DIRQS];
  

+};

+/* Description of GPIO 'bank' ex. {ncore, score. ssus} */ +struct gpio_bank {

•   const int gpio_count;
  

•   const u8 *gpio_to_pad;
  

•   const int legacy_base;
  

•   const unsigned long pad_base;
  

•   const u8 has_wake_en:1;
  

•   const u8 gpio_f1_range_start;
  

•   const u8 gpio_f1_range_end;
  

+};

+/* Function to call to setup the GPIOs */ +void setup_soc_gpios(struct byt_gpio_config *config);

+/* Functions / defines for changing GPIOs in romstage */ +/* SCORE Pad definitions. */ +#define UART_RXD_PAD 82 +#define UART_TXD_PAD 83 +#define PCU_SMB_CLK_PAD 88 +#define PCU_SMB_DATA_PAD 90

+static inline unsigned int score_pconf0(int pad_num) +{

•   return GPSCORE_PAD_BASE + pad_num * 16;
  

+}

+static inline unsigned int ssus_pconf0(int pad_num) +{

•   return GPSSUS_PAD_BASE + pad_num * 16;
  

+}

+static inline void score_select_func(int pad, int func) +{

•   uint32_t reg;
  

•   uint32_t pconf0_addr = score_pconf0(pad);
  

•   reg = readl(pconf0_addr);
  

•   reg &= ~0x7;
  

•   reg |= func & 0x7;
  

•   writel(pconf0_addr, reg);
  

+}

+static inline void ssus_select_func(int pad, int func) +{

•   uint32_t reg;
  

•   uint32_t pconf0_addr = ssus_pconf0(pad);
  

•   reg = readl(pconf0_addr);
  

•   reg &= ~0x7;
  

•   reg |= func & 0x7;
  

•   writel(pconf0_addr, reg);
  

You can replace these 4 lines with:

clrsetbits_le32(pconf0_addr, 7, func);

Also should 7 be in a #define somewhere as a MASK?

+}

+/* These functions require that the input pad be configured as an input GPIO */ +static inline int score_get_gpio(int pad) +{

•   uint32_t val_addr = score_pconf0(pad) + PAD_VAL_REG;
  

•   return readl(val_addr) & PAD_VAL_HIGH;
  

+}

+static inline int ssus_get_gpio(int pad) +{

•   uint32_t val_addr = ssus_pconf0(pad) + PAD_VAL_REG;
  

•   return readl(val_addr) & PAD_VAL_HIGH;
  

+}

Can you add a comment here as to what ssus means?

+static inline void ssus_disable_internal_pull(int pad) +{

•   const uint32_t pull_mask = ~(0xf << 7);
  

•   writel(ssus_pconf0(pad), readl(ssus_pconf0(pad)) & pull_mask);
  

+}

#endif /* _X86_ARCH_GPIO_H_ */ diff --git a/arch/x86/include/asm/arch-baytrail/iomap.h b/arch/x86/include/asm/arch-baytrail/iomap.h new file mode 100644 index 0000000..9624929 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/iomap.h @@ -0,0 +1,73 @@ +/*

• • From Coreboot file of same name

Yes like this!

• • Copyright (C) 2014 Google, Inc
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef _BAYTRAIL_IOMAP_H_ +#define _BAYTRAIL_IOMAP_H_

+/*

• • Memory Mapped IO bases.
• */

+/* PCI Configuration Space */ +#define MCFG_BASE_ADDRESS CONFIG_MMCONF_BASE_ADDRESS +#define MCFG_BASE_SIZE 0x10000000

+/* Transactions in this range will abort */ +#define ABORT_BASE_ADDRESS 0xfeb00000 +#define ABORT_BASE_SIZE 0x00100000

+/* Power Management Controller */ +#define PMC_BASE_ADDRESS 0xfed03000 +#define PMC_BASE_SIZE 0x400

+/* IO Memory */ +#define IO_BASE_ADDRESS 0xfed0c000 +#define IO_BASE_OFFSET_GPSCORE 0x0000 +#define IO_BASE_OFFSET_GPNCORE 0x1000 +#define IO_BASE_OFFSET_GPSSUS 0x2000 +#define IO_BASE_SIZE 0x4000

+/* Intel Legacy Block */ +#define ILB_BASE_ADDRESS 0xfed08000 +#define ILB_BASE_SIZE 0x400

+/* SPI Bus */ +#define SPI_BASE_ADDRESS 0xfed01000 +#define SPI_BASE_SIZE 0x400

+/* MODPHY */ +#define MPHY_BASE_ADDRESS 0xfef00000 +#define MPHY_BASE_SIZE 0x100000

+/* Power Management Unit */ +#define PUNIT_BASE_ADDRESS 0xfed05000 +#define PUNIT_BASE_SIZE 0x800

+/* Root Complex Base Address */ +#define RCBA_BASE_ADDRESS 0xfed1c000 +#define RCBA_BASE_SIZE 0x400

Quark has some of these in Kconfig. Can you move these to common arch/x86/Kconfig and use them there? Then you can add the values you want in either arch/x86/baytrail/Kconig or the individual config files in configs/.

+/* High Performance Event Timer */ +#define HPET_BASE_ADDRESS 0xfed00000 +#define HPET_BASE_SIZE 0x400

+/* Temporary Base Address */ +#define TEMP_BASE_ADDRESS 0xfd000000

+/*

• • IO Port bases.
• */

+#define ACPI_BASE_ADDRESS 0x0400 +#define ACPI_BASE_SIZE 0x80

+#define GPIO_BASE_ADDRESS 0x0500 +#define GPIO_BASE_SIZE 0x100

+#define SMBUS_BASE_ADDRESS 0xefa0

+#endif diff --git a/arch/x86/include/asm/arch-baytrail/irq.h b/arch/x86/include/asm/arch-baytrail/irq.h new file mode 100644 index 0000000..d4d3612 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/irq.h @@ -0,0 +1,119 @@ +/*

• • From Coreboot file of same name
• • Copyright (C) 2014 Google, Inc
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef _BAYTRAIL_IRQ_H_ +#define _BAYTRAIL_IRQ_H_

+#define PIRQA_APIC_IRQ 16 +#define PIRQB_APIC_IRQ 17 +#define PIRQC_APIC_IRQ 18 +#define PIRQD_APIC_IRQ 19 +#define PIRQE_APIC_IRQ 20 +#define PIRQF_APIC_IRQ 21 +#define PIRQG_APIC_IRQ 22 +#define PIRQH_APIC_IRQ 23 +/* The below IRQs are for when devices are in ACPI mode. Active low. */ +#define LPE_DMA0_IRQ 24 +#define LPE_DMA1_IRQ 25 +#define LPE_SSP0_IRQ 26 +#define LPE_SSP1_IRQ 27 +#define LPE_SSP2_IRQ 28 +#define LPE_IPC2HOST_IRQ 29 +#define LPSS_I2C1_IRQ 32 +#define LPSS_I2C2_IRQ 33 +#define LPSS_I2C3_IRQ 34 +#define LPSS_I2C4_IRQ 35 +#define LPSS_I2C5_IRQ 36 +#define LPSS_I2C6_IRQ 37 +#define LPSS_I2C7_IRQ 38 +#define LPSS_HSUART1_IRQ 39 +#define LPSS_HSUART2_IRQ 40 +#define LPSS_SPI_IRQ 41 +#define LPSS_DMA1_IRQ 42 +#define LPSS_DMA2_IRQ 43 +#define SCC_EMMC_IRQ 44 +#define SCC_SDIO_IRQ 46 +#define SCC_SD_IRQ 47 +#define GPIO_NC_IRQ 48 +#define GPIO_SC_IRQ 49 +#define GPIO_SUS_IRQ 50 +/* GPIO direct / dedicated IRQs. */ +#define GPIO_S0_DED_IRQ_0 51 +#define GPIO_S0_DED_IRQ_1 52 +#define GPIO_S0_DED_IRQ_2 53 +#define GPIO_S0_DED_IRQ_3 54 +#define GPIO_S0_DED_IRQ_4 55 +#define GPIO_S0_DED_IRQ_5 56 +#define GPIO_S0_DED_IRQ_6 57 +#define GPIO_S0_DED_IRQ_7 58 +#define GPIO_S0_DED_IRQ_8 59 +#define GPIO_S0_DED_IRQ_9 60 +#define GPIO_S0_DED_IRQ_10 61 +#define GPIO_S0_DED_IRQ_11 62 +#define GPIO_S0_DED_IRQ_12 63 +#define GPIO_S0_DED_IRQ_13 64 +#define GPIO_S0_DED_IRQ_14 65 +#define GPIO_S0_DED_IRQ_15 66 +#define GPIO_S5_DED_IRQ_0 67 +#define GPIO_S5_DED_IRQ_1 68 +#define GPIO_S5_DED_IRQ_2 69 +#define GPIO_S5_DED_IRQ_3 70 +#define GPIO_S5_DED_IRQ_4 71 +#define GPIO_S5_DED_IRQ_5 72 +#define GPIO_S5_DED_IRQ_6 73 +#define GPIO_S5_DED_IRQ_7 74 +#define GPIO_S5_DED_IRQ_8 75 +#define GPIO_S5_DED_IRQ_9 76 +#define GPIO_S5_DED_IRQ_10 77 +#define GPIO_S5_DED_IRQ_11 78 +#define GPIO_S5_DED_IRQ_12 79 +#define GPIO_S5_DED_IRQ_13 80 +#define GPIO_S5_DED_IRQ_14 81 +#define GPIO_S5_DED_IRQ_15 82

Would an enum be better here and some of the others below?

+/* DIRQs - Two levels of expansion to evaluate to numeric constants for ASL. */ +#define _GPIO_S0_DED_IRQ(slot) GPIO_S0_DED_IRQ_##slot +#define _GPIO_S5_DED_IRQ(slot) GPIO_S5_DED_IRQ_##slot +#define GPIO_S0_DED_IRQ(slot) _GPIO_S0_DED_IRQ(slot) +#define GPIO_S5_DED_IRQ(slot) _GPIO_S5_DED_IRQ(slot)

+/* PIC IRQ settings. */ +#define PIRQ_PIC_IRQDISABLE 0x0 +#define PIRQ_PIC_IRQ3 0x3 +#define PIRQ_PIC_IRQ4 0x4 +#define PIRQ_PIC_IRQ5 0x5 +#define PIRQ_PIC_IRQ6 0x6 +#define PIRQ_PIC_IRQ7 0x7 +#define PIRQ_PIC_IRQ9 0x9 +#define PIRQ_PIC_IRQ10 0xa +#define PIRQ_PIC_IRQ11 0xb +#define PIRQ_PIC_IRQ12 0xc +#define PIRQ_PIC_IRQ14 0xe +#define PIRQ_PIC_IRQ15 0xf

+/* Overloaded term, but these values determine the per device route. */

per-device

Also can you please drop the period at the end of the comment?

+#define PIRQA 0 +#define PIRQB 1 +#define PIRQC 2 +#define PIRQD 3 +#define PIRQE 4 +#define PIRQF 5 +#define PIRQG 6 +#define PIRQH 7

+/* These registers live behind the ILB_BASE_ADDRESS */

What what are they?

+#define ACTL 0x00 +# define SCIS_MASK 0x07 +# define SCIS_IRQ9 0x00 +# define SCIS_IRQ10 0x01 +# define SCIS_IRQ11 0x02 +# define SCIS_IRQ20 0x04 +# define SCIS_IRQ21 0x05 +# define SCIS_IRQ22 0x06 +# define SCIS_IRQ23 0x07

+#endif /* _BAYTRAIL_IRQ_H_ */ diff --git a/arch/x86/include/asm/arch-baytrail/irqroute.h b/arch/x86/include/asm/arch-baytrail/irqroute.h new file mode 100644 index 0000000..f129880 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/irqroute.h @@ -0,0 +1,67 @@ +/*

• • From Coreboot file of same name
• • Copyright (C) 2014 Google, Inc
• • Copyright (C) 2014 Sage Electronic Engineering, LLC.
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef IRQROUTE_H +#define IRQROUTE_H

+#include <asm/arch/irq.h> +#include <asm/arch/pci_devs.h>

+/*

• *IR02h GFX INT(A) - PIRQ A
• *IR10h EMMC INT(ABCD) - PIRQ DEFG
• *IR11h SDIO INT(A) - PIRQ B
• *IR12h SD INT(A) - PIRQ C
• *IR13h SATA INT(A) - PIRQ D
• *IR14h XHCI INT(A) - PIRQ E
• *IR15h LP Audio INT(A) - PIRQ F
• *IR17h MMC INT(A) - PIRQ F
• *IR18h SIO INT(ABCD) - PIRQ BADC
• *IR1Ah TXE INT(A) - PIRQ F
• *IR1Bh HD Audio INT(A) - PIRQ G
• *IR1Ch PCIe INT(ABCD) - PIRQ EFGH
• *IR1Dh EHCI INT(A) - PIRQ D
• *IR1Eh SIO INT(ABCD) - PIRQ BDEF
• *IR1Fh LPC INT(ABCD) - PIRQ HGBC
• */

+#define PCI_DEV_PIRQ_ROUTES \

•   PCI_DEV_PIRQ_ROUTE(GFX_DEV,    A, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(EMMC_DEV,   D, E, F, G), \
  

•   PCI_DEV_PIRQ_ROUTE(SDIO_DEV,   B, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SD_DEV,     C, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SATA_DEV,   D, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(XHCI_DEV,   E, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(LPE_DEV,    F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(MMC45_DEV,  F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SIO1_DEV,   B, A, D, C), \
  

•   PCI_DEV_PIRQ_ROUTE(TXE_DEV,    F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(HDA_DEV,    G, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(PCIE_DEV,   E, F, G, H), \
  

•   PCI_DEV_PIRQ_ROUTE(EHCI_DEV,   D, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SIO2_DEV,   B, D, E, F), \
  

•   PCI_DEV_PIRQ_ROUTE(PCU_DEV,    H, G, B, C)
  

Is this actually used? In general I think this sort of monstrosity is better off in a C file.

+/*

• • Route each PIRQ[A-H] to a PIC IRQ[0-15]
• • Reserved: 0, 1, 2, 8, 13
• • PS2 keyboard: 12
• • ACPI/SCI: 9
• • Floppy: 6
• */

+#define PIRQ_PIC_ROUTES \

•   PIRQ_PIC(A,  4), \
  

•   PIRQ_PIC(B,  5), \
  

•   PIRQ_PIC(C,  7), \
  

•   PIRQ_PIC(D, 10), \
  

•   PIRQ_PIC(E, 11), \
  

•   PIRQ_PIC(F, 12), \
  

•   PIRQ_PIC(G, 14), \
  

•   PIRQ_PIC(H, 15)
  

+#endif /* IRQROUTE_H */ diff --git a/arch/x86/include/asm/arch-baytrail/pci_devs.h b/arch/x86/include/asm/arch-baytrail/pci_devs.h new file mode 100644 index 0000000..579a228 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/pci_devs.h @@ -0,0 +1,144 @@ +/*

• • From Coreboot file of same name
• • Copyright (C) 2014 Google, Inc
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef _BAYTRAIL_PCI_DEVS_H_ +#define _BAYTRAIL_PCI_DEVS_H_

+/* All these devices live on bus 0 with the associated device and function */

+/* SoC transaction router */ +#define SOC_DEV 0x0 +#define SOC_FUNC 0 +# define SOC_DEVID 0x0f00

+/* Graphics and Display */ +#define GFX_DEV 0x2 +#define GFX_FUNC 0 +# define GFX_DEVID 0x0f31

+/* SDIO Port */ +#define SDIO_DEV 0x11 +#define SDIO_FUNC 0 +# define SDIO_DEVID 0x0f15

+/* SD Port */ +#define SD_DEV 0x12 +#define SD_FUNC 0 +# define SD_DEVID 0x0f16

+/* SATA */ +#define SATA_DEV 0x13 +#define SATA_FUNC 0 +#define IDE1_DEVID 0x0f20 +#define IDE2_DEVID 0x0f21 +#define AHCI1_DEVID 0x0f22 +#define AHCI2_DEVID 0x0f23

+/* xHCI */ +#define XHCI_DEV 0x14 +#define XHCI_FUNC 0 +# define XHCI_DEVID 0x0f35

+/* LPE Audio */ +#define LPE_DEV 0x15 +#define LPE_FUNC 0 +# define LPE_DEVID 0x0f28

+/* MMC Port */ +#define MMC_DEV 0x17 +#define MMC_FUNC 0 +# define MMC_DEVID 0x0f50

+/* Serial IO 1 */ +#define SIO1_DEV 0x18 +# define SIO_DMA1_DEV SIO1_DEV +# define SIO_DMA1_FUNC 0 +# define SIO_DMA1_DEVID 0x0f40 +# define I2C1_DEV SIO1_DEV +# define I2C1_FUNC 1 +# define I2C1_DEVID 0x0f41 +# define I2C2_DEV SIO1_DEV +# define I2C2_FUNC 2 +# define I2C2_DEVID 0x0f42 +# define I2C3_DEV SIO1_DEV +# define I2C3_FUNC 3 +# define I2C3_DEVID 0x0f43 +# define I2C4_DEV SIO1_DEV +# define I2C4_FUNC 4 +# define I2C4_DEVID 0x0f44 +# define I2C5_DEV SIO1_DEV +# define I2C5_FUNC 5 +# define I2C5_DEVID 0x0f45 +# define I2C6_DEV SIO1_DEV +# define I2C6_FUNC 6 +# define I2C6_DEVID 0x0f46 +# define I2C7_DEV SIO1_DEV +# define I2C7_FUNC 7 +# define I2C7_DEVID 0x0f47

+/* Trusted Execution Engine */ +#define TXE_DEV 0x1a +#define TXE_FUNC 0 +# define TXE_DEVID 0x0f18

+/* HD Audio */ +#define HDA_DEV 0x1b +#define HDA_FUNC 0 +# define HDA_DEVID 0x0f04

+/* PCIe Ports */ +#define PCIE_DEV 0x1c +# define PCIE_PORT1_DEV PCIE_DEV +# define PCIE_PORT1_FUNC 0 +# define PCIE_PORT1_DEVID 0x0f48 +# define PCIE_PORT2_DEV PCIE_DEV +# define PCIE_PORT2_FUNC 1 +# define PCIE_PORT2_DEVID 0x0f4a +# define PCIE_PORT3_DEV PCIE_DEV +# define PCIE_PORT3_FUNC 2 +# define PCIE_PORT3_DEVID 0x0f4c +# define PCIE_PORT4_DEV PCIE_DEV +# define PCIE_PORT4_FUNC 3 +# define PCIE_PORT4_DEVID 0x0f4e

+/* EHCI */ +#define EHCI_DEV 0x1d +#define EHCI_FUNC 0 +# define EHCI_DEVID 0x0f34

+/* Serial IO 2 */ +#define SIO2_DEV 0x1e +# define SIO_DMA2_DEV SIO2_DEV +# define SIO_DMA2_FUNC 0 +# define SIO_DMA2_DEVID 0x0f06 +# define PWM1_DEV SIO2_DEV +# define PWM1_FUNC 1 +# define PWM1_DEVID 0x0f08 +# define PWM2_DEV SIO2_DEV +# define PWM2_FUNC 2 +# define PWM2_DEVID 0x0f09 +# define HSUART1_DEV SIO2_DEV +# define HSUART1_FUNC 3 +# define HSUART1_DEVID 0x0f0a +# define HSUART2_DEV SIO2_DEV +# define HSUART2_FUNC 4 +# define HSUART2_DEVID 0x0f0c +# define SPI_DEV SIO2_DEV +# define SPI_FUNC 5 +# define SPI_DEVID 0xf0e

+/* Platform Controller Unit */ +#define PCU_DEV 0x1f +# define LPC_DEV PCU_DEV +# define LPC_FUNC 0 +# define LPC_DEVID 0x0f1c +# define SMBUS_DEV PCU_DEV +# define SMBUS_FUNC 3 +# define SMBUS_DEVID 0x0f12

I suppose this will move to device tree one day (one PCI support merges) but for now this looks fine. You could tab out the values if you could be bothered...

+#endif /* _BAYTRAIL_PCI_DEVS_H_ */ diff --git a/arch/x86/include/asm/arch-baytrail/pmc.h b/arch/x86/include/asm/arch-baytrail/pmc.h new file mode 100644 index 0000000..16d4ce7 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/pmc.h @@ -0,0 +1,253 @@ +/*

• • From Coreboot file of same name
• • Copyright (C) 2014 Google, Inc
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef _BAYTRAIL_PMC_H_ +#define _BAYTRAIL_PMC_H_

Remove extra blank line

+#define IOCOM1 0x3f8

See ibmpc.h

+/* Memory mapped IO registers behind PMC_BASE_ADDRESS */ +#define PRSTS 0x00 +# define PMC_WDT_STS (1 << 15) +# define SEC_GBLRST_STS (1 << 7) +# define SEC_WDT_STS (1 << 6) +# define WOL_OVR_WK_STS (1 << 5) +# define PMC_WAKE_STS (1 << 4) +#define PMC_CFG 0x08 +# define SPS (1 << 5) +# define NO_REBOOT (1 << 4) +# define SX_ENT_TO_EN (1 << 3) +# define TIMING_T581_SHIFT (0) +# define TIMING_T581_MASK (3 << TIMING_T581_SHIFT) +# define TIMING_T581_10US (0 << TIMING_T581_SHIFT) +# define TIMING_T581_100US (1 << TIMING_T581_SHIFT) +# define TIMING_T581_1MS (2 << TIMING_T581_SHIFT) +# define TIMING_T581_10MS (3 << TIMING_T581_SHIFT)

In general I would prefer:

enum { TIMING_T581_10US = 0, TIMING_T581_100US, ... }

The the caller can do the shift. The mask is fine though. You don't have to change it here, it's just the style I'm trying to keep most of the time.

+#define VLV_PM_STS 0x0c +# define PMC_MSG_FULL_STS (1 << 24) +# define PMC_MSG_4_FULL_STS (1 << 23) +# define PMC_MSG_3_FULL_STS (1 << 22) +# define PMC_MSG_2_FULL_STS (1 << 21) +# define PMC_MSG_1_FULL_STS (1 << 20) +# define CODE_REQ (1 << 8) +# define HPR_ENT_TO (1 << 2) +# define SX_ENT_TO (1 << 1) +#define GEN_PMCON1 0x20 +# define UART_EN (1 << 24) +# define DISB (1 << 23) +# define MEM_SR (1 << 21) +# define SRS (1 << 20) +# define CTS (1 << 19) +# define MS4V (1 << 18) +# define PWR_FLR (1 << 16) +# define PME_B0_S5_DIS (1 << 15) +# define SUS_PWR_FLR (1 << 14) +# define WOL_EN_OVRD (1 << 13) +# define DIS_SLP_X_STRCH_SUS_UP (1 << 12) +# define GEN_RST_STS (1 << 9) +# define RPS (1 << 2) +# define AFTERG3_EN (1 << 0) +#define GEN_PMCON2 0x24 +# define SLPSX_STR_POL_LOCK (1 << 18) +# define BIOS_PCI_EXP_EN (1 << 10) +# define PWRBTN_LVL (1 << 9) +# define SMI_LOCK (1 << 4) +#define ETR 0x48 +# define CF9LOCK (1 << 31) +# define LTR_DEF (1 << 22) +# define IGNORE_HPET (1 << 21) +# define CF9GR (1 << 20) +# define CWORWRE (1 << 18) +#define FUNC_DIS 0x34 +# define SIO_DMA2_DIS (1 << 0) +# define PWM1_DIS (1 << 1) +# define PWM2_DIS (1 << 2) +# define HSUART1_DIS (1 << 3) +# define HSUART2_DIS (1 << 4) +# define SPI_DIS (1 << 5) +# define SDIO_DIS (1 << 9) +# define SD_DIS (1 << 10) +# define MMC_DIS (1 << 11) +# define HDA_DIS (1 << 12) +# define LPE_DIS (1 << 13) +# define OTG_DIS (1 << 14) +# define XHCI_DIS (1 << 15) +# define SATA_DIS (1 << 17) +# define EHCI_DIS (1 << 18) +# define TXE_DIS (1 << 19) +# define PCIE_PORT1_DIS (1 << 20) +# define PCIE_PORT2_DIS (1 << 21) +# define PCIE_PORT3_DIS (1 << 22) +# define PCIE_PORT4_DIS (1 << 23) +# define SIO_DMA1_DIS (1 << 24) +# define I2C1_DIS (1 << 25) +# define I2C2_DIS (1 << 26) +# define I2C3_DIS (1 << 27) +# define I2C4_DIS (1 << 28) +# define I2C5_DIS (1 << 29) +# define I2C6_DIS (1 << 30) +# define I2C7_DIS (1 << 31) +#define FUNC_DIS2 0x38 +# define USH_SS_PHY_DIS (1 << 2) +# define OTG_SS_PHY_DIS (1 << 1) +# define SMBUS_DIS (1 << 0) +#define GPIO_ROUT 0x58 +# define ROUTE_MASK 3 +# define ROUTE_NONE 0 +# define ROUTE_SMI 1 +# define ROUTE_SCI 2 +#define PLT_CLK_CTL_0 0x60 +#define PLT_CLK_CTL_1 0x64 +#define PLT_CLK_CTL_2 0x68 +#define PLT_CLK_CTL_3 0x6c +#define PLT_CLK_CTL_4 0x70 +#define PLT_CLK_CTL_5 0x74 +# define CLK_FREQ_25MHZ (0x0 << 2) +# define CLK_FREQ_19P2MHZ (0x1 << 2) +# define CLK_CTL_D3_LPE (0x0 << 0) +# define CLK_CTL_ON (0x1 << 0) +# define CLK_CTL_OFF (0x2 << 0) +#define PME_STS 0xc0 +#define GPE_LEVEL_EDGE 0xc4 +# define GPE_EDGE 0 +# define GPE_LEVEL 1 +#define GPE_POLARITY 0xc8 +# define GPE_ACTIVE_HIGH 1 +# define GPE_ACTIVE_LOW 0 +#define LOCK 0xcc

+/* IO Mapped registers behind ACPI_BASE_ADDRESS */

I/O-mapped registers

(i.e. you should have a hyphen when you create an adjective like that)

+#define PM1_STS 0x00 +#define WAK_STS (1 << 15) +#define PCIEXPWAK_STS (1 << 14) +#define USB_STS (1 << 13) +#define PRBTNOR_STS (1 << 11) +#define RTC_STS (1 << 10) +#define PWRBTN_STS (1 << 8) +#define GBL_STS (1 << 5) +#define TMROF_STS (1 << 0) +#define PM1_EN 0x02 +#define PCIEXPWAK_DIS (1 << 14) +#define USB_WAKE_EN (1 << 13) +#define RTC_EN (1 << 10) +#define PWRBTN_EN (1 << 8) +#define GBL_EN (1 << 5) +#define TMROF_EN (1 << 0) +#define PM1_CNT 0x04 +#define SLP_EN (1 << 13) +#define SLP_TYP_SHIFT 10 +#define SLP_TYP (7 << SLP_TYP_SHIFT) +#define SLP_TYP_S0 0 +#define SLP_TYP_S1 1 +#define SLP_TYP_S3 5 +#define SLP_TYP_S4 6 +#define SLP_TYP_S5 7 +#define GBL_RLS (1 << 2) +#define BM_RLD (1 << 1) +#define SCI_EN (1 << 0) +#define PM1_TMR 0x08 +#define GPE0_STS 0x20 +#define CORE_GPIO_STS7 (1 << 31) +#define CORE_GPIO_STS6 (1 << 30) +#define CORE_GPIO_STS5 (1 << 29) +#define CORE_GPIO_STS4 (1 << 28) +#define CORE_GPIO_STS3 (1 << 27) +#define CORE_GPIO_STS2 (1 << 26) +#define CORE_GPIO_STS1 (1 << 25) +#define CORE_GPIO_STS0 (1 << 24) +#define SUS_GPIO_STS7 (1 << 23) +#define SUS_GPIO_STS6 (1 << 22) +#define SUS_GPIO_STS5 (1 << 21) +#define SUS_GPIO_STS4 (1 << 20) +#define SUS_GPIO_STS3 (1 << 19) +#define SUS_GPIO_STS2 (1 << 18) +#define SUS_GPIO_STS1 (1 << 17) +#define SUS_GPIO_STS0 (1 << 16) +#define PME_B0_STS (1 << 13) +#define BATLOW_STS (1 << 10) +#define PCI_EXP_STS (1 << 9) +#define PCIE_WAKE3_STS (1 << 8) +#define PCIE_WAKE2_STS (1 << 7) +#define PCIE_WAKE1_STS (1 << 6) +#define GUNIT_SCI_STS (1 << 5) +#define PUNIT_SCI_STS (1 << 4) +#define PCIE_WAKE0_STS (1 << 3) +#define SWGPE_STS (1 << 2) +#define HOT_PLUG_STS (1 << 1) +#define GPE0_EN 0x28 +#define CORE_GPIO_EN7 (1 << 31) +#define CORE_GPIO_EN6 (1 << 30) +#define CORE_GPIO_EN5 (1 << 29) +#define CORE_GPIO_EN4 (1 << 28) +#define CORE_GPIO_EN3 (1 << 27) +#define CORE_GPIO_EN2 (1 << 26) +#define CORE_GPIO_EN1 (1 << 25) +#define CORE_GPIO_EN0 (1 << 24) +#define SUS_GPIO_EN7_BIT 23 +#define SUS_GPIO_EN7 (1 << SUS_GPIO_EN7_BIT) +#define SUS_GPIO_EN6_BIT 22 +#define SUS_GPIO_EN6 (1 << SUS_GPIO_EN6_BIT) +#define SUS_GPIO_EN5_BIT 21 +#define SUS_GPIO_EN5 (1 << SUS_GPIO_EN5_BIT) +#define SUS_GPIO_EN4_BIT 20 +#define SUS_GPIO_EN4 (1 << SUS_GPIO_EN4_BIT) +#define SUS_GPIO_EN3_BIT 19 +#define SUS_GPIO_EN3 (1 << SUS_GPIO_EN3_BIT) +#define SUS_GPIO_EN2_BIT 18 +#define SUS_GPIO_EN2 (1 << SUS_GPIO_EN2_BIT) +#define SUS_GPIO_EN1_BIT 17 +#define SUS_GPIO_EN1 (1 << SUS_GPIO_EN1_BIT) +#define SUS_GPIO_EN0_BIT 16 +#define SUS_GPIO_EN0 (1 << SUS_GPIO_EN0_BIT) +#define PME_B0_EN (1 << 13) +#define BATLOW_EN (1 << 10) +#define PCI_EXP_EN (1 << 9) +#define PCIE_WAKE3_EN (1 << 8) +#define PCIE_WAKE2_EN (1 << 7) +#define PCIE_WAKE1_EN (1 << 6) +#define PCIE_WAKE0_EN (1 << 3) +#define SWGPE_EN (1 << 2) +#define HOT_PLUG_EN (1 << 1) +#define _ACPI_ENABLE_WAKE_SUS_GPIO(x) SUS_GPIO_EN##x##_BIT +#define ACPI_ENABLE_WAKE_SUS_GPIO(x) _ACPI_ENABLE_WAKE_SUS_GPIO(x) +#define SMI_EN 0x30 +#define INTEL_USB2_EN (1 << 18) /* Intel-Specific USB2 SMI logic */ +#define USB_EN (1 << 17) /* Legacy USB2 SMI logic */ +#define PERIODIC_EN (1 << 14) /* SMI on PERIODIC_STS in SMI_STS */ +#define TCO_EN (1 << 13) /* Enable TCO Logic (BIOSWE et al) */ +#define BIOS_RLS (1 << 7) /* asserts SCI on bit set */ +#define SWSMI_TMR_EN (1 << 6) /* start software smi timer on bit set */ +#define APMC_EN (1 << 5) /* Writes to APM_CNT cause SMI# */ +#define SLP_SMI_EN (1 << 4) /* Write to SLP_EN in PM1_CNT asserts SMI# */ +#define BIOS_EN (1 << 2) /* Assert SMI# on setting GBL_RLS bit */ +#define EOS (1 << 1) /* End of SMI (deassert SMI#) */ +#define GBL_SMI_EN (1 << 0) /* SMI# generation at all? */ +#define SMI_STS 0x34 +#define ALT_GPIO_SMI 0x38 +#define UPRWC 0x3c +# define UPRWC_WR_EN (1 << 1) /* USB Per-Port Registers Write Enable */ +#define GPE_CTRL 0x40 +#define PM2A_CNT_BLK 0x50 +#define TCO_RLD 0x60 +#define TCO_STS 0x64 +# define SECOND_TO_STS (1 << 17) +# define TCO_TIMEOUT (1 << 3) +#define TCO1_CNT 0x68 +# define TCO_LOCK (1 << 12) +# define TCO_TMR_HALT (1 << 11) +#define TCO_TMR 0x70

+/* I/O ports */ +#define RST_CNT 0xcf9 +# define FULL_RST (1 << 3) +# define RST_CPU (1 << 2) +# define SYS_RST (1 << 1)

+#endif /* _BAYTRAIL_PMC_H_ */

diff --git a/board/intel/minnowmax/minnowmax.c b/board/intel/minnowmax/minnowmax.c index 6e82b16..c7b53f5 100644 --- a/board/intel/minnowmax/minnowmax.c +++ b/board/intel/minnowmax/minnowmax.c @@ -9,15 +9,227 @@ #include <asm/pnp_def.h> #include <netdev.h> #include <smsc_lpc47m.h> +#include <asm/arch/gpio.h>

#define SERIAL_DEV PNP_DEV(0x2e, 4)

DECLARE_GLOBAL_DATA_PTR;

+/*

• • For multiplexed functions, look in EDS:
• • 10.3 Ball Name and Function by Location
• • The pads list is in the BWG_VOL2 Rev1p2:
• • Note that Pad # is not the same as GPIO#
• • 37 GPIO Handling:
• • Table 37-1. SCORE Pads List
• • Table 37-2. SSUSORE Pads List
• */

+/* NCORE GPIOs */ +static const struct byt_gpio_map gpncore_gpio_map[] = {

•   GPIO_FUNC2,     /* GPIO_S0_NC[00] - HDMI_HPD */
  

•   GPIO_FUNC2,     /* GPIO_S0_NC[01] - HDMI_DDCDAT */
  

•   GPIO_FUNC2,     /* GPIO_S0_NC[02] - HDMI_DDCCLK */
  

•   GPIO_NC,        /* GPIO_S0_NC[03] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[04] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[05] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[06] - No Connect */
  

•   GPIO_FUNC2,     /* GPIO_S0_NC[07] - DDI1_DDCDAT */
  

•   GPIO_NC,        /* GPIO_S0_NC[08] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[09] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[10] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_NC[11] - No Connect */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S0_NC[12] - TP15 */
  

•   GPIO_NC, /* GPIO_S0_NC[13] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[14] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[15] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[16] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[17] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[18] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[19] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[20] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[21] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[22] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[23] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[24] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[25] - No Connect */
  

•   GPIO_NC, /* GPIO_S0_NC[26] - No Connect */
  

•   GPIO_END
  

+};

+/* SCORE GPIOs (GPIO_S0_SC_XX)*/ +static const struct byt_gpio_map gpscore_gpio_map[] = {

•   GPIO_FUNC1,     /* GPIO_S0_SC[000] - SATA_GP0 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[001] - SATA_GP1 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[002] - SATA_LED_B */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[003] - PCIE_CLKREQ_0 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[004] - PCIE_CLKREQ_1 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[005] - PCIE_CLKREQ_2 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[006] - PCIE_CLKREQ_3 */
  

•   GPIO_FUNC2,     /* GPIO_S0_SC[007] - SD3_WP */
  

•   GPIO_NC,        /* GPIO_S0_SC[008] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[009] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[010] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[011] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[012] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[013] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[014] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[015] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[016] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[017] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[018] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[019] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[020] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[021] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[022] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[023] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[024] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[025] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[026] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[027] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[028] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[029] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[030] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[031] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[032] - No Connect */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[033] - SD3_CLK */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[034] - SD3_D0 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[035] - SD3_D1 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[036] - SD3_D2 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[037] - SD3_D3 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[038] - SD3_CD# */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[039] - SD3_CMD */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[040] - TP12 (SD3_1P8EN) */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[041] - TP11 (/SD3_PWREN) */
  

•   GPIO_NC,        /* GPIO_S0_SC[042] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[043] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[044] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[045] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[046] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[047] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[048] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[049] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[050] - No Connect */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[051] - PCU_SMB_DATA */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[052] - PCU_SMB_CLK */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[053] - PCU_SMB_ALERT */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[054] - ILB_8254_SPKR */
  

•   /* GPIO_S0_SC[055] - TP8 (GPIO_S0_SC_55) */
  

•   GPIO_FUNC(0, PULL_UP, 20K),
  

•   GPIO_FUNC0,     /* GPIO_S0_SC[056] - GPIO_S0_SC_56 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[057] - PCU_UART3_TXD */
  

•   /* GPIO_S0_SC[058] - TP9 (GPIO_S0_SC_58) */
  

•   GPIO_FUNC(0, PULL_UP, 20K),
  

•   GPIO_FUNC0,     /* GPIO_S0_SC[059] - HDMI_DCDC_ENB */
  

•   GPIO_FUNC0,     /* GPIO_S0_SC[060] - HDMI_LDSW_ENB */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[061] - PCU_UART3_RXD */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[062] - LPE_I2S_CLK */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[063] - LPE_I2S_FRM */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[064] - LPE_I2S_DATIN */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[065] - LPE_I2S_DATOUT */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[066] - SOC_SIO_SPI_CS1 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[067] - SOC_SIO_SPI_MISO */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[068] - SOC_SIO_SPI_MOSI */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[069] - SOC_SIO_SPI_CLK */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[070] - SIO_UART1_RXD */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[071] - SIO_UART1_TXD */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[072] - SIO_UART1_RTSB */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[073] - SIO_UART1_CTSB */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[074] - SIO_UART2_RXD */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[075] - SIO_UART2_TXD */
  

•   GPIO_NC,        /* GPIO_S0_SC[076] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[077] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[078] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[079] - No Connect */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[080] - TP6 (SIO_I2C1_SDA) */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[081] - TP5 (SIO_I2C1_SCL) */
  

•   GPIO_NC,        /* GPIO_S0_SC[082] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[083] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[084] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[085] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[086] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[087] - No Connect */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[088] - LSS_I2C_SDA */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[089] - LSS_I2C_SCL */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[090] - EXP_I2C_SDA */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[091] - EXP_I2C_SCL */
  

•   GPIO_FUNC(1, PULL_UP, 20K),     /* GPIO_S0_SC[092] - TP13 */
  

•   GPIO_FUNC(1, PULL_UP, 20K),     /* GPIO_S0_SC[093] - TP16 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[094] - SOC_PWM0 */
  

•   GPIO_FUNC1,     /* GPIO_S0_SC[095] - SOC_PWM1 */
  

•   GPIO_NC,        /* GPIO_S0_SC[096] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[097] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[098] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[099] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[100] - No Connect */
  

•   GPIO_NC,        /* GPIO_S0_SC[101] - No Connect */
  

•   GPIO_END
  

+};

+/* SSUS GPIOs (GPIO_S5) */ +static const struct byt_gpio_map gpssus_gpio_map[] = {

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[00] - SOC_GPIO_S5_0 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[01] - SOC_GPIO_S5_1 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[02] - SOC_GPIO_S5_2 */
  

•   GPIO_FUNC6,     /* GPIO_S5[03] - mPCIE_WAKEB */
  

•   GPIO_NC,        /* GPIO_S5[04] - No Connect */
  

•   GPIO_INPUT,     /* GPIO_S5[05] - BOM_OP1 */
  

•   GPIO_INPUT,     /* GPIO_S5[06] - BOM_OP2 */
  

•   GPIO_INPUT,     /* GPIO_S5[07] - BOM_OP3 */
  

•   GPIO_OUT_HIGH,  /* GPIO_S5[08] - SOC_USB_HOST_EN0 */
  

•   GPIO_OUT_HIGH,  /* GPIO_S5[09] - SOC_USB_HOST_EN1 */
  

•   GPIO_OUT_HIGH,  /* GPIO_S5[10] - GPIO_S5_10_UNLOCK */
  

•   GPIO_FUNC0,     /* GPIO_S5[11] - SUSPWRDNACK (TP14) */
  

•   GPIO_FUNC0,     /* GPIO_S5[12] - PMC_SUSCLK0 */
  

•   GPIO_FUNC1,     /* GPIO_S5[13] - PMC_SLP_S0IX (TP10) */
  

•   GPIO_FUNC1,     /* GPIO_S5[14] - GPIO_S514_J20 */
  

•   GPIO_FUNC0,     /* GPIO_S5[15] - PMC_PCIE_WAKE_R */
  

•   GPIO_FUNC0,     /* GPIO_S5[16] - PMC_PWRBTN */
  

•   GPIO_NC1,       /* GPIO_S5[17] - No Connect */
  

•   GPIO_FUNC1,     /* GPIO_S5[18] - LPCPD_L (TP7) */
  

•   GPIO_FUNC0,     /* GPIO_S5[19] - SOC_USB_HOST_OC0 */
  

•   GPIO_FUNC0,     /* GPIO_S5[20] - SOC_USB_HOST_OC1 */
  

•   GPIO_FUNC0,     /* GPIO_S5[21] - SOC_SPI_CS1B */
  

•   GPIO_NC,        /* GPIO_S5[22] - No Connect */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[23] - XDP_H_OBSDATA_A0 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[24] - XDP_H_OBSDATA_A1 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[25] - XDP_H_OBSDATA_A2 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[26] - XDP_H_OBSDATA_A3 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[27] - EXP_GPIO1 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[28] - EXP_GPIO2 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[29] - EXP_GPIO3 */
  

•   GPIO_FUNC(0, PULL_UP, 20K),     /* GPIO_S5[30] - EXP_GPIO4 */
  

•   GPIO_NC,        /* GPIO_S5[31] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[32] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[33] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[34] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[35] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[36] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[37] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[38] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[39] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[40] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[41] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[42] - No Connect */
  

•   GPIO_NC,        /* GPIO_S5[43] - No Connect */
  

•   GPIO_END
  

Yes I see what you mean about wanting to move this to the device tree.

+};

+static struct byt_gpio_config gpio_config = {

•   .ncore = gpncore_gpio_map,
  

•   .score = gpscore_gpio_map,
  

•   .ssus = gpssus_gpio_map,
  

•   .core_dirq = NULL,
  

•   .sus_dirq = NULL,
  

You can drop these last two, they will be NULL anyway.

+};

int board_early_init_f(void) { lpc47m_enable_serial(SERIAL_DEV, UART0_BASE);

•   setup_soc_gpios(&gpio_config);
  

•   return 0;
  

}

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure

* TODO(you@email.com): Add a ...

• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

#endif /* __CONFIG_H */

2.1.4

Regards, Simon

--- Added to GNATS database as unassigned-patches/131

Responsible: patch-coord Message-Id: CAPnjgZ2wsg+0d-WXgbwN7Gvm_9TRX9y+Exo6e4a0xW6FAx6_AA@mail.gmail.com In-Reply-To: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr Patch-Date: Wed Feb 18 05:04:46 +0100 2015

Hi Simon,

With a little bit of delay here are the responses ... :)

On 02/17/2015 08:04 PM, Simon Glass wrote:

Hi Gabriel,

On 15 February 2015 at 14:55, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

Thanks for the patch!

I have mostly nits except for one comment about register access which is different in U-Boot...

I read all the comments and I agree on almost all of them but I have some questions.

...

•           /* Add correct func to GPIO pad config */
  

•           pad_conf0 = config->pad_conf0;
  

•           if (config->is_gpio) {
  

•                   if (gpio >= bank->gpio_f1_range_start &&
  

•                       gpio <= bank->gpio_f1_range_end)
  

•                           pad_conf0 |= PAD_FUNC1;
  

•                   else
  

•                           pad_conf0 |= PAD_FUNC0;
  

•           }
  

•           writel(reg + PAD_CONF0_REG, pad_conf0);
  

•           writel(reg + PAD_CONF1_REG, config->pad_conf1);
  

•           writel(reg + PAD_VAL_REG, config->pad_val);
  

•   }
  

•   if (bank->legacy_base != GP_LEGACY_BASE_NONE)
  

•           for (set = 0; set <= (bank->gpio_count - 1) / 32; ++set) {
  

•                   reg = bank->legacy_base + 0x20 * set;
  

•                   outl(use_sel[set], reg + LEGACY_USE_SEL_REG);
  

•                   outl(io_sel[set], reg + LEGACY_IO_SEL_REG);
  

•                   outl(gp_lvl[set], reg + LEGACY_GP_LVL_REG);
  

•                   outl(tpe[set], reg + LEGACY_TPE_REG);
  

•                   outl(tne[set], reg + LEGACY_TNE_REG);
  

•                   /* TS registers are WOC  */
  

If you know what this comment means, please spell it out without abbreviations.

Actually, I don't know the meaning of WOC and I couldn't find a definition in the datasheet.

...

•                   outl(0, reg + LEGACY_TS_REG);
  

•                   if (bank->has_wake_en)
  

•                           outl(wake_en[set], reg + LEGACY_WAKE_EN_REG);
  

•           }
  

+}

+static void setup_gpio_route(const struct byt_gpio_map *sus,

•           const struct byt_gpio_map *core)
  

+{

•   uint32_t route_reg = 0;
  

•   int i;
  

•   for (i = 0; i < 8; i++) {
  

•           /* SMI takes precedence and wake_en implies SCI. */
  

•           if (sus[i].smi)
  

•                   route_reg |= ROUTE_SMI << (2 * i);
  

•           else if (sus[i].sci)
  

•                   route_reg |= ROUTE_SCI << (2 * i);
  

•           if (core[i].smi)
  

•                   route_reg |= ROUTE_SMI << (2 * (i + 8));
  

•           else if (core[i].sci)
  

•                   route_reg |= ROUTE_SCI << (2 * (i + 8));
  

•   }
  

What happens to route_reg after this? I don't see it get returned.

I will remove the code, actually it was used when the SMI was enabled.

...

+#define GPIO_LEVEL_LOW 0 +#define GPIO_LEVEL_HIGH 1

+#define GPIO_PEDGE_DISABLE 0 +#define GPIO_PEDGE_ENABLE 1

+#define GPIO_NEDGE_DISABLE 0 +#define GPIO_NEDGE_ENABLE 1

+/* config0[29] - Disable second mask */ +#define PAD_MASK2_DISABLE (1 << 29)

+/* config0[27] - Direct Irq En */ +#define PAD_IRQ_EN (1 << 27)

+/* config0[26] - gd_tne */ +#define PAD_TNE_IRQ (1 << 26)

+/* config0[25] - gd_tpe */ +#define PAD_TPE_IRQ (1 << 25)

+/* config0[24] - Gd Level */ +#define PAD_LEVEL_IRQ (1 << 24) +#define PAD_EDGE_IRQ (0 << 24)

+/* config0[17] - Slow clkgate / glitch filter */ +#define PAD_SLOWGF_ENABLE (1 << 17)

+/* config0[16] - Fast clkgate / glitch filter */ +#define PAD_FASTGF_ENABLE (1 << 16)

+/* config0[15] - Hysteresis enable (inverted) */ +#define PAD_HYST_DISABLE (1 << 15) +#define PAD_HYST_ENABLE (0 << 15)

+/* config0[14:13] - Hysteresis control */ +#define PAD_HYST_CTRL_DEFAULT (2 << 13)

+/* config0[11] - Bypass Flop */ +#define PAD_FLOP_BYPASS (1 << 11) +#define PAD_FLOP_ENABLE (0 << 11)

+/* config0[10:9] - Pull str */ +#define PAD_PU_2K (0 << 9) +#define PAD_PU_10K (1 << 9) +#define PAD_PU_20K (2 << 9) +#define PAD_PU_40K (3 << 9)

+/* config0[8:7] - Pull assign */ +#define PAD_PULL_DISABLE (0 << 7) +#define PAD_PULL_UP (1 << 7) +#define PAD_PULL_DOWN (2 << 7)

+/* config0[2:0] - Func. pin mux */ +#define PAD_FUNC0 0x0 +#define PAD_FUNC1 0x1 +#define PAD_FUNC2 0x2 +#define PAD_FUNC3 0x3 +#define PAD_FUNC4 0x4 +#define PAD_FUNC5 0x5 +#define PAD_FUNC6 0x6

These could be an anonymous enum (optional)

For me, only the PAD_FUNCX could be part of an enum.

...

+/* pad config0 power-on values - We will not often want to change these */ +#define PAD_CONFIG0_DEFAULT (PAD_MASK2_DISABLE | PAD_SLOWGF_ENABLE | \

•                            PAD_FASTGF_ENABLE     | PAD_HYST_DISABLE | \
  

•                            PAD_HYST_CTRL_DEFAULT | PAD_FLOP_BYPASS)
  

Then this could be part of the same enum, and you avoid the line continuations.

Actually, I don't really see how the enum will avoid this? Do you have an example somewhere of what you are thinking about?

...

+/* pad config1 reg power-on values - Shouldn't need to change this */ +#define PAD_CONFIG1_DEFAULT 0x8000

+/* pad_val[2] - Iinenb - active low */ +#define PAD_VAL_INPUT_DISABLE (1 << 2) +#define PAD_VAL_INPUT_ENABLE (0 << 2)

+/* pad_val[1] - Ioutenb - active low */ +#define PAD_VAL_OUTPUT_DISABLE (1 << 1) +#define PAD_VAL_OUTPUT_ENABLE (0 << 1)

+/* Input / Output state should usually be mutually exclusive */ +#define PAD_VAL_INPUT (PAD_VAL_INPUT_ENABLE | PAD_VAL_OUTPUT_DISABLE) +#define PAD_VAL_OUTPUT (PAD_VAL_OUTPUT_ENABLE | PAD_VAL_INPUT_DISABLE)

+/* pad_val[0] - Value */ +#define PAD_VAL_HIGH (1 << 0) +#define PAD_VAL_LOW (0 << 0)

+/* pad_val reg power-on default varies by pad, and apparently can cause issues

• • if not set correctly, even if the pin isn't configured as GPIO. */

+#define PAD_VAL_DEFAULT PAD_VAL_INPUT

+/* Configure GPIOs as MMIO by default */ +#define GPIO_INPUT_PU_10K(_func) \

•   { .pad_conf0 = PAD_FUNC##_func | PAD_PU_10K | \
  

•           PAD_PULL_UP | \
  

•           PAD_CONFIG0_DEFAULT, \
  

•     .pad_conf1 = PAD_CONFIG1_DEFAULT, \
  

•     .pad_val   = PAD_VAL_INPUT, \
  

•     .use_sel   = GPIO_USE_MMIO, \
  

•     .is_gpio   = 1 }
  

I'm not a big fan of this sort of thing- #defines for structures in header files. It feels pretty ugly?

I wonder if there is another way of doing it?

I agree, it's ugly, but I don't see any other 'clean' way to that. I believe with the device tree support we should be able to configure everything outside of this file.

...

+#define PIRQA 0 +#define PIRQB 1 +#define PIRQC 2 +#define PIRQD 3 +#define PIRQE 4 +#define PIRQF 5 +#define PIRQG 6 +#define PIRQH 7

+/* These registers live behind the ILB_BASE_ADDRESS */

What what are they?

It was used for the PCI IRQ routing, but I think I can drop these modifications, we don't really need this in this patch.

...

+#define ACTL 0x00 +# define SCIS_MASK 0x07 +# define SCIS_IRQ9 0x00 +# define SCIS_IRQ10 0x01 +# define SCIS_IRQ11 0x02 +# define SCIS_IRQ20 0x04 +# define SCIS_IRQ21 0x05 +# define SCIS_IRQ22 0x06 +# define SCIS_IRQ23 0x07

+#endif /* _BAYTRAIL_IRQ_H_ */ diff --git a/arch/x86/include/asm/arch-baytrail/irqroute.h b/arch/x86/include/asm/arch-baytrail/irqroute.h new file mode 100644 index 0000000..f129880 --- /dev/null +++ b/arch/x86/include/asm/arch-baytrail/irqroute.h @@ -0,0 +1,67 @@ +/*

• • From Coreboot file of same name
• • Copyright (C) 2014 Google, Inc
• • Copyright (C) 2014 Sage Electronic Engineering, LLC.
• • SPDX-License-Identifier: GPL-2.0
• */

+#ifndef IRQROUTE_H +#define IRQROUTE_H

+#include <asm/arch/irq.h> +#include <asm/arch/pci_devs.h>

+/*

• *IR02h GFX INT(A) - PIRQ A
• *IR10h EMMC INT(ABCD) - PIRQ DEFG
• *IR11h SDIO INT(A) - PIRQ B
• *IR12h SD INT(A) - PIRQ C
• *IR13h SATA INT(A) - PIRQ D
• *IR14h XHCI INT(A) - PIRQ E
• *IR15h LP Audio INT(A) - PIRQ F
• *IR17h MMC INT(A) - PIRQ F
• *IR18h SIO INT(ABCD) - PIRQ BADC
• *IR1Ah TXE INT(A) - PIRQ F
• *IR1Bh HD Audio INT(A) - PIRQ G
• *IR1Ch PCIe INT(ABCD) - PIRQ EFGH
• *IR1Dh EHCI INT(A) - PIRQ D
• *IR1Eh SIO INT(ABCD) - PIRQ BDEF
• *IR1Fh LPC INT(ABCD) - PIRQ HGBC
• */

+#define PCI_DEV_PIRQ_ROUTES \

•   PCI_DEV_PIRQ_ROUTE(GFX_DEV,    A, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(EMMC_DEV,   D, E, F, G), \
  

•   PCI_DEV_PIRQ_ROUTE(SDIO_DEV,   B, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SD_DEV,     C, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SATA_DEV,   D, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(XHCI_DEV,   E, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(LPE_DEV,    F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(MMC45_DEV,  F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SIO1_DEV,   B, A, D, C), \
  

•   PCI_DEV_PIRQ_ROUTE(TXE_DEV,    F, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(HDA_DEV,    G, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(PCIE_DEV,   E, F, G, H), \
  

•   PCI_DEV_PIRQ_ROUTE(EHCI_DEV,   D, A, A, A), \
  

•   PCI_DEV_PIRQ_ROUTE(SIO2_DEV,   B, D, E, F), \
  

•   PCI_DEV_PIRQ_ROUTE(PCU_DEV,    H, G, B, C)
  

Is this actually used? In general I think this sort of monstrosity is better off in a C file.

It was when I was doing the IRQ routing, but I dropped the function as this is not necessary at the moment, I will remove it.

Thanks, Regards, Gabriel

--- Added to GNATS database as unassigned-patches/133

Responsible: patch-coord Message-Id: 54EDF488.8060102@huau-gabriel.fr In-Reply-To: CAPnjgZ2wsg+0d-WXgbwN7Gvm_9TRX9y+Exo6e4a0xW6FAx6_AA@mail.gmail.com References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr CAPnjgZ2wsg+0d-WXgbwN7Gvm_9TRX9y+Exo6e4a0xW6FAx6_AA@mail.gmail.com Patch-Date: Wed Feb 25 17:12:56 +0100 2015

Hi Gabriel,

On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

[snip]

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure
• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

Why undef these two? The BayTrail SoC does support GPIO banks in the legacy bridge.

#endif /* __CONFIG_H */

Regards, Bin

--- Added to GNATS database as unassigned-patches/132

Responsible: patch-coord Message-Id: CAEUhbmVRSwmt8HnW445a2kLzE12dmZcgJ+_Y7uj0u6uqvS=_7g@mail.gmail.com In-Reply-To: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr Patch-Date: Wed Feb 25 08:52:14 +0100 2015

Hi Bin,

On 02/24/2015 11:52 PM, Bin Meng wrote:

Hi Gabriel,

On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

[snip]

...

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure
• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

Why undef these two? The BayTrail SoC does support GPIO banks in the legacy bridge.

I might misunderstood the GPIO subsystem but I thought there was 2 banks available through the PCU iLB GPIO controller which contains the SCORE and SSUS (102 / 44 pins). The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I thought it was just a different controller from the Baytrail, but if I can use it to control all the GPIOs + doing the IO mapping, I'll be glad to do it!

...

#endif /* __CONFIG_H */

Regards, Bin

Regards, Gabriel

--- Added to GNATS database as unassigned-patches/134

Responsible: patch-coord Message-Id: 54EDF800.8060907@huau-gabriel.fr In-Reply-To: CAEUhbmVRSwmt8HnW445a2kLzE12dmZcgJ+_Y7uj0u6uqvS=_7g@mail.gmail.com References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr CAEUhbmVRSwmt8HnW445a2kLzE12dmZcgJ+_Y7uj0u6uqvS=_7g@mail.gmail.com Patch-Date: Wed Feb 25 17:27:44 +0100 2015

Hi Gabriel,

On Thu, Feb 26, 2015 at 12:27 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

Hi Bin,

On 02/24/2015 11:52 PM, Bin Meng wrote:

...

Hi Gabriel,

On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

[snip]

...

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure
• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

Why undef these two? The BayTrail SoC does support GPIO banks in the legacy bridge.

I might misunderstood the GPIO subsystem but I thought there was 2 banks available through the PCU iLB GPIO controller which contains the SCORE and SSUS (102 / 44 pins). The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I thought it was just a different controller from the Baytrail, but if I can use it to control all the GPIOs + doing the IO mapping, I'll be glad to do it!

I checked the BayTrail datasheet. Its GPIO is in the iLB (legacy bridge), which is the same as other IA chipset (Ivybridge, TunnelCreek, Quark). It has 4 banks in core domain and 2 banks in sus domain. So 6 banks in total. You need define 6 gpio nodes in the minnowmax board dts file. You should be able to use the existing gpio driver to configure.

...

...

#endif /* __CONFIG_H */

Regards, Bin

--- Added to GNATS database as unassigned-patches/135

Responsible: patch-coord Message-Id: CAEUhbmVycAjfq_4rtC_G8WWhyd5k3Fw_FCDJmhA-0hBUF72TFA@mail.gmail.com In-Reply-To: 54EDF800.8060907@huau-gabriel.fr References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr CAEUhbmVRSwmt8HnW445a2kLzE12dmZcgJ+_Y7uj0u6uqvS=_7g@mail.gmail.com 54EDF800.8060907@huau-gabriel.fr Patch-Date: Fri Feb 27 04:30:19 +0100 2015

Hi Bin,

On 02/26/2015 07:30 PM, Bin Meng wrote:

Hi Gabriel,

On Thu, Feb 26, 2015 at 12:27 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/24/2015 11:52 PM, Bin Meng wrote:

...

Hi Gabriel,

On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

[snip]

...

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure
• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

Why undef these two? The BayTrail SoC does support GPIO banks in the legacy bridge.

I might misunderstood the GPIO subsystem but I thought there was 2 banks available through the PCU iLB GPIO controller which contains the SCORE and SSUS (102 / 44 pins). The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I thought it was just a different controller from the Baytrail, but if I can use it to control all the GPIOs + doing the IO mapping, I'll be glad to do it!

I checked the BayTrail datasheet. Its GPIO is in the iLB (legacy bridge), which is the same as other IA chipset (Ivybridge, TunnelCreek, Quark). It has 4 banks in core domain and 2 banks in sus domain. So 6 banks in total. You need define 6 gpio nodes in the minnowmax board dts file. You should be able to use the existing gpio driver to configure.

Thanks for the clarification! Actually, I saw it today when I was doing some tests and I configured the 6 banks in the devices tree. I also fixed the GPIO base address to 0x48 but I got some issues like the fact I'm reading only 0 from all the registers.

The registers are configured to be in the IO Space (0x500), I checked the PCI configuration space to make sure that everything is enabled correctly, but I'm still missing something.

Once I'll be able to use these GPIOs, I will update the entire patch to remove the port from Coreboot as this is not necessary.

...

...

...

#endif /* __CONFIG_H */

Regards, Bin

Regards, Gabriel

--- Added to GNATS database as unassigned-patches/136

Responsible: patch-coord Message-Id: 54F022B9.8060000@huau-gabriel.fr In-Reply-To: CAEUhbmVycAjfq_4rtC_G8WWhyd5k3Fw_FCDJmhA-0hBUF72TFA@mail.gmail.com References: 1424037328-31636-1-git-send-email-contact@huau-gabriel.fr CAEUhbmVRSwmt8HnW445a2kLzE12dmZcgJ+_Y7uj0u6uqvS=_7g@mail.gmail.com 54EDF800.8060907@huau-gabriel.fr CAEUhbmVycAjfq_4rtC_G8WWhyd5k3Fw_FCDJmhA-0hBUF72TFA@mail.gmail.com Patch-Date: Fri Feb 27 08:54:33 +0100 2015

Hi Gabriel,

On 27 February 2015 at 01:52, Bin Meng bmeng.cn@gmail.com wrote:

Hi Gabriel,

On Fri, Feb 27, 2015 at 3:54 PM, gabriel huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/26/2015 07:30 PM, Bin Meng wrote:

...

Hi Gabriel,

On Thu, Feb 26, 2015 at 12:27 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/24/2015 11:52 PM, Bin Meng wrote:

...

Hi Gabriel,

On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Configure the pinctrl as it required to make some IO controllers working (USB/UART/I2C/...). The idea would be in the next version to modify the pch GPIO driver and configure these pins through the device tree.

These modifications are ported from the coreboot project.

Signed-off-by: Gabriel Huau contact@huau-gabriel.fr

arch/x86/cpu/baytrail/Makefile | 1 + arch/x86/cpu/baytrail/gpio.c | 206 +++++++++++++++ arch/x86/include/asm/arch-baytrail/gpio.h | 364 ++++++++++++++++++++++++++ arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ arch/x86/include/asm/arch-baytrail/pmc.h | 253 ++++++++++++++++++ board/intel/minnowmax/minnowmax.c | 212 +++++++++++++++ include/configs/minnowmax.h | 11 + 10 files changed, 1450 insertions(+) create mode 100644 arch/x86/cpu/baytrail/gpio.c create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h

[snip]

...

diff --git a/include/configs/minnowmax.h b/include/configs/minnowmax.h index 823e051..738c6fa 100644 --- a/include/configs/minnowmax.h +++ b/include/configs/minnowmax.h @@ -69,4 +69,15 @@ /* Avoid a warning in the Realtek Ethernet driver */ #define CONFIG_SYS_CACHELINE_SIZE 16

+/*

• • Baytrail has 3 GPIOs bank over PCI, there is no
• • driver at the moment so let's disable the command
• • and the default x86 driver to avoid any collision
• • with the GPIO mapping code.
• • @TODO: adding a baytrail-gpio driver and configure
• • the muxing through the device tree
• */

+#undef CONFIG_INTEL_ICH6_GPIO +#undef CONFIG_CMD_GPIO

Why undef these two? The BayTrail SoC does support GPIO banks in the legacy bridge.

I might misunderstood the GPIO subsystem but I thought there was 2 banks available through the PCU iLB GPIO controller which contains the SCORE and SSUS (102 / 44 pins). The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I thought it was just a different controller from the Baytrail, but if I can use it to control all the GPIOs + doing the IO mapping, I'll be glad to do it!

I checked the BayTrail datasheet. Its GPIO is in the iLB (legacy bridge), which is the same as other IA chipset (Ivybridge, TunnelCreek, Quark). It has 4 banks in core domain and 2 banks in sus domain. So 6 banks in total. You need define 6 gpio nodes in the minnowmax board dts file. You should be able to use the existing gpio driver to configure.

Thanks for the clarification! Actually, I saw it today when I was doing some tests and I configured the 6 banks in the devices tree. I also fixed the GPIO base address to 0x48 but I got some issues like the fact I'm reading only 0 from all the registers.

Yep, the offset should be 0x48 for BayTrail.

...

The registers are configured to be in the IO Space (0x500), I checked the PCI configuration space to make sure that everything is enabled correctly, but I'm still missing something.

I checked the gpio driver codes, and it currently has:

    /*
     * Okay, I guess we're looking at the right device. The actual
     * GPIO registers are in the PCI device's I/O space, starting
     * at the offset that we just read. Bit 0 indicates that it's
     * an I/O address, not a memory address, so mask that off.
     */
    gpiobase = tmplong & 0xfffe;

This should be changed to

    gpiobase = tmplong & 0xfffc;

as bit1 is the enable bit on BayTrail (Intel changes this GPIO base register again for BayTrail, sigh...)

...

Once I'll be able to use these GPIOs, I will update the entire patch to remove the port from Coreboot as this is not necessary.

...

...

...

...

#endif /* __CONFIG_H */

What is the next step with this patch please? It would be good to apply it to with the changes discussed.

Regards, Simon

Hi Gabriel,

On 1 April 2015 at 05:20, Gabriel Huau contact@huau-gabriel.fr wrote:

Hi Simon,

On 03/31/2015 07:32 PM, Simon Glass wrote:

...

Hi Gabriel,

On 27 February 2015 at 01:52, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Gabriel,

On Fri, Feb 27, 2015 at 3:54 PM, gabriel huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/26/2015 07:30 PM, Bin Meng wrote:

...

Hi Gabriel,

On Thu, Feb 26, 2015 at 12:27 AM, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/24/2015 11:52 PM, Bin Meng wrote: > > Hi Gabriel, > > On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau > contact@huau-gabriel.fr > wrote: >> >> Configure the pinctrl as it required to make some IO controllers >> working (USB/UART/I2C/...). >> The idea would be in the next version to modify the pch GPIO driver >> and >> configure these pins through the device tree. >> >> These modifications are ported from the coreboot project. >> >> Signed-off-by: Gabriel Huau contact@huau-gabriel.fr >> --- >> arch/x86/cpu/baytrail/Makefile | 1 + >> arch/x86/cpu/baytrail/gpio.c | 206 >> +++++++++++++++ >> arch/x86/include/asm/arch-baytrail/gpio.h | 364 >> ++++++++++++++++++++++++++ >> arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ >> arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ >> arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ >> arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 ++++++++++ >> arch/x86/include/asm/arch-baytrail/pmc.h | 253 >> ++++++++++++++++++ >> board/intel/minnowmax/minnowmax.c | 212 >> +++++++++++++++ >> include/configs/minnowmax.h | 11 + >> 10 files changed, 1450 insertions(+) >> create mode 100644 arch/x86/cpu/baytrail/gpio.c >> create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h >> create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h >> create mode 100644 arch/x86/include/asm/arch-baytrail/irqroute.h >> create mode 100644 arch/x86/include/asm/arch-baytrail/pci_devs.h >> create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h >> > [snip] > >> diff --git a/include/configs/minnowmax.h >> b/include/configs/minnowmax.h >> index 823e051..738c6fa 100644 >> --- a/include/configs/minnowmax.h >> +++ b/include/configs/minnowmax.h >> @@ -69,4 +69,15 @@ >> /* Avoid a warning in the Realtek Ethernet driver */ >> #define CONFIG_SYS_CACHELINE_SIZE 16 >> >> +/* >> + * Baytrail has 3 GPIOs bank over PCI, there is no >> + * driver at the moment so let's disable the command >> + * and the default x86 driver to avoid any collision >> + * with the GPIO mapping code. >> + * @TODO: adding a baytrail-gpio driver and configure >> + * the muxing through the device tree >> + */ >> +#undef CONFIG_INTEL_ICH6_GPIO >> +#undef CONFIG_CMD_GPIO >> + > > Why undef these two? The BayTrail SoC does support GPIO banks in the > legacy bridge.

I might misunderstood the GPIO subsystem but I thought there was 2 banks available through the PCU iLB GPIO controller which contains the SCORE and SSUS (102 / 44 pins). The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I thought it was just a different controller from the Baytrail, but if I can use it to control all the GPIOs + doing the IO mapping, I'll be glad to do it!

I checked the BayTrail datasheet. Its GPIO is in the iLB (legacy bridge), which is the same as other IA chipset (Ivybridge, TunnelCreek, Quark). It has 4 banks in core domain and 2 banks in sus domain. So 6 banks in total. You need define 6 gpio nodes in the minnowmax board dts file. You should be able to use the existing gpio driver to configure.

Thanks for the clarification! Actually, I saw it today when I was doing some tests and I configured the 6 banks in the devices tree. I also fixed the GPIO base address to 0x48 but I got some issues like the fact I'm reading only 0 from all the registers.

Yep, the offset should be 0x48 for BayTrail.

...

The registers are configured to be in the IO Space (0x500), I checked the PCI configuration space to make sure that everything is enabled correctly, but I'm still missing something.

I checked the gpio driver codes, and it currently has:

     /*
      * Okay, I guess we're looking at the right device. The actual
      * GPIO registers are in the PCI device's I/O space, starting
      * at the offset that we just read. Bit 0 indicates that it's
      * an I/O address, not a memory address, so mask that off.
      */
     gpiobase = tmplong & 0xfffe;

This should be changed to

     gpiobase = tmplong & 0xfffc;

as bit1 is the enable bit on BayTrail (Intel changes this GPIO base register again for BayTrail, sigh...)

...

Once I'll be able to use these GPIOs, I will update the entire patch to remove the port from Coreboot as this is not necessary.

...

...

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

What is the next step with this patch please? It would be good to apply it to with the changes discussed.

Sorry, actually I was super busy and wasn't able to work on the minnowboard max ... I should have some time this week end. But you can go ahead and drop this patch, I will submit a new one because most of the modification are actually not needed, we can use the generic pch_gpio driver already present in u-boot with some modification/fix.

My only problem at the moment, is to find a solution to declare properly the pin muxing for every pins. I don't want to extend the pch_gpio_set* structures because we have 6 banks on the MNW ... I was thinking to use the device tree but I'm not really fan of the idea to change the pin muxing based on the device tree, it should be only a hardware description. My idea at the moment, is to do the pin muxing through functions named "gpio_set_cfg/gpio_set_pull/..." called in the board file. A little bit like the driver s5p_gpio.c and board file board/sunxi/board.c. What is your though about that?

I think pin mux is part of the hardware description - and each board can do it differently depending on how the circuit works. I believe device tree is exactly the right place.

Is there a suitable binding or do you need to make one up?

Regards, Simon

Hi Gabriel,

On 6 April 2015 at 00:10, Gabriel Huau contact@huau-gabriel.fr wrote:

Hi Simon,

On 04/05/2015 11:31 AM, Simon Glass wrote:

...

Hi Gabriel,

On 1 April 2015 at 05:20, Gabriel Huau contact@huau-gabriel.fr wrote:

...

Hi Simon,

On 03/31/2015 07:32 PM, Simon Glass wrote:

...

Hi Gabriel,

On 27 February 2015 at 01:52, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Gabriel,

On Fri, Feb 27, 2015 at 3:54 PM, gabriel huau contact@huau-gabriel.fr wrote:

...

Hi Bin,

On 02/26/2015 07:30 PM, Bin Meng wrote: > > Hi Gabriel, > > On Thu, Feb 26, 2015 at 12:27 AM, Gabriel Huau > contact@huau-gabriel.fr > wrote: >> >> Hi Bin, >> >> >> On 02/24/2015 11:52 PM, Bin Meng wrote: >>> >>> Hi Gabriel, >>> >>> On Mon, Feb 16, 2015 at 5:55 AM, Gabriel Huau >>> contact@huau-gabriel.fr >>> wrote: >>>> >>>> Configure the pinctrl as it required to make some IO controllers >>>> working (USB/UART/I2C/...). >>>> The idea would be in the next version to modify the pch GPIO >>>> driver >>>> and >>>> configure these pins through the device tree. >>>> >>>> These modifications are ported from the coreboot project. >>>> >>>> Signed-off-by: Gabriel Huau contact@huau-gabriel.fr >>>> --- >>>> arch/x86/cpu/baytrail/Makefile | 1 + >>>> arch/x86/cpu/baytrail/gpio.c | 206 >>>> +++++++++++++++ >>>> arch/x86/include/asm/arch-baytrail/gpio.h | 364 >>>> ++++++++++++++++++++++++++ >>>> arch/x86/include/asm/arch-baytrail/iomap.h | 73 ++++++ >>>> arch/x86/include/asm/arch-baytrail/irq.h | 119 +++++++++ >>>> arch/x86/include/asm/arch-baytrail/irqroute.h | 67 +++++ >>>> arch/x86/include/asm/arch-baytrail/pci_devs.h | 144 >>>> ++++++++++ >>>> arch/x86/include/asm/arch-baytrail/pmc.h | 253 >>>> ++++++++++++++++++ >>>> board/intel/minnowmax/minnowmax.c | 212 >>>> +++++++++++++++ >>>> include/configs/minnowmax.h | 11 + >>>> 10 files changed, 1450 insertions(+) >>>> create mode 100644 arch/x86/cpu/baytrail/gpio.c >>>> create mode 100644 arch/x86/include/asm/arch-baytrail/iomap.h >>>> create mode 100644 arch/x86/include/asm/arch-baytrail/irq.h >>>> create mode 100644 >>>> arch/x86/include/asm/arch-baytrail/irqroute.h >>>> create mode 100644 >>>> arch/x86/include/asm/arch-baytrail/pci_devs.h >>>> create mode 100644 arch/x86/include/asm/arch-baytrail/pmc.h >>>> >>> [snip] >>> >>>> diff --git a/include/configs/minnowmax.h >>>> b/include/configs/minnowmax.h >>>> index 823e051..738c6fa 100644 >>>> --- a/include/configs/minnowmax.h >>>> +++ b/include/configs/minnowmax.h >>>> @@ -69,4 +69,15 @@ >>>> /* Avoid a warning in the Realtek Ethernet driver */ >>>> #define CONFIG_SYS_CACHELINE_SIZE 16 >>>> >>>> +/* >>>> + * Baytrail has 3 GPIOs bank over PCI, there is no >>>> + * driver at the moment so let's disable the command >>>> + * and the default x86 driver to avoid any collision >>>> + * with the GPIO mapping code. >>>> + * @TODO: adding a baytrail-gpio driver and configure >>>> + * the muxing through the device tree >>>> + */ >>>> +#undef CONFIG_INTEL_ICH6_GPIO >>>> +#undef CONFIG_CMD_GPIO >>>> + >>> >>> Why undef these two? The BayTrail SoC does support GPIO banks in >>> the >>> legacy bridge. >> >> I might misunderstood the GPIO subsystem but I thought there was 2 >> banks >> available through the PCU iLB GPIO controller which contains the >> SCORE >> and >> SSUS (102 / 44 pins). >> The intel_ich6_gpio has a limitation of 32 GPIOs per bank and I >> thought >> it >> was just a different controller from the Baytrail, but if I can use >> it >> to >> control all the GPIOs + doing the IO mapping, I'll be glad to do it! > > I checked the BayTrail datasheet. Its GPIO is in the iLB (legacy > bridge), which is the same as other IA chipset (Ivybridge, > TunnelCreek, Quark). It has 4 banks in core domain and 2 banks in sus > domain. So 6 banks in total. You need define 6 gpio nodes in the > minnowmax board dts file. You should be able to use the existing gpio > driver to configure.

Thanks for the clarification! Actually, I saw it today when I was doing some tests and I configured the 6 banks in the devices tree. I also fixed the GPIO base address to 0x48 but I got some issues like the fact I'm reading only 0 from all the registers.

Yep, the offset should be 0x48 for BayTrail.

...

The registers are configured to be in the IO Space (0x500), I checked the PCI configuration space to make sure that everything is enabled correctly, but I'm still missing something.

I checked the gpio driver codes, and it currently has:

      /*
       * Okay, I guess we're looking at the right device. The

actual * GPIO registers are in the PCI device's I/O space, starting * at the offset that we just read. Bit 0 indicates that it's * an I/O address, not a memory address, so mask that off. */ gpiobase = tmplong & 0xfffe;

This should be changed to

      gpiobase = tmplong & 0xfffc;

as bit1 is the enable bit on BayTrail (Intel changes this GPIO base register again for BayTrail, sigh...)

...

Once I'll be able to use these GPIOs, I will update the entire patch to remove the port from Coreboot as this is not necessary.

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

What is the next step with this patch please? It would be good to apply it to with the changes discussed.

Sorry, actually I was super busy and wasn't able to work on the minnowboard max ... I should have some time this week end. But you can go ahead and drop this patch, I will submit a new one because most of the modification are actually not needed, we can use the generic pch_gpio driver already present in u-boot with some modification/fix.

My only problem at the moment, is to find a solution to declare properly the pin muxing for every pins. I don't want to extend the pch_gpio_set* structures because we have 6 banks on the MNW ... I was thinking to use the device tree but I'm not really fan of the idea to change the pin muxing based on the device tree, it should be only a hardware description. My idea at the moment, is to do the pin muxing through functions named "gpio_set_cfg/gpio_set_pull/..." called in the board file. A little bit like the driver s5p_gpio.c and board file board/sunxi/board.c. What is your though about that?

I think pin mux is part of the hardware description - and each board can do it differently depending on how the circuit works. I believe device tree is exactly the right place.

Is there a suitable binding or do you need to make one up?

Regards, Simon

Agreed.

I was thinking to use the gpioX node and create a binding 'pch,pins', for example:

gpioa { compatible = "intel,ich6-gpio" u-boot,dm-pre-reloc; reg = <0 0x20> bank-name = "A";

pch,pins = <
    BYT_UART_HD_RSTB MUX_MODE0
>;

}

My problem is the pad value/direction for the GPIO are in another register, so I cannot really easily do something like "BYT_UART_HD_RSTB (MUX_MODE0 | GPIO_OUTPUT)" or even "BYT_UART_HD_RSTB (MUX_MODE0 | GPIO_OUTPUT_HIGH)". I was thinking to something like this:

pch,pins = <
    BYT_UART_HD_RSTB_CFG (MUX_MODE0 | PULL_20K)
    BYT_UART_HD_RSTB_PAD (GPIO_OUTPUT_HIGH)

    BYT_UART_HD_RSTC_CFG (MUX_MODE1)
    BYT_UART_HD_RSTC_PAD NONE

    ...
>;

But it means that we will need to use 2 lines to defines a pin, even if the second line could define 'nothing', I'm personally fine with that, but not sure if this is the best solution.

Are these #defines from a binding file that you would include?

I don't think 2 lines is a problem, but on the other handle the device tree binding does not need to match the register layout exactly. In general you can do things like:

pch,pins { pin0 { reg = <0>; some-property = <3>; boolean-prop; } pin1 { ... };

but to be more efficient I understand that bitfields are better. But having a few properties is fine. The code can translate them.

Also, I was going to put the code in drivers/gpios/intel_ich6_gpio.c but the GPIO muxing code is called only when we run the command 'gpio', I'll will try to find a better solution as it needs to be call as early as possible.

If you do something like:

struct udevice *dev;

uclass_get_device(UCLASS_GPIO, 0, &dev);

it will init the GPIOs. But I think what you really want here is pinmux. Perhaps add a special function that sets up the pinmux, unrelated to the GPIOs? Linux has the concept of pinctrl which is not the same as GPIOs. Pins can be used as GPIOs but also other functions.

For now, how about just having a function called pinctrl_init() which reads the config out of the device tree and applies it? If you are keen, you could turn it into a driver in a new UCLASS_PINCTRL uclass.

Regards, Simon