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

[Jingyi Wang <jingyi.wang@oss.qualcomm.com>]

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