andy-shev / linux

Linux kernel source tree
Other
25 stars 11 forks source link

I2C-6 Doesn't Work #15

Closed iracigt closed 7 years ago

iracigt commented 7 years ago

As was previously mentioned on the Intel forums, i2c-6 doesn't work with the newer kernels because it requires setting the pin mux. Intel had previously exported this functionality through a (somewhat clunky) debugfs interface that worked with the old version of the GPIO driver included with the official 3.x kernels.

For a project I'm working on, I need both I2C buses. What would it take to get i2c-6 working, both temporarily and in a way that could be sent upstream? I'm willing to work on this and to learn, but have limited background with Linux kernel development (took an operating systems course using a teaching kernel, but never contributed to Linux).

So far, I've considered writing a userspace program that sets the pin mux, but it looks like the I2C pins are in a protected region that requires using an IPC channel. This rules out a userspace program because that would be complicated and we couldn't safely lock the IPC.

It looks like much of the groundwork has been done for this in pinctrl-merrifield.c. What is the appropriate place to be calling the pinctrl code from?

htot commented 7 years ago

I have been checking how much work it would be to port mraa to newer kernels. In mraa's edison platform code I stumbled into https://github.com/intel-iot-devkit/mraa/blob/master/src/x86/intel_edison_fab_c.c line #127. Here it is attempted to change the pinmux using SYSFS_CLASS_GPIO "/gpio%i/pinmux", which doesn't exist on 3.14 eds and if that fails using DEBUGFS_PINMODE_PATH "%i/current_pinmux", which is not really standard. So I'm wondering if the former is standard for later kernels, or just another hack for eds that never got implemented. If more or less standard it might be a suitable interface for code that would live in pinctrl-merrifield.c

andy-shev commented 7 years ago

The patches are in my tree, though I didn't make them working. You may try to amend them. Though, I'm not going to upstream them by few reasons, main of which is badly prepared firmware. We might fix this via U-Boot nevertheless.

htot commented 7 years ago

Looks like everything is in place and only needs to be set when the i2c driver is probed. Correct?

svenschwermer commented 6 years ago

Hi! I spent some time and figured out a way to make i2c-6 work, see https://svenschwermer.de/2017/10/14/intel-edison-i2c6-with-vanilla-linux.html

iracigt commented 6 years ago

Thanks for the link! Turns out I went down the same path last week and finally got it working too. I ended up creating an small module that just requested the pin muxing I wanted and loading that. It's more complicated, but had the advantage of allowing the I2C drivers to be present at boot and for the kernel to find the GPIO expander we're using at boot. I'll try to get my code up soon, though for most people your fix is probably better.

farfromrefug commented 6 years ago

@iracigt could you share your code? We are looking at doing the same thing. What is what you call a gpio expander? Cause may be we could use that too if you are willing to share. Thanks

floion commented 6 years ago

Hi @andy-shev @htot is uboot now doing this pinmuxing?

andy-shev commented 6 years ago

@floion Nope. We need a volunteer who can do this.

See Sven's comment above how to enable it in Linux.

floion commented 6 years ago

Thanks for the follow-up, Andy. Using Sven's patch at the moment for enabling i2c-6. I guess we will need do the same for other pinmux'ing we want?

andy-shev commented 6 years ago

@floion Not same for sure. I2C is just special, other muxings can be done in usual way via pin control lookup tables.

alext-mkrs commented 6 years ago

I'd be glad to volunteer, just need to finish some current project, that'd take me about a week or two from now and if no one does this by then I'll bite.

htot commented 6 years ago

@andy-shev What would be an upstream-able solution for this? Do you have any ideas?

alext-mkrs commented 6 years ago

So I think I'm ready to start looking into this one. But as I'm new to U-Boot's code, if you indeed have any hints or ideas, @andy-shev - I'm all ears.

andy-shev commented 6 years ago

@htot @alext-mkrs Like I mentioned before, the upstream solution is to provide pin control and GPIO drivers for Intel Merrifield in U-Boot. This will give an essential part to configure board at early stage as needed.

alext-mkrs commented 6 years ago

@andy-shev, yeah, thanks - that part is understood, I meant more like the next level of detail, i.e. where specifically would that fit best in U-Boot, do you think it makes any sense to port the driver from Linux vs. write a new one tailored to U-Boot driver model - that sort of thing, like a basic direction advice.

andy-shev commented 6 years ago

@alext-mkrs Linux driver (more precisely drivers) can be considered as a documentation, though U-Boot driver (or drivers) should be made according to U-Boot practices, e.g. Driver Model.

htot commented 6 years ago

But as I understood the issue is not with the I2C driver per sé, but setting up one of the needed pin mode? As @iracigt mentions above he created a small module to do that. Maybe this code could be used as example what to add to u-boot?

alext-mkrs commented 6 years ago

@htot, the thing is not with the code itself, but the order two IP blocks are probed, that's why the module approach helps - not because it contains some different code, but because it can be loaded after the SCU device (which owns those pins) is probed.

Ok, thanks Andy, I'll look into this.

andy-shev commented 6 years ago

@alext-mkrs The pins are not owned by SCU, they are protected by internal bus firewall, and SCU just has more permissions than usual I/O writes/reads.

alext-mkrs commented 6 years ago

Oh, I see, that's interesting - thanks. I don't think this is described in any external Edison docs, so hopefully the Linux driver has enough information to do it meaningfully for u-boot.

andy-shev commented 6 years ago

@alext-mkrs ...Linux driver from stock kernel. Unfortunately I have no firmware specification documents either. But I can share some of my observations (basically I extracted what is needed in my never-be-upstreamed patch set which enables I2C bus 6).

alext-mkrs commented 6 years ago

Thanks, I'll surely take a look. Haven't yet got this unfortunately still :\ but this is the first on my list.

alext-mkrs commented 6 years ago

Okay, so I planned to get a bit deeper into the u-boot, however life has executed its rights and looks like I won't and I won't have time to look into this particular one anytime soon :( So FYI and it's again up for grabs for anyone willing.

htot commented 6 years ago

That's unfortunate, I hope everything is ok with you. In the meanwhile I have gone ahead and put @svenschwermer patch as a temporary solution in my meta-intel-edison rocko. It works only in the non-acpi case.

alext-mkrs commented 6 years ago

Thanks. It's ok, just that I have to change focus somewhat and I honestly don't see how I can find time for this specific one in a foreseeable future - so I though it would only be fair to let everyone know about that explicitly.

htot commented 5 years ago

Hi! I spent some time and figured out a way to make i2c-6 work, see https://svenschwermer.de/2017/10/14/intel-edison-i2c6-with-vanilla-linux.html

@svenschwermer We now have a way of enabling I2C-6 from U-Boot. Nevertheless I have used your patch for our non-acpi enabled kernel and non-I2C enabled U-Boot - but never tested if I2C-6 really worked by connecting a device.

Now I have linux 4.18 and after testing the new U-Boot I decided to try our old U-Boot and kernel with your patch and find that it doesn't work. Our kernel recipe (ok, for the 4.16 kernel) is here, your patch here

root@edison:~# cat /sys/kernel/debug/pinctrl/pinctrl-merrifield/pins
registered pins: 233
....
pin 101 (GP19_I2C_1_SCL) mode 1 0x00203511
pin 102 (GP20_I2C_1_SDA) mode 1 0x00203511
pin 103 (GP21_I2C_2_SCL) mode 1 0x00203501
pin 104 (GP22_I2C_2_SDA) mode 1 0x00203581
pin 105 (GP17_I2C_3_SCL_HDMI) mode 1 0x20203421
pin 106 (GP18_I2C_3_SDA_HDMI) mode 1 0x202034a1
pin 107 (GP23_I2C_4_SCL) mode 1 0x00203501
pin 108 (GP24_I2C_4_SDA) mode 1 0x00203581
pin 109 (GP25_I2C_5_SCL) mode 1 0x00203101
pin 110 (GP26_I2C_5_SDA) mode 1 0x00203181
pin 111 (GP27_I2C_6_SCL) GPIO 0x00000510
pin 112 (GP28_I2C_6_SDA) GPIO 0x00000590
pin 113 (GP29_I2C_7_SCL) mode 1 0x00203501
pin 114 (GP30_I2C_7_SDA) mode 1 0x00203581

I looks pin mode is not set to the correct mode. Would you know what causes that? Something in 4.18?

Even though we are going in the direction of using ACPI and having this set in U-Boot, I would like to have this option working too.

htot commented 5 years ago

@svenschwermer I found it. The Edison Arduino Hardware guide tells us to do this:

echo 28 > /sys/class/gpio/export
echo 27 > /sys/class/gpio/export

But when we touch these pins, they change to GPIO instead of mode 1. For future reference, this script correctly sets the I2C-6 bus (tested with 4.18):

#!/bin/sh
echo 204 > /sys/class/gpio/export
echo 205 > /sys/class/gpio/export
echo 236 > /sys/class/gpio/export
echo 237 > /sys/class/gpio/export
echo 14 > /sys/class/gpio/export
echo 165 > /sys/class/gpio/export
echo 212 > /sys/class/gpio/export
echo 213 > /sys/class/gpio/export
echo 214 > /sys/class/gpio/export
echo low > /sys/class/gpio/gpio214/direction
echo low > /sys/class/gpio/gpio204/direction # HW Guide §11.6 is wrong, follow Table 4
echo low > /sys/class/gpio/gpio205/direction # HW Guide §11.6 is wrong, follow Table 4
echo in > /sys/class/gpio/gpio14/direction
echo in > /sys/class/gpio/gpio165/direction
echo low > /sys/class/gpio/gpio236/direction
echo low > /sys/class/gpio/gpio237/direction
echo in > /sys/class/gpio/gpio212/direction
echo in > /sys/class/gpio/gpio213/direction
echo high > /sys/class/gpio/gpio214/direction

With modprobe -i i2c-dev and TSL2561 connected:

root@edison:~# i2cdetect -r -y 6
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:          -- -- -- -- -- -- -- -- -- -- -- -- -- 
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
30: -- -- -- -- -- -- -- -- -- 39 -- -- -- -- -- -- 
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
70: -- -- -- -- -- -- -- --                         
andy-shev commented 5 years ago

@htot Just a side note, that this is a script for non-ACPI version of kernel/U-Boot. in ACPI case this settings are done via ASL code.

htot commented 5 years ago

Absolutely.