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

Gabriel Huau contact at huau-gabriel.fr
Mon Apr 6 08:10:39 CEST 2015


Hi Simon,

On 04/05/2015 11:31 AM, Simon Glass wrote:
> 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

Agreed.

I was thinking to use the gpioX node and create a binding 'pch,pins', 
for example:

gpioa {
     compatible = "intel,ich6-gpio"
     u-boot,dm-pre-reloc;
     reg = <0 0x20>
     bank-name = "A";

     pch,pins = <
         BYT_UART_HD_RSTB MUX_MODE0
     >;
}

My problem is the pad value/direction for the GPIO are in another 
register, so I cannot really easily do something like "BYT_UART_HD_RSTB 
(MUX_MODE0 | GPIO_OUTPUT)" or even "BYT_UART_HD_RSTB (MUX_MODE0 | 
GPIO_OUTPUT_HIGH)". I was thinking to something like this:

     pch,pins = <
         BYT_UART_HD_RSTB_CFG (MUX_MODE0 | PULL_20K)
         BYT_UART_HD_RSTB_PAD (GPIO_OUTPUT_HIGH)

         BYT_UART_HD_RSTC_CFG (MUX_MODE1)
         BYT_UART_HD_RSTC_PAD NONE

         ...
     >;

But it means that we will need to use 2 lines to defines a pin, even if 
the second line could define 'nothing', I'm personally fine with that, 
but not sure if this is the best solution.

Also, I was going to put the code in drivers/gpios/intel_ich6_gpio.c but 
the GPIO muxing code is called only when we run the command 'gpio', I'll 
will try to find a better solution as it needs to be call as early as 
possible.

Regards,
Gabriel


More information about the U-Boot mailing list