Re: [PATCH] remoteproc: xlnx: check remote node state

["Shah, Tanmay" <tanmays@amd.com>]

On 4/27/2026 12:52 PM, Mathieu Poirier wrote:
> Good morning
> 
> On Mon, Apr 27, 2026 at 11:15:29AM -0500, Shah, Tanmay wrote:
>> Hello Beleswar,
>>
>> Thanks for reviews. Please find my answer below:
>>
>> On 4/24/2026 10:51 PM, Padhi, Beleswar wrote:
>>> Hi Tanmay,
>>>
>>> In $subject-line, s/remote node/remoteproc
>>>
>>
>> Ack. 'node' is platform management firmware term, which might not be
>> right here. subject line already contains remoteproc so no need to have
>> it again. Instead, will replace 'node' with 'core'. new subject:
>>
>> remoteproc: xlnx: check remote core state.
>>
> 
> Much better.
>  
>>> On 4/25/2026 8:32 AM, Tanmay Shah wrote:
>>>> The remote state is set to RPROC_DETACHED if the resource table is found
>>>> in the memory. However, this can be wrong if the remote is not started,
>>>> but firmware is still loaded in the memory. Use PM_GET_NODE_STATUS call
>>>> to the firmware to request the state of the RPU node. If the RPU is
>>>> actually out of reset and running, only then move the remote state to
>>>> RPROC_DETACHED, otherwise keep the remote state to RPROC_OFFLINE.
>>>
>>>
>>> This is a good additional check. However, one thing to note is

[...]

>>>> +
>>>> +        /*
>>>> +         * If RPU state is power on and out of reset i.e. running, then
>>>> +         * assign RPROC_DETACHED state. If the RPU is not out of reset
>>>> +         * then do not attempt to attach to the remote processor.
>>>> +         */
>>>> +        if (status == PM_NODE_RUNNING) {
>>>> +            if (zynqmp_r5_get_rsc_table_va(r5_core))
>>>> +                dev_dbg(r5_core->dev, "rsc tbl not found\n");
>>>
>>>
>>> Do you still want to set state = RPROC_DETACHED if resource table is not
>>> found in the
>>> memory?
>>>
>>
>> Yes. Not all the firmware that is running on remote core is expected to
>> have the resource table. The firmware might not use RPMsg at all, and in
>> that case resource table becomes irrelevant. However, we still need to
>> make sure that running core is not reported as offline.
> 
> Please add the above explanation to the inlined comment.  Otherwise I'm good
> with this patch but I'll need an RB from Michael before moving forward.  
> 

Ack.

> Do you see this as a bug fix?  Is there a point adding this patch to the stable
> kernels?
> 

That's a good point. I should add Fixes tag as well, for the commit that
introduced attach-detach feature.

I will do that in v2.

Thanks,
Tanmay

>>
>> Thanks.
>>
>>> Thanks,
>>> Beleswar
>>>
>>>> +            r5_core->rproc->state = RPROC_DETACHED;
>>>> +        }
>>>>       }
>>>>         return 0;
>>>> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/
>>>> firmware/xlnx-zynqmp.h
>>>> index d70dcd462b44..7e27b0f7bf7e 100644
>>>> --- a/include/linux/firmware/xlnx-zynqmp.h
>>>> +++ b/include/linux/firmware/xlnx-zynqmp.h
>>>> @@ -542,6 +542,18 @@ enum pm_gem_config_type {
>>>>       GEM_CONFIG_FIXED = 2,
>>>>   };
>>>>   +/**
>>>> + * enum pm_node_status - Device node status provided by xilpm fw
>>>> + * @PM_NODE_UNUSED: Device is not used
>>>> + * @PM_NODE_RUNNING: Device is power-on and out of reset
>>>> + * @PM_NODE_HALT: Device is power-on but in the reset state
>>>> + */
>>>> +enum pm_node_status {
>>>> +    PM_NODE_UNUSED = 0,
>>>> +    PM_NODE_RUNNING = 1,
>>>> +    PM_NODE_HALT = 12,
>>>> +};
>>>> +
>>>>   /**
>>>>    * struct zynqmp_pm_query_data - PM query data
>>>>    * @qid:    query ID
>>>> @@ -630,6 +642,8 @@ int zynqmp_pm_set_rpu_mode(u32 node_id, enum
>>>> rpu_oper_mode rpu_mode);
>>>>   int zynqmp_pm_set_tcm_config(u32 node_id, enum rpu_tcm_comb tcm_mode);
>>>>   int zynqmp_pm_get_node_status(const u32 node, u32 *const status,
>>>>                     u32 *const requirements, u32 *const usage);
>>>> +int zynqmp_pm_get_rpu_node_status(const u32 node, u32 *const status,
>>>> +                  u32 *const requirements, u32 *const usage);
>>>>   int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config,
>>>> u32 value);
>>>>   int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
>>>>                    u32 value);
>>>> @@ -939,6 +953,13 @@ static inline int zynqmp_pm_get_node_status(const
>>>> u32 node, u32 *const status,
>>>>       return -ENODEV;
>>>>   }
>>>>   +static inline int zynqmp_pm_get_rpu_node_status(const u32 node, u32
>>>> *const status,
>>>> +                        u32 *const requirements,
>>>> +                        u32 *const usage)
>>>> +{
>>>> +    return -ENODEV;
>>>> +}
>>>> +
>>>>   static inline int zynqmp_pm_set_sd_config(u32 node,
>>>>                         enum pm_sd_config_type config,
>>>>                         u32 value)
>>>>
>>>> base-commit: 6f860d238b44da8ac57be25289b9f4410691c4e2
>>