Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

examples: rpmsg-proxy-app: Update drivers being removed #10

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

bentheredonethat
Copy link
Contributor

As rpmsg_char driver will still be in use by rpmsg_ctrl, update to remove drivers in correct order to remove error:
'modprobe: FATAL: Module rpmsg_char is in use.'

Signed-off-by: Ben Levinsky ben.levinsky@amd.com

@@ -207,7 +207,8 @@ int file_write(char *path, char *str)
/* Stop remote CPU and Unload drivers */
void stop_remote(void)
{
system("modprobe -r rpmsg_char");
system("modprobe -r rpmsg_ctrl");
system("modprobe -r virtio_rpmsg_bus");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more need to call system("modprobe -r rpmsg_char"); ?
If no more needed please provide the reason in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @arnopo because (at least on the Xilinx-AMD setup) when we remove virtio_rpmsg_bus, the rpsg_char driver is removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attempting to remove the rpmsg_char won't work because the other drivers have it in use

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what i can see on the stm32mp1 in term of "used by"( the modules are automatically loaded on device probe)

root@stm32mp15-disco-oss:~# lsmod
Module                  Size  Used by
rpmsg_client_sample    16384  0
rpmsg_tty              16384  0
rpmsg_ctrl             16384  0
rpmsg_char             16384  1 rpmsg_ctrl
virtio_rpmsg_bus       20480  0
rpmsg_ns               16384  1 virtio_rpmsg_bus
rpmsg_core             16384  6 rpmsg_client_sample,rpmsg_tty,rpmsg_char,rpmsg_ctrl,rpmsg_ns,virtio_rpmsg_bus

When I remove the rpmsg_ctrl, this remove the rpmsg_char . So that seems coherent.

If I have a look to the code : https://github.com/OpenAMP/openamp-system-reference/blob/1e91a368c674574e1eea70f495891a0b814952e4/examples/linux/rpmsg-proxy-app/proxy_app.c#L436..L444

There seems to be an imbalance between the mprobe and mprobe -r.
the rpmsg_char and rpmsg_ctrl modules are loaded and the rpmsg_ctrl and the virtio_rpmsg_bus modules are removed.

Is it possible to perform a lsmod before, during and after running the application ?
I would like understand from which state you start and in which state you are during and after the demo.

Copy link
Contributor Author

@bentheredonethat bentheredonethat Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arnopo sure.

here is at boot:

lsmod
Module Size Used by
zynqmp_r5_remoteproc 20480 0
uio_pdrv_genirq 16384 0
cfg80211 360448 0
zocl 184320 0

Here is after loading firmware:

echo start > /sys/class/remoteproc/remoteproc0/state
[ 140.168816] remoteproc remoteproc0: powering up ff9a0000.rf5ss:r5f_0
[ 140.171704] remoteproc remoteproc0: Booting fw image image_rpc_demo, size 621132
Starting application...
[ 140.459880] rproc-virtio rproc-virtio.0: registered virtio0 (type 7)
[ 140.460311] remoteproc remoteproc0: remote processor ff9a0000.rf5ss:r5f_0 is now up
0 L7 registered generic bus
1 L7 init_system():160 c_buf,c_len = 0x3ed2017c,4096
2 L6 platform_init():169 platform_create_proc()
3 L6 platform_create_proc():102 rsc_table, rsc_size = 0x3ed20000, 0x100
4 L7 zynqmp_r5_a53_proc_init():73 metal_device_open(generic, poll_dev, 0x2ca8)
5 L7 platform_create_proc():112 poll{name,bus,chn_mask} = poll_dev,generic,0x20
6 L7 zynqmp_r5_a53_proc_mmap():138 lpa,lda= 0x3ed20000,0xffffffff
7 L7 zynqmp_r5_a53_proc_mmap():150 mem= 0x3f58
8 L7 zynqmp_r5_a53_proc_mmap():154 tmpio= 0x3f98
9 L7 zynqmp_r5_a53_proc_mmap():138 lpa,lda= 0x3ed40000,0xffffffff
10 L7 zynqmp_r5_a53_proc_mmap():150 mem= 0x3fe0
11 L7 zynqmp_r5_a53_proc_mmap():154 tmpio= 0x4020
12 L6 platform_create_proc():141 Initialize remoteproc successfully.
13 L6 platform_create_rpmsg_vdev():201 creating remoteproc virtio rproc 0x3ed20138
14 L6 platform_create_rpmsg_vdev():209 initializing rpmsg shared buffer pool
15 L6 platform_create_rpmsg_vdev():214 initializing rpmsg vdev
[ 140.554347] virtio_rpmsg_bus virtio0: rpmsg host is online
Initializating I/Os redirection...
[ 140.557803] virtio_rpmsg_bus virtio0: creating channel rpmsg-openamp-demo-channel addr 0x400
xilinx-vck190-20231:/lib/firmware#
xilinx-vck190-20231:/lib/firmware# lsmod
Module Size Used by
rpmsg_ctrl 16384 0
rpmsg_char 16384 1 rpmsg_ctrl
virtio_rpmsg_bus 20480 0
rpmsg_ns 16384 1 virtio_rpmsg_bus
rpmsg_core 16384 4 virtio_rpmsg_bus,rpmsg_char,rpmsg_ctrl,rpmsg_ns
zynqmp_r5_remoteproc 20480 0
uio_pdrv_genirq 16384 0
cfg80211 360448 0
zocl 184320 0

after running application:
lsmod
Module Size Used by
rpmsg_ctrl 16384 0
rpmsg_char 16384 1 rpmsg_ctrl
virtio_rpmsg_bus 20480 0
rpmsg_ns 16384 1 virtio_rpmsg_bus
rpmsg_core 16384 4 virtio_rpmsg_bus,rpmsg_char,rpmsg_ctrl,rpmsg_ns
zynqmp_r5_remoteproc 20480 0
uio_pdrv_genirq 16384 0
cfg80211 360448 0
zocl 184320 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the traces! We have same behavior.

  • Do we really need to remove the modules at the end of the application? If new dependencies are introduced later, we will probably have to update it.
  • Same for the mode probe that loads the modules at the beginning of that application.

I wonder if cleaning all these modprobes should not be the most convenient solution...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree with that sentiment. i can send a patch to remove the modprobe's in general?

@TanmayShah-xilinx do you agree?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree. We should get rid of all the modprobes from the Application and provide instructions in README file to perform modprobe before running the demo. So, when you send patch to remove modprobe, please update README files for each demo accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will do.

@bentheredonethat
Copy link
Contributor Author

@arnopo sent new patch to remove driver load/unload

thanks

@tnmysh
Copy link
Collaborator

tnmysh commented Feb 3, 2023

@bentheredonethat I would prefer that README files for each example is updated in the same commit.

Thanks,
Tanmay

As the requirements for RPMsg usage can change over time, remove the loading
and unloading of drivers in the Linux app. Instead put the onus on the user to
manage the driver.

Update documentation of demos in READMEs

Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
@arnopo arnopo merged commit 7f1fb3b into OpenAMP:main Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants