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

[Bin Meng]

Hi Simon,

On Fri, Dec 5, 2014 at 11:08 PM, Simon Glass <sjg at chromium.org> wrote:
> 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?

Will do in v2.

Regards,
Bin