Re: [PATCH 2/2] arm64: dts: allwinner: h313: Add Amediatech X96Q
From: J. Neuschäfer <hidden>
Date: 2025-09-15 18:37:03
Also in:
linux-arm-kernel, linux-sunxi, lkml
On Sat, Sep 13, 2025 at 05:36:12PM +0800, Chen-Yu Tsai wrote:
On Fri, Sep 12, 2025 at 5:54 PM Andre Przywara [off-list ref] wrote:quoted
On Fri, 12 Sep 2025 01:52:10 +0200 J. Neuschäfer via B4 Relay [off-list ref] wrote: Hi, many thanks for posting the DT, I really wish more people would do that!quoted
From: "J. Neuschäfer" <redacted> The X96Q is a set-top box with an H313 SoC, AXP305 PMIC, 1 or 2 GiB RAM, 8 or 16 GiB eMMC flash, 2x USB A, Micro-SD, HDMI, Ethernet, audio/video output, and infrared input. https://x96mini.com/products/x96q-tv-box-android-10-set-top-box Tested, works: - debug UART - status LED - USB ports in host mode - MicroSD - eMMC - recovery button hidden behind audio/video port - analog audio (line out) Does not work: - Ethernet (requires AC200 MFD/EPHY driver) - analog video output (requires AC200 driver) - HDMI audio/video output Untested: - "OTG" USB port in device mode - built-in IR receiver - external IR receiver - WLAN (requires out-of-tree XRadio driver) Table of regulators on the downstream kernel, for reference: vcc-5v 1 15 0 unknown 5000mV 0mA 5000mV 5000mV dcdca 0 0 0 unknown 900mV 0mA 0mV 0mV dcdcb 0 0 0 unknown 1350mV 0mA 0mV 0mV dcdcc 0 0 0 unknown 900mV 0mA 0mV 0mV dcdcd 0 0 0 unknown 1500mV 0mA 0mV 0mV dcdce 0 0 0 unknown 3300mV 0mA 0mV 0mV aldo1 0 0 0 unknown 3300mV 0mA 0mV 0mV aldo2 0 0 0 unknown 700mV 0mA 0mV 0mV aldo3 0 0 0 unknown 700mV 0mA 0mV 0mV bldo1 0 0 0 unknown 1800mV 0mA 0mV 0mV bldo2 0 0 0 unknown 1800mV 0mA 0mV 0mV bldo3 0 0 0 unknown 700mV 0mA 0mV 0mV bldo4 0 0 0 unknown 700mV 0mA 0mV 0mV cldo1 0 0 0 unknown 2500mV 0mA 0mV 0mV cldo2 0 0 0 unknown 700mV 0mA 0mV 0mV cldo3 0 0 0 unknown 700mV 0mA 0mV 0mV Signed-off-by: J. Neuschäfer <redacted> ---
[...]
quoted
quoted
+&mmc0 { + vmmc-supply = <®_aldo1>; + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ + disable-wp; + bus-width = <4>; + max-frequency = <150000000>;That line is already in the .dtsi file, so redundant.quoted
+ status = "okay"; + /* µSD */If we really need this comment, it should be above, right after the "&mmc0 {". And I wonder if it should be "microSD" instead.Yes. Please use ASCII in the code if possible, since some of us have setups that don't quite work well with extended character sets.
ACK
quoted
quoted
+}; + +&mmc1 { + /* TODO: XRadio XR819 WLAN */Either you just keep the comment, an mention mmc1, but don't reference the node, or you add the properties that you know of already, like vmmc-supply, vqmmc-supply, mmc-pwrseq, bus-width, non-removable. But this "empty reference with a comment" is somewhat odd.I'd say just fill it in completely so that the mmc host is enabled and the SDIO card is detected. Missing driver support for the chip is a different issue, but since this is an enumerable bus it shouldn't prevent you from describing everything already.
I gave it a try just now, but I wasn't successful with enumeration:
&mmc1 {
/* XRadio XR819 WLAN */
vmmc-supply = <®_aldo1>;
vqmmc-supply = <®_bldo1>;
mmc-pwrseq = <&wifi_pwrseq>;
bus-width = <4>;
non-removable;
mmc-ddr-1_8v;
status = "okay";
};
The result is unsuccessful (with or without the questionable mmc-ddr-1_8v):
[ 1.607511] mmc1: Failed to initialize a non-removable card
The downstream DT mentions a few relevant properties:
/wlan {
wlan_regon = <&pio 6 18 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&losc>;
};
...
losc: clk_losc@0 {
linux,phandle = <0xd3>;
phandle = <0xd3>;
allwinner,drive = <0x02>;
allwinner,function = "x32kfout";
allwinner,muxsel = <0x03>;
allwinner,pins = "PG10";
allwinner,pull = <0x01>;
};
Translating this (roughly) into mainline bindings:
mmc-pwrseq = <&wifi_pwrseq>;
...
wifi_pwrseq: pwrseq {
compatible = "mmc-pwrseq-emmc";
reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>;
clocks = <&rtc CLK_OSC32K_FANOUT>;
clock-names = "ext_clock";
pinctrl-names = "default";
pinctrl-0 = <&x32clk_fanout_pin>;
};
... it fails in drivers/reset/core.c, because __reset_add_reset_gpio_device
doesn't handle #gpio-cells = <3>, which is the case for &pio:
/*
* Currently only #gpio-cells=2 is supported with the meaning of:
* args[0]: GPIO number
* args[1]: GPIO flags
* TODO: Handle other cases.
*/
if (args->args_count != 2)
return -ENOENT;
Relatedly, I'd expect this limitation to break WiFi on sun50i-h313-tanix-tx1.dts
(upstreamed by Andre) as well.
quoted
quoted
+}; + +&mmc2 { + vmmc-supply = <®_aldo1>; + vqmmc-supply = <®_bldo1>; + non-removable; + cap-mmc-hw-reset; + mmc-ddr-1_8v; + mmc-hs200-1_8v; + bus-width = <8>; + max-frequency = <100000000>;Are you sure you need that?
After some more testing, it turns out that I do need to limit the frequency to 100 MHz. At the default of 150 MHz, stable operation of the eMMC isn't possible (all other properties being as they are).
quoted
quoted
+ status = "okay"; + /* eMMC */Please move that comment up.I don't think it's necessary though. hs200 and 8-bit width would make it obvious that it's an eMMC.
Not necessary, but perhaps still useful for someone having a quick glance at the devicetree, so I'd prefer to keep it in. Best regards, J. Neuschäfer