search

MichaIng / DietPi

Lightweight justice for your single-board computer!
https://dietpi.com/
GNU General Public License v2.0
4.8k stars 494 forks source link

Radxa Zero | Fix I2C bus assignation & add missing overlay for alt I2C/UART interfaces #6346

Open ghost opened 1 year ago

ghost commented 1 year ago

Creating a bug report/issue

Required Information

Steps to reproduce

Added overlay to dietpiEnv.txt user_overlays=dietpi-usb-otg meson-g12a-i2c-ee-m3-gpioa-14-gpioa-15

Expected behaviour

I2C_EE_M3 (/dev/i2c-3) is listed and usable.

/dev/i2c-1 is for the HDMI and should probably be /dev/i2c-2 if we stick to Radxa kernel naming convention since they alias 4 i2c interfaces but only 3 are usable (1, 3, 4)

Actual behaviour

I2C_EE_M3 (/dev/i2c-3) is not listed.

Rather there are:

/dev/i2c-0
/dev/i2c-1

/dev/i2c-0 is actually I2C_EE_M3 (/dev/i2c-3)

/dev/i2c-1 is for the hdmi

Further it's missing overlay for enabling other i2c/uart interfaces.

Extra details

See below URL for the I2C naming scheme according to their overlay: https://wiki.radxa.com/Zero/Ubuntu#common_hardware_interface

I suppose this problem comes from the way armbian organises their dtb as I think DietPi uses their if I'm not wrong? I also have sadly very little experience in that, I can see some dtb overlay in the meson folder but the content is not very meaningful.

This aside, can someone tells the difference between using "overlays" and "user_overlays" in dietpiEnv.txt? To me, a basic user, this seems redundant at first sight.

Thank you for this great distro.

MichaIng commented 1 year ago

I suppose this problem comes from the way armbian organises their dtb as I think DietPi uses their if I'm not wrong?

Exactly. Here is the patch/source code of the overlays available on the Armbian meson64 kernel: https://github.com/armbian/build/blob/main/patch/kernel/archive/meson64-6.1/general-meson64-overlays.patch

The two I2C related overlays are for meson gxbb (NanoPi K2, Odroid C2) only, so there is none such for Radxa Zero. The first two I2C interfaces are instead enabled OOTB, as you can see. For the /dev/i2c-3 a new overlay would need to be created, and I'm also not sufficiently experienced to do this effectively.

At least we can have a look whether there are other I2C nodes registered already:

find /proc/device-tree/ -type d -name '*i2c*' | while read -r node; do [[ -f $node/status ]] && { echo -n "$node: "; cat "$node/status"; echo; }; done
ghost commented 1 year ago

@MichaIng Just to clear a confusion, in this issue #4831 the correct overlays were available and apparently working.

So the DietPi image was first using the kernel of Radxa then moved to the Armbian one?

MichaIng commented 1 year ago

So the DietPi image was first using the kernel of Radxa then moved to the Armbian one?

Exactly. The Linux ~4.4~ 5.10 Radxa provides is to old to be usable with modern Debian versions.

ghost commented 1 year ago

@MichaIng Can you somehow stop including OOTB the redundant dtb from Armbian if I push updated ones to you? Including for future kernel update from Armbian.

Speaking of that, it seems there is one update https://github.com/armbian/build/tree/main/patch/kernel/archive/meson64-6.2

Not so many changes apparently though

MichaIng commented 1 year ago

We cannot remove/skip the whole dtb from the Armbian package. That would mean that booting could be broken as fast as users apply another APT upgrade. If you have a fix or addition, I suggest to open a PR, adding it as patch (or fixing another patch) at Armbian. If it is possible to apply it as device tree overlay, we can add it to /boot/overlay-user on DietPi update, to apply it that way.

Speaking of that, it seems there is one update https://github.com/armbian/build/tree/main/patch/kernel/archive/meson64-6.2

That's the "edge" kernel branch. I'm quite sure it contains no changes regarding device tree overlay sources and related patches, but you can test it:

apt install linux-{image,dtb}-edge-meson64
ghost commented 1 year ago

@MichaIng Might try to put a patch at Armbian... Won't, only maintainers can make a ticket, and it's a low priority for this kind of interface issue.

This link below has been helpful to learn more: https://docs.armbian.com/User-Guide_Allwinner_overlays/

The first two I2C interfaces are instead enabled OOTB

The files that defines whether they are enabled OOTB are here: https://github.com/radxa/kernel/blob/linux-5.10.y-radxa-zero/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts

They are not compiled. Many variables are set after compilation.

find /proc/device-tree/ -type d -name 'i2c' | while read -r node; do [[ -f $node/status ]] && { echo -n "$node: "; cat "$node/status"; echo; }; done

/proc/device-tree/soc/bus@ff800000/i2c@5000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1d000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1f000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1c000: okay
/proc/device-tree/soc/bus@ffd00000/i2c@1e000: disabled
ls -la /sys/bus/i2c/devices/
0-0022 -> ../../../devices/platform/soc/ffd00000.bus/ffd1c000.i2c/i2c-0/0-0022
i2c-0 -> ../../../devices/platform/soc/ffd00000.bus/ffd1c000.i2c/i2c-0
i2c-1 -> ../../../devices/platform/soc/ff600000.bus/ff600000.hdmi-tx/i2c-1

/i2c-0/0-0022 is fusb302 according to the dts. fusb302 is a chip for linking usb type c to the microprocessor via i2c.

/dev/i2c-1 points to hdmi, this seems normal, though the naming isn't following the ones of Radxa so it's confusing (should probably be i2c-2)

Armbian probably won't fix any of this for a long time, this is considered a "low priority issue", following their overlay rules.

Related but for Odroid: https://forum.armbian.com/topic/24417-i2c-and-uart-issue-on-odroid-n2/

UART_AO_A and I2C_EE_M3 are enabled OOTB for the Radxa Zero, which corresponds to the default GPIO layout of the RPi.

If anyone else want to contribute at some point and refer to this... I'm stopping here.

MichaIng commented 1 year ago

Won't, only maintainers can make a ticket

Everyone can open a pull request: https://github.com/armbian/build/pulls

The files that defines whether they are enabled OOTB are here: https://github.com/radxa/kernel/blob/linux-5.10.y-radxa-zero/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts

Nope, this is Radxa's vendor kernel while we/Armbian uses mainline Linux: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts?h=linux-6.1.y

/proc/device-tree/soc/bus@ff800000/i2c@5000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1d000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1f000: disabled
/proc/device-tree/soc/bus@ffd00000/i2c@1c000: okay
/proc/device-tree/soc/bus@ffd00000/i2c@1e000: disabled

So there are other I2C nodes which can be simply enabled with a device tree overlay. When you look into these directories, or in combination with the device tree source code in combination with the kernel patches applied by Armbian, you should find out with GPIO pins (if at all) they use.

ghost commented 1 year ago

Everyone can open a pull request: https://github.com/armbian/build/pulls

They'll probably close it if not on the maintainer list for the board. If you look closely at all the pulls and issues, it's only the armbian developpers.

The maintainers can make ticket on gitlab and get approved/denied and then devs do things and push on github something like that seems so...

Nope, this is Radxa's vendor kernel while we/Armbian uses mainline Linux:

Yes it is, I didn't do a lengthy explanation as to why because it is too long but something about comparing both.

Anyway this issue is finally looking to be something about playing with sysfs so that bus are correctly assigned to their devices... which I know nothing about. Then some copy/paste of the Radxa kernel overlay.

I'm so sorry that I won't be doing it at the end despite opening this ticket, it's been already taking a long time to learn how works the device trees, it was instructing though.

MichaIng commented 1 year ago

They'll probably close it if not on the maintainer list for the board. If you look closely at all the pulls and issues, it's only the armbian developpers.

Do you have some reference for this practice? It is normal that most PRs are done from actual project maintainers/contributors. But I remember quite a lot of PRs from non-maintainers, at least not that I was aware of it. Also there are PRs for SBCs which do not even have an official maintainer. Last PR I opened and which was merged is here: https://github.com/armbian/build/pull/3859 Would be basically a waste to categorically deny good PRs only based on who is opening them, and very sad if Armbian would have recently changed internal policies to go that route. Still, if a PR is good and fixes something, an actual maintainer could grab the commit(s) to open an own PR, hopefully preserving credits for the original author. One needs to open a related ticket here: https://armbian.atlassian.net/jira/software/c/projects/AR/issues/?filter=allissues

Another route would be a bug report at the Armbian forum, of course after verifying the same issue on an Armbian image:

Yes it is, I didn't do a lengthy explanation as to why because it is too long but something about comparing both.

You mean "Yes it is mainline Linux" or "Yes it is vendor Linux"? It is the first, starting with the fact that the two files indeed differ, that Armbian uses Linux 6.1 while Radxa's branch is 5.10. It is basically a fork of the Rockchip kernel repo: https://github.com/rockchip-linux/kernel I think when aiming to fix the mainline kernel, the Radxa sources won't help much as internal references structure may be significantly different. I'd check what is expected based on running a Radxa kernel image, then check mainline kernel source code to hopefully find out why this does not apply.

However, as there are other I2C nodes, at least we can check whether they do enable the /dev/i2c-3 that you expect. To just enabled all of them:

apt install device-tree-compiler
mkdir -p /boot/overlay-user
dtc -I dts -O dtb -o /boot/overlay-user/i2c.dtbo <<< '/dts-v1/;
/plugin/;
/ {
    compatible = "radxa,zero", "amlogic,g12a";
    fragment@0 {
        target-path = "/soc/bus@ff800000/i2c@5000";
        __overlay__ {
            status = "okay";
        };
    };
    fragment@1 {
        target-path = "/soc/bus@ffd00000/i2c@1d000";
        __overlay__ {
            status = "okay";
        };
    };
    fragment@2 {
        target-path = "/soc/bus@ffd00000/i2c@1f000";
        __overlay__ {
            status = "okay";
        };
    };
    fragment@3 {
        target-path = "/soc/bus@ffd00000/i2c@1e000";
        __overlay__ {
            status = "okay";
        };
    };
};'
G_CONFIG_INJECT 'user_overlays=' 'user_overlays=i2c' /boot/dietpiEnv.txt

... found the related upstream node definitions: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi?h=linux-6.1.y#n2243 Interestingly, there i2c-3 is /soc/bus@ffd00000/i2c@1c000, which is enabled already in your case.

And this Armbian patch will be the culprit, which seems to use i2c3 for the USB-C controller: https://github.com/armbian/build/blob/main/patch/kernel/archive/meson64-6.1/board-radxa-zero-dts-add-support-for-the-usb-c-controller.patch The patch was added here, when there seem to have been no device tree upstream yet: https://github.com/armbian/build/pull/3125/commits/a7ebfd8 And edited here to explicitly edit the i2c3 node: https://github.com/armbian/build/pull/4420/commits/4396ffc