[PATCH 0/3 v7] Misc SCM changes

Mukesh Ojha posted 3 patches 2 years, 3 months ago
There is a newer version of this series
drivers/firmware/qcom_scm.c            | 32 ++++++++++++++++++++++++++++++--
drivers/pinctrl/qcom/pinctrl-msm.c     | 10 ++++------
include/linux/firmware/qcom/qcom_scm.h |  2 ++
3 files changed, 36 insertions(+), 8 deletions(-)
[PATCH 0/3 v7] Misc SCM changes
Posted by Mukesh Ojha 2 years, 3 months ago
I have given version to this series as v7 as it has already
gone through v6 and later got added to minidump patch series
However, these 3 patches can go independently and has no
relation with minidump hence, separated it from minidump series.

Change from minidump-v5(13/17-15/17):https://lore.kernel.org/lkml/1694429639-21484-1-git-send-email-quic_mojha@quicinc.com/
 - Removed mistakenly added macros.
   https://lore.kernel.org/lkml/9da888dc-401a-4cbb-b616-b4654fa79e35@quicinc.com/
 - Added Acked-by tag from Linus.w to 2/3.

Changes from dload series v6: https://lore.kernel.org/lkml/1680076012-10785-1-git-send-email-quic_mojha@quicinc.com/
 - Rebased it on latest tag available on linux-next
 - Added missed Poovendhan sign-off on 15/17 and tested-by tag from
   Kathiravan. Thanks to him for testing and reminding me of missing sign-off.
 - Addressed comments made on dload mode patch v6 version

Changes in v6:
  - Applied suggested API change(at v4) by [dmitry.baryshkov] 

Changes in v5: https://lore.kernel.org/lkml/1680017869-22421-1-git-send-email-quic_mojha@quicinc.com/
  - Tried to fix the issue reported by kernel test robot
    https://lore.kernel.org/lkml/202303280535.acb66sQT-lkp@intel.com/

  - Applied some of the improvement suggested by [Bjorn.andersson]
 
    . Dropped 'both' instead support full,mini or mini,full for setting download
    mode to collect both minidump and full dump.
    
    . logging improvement.
    

Changes in v4: https://lore.kernel.org/lkml/1679935281-18445-1-git-send-email-quic_mojha@quicinc.com/
  - val should be shifted within the function [srinivas.kandagatla]
    i.e new = (old & ~mask) | (val << ffs(mask) - 1);
  - Added Acked-by [linus.walleij] on pinctrl change.

Changes in v3 : https://lore.kernel.org/lkml/1679070482-8391-1-git-send-email-quic_mojha@quicinc.com/
 - Removed [1] from the series and sent as a separate patch[2], although this series
   should be applied on top [2].
  [1] https://lore.kernel.org/lkml/1677664555-30191-2-git-send-email-quic_mojha@quicinc.com/
  [2] https://lore.kernel.org/lkml/1678979666-551-1-git-send-email-quic_mojha@quicinc.com/
 - Introduce new exported symbol on suggestion from [srinivas.kandagatla]
 - Use the symbol from drivers/pinctrl/qcom/pinctrl-msm.c.
 - Addressed comment given by [dmitry.baryshkov]
 - Converted non-standard Originally-by to Signed-off-by.

Changes in v2: https://lore.kernel.org/lkml/1677664555-30191-1-git-send-email-quic_mojha@quicinc.com/
 - Addressed comment made by [bjorn]
 - Added download mask.
 - Passed download mode as parameter
 - Accept human accepatable download mode string.
 - enable = !!dload_mode
 - Shifted module param callback to somewhere down in
   the file so that it no longer need to know the
   prototype of qcom_scm_set_download_mode()
 - updated commit text.

Mukesh Ojha (3):
  firmware: qcom_scm: provide a read-modify-write function
  pinctrl: qcom: Use qcom_scm_io_update_field()
  firmware: scm: Modify only the download bits in TCSR register

 drivers/firmware/qcom_scm.c            | 32 ++++++++++++++++++++++++++++++--
 drivers/pinctrl/qcom/pinctrl-msm.c     | 10 ++++------
 include/linux/firmware/qcom/qcom_scm.h |  2 ++
 3 files changed, 36 insertions(+), 8 deletions(-)

-- 
2.7.4
Re: [PATCH 0/3 v7] Misc SCM changes
Posted by Dmitry Baryshkov 2 years, 2 months ago
On Wed, 4 Oct 2023 at 20:26, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> I have given version to this series as v7 as it has already
> gone through v6 and later got added to minidump patch series
> However, these 3 patches can go independently and has no
> relation with minidump hence, separated it from minidump series.

Please describe the merge strategy, since these patches cross the
subsystem boundary. Linusw has acked patch2, which might mean that he
is fine for it to be merged via the arm-soc? tree. However as nothing
was mentioned in the cover letter, Bjorn has marked patch2 as not
applicable (as it is outside of the usual subsystem area).

-- 
With best wishes
Dmitry
Re: [PATCH 0/3 v7] Misc SCM changes
Posted by Mukesh Ojha 2 years, 2 months ago

On 10/25/2023 1:37 AM, Dmitry Baryshkov wrote:
> On Wed, 4 Oct 2023 at 20:26, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>>
>> I have given version to this series as v7 as it has already
>> gone through v6 and later got added to minidump patch series
>> However, these 3 patches can go independently and has no
>> relation with minidump hence, separated it from minidump series.
> 
> Please describe the merge strategy, since these patches cross the
> subsystem boundary. Linusw has acked patch2, which might mean that he
> is fine for it to be merged via the arm-soc? tree. However as nothing
> was mentioned in the cover letter, Bjorn has marked patch2 as not
> applicable (as it is outside of the usual subsystem area).

Thanks for your time in reviewing this series.

Sent another version here after addressing the comments.

https://lore.kernel.org/lkml/1698235506-16993-1-git-send-email-quic_mojha@quicinc.com/

-Mukesh



>
Re: [PATCH 0/3 v7] Misc SCM changes
Posted by Kathiravan Thirumoorthy 2 years, 2 months ago
On 10/4/2023 10:55 PM, Mukesh Ojha wrote:
> I have given version to this series as v7 as it has already
> gone through v6 and later got added to minidump patch series
> However, these 3 patches can go independently and has no
> relation with minidump hence, separated it from minidump series.


Mukesh, Can you rebase this series on top of linux-next, since there is 
a conflict?


Bjorn, after rebase is done, will you able to pick it up for v6.7 if 
there is a time? These patches(#1  and #3) are required for the crash 
dump collection on IPQ9574 and IPQ5332 SoCs.


>
> Change from minidump-v5(13/17-15/17):https://lore.kernel.org/lkml/1694429639-21484-1-git-send-email-quic_mojha@quicinc.com/
>   - Removed mistakenly added macros.
>     https://lore.kernel.org/lkml/9da888dc-401a-4cbb-b616-b4654fa79e35@quicinc.com/
>   - Added Acked-by tag from Linus.w to 2/3.
>
> Changes from dload series v6: https://lore.kernel.org/lkml/1680076012-10785-1-git-send-email-quic_mojha@quicinc.com/
>   - Rebased it on latest tag available on linux-next
>   - Added missed Poovendhan sign-off on 15/17 and tested-by tag from
>     Kathiravan. Thanks to him for testing and reminding me of missing sign-off.
>   - Addressed comments made on dload mode patch v6 version
>
> Changes in v6:
>    - Applied suggested API change(at v4) by [dmitry.baryshkov]
>
> Changes in v5: https://lore.kernel.org/lkml/1680017869-22421-1-git-send-email-quic_mojha@quicinc.com/
>    - Tried to fix the issue reported by kernel test robot
>      https://lore.kernel.org/lkml/202303280535.acb66sQT-lkp@intel.com/
>
>    - Applied some of the improvement suggested by [Bjorn.andersson]
>   
>      . Dropped 'both' instead support full,mini or mini,full for setting download
>      mode to collect both minidump and full dump.
>      
>      . logging improvement.
>      
>
> Changes in v4: https://lore.kernel.org/lkml/1679935281-18445-1-git-send-email-quic_mojha@quicinc.com/
>    - val should be shifted within the function [srinivas.kandagatla]
>      i.e new = (old & ~mask) | (val << ffs(mask) - 1);
>    - Added Acked-by [linus.walleij] on pinctrl change.
>
> Changes in v3 : https://lore.kernel.org/lkml/1679070482-8391-1-git-send-email-quic_mojha@quicinc.com/
>   - Removed [1] from the series and sent as a separate patch[2], although this series
>     should be applied on top [2].
>    [1] https://lore.kernel.org/lkml/1677664555-30191-2-git-send-email-quic_mojha@quicinc.com/
>    [2] https://lore.kernel.org/lkml/1678979666-551-1-git-send-email-quic_mojha@quicinc.com/
>   - Introduce new exported symbol on suggestion from [srinivas.kandagatla]
>   - Use the symbol from drivers/pinctrl/qcom/pinctrl-msm.c.
>   - Addressed comment given by [dmitry.baryshkov]
>   - Converted non-standard Originally-by to Signed-off-by.
>
> Changes in v2: https://lore.kernel.org/lkml/1677664555-30191-1-git-send-email-quic_mojha@quicinc.com/
>   - Addressed comment made by [bjorn]
>   - Added download mask.
>   - Passed download mode as parameter
>   - Accept human accepatable download mode string.
>   - enable = !!dload_mode
>   - Shifted module param callback to somewhere down in
>     the file so that it no longer need to know the
>     prototype of qcom_scm_set_download_mode()
>   - updated commit text.
>
> Mukesh Ojha (3):
>    firmware: qcom_scm: provide a read-modify-write function
>    pinctrl: qcom: Use qcom_scm_io_update_field()
>    firmware: scm: Modify only the download bits in TCSR register
>
>   drivers/firmware/qcom_scm.c            | 32 ++++++++++++++++++++++++++++++--
>   drivers/pinctrl/qcom/pinctrl-msm.c     | 10 ++++------
>   include/linux/firmware/qcom/qcom_scm.h |  2 ++
>   3 files changed, 36 insertions(+), 8 deletions(-)
>
Re: [PATCH 0/3 v7] Misc SCM changes
Posted by Dmitry Baryshkov 2 years, 2 months ago
On Tue, 24 Oct 2023 at 16:31, Kathiravan Thirumoorthy
<quic_kathirav@quicinc.com> wrote:
>
>
> On 10/4/2023 10:55 PM, Mukesh Ojha wrote:
> > I have given version to this series as v7 as it has already
> > gone through v6 and later got added to minidump patch series
> > However, these 3 patches can go independently and has no
> > relation with minidump hence, separated it from minidump series.
>
>
> Mukesh, Can you rebase this series on top of linux-next, since there is
> a conflict?
>
>
> Bjorn, after rebase is done, will you able to pick it up for v6.7 if
> there is a time? These patches(#1  and #3) are required for the crash
> dump collection on IPQ9574 and IPQ5332 SoCs.

It is not obvious that they are fixes for the crash. You did not add
Fixes tags, you didn't follow
Documentation/process/stable-kernel-rules.rst. Cover letter is
useless. How can we guess that they are urgent / important?

-- 
With best wishes
Dmitry