[5/5] arm64: dts: qcom: msm8916: Enable modem and WiFi

Message ID 20210312003318.3273536-6-bjorn.andersson@linaro.org
State New
Headers show
Series
  • qcom: wcnss: Allow overriding firmware form DT
Related show

Commit Message

Bjorn Andersson March 12, 2021, 12:33 a.m.
Enable the modem and WiFi subsystems and specify msm8916 specific
firmware path for these and the WCNSS control service.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Bryan O'Donoghue March 15, 2021, 11:57 a.m. | #1
On 12/03/2021 00:33, Bjorn Andersson wrote:
> firmware-name = "qcom/msm8916/WCNSS_qcom_wlan_nv.bin";

On the Linaro debian build I have to do this

+       firmware-name = "wlan/prima/WCNSS_qcom_wlan_nv.bin";

---
bod
Bryan O'Donoghue March 15, 2021, 12:01 p.m. | #2
On 12/03/2021 00:33, Bjorn Andersson wrote:
> Enable the modem and WiFi subsystems and specify msm8916 specific

> firmware path for these and the WCNSS control service.

> 

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---

>   arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++

>   arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-

>   2 files changed, 13 insertions(+), 1 deletion(-)

> 

> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> index 6aef0c2e4f0a..448e3561ef63 100644

> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> @@ -305,6 +305,12 @@ &mdss {

>   	status = "okay";

>   };

>   

> +&mpss {

> +	status = "okay";

> +

> +	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";

> +};

> +

>   &pm8916_resin {

>   	status = "okay";

>   	linux,code = <KEY_VOLUMEDOWN>;

> @@ -312,6 +318,8 @@ &pm8916_resin {

>   

>   &pronto {

>   	status = "okay";

> +

> +	firmware-name = "qcom/msm8916/wcnss.mbn";

>   };


On Debian I have to do this


index 2a6a23cb14ca..597cdc8f51cc 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -33,7 +33,7 @@
  #include "qcom_wcnss.h"

  #define WCNSS_CRASH_REASON_SMEM                422
-#define WCNSS_FIRMWARE_NAME            "wcnss.mdt"
+#define WCNSS_FIRMWARE_NAME            "qcom/msm8916/wcnss.mdt"

so I guess wcnss_probe() -> rproc_alloc() wants this fix too.

---
bod
Bjorn Andersson March 18, 2021, 2:50 p.m. | #3
On Mon 15 Mar 07:01 CDT 2021, Bryan O'Donoghue wrote:

> On 12/03/2021 00:33, Bjorn Andersson wrote:
> > Enable the modem and WiFi subsystems and specify msm8916 specific
> > firmware path for these and the WCNSS control service.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++
> >   arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-
> >   2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > index 6aef0c2e4f0a..448e3561ef63 100644
> > --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > @@ -305,6 +305,12 @@ &mdss {
> >   	status = "okay";
> >   };
> > +&mpss {
> > +	status = "okay";
> > +
> > +	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
> > +};
> > +
> >   &pm8916_resin {
> >   	status = "okay";
> >   	linux,code = <KEY_VOLUMEDOWN>;
> > @@ -312,6 +318,8 @@ &pm8916_resin {
> >   &pronto {
> >   	status = "okay";
> > +
> > +	firmware-name = "qcom/msm8916/wcnss.mbn";
> >   };
> 
> On Debian I have to do this
> 
> 
> index 2a6a23cb14ca..597cdc8f51cc 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -33,7 +33,7 @@
>  #include "qcom_wcnss.h"
> 
>  #define WCNSS_CRASH_REASON_SMEM                422
> -#define WCNSS_FIRMWARE_NAME            "wcnss.mdt"
> +#define WCNSS_FIRMWARE_NAME            "qcom/msm8916/wcnss.mdt"
> 
> so I guess wcnss_probe() -> rproc_alloc() wants this fix too.
> 

Can you confirm that you're saying that you want below patch, which I
just merged?

https://lore.kernel.org/linux-remoteproc/20210312002441.3273183-1-bjorn.andersson@linaro.org/

(Which makes it possible to specify firmware name per platform/board)

Regards,
Bjorn
Bryan O'Donoghue March 19, 2021, 2:41 p.m. | #4
On 18/03/2021 14:50, Bjorn Andersson wrote:
> On Mon 15 Mar 07:01 CDT 2021, Bryan O'Donoghue wrote:
> 
>> On 12/03/2021 00:33, Bjorn Andersson wrote:
>>> Enable the modem and WiFi subsystems and specify msm8916 specific
>>> firmware path for these and the WCNSS control service.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> ---
>>>    arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++
>>>    arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-
>>>    2 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
>>> index 6aef0c2e4f0a..448e3561ef63 100644
>>> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
>>> @@ -305,6 +305,12 @@ &mdss {
>>>    	status = "okay";
>>>    };
>>> +&mpss {
>>> +	status = "okay";
>>> +
>>> +	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
>>> +};
>>> +
>>>    &pm8916_resin {
>>>    	status = "okay";
>>>    	linux,code = <KEY_VOLUMEDOWN>;
>>> @@ -312,6 +318,8 @@ &pm8916_resin {
>>>    &pronto {
>>>    	status = "okay";
>>> +
>>> +	firmware-name = "qcom/msm8916/wcnss.mbn";
>>>    };
>>
>> On Debian I have to do this
>>
>>
>> index 2a6a23cb14ca..597cdc8f51cc 100644
>> --- a/drivers/remoteproc/qcom_wcnss.c
>> +++ b/drivers/remoteproc/qcom_wcnss.c
>> @@ -33,7 +33,7 @@
>>   #include "qcom_wcnss.h"
>>
>>   #define WCNSS_CRASH_REASON_SMEM                422
>> -#define WCNSS_FIRMWARE_NAME            "wcnss.mdt"
>> +#define WCNSS_FIRMWARE_NAME            "qcom/msm8916/wcnss.mdt"
>>
>> so I guess wcnss_probe() -> rproc_alloc() wants this fix too.
>>
> 
> Can you confirm that you're saying that you want below patch, which I
> just merged?
> 
> https://lore.kernel.org/linux-remoteproc/20210312002441.3273183-1-bjorn.andersson@linaro.org/
> 
> (Which makes it possible to specify firmware name per platform/board)
> 
> Regards,
> Bjorn
> 

yep

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Stephan Gerhold March 19, 2021, 3:19 p.m. | #5
Hi Bjorn,

On Thu, Mar 11, 2021 at 04:33:18PM -0800, Bjorn Andersson wrote:
> Enable the modem and WiFi subsystems and specify msm8916 specific
> firmware path for these and the WCNSS control service.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

The changes itself look good to me, but the commit message is really
misleading. It does not mention anywhere that the change actually just
enables "modem" on apq8016-sbc instead of "msm8916". :)

Also, WCNSS was actually enabled before already (with the default
firmware path). In my opinion, it would be clearer to change the
firmware-name for it in an extra patch.

> ---
>  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++
>  arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> index 6aef0c2e4f0a..448e3561ef63 100644
> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> @@ -305,6 +305,12 @@ &mdss {
>  	status = "okay";
>  };
>  
> +&mpss {
> +	status = "okay";
> +
> +	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
> +};
> +
>  &pm8916_resin {
>  	status = "okay";
>  	linux,code = <KEY_VOLUMEDOWN>;
> @@ -312,6 +318,8 @@ &pm8916_resin {
>  
>  &pronto {
>  	status = "okay";
> +
> +	firmware-name = "qcom/msm8916/wcnss.mbn";
>  };
>  

How do I get a .mbn from the wcnss.{mdt,.b??} files provided in the
DB410c firmware package? I guess I should just run them through
https://github.com/andersson/pil-squasher?

Also, is the single file format (mbn) preferred now? Not sure if there
is any significant difference except having less files laying around.

Thanks,
Stephan
Bjorn Andersson May 31, 2021, 4:33 p.m. | #6
On Fri 19 Mar 10:19 CDT 2021, Stephan Gerhold wrote:

> Hi Bjorn,
> 

Hi Stephan,

I thought I wrote you a reply, but I don't see it on lore or in my
mailbox...

> On Thu, Mar 11, 2021 at 04:33:18PM -0800, Bjorn Andersson wrote:
> > Enable the modem and WiFi subsystems and specify msm8916 specific
> > firmware path for these and the WCNSS control service.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> The changes itself look good to me, but the commit message is really
> misleading. It does not mention anywhere that the change actually just
> enables "modem" on apq8016-sbc instead of "msm8916". :)
> 
> Also, WCNSS was actually enabled before already (with the default
> firmware path). In my opinion, it would be clearer to change the
> firmware-name for it in an extra patch.
> 

You're right, I will rewrite the commit message.

> > ---
> >  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 12 ++++++++++++
> >  arch/arm64/boot/dts/qcom/msm8916.dtsi     |  2 +-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > index 6aef0c2e4f0a..448e3561ef63 100644
> > --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> > @@ -305,6 +305,12 @@ &mdss {
> >  	status = "okay";
> >  };
> >  
> > +&mpss {
> > +	status = "okay";
> > +
> > +	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
> > +};
> > +
> >  &pm8916_resin {
> >  	status = "okay";
> >  	linux,code = <KEY_VOLUMEDOWN>;
> > @@ -312,6 +318,8 @@ &pm8916_resin {
> >  
> >  &pronto {
> >  	status = "okay";
> > +
> > +	firmware-name = "qcom/msm8916/wcnss.mbn";
> >  };
> >  
> 
> How do I get a .mbn from the wcnss.{mdt,.b??} files provided in the
> DB410c firmware package? I guess I should just run them through
> https://github.com/andersson/pil-squasher?
> 

Yes, .mdt + .bNN files can be put back together as a .mbn by running
them through the pil-squasher.

It's worth mentioning that the suffix doesn't matter for the loader, so
symlinking the .mdt to .mbn (in either way) will keep the kernel
happy - if that simplifies transitions in either way.

> Also, is the single file format (mbn) preferred now? Not sure if there
> is any significant difference except having less files laying around.
> 

We've had numerous cases where people has managed to mix the parts from
different versions of the firmware, sometimes with the nice result of
the loader complaining that some particular .bNN file is missing
other times just with the signature check (mysteriously) failing.

Lumping the parts into a single .mbn resolves all these issues and
hence I prefer this. It is however not a requirement.

Regards,
Bjorn

Patch

diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 6aef0c2e4f0a..448e3561ef63 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -305,6 +305,12 @@  &mdss {
 	status = "okay";
 };
 
+&mpss {
+	status = "okay";
+
+	firmware-name = "qcom/msm8916/mba.mbn", "qcom/msm8916/modem.mbn";
+};
+
 &pm8916_resin {
 	status = "okay";
 	linux,code = <KEY_VOLUMEDOWN>;
@@ -312,6 +318,8 @@  &pm8916_resin {
 
 &pronto {
 	status = "okay";
+
+	firmware-name = "qcom/msm8916/wcnss.mbn";
 };
 
 &sdhc_1 {
@@ -394,6 +402,10 @@  &wcd_codec {
 	qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
 };
 
+&wcnss_ctrl {
+	firmware-name = "qcom/msm8916/WCNSS_qcom_wlan_nv.bin";
+};
+
 /* Enable CoreSight */
 &cti0 { status = "okay"; };
 &cti1 { status = "okay"; };
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 5353da521974..1118836c15dd 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1738,7 +1738,7 @@  smd-edge {
 
 				label = "pronto";
 
-				wcnss {
+				wcnss_ctrl: wcnss {
 					compatible = "qcom,wcnss";
 					qcom,smd-channels = "WCNSS_CTRL";