[PATCH 0/2] soc: qcom: smp2p: Add support for remoteproc early attach

[PATCH 0/2] soc: qcom: smp2p: Add support for remoteproc early attach

[Jingyi Wang]

Some remoteproc will boot during earlier boot stages, add callback
.irq_get_irqchip_state for remoteproc to check the states in smp2p
and mark the state "attached", also add smp2p v2 support.

Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
---
Chris Lew (2):
      soc: qcom: smp2p: Add irqchip state support
      soc: qcom: smp2p: Add support for smp2p v2

 drivers/soc/qcom/smp2p.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 3 deletions(-)
---
base-commit: ae2d20002576d2893ecaff25db3d7ef9190ac0b6
change-id: 20250923-smp2p-1591de8af164

Best regards,
-- 
Jingyi Wang <jingyi.wang@oss.qualcomm.com>

[PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Jingyi Wang]

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

A remoteproc booted during earlier boot stages such as UEFI or the
bootloader, may need to be attached to without restarting the remoteproc
hardware. To do this the remoteproc will need to check the ready and
handover states in smp2p without an interrupt notification.

Add support for the .irq_get_irqchip_state callback so remoteproc can
read the current state of the fatal, ready and handover bits.

Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
---
 drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
index cb515c2340c1..e2cfd9ec8875 100644
--- a/drivers/soc/qcom/smp2p.c
+++ b/drivers/soc/qcom/smp2p.c
@@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
 	}
 }
 
+static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
+{
+	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
+	unsigned int pid = smp2p->remote_pid;
+	char buf[SMP2P_MAX_ENTRY_NAME];
+	struct smp2p_smem_item *in;
+	struct smp2p_entry *entry;
+	size_t size;
+	int i;
+
+	in = qcom_smem_get(pid, smem_id, &size);
+	if (IS_ERR(in))
+		return;
+
+	smp2p->in = in;
+
+	/* Check if version is initialized and set to v2 */
+	if (in->version == 0)
+		return;
+
+	for (i = smp2p->valid_entries; i < in->valid_entries; i++) {
+		list_for_each_entry(entry, &smp2p->inbound, node) {
+			memcpy(buf, in->entries[i].name, sizeof(buf));
+			if (!strcmp(buf, entry->name)) {
+				entry->value = &in->entries[i].value;
+				entry->last_value = readl(entry->value);
+				break;
+			}
+		}
+	}
+	smp2p->valid_entries = i;
+}
+
 static void qcom_smp2p_notify_in(struct qcom_smp2p *smp2p)
 {
 	struct smp2p_smem_item *in;
@@ -368,12 +401,31 @@ static void smp2p_irq_print_chip(struct irq_data *irqd, struct seq_file *p)
 	seq_printf(p, "%8s", dev_name(entry->smp2p->dev));
 }
 
+static int smp2p_irq_get_irqchip_state(struct irq_data *irqd, enum irqchip_irq_state which,
+				       bool *state)
+{
+	struct smp2p_entry *entry = irq_data_get_irq_chip_data(irqd);
+	u32 val;
+
+	if (which != IRQCHIP_STATE_LINE_LEVEL)
+		return -EINVAL;
+
+	if (!entry->value)
+		return -ENODEV;
+
+	val = readl(entry->value);
+	*state = !!(val & BIT(irqd_to_hwirq(irqd)));
+
+	return 0;
+}
+
 static struct irq_chip smp2p_irq_chip = {
 	.name           = "smp2p",
 	.irq_mask       = smp2p_mask_irq,
 	.irq_unmask     = smp2p_unmask_irq,
 	.irq_set_type	= smp2p_set_irq_type,
 	.irq_print_chip = smp2p_irq_print_chip,
+	.irq_get_irqchip_state = smp2p_irq_get_irqchip_state,
 };
 
 static int smp2p_irq_map(struct irq_domain *d,
@@ -618,6 +670,9 @@ static int qcom_smp2p_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Check inbound entries in the case of early boot processor */
+	qcom_smp2p_start_in(smp2p);
+
 	/* Kick the outgoing edge after allocating entries */
 	qcom_smp2p_kick(smp2p);
 

-- 
2.25.1

[PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Jingyi Wang]

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>
Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
---
 drivers/soc/qcom/smp2p.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
index e2cfd9ec8875..5ea64a83c678 100644
--- a/drivers/soc/qcom/smp2p.c
+++ b/drivers/soc/qcom/smp2p.c
@@ -48,10 +48,13 @@
 #define SMP2P_MAGIC 0x504d5324
 #define SMP2P_ALL_FEATURES	SMP2P_FEATURE_SSR_ACK
 
+#define SMP2P_VERSION_1 1
+#define SMP2P_VERSION_2 2
+
 /**
  * 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
@@ -180,14 +183,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 > SMP2P_VERSION_1) {
+		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)
@@ -222,6 +234,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];
@@ -516,6 +542,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));
@@ -537,12 +564,14 @@ 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);
+
 	/*
 	 * 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.25.1

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Konrad Dybcio]

On 9/24/25 6:18 AM, Jingyi Wang wrote:
> From: Chris Lew <chris.lew@oss.qualcomm.com>
> 
> A remoteproc booted during earlier boot stages such as UEFI or the
> bootloader, may need to be attached to without restarting the remoteproc
> hardware. To do this the remoteproc will need to check the ready and
> handover states in smp2p without an interrupt notification.
> 
> Add support for the .irq_get_irqchip_state callback so remoteproc can
> read the current state of the fatal, ready and handover bits.
> 
> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> index cb515c2340c1..e2cfd9ec8875 100644
> --- a/drivers/soc/qcom/smp2p.c
> +++ b/drivers/soc/qcom/smp2p.c
> @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
>  	}
>  }
>  
> +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
> +{
> +	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> +	unsigned int pid = smp2p->remote_pid;
> +	char buf[SMP2P_MAX_ENTRY_NAME];
> +	struct smp2p_smem_item *in;
> +	struct smp2p_entry *entry;
> +	size_t size;
> +	int i;
> +
> +	in = qcom_smem_get(pid, smem_id, &size);
> +	if (IS_ERR(in))
> +		return;
> +
> +	smp2p->in = in;
> +
> +	/* Check if version is initialized and set to v2 */
> +	if (in->version == 0)
> +		return;

This doesn't seem to be fully in line with the comment

Konrad

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Konrad Dybcio]

On 9/24/25 6:18 AM, Jingyi Wang 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. 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>
> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> ---

[...]

> +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;

are in and out versions supposed to be out of sync in case of
error?

> +}
> +
>  static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>  {
>  	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> @@ -516,6 +542,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));
> @@ -537,12 +564,14 @@ 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);
> +
>  	/*
>  	 * 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;

= in_version ?: 1

Konrad

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Deepak Kumar Singh]

On 9/24/2025 8:20 PM, Konrad Dybcio wrote:
> On 9/24/25 6:18 AM, Jingyi Wang wrote:
>> From: Chris Lew <chris.lew@oss.qualcomm.com>
>>
>> A remoteproc booted during earlier boot stages such as UEFI or the
>> bootloader, may need to be attached to without restarting the remoteproc
>> hardware. To do this the remoteproc will need to check the ready and
>> handover states in smp2p without an interrupt notification.
>>
>> Add support for the .irq_get_irqchip_state callback so remoteproc can
>> read the current state of the fatal, ready and handover bits.
>>
>> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>> ---
>>   drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
>> index cb515c2340c1..e2cfd9ec8875 100644
>> --- a/drivers/soc/qcom/smp2p.c
>> +++ b/drivers/soc/qcom/smp2p.c
>> @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
>>   	}
>>   }
>>   
>> +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>> +{
>> +	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
>> +	unsigned int pid = smp2p->remote_pid;
>> +	char buf[SMP2P_MAX_ENTRY_NAME];
>> +	struct smp2p_smem_item *in;
>> +	struct smp2p_entry *entry;
>> +	size_t size;
>> +	int i;
>> +
>> +	in = qcom_smem_get(pid, smem_id, &size);
>> +	if (IS_ERR(in))
>> +		return;
>> +
>> +	smp2p->in = in;
>> +
>> +	/* Check if version is initialized and set to v2 */
>> +	if (in->version == 0)
>> +		return;
> 
> This doesn't seem to be fully in line with the comment
> 
> Konrad
> 
Hi Konard,

Can you please elaborate more on this?
in->version == 0 means remote has not initialized the version yet, so no 
need of enumerating entries. For other case i.e in->version == 1 or 2, 
in entries added by early booted remote has to be enumerated.

Thanks,
Deepak

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Deepak Kumar Singh]

On 9/24/2025 8:27 PM, Konrad Dybcio wrote:
> On 9/24/25 6:18 AM, Jingyi Wang 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. 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>
>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +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;
> 
> are in and out versions supposed to be out of sync in case of
> error?
> 
I think that should be ok. If we return error smp2p connection will be 
completely broken. With default version 1 partial features can be 
supported even if remote is using version 2. Some features like smp2p 
connection after subsystem restart may be affected by this.>> +}
>> +
>>   static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>>   {
>>   	unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
>> @@ -516,6 +542,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));
>> @@ -537,12 +564,14 @@ 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);
>> +
>>   	/*
>>   	 * 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;
> 
> = in_version ?: 1
> 
> Konrad
> 
We want to assign in_version when value is 1/2 and 1 if value is 0 i.e. 
error case.

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Konrad Dybcio]

On 10/21/25 10:12 AM, Deepak Kumar Singh wrote:
> 
> 
> On 9/24/2025 8:20 PM, Konrad Dybcio wrote:
>> On 9/24/25 6:18 AM, Jingyi Wang wrote:
>>> From: Chris Lew <chris.lew@oss.qualcomm.com>
>>>
>>> A remoteproc booted during earlier boot stages such as UEFI or the
>>> bootloader, may need to be attached to without restarting the remoteproc
>>> hardware. To do this the remoteproc will need to check the ready and
>>> handover states in smp2p without an interrupt notification.
>>>
>>> Add support for the .irq_get_irqchip_state callback so remoteproc can
>>> read the current state of the fatal, ready and handover bits.
>>>
>>> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
>>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>> ---
>>>   drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
>>> index cb515c2340c1..e2cfd9ec8875 100644
>>> --- a/drivers/soc/qcom/smp2p.c
>>> +++ b/drivers/soc/qcom/smp2p.c
>>> @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
>>>       }
>>>   }
>>>   +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>>> +{
>>> +    unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
>>> +    unsigned int pid = smp2p->remote_pid;
>>> +    char buf[SMP2P_MAX_ENTRY_NAME];
>>> +    struct smp2p_smem_item *in;
>>> +    struct smp2p_entry *entry;
>>> +    size_t size;
>>> +    int i;
>>> +
>>> +    in = qcom_smem_get(pid, smem_id, &size);
>>> +    if (IS_ERR(in))
>>> +        return;
>>> +
>>> +    smp2p->in = in;
>>> +
>>> +    /* Check if version is initialized and set to v2 */
>>> +    if (in->version == 0)
>>> +        return;
>>
>> This doesn't seem to be fully in line with the comment
>>
>> Konrad
>>
> Hi Konard,
> 
> Can you please elaborate more on this?
> in->version == 0 means remote has not initialized the version yet, so no need of enumerating entries. For other case i.e in->version == 1 or 2, in entries added by early booted remote has to be enumerated.

It's not at all obvious that 0 is supposed to mean "uninitialized"

Please #define it

Konrad

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Konrad Dybcio]

On 10/21/25 10:23 AM, Deepak Kumar Singh wrote:
> 
> 
> On 9/24/2025 8:27 PM, Konrad Dybcio wrote:
>> On 9/24/25 6:18 AM, Jingyi Wang 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. 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>
>>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>> +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;
>>
>> are in and out versions supposed to be out of sync in case of
>> error?
>>
> I think that should be ok. If we return error smp2p connection will be completely broken. With default version 1 partial features can be supported even if remote is using version 2. Some features like smp2p connection after subsystem restart may be affected by this.>> +}

Perhaps a different question is in order.. do we ever expect smem_get to
fail under normal conditions?

[...]

>>>       /*
>>>        * 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;
>>
>> = in_version ?: 1
>>
>> Konrad
>>
> We want to assign in_version when value is 1/2 and 1 if value is 0 i.e. error case.

That's what this syntax does, with less characters

Konrad

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Deepak Kumar Singh]

On 10/21/2025 3:09 PM, Konrad Dybcio wrote:
> On 10/21/25 10:23 AM, Deepak Kumar Singh wrote:
>>
>>
>> On 9/24/2025 8:27 PM, Konrad Dybcio wrote:
>>> On 9/24/25 6:18 AM, Jingyi Wang 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. 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>
>>>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>>> ---
>>>
>>> [...]
>>>
>>>> +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;
>>>
>>> are in and out versions supposed to be out of sync in case of
>>> error?
>>>
>> I think that should be ok. If we return error smp2p connection will be completely broken. With default version 1 partial features can be supported even if remote is using version 2. Some features like smp2p connection after subsystem restart may be affected by this.>> +}
> 
> Perhaps a different question is in order.. do we ever expect smem_get to
> fail under normal conditions?
> 
> [...]
> 
Good point, i think that should never happen for early boot processor 
which will use version 2. That can possibly happen for processor that is 
coming late than local host(version 1). In that case anyway we are 
setting default version 1 and proceeding. >>>>        /*
>>>>         * 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;
>>>
>>> = in_version ?: 1
>>>
>>> Konrad
>>>
>> We want to assign in_version when value is 1/2 and 1 if value is 0 i.e. error case.
> 
> That's what this syntax does, with less characters
> 
> Konrad
Yes in_version ?:1 is short hand but i find (in_version) ? in_version : 
1; more readable and obvious.

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Deepak Kumar Singh]

On 10/21/2025 3:05 PM, Konrad Dybcio wrote:
> On 10/21/25 10:12 AM, Deepak Kumar Singh wrote:
>>
>>
>> On 9/24/2025 8:20 PM, Konrad Dybcio wrote:
>>> On 9/24/25 6:18 AM, Jingyi Wang wrote:
>>>> From: Chris Lew <chris.lew@oss.qualcomm.com>
>>>>
>>>> A remoteproc booted during earlier boot stages such as UEFI or the
>>>> bootloader, may need to be attached to without restarting the remoteproc
>>>> hardware. To do this the remoteproc will need to check the ready and
>>>> handover states in smp2p without an interrupt notification.
>>>>
>>>> Add support for the .irq_get_irqchip_state callback so remoteproc can
>>>> read the current state of the fatal, ready and handover bits.
>>>>
>>>> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
>>>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>>>> ---
>>>>    drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 55 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
>>>> index cb515c2340c1..e2cfd9ec8875 100644
>>>> --- a/drivers/soc/qcom/smp2p.c
>>>> +++ b/drivers/soc/qcom/smp2p.c
>>>> @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
>>>>        }
>>>>    }
>>>>    +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
>>>> +{
>>>> +    unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
>>>> +    unsigned int pid = smp2p->remote_pid;
>>>> +    char buf[SMP2P_MAX_ENTRY_NAME];
>>>> +    struct smp2p_smem_item *in;
>>>> +    struct smp2p_entry *entry;
>>>> +    size_t size;
>>>> +    int i;
>>>> +
>>>> +    in = qcom_smem_get(pid, smem_id, &size);
>>>> +    if (IS_ERR(in))
>>>> +        return;
>>>> +
>>>> +    smp2p->in = in;
>>>> +
>>>> +    /* Check if version is initialized and set to v2 */
>>>> +    if (in->version == 0)
>>>> +        return;
>>>
>>> This doesn't seem to be fully in line with the comment
>>>
>>> Konrad
>>>
>> Hi Konard,
>>
>> Can you please elaborate more on this?
>> in->version == 0 means remote has not initialized the version yet, so no need of enumerating entries. For other case i.e in->version == 1 or 2, in entries added by early booted remote has to be enumerated.
> 
> It's not at all obvious that 0 is supposed to mean "uninitialized"
> 
> Please #define it
> 
> Konrad
I think that can be added or instead we can replace (in->version == 0 
)with (in->version != SMP2P_VERSION_2).

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Bjorn Andersson]

On Wed, Oct 22, 2025 at 04:27:28PM +0530, Deepak Kumar Singh wrote:
> 
> 
> On 10/21/2025 3:05 PM, Konrad Dybcio wrote:
> > On 10/21/25 10:12 AM, Deepak Kumar Singh wrote:
> > > 
> > > 
> > > On 9/24/2025 8:20 PM, Konrad Dybcio wrote:
> > > > On 9/24/25 6:18 AM, Jingyi Wang wrote:
> > > > > From: Chris Lew <chris.lew@oss.qualcomm.com>
> > > > > 
> > > > > A remoteproc booted during earlier boot stages such as UEFI or the
> > > > > bootloader, may need to be attached to without restarting the remoteproc
> > > > > hardware. To do this the remoteproc will need to check the ready and
> > > > > handover states in smp2p without an interrupt notification.
> > > > > 
> > > > > Add support for the .irq_get_irqchip_state callback so remoteproc can
> > > > > read the current state of the fatal, ready and handover bits.
> > > > > 
> > > > > Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> > > > > Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > > > Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > > > ---
> > > > >    drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 55 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> > > > > index cb515c2340c1..e2cfd9ec8875 100644
> > > > > --- a/drivers/soc/qcom/smp2p.c
> > > > > +++ b/drivers/soc/qcom/smp2p.c
> > > > > @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
> > > > >        }
> > > > >    }
> > > > >    +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
> > > > > +{
> > > > > +    unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> > > > > +    unsigned int pid = smp2p->remote_pid;
> > > > > +    char buf[SMP2P_MAX_ENTRY_NAME];
> > > > > +    struct smp2p_smem_item *in;
> > > > > +    struct smp2p_entry *entry;
> > > > > +    size_t size;
> > > > > +    int i;
> > > > > +
> > > > > +    in = qcom_smem_get(pid, smem_id, &size);
> > > > > +    if (IS_ERR(in))
> > > > > +        return;
> > > > > +
> > > > > +    smp2p->in = in;
> > > > > +
> > > > > +    /* Check if version is initialized and set to v2 */
> > > > > +    if (in->version == 0)
> > > > > +        return;
> > > > 
> > > > This doesn't seem to be fully in line with the comment
> > > > 
> > > > Konrad
> > > > 
> > > Hi Konard,
> > > 
> > > Can you please elaborate more on this?
> > > in->version == 0 means remote has not initialized the version yet, so no need of enumerating entries. For other case i.e in->version == 1 or 2, in entries added by early booted remote has to be enumerated.
> > 
> > It's not at all obvious that 0 is supposed to mean "uninitialized"
> > 
> > Please #define it
> > 
> > Konrad
> I think that can be added or instead we can replace (in->version == 0 )with
> (in->version != SMP2P_VERSION_2).
> 

I agree with Konrad regarding the discrepancy between comment and code,
"Initialized and set to 2" means specifically version 2, while checking
against 0 means "any of the remaining 255 possible values.

I don't think we need a define for the version number 2.


But we most definitely need a comment about why the remainder shouldn't
be executed for all other (initialized) versions. Today, specifically,
why isn't this code valid for version 1?

Regards,
Bjorn

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Bjorn Andersson]

On Tue, Sep 23, 2025 at 09:18:43PM -0700, Jingyi Wang 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. 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>
> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>

Please confirm that you really co-developed (pair programming) this
patch with Chris.

Isn't this a patch from Chris, that you're "forwarding", i.e. both
Signed-off-by should be there, but the Co-developed-by shouldn't.

> ---
>  drivers/soc/qcom/smp2p.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> index e2cfd9ec8875..5ea64a83c678 100644
> --- a/drivers/soc/qcom/smp2p.c
> +++ b/drivers/soc/qcom/smp2p.c
> @@ -48,10 +48,13 @@
>  #define SMP2P_MAGIC 0x504d5324
>  #define SMP2P_ALL_FEATURES	SMP2P_FEATURE_SSR_ACK
>  
> +#define SMP2P_VERSION_1 1
> +#define SMP2P_VERSION_2 2

#define ONE 1
#define TWO 2

#define PLEASE_DONT true

> +
>  /**
>   * 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
> @@ -180,14 +183,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 > SMP2P_VERSION_1) {
> +		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)
> @@ -222,6 +234,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];
> @@ -516,6 +542,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));
> @@ -537,12 +564,14 @@ 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);
> +
>  	/*
>  	 * 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;

Doesn't this imply that if the remoteproc advertises support for version
3, then we suddenly also support version 3?


I don't remember if we've talked about how version handling should work
in this driver, but we should certainly define and document that in the
comment at the top of this driver.

Regards,
Bjorn

>  
>  	qcom_smp2p_kick(smp2p);
>  
> 
> -- 
> 2.25.1
> 

Re: [PATCH 1/2] soc: qcom: smp2p: Add irqchip state support

[Christopher Lew]

On Wed, Oct 22, 2025 at 3:10 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Wed, Oct 22, 2025 at 04:27:28PM +0530, Deepak Kumar Singh wrote:
> >
> >
> > On 10/21/2025 3:05 PM, Konrad Dybcio wrote:
> > > On 10/21/25 10:12 AM, Deepak Kumar Singh wrote:
> > > >
> > > >
> > > > On 9/24/2025 8:20 PM, Konrad Dybcio wrote:
> > > > > On 9/24/25 6:18 AM, Jingyi Wang wrote:
> > > > > > From: Chris Lew <chris.lew@oss.qualcomm.com>
> > > > > >
> > > > > > A remoteproc booted during earlier boot stages such as UEFI or the
> > > > > > bootloader, may need to be attached to without restarting the remoteproc
> > > > > > hardware. To do this the remoteproc will need to check the ready and
> > > > > > handover states in smp2p without an interrupt notification.
> > > > > >
> > > > > > Add support for the .irq_get_irqchip_state callback so remoteproc can
> > > > > > read the current state of the fatal, ready and handover bits.
> > > > > >
> > > > > > Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> > > > > > Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > > > > Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > > > > > ---
> > > > > >    drivers/soc/qcom/smp2p.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >    1 file changed, 55 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> > > > > > index cb515c2340c1..e2cfd9ec8875 100644
> > > > > > --- a/drivers/soc/qcom/smp2p.c
> > > > > > +++ b/drivers/soc/qcom/smp2p.c
> > > > > > @@ -222,6 +222,39 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
> > > > > >        }
> > > > > >    }
> > > > > >    +static void qcom_smp2p_start_in(struct qcom_smp2p *smp2p)
> > > > > > +{
> > > > > > +    unsigned int smem_id = smp2p->smem_items[SMP2P_INBOUND];
> > > > > > +    unsigned int pid = smp2p->remote_pid;
> > > > > > +    char buf[SMP2P_MAX_ENTRY_NAME];
> > > > > > +    struct smp2p_smem_item *in;
> > > > > > +    struct smp2p_entry *entry;
> > > > > > +    size_t size;
> > > > > > +    int i;
> > > > > > +
> > > > > > +    in = qcom_smem_get(pid, smem_id, &size);
> > > > > > +    if (IS_ERR(in))
> > > > > > +        return;
> > > > > > +
> > > > > > +    smp2p->in = in;
> > > > > > +
> > > > > > +    /* Check if version is initialized and set to v2 */
> > > > > > +    if (in->version == 0)
> > > > > > +        return;
> > > > >
> > > > > This doesn't seem to be fully in line with the comment
> > > > >
> > > > > Konrad
> > > > >
> > > > Hi Konard,
> > > >
> > > > Can you please elaborate more on this?
> > > > in->version == 0 means remote has not initialized the version yet, so no need of enumerating entries. For other case i.e in->version == 1 or 2, in entries added by early booted remote has to be enumerated.
> > >
> > > It's not at all obvious that 0 is supposed to mean "uninitialized"
> > >
> > > Please #define it
> > >
> > > Konrad
> > I think that can be added or instead we can replace (in->version == 0 )with
> > (in->version != SMP2P_VERSION_2).
> >
>
> I agree with Konrad regarding the discrepancy between comment and code,
> "Initialized and set to 2" means specifically version 2, while checking
> against 0 means "any of the remaining 255 possible values.
>
> I don't think we need a define for the version number 2.
>
>
> But we most definitely need a comment about why the remainder shouldn't
> be executed for all other (initialized) versions. Today, specifically,
> why isn't this code valid for version 1?
>

I think I had made an assumption that the processors still supporting
V1 were remoteproc managed processors, for those processors this check
would always return early because those remoteprocs boot after smp2p
probes.
If this code somehow executed on a v1 edge, then we would most likely
miss some notifications. That's abnormal behavior so let's change the
check to explicitly look for 2.

Thanks,
Chris

> Regards,
> Bjorn

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Jingyi Wang]

On 10/23/2025 6:19 AM, Bjorn Andersson wrote:
> On Tue, Sep 23, 2025 at 09:18:43PM -0700, Jingyi Wang 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. 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>
>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> 
> Please confirm that you really co-developed (pair programming) this
> patch with Chris.
> 
> Isn't this a patch from Chris, that you're "forwarding", i.e. both
> Signed-off-by should be there, but the Co-developed-by shouldn't.
> 

I remembered I did some minor updated, will delete that in next version.

Thanks,
Jingyi

>> ---
>>  drivers/soc/qcom/smp2p.c | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
>> index e2cfd9ec8875..5ea64a83c678 100644
>> --- a/drivers/soc/qcom/smp2p.c
>> +++ b/drivers/soc/qcom/smp2p.c
>> @@ -48,10 +48,13 @@
>>  #define SMP2P_MAGIC 0x504d5324
>>  #define SMP2P_ALL_FEATURES	SMP2P_FEATURE_SSR_ACK
>>  
>> +#define SMP2P_VERSION_1 1
>> +#define SMP2P_VERSION_2 2
> 
> #define ONE 1
> #define TWO 2
> 
> #define PLEASE_DONT true
> 
>> +
>>  /**
>>   * 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
>> @@ -180,14 +183,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 > SMP2P_VERSION_1) {
>> +		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)
>> @@ -222,6 +234,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];
>> @@ -516,6 +542,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));
>> @@ -537,12 +564,14 @@ 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);
>> +
>>  	/*
>>  	 * 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;
> 
> Doesn't this imply that if the remoteproc advertises support for version
> 3, then we suddenly also support version 3?
> 
> 
> I don't remember if we've talked about how version handling should work
> in this driver, but we should certainly define and document that in the
> comment at the top of this driver.
> 
> Regards,
> Bjorn
> 
>>  
>>  	qcom_smp2p_kick(smp2p);
>>  
>>
>> -- 
>> 2.25.1
>>

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Christopher Lew]

On Tue, Oct 21, 2025 at 2:39 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 10/21/25 10:23 AM, Deepak Kumar Singh wrote:
> >
> >
> > On 9/24/2025 8:27 PM, Konrad Dybcio wrote:
> >> On 9/24/25 6:18 AM, Jingyi Wang 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. 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>
> >>> Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> >>> Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> >>> ---
> >>
> >> [...]
> >>
> >>> +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;
> >>
> >> are in and out versions supposed to be out of sync in case of
> >> error?
> >>
> > I think that should be ok. If we return error smp2p connection will be completely broken. With default version 1 partial features can be supported even if remote is using version 2. Some features like smp2p connection after subsystem restart may be affected by this.>> +}
>
> Perhaps a different question is in order.. do we ever expect smem_get to
> fail under normal conditions?
>

We would, this new flow gets executed for all the smp2p device probes.
For remoteprocs booted by HLOS, I would expect this call to
qcom_smem_get() to fail during smp2p probe time, so I have a silent
error. The in item will be found when we get the first ipcc interrupt
from the remote. I would only expect qcom_smem_get() to succeed here
on smp2p devices for early boot processors.

> [...]
>
> >>>       /*
> >>>        * 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;
> >>
> >> = in_version ?: 1
> >>
> >> Konrad
> >>
> > We want to assign in_version when value is 1/2 and 1 if value is 0 i.e. error case.
>
> That's what this syntax does, with less characters
>
> Konrad

Re: [PATCH 2/2] soc: qcom: smp2p: Add support for smp2p v2

[Christopher Lew]

On Wed, Oct 22, 2025 at 3:16 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Tue, Sep 23, 2025 at 09:18:43PM -0700, Jingyi Wang 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. 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>
> > Co-developed-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
> > Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
>
> Please confirm that you really co-developed (pair programming) this
> patch with Chris.
>
> Isn't this a patch from Chris, that you're "forwarding", i.e. both
> Signed-off-by should be there, but the Co-developed-by shouldn't.
>
> > ---
> >  drivers/soc/qcom/smp2p.c | 35 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> > index e2cfd9ec8875..5ea64a83c678 100644
> > --- a/drivers/soc/qcom/smp2p.c
> > +++ b/drivers/soc/qcom/smp2p.c
> > @@ -48,10 +48,13 @@
> >  #define SMP2P_MAGIC 0x504d5324
> >  #define SMP2P_ALL_FEATURES   SMP2P_FEATURE_SSR_ACK
> >
> > +#define SMP2P_VERSION_1 1
> > +#define SMP2P_VERSION_2 2
>
> #define ONE 1
> #define TWO 2
>
> #define PLEASE_DONT true
>
> > +
> >  /**
> >   * 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
> > @@ -180,14 +183,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 > SMP2P_VERSION_1) {
> > +             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)
> > @@ -222,6 +234,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];
> > @@ -516,6 +542,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));
> > @@ -537,12 +564,14 @@ 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);
> > +
> >       /*
> >        * 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;
>
> Doesn't this imply that if the remoteproc advertises support for version
> 3, then we suddenly also support version 3?
>

Yea I think this is a result of a quick fix and certainty that no
firmware was running around with version 3. We can clean this up.

>
> I don't remember if we've talked about how version handling should work
> in this driver, but we should certainly define and document that in the
> comment at the top of this driver.
>

We did, there's provisions for the remote to version down or place a
0xFF value if it isn't capable of versioning down. Unfortunately I
think this versioning down behavior comes as part of V2.
Because we have to support older firmware, it was better to mirror V1
for older remotes in case they couldn't understand Linux doing the
version down sequence.

We don't have any remote with V3 so I didnt implement that part of the
V2 smp2p spec.

> Regards,
> Bjorn
>
> >
> >       qcom_smp2p_kick(smp2p);
> >
> >
> > --
> > 2.25.1
> >