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

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

[Dmitry Baryshkov]

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?

-- 
With best wishes
Dmitry

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

[Kalle Valo]

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.

If we were to require that the calibration data needs to be in DT, and
not use OTP at all, that would limit the on types of devices ath11k can
be used. And I don't even know how we could easily extract the
calibration data from OTP and it would be extra work for everyone.
Honestly I don't really see any benefits from this, only negatives.

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

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

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

[Dmitry Baryshkov]

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, 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.

-- 
With best wishes
Dmitry

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

[Kalle Valo]

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

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

[Dmitry Baryshkov]

On Mon, Sep 09, 2024 at 03:27:48PM GMT, Kalle Valo wrote:
> 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.

Ack, thanks :-)

> 
> 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?

Sure. Let's agree that I'll send a patch after -rc1, if I don't forget
about it.

-- 
With best wishes
Dmitry