shikra-evk: add phone to MACHINE_FEATURES#2665
shikra-evk: add phone to MACHINE_FEATURES#2665Mrigank Dembla (mdembla-bit) wants to merge 1 commit into
Conversation
Dmitry Baryshkov (lumag)
left a comment
There was a problem hiding this comment.
Pleasev start your commit message by describing the issue you are trying to solve.
0a00e70 to
b9b1863
Compare
Hi Dmitry Baryshkov (@lumag) , updated the commit text, as well as PR description. Please let me know if this is fine? |
Did you start your commit message with the description of the issue? No, you didn't. So you can guess my answer. Also please drop the description of the change. "This change does this" is completely redundant, you can see it from the patch itself. Why do you need to repeat yourself? |
b9b1863 to
7e00ed5
Compare
I've updated the commit message based on your feedback to better explain the problem being solved and the role of phone in enabling ModemManager support. |
Dmitry Baryshkov (lumag)
left a comment
There was a problem hiding this comment.
Why do we want to support cellular calls on Shikra?
Test Results 110 files 657 suites 7h 38m 50s ⏱️ For more details on these failures, see this check. Results for commit 2e15d74. ♻️ This comment has been updated with latest results. |
Shikra includes modem support and cellular data connectivity is an expected platform capability. This change enables the standard ModemManager userspace stack required to make use of the modem for WWAN data calls. |
This should be a part of the commit message. Which board has a modem? Is it a PCIe modem or a part of the SoC, etc. |
7e00ed5 to
2e15d74
Compare
Thanks. I've updated the commit message accordingly. It now explicitly states that shikra-evk requires WWAN connectivity support for the modem integrated into the SoC and explains that the phone MACHINE_FEATURE is needed to enable the required ModemManager userspace support. |
Dmitry Baryshkov (lumag)
left a comment
There was a problem hiding this comment.
shikra-evk lacks WWAN connectivity support for the modem integrated
into the SoC.
Lacks, ok. Does it specify, that Shikra has internal modem? No. Which SKUs have it? Does it make sense to split configuration into two different machines (like it is done for QCM6490-IDP vs RB3 Gen2) or does it make sense to keep all of them enabled as a single machine entry?
Is the cellular modem supported by the published qdsp6sw firmware or does it require a separate firmware file, to be licenced from Qualcomm?
The CQM variant supports an integrated CAT4 cellular modem in addition to WLAN running on the Q6. The CQS variant supports Audio and WLAN on the Q6, but does not include cellular modem functionality.
Maintaining separate machine configurations does not provide much value as the there is no difference in the publicly available firmware between the CQM and CQS variants. Even the CDT is shared by these variants.
The qdsp6sw firmware published in linux-firmware.git does not include support for the cellular modem. Cellular modem functionality requires an additional modem firmware package that is provided separately to licensed customers. |
|
Thanks! Now you can turn it into a nice commit message. |
|
Yeah, please fix the commit message, we do have variants which do have such "machine feature", it is just that we are using one board conf supporting multiple variants, which is fine. |
The Shikra CQM variant includes an integrated LTE Cat 4 modem. To support WWAN connectivity on modem-enabled configurations, the userspace modem management stack (ModemManager) must be enabled. qcom-distro enables ModemManager only for machines advertising the 'phone' capability through MACHINE_FEATURES. The CQM and CQS variants share the same publicly available firmware and CDT, so maintaining separate machine definitions provides little value. Note: cellular modem functionality requires an additional modem firmware package provided separately to licensed customers. Signed-off-by: Mrigank Dembla <mdembla@qti.qualcomm.com>
2e15d74 to
ed69165
Compare
|
Thanks. I've pushed an updated commit message covering the CQM integrated modem support and the shared shikra-evk configuration. |
shikra-evk does not support cellular data calls, as modemmanager is
not enabled for this machine.
modemmanager is conditionally enabled at the distro layer based on
'phone' being present in a machine's MACHINE_FEATURES. Add 'phone'
to MACHINE_FEATURES for shikra-evk so that modemmanager gets enabled
and cellular data connectivity is supported.