Re: [PATCH] arm64: dts: qcom: sa8775p-ride: add WiFi/BT nodes

[Kalle Valo <kvalo@kernel.org>]

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On Mon, 9 Sept 2024 at 14:31, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> > On Fri, Sep 06, 2024 at 08:19:28AM GMT, Miaoqing Pan wrote:
>> >
>> >>
>> >>
>> >> On 9/5/2024 8:49 PM, Dmitry Baryshkov wrote:
>> >> > On Thu, Sep 05, 2024 at 02:48:17PM GMT, Miaoqing Pan wrote:
>> >> > > Add a node for the PMU module of the WCN6855 present on the sa8775p-ride
>> >> > > board. Assign its LDO power outputs to the existing WiFi/Bluetooth module.
>> >> > >
>> >> > > Signed-off-by: Miaoqing Pan <quic_miaoqing@quicinc.com>
>> >> > > ---
>> >> > >   arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 119 +++++++++++++++++++++
>> >> > >   arch/arm64/boot/dts/qcom/sa8775p.dtsi      |   2 +-
>> >> > >   2 files changed, 120 insertions(+), 1 deletion(-)
>> >> > >
>> >> > > @@ -837,3 +939,20 @@ &usb_2_hsphy {
>> >> > >   &xo_board_clk {
>> >> > >          clock-frequency = <38400000>;
>> >> > >   };
>> >> > > +
>> >> > > +&pcieport0 {
>> >> > > +        wifi@0 {
>> >> > > +                compatible = "pci17cb,1101";
>> >> > > +                reg = <0x10000 0x0 0x0 0x0 0x0>;
>> >> > > +
>> >> > > +                vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
>> >> > > +                vddaon-supply = <&vreg_pmu_aon_0p59>;
>> >> > > +                vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
>> >> > > +                vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
>> >> > > +                vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>;
>> >> > > +                vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>;
>> >> > > +                vddrfa1p7-supply = <&vreg_pmu_rfa_1p7>;
>> >> > > +                vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>;
>> >> > > +                vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>;
>> >> >
>> >> > Please add
>> >> >
>> >> > qcom,ath11k-calibration-variant = "name";
>> >>
>> >> No need, here the WiFi node is for 'drivers/pci/pwrctl', not ath11k driver.
>> >
>> > NAK, nodes describe hardware, not drivers. And we have had enough issues
>> > with the WCN wifi having collisions on the board-id / chip-id / etc.
>> >
>> > Maybe we should make calibration-data required for the DT-based systems?
>> > Kalle, WDYT?
>>
>> I don't know exactly what hardware you are referring so this is just a
>> quick and vague answer, take all this as grain of salt.
>>
>> I don't have any numbers but I'm assuming most of the
>> ath10k/ath11k/ath12k devices have the calibration data stored in OTP
>> inside the chip. There are also devices which store the calibration
>> outside the chip, for example in DT, but my understanding is that they
>> are a minority.
>
> Please excuse me, the comment was about the
> qcom,athNk-calibration-variant

Ah :) In case you were wondering, I was talking above about
qcom,ath10k-calibration-data property which is the full calibration
data, board files are not used at all in that case. So please ignore all
that I said.

> the property used to identify the BDF within board-2.bin. Currently
> it is optional, which leads to developers omitting it. I think it's
> worth making it a required property for new DT devices, making sure
> that we don't have an issue with the plenty of boards programmed to
> use 0xff as the board_id. Or just using the same ID, but different BDF
> files.

This is also a quick answer, the merge window is getting close and we
are finalising the ath12k MLO support so not much free time right now.

Some chipsets do provide unique information for the ath10k/ath11k/ath12k
driver to choose the correct board file, but I guess they are mostly
mobile chipsets. Though not even all mobile chipsets provide that, as
you are painfully aware. For AP chipsets it seems to be a rule that they
don't provide unique information to choose the board file and a variant
field is always needed. So I think there is a point in your proposal.

Backward compatibility is important but I think we already handle that,
at least in ath11k, as it's first try with variant field and then
without variant. But this needs more investigation on both ath11k and
ath12k. I hope for ath10k we would not need to touch the driver anymore.

Can we revisit this in few weeks, after MLO is in better shape, and
maybe you could start a new thread?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches