[U-Boot] [PATCH 10/25] x86: ich6-gpio: Add Intel Tunnel Creek GPIO support

[Bin Meng]

Hi Simon,

On Fri, Dec 5, 2014 at 6:43 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 4 December 2014 at 08:01, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Intel Tunnel Creek GPIO register block is compatible with current
>> ich6-gpio driver, except the offset and content of GPIO block base
>> address register in the LPC PCI configuration space are different.
>>
>> Use u16 instead of u32 to store the 16-bit I/O address of the GPIO
>> registers so that it could support both Ivybridge and Tunnel Creek.
>
> How does that help?

The Tunnel Creek GPIO base address register bit31 is bit to control
the I/O decode of the GPIO block (1 means decode enable), while on the
Ivybridge bit31-bit16 is reserved (read returns 0).

>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>  arch/x86/include/asm/arch-queensbay/gpio.h | 13 +++++++++++++
>>  arch/x86/include/asm/gpio.h                |  2 +-
>>  board/google/chromebook_link/link.c        |  2 +-
>>  drivers/gpio/intel_ich6_gpio.c             | 17 ++++++++---------
>>  4 files changed, 23 insertions(+), 11 deletions(-)
>>  create mode 100644 arch/x86/include/asm/arch-queensbay/gpio.h
>>
>> diff --git a/arch/x86/include/asm/arch-queensbay/gpio.h b/arch/x86/include/asm/arch-queensbay/gpio.h
>> new file mode 100644
>> index 0000000..ab4e059
>> --- /dev/null
>> +++ b/arch/x86/include/asm/arch-queensbay/gpio.h
>> @@ -0,0 +1,13 @@
>> +/*
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef _X86_ARCH_GPIO_H_
>> +#define _X86_ARCH_GPIO_H_
>> +
>> +/* Where in config space is the register that points to the GPIO registers? */
>> +#define PCI_CFG_GPIOBASE 0x44
>> +
>> +#endif /* _X86_ARCH_GPIO_H_ */
>> diff --git a/arch/x86/include/asm/gpio.h b/arch/x86/include/asm/gpio.h
>> index 5540d42..83878a7 100644
>> --- a/arch/x86/include/asm/gpio.h
>> +++ b/arch/x86/include/asm/gpio.h
>> @@ -11,7 +11,7 @@
>>  #include <asm-generic/gpio.h>
>>
>>  struct ich6_bank_platdata {
>> -       uint32_t base_addr;
>> +       uint16_t base_addr;
>>         const char *bank_name;
>>  };
>>
>> diff --git a/board/google/chromebook_link/link.c b/board/google/chromebook_link/link.c
>> index 4d95c1c..9978e92 100644
>> --- a/board/google/chromebook_link/link.c
>> +++ b/board/google/chromebook_link/link.c
>> @@ -125,7 +125,7 @@ int board_early_init_f(void)
>>         return 0;
>>  }
>>
>> -void setup_pch_gpios(u32 gpiobase, const struct pch_gpio_map *gpio)
>> +void setup_pch_gpios(u16 gpiobase, const struct pch_gpio_map *gpio)
>>  {
>>         /* GPIO Set 1 */
>>         if (gpio->set1.level)
>> diff --git a/drivers/gpio/intel_ich6_gpio.c b/drivers/gpio/intel_ich6_gpio.c
>> index 3a2e1b0..3b2962a 100644
>> --- a/drivers/gpio/intel_ich6_gpio.c
>> +++ b/drivers/gpio/intel_ich6_gpio.c
>> @@ -39,12 +39,12 @@
>>
>>  struct ich6_bank_priv {
>>         /* These are I/O addresses */
>> -       uint32_t use_sel;
>> -       uint32_t io_sel;
>> -       uint32_t lvl;
>> +       uint16_t use_sel;
>> +       uint16_t io_sel;
>> +       uint16_t lvl;
>>  };
>>
>> -__weak void setup_pch_gpios(u32 gpiobase, const struct pch_gpio_map *gpio)
>> +__weak void setup_pch_gpios(u16 gpiobase, const struct pch_gpio_map *gpio)
>>  {
>>         return;
>>  }
>> @@ -62,7 +62,7 @@ static int gpio_ich6_ofdata_to_platdata(struct udevice *dev)
>>         u8 tmpbyte;
>>         u16 tmpword;
>>         u32 tmplong;
>> -       u32 gpiobase;
>> +       u16 gpiobase;
>>         int offset;
>>
>>         /* Where should it be? */
>> @@ -121,11 +121,10 @@ static int gpio_ich6_ofdata_to_platdata(struct udevice *dev)
>>         /*
>>          * GPIOBASE moved to its current offset with ICH6, but prior to
>>          * that it was unused (or undocumented). Check that it looks
>> -        * okay: not all ones or zeros, and mapped to I/O space (bit 0).
>> +        * okay: not all ones or zeros.
>>          */
>>         tmplong = pci_read_config32(pci_dev, PCI_CFG_GPIOBASE);
>> -       if (tmplong == 0x00000000 || tmplong == 0xffffffff ||
>> -           !(tmplong & 0x00000001)) {
>
> Why take out this check?

Again the Tunnel Creek GPIO base address register bit0 is reserved
(read returns 0), while on the Ivybridge the bit0 is used to indicate
it is an I/O space.

[snip]

Regards,
Bin