Message-ID: <93f916d1-83b9-41c0-bb05-a785fb730088@oss.qualcomm.com>
Date: Tue, 1 Apr 2025 23:15:03 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Aleksandrs Vinarskis <alex.vinarskis@...il.com>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Dmitry Baryshkov <lumag@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Abel Vesa <abel.vesa@...aro.org>,
        Johan Hovold <johan+linaro@...nel.org>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, Konrad Dybcio <konradybcio@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1 6/6] arm64: dts: qcom: Add support for X1-based Asus
 Zenbook A14

On 4/1/25 8:05 PM, Aleksandrs Vinarskis wrote:
> On Tue, 1 Apr 2025 at 17:59, Konrad Dybcio
> <konrad.dybcio@....qualcomm.com> wrote:
>>
>> On 3/31/25 11:53 PM, Aleksandrs Vinarskis wrote:
>>> Initial support for Asus Zenbook A14. Particular moddel exists
>>> in X1-26-100, X1P-42-100 (UX3407QA) and X1E-78-100 (UX3407RA).
>>>
>>> Mostly similar to other X1-based laptops. Notable differences are:
>>> * Wifi/Bluetooth combo being Qualcomm FastConnect 6900 on UX3407QA
>>>   and Qualcomm FastConnect 7800 on UX3407RA
>>> * USB Type-C retimers are Parade PS8833, appear to behave identical
>>>   to Parade PS8830
>>> * gpio90 is TZ protected
>>

[...]

>>
>>> +&spi10 {
>>> +     status = "disabled";
>>> +
>>> +     /* Unknown device */
>>> +};
>>
>> Does the device crash if you enable this bus? Keeping it 'okay' would
>> make it easier for folks to poke at it
> 
> It does boot just fine, but does not initialize:
> ```
> geni_spi a88000.spi: Invalid proto 9
> ...
> qnoc-x1e80100 interconnect-1: sync_state() pending due to a88000.spi
> ...
> ```
> 
> I only quickly checked that 9 is indeed invalid state, iirc should've
> been 2. But haven't looked deeper into it, so left it disabled. So I
> thought best to leave it off for now. Unless you prefer to drop it
> altogether?

That means this QUP is configured to work as a QSPI host, which is not yet 
supported upstream. I looked at the DSDT you submitted to aa64-laptops, but
there doesn't seem to be anything connected there, perhaps it's loaded at
runtime. Since your keyboard and touchpad work, maybe it's a touchscreen?


> 
>>
>>> +
>>> +&tlmm {
>>> +     gpio-reserved-ranges = <44 4>,  /* SPI11, TZ Protected */
>>> +                            <90 1>;  /* Unknown, TZ Protected */
>>> +
>>> +     bt_en_default: bt-en-sleep {
>>> +             pins = "gpio116";
>>> +             function = "gpio";
>>> +             output-low;
>>> +             bias-disable;
>>> +             qcom,drive-strength = <16>;
>>
>> drop "qcom," and please keep the order of:
>>
>> pins
>> function
>> drive-strength
>> bias
>> output/input
>>
>> as you did below
> 
> Will do.
> 
> Should I also drop 'qcom,' from the 'misc_3p3_reg_en' and adjust order
> the same way, or that one is somehow special?

Sort of. &tlmm and &pm8xxx_gpios use two different drivers, each one
of which has slightly different expectations about their subnodes.
 > 
>>
>>> +
>>> +/ {
>>> +     model = "ASUS Zenbook A14 UX3407RA";
>>
>> There's no strict policy, but variants usually go in braces
> 
> Parenthesis I guess, "ASUS Zenbook A14 (UX3407RA)" ?

Ugh, yes!

[...]

>>> +
>>> +&gpu_zap_shader {
>>> +     firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qcdxkmsuc8380.mbn";
>>> +};
>>
>> This file is not going to work on this SoC, you can drop it
> 
> I guess it would need a different firmware name? If yes, can we
> already add the new name, such that once x1p42100 gains GPU support it
> will get enabled 'automatically'?

The filename is different indeed. You can add it, as currently this
property is not yet consumed by anything, anyway.

[...]

>>
>>> +
>>> +&remoteproc_adsp {
>>> +     firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qcadsp8380.mbn",
>>> +                     "qcom/x1p42100/ASUSTeK/zenbook-a14/adsp_dtbs.elf";
>>> +
>>> +     status = "okay";
>>> +};
>>> +
>>> +&remoteproc_cdsp {
>>> +     firmware-name = "qcom/x1p42100/ASUSTeK/zenbook-a14/qccdsp8380.mbn",
>>> +                     "qcom/x1p42100/ASUSTeK/zenbook-a14/cdsp_dtbs.elf";
>>> +
>>> +     status = "okay";
>>
>> Are the DSP firmware files actually different between the two?
> 
> CDSP is the same. ADSP blobs to my surprise are different, both '.elf'
> and '.mbn'. But like I wrote in the cover letter, perhaps Asus just
> forgot to update adsp firmware? Though according to changelong on
> device pages [2],[4] both have "ADSP Driver : 2.0.4135.0200"
> 
> Compared by:
> * Downloading UX3407QA's drivers [1], from the device page [2] and
> UX3407RA'a drivers [3] from the device page [4]
> * Extract and flatten with `7z e filename.exe`
> * Compare via `md5sum *dsp*elf *dsp*mbn *dsp*jsn`
> 
> Though, even if the blobs would be/will be the same, I think it is
> still beneficial to define firmware path per model, as this makes
> firmware extraction from driver/Windows partition and placement much
> easier. Unfortunately, as it seems so far, most of the devices besides
> Lenovos are not having firmware upstreamed, so this is pretty
> relevant. Eg. Ubuntu already has 'firmware extracting tool' [5] (draft
> MR to include Zenbook as well), I'm guessing other distros have
> something similar, though I haven't followed up.
> 
> On the other hand, these tools could of course get path from device
> tree directly, eg. via `cat
> /sys/firmware/devicetree/base/soc@...emoteproc@...00000/firmware-name`,
> then having all the blobs for the device in one location is less
> relevant...

More importantly, different blobs may (but don't necessarily have to) include
different, hardcoded expectations about the board (or platform) they run on.
So let's keep them separate.

Konrad

Powered by blists - more mailing lists