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.

Sign up for GitHub

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

Zephyr opanamp rsc dev 1 #1

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Zephyr opanamp rsc dev 1 #1

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5297cc8
samples: subsystem/ipc/opanamp_rsc: Add imx8mp_evk cortex m7
alxlastur Sep 9, 2024
2975b02
samples: subsystem/ipc/opanamp_rsc: Add imx93_evk cortex m33
alxlastur Sep 9, 2024
8a4fdee
dts: arm: nxp_imx93_m33.dtsi: Add mailbox unit node mu1 in dtsi file
alxlastur Sep 9, 2024
3c385fb
samples: ipc: openamp_rsc: main_remote.c
alxlastur Sep 9, 2024
46903cd
linker: add resource table section general linker file
alxlastur Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions samples/subsys/ipc/openamp_rsc_table/src/main_remote.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
Copy link
Collaborator

@dbaluta dbaluta Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We shouldn't start a commit message with an explanation of what the code DOES. But with an explanation of WHY the code is needed.

Also try to use natural language when explaining things. For the rest people can just inspect the code.

So, let's reformat the commit message as follows:

samples: openamp_rsc_table: Don't hardcode notification channel ID to 0

Currently, Zephyr is always sending back notifications to AP (e.g Linux in our case)
on channel 0. 

But this currently doesn't work if Linux uses other channel id for communication.
So, save the channel id used by Linux and use it to send back replies.

Copy link
Owner Author

Choose a reason for hiding this comment

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

should we put this in the same PR for imx8mp? Asking because the example wouldn't work if we don't modify the channel

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alxlastur yes, that makes sense.

Copy link
Owner Author

@alxlastur alxlastur Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also, by mistake the first time I made the PR, I made it over the one from the upstream zephyr github (not nxp one) and closed it immediately. But someone put some comments even if it was closed:
image

Then realised it was closed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have a link for this?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@dbaluta zephyrproject-rtos#78228

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, lets go on with the plan we already have. And once you add the PR I will copy paste @arnopo'
s comment and restart the discussion from there.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok @dbaluta , then I will make a PR for imx8mp with 2 commits:

  • First commit contains: samples/subsys/ipc/openamp_rsc_table/boards/imx8mp_evk_mimx8ml8_m7.conf and samples/subsys/ipc/openamp_rsc_table/boards/imx8mp_evk_mimx8ml8_m7.overlay

  • Second commit contain samples/subsys/ipc/openamp_rsc_table/src/main_remote.c

Right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. Make sure you add proper commit messages as I previously commented.

* Copyright (c) 2020, STMICROELECTRONICS
* Copyright 2024 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -63,6 +64,10 @@ struct rpmsg_rcv_msg {
size_t len;
};

struct ipm_channel {
uint8_t channel_id;
};

static struct metal_io_region *shm_io = &shm_io_data;

static struct metal_io_region *rsc_io = &rsc_io_data;
Expand All @@ -78,14 +83,19 @@ static struct rpmsg_rcv_msg sc_msg = {.data = rx_sc_msg};
static struct rpmsg_endpoint tty_ept;
static struct rpmsg_rcv_msg tty_msg;

static struct ipm_channel app_ipm_channel;

static K_SEM_DEFINE(data_sem, 0, 1);
static K_SEM_DEFINE(data_sc_sem, 0, 1);
static K_SEM_DEFINE(data_tty_sem, 0, 1);

static void platform_ipm_callback(const struct device *dev, void *context,
uint32_t id, volatile void *data)
{
struct ipm_channel *ipm_ch = (struct ipm_channel *)context;

LOG_DBG("%s: msg received from mb %d", __func__, id);
ipm_ch->channel_id = id;
k_sem_give(&data_sem);
}

Expand Down Expand Up @@ -130,10 +140,10 @@ static void new_service_cb(struct rpmsg_device *rdev, const char *name,

int mailbox_notify(void *priv, uint32_t id)
{
ARG_UNUSED(priv);
struct ipm_channel *ipm_ch = (struct ipm_channel *)priv;

LOG_DBG("%s: msg received", __func__);
ipm_send(ipm_handle, 0, id, NULL, 0);
ipm_send(ipm_handle, 0, ipm_ch->channel_id, NULL, 0);

return 0;
}
Expand Down Expand Up @@ -167,7 +177,7 @@ int platform_init(void)
return -1;
}

ipm_register_callback(ipm_handle, platform_ipm_callback, NULL);
ipm_register_callback(ipm_handle, platform_ipm_callback, (void *)&app_ipm_channel);

status = ipm_set_enabled(ipm_handle, 1);
if (status) {
Expand Down Expand Up @@ -197,7 +207,7 @@ platform_create_rpmsg_vdev(unsigned int vdev_index,

vdev = rproc_virtio_create_vdev(VIRTIO_DEV_DEVICE, VDEV_ID,
rsc_table_to_vdev(rsc_table),
rsc_io, NULL, mailbox_notify, NULL);
rsc_io, (void *)&app_ipm_channel, mailbox_notify, NULL);

if (!vdev) {
LOG_ERR("failed to create vdev");
Expand Down