From: Chris Lew <chris.lew@oss.qualcomm.com>

Some remoteproc need smp2p v2 support, mirror the version written by the
remote if the remote supports v2. This is needed if the remote does not
have backwards compatibility with v1. And reset entry last value on SSR for
smp2p v2.

Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
---
 drivers/soc/qcom/smp2p.c | 41 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
index 39628df36745..c35ca7535c14 100644
--- a/drivers/soc/qcom/smp2p.c
+++ b/drivers/soc/qcom/smp2p.c
@@ -36,6 +36,10 @@
  * The driver uses the Linux GPIO and interrupt framework to expose a virtual
  * GPIO for each outbound entry and a virtual interrupt controller for each
  * inbound entry.
+ *
+ * Driver supports two versions:
+ * V1 - For processor that start after local host
+ * V2 - For processor that start in early boot sequence
  */
 
 #define SMP2P_MAX_ENTRY 16
@@ -50,11 +54,12 @@
 
 #define ONE 1
 #define TWO 2
+#define MAX_VERSION TWO
 
 /**
  * struct smp2p_smem_item - in memory communication structure
  * @magic:		magic number
- * @version:		version - must be 1
+ * @version:		version
  * @features:		features flag - currently unused
  * @local_pid:		processor id of sending end
  * @remote_pid:		processor id of receiving end
@@ -183,14 +188,23 @@ static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
 static bool qcom_smp2p_check_ssr(struct qcom_smp2p *smp2p)
 {
 	struct smp2p_smem_item *in = smp2p->in;
+	struct smp2p_entry *entry;
 	bool restart;
 
 	if (!smp2p->ssr_ack_enabled)
 		return false;
 
 	restart = in->flags & BIT(SMP2P_FLAGS_RESTART_DONE_BIT);
+	restart = restart != smp2p->ssr_ack;
+	if (restart && in->version > ONE) {
+		list_for_each_entry(entry, &smp2p->inbound, node) {
+			if (!entry->value)
+				continue;
+			entry->last_value = 0;
+		}
+	}
 
-	return restart != smp2p->ssr_ack;
+	return restart;
 }
 
 static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
@@ -225,6 +239,20 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
 	}
 }
 
+static int qcom_smp2p_in_version(struct qcom_smp2p *smp2p)
+{
+	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
+	unsigned int pid = smp2p->remote_pid;
+	struct smp2p_smem_item *in;
+	size_t size;
+
+	in = qcom_smem_get(pid, smem_id, &size);
+	if (IS_ERR(in))
+		return 0;
+
+	return in->version;
+}
+
 static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
 {
 	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
@@ -522,6 +550,7 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
 	struct smp2p_smem_item *out;
 	unsigned smem_id = smp2p->smem_items[SMP2P_OUTBOUND];
 	unsigned pid = smp2p->remote_pid;
+	u8 in_version;
 	int ret;
 
 	ret = qcom_smem_alloc(pid, smem_id, sizeof(*out));
@@ -543,12 +572,18 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
 	out->valid_entries = 0;
 	out->features = SMP2P_ALL_FEATURES;
 
+	in_version = qcom_smp2p_in_version(smp2p);
+	if (in_version > MAX_VERSION) {
+		dev_err(smp2p->dev, "Unsupported smp2p version\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * Make sure the rest of the header is written before we validate the
 	 * item by writing a valid version number.
 	 */
 	wmb();
-	out->version = 1;
+	out->version = (in_version) ? in_version : 1;
 
 	qcom_smp2p_kick(smp2p);
 
-- 
2.34.1

On Mon, Nov 03, 2025 at 08:59:29PM +0530, Deepak Kumar Singh wrote:
> From: Chris Lew <chris.lew@oss.qualcomm.com>
> 
> Some remoteproc need smp2p v2 support, mirror the version written by the
> remote if the remote supports v2.

I don't think they _need_ v2 support. The subsystem might implement v2
and only support v2...

> This is needed if the remote does not have backwards compatibility
> with v1.

I guess this retroactively amends the previous sentence to make it
valid? Please rewrite these two sentences.

> And reset entry last value on SSR for smp2p v2.

The first two sentences described a problem and a "solution" to that
problem, here you're just throwing in a fact.

Please document what version 2 actually is and make it clear why
resetting "entry last value on SSR".

> 
> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/smp2p.c | 41 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> index 39628df36745..c35ca7535c14 100644
> --- a/drivers/soc/qcom/smp2p.c
> +++ b/drivers/soc/qcom/smp2p.c
> @@ -36,6 +36,10 @@
>   * The driver uses the Linux GPIO and interrupt framework to expose a virtual
>   * GPIO for each outbound entry and a virtual interrupt controller for each
>   * inbound entry.
> + *
> + * Driver supports two versions:
> + * V1 - For processor that start after local host
> + * V2 - For processor that start in early boot sequence
>   */
>  
>  #define SMP2P_MAX_ENTRY 16
> @@ -50,11 +54,12 @@
>  
>  #define ONE 1
>  #define TWO 2
> +#define MAX_VERSION TWO
>  
>  /**
>   * struct smp2p_smem_item - in memory communication structure
>   * @magic:		magic number
> - * @version:		version - must be 1
> + * @version:		version
>   * @features:		features flag - currently unused
>   * @local_pid:		processor id of sending end
>   * @remote_pid:		processor id of receiving end
> @@ -183,14 +188,23 @@ static void qcom_smp2p_kick(struct qcom_smp2p *smp2p)
>  static bool qcom_smp2p_check_ssr(struct qcom_smp2p *smp2p)
>  {
>  	struct smp2p_smem_item *in = smp2p->in;
> +	struct smp2p_entry *entry;
>  	bool restart;
>  
>  	if (!smp2p->ssr_ack_enabled)
>  		return false;
>  
>  	restart = in->flags & BIT(SMP2P_FLAGS_RESTART_DONE_BIT);
> +	restart = restart != smp2p->ssr_ack;

This is hard to read, please try to avoid the immediate reassignment.

> +	if (restart && in->version > ONE) {
> +		list_for_each_entry(entry, &smp2p->inbound, node) {
> +			if (!entry->value)
> +				continue;
> +			entry->last_value = 0;

Why do we only do this for version 2+?

> +		}
> +	}
>  
> -	return restart != smp2p->ssr_ack;
> +	return restart;
>  }
>  
>  static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
> @@ -225,6 +239,20 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
>  	}
>  }
>  
> +static int qcom_smp2p_in_version(struct qcom_smp2p *smp2p)
> +{
> +	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> +	unsigned int pid = smp2p->remote_pid;
> +	struct smp2p_smem_item *in;
> +	size_t size;
> +
> +	in = qcom_smem_get(pid, smem_id, &size);
> +	if (IS_ERR(in))
> +		return 0;
> +
> +	return in->version;
> +}
> +
>  static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>  {
>  	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> @@ -522,6 +550,7 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
>  	struct smp2p_smem_item *out;
>  	unsigned smem_id = smp2p->smem_items[SMP2P_OUTBOUND];
>  	unsigned pid = smp2p->remote_pid;
> +	u8 in_version;
>  	int ret;
>  
>  	ret = qcom_smem_alloc(pid, smem_id, sizeof(*out));
> @@ -543,12 +572,18 @@ static int qcom_smp2p_alloc_outbound_item(struct qcom_smp2p *smp2p)
>  	out->valid_entries = 0;
>  	out->features = SMP2P_ALL_FEATURES;
>  
> +	in_version = qcom_smp2p_in_version(smp2p);

This is a bit obfuscated in my view, and doesn't seem complete.

We're calling qcom_smp2p_alloc_outbound_item() during probe(), at which
time any non-early booted subsystem will yet to have been launched and
hence they haven't allocated their SMP2P SMEM item.

So in_version will be 0, which is less than 2, so therefor we're running
version 1.

If the subsystem is then brought out of reset and it implements version
2 (we intended it to be launched by bootloader, but it wasn't - this
shouldn't be a problem), we will have a version "mismatch".

Upon first interrupt from the remote we will determine in
qcom_smp2p_negotiate() that we're version 1 and they are version 2, so
we will not complete the negotiation - and thereby not deliver
interrupts.

> +	if (in_version > MAX_VERSION) {
> +		dev_err(smp2p->dev, "Unsupported smp2p version\n");

I think we can afford ourself to add "...: %d\n", in_version); here. It
would make the error print directly actionable the day we hit it (or
make it obvious if we hit this error due to a bogus in_version).

Regards,
Bjorn

> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * Make sure the rest of the header is written before we validate the
>  	 * item by writing a valid version number.
>  	 */
>  	wmb();
> -	out->version = 1;
> +	out->version = (in_version) ? in_version : 1;
>  
>  	qcom_smp2p_kick(smp2p);
>  
> -- 
> 2.34.1
>