Re: [PATCH v2 2/5] arm64: dts: qcom: Add AYN QCS8550 Common

[Konrad Dybcio]

On 3/11/26 6:44 PM, Aaron Kling via B4 Relay wrote:
> From: Teguh Sobirin <teguh@xxxxxxxx>
> 
> This adds a base dtb of everything common between the AYN QCS8550
> devices. It is intended to be extended by device specific overlays.
> 
> Signed-off-by: Teguh Sobirin <teguh@xxxxxxxx>
> Co-developed-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> Signed-off-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> ---

[...]

> +	pwm_fan: pwm-fan {
> +		compatible = "pwm-fan";
> +
> +		fan-supply = <&vdd_fan_5v0>;
> +		pwms = <&pm8550_pwm 3 50000>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&fan_pwm_active>, <&fan_int>;

property-n
property-names

in this order, everywhere, please

> +
> +		pulses-per-revolution = <4>;
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <13 IRQ_TYPE_EDGE_FALLING>;

interrupts-extended = <&tlmm 13 IRQ_...>;

[...]

> +		model = "AYN-Odin2";
> +		audio-routing =
> +				"IN1_HPHL", "HPHL_OUT",

Let's drop this empty linebreak


> +				"IN2_HPHR", "HPHR_OUT",
> +				"AMIC2", "MIC BIAS2",
> +				"TX SWR_INPUT1", "ADC2_OUTPUT";
> +
> +		speaker-i2s-dai-link {
> +			link-name = "Primary MI2S Playback";
> +
> +			cpu {
> +				sound-dai = <&q6apmbedai PRIMARY_MI2S_RX>;
> +			};

'co'dec < 'cp'u, please resort

[...]

> +	vdd_fan_5v0: vdd-fan-5v0-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_fan_5v0";
> +
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +
> +		gpio = <&tlmm 109 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&fan_pwr_active>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};

oh, I didn't know this binding existed.. but it seems valid indeed!

[...]

> +&i2c12 {
> +	clock-frequency = <400000>;
> +	status = "okay";

Let's uniformly keep a \n before status

> +};
> +
> +&i2c_master_hub_0 {
> +	status = "okay";

Please add a clock-frequency
(you can read it back at runtime running a vendor kernel if you don't have a
better source)

[...]

> +	spk_amp_l: spk_amp_l@34 {

underscores are no bueno in node names (between ':' and '@'), and they should
be generic, let's try amplifier@

[...]

> +&iris {
> +	status = "okay";

firmware-name?

[...]

> +&sdhc_2 {
> +	cd-gpios = <&pm8550_gpios 12 GPIO_ACTIVE_LOW>;
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
> +	pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
> +	vmmc-supply = <&vreg_l9b_2p9>;
> +	vqmmc-supply = <&vreg_l8b_1p8>;
> +	max-sd-hs-hz = <37500000>;

It's already in 8550.dtsi, you can drop it

> +	no-sdio;
> +	no-mmc;
> +
> +	qcom,dll-config = <0x0007442c>;

Is that changed in your downstream tree?


[...]

> +&swr1 {
> +	status = "okay";
> +	wcd_rx: codec@0,4 {

Let's keep a \n between properties and the subsequent subnodes,
also file-wide

[...]

> +&tlmm {
> +	gpio-reserved-ranges = <32 8>;
> +
> +	dsi_p_rst_active: dsi-p-rst-active-state {
> +		pins = "gpio133";

https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

Let's order them by the pin index (it's a fairly new development so other
8550 devices don't really have that)

[...]

> +&usb_dp_qmpphy {
> +	vdda-phy-supply = <&vreg_l3e_1p2>;
> +	vdda-pll-supply = <&vreg_l3f_0p88>;
> +
> +	mode-switch;

Already present in sm8550.dtsi

Konrad