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