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

Sun Apr 5 20:31:32 CEST 2015

Hi Gabriel,

On 1 April 2015 at 05:20, Gabriel Huau <contact at 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 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.
>
>
> 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