| Message ID | 20220912232526.27427-1-quic_jjohnson@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Make QMI message rules const | expand |
On 9/12/22 6:25 PM, Jeff Johnson wrote: > Change ff6d365898d ("soc: qcom: qmi: use const for struct > qmi_elem_info") allows QMI message encoding/decoding rules to be > const. So now update the definitions in the various client to take > advantage of this. Patches for ath10k and ath11k were perviously sent > separately. I have had this on my "to-do list" for ages. The commit you mention updates the code to be explicit about not modifying this data, which is great. I scanned over the changes, and I assume that all you did was make every object having the qmi_elem_info structure type be defined as constant. Why aren't you changing the "ei_array" field in the qmi_elem_info structure to be const? Or the "ei" field of the qmi_msg_handler structure? And the qmi_response_type_v01_ei array (and so on)? I like what you're doing, but can you comment on what your plans are beyond this series? Do you intend to make the rest of these fields const? Thanks. -Alex > This series depends upon: > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c > > This is in the for-next banch of: > git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git > > Hence this series is also based upon that tree/branch. > > Jeff Johnson (4): > net: ipa: Make QMI message rules const > remoteproc: sysmon: Make QMI message rules const > slimbus: qcom-ngd-ctrl: Make QMI message rules const > soc: qcom: pdr: Make QMI message rules const > > drivers/net/ipa/ipa_qmi_msg.c | 20 ++++++++++---------- > drivers/net/ipa/ipa_qmi_msg.h | 20 ++++++++++---------- > drivers/remoteproc/qcom_sysmon.c | 8 ++++---- > drivers/slimbus/qcom-ngd-ctrl.c | 8 ++++---- > drivers/soc/qcom/pdr_internal.h | 20 ++++++++++---------- > 5 files changed, 38 insertions(+), 38 deletions(-) >
On 9/13/2022 6:58 AM, Alex Elder wrote: > On 9/12/22 6:25 PM, Jeff Johnson wrote: >> Change ff6d365898d ("soc: qcom: qmi: use const for struct >> qmi_elem_info") allows QMI message encoding/decoding rules to be >> const. So now update the definitions in the various client to take >> advantage of this. Patches for ath10k and ath11k were perviously sent >> separately. > > I have had this on my "to-do list" for ages. > The commit you mention updates the code to be > explicit about not modifying this data, which > is great. > > I scanned over the changes, and I assume that > all you did was make every object having the > qmi_elem_info structure type be defined as > constant. > > Why aren't you changing the "ei_array" field in > the qmi_elem_info structure to be const? Or the > "ei" field of the qmi_msg_handler structure? And > the qmi_response_type_v01_ei array (and so on)? > > I like what you're doing, but can you comment > on what your plans are beyond this series? > Do you intend to make the rest of these fields > const? Hi Alex, My primary focus is the ath* wireless drivers, and my primary goal was to make the tables there const. So this series, along with the two out-of-series patches for ath10k and ath11k complete that scope of work. The lack of the other changes to the QMI data structures is simply due to me not looking in depth at the QMI code beyond the registration interface. I'll be happy to revisit this as a separate cleanup. /jeff
On 9/13/22 1:51 PM, Jeff Johnson wrote: > On 9/13/2022 6:58 AM, Alex Elder wrote: >> On 9/12/22 6:25 PM, Jeff Johnson wrote: >>> Change ff6d365898d ("soc: qcom: qmi: use const for struct >>> qmi_elem_info") allows QMI message encoding/decoding rules to be >>> const. So now update the definitions in the various client to take >>> advantage of this. Patches for ath10k and ath11k were perviously sent >>> separately. >> >> I have had this on my "to-do list" for ages. >> The commit you mention updates the code to be >> explicit about not modifying this data, which >> is great. >> >> I scanned over the changes, and I assume that >> all you did was make every object having the >> qmi_elem_info structure type be defined as >> constant. >> >> Why aren't you changing the "ei_array" field in >> the qmi_elem_info structure to be const? Or the >> "ei" field of the qmi_msg_handler structure? And >> the qmi_response_type_v01_ei array (and so on)? >> >> I like what you're doing, but can you comment >> on what your plans are beyond this series? >> Do you intend to make the rest of these fields >> const? > > Hi Alex, > My primary focus is the ath* wireless drivers, and my primary goal was > to make the tables there const. So this series, along with the two > out-of-series patches for ath10k and ath11k complete that scope of work. > > The lack of the other changes to the QMI data structures is simply due > to me not looking in depth at the QMI code beyond the registration > interface. > > I'll be happy to revisit this as a separate cleanup. Sounds good to me. Like I said I've wanted to do this myself, and as long as you've gotten this far I'd like to see it taken to completion. Compile-testing is most likely sufficient to make sure you got it right. I cherry-picked the one commit, and downloaded the series and found no new build warnings. Checkpatch would prefer you used "ff6d365898d4" rather than "ff6d365898d" for the commit ID, but that's OK. Anyway, for the whole series: Reviewed-by: Alex Elder <elder@linaro.org> > /jeff >
On 9/13/2022 1:21 PM, Alex Elder wrote: > I cherry-picked the one commit, and downloaded the series > and found no new build warnings. Checkpatch would prefer > you used "ff6d365898d4" rather than "ff6d365898d" for the > commit ID, but that's OK. I'll clean that up in a v2 > > Anyway, for the whole series: > > Reviewed-by: Alex Elder <elder@linaro.org> Thanks!
Commit ff6d365898d4 ("soc: qcom: qmi: use const for struct
qmi_elem_info") allows QMI message encoding/decoding rules to be
const. So now update the definitions in the various clients to take
advantage of this. Patches for ath10k and ath11k were previously sent
separately.
This series depends upon:
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c
This is in the for-next branch of:
git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
Hence this series is also based upon that tree/branch.
Changes in v2 (applied to all 4 patches in the series):
- Fixed the truncated hash ff6d365898d ==> ff6d365898d4
- Added RB tags
Jeff Johnson (4):
net: ipa: Make QMI message rules const
remoteproc: sysmon: Make QMI message rules const
slimbus: qcom-ngd-ctrl: Make QMI message rules const
soc: qcom: pdr: Make QMI message rules const
drivers/net/ipa/ipa_qmi_msg.c | 20 ++++++++++----------
drivers/net/ipa/ipa_qmi_msg.h | 20 ++++++++++----------
drivers/remoteproc/qcom_sysmon.c | 8 ++++----
drivers/slimbus/qcom-ngd-ctrl.c | 8 ++++----
drivers/soc/qcom/pdr_internal.h | 20 ++++++++++----------
5 files changed, 38 insertions(+), 38 deletions(-)
Following up on: On 9/13/2022 6:58 AM, Alex Elder wrote: > Why aren't you changing the "ei_array" field in > the qmi_elem_info structure to be const? Or the > "ei" field of the qmi_msg_handler structure? And > the qmi_response_type_v01_ei array (and so on)? All of these suggestions were actually part of the prerequisite patch: <https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c> So I think all of the comments have been addressed. Thanks! /jeff
On Wed, 14 Sep 2022 16:47:01 -0700, Jeff Johnson wrote: > Commit ff6d365898d4 ("soc: qcom: qmi: use const for struct > qmi_elem_info") allows QMI message encoding/decoding rules to be > const. So now update the definitions in the various clients to take > advantage of this. Patches for ath10k and ath11k were previously sent > separately. > > This series depends upon: > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c > > [...] Applied, thanks! [1/4] net: ipa: Make QMI message rules const (no commit info) [2/4] remoteproc: sysmon: Make QMI message rules const (no commit info) [3/4] slimbus: qcom-ngd-ctrl: Make QMI message rules const (no commit info) [4/4] soc: qcom: pdr: Make QMI message rules const commit: afc7b849ebcf063ca84a79c749d4996a8781fc55 Best regards,
On Wed, 14 Sep 2022 16:47:01 -0700, Jeff Johnson wrote: > Commit ff6d365898d4 ("soc: qcom: qmi: use const for struct > qmi_elem_info") allows QMI message encoding/decoding rules to be > const. So now update the definitions in the various clients to take > advantage of this. Patches for ath10k and ath11k were previously sent > separately. > > This series depends upon: > https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c > > [...] Applied, thanks! [1/4] net: ipa: Make QMI message rules const (no commit info) [2/4] remoteproc: sysmon: Make QMI message rules const commit: 7bd156cbbd0add4b869a7d997d057b76c329f4e5 [3/4] slimbus: qcom-ngd-ctrl: Make QMI message rules const (no commit info) [4/4] soc: qcom: pdr: Make QMI message rules const (no commit info) Best regards,
Change ff6d365898d ("soc: qcom: qmi: use const for struct qmi_elem_info") allows QMI message encoding/decoding rules to be const. So now update the definitions in the various client to take advantage of this. Patches for ath10k and ath11k were perviously sent separately. This series depends upon: https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=ff6d365898d4d31bd557954c7fc53f38977b491c This is in the for-next banch of: git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git Hence this series is also based upon that tree/branch. Jeff Johnson (4): net: ipa: Make QMI message rules const remoteproc: sysmon: Make QMI message rules const slimbus: qcom-ngd-ctrl: Make QMI message rules const soc: qcom: pdr: Make QMI message rules const drivers/net/ipa/ipa_qmi_msg.c | 20 ++++++++++---------- drivers/net/ipa/ipa_qmi_msg.h | 20 ++++++++++---------- drivers/remoteproc/qcom_sysmon.c | 8 ++++---- drivers/slimbus/qcom-ngd-ctrl.c | 8 ++++---- drivers/soc/qcom/pdr_internal.h | 20 ++++++++++---------- 5 files changed, 38 insertions(+), 38 deletions(-)