Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables

[Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>]

On 5/13/26 08:30, Daniel Baluta wrote:
> On 5/12/26 17:53, Shah, Tanmay wrote:
>>
>>
>> On 5/12/2026 2:55 AM, Daniel Baluta wrote:
>>> On 5/12/26 00:18, Ben Levinsky wrote:
>>>> [You don't often get email from ben.levinsky@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>
>>>> Add a small helper around rproc_elf_load_rsc_table() for remoteproc
>>>> drivers that treat a missing ELF resource table as optional. The helper
>>>> returns success on -EINVAL and propagates other failures unchanged.
>>>>
>>>> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
>>>> ---
>>>>   drivers/remoteproc/remoteproc_internal.h | 12 ++++++++++++
>>>>   1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
>>>> index 3724a47a9748..dff87e468837 100644
>>>> --- a/drivers/remoteproc/remoteproc_internal.h
>>>> +++ b/drivers/remoteproc/remoteproc_internal.h
>>>> @@ -146,6 +146,18 @@ static inline int rproc_mem_entry_iounmap(struct rproc *rproc,
>>>>          return 0;
>>>>   }
>>>>
>>>> +static inline int rproc_elf_load_rsc_table_optional(struct rproc *rproc,
>>>> +                                                   const struct firmware *fw)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       ret = rproc_elf_load_rsc_table(rproc, fw);
>>>> +       if (ret == -EINVAL)
>>>> +               dev_dbg(&rproc->dev, "no resource table found\n");
>>>
>>> You are changing loglevel here. Initial drivers use dev_info or dev_warn. At least I'm used
>>> with seeing this messages in the logs.
>>>
>>> So, what do you think on adding at least dev_info to this instead of dev_dbg?
>>>
>>
>> Actually can we leave that choice to the platform driver ? There are
>> many use cases where the remoteproc subsystem is used to load and start
>> the remote core and the firmware doesn't have the resource table. We
>> don't want to make info level log for such use cases, as the resource
>> table is not expected in the first place there.
> 
> Agree, this is the best way to go.
> 
> 

LGTM

If you keep the rproc_elf_load_rsc_table_optional() helper, I would 
suggest inverting the logic for dev_dbg(). Regarding the discussion, it 
seems more logical to print a message when a resource table is found.
An add-on could be to also print the address and size found.

Thanks,
Arnaud