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

samples/ipc/openamp: Introduce OpenAMP v2018.10 Remote Echo Sample #11934

Conversation

diegosueiro
Copy link
Contributor

Introduces the OpenAMP Remote Echo Sample to have Linux and Zephyr communicating in i.MX7/6SX SoCs.

Note that this PR depends on #11355.

@diegosueiro
Copy link
Contributor Author

ping @galak and @wjliang.

@diegosueiro
Copy link
Contributor Author

Just need to review the last commit: samples/ipc/openamp: Introduce OpenAMP Remote Echo Sample

@stanislav-poboril
Copy link
Collaborator

@diegosueiro

Did you have anything printed out on Zephyr console?

If I remember it correctly I've got -2005 on endpoint creation on Zephyr console. Maybe just some memory allocation problem or something on Zephyr side.

What kernel Linux source are you using?

I've been using UDOObuntu version of Linux kernel (something old like 3.15), patched for RPMsg - it worked with RPMsg-Lite examples previously.

Did you check the devicetree nodes in the Linux side that I put in the README.rst?

Yes, it matches (except for the shared memory region which I have adjusted correspondingly in the Zephyr application).

What is the output of dmesg | grep -iE "virtio|vring|rpmsg|virtqueue" in the Linux?

I will try it on Wednesday. But I remember grep -i rpmsg returned just two rows. If I modprobe imx_rpmsg_tty, it proceeds without error but the /dev/ttyRPMSG is not created.

@diegosueiro diegosueiro force-pushed the openamp-v2018.10-imx-sample branch from 931453b to 51a3445 Compare December 11, 2018 05:21
@diegosueiro
Copy link
Contributor Author

@stanislav-poboril ,

I've been using UDOObuntu version of Linux kernel (something old like 3.15), patched for RPMsg - it worked with RPMsg-Lite examples previously.

Can you please make this change on this file and post the output?

-       dev_dbg(&vdev->dev, "buffers: va %p, dma 0x%llx\n", bufs_va,
+       dev_info(&vdev->dev, "buffers: va %p, dma 0x%llx\n", bufs_va,

@diegosueiro
Copy link
Contributor Author

@nashif and @galak,

I have no idea how to fix this sanitycheck error.

@stanislav-poboril
Copy link
Collaborator

@diegosueiro

dmesg | grep -iE "virtio|vring|rpmsg|virtqueue

# dmesg | grep -iE "virtio|vring|rpmsg|virtqueue"
[    0.136597] virtio_rpmsg_bus virtio0: rpmsg host is online
[    0.136695] imx rpmsg driver is registered.

Can you please make this change on this file and post the output?

Sorry I could not test this change in the Linux kernel today. And I am not available till end of the year. Maybe we can remove this example from UDOO board?

This return 0.

uint32_t virtqueue_get_desc_size(struct virtqueue *vq)
{
uint16_t head_idx = 0;
uint16_t avail_idx = 0;
uint32_t len = 0;
printk("vq->vq_available_idx %u\n", vq->vq_available_idx);
printk("vq->vq_ring.avail->idx %u\n", vq->vq_ring.avail->idx);
if (vq->vq_available_idx == vq->vq_ring.avail->idx) {
	printk("ret 0\n");
	return 0;
}


    ***** Booting Zephyr OS zephyr-v1.13.0-2509-g1daf559 *****
Starting application thread!
RPMSG_BUFFER_SIZE = 512
virtqueue_allocate sizeof(struct virtqueue)=56, num_desc_extra=256, sizeof(struct vq_desc_extra)=8
vq_size 2104
virtqueue_allocate sizeof(struct virtqueue)=56, num_desc_extra=256, sizeof(struct vq_desc_extra)=8
vq_size 2104
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
desc size 0
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
desc size u 0
sizeof(struct rpmsg_hdr) 16
src 0
dst 53
avail_size -16
size 40
rpmsg_create_ept failed: -2005
OpenAMP remote echo demo ended.

@diegosueiro
Copy link
Contributor Author

@diegosueiro

dmesg | grep -iE "virtio|vring|rpmsg|virtqueue

# dmesg | grep -iE "virtio|vring|rpmsg|virtqueue"
[    0.136597] virtio_rpmsg_bus virtio0: rpmsg host is online
[    0.136695] imx rpmsg driver is registered.

Can you please make this change on this file and post the output?

Sorry I could not test this change in the Linux kernel today. And I am not available till end of the year. Maybe we can remove this example from UDOO board?

This print will help to see what is the beginning of the shared memory address.
No problem. I can remove the i.MX6SX from this sample.

This return 0.

uint32_t virtqueue_get_desc_size(struct virtqueue *vq)
{
uint16_t head_idx = 0;
uint16_t avail_idx = 0;
uint32_t len = 0;
printk("vq->vq_available_idx %u\n", vq->vq_available_idx);
printk("vq->vq_ring.avail->idx %u\n", vq->vq_ring.avail->idx);
if (vq->vq_available_idx == vq->vq_ring.avail->idx) {
	printk("ret 0\n");
	return 0;
}


    ***** Booting Zephyr OS zephyr-v1.13.0-2509-g1daf559 *****
Starting application thread!
RPMSG_BUFFER_SIZE = 512
virtqueue_allocate sizeof(struct virtqueue)=56, num_desc_extra=256, sizeof(struct vq_desc_extra)=8
vq_size 2104
virtqueue_allocate sizeof(struct virtqueue)=56, num_desc_extra=256, sizeof(struct vq_desc_extra)=8
vq_size 2104
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
desc size 0
vq->vq_available_idx 0
vq->vq_ring.avail->idx 0
ret 0
desc size u 0
sizeof(struct rpmsg_hdr) 16
src 0
dst 53
avail_size -16
size 40
rpmsg_create_ept failed: -2005
OpenAMP remote echo demo ended.

This can be happening because it is looking into the wrong VRING_TX address. In the VRING_TX the master puts the available virtqueues. I'm not sure if it is possible to plug a debugger into the Udoo Neo, because using a debugger helped me a lot.

galak and others added 5 commits December 13, 2018 05:26
Since there's an official release lets update to it.

Signed-off-by: Kumar Gala <[email protected]>
Update to new OpenAMP v2018.10 release.  This release allows us to
utilize just rpmsg without remoteproc.  The API set has changed and
requires updates to the openamp sample.  Additionally, the changes
in this release reduce the code size footprint, and support a static
allocation memory model.

Signed-off-by: Kumar Gala <[email protected]>
…default

The lpcxpresso54114_m0 tends to be the secondary core so doesn't
typically have a UART setup for it.  We had SEGGER support enabled by
default previously, but the new Kconfig option CONFIG_USE_SEGGER_RTT
needs to be set as well now.

Signed-off-by: Kumar Gala <[email protected]>
The v2018.10 release of OpenAMP reworks the API set and splits the
remoteproc vs rpmsg interfaces so one can use rpmsg without remoteproc.

This helps drastically reduce the code footprint utilized by OpenAMP.
The remote see around 4k reduction in code size.

Signed-off-by: Kumar Gala <[email protected]>
Introduces the OpenAMP Remote Echo Sample to have Linux and Zephyr
communicating in i.MX7/6SX SoCs.

Signed-off-by: Diego Sueiro <[email protected]>
@diegosueiro diegosueiro force-pushed the openamp-v2018.10-imx-sample branch from 51a3445 to 62c0d74 Compare December 13, 2018 05:49
@diegosueiro
Copy link
Contributor Author

@stanislav-poboril,

Sorry I could not test this change in the Linux kernel today. And I am not available till end of the year. Maybe we can remove this example from UDOO board?

This print will help to see what is the beginning of the shared memory address.
No problem. I can remove the i.MX6SX from this sample.

I'll wait for the others review because I think this PR will be merged next year anyway. And maybe we will have time to debug this on the Udoo Neo.

Do you have a public repo where I can see the imx_rpmsg driver ported to the Udoo Neo kernel?

@diegosueiro
Copy link
Contributor Author

@arnop2 and @wjliang,

Can you please review this PR?

@arnopo
Copy link
Collaborator

arnopo commented Dec 13, 2018

@diegosueiro
i had a look to your PR. First of all, I would like to understand the context ( not familiar with the IMX environment). It is not the way i would implement the example, but probably it is just not the same context as mine...

  • Your sample concerns inter-processor communication between a Linux and a Zephyr, right?
  • On Linux side do you use the generic remoteproc framework to load the Zephyr firmware and the generic rpmsg framework for the IPC? Because i can not see any resource table that should be needed in this case.
    I sent to Kumar few month ago a sample to echo the Linux rpmsg sample client. Don't know if this could help you...it is based on resource table usage.

@diegosueiro
Copy link
Contributor Author

@arnop2,

Thanks for taking time to look into this.

@diegosueiro
i had a look to your PR. First of all, I would like to understand the context ( not familiar with the IMX environment). It is not the way i would implement the example, but probably it is just not the same context as mine...

  • Your sample concerns inter-processor communication between a Linux and a Zephyr, right?

Yes, that is right.

  • On Linux side do you use the generic remoteproc framework to load the Zephyr firmware and the generic rpmsg framework for the IPC? Because i can not see any resource table that should be needed in this case.

For the i.MX processors in the mainline there are only the mailbox driver and the remoteproc, but the later doesn't support "extracting" the resource table data from the remote firmware binary. And there is no rpmsg driver.
But, NXP provides in their source repository the rpmsg driver where the vrings data comes from the device tree. You can use this repo as a reference.
The remote firmware loading can be done by u-boot or a userspace tool.

This why we don't need to use the remoteproc framework for the lifecycle management and the vring data extraction.

I already have Zephyr and Linux communicating by using the RPMsg-lite: slides and video.

I sent to Kumar few month ago a sample to echo the Linux rpmsg sample client. Don't know if this could help you...it is based on resource table usage.

@arnopo
Copy link
Collaborator

arnopo commented Dec 13, 2018

@arnop2,

Thanks for taking time to look into this.

@diegosueiro
i had a look to your PR. First of all, I would like to understand the context ( not familiar with the IMX environment). It is not the way i would implement the example, but probably it is just not the same context as mine...

  • Your sample concerns inter-processor communication between a Linux and a Zephyr, right?

Yes, that is right.

  • On Linux side do you use the generic remoteproc framework to load the Zephyr firmware and the generic rpmsg framework for the IPC? Because i can not see any resource table that should be needed in this case.

For the i.MX processors in the mainline there are only the mailbox driver and the remoteproc, but the later doesn't support "extracting" the resource table data from the remote firmware binary. And there is no rpmsg driver.
But, NXP provides in their source repository the rpmsg driver where the vrings data comes from the device tree. You can use this repo as a reference.
The remote firmware loading can be done by u-boot or a userspace tool.

This why we don't need to use the remoteproc framework for the lifecycle management and the vring data extraction.

I already have Zephyr and Linux communicating by using the RPMsg-lite: slides and video.

Thank for the clarification! now i better understand your implementation.

@arnopo
Copy link
Collaborator

arnopo commented Dec 13, 2018

@diegosueiro

I reviewed your patch, i did not see anything wrong.
But my concern here is that it is not the generic way to implement communication with the Linux OS, but IMX platform specific. From my point of view a good example would be to answer to the rpmsg client sample driver. This drivers follow the generic implementation of the rpmsg protocol in Linux, so that would allows Zephyr user to simply test it without implementing a specific Linux drivers and/or a specific zephyr application. Do you plan also to implement it?

@diegosueiro
Copy link
Contributor Author

@diegosueiro

I reviewed your patch, i did not see anything wrong.
But my concern here is that it is not the generic way to implement communication with the Linux OS, but IMX platform specific. From my point of view a good example would be to answer to the rpmsg client sample driver. This drivers follow the generic implementation of the rpmsg protocol in Linux, so that would allows Zephyr user to simply test it without implementing a specific Linux drivers and/or a specific zephyr application. Do you plan also to implement it?

At the moment this sample is tight up to the iMX processors and RPMsg driver implementation in the Linux side only for the virtio operations. Is there any other place in this sample that is specific to the imx?
I'm not planning to upstream a more generic Linux RPMsg driver in the short term.

@arnopo
Copy link
Collaborator

arnopo commented Dec 13, 2018

@diegosueiro
I reviewed your patch, i did not see anything wrong.
But my concern here is that it is not the generic way to implement communication with the Linux OS, but IMX platform specific. From my point of view a good example would be to answer to the rpmsg client sample driver. This drivers follow the generic implementation of the rpmsg protocol in Linux, so that would allows Zephyr user to simply test it without implementing a specific Linux drivers and/or a specific zephyr application. Do you plan also to implement it?

At the moment this sample is tight up to the iMX processors and RPMsg driver implementation in the Linux side only for the virtio operations. Is there any other place in this sample that is specific to the imx?
I'm not planning to upstream a more generic Linux RPMsg driver in the short term.

i'm not sure that we speak about the same things, so lets me precise my thinking. When i spoke about the rpmsg sample client, i was referred to this one that is already in the kernel mainline
https://elixir.bootlin.com/linux/latest/source/samples/rpmsg/rpmsg_client_sample.c
what i would expect is that the sample echo answer to this driver.
your sample is compatible with the NXP branch based on a Kernel 4.9, but i 'm not sure that it is straightforward to integrate it on another platform with the lasted Kernel.
This is my concern. So, if the aim of your sample is to show an example for a specific platform running a specific kernel, i'm ok.
But if this example has to be maintained and integrated on some other platforms, i would suggest to change it...

@stanislav-poboril
Copy link
Collaborator

@diegosueiro

Do you have a public repo where I can see the imx_rpmsg driver ported to the Udoo Neo kernel?

UDOO Neo kernel is here and it was patched like described here. Ignore the "Build M4 Image" section. Also ignore the "rpmsg" branch in UDOO kernel repo, I haven't used it. One more difference is that I have used later version of UDOO kernel than described in the readme and patched the latest commit (5111ea94bbd0818035351fce39c83f1dbf81457b).

I'm not sure if it is possible to plug a debugger into the Udoo Neo, because using a debugger helped me a lot.

I used Lauterbach Trace 32 to debug UDOO Neo's M4 core before. There are only pads for the debug connector, so one have to solder the connector or the wires directly. Pausing the debugger stops also the A9 core and having it stopped for longer time stops also the USB connection with a PC. But it is still usable.

CONFIG_HAVE_IMX_RPMSG=y
CONFIG_RPMSG=y
CONFIG_RPMSG_VIRTIO=y
CONFIG_IMX_RPMSG_TTY=m
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason to enable IMX_RPMST_TTY driver?
Just check the .c file, it looks like it implements the echo, from meta-openamp, there is userspace demo which uses the kernel rpmsg_char driver, as @arnop2 mentioned, there is also kernel sample driver.
Here is the OpenAMP apps for the kernel sample driver: https://github.com/OpenAMP/open-amp/blob/master/apps/examples/rpmsg_sample_echo/rpmsg-sample-ping.c

It looks like only the virtio operation implementation is different, maybe you can reuse that application.

}

/* setup vdev */
vq[0] = virtqueue_allocate(VRING_SIZE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this function will do dynamic memory allocation. Just in case, if you want to avoid dynamic allocation, you can statically defined the virtqueue. As your demo is RPMSG_REMOTE, you can try static virtqueue vq[2].


/* Send data back to master */
memset(payload, 0, RPMSG_BUFFER_SIZE);
memcpy(payload, data, len);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to do memset 0.

@wjliang
Copy link
Collaborator

wjliang commented Dec 19, 2018

Just from this commit: 62c0d74
It looks like it is also doing echoing, which is similar to this pull request #11355 (@galak). Only the notification and the shared memory is different. just wonder if it can combine these two into one demo, and use configuration options to enable different configuration?

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

+1 for doc changes, thanks.

@ioannisg
Copy link
Member

@diegosueiro could you rebase this PR, if there's a plan to work on it again?

@galak
Copy link
Collaborator

galak commented Apr 17, 2020

closing at this has be stale for sometime, feel free to re-open and update.

@galak galak closed this Apr 17, 2020
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.