Re: [PATCH v3 3/3] arm64: dts: allwinner: add Radxa Cubie A7S board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] arm64: dts: allwinner: cubie-a7s DTB build fails on missing
  A733 CCU headers
- [High] arm64: dts: allwinner: unsupported x-powers,axp318w PMIC blocks
  consumer probe
- [High] arm64: dts: allwinner: cubie-a7s sets 1.8V vcc-pf-supply for
  3.3V-only mmc0
--

commit 572e2012767dbd5d4ea9dfb9c1da49853b19d780
Author: Enzo Adriano <enzo.adriano.code@gmail.com>

arm64: dts: allwinner: add Radxa Cubie A7S board

This commit adds the device tree for the Radxa Cubie A7S board based on the
Allwinner A733 SoC, enabling UART0, the AXP318W PMIC, and MMC0, and wires it
into the sunxi DTB build.

> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> index d1f768dd6461d..792152525b433 100644
> --- a/arch/arm64/boot/dts/allwinner/Makefile
> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> @@ -68,3 +68,4 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun55i-a527-cubie-a5e.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun55i-h728-x96qpro+.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun55i-t527-avaota-a1.dtb
>  dtb-$(CONFIG_ARCH_SUNXI) += sun55i-t527-orangepi-4a.dtb
> +dtb-$(CONFIG_ARCH_SUNXI) += sun60i-a733-cubie-a7s.dtb

[Severity: High]
Does adding sun60i-a733-cubie-a7s.dtb to dtb-$(CONFIG_ARCH_SUNXI) break the
DTB build?

sun60i-a733-cubie-a7s.dts includes sun60i-a733.dtsi and references
CLK_BUS_R_RTC and CLK_R_AHB in the &rtc node, and sun60i-a733.dtsi includes:

arch/arm64/boot/dts/allwinner/sun60i-a733.dtsi {
	#include <dt-bindings/clock/allwinner,sun60i-a733-ccu.h>
	#include <dt-bindings/clock/allwinner,sun60i-a733-r-ccu.h>
	#include <dt-bindings/interrupt-controller/arm-gic.h>
	#include <dt-bindings/reset/allwinner,sun60i-a733-ccu.h>
	#include <dt-bindings/reset/allwinner,sun60i-a733-r-ccu.h>
}

None of those four A733 clock and reset binding headers exist in
include/dt-bindings/, so compiling dtbs with CONFIG_ARCH_SUNXI enabled fails
at the preprocessor stage.  In addition, there are no in-tree drivers for
allwinner,sun60i-a733-ccu or allwinner,sun60i-a733-r-ccu to provide clocks and
resets at runtime.

> diff --git a/arch/arm64/boot/dts/allwinner/sun60i-a733-cubie-a7s.dts b/arch/arm64/boot/dts/allwinner/sun60i-a733-cubie-a7s.dts
> new file mode 100644
> index 0000000000000..341c24360ce29
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun60i-a733-cubie-a7s.dts

[ ... ]

> +&mmc0 {
> +	bus-width = <4>;
> +	cap-sd-highspeed;
> +	cd-gpios = <&pio 5 6 (GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)>;
> +	disable-wp;
> +	max-frequency = <50000000>;
> +	no-1-8-v;
> +	vmmc-supply = <&reg_swout2>;
> +	/* The 3.3 V path uses VCC-IO; no Port F voltage switching yet. */
> +	vqmmc-supply = <&reg_swout2>;
> +	status = "okay";
> +};
> +
> +&osc {
> +	clock-frequency = <26000000>;
> +};
> +
> +&pio {
> +	vcc-pb-supply = <&reg_swout2>;
> +	vcc-pc-supply = <&reg_cldo5>;
> +	vcc-pd-supply = <&reg_swout1>;
> +	vcc-pe-supply = <&reg_bldo1>;
> +	vcc-pf-supply = <&reg_cldo5>;

[Severity: High]
Will setting vcc-pf-supply = <&reg_cldo5> configure Port F's IO bias for 1.8 V
while &mmc0 operates at 3.3 V?

&mmc0 uses Port F pins (PF0-PF6) with no-1-8-v and vqmmc-supply =
<&reg_swout2> for 3.3 V signaling, whereas reg_cldo5 is fixed at 1800000 uV.
When mmc0 requests its pins, sunxi_pmx_request() ->
sunxi_pinctrl_set_io_bias_cfg() reads the voltage of vcc-pf-supply and
programs the bank's PIO_POW_MOD_SEL bit for 1.8 V whenever uV <= 1800000,
mismatching the 3.3 V SD signaling path.  Pointing vcc-pf-supply at the 3.3 V
supply (&reg_swout2, with a defined 3.3 V voltage or parent supply) would keep
Port F's bias configuration aligned with 3.3 V signaling.

> +	vcc-pg-supply = <&reg_aldo1>;
> +	vcc-ph-supply = <&reg_swout2>;
> +	vcc-pj-supply = <&reg_swout1>;
> +	vcc-pk-supply = <&reg_bldo1>;
> +};
> +
> +&r_i2c0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&r_i2c0_pl_pins>;
> +
> +	axp318w: pmic@36 {
> +		compatible = "x-powers,axp318w";
> +		reg = <0x36>;
> +		interrupt-parent = <&nmi_intc>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +
> +		aldo156in-supply = <&reg_ps>;
> +		aldo234in-supply = <&reg_aldo1>;
> +		bldoin-supply = <&reg_ldo_input>;
> +		cldoin-supply = <&reg_ldo_input>;
> +		dldoin-supply = <&reg_ps>;
> +		eldoin-supply = <&reg_dcdc9>;
> +		ips-supply = <&reg_ps>;
> +		vin19-supply = <&reg_ps>;
> +		vin23-supply = <&reg_ps>;
> +		vin45-supply = <&reg_ps>;
> +		vin678-supply = <&reg_ps>;

[Severity: High]
Does axp318w prevent &pio, &uart0, and &mmc0 from probing because
"x-powers,axp318w" has no in-tree driver or DT binding?

Neither drivers/mfd/axp20x*.c, drivers/regulator/axp20x-regulator.c, nor
Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml supports
"x-powers,axp318w" (or its grouped input supplies and swout1/swout2/dcdc8/dcdc9
regulators).  Because &pio, &r_pio, and &mmc0 reference child regulator nodes
under axp318w, of_regulator_dev_lookup() finds the DT nodes without registered
regulator devices and returns -EPROBE_DEFER, causing sunxi_pmx_request() for
uart0 and mmc0 as well as mmc_regulator_get_supply() to defer indefinitely.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794227241.git.enzo.adriano.code@gmail.com?part=3