[U-Boot] [PATCH] x86: minnowmax: add GPIO mapping support

[Simon Glass]

Hi Gabriel,

On 27 February 2015 at 01:52, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Gabriel,
>
> On Fri, Feb 27, 2015 at 3:54 PM, gabriel huau <contact at 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 at 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 at 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 at 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