2020-10-08 13:15:47

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: Enabled MHI device over PCIe

Hi,

On Thu, Oct 08, 2020 at 06:02:24PM +0530, Gokul Sriram Palanisamy wrote:
> Enabled MHI device support over PCIe and added memory
> reservation required for MHI enabled QCN9000 PCIe card.
>
> Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi | 47 ++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> index 0827055..e5c1ec0 100644
> --- a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> @@ -24,6 +24,22 @@
> device_type = "memory";
> reg = <0x0 0x40000000 0x0 0x20000000>;
> };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + qcn9000_pcie0: memory@50f00000 {
> + no-map;
> + reg = <0x0 0x50f00000 0x0 0x03700000>;
> + };
> +
> + qcn9000_pcie1: memory@54600000 {
> + no-map;
> + reg = <0x0 0x54600000 0x0 0x03700000>;
> + };
> + };
> };
>
> &blsp1_spi1 {
> @@ -45,11 +61,42 @@
> &pcie0 {
> status = "ok";
> perst-gpio = <&tlmm 58 0x1>;
> +
> + pcie0_rp: pcie0_rp {
> + reg = <0 0 0 0 0>;
> +
> + status = "ok";
> + mhi_0: qcom,mhi@0 {

MHI doesn't support devicetree as of now so how is this supposed to work?
Have you tested this series with mainline?

Thanks,
Mani

> + reg = <0 0 0 0 0 >;
> +
> + qrtr_instance_id = <0x20>;
> + base-addr = <0x50f00000>;
> + m3-dump-addr = <0x53c00000>;
> + etr-addr = <0x53d00000>;
> + qcom,caldb-addr = <0x53e00000>;
> + };
> + };
> };
>
> &pcie1 {
> status = "ok";
> perst-gpio = <&tlmm 61 0x1>;
> +
> + pcie1_rp: pcie1_rp {
> + reg = <0 0 0 0 0>;
> +
> + status = "ok";
> + mhi_1: qcom,mhi@1 {
> + reg = <0 0 0 0 0 >;
> +
> + qrtr_instance_id = <0x21>;
> + base-addr = <0x54600000>;
> + m3-dump-addr = <0x57300000>;
> + etr-addr = <0x57400000>;
> + qcom,caldb-addr = <0x57500000>;
> + };
> + };
> + };
> };
>
> &qmp_pcie_phy0 {
> --
> 2.7.4
>


2020-10-08 18:45:36

by Gokul Sriram Palanisamy

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: Enabled MHI device over PCIe

On 2020-10-08 18:41, Manivannan Sadhasivam wrote:
> Hi,
>
> On Thu, Oct 08, 2020 at 06:02:24PM +0530, Gokul Sriram Palanisamy
> wrote:
>> Enabled MHI device support over PCIe and added memory
>> reservation required for MHI enabled QCN9000 PCIe card.
>>
>> Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi | 47
>> ++++++++++++++++++++++++++++++
>> 1 file changed, 47 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> index 0827055..e5c1ec0 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> @@ -24,6 +24,22 @@
>> device_type = "memory";
>> reg = <0x0 0x40000000 0x0 0x20000000>;
>> };
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + qcn9000_pcie0: memory@50f00000 {
>> + no-map;
>> + reg = <0x0 0x50f00000 0x0 0x03700000>;
>> + };
>> +
>> + qcn9000_pcie1: memory@54600000 {
>> + no-map;
>> + reg = <0x0 0x54600000 0x0 0x03700000>;
>> + };
>> + };
>> };
>>
>> &blsp1_spi1 {
>> @@ -45,11 +61,42 @@
>> &pcie0 {
>> status = "ok";
>> perst-gpio = <&tlmm 58 0x1>;
>> +
>> + pcie0_rp: pcie0_rp {
>> + reg = <0 0 0 0 0>;
>> +
>> + status = "ok";
>> + mhi_0: qcom,mhi@0 {
>
> MHI doesn't support devicetree as of now so how is this supposed to
> work?
> Have you tested this series with mainline?
>
> Thanks,
> Mani
>

Hi Mani,
This node entries will be consumed by ath11k driver and is not supposed
to be consumed by mhi driver.
And yes, it is tested on Mainline.

Regards,
Gokul

>> + reg = <0 0 0 0 0 >;
>> +
>> + qrtr_instance_id = <0x20>;
>> + base-addr = <0x50f00000>;
>> + m3-dump-addr = <0x53c00000>;
>> + etr-addr = <0x53d00000>;
>> + qcom,caldb-addr = <0x53e00000>;
>> + };
>> + };
>> };
>>
>> &pcie1 {
>> status = "ok";
>> perst-gpio = <&tlmm 61 0x1>;
>> +
>> + pcie1_rp: pcie1_rp {
>> + reg = <0 0 0 0 0>;
>> +
>> + status = "ok";
>> + mhi_1: qcom,mhi@1 {
>> + reg = <0 0 0 0 0 >;
>> +
>> + qrtr_instance_id = <0x21>;
>> + base-addr = <0x54600000>;
>> + m3-dump-addr = <0x57300000>;
>> + etr-addr = <0x57400000>;
>> + qcom,caldb-addr = <0x57500000>;
>> + };
>> + };
>> + };
>> };
>>
>> &qmp_pcie_phy0 {
>> --
>> 2.7.4
>>

2020-10-09 08:28:24

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: Enabled MHI device over PCIe

On Thu, Oct 08, 2020 at 11:03:42PM +0530, [email protected] wrote:
> On 2020-10-08 18:41, Manivannan Sadhasivam wrote:
> > Hi,
> >
> > On Thu, Oct 08, 2020 at 06:02:24PM +0530, Gokul Sriram Palanisamy wrote:
> > > Enabled MHI device support over PCIe and added memory
> > > reservation required for MHI enabled QCN9000 PCIe card.
> > >
> > > Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi | 47
> > > ++++++++++++++++++++++++++++++
> > > 1 file changed, 47 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> > > b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> > > index 0827055..e5c1ec0 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi

[...]

> > > + pcie0_rp: pcie0_rp {
> > > + reg = <0 0 0 0 0>;
> > > +
> > > + status = "ok";
> > > + mhi_0: qcom,mhi@0 {
> >
> > MHI doesn't support devicetree as of now so how is this supposed to
> > work?
> > Have you tested this series with mainline?
> >
> > Thanks,
> > Mani
> >
>
> Hi Mani,
> This node entries will be consumed by ath11k driver and is not supposed to
> be consumed by mhi driver.
> And yes, it is tested on Mainline.
>

Can you please point me to the relevant binding or the code which consumes this
change?

Also please explain what it does! For enabling MHI support over PCIe you don't
need this node at all. You just need to define the PCIe device ID in the ath11k
driver and that's it.

Adding Kalle to this thread...

Thanks,
Mani

> Regards,
> Gokul
>
> > > + reg = <0 0 0 0 0 >;
> > > +
> > > + qrtr_instance_id = <0x20>;
> > > + base-addr = <0x50f00000>;
> > > + m3-dump-addr = <0x53c00000>;
> > > + etr-addr = <0x53d00000>;
> > > + qcom,caldb-addr = <0x53e00000>;
> > > + };
> > > + };
> > > };
> > >
> > > &pcie1 {
> > > status = "ok";
> > > perst-gpio = <&tlmm 61 0x1>;
> > > +
> > > + pcie1_rp: pcie1_rp {
> > > + reg = <0 0 0 0 0>;
> > > +
> > > + status = "ok";
> > > + mhi_1: qcom,mhi@1 {
> > > + reg = <0 0 0 0 0 >;
> > > +
> > > + qrtr_instance_id = <0x21>;
> > > + base-addr = <0x54600000>;
> > > + m3-dump-addr = <0x57300000>;
> > > + etr-addr = <0x57400000>;
> > > + qcom,caldb-addr = <0x57500000>;
> > > + };
> > > + };
> > > + };
> > > };
> > >
> > > &qmp_pcie_phy0 {
> > > --
> > > 2.7.4
> > >

2020-10-09 12:52:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: dts: Enabled MHI device over PCIe

+ ath11k list

Manivannan Sadhasivam <[email protected]> writes:

> On Thu, Oct 08, 2020 at 11:03:42PM +0530, [email protected] wrote:
>> On 2020-10-08 18:41, Manivannan Sadhasivam wrote:
>> > Hi,
>> >
>> > On Thu, Oct 08, 2020 at 06:02:24PM +0530, Gokul Sriram Palanisamy wrote:
>> > > Enabled MHI device support over PCIe and added memory
>> > > reservation required for MHI enabled QCN9000 PCIe card.
>> > >
>> > > Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
>> > > ---
>> > > arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi | 47
>> > > ++++++++++++++++++++++++++++++
>> > > 1 file changed, 47 insertions(+)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> > > b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> > > index 0827055..e5c1ec0 100644
>> > > --- a/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>> > > +++ b/arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi
>
> [...]
>
>> > > + pcie0_rp: pcie0_rp {
>> > > + reg = <0 0 0 0 0>;
>> > > +
>> > > + status = "ok";
>> > > + mhi_0: qcom,mhi@0 {
>> >
>> > MHI doesn't support devicetree as of now so how is this supposed to
>> > work?
>> > Have you tested this series with mainline?
>> >
>> > Thanks,
>> > Mani
>> >
>>
>> Hi Mani,
>> This node entries will be consumed by ath11k driver and is not supposed to
>> be consumed by mhi driver.
>> And yes, it is tested on Mainline.

Upstream ath11k does not yet support QCN9074 so I don't see how this is
tested with mainline ath11k. You must be using some out-of-tree
_unofficial_ ath11k patches.

> Can you please point me to the relevant binding or the code which consumes this
> change?
>
> Also please explain what it does! For enabling MHI support over PCIe you don't
> need this node at all. You just need to define the PCIe device ID in the ath11k
> driver and that's it.
>
> Adding Kalle to this thread...

So currently QCN9074 firmware needs 55 MB of contiguous host memory and
I suspect one reason for these DT entries is an ugly hack to provide
that memory range to the firmware.

We are currently preparing QCN9074 patches for ath11k and finding a
solution how to implement these properly in ath11k. Hopefully there's no
need for hacks like this, we know more once we get the ath11k QCN9074
patches ready. Please drop this patch.

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

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