Re: [PATCH v2 3/3] arm64: dts: allwinner: A133: add support for Baijie Helper A133 board

[Alexander Sverdlin <alexander.sverdlin@gmail.com>]

Hi Paul,

thanks for the review!

On Mon, 2026-05-18 at 13:52 +0200, Paul Kocialkowski wrote:
> 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-baije-core.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a133-baije-core.dtsi
> > new file mode 100644
> > index 000000000000..65b094f30bf5
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-baije-core.dtsi

[]

> You should add:
> 
> 	chosen {
> 		stdout-path = "serial0:115200n8";
> 	};

I actually have it in .dts, but it's theoretically possible to deploy
the core board in a way that serial0 is *not* a console, so the above
probably will not be valid in all cases in .dtsi.

> 
> 
> 
> > +&reg_dcdc2 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <500000>;
> > +	regulator-max-microvolt = <1300000>;
> 
> Should be:
> 	regulator-min-microvolt = <900000>;
> 	regulator-max-microvolt = <1300000>;

0.81..1.2v according to A133 Datasheet Revision 1.1 Jul.14, 2020?

> 
> > +&reg_dcdc4 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <500000>;
> > +	regulator-max-microvolt = <1300000>;
> > +	regulator-name = "vdd-sys";
> 
> Should be:
> 	regulator-min-microvolt = <810000>;
> 	regulator-max-microvolt = <990000>;
> 	regulator-name = "vcc-usb-sys";

I'm a bit puzzled here: datasheet says 0.9..1.0v
and it has no "Typ" value, similar to VDD_CPU, but
VDD_SYS is not part of OPP tables, so who is going
to adjust this? Or shall it be just

regulator-min-microvolt = <950000>;
regulator-max-microvolt = <950000>;

?

> 
> > +};
> > +
> > +&reg_dcdc5 {
> > +	regulator-always-on;
> > +	regulator-min-microvolt = <800000>;
> > +	regulator-max-microvolt = <1840000>;
> > +	regulator-name = "vcc-dram";
> 
> Should be:
> 	regulator-min-microvolt = <1100000>;
> 	regulator-max-microvolt = <1100000>;
> 	regulator-name = "vcc-dram-2";
> 
> ALDO2 is the main DRAM supply, this is the second one.

Core schematics mentions 1.1V/1.2/1.35/1.5 on this rail...
Currently U-Boot has CONFIG_AXP_DCDC5_VOLT=1100, but potentially
this is adjustable, right? At some point LPDDR4 chips they
are soldering today will be unavailable. And in the current
market it will happen rather sooner than later...

> 
> > +};
> > +
> > +/* DCDC6 unused */
> > +
> > +&reg_dldo1 {
> > +	regulator-min-microvolt = <700000>;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-enable-ramp-delay = <1000>;
> 
> Should be:
> 	regulator-min-microvolt = <1800000>;
> 	regulator-max-microvolt = <1800000>;
> 	regulator-name = "vcc-pg";

Do suggest to drop vendor's

regulator-enable-ramp-delay = <1000>;

in all cases?

> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-baijie-helper.dts b/arch/arm64/boot/dts/allwinner/sun50i-a133-baijie-helper.dts
> > new file mode 100644
> > index 000000000000..ccbca5d0a40c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-baijie-helper.dts

[]

> > +	aliases {
> > +		serial0 = &uart0;
> 
> The is best added to the core dtsi.
> 
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> 
> Ditto.

But it only physically materializes in Helperboard, the carrier.
Potentially this one can be left floating or used for something else.

-- 
Alexander Sverdlin.