patch/kernel/archive/rockchip64-6.18/general-rk3588-i2s-mclk-output-gate-4-gate-grf-clocks.patch (1)

109-118: Consider adding a warning on SYS_GRF registration failure.

If syscon_regmap_lookup_by_compatible() fails or kzalloc() returns NULL, the SYS_GRF won't be registered in aux_grf_table. The GATE_GRF clock branches will then silently fail to find the correct GRF, potentially leaving MCLK gates in bootloader-defined state without any indication.

Given this is early init context, a pr_warn would help diagnose audio issues:

Optional: Add warning for registration failure

 	/* Register SYS_GRF for I2S MCLK output to IO gate clocks */
 	sys_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3588-sys-grf");
-	if (!IS_ERR(sys_grf)) {
+	if (IS_ERR(sys_grf)) {
+		pr_warn("%s: failed to lookup SYS_GRF, I2S MCLK gates unavailable\n", __func__);
+	} else {
 		sys_grf_e = kzalloc(sizeof(*sys_grf_e), GFP_KERNEL);
 		if (sys_grf_e) {
 			sys_grf_e->grf = sys_grf;
 			sys_grf_e->type = grf_type_sys;
 			hash_add(ctx->aux_grf_table, &sys_grf_e->node, grf_type_sys);
+		} else {
+			pr_warn("%s: failed to allocate SYS_GRF entry\n", __func__);
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@patch/kernel/archive/rockchip64-6.18/general-rk3588-i2s-mclk-output-gate-4-gate-grf-clocks.patch`
around lines 109 - 118, When registering SYS_GRF via
syscon_regmap_lookup_by_compatible, add diagnostic warnings if the lookup fails
or kzalloc returns NULL so failures aren't silent: check the return of
syscon_regmap_lookup_by_compatible("rockchip,rk3588-sys-grf") and emit a pr_warn
including the error (or that lookup returned NULL/IS_ERR) if it fails, and emit
a pr_warn if kzalloc for sys_grf_e returns NULL before attempting to set
sys_grf_e->grf/type or hash_add(ctx->aux_grf_table,...); reference the symbols
syscon_regmap_lookup_by_compatible, sys_grf, sys_grf_e, kzalloc, aux_grf_table
and GATE_GRF in the message to aid diagnosis.

patch/kernel/archive/rockchip64-6.18/general-rk3588-i2s-mclk-output-gate-4-gate-grf-clocks.patch (1)

109-118: Consider adding a warning on allocation failure.

The syscon lookup failure is handled silently, which is reasonable since the GRF may not be present in some DT configurations. However, a kzalloc failure at this stage would be unexpected and could silently break the GATE_GRF clocks without any indication.

An optional improvement would be to log a warning on allocation failure to aid debugging:

📝 Optional: Add warning on kzalloc failure

 	/* Register SYS_GRF for I2S MCLK output to IO gate clocks */
 	sys_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3588-sys-grf");
 	if (!IS_ERR(sys_grf)) {
 		sys_grf_e = kzalloc(sizeof(*sys_grf_e), GFP_KERNEL);
 		if (sys_grf_e) {
 			sys_grf_e->grf = sys_grf;
 			sys_grf_e->type = grf_type_sys;
 			hash_add(ctx->aux_grf_table, &sys_grf_e->node, grf_type_sys);
+		} else {
+			pr_warn("%s: failed to allocate aux_grf for sys_grf\n", __func__);
 		}
 	}

That said, this is consistent with upstream submission patterns for clock drivers, so keeping it as-is is also acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@patch/kernel/archive/rockchip64-6.18/general-rk3588-i2s-mclk-output-gate-4-gate-grf-clocks.patch`
around lines 109 - 118, The syscon lookup and kzalloc block silently ignores
allocation failure; update the allocation path in the code that calls
syscon_regmap_lookup_by_compatible("rockchip,rk3588-sys-grf") so that if
kzalloc(sizeof(*sys_grf_e), GFP_KERNEL) returns NULL you emit a warning (e.g.,
dev_warn/dev_warn_probe or pr_warn) indicating that allocation of sys_grf_e
failed and that GATE_GRF clocks may not be registered; keep the current behavior
(no further action) after logging, and reference the same symbols sys_grf,
sys_grf_e, ctx->aux_grf_table and the existing hash_add call so the only change
is adding the warning on kzalloc failure.

patch/u-boot/v2026.04/dt_upstream_rockchip/rk3588-mekotronics-r58x-pro.dts (2)

34-39: TODO: Consider adding wakeup-source for volume button.

The comment indicates uncertainty about whether to enable wakeup-source for the volume-up button. If the hardware supports wake from suspend via this button, adding wakeup-source; would be appropriate.

Would you like me to help verify the hardware capability or generate a solution that implements this feature?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@patch/u-boot/v2026.04/dt_upstream_rockchip/rk3588-mekotronics-r58x-pro.dts`
around lines 34 - 39, The volume-up button node (vol_up / label "VolumeUp")
should be marked as a wake source if the GPIO/controller and board hardware
support wake-from-suspend: add the wakeup-source; property to the vol_up node
(the same node that contains linux,code = <KEY_VOLUMEUP> and gpios = <&gpio3
RK_PD0 GPIO_ACTIVE_HIGH>). Before committing, verify the GPIO controller (&gpio3
/ RK_PD0) and SoC power domain support wake events and test suspend/resume to
confirm the button wakes the system.

---

614-617: TODO: FUSB302 USB Type-C controller support is missing.

The pinctrl for usbc0_int is defined but the actual FUSB302 node is not present. This limits USB-C functionality (no PD negotiation, role switching).

Would you like me to help generate a FUSB302 node configuration, or should I open an issue to track this enhancement?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@patch/u-boot/v2026.04/dt_upstream_rockchip/rk3588-mekotronics-r58x-pro.dts`
around lines 614 - 617, The pinctrl for usb-typec (symbols: usb-typec,
usbc0_int, usbc0-int) declares an interrupt but the FUSB302 device node is
missing—add a child node for the FUSB302 controller (name it e.g. fusb302@<reg>
or fusb302-usbc0) with the correct compatible string (e.g. "onsemi,fusb302" or
the upstream binding name), reg address for its I2C/SPI bus, an interrupt
specifier referencing usbc0-int, gpio lines if used for reset/alert, pinctrl-0 =
<&usbc0_int> and pinctrl-names = "default", and any vbus-supply or
interrupt-parent properties required by the fusb302 binding so the kernel can
bind PD/role-switching support; update the bus node (i2c/spi) to include the new
device in its children if needed.