[U-Boot] [PATCH] x86: minnowmax: add GPIO mapping support
gabriel huau
contact at huau-gabriel.fr
Thu Apr 9 04:45:06 CEST 2015
Hi Simon,
On 04/07/2015 07:03 PM, Simon Glass wrote:
> Hi Gabriel,
>
> On 6 April 2015 at 00:10, Gabriel Huau <contact at huau-gabriel.fr> wrote:
>> 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.
> Are these #defines from a binding file that you would include?
>
> I don't think 2 lines is a problem, but on the other handle the device
> tree binding does not need to match the register layout exactly. In
> general you can do things like:
>
> pch,pins {
> pin0 {
> reg = <0>;
> some-property = <3>;
> boolean-prop;
> }
> pin1 {
> ...
> };
>
> but to be more efficient I understand that bitfields are better. But
> having a few properties is fine. The code can translate them.
>
>> 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.
> If you do something like:
>
> struct udevice *dev;
>
> uclass_get_device(UCLASS_GPIO, 0, &dev);
>
> it will init the GPIOs. But I think what you really want here is
> pinmux. Perhaps add a special function that sets up the pinmux,
> unrelated to the GPIOs? Linux has the concept of pinctrl which is not
> the same as GPIOs. Pins can be used as GPIOs but also other functions.
>
> For now, how about just having a function called pinctrl_init() which
> reads the config out of the device tree and applies it? If you are
> keen, you could turn it into a driver in a new UCLASS_PINCTRL uclass.
>
> Regards,
> Simon
Thanks for you feedback! Actually I'm going to try to implement
something close to your idea this week end and we can see what's going
on :).
I think you can close this thread/patch, I will submit a new patch
series where we will be able to discuss of the implementation if you are
ok with that.
Regards,
Gabriel
More information about the U-Boot
mailing list