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

[Simon Glass]

Hi Bin,

On 5 December 2014 at 01:35, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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.

OK can you add a comment here about that?

Regards,
Simon