Thread (15 messages) 15 messages, 5 authors, 2025-09-15

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 = <&reg_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 = <&reg_aldo1>;
		vqmmc-supply = <&reg_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 = <&reg_aldo1>;
+     vqmmc-supply = <&reg_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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help