On 10/24/2016 06:58 AM, Sekhar Nori wrote:
On Saturday 22 October 2016 12:06 AM, David Lechner wrote:
This adds a device tree definition file for LEGO MINDSTORMS EV3.
Thanks for the patch!
What is working:
* Pin muxing
* MicroSD card reader
* UART on input port 1
What is partially working:
* Buttons - working after GPIO fix
* LEDs - working after GPIO fix
* Poweroff/reset - working after GPIO fix
Is the GPIO fix something that will go in v4.9-rc cycle ?
Not sure. This is still being discussed.
http://www.gossamer-threads.com/lists/linux/kernel/2550178
* Flash memory - driver loads but can't read the block devices - this is
probably due to the fact that we are not able to configure the SPI to
use DMA via device tree
Hmm, I would not have expected PIO mode to be so inefficient that you
are unable to even read the block device.
I am getting a -EIO error. I haven't been able to trace down exactly
what is causing it yet though.
...
+/ {
+ compatible = "lego,ev3", "ti,da850";
+ model = "LEGO MINDSTORMS EV3";
+
+ soc@1c00000 {
+ /*
+ * (ab)using pinctrl-single to disable all internal pullups/
+ * pulldowns on I/O.
+ */
+ pinmux@22c00c {
+ compatible = "pinctrl-single";
+ reg = <0x22c00c 0x4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pinctrl-single,bit-per-mux;
+ pinctrl-single,register-width = <32>;
+ pinctrl-single,function-mask = <0xf>;
+ /*
+ * There is a bug in pinctrl-single that prevents us
+ * from setting function-mask to 1, so doing things
+ * in groups of 4. Doesn't really matter since we are
+ * disabling all at once anyway.
+ */
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pupu_disable>;
+
+ pupu_disable: pinmux_all_pins {
+ pinctrl-single,bits = <
+ 0x0 0x00000000 0xffffffff
+ >;
+ };
Sigh. This is quite an abuse :)
I know we don't have a good way to configure this in kernel today. And I
am surprised we never had to care about disabling pullups so far. Can
you clarify why you need it? I assume there is some contention you want
to avoid, but on which interface?
The EV3 was designed with external pullup/pulldown everywhere. I know
for certain that it breaks one of the buttons if you do not disable the
internal ones. I imagine that it would have subtle effects elsewhere if
they are not disabled.
I have not gone through each pullup/pulldown bank individually, but it
would not surprise me at all if there was at least one thing on most of
them that would be adversely affected.
I dont think this can be done this way using pinctrl-single. A small
driver to handle pullup/down control for da850 may have to be added to
drivers/pinctrl. It will be better to check with Linus Walleij on his
thoughts using a new thread ccing the pinctrl subsystem list as well.
I will be glad to try to make a driver, but when I ran into this problem
I could not find much information on how to handle banks of
pullup/pulldown. Most of what I saw was for ones that can be
individually controlled. If anyone knows something like this already
that I could look at, it would be helpful to me.
[...]
+ in1_pins: pinmux_in1_pins {
+ pinctrl-single,bits = <
+ /* GP0[15] */
+ 0x0 0x00000008 0x0000000f
+ /* GP0[2] */
+ 0x4 0x00800000 0x00f00000
+ /* GP2[2] */
+ 0x18 0x00800000 0x00f00000
+ /* GP8[10], GP8[11] */
+ 0x48 0x88000000 0xff000000
+ >;
+ };
I see that this is not really used. Can you add these when you actually
use them. Looks like that applies to some other definitions like this below.
It will be possible to uses these gpios via sysfs (until a proper driver
for input and output ports is merged). So how about I attach these to
the gpio node for now?
+&ehrpwm1 {
+ status = "disabled";
Hmm, disabled? Can you add this node when you actually use it?
Not sure why I have this disabled. Like the gpios, the pwms can be used
via sysfs, so I would like to leave them.
+ pinctrl-names = "default";
+ /* MBPWM, MAPWM */
+ pinctrl-0 = <&ehrpwm1a_pins>, <&ehrpwm1b_pins>;
+};
+
+&ecap1 {
+ status = "disabled";
same here and other places below.
+ pinctrl-names = "default";
+ /* MDPWM */
+ pinctrl-0 = <&ecap1_pins>;
+};
+
+&spi0 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi0_pins>, <&spi0_cs0_pin>, <&spi0_cs3_pin>;
+ dmas = <&edma0 14 0>, <&edma0 15 0>;
+ dma-names = "rx", "tx";
+
+ spi-flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "n25q128a13", "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <50000000>;
+ ti,spi-wdelay = <8>;
+
+ partition@0 {
+ label = "U-Boot";
+ reg = <0 0x40000>;
Thats 256KB for U-Boot and MLO (I assume in concatenated AIS image). Is
that sufficient for future too? Moving partitions later is tough ask
because that means users will lose data when they upgrade the kernel
because of partitions moving around. Just a suggestion to keep future
U-Boot bloat in mind and not use a "just fits" number.
The MLO is on an EEPROM in the EV3, so the U-Boot partition is just
U-boot. The SoC boots from I2C, which then runs whatever is as 0x0 on
the flash memory.
This partition table matches the partition scheme used on the official
LEGO firmware that ships with the devices. Most people running their own
kernel will probably be loading it from a microSD card, leaving the
official firmware intact and therefore will always have this partition
table.
My thinking is that if someone does want to use a different partitioning
scheme, they can build their own U-Boot and configure it to modify the
device tree with a new partition table.
The way the LEGO firmware flashing utility works, it wipes out the
entire flash memory each time you flash the firmware. So, data loss is
not a concern - you will loose your data anyway.
+ };
+
+ partition@40000 {
+ label = "U-Boot Env";
+ reg = <0x40000 0x10000>;
+ };
+
+ partition@50000 {
+ label = "Kernel";
+ reg = <0x50000 0x200000>;
+ };
+
+ partition@250000 {
+ label = "Filesystem";
+ reg = <0x250000 0xa50000>;
+ };
+
+ partition@cb0000 {
+ label = "Storage";
+ reg = <0xcb0000 0x2f0000>;
+ };
+ };
+
+ /* TODO: ADC goes here */
I would drop this comment.
ack
+};
+
+&spi1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi1_pins>, <&spi1_cs0_pin>;
+
+ /* TODO: LCD Display goes here */
Add this node when you actually have display working.
What if we set this up as a spidev node instead? This way the display
could be used from userspace without a driver.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
