Hi Simon,
On 04/07/2015 07:03 PM, Simon Glass wrote:
Hi Gabriel,
On 6 April 2015 at 00:10, Gabriel Huau <cont...@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 <cont...@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...@gmail.com> wrote:
Hi Gabriel,
On Fri, Feb 27, 2015 at 3:54 PM, gabriel huau <cont...@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
<cont...@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
<cont...@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 <cont...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
