2020-07-31 11:48:45

by Arnaud POULIQUEN

Subject: [PATCH 9/9] rpmsg: ns: name service announcement endianness

The endianness has to be fixed to ensure that both sides can
decode the message. The Little endian format is used according
to the generic virtio implementation.

Signed-off-by: Arnaud Pouliquen <[email protected]>
---
drivers/rpmsg/rpmsg_ns.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index cc2bd47c415a..b478e2b55213 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -33,8 +33,8 @@ enum rpmsg_ns_flags {
*/
struct rpmsg_ns_msg {
char name[RPMSG_NAME_SIZE];
- u32 addr;
- u32 flags;
+ __le32 addr;
+ __le32 flags;
} __packed;

/**
@@ -57,8 +57,8 @@ static int rpmsg_ns_channel_announce(struct rpmsg_device *rpdev, int ns_flag)
return -EINVAL;

strncpy(nsm.name, rpdev->id.name, RPMSG_NAME_SIZE);
- nsm.addr = rpdev->ept->addr;
- nsm.flags = ns_flag;
+ nsm.addr = cpu_to_le32(rpdev->ept->addr);
+ nsm.flags = cpu_to_le32(ns_flag);

return rpmsg_send_offchannel(rpdev->ept, RPMSG_NS_ADDR, RPMSG_NS_ADDR,
&nsm, sizeof(nsm));
@@ -84,6 +84,7 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
struct rpmsg_device *newch;
struct rpmsg_channel_info chinfo;
struct device *dev = &rpdev->dev;
+ unsigned int flags = le32_to_cpu(msg->flags);
int ret;

#if defined(CONFIG_DYNAMIC_DEBUG)
@@ -101,13 +102,13 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,

strncpy(chinfo.name, msg->name, sizeof(chinfo.name));
chinfo.src = RPMSG_ADDR_ANY;
- chinfo.dst = msg->addr;
+ chinfo.dst = le32_to_cpu(msg->addr);

dev_info(dev, "%sing channel %s addr 0x%x\n",
- msg->flags & RPMSG_NS_DESTROY ? "destroy" : "creat",
- msg->name, msg->addr);
+ flags & RPMSG_NS_DESTROY ? "destroy" : "creat",
+ msg->name, chinfo.dst);

- if (msg->flags & RPMSG_NS_DESTROY) {
+ if (flags & RPMSG_NS_DESTROY) {
ret = rpmsg_release_channel(rpdev, &chinfo);
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
--
2.17.1

2020-08-24 23:06:05

by Mathieu Poirier

Subject: Re: [PATCH 9/9] rpmsg: ns: name service announcement endianness

On Fri, Jul 31, 2020 at 01:47:32PM +0200, Arnaud Pouliquen wrote:
> The endianness has to be fixed to ensure that both sides can
> decode the message. The Little endian format is used according
> to the generic virtio implementation.
>
> Signed-off-by: Arnaud Pouliquen <[email protected]>
> ---
> drivers/rpmsg/rpmsg_ns.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
> index cc2bd47c415a..b478e2b55213 100644
> --- a/drivers/rpmsg/rpmsg_ns.c
> +++ b/drivers/rpmsg/rpmsg_ns.c
> @@ -33,8 +33,8 @@ enum rpmsg_ns_flags {
> */
> struct rpmsg_ns_msg {
> char name[RPMSG_NAME_SIZE];
> - u32 addr;
> - u32 flags;
> + __le32 addr;
> + __le32 flags;
> } __packed;
>
> /**
> @@ -57,8 +57,8 @@ static int rpmsg_ns_channel_announce(struct rpmsg_device *rpdev, int ns_flag)
> return -EINVAL;
>
> strncpy(nsm.name, rpdev->id.name, RPMSG_NAME_SIZE);
> - nsm.addr = rpdev->ept->addr;
> - nsm.flags = ns_flag;
> + nsm.addr = cpu_to_le32(rpdev->ept->addr);
> + nsm.flags = cpu_to_le32(ns_flag);
>
> return rpmsg_send_offchannel(rpdev->ept, RPMSG_NS_ADDR, RPMSG_NS_ADDR,
> &nsm, sizeof(nsm));
> @@ -84,6 +84,7 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
> struct rpmsg_device *newch;
> struct rpmsg_channel_info chinfo;
> struct device *dev = &rpdev->dev;
> + unsigned int flags = le32_to_cpu(msg->flags);
> int ret;
>
> #if defined(CONFIG_DYNAMIC_DEBUG)
> @@ -101,13 +102,13 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
>
> strncpy(chinfo.name, msg->name, sizeof(chinfo.name));
> chinfo.src = RPMSG_ADDR_ANY;
> - chinfo.dst = msg->addr;
> + chinfo.dst = le32_to_cpu(msg->addr);
>
> dev_info(dev, "%sing channel %s addr 0x%x\n",
> - msg->flags & RPMSG_NS_DESTROY ? "destroy" : "creat",
> - msg->name, msg->addr);
> + flags & RPMSG_NS_DESTROY ? "destroy" : "creat",
> + msg->name, chinfo.dst);
>
> - if (msg->flags & RPMSG_NS_DESTROY) {
> + if (flags & RPMSG_NS_DESTROY) {
> ret = rpmsg_release_channel(rpdev, &chinfo);
> if (ret)
> dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);

Please merge this patch to patch 5, I don't see a need for doing this
separately.

Thanks,
Mathieu

> --
> 2.17.1
>