Re: [PATCH v5 3/6] arm64: dts: qcom: Add AYN QCS8550 Common

[Aaron Kling]

On Fri, Apr 24, 2026 at 7:11 AM Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
>
> On 4/8/26 9:41 PM, Aaron Kling via B4 Relay wrote:
> > From: Teguh Sobirin <teguh@xxxxxxxx>
> >
> > This contains everything common between the AYN QCS8550 devices. It will
> > be included by device specific dts'.
> >
> > Signed-off-by: Teguh Sobirin <teguh@xxxxxxxx>
> > Co-developed-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> > Signed-off-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> > ---
>
> [...]
>
> > +     // The tzlog label is required by ABL to apply a dtbo, but it can be on any node
>
> I don't know if the policy changed, but I think C-style (/* Foo */)
> comments are still preferred

Ack

> [...]
>
>
> > +     // The arch_timer label is unused here, but is required by ABL to apply a dtbo
> > +     arch_timer: timer { };
>
> ditto

Ack

> [...]
>
> > +&pm8550_gpios {
> > +     fan_pwm_active: fan-pwm-active-state {
> > +             pins = "gpio8";
> > +             function = "func1";
> > +             input-disable;
> > +             output-enable;
> > +             output-low;
>
> Looks like this should be a regulator then, probably?

Mmm, what would it be tied to, then? The fan already has a reg. I
presume just modeling it as an always on reg tied to nothing is
undesirable. I also have no idea what the voltage would be.

> [...]
>
> > +     wcd_default: wcd-reset-n-active-state {
> > +             pins = "gpio108";
> > +             function = "gpio";
> > +             drive-strength = <16>;
> > +             bias-disable;
> > +             output-low;
>
> no need for this property

I'll start with saying that I know basically nothing about qcom
hardware design and what the average pinmuxing layout looks like. But
I do note that a lot of existing devices have this exact same node,
for example the sm8550 hdk [0]. Is there something that makes these
devices different? Or is this unnecessary everywhere?

> > +     };
> > +
> > +     fan_pwr_active: fan-pwr-active-state {
> > +             pins = "gpio109";
> > +             function = "gpio";
> > +             drive-strength = <2>;
> > +             bias-disable;
> > +             output-low;
>
> likewise, especially since it's the opposite of the active state
> defined in the vreg node

Ack, this one makes sense since the fan power sequence will set stuff
as necessary.

> [...]
>
> > +     usb0_sbu_default: usb0-sbu-state {
> > +             oe-n-pins {
> > +                     pins = "gpio140";
> > +                     function = "gpio";
> > +                     bias-disable;
> > +                     drive-strength = <16>;
> > +                     output-high;
>
> This is probably not required too.. unless there's a hw bug?
>
> fwiw 16 mA is a very high drive-strength - does this come from vendor
> sources?

I do not see any pinmux for gpio140 in the downstream dt or anything
matching pi3usb102 at all, I'm not sure how it's handled there. The
original source of this dt was written before there was a public gpl
code release from AYN. I do see other qcom users of the pi3usb102
doing similar however, for example the sc8280xp crd [1]. So I've got
the same question as above: is there something different here, or is
it possible other existing copies of this are also wrong?

Aaron

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8550-hdk.dts?h=v7.0#n1302
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts?h=v7.0#n1175