[v1] soc: qcom: smp2p: Add support for remoteproc early attach

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

Posted by Jingyi Wang 4 months, 2 weeks ago

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

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

Posted by Konrad Dybcio 4 months, 2 weeks ago

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 1/2] soc: qcom: smp2p: Add irqchip state support

Posted by Deepak Kumar Singh 3 months, 2 weeks ago


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 1/2] soc: qcom: smp2p: Add irqchip state support

Posted by Konrad Dybcio 3 months, 2 weeks ago

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 1/2] soc: qcom: smp2p: Add irqchip state support

Posted by Deepak Kumar Singh 3 months, 2 weeks ago


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

Posted by Bjorn Andersson 3 months, 2 weeks ago

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 1/2] soc: qcom: smp2p: Add irqchip state support

Posted by Christopher Lew 3 months, 2 weeks ago

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