On Wed, Jun 25, 2025 at 12:12:03PM -0500, Andrew Davis wrote: > 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. >
Ah! I never saw your reply, apologies for that. > > > > > > Signed-off-by: Andrew Davis <a...@ti.com> > > > Acked-by: Hari Nagalla <hnaga...@ti.com> > > > Tested-by: Hari Nagalla <hnaga...@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. The part I was missing is the call to add_uevent_var() that uses @rpdev->id.name in rpmsg_uevent() - with that in mind it makes sense. I have applied your patch. Thanks, Mathieu > > 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 > > >
