Re: [PATCH v2] rpmsg: char: Export alias for RPMSG ID rpmsg-raw from table

[Andrew Davis <afd@ti.com>]

On 6/25/25 11:13 AM, Mathieu Poirier wrote:
> Good day,
> 
> On Thu, Jun 19, 2025 at 03:57:22PM -0500, Andrew Davis wrote:
>> Module aliases are used by userspace to identify the correct module to
>> load for a detected hardware. The currently supported RPMSG device IDs for
>> this module include "rpmsg-raw", but the module alias is "rpmsg_chrdev".
>>
>> Use the helper macro MODULE_DEVICE_TABLE(rpmsg) to export the correct
>> supported IDs. And while here, to keep backwards compatibility we also add
>> the other ID "rpmsg_chrdev" so that it is also still exported as an alias.
>>
>> This has the side benefit of adding support for some legacy firmware
>> which still uses the original "rpmsg_chrdev" ID. This was the ID used for
>> this driver before it was upstreamed (as reflected by the module alias).
> 
> I was surprised to receive this patch - my question from almost a year back is
> still pending.
> 

I answered[0] your question and never received any follow up questions so I had
assumed the answer was satisfactory.

>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> Acked-by: Hari Nagalla <hnagalla@ti.com>
>> Tested-by: Hari Nagalla <hnagalla@ti.com>
>> ---
>>
>> Changes for v2:
>>   - Rebased on v6.16-rc1
>>   - Added Acked/Tested-by
>>
>> [v1] https://www.spinics.net/lists/linux-remoteproc/msg18959.html
>>
>>   drivers/rpmsg/rpmsg_char.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
>> index eec7642d26863..96fcdd2d7093c 100644
>> --- a/drivers/rpmsg/rpmsg_char.c
>> +++ b/drivers/rpmsg/rpmsg_char.c
>> @@ -522,8 +522,10 @@ static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev)
>>   
>>   static struct rpmsg_device_id rpmsg_chrdev_id_table[] = {
>>   	{ .name	= "rpmsg-raw" },
>> +	{ .name	= "rpmsg_chrdev" },
>>   	{ },
>>   };
>> +MODULE_DEVICE_TABLE(rpmsg, rpmsg_chrdev_id_table);
> 
> ... and I still don't understand why this patch is needed.  What is broken that
> this patch fixes?
> 

Today when this driver is built as a module it does not automatically load
when a matching firmware is started. You can see examples of documentation
working around this by manually loading it[1] and even applications
attempting the same in code[2]. This should not be needed. Here is why
this happens and how this patch fixes it:

A given firmware that makes use of this driver will have one of two
RPMSG device IDs: "rpmsg-raw" or "rpmsg_chrdev". Let's look at each in
turn:

If the ID is "rpmsg_chrdev" then this driver module will be loaded into
the kernel (that is what the MODULE_ALIAS line below did). But it will
not cause the driver module to probe, as probe is triggered by a match
in the above rpmsg_device_id table.

If the ID is "rpmsg-raw" then this driver module will probe with the
firmware, but only if this driver module was already loaded into the
kernel, or was built-in to the kernel.

By adding "rpmsg_chrdev" to the table we make this driver probe for
firmware with that ID. And by adding MODULE_DEVICE_TABLE we make both
IDs trigger the module to be loaded if it has not been already.

This patch makes it so both IDs do both needed actions. Where before
each ID would only do one action, but not the other.

Andrew

[0] https://www.spinics.net/lists/linux-remoteproc/msg19814.html
[1] https://github.com/OpenAMP/openamp-system-reference/blob/main/examples/linux/rpmsg-mat-mul/README.md?plain=1#L42
[2] https://github.com/Xilinx/meta-openamp/blob/master/recipes-openamp/rpmsg-examples/rpmsg-mat-mul/mat_mul_demo.c#L306

> Thanks,
> Mathieu
> 
>>   
>>   static struct rpmsg_driver rpmsg_chrdev_driver = {
>>   	.probe = rpmsg_chrdev_probe,
>> @@ -565,6 +567,5 @@ static void rpmsg_chrdev_exit(void)
>>   }
>>   module_exit(rpmsg_chrdev_exit);
>>   
>> -MODULE_ALIAS("rpmsg:rpmsg_chrdev");
>>   MODULE_DESCRIPTION("RPMSG device interface");
>>   MODULE_LICENSE("GPL v2");
>> -- 
>> 2.39.2
>>