[PATCH 0/3 v7] Misc SCM changes

[PATCH 0/3 v7] Misc SCM changes

[Mukesh Ojha]

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

[PATCH v7 1/3] firmware: qcom_scm: provide a read-modify-write function

[Mukesh Ojha]

It was realized by Srinivas K. that there is a need of
read-modify-write scm exported function so that it can
be used by multiple clients.

Let's introduce qcom_scm_io_update_field() which masks
out the bits and write the passed value to that bit-offset.

Suggested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332
---
 drivers/firmware/qcom_scm.c            | 20 ++++++++++++++++++++
 include/linux/firmware/qcom/qcom_scm.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 520de9b5633a..084e4782b88d 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -122,6 +122,7 @@ static const char * const qcom_scm_convention_names[] = {
 };
 
 static struct qcom_scm *__scm;
+static DEFINE_SPINLOCK(lock);
 
 static int qcom_scm_clk_enable(void)
 {
@@ -481,6 +482,25 @@ static int qcom_scm_disable_sdi(void)
 	return ret ? : res.result[0];
 }
 
+int qcom_scm_io_update_field(phys_addr_t addr, unsigned int mask, unsigned int val)
+{
+	unsigned int old, new;
+	int ret;
+
+	spin_lock(&lock);
+	ret = qcom_scm_io_readl(addr, &old);
+	if (ret)
+		goto unlock;
+
+	new = (old & ~mask) | (val & mask);
+
+	ret = qcom_scm_io_writel(addr, new);
+unlock:
+	spin_unlock(&lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_io_update_field);
+
 static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 {
 	struct qcom_scm_desc desc = {
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index ccaf28846054..5497eaf9c7b5 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -82,6 +82,8 @@ bool qcom_scm_pas_supported(u32 peripheral);
 
 int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
 int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
+int qcom_scm_io_update_field(phys_addr_t addr, unsigned int mask,
+			     unsigned int val);
 
 bool qcom_scm_restore_sec_cfg_available(void);
 int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
-- 
2.7.4

[PATCH v7 2/3] pinctrl: qcom: Use qcom_scm_io_update_field()

[Mukesh Ojha]

Use qcom_scm_io_update_field() exported function in
pinctrl-msm driver.

Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 115b83e2d8e6..2b9182375702 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1078,22 +1078,20 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	if (g->intr_target_width)
 		intr_target_mask = GENMASK(g->intr_target_width - 1, 0);
 
+	intr_target_mask <<= g->intr_target_bit;
 	if (pctrl->intr_target_use_scm) {
 		u32 addr = pctrl->phys_base[0] + g->intr_target_reg;
 		int ret;
 
-		qcom_scm_io_readl(addr, &val);
-		val &= ~(intr_target_mask << g->intr_target_bit);
-		val |= g->intr_target_kpss_val << g->intr_target_bit;
-
-		ret = qcom_scm_io_writel(addr, val);
+		val = g->intr_target_kpss_val << g->intr_target_bit;
+		ret = qcom_scm_io_update_field(addr, intr_target_mask, val);
 		if (ret)
 			dev_err(pctrl->dev,
 				"Failed routing %lu interrupt to Apps proc",
 				d->hwirq);
 	} else {
 		val = msm_readl_intr_target(pctrl, g);
-		val &= ~(intr_target_mask << g->intr_target_bit);
+		val &= ~intr_target_mask;
 		val |= g->intr_target_kpss_val << g->intr_target_bit;
 		msm_writel_intr_target(val, pctrl, g);
 	}
-- 
2.7.4

[PATCH v7 3/3] firmware: scm: Modify only the download bits in TCSR register

[Mukesh Ojha]

Crashdump collection is based on the DLOAD bit of TCSR register.
To retain other bits, we read the register and modify only the
DLOAD bit as the other bits have their own significance.

Co-developed-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332
---
 drivers/firmware/qcom_scm.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 084e4782b88d..da3d028f6451 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -4,6 +4,8 @@
  */
 
 #include <linux/arm-smccc.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/cpumask.h>
@@ -27,6 +29,10 @@
 static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
 module_param(download_mode, bool, 0);
 
+#define QCOM_DLOAD_MASK		GENMASK(5, 4)
+#define QCOM_DLOAD_FULLDUMP	0x1
+#define QCOM_DLOAD_NODUMP	0x0
+
 struct qcom_scm {
 	struct device *dev;
 	struct clk *core_clk;
@@ -518,6 +524,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 static void qcom_scm_set_download_mode(bool enable)
 {
+	u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
 	bool avail;
 	int ret = 0;
 
@@ -527,8 +534,9 @@ static void qcom_scm_set_download_mode(bool enable)
 	if (avail) {
 		ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
 	} else if (__scm->dload_mode_addr) {
-		ret = qcom_scm_io_writel(__scm->dload_mode_addr,
-				enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+		ret = qcom_scm_io_update_field(__scm->dload_mode_addr,
+					       QCOM_DLOAD_MASK,
+					       FIELD_PREP(QCOM_DLOAD_MASK, val));
 	} else {
 		dev_err(__scm->dev,
 			"No available mechanism for setting download mode\n");
-- 
2.7.4

Re: [PATCH 0/3 v7] Misc SCM changes

[Kathiravan Thirumoorthy]

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

[Dmitry Baryshkov]

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

Re: [PATCH v7 3/3] firmware: scm: Modify only the download bits in TCSR register

[Dmitry Baryshkov]

On Wed, 4 Oct 2023 at 20:28, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> Crashdump collection is based on the DLOAD bit of TCSR register.
> To retain other bits, we read the register and modify only the
> DLOAD bit as the other bits have their own significance.

Nit: please take a look at
`Documentation/process/submitting-patches.rst`: `Describe your changes
in imperative mood'.

We do not read registers. Driver does. Nevertheless, this is a minor
issue, which shouldn't prevent this patch from being applied.

>
> Co-developed-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/firmware/qcom_scm.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 084e4782b88d..da3d028f6451 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -4,6 +4,8 @@
>   */
>
>  #include <linux/arm-smccc.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/cpumask.h>
> @@ -27,6 +29,10 @@
>  static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
>  module_param(download_mode, bool, 0);
>
> +#define QCOM_DLOAD_MASK                GENMASK(5, 4)
> +#define QCOM_DLOAD_FULLDUMP    0x1
> +#define QCOM_DLOAD_NODUMP      0x0

Nit: it might be better to move these defines after all struct definitions.

> +
>  struct qcom_scm {
>         struct device *dev;
>         struct clk *core_clk;
> @@ -518,6 +524,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
>
>  static void qcom_scm_set_download_mode(bool enable)
>  {
> +       u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
>         bool avail;
>         int ret = 0;
>
> @@ -527,8 +534,9 @@ static void qcom_scm_set_download_mode(bool enable)
>         if (avail) {
>                 ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
>         } else if (__scm->dload_mode_addr) {
> -               ret = qcom_scm_io_writel(__scm->dload_mode_addr,
> -                               enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
> +               ret = qcom_scm_io_update_field(__scm->dload_mode_addr,
> +                                              QCOM_DLOAD_MASK,
> +                                              FIELD_PREP(QCOM_DLOAD_MASK, val));
>         } else {
>                 dev_err(__scm->dev,
>                         "No available mechanism for setting download mode\n");
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v7 1/3] firmware: qcom_scm: provide a read-modify-write function

[Dmitry Baryshkov]

On Wed, 4 Oct 2023 at 20:26, Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> It was realized by Srinivas K. that there is a need of
> read-modify-write scm exported function so that it can
> be used by multiple clients.
>
> Let's introduce qcom_scm_io_update_field() which masks
> out the bits and write the passed value to that bit-offset.
>
> Suggested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332
> ---
>  drivers/firmware/qcom_scm.c            | 20 ++++++++++++++++++++
>  include/linux/firmware/qcom/qcom_scm.h |  2 ++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 520de9b5633a..084e4782b88d 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -122,6 +122,7 @@ static const char * const qcom_scm_convention_names[] = {
>  };
>
>  static struct qcom_scm *__scm;
> +static DEFINE_SPINLOCK(lock);

Please move the spinlock inside __scm.

>
>  static int qcom_scm_clk_enable(void)
>  {
> @@ -481,6 +482,25 @@ static int qcom_scm_disable_sdi(void)
>         return ret ? : res.result[0];
>  }
>
> +int qcom_scm_io_update_field(phys_addr_t addr, unsigned int mask, unsigned int val)

qcom_scm_io_rmw ? qcom_scm_io_update_bits? These might be better names
and they follow the existing function names.

> +{
> +       unsigned int old, new;
> +       int ret;
> +
> +       spin_lock(&lock);
> +       ret = qcom_scm_io_readl(addr, &old);
> +       if (ret)
> +               goto unlock;
> +
> +       new = (old & ~mask) | (val & mask);
> +
> +       ret = qcom_scm_io_writel(addr, new);
> +unlock:
> +       spin_unlock(&lock);
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_io_update_field);
> +
>  static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
>  {
>         struct qcom_scm_desc desc = {
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index ccaf28846054..5497eaf9c7b5 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -82,6 +82,8 @@ bool qcom_scm_pas_supported(u32 peripheral);
>
>  int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
>  int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
> +int qcom_scm_io_update_field(phys_addr_t addr, unsigned int mask,
> +                            unsigned int val);
>
>  bool qcom_scm_restore_sec_cfg_available(void);
>  int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH 0/3 v7] Misc SCM changes

[Dmitry Baryshkov]

On Tue, 24 Oct 2023 at 19:00, Kathiravan Thirumoorthy
<quic_kathirav@quicinc.com> wrote:
>
>
> On 10/24/2023 8:38 PM, Dmitry Baryshkov wrote:
>
> 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?
>
>
> Dmitry,

Could you please turn off HTML message composition. For example your
message completely messed up the quoting in the text above.

> These patches are not the *fixes* for the existing crashes, these are required to *enable* the crash dump / ram dump collection by boot loader when qcom_scm.download_mode is set to 1 on IPQ9574 and IPQ5332 SoCs.

Please excuse me, I misread your message, mea culpa. Indeed, they are
not a fix for the existing error...

>
> Reason why I *requested* to pick it up for 6.7 if possible is, initial version is submitted in Jan 2023 by Poovendhan[1] and then later Mukesh integrated the initial series into his minidump series. Then I separated out these patches[2] from mindump series since there is no dependency for these patches to be part of minidump series but unfortunately again integrated back into the minidump series. Finally Mukesh again separated out these patches now.
>
> Since there are no active comments to be addressed, I was hoping this series to be picked for 6.7. As long as these patches doesn't go out of the radar, I'm fine :)
>


--
With best wishes
Dmitry

Re: [PATCH 0/3 v7] Misc SCM changes

[Dmitry Baryshkov]

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

[Kathiravan Thirumoorthy]

On 10/25/2023 1:34 AM, Dmitry Baryshkov wrote:
> On Tue, 24 Oct 2023 at 19:00, Kathiravan Thirumoorthy
> <quic_kathirav@quicinc.com> wrote:
>>
>>
>> On 10/24/2023 8:38 PM, Dmitry Baryshkov wrote:
>>
>> 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?
>>
>>
>> Dmitry,
> 
> Could you please turn off HTML message composition. For example your
> message completely messed up the quoting in the text above.


My bad. After the mail client update, HTML message composition is enabled.

> 
>> These patches are not the *fixes* for the existing crashes, these are required to *enable* the crash dump / ram dump collection by boot loader when qcom_scm.download_mode is set to 1 on IPQ9574 and IPQ5332 SoCs.
> 
> Please excuse me, I misread your message, mea culpa. Indeed, they are
> not a fix for the existing error...


Yeah, no problem at all.


> 
>>
>> Reason why I *requested* to pick it up for 6.7 if possible is, initial version is submitted in Jan 2023 by Poovendhan[1] and then later Mukesh integrated the initial series into his minidump series. Then I separated out these patches[2] from mindump series since there is no dependency for these patches to be part of minidump series but unfortunately again integrated back into the minidump series. Finally Mukesh again separated out these patches now.
>>
>> Since there are no active comments to be addressed, I was hoping this series to be picked for 6.7. As long as these patches doesn't go out of the radar, I'm fine :)
>>
> 
> 
> --
> With best wishes
> Dmitry

Re: [PATCH 0/3 v7] Misc SCM changes

[Mukesh Ojha]

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



>