Hi Jimmy,
Am Donnerstag, 31. Oktober 2024, 04:14:51 CET schrieb Jimmy Hon:
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts
> new file mode 100644
> index 000000000000..d31b13f99fdc
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-max.dts
> @@ -0,0 +1,887 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,vop2.h>
> +#include "rk3588.dtsi"
can you check how similar the rk3588-orangepi--5-max is to the
already existing orangepi-5-plus please?
For rk3588s-orangepi-5 and orangepi-5b we already have this nicely
shared dtsi for commit things, and somehow I assume this might be
similar for 5-plus vs 5-max.
> +
> +/ {
> + model = "Xunlong Orange Pi 5 Max";
> + compatible = "xunlong,orangepi-5-max", "rockchip,rk3588";
> +
[...]
> + /* PMIC_EXT_EN */
> + vcc_1v1_nldo_s3: vcc-1v1-ndlo-s3-regulator {
vcc_1v1_nldo_s3: regulator-vcc-1v1-ndlo-s3 {
preferred pattern for fixed-regulator nodenames is regulator-......
Same for the othe fixed regulators below.
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_1v1_nldo_s3";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + vin-supply = <&vcc5v0_sys>;
> + };
> +
> + /* pcie eth. not a real regulator. 33VAUX */
> + vcc_3v3_aux: vcc-3v3-aux-regulator {
> + compatible = "regulator-fixed";
> + /* Ethernet_power_en */
> + gpios = <&gpio0 RK_PD3 GPIO_ACTIVE_LOW>;
> + regulator-name = "33vaux";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + startup-delay-us = <50000>;
> + vin-supply = <&vcc_3v3_s3>;
> + };
> +
> + vcc3v3_pcie30: vcc3v3-pcie30-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + /* PCIE_PWREN_H */
> + gpios = <&gpio2 RK_PB6 GPIO_ACTIVE_HIGH>;
> + regulator-name = "vcc3v3_pcie30";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + startup-delay-us = <5000>;
> + vin-supply = <&vcc5v0_sys>;
> + };
> +
> + /* Regulator is enabled whenever vcc_1v8_s0 is on */
> + vcc_3v3_s0: vcc-3v3-s0-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_3v3_s0";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <&vcc_3v3_s3>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + vcc5v0_sys: vcc5v0-sys-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc5v0_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + /* Represents the vcc5v0_usb20 in the schematic */
> + /* Also represents the vcc5v0_usb30 in the schematic,
> + both regulators share the same enable gpio */
please make that one comment block and use appropriate formatting
/*
* Represents the vcc5v0_usb20 and vcc5v0_usb30 in the schematic,
* both regulators share the same enable gpio
*/
> + vcc5v0_host: vcc5v0-host-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + /* USB_HOST_PWREN */
> + gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usb_host_pwren>;
> + regulator-name = "vcc5v0_host";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + vin-supply = <&vcc5v0_sys>;
> + };
> +
> + vcc5v0_usb30_otg: vcc5v0-usb30-otg-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + /* USB_OTG_PWREN */
> + gpios = <&gpio4 RK_PB3 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usb_otg_pwren>;
> + regulator-name = "vcc5v0_usb30_otg";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + vin-supply = <&vcc5v0_sys>;
> + };
> +};
> + hym8563: rtc@51 {
> + compatible = "haoyu,hym8563";
> + reg = <0x51>;
> + /* RTC_INT_L */
what does this comment try to explain? If that is the name of the pin,
just name the pinctrl below accordingly (instead of hym8563_int
if applicable) .
> + interrupt-parent = <&gpio0>;
> + interrupts = <RK_PC4 IRQ_TYPE_LEVEL_LOW>;
> + #clock-cells = <0>;
> + clock-output-names = "hym8563";
> + pinctrl-names = "default";
> + pinctrl-0 = <&hym8563_int>;
> + wakeup-source;
> + };
> +};
> +
> +&pwm9 {
> + pinctrl-names = "active";
this needs to be "default" to actually work
> + pinctrl-0 = <&pwm9m2_pins>;
> + status = "okay";
> +};
Thanks
Heiko
