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

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

[Andrew Davis]

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

Signed-off-by: Andrew Davis <afd@ti.com>
---
 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);
 
 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

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

[Hari Nagalla]

On 7/29/24 11:45, 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).
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
Acked/Tested-by: Hari Nagalla <hnagalla@ti.com>
> ---
>   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);
>   
>   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");

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

[Mathieu Poirier]

Hi Andrew,

On Mon, Jul 29, 2024 at 11:45:27AM -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).
> 
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  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);

So you want to be able to do both "modprobe rpmsg-raw" and "modprobe
rpmsg_chrdev" ?

I'm not sure to get the aim of your patch and will need a little more details.

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
> 

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

[Andrew Davis]

On 8/11/24 4:59 PM, Mathieu Poirier wrote:
> Hi Andrew,
> 
> On Mon, Jul 29, 2024 at 11:45:27AM -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).
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   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);
> 
> So you want to be able to do both "modprobe rpmsg-raw" and "modprobe
> rpmsg_chrdev" ?
> 
> I'm not sure to get the aim of your patch and will need a little more details.
> 

So there are two parts to driver-device binding, loading the driver into the
kernel, and matching the device to the driver. When a firmware (or any device)
is detected the kernel starts by emitting a signal to userspace so it can load
the kernel module for a driver if available. Then the kernel looks through
its device tables, if a match is found it probe()s that driver.

In this case, for the first part, this driver has the "ALIAS" set as
"rpmsg:rpmsg_chrdev", and so only firmware which sends an rpmsg name
equal to "rpmsg_chrdev" will match and cause this module to be loaded
if it is not already loaded (or built-in).

But this driver's "device_id" table only matches with "rpmsg-raw".
So only firmware with that specific rpmsg name will actually probe().

This means for a given firmware, either automatic module loading will
not work, or driver binding will not work. I want both to work for
both rpmsg names.

We put both names in the device_id table, then use MODULE_DEVICE_TABLE()
on that table. What MODULE_DEVICE_TABLE() does is it creates a matching
module ALIAS for all items in the table. This way any matching ID will
also load the module. And we can then drop the explicit MODULE_ALIAS at the
bottom as it will be created for us for both names. This keeps the ID table
and the module ALIASes consistent.

This patch is still valid and applies on v6.13-rc1, but if you would
like me to re-send it to help your patch tracking just let me know.

Thanks,
Andrew

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