[U-Boot] [PATCH] x86: minnowmax: add GPIO mapping support
Simon Glass
sjg at chromium.org
Wed Apr 8 04:03:52 CEST 2015
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
More information about the U-Boot
mailing list