* [PATCH] rpmsg_ns: Work around TI non-standard message
@ 2024-10-11 12:39 Richard Weinberger
2024-10-12 15:53 ` kernel test robot
2024-10-15 16:48 ` Mathieu Poirier
0 siblings, 2 replies; 10+ messages in thread
From: Richard Weinberger @ 2024-10-11 12:39 UTC (permalink / raw)
To: linux-remoteproc
Cc: linux-kernel, mathieu.poirier, andersson, upstream+rproc,
Richard Weinberger, ohad, s-anna, t-kristo
Texas Instruments ships a patch in their vendor kernels,
which adds a new NS message that includes a description field.
While TI is free to do whatever they want in their copy of the kernel,
it becomes a mess when people switch to a mainline kernel and want
to use their existing DSP programs with it.
To make it easier to migrate to a mainline kernel,
let's make the kernel aware of their non-standard extension but
briefly ignore the description field.
[0] https://patchwork.kernel.org/project/linux-remoteproc/patch/20190815231448.10100-1-s-anna@ti.com/
[1] https://stash.phytec.com/projects/PUB/repos/linux-phytec-ti/commits/aeded1f439effc84aa9f4e341a6e92ce1844ab98#drivers/rpmsg/virtio_rpmsg_bus.c
Cc: ohad@wizery.com
Cc: s-anna@ti.com
Cc: t-kristo@ti.com
Signed-off-by: Richard Weinberger <richard@nod.at>
---
FWIW, this is a forward port of a patch I'm using on v6.6.
Thanks,
//richard
---
drivers/rpmsg/rpmsg_ns.c | 30 ++++++++++++++++++++++--------
include/linux/rpmsg/ns.h | 8 ++++++++
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index bde8c8d433e0a..2fb3721eb0141 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -31,10 +31,11 @@ EXPORT_SYMBOL(rpmsg_ns_register_device);
static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
void *priv, u32 src)
{
- struct rpmsg_ns_msg *msg = data;
struct rpmsg_device *newch;
struct rpmsg_channel_info chinfo;
struct device *dev = rpdev->dev.parent;
+ __rpmsg32 ns_addr, ns_flags;
+ char *ns_name;
int ret;
#if defined(CONFIG_DYNAMIC_DEBUG)
@@ -42,23 +43,36 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
data, len, true);
#endif
- if (len != sizeof(*msg)) {
+ if (len == sizeof(struct rpmsg_ns_msg)) {
+ struct rpmsg_ns_msg *msg = data;
+
+ ns_addr = msg->addr;
+ ns_flags = msg->flags;
+ ns_name = msg->name;
+ } else if (len == sizeof(struct __rpmsg_ns_msg_ti)) {
+ struct __rpmsg_ns_msg_ti *msg = data;
+
+ ns_addr = msg->addr;
+ ns_flags = msg->flags;
+ ns_name = msg->name;
+ dev_warn(dev, "non-standard ns msg found\n");
+ } else {
dev_err(dev, "malformed ns msg (%d)\n", len);
return -EINVAL;
}
/* don't trust the remote processor for null terminating the name */
- msg->name[RPMSG_NAME_SIZE - 1] = '\0';
+ ns_name[RPMSG_NAME_SIZE - 1] = '\0';
- strscpy_pad(chinfo.name, msg->name, sizeof(chinfo.name));
+ strscpy_pad(chinfo.name, ns_name, sizeof(chinfo.name));
chinfo.src = RPMSG_ADDR_ANY;
- chinfo.dst = rpmsg32_to_cpu(rpdev, msg->addr);
+ chinfo.dst = rpmsg32_to_cpu(rpdev, ns_addr);
dev_info(dev, "%sing channel %s addr 0x%x\n",
- rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY ?
- "destroy" : "creat", msg->name, chinfo.dst);
+ rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY ?
+ "destroy" : "creat", ns_name, chinfo.dst);
- if (rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY) {
+ if (rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY) {
ret = rpmsg_release_channel(rpdev, &chinfo);
if (ret)
dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
diff --git a/include/linux/rpmsg/ns.h b/include/linux/rpmsg/ns.h
index a7804edd6d58f..60fca84ad4cea 100644
--- a/include/linux/rpmsg/ns.h
+++ b/include/linux/rpmsg/ns.h
@@ -26,6 +26,14 @@ struct rpmsg_ns_msg {
__rpmsg32 flags;
} __packed;
+/* Non-standard extended ns message by Texas Instruments */
+struct __rpmsg_ns_msg_ti {
+ char name[RPMSG_NAME_SIZE];
+ char desc[RPMSG_NAME_SIZE]; /* ignored */
+ u32 addr;
+ u32 flags;
+} __packed;
+
/**
* enum rpmsg_ns_flags - dynamic name service announcement flags
*
--
2.35.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-11 12:39 [PATCH] rpmsg_ns: Work around TI non-standard message Richard Weinberger
@ 2024-10-12 15:53 ` kernel test robot
2024-10-14 9:24 ` Richard Weinberger
2024-10-15 16:48 ` Mathieu Poirier
1 sibling, 1 reply; 10+ messages in thread
From: kernel test robot @ 2024-10-12 15:53 UTC (permalink / raw)
To: Richard Weinberger, linux-remoteproc
Cc: oe-kbuild-all, linux-kernel, mathieu.poirier, andersson,
upstream+rproc, Richard Weinberger, ohad, s-anna, t-kristo
Hi Richard,
kernel test robot noticed the following build warnings:
[auto build test WARNING on remoteproc/rpmsg-next]
[also build test WARNING on linus/master v6.12-rc2 next-20241011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Richard-Weinberger/rpmsg_ns-Work-around-TI-non-standard-message/20241011-204122
base: https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rpmsg-next
patch link: https://lore.kernel.org/r/20241011123922.23135-1-richard%40nod.at
patch subject: [PATCH] rpmsg_ns: Work around TI non-standard message
config: x86_64-randconfig-121-20241012 (https://download.01.org/0day-ci/archive/20241012/202410122348.irTWFe4S-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241012/202410122348.irTWFe4S-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410122348.irTWFe4S-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/rpmsg/rpmsg_ns.c:55:25: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __rpmsg32 [assigned] [usertype] ns_addr @@ got unsigned int [usertype] addr @@
drivers/rpmsg/rpmsg_ns.c:55:25: sparse: expected restricted __rpmsg32 [assigned] [usertype] ns_addr
drivers/rpmsg/rpmsg_ns.c:55:25: sparse: got unsigned int [usertype] addr
>> drivers/rpmsg/rpmsg_ns.c:56:26: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __rpmsg32 [assigned] [usertype] ns_flags @@ got unsigned int [usertype] flags @@
drivers/rpmsg/rpmsg_ns.c:56:26: sparse: expected restricted __rpmsg32 [assigned] [usertype] ns_flags
drivers/rpmsg/rpmsg_ns.c:56:26: sparse: got unsigned int [usertype] flags
vim +55 drivers/rpmsg/rpmsg_ns.c
45
46 if (len == sizeof(struct rpmsg_ns_msg)) {
47 struct rpmsg_ns_msg *msg = data;
48
49 ns_addr = msg->addr;
50 ns_flags = msg->flags;
51 ns_name = msg->name;
52 } else if (len == sizeof(struct __rpmsg_ns_msg_ti)) {
53 struct __rpmsg_ns_msg_ti *msg = data;
54
> 55 ns_addr = msg->addr;
> 56 ns_flags = msg->flags;
57 ns_name = msg->name;
58 dev_warn(dev, "non-standard ns msg found\n");
59 } else {
60 dev_err(dev, "malformed ns msg (%d)\n", len);
61 return -EINVAL;
62 }
63
64 /* don't trust the remote processor for null terminating the name */
65 ns_name[RPMSG_NAME_SIZE - 1] = '\0';
66
67 strscpy_pad(chinfo.name, ns_name, sizeof(chinfo.name));
68 chinfo.src = RPMSG_ADDR_ANY;
69 chinfo.dst = rpmsg32_to_cpu(rpdev, ns_addr);
70
71 dev_info(dev, "%sing channel %s addr 0x%x\n",
72 rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY ?
73 "destroy" : "creat", ns_name, chinfo.dst);
74
75 if (rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY) {
76 ret = rpmsg_release_channel(rpdev, &chinfo);
77 if (ret)
78 dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
79 } else {
80 newch = rpmsg_create_channel(rpdev, &chinfo);
81 if (!newch)
82 dev_err(dev, "rpmsg_create_channel failed\n");
83 }
84
85 return 0;
86 }
87
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-12 15:53 ` kernel test robot
@ 2024-10-14 9:24 ` Richard Weinberger
0 siblings, 0 replies; 10+ messages in thread
From: Richard Weinberger @ 2024-10-14 9:24 UTC (permalink / raw)
To: Richard Weinberger, linux-remoteproc, upstream
Cc: oe-kbuild-all, linux-kernel, mathieu.poirier, andersson,
upstream+rproc, Richard Weinberger, ohad, s-anna, t-kristo,
kernel test robot
Am Samstag, 12. Oktober 2024, 17:53:16 CEST schrieb kernel test robot:
> sparse warnings: (new ones prefixed by >>)
> >> drivers/rpmsg/rpmsg_ns.c:55:25: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __rpmsg32 [assigned] [usertype] ns_addr @@ got unsigned int [usertype] addr @@
> drivers/rpmsg/rpmsg_ns.c:55:25: sparse: expected restricted __rpmsg32 [assigned] [usertype] ns_addr
> drivers/rpmsg/rpmsg_ns.c:55:25: sparse: got unsigned int [usertype] addr
> >> drivers/rpmsg/rpmsg_ns.c:56:26: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __rpmsg32 [assigned] [usertype] ns_flags @@ got unsigned int [usertype] flags @@
> drivers/rpmsg/rpmsg_ns.c:56:26: sparse: expected restricted __rpmsg32 [assigned] [usertype] ns_flags
> drivers/rpmsg/rpmsg_ns.c:56:26: sparse: got unsigned int [usertype] flags
sprase is right.
I missed to replace u32 in the struct by __rpmsg32.
Thanks,
//richard
--
sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-11 12:39 [PATCH] rpmsg_ns: Work around TI non-standard message Richard Weinberger
2024-10-12 15:53 ` kernel test robot
@ 2024-10-15 16:48 ` Mathieu Poirier
2024-10-15 16:58 ` Richard Weinberger
1 sibling, 1 reply; 10+ messages in thread
From: Mathieu Poirier @ 2024-10-15 16:48 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-remoteproc, linux-kernel, andersson, upstream+rproc, ohad,
s-anna, t-kristo
Good morning Richard,
On Fri, Oct 11, 2024 at 02:39:22PM +0200, Richard Weinberger wrote:
> Texas Instruments ships a patch in their vendor kernels,
> which adds a new NS message that includes a description field.
> While TI is free to do whatever they want in their copy of the kernel,
> it becomes a mess when people switch to a mainline kernel and want
> to use their existing DSP programs with it.
I suspect there is a lot more things to change when going from downstream to a
mainline kernel.
>
> To make it easier to migrate to a mainline kernel,
> let's make the kernel aware of their non-standard extension but
> briefly ignore the description field.
In my opinion the real fix here is to get TI to use the standard message
announcement structure. The ->desc field doesn't seem to be that useful since
it gets discarted.
Thanks,
Mathieu
>
> [0] https://patchwork.kernel.org/project/linux-remoteproc/patch/20190815231448.10100-1-s-anna@ti.com/
> [1] https://stash.phytec.com/projects/PUB/repos/linux-phytec-ti/commits/aeded1f439effc84aa9f4e341a6e92ce1844ab98#drivers/rpmsg/virtio_rpmsg_bus.c
>
> Cc: ohad@wizery.com
> Cc: s-anna@ti.com
> Cc: t-kristo@ti.com
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> FWIW, this is a forward port of a patch I'm using on v6.6.
>
> Thanks,
> //richard
> ---
> drivers/rpmsg/rpmsg_ns.c | 30 ++++++++++++++++++++++--------
> include/linux/rpmsg/ns.h | 8 ++++++++
> 2 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
> index bde8c8d433e0a..2fb3721eb0141 100644
> --- a/drivers/rpmsg/rpmsg_ns.c
> +++ b/drivers/rpmsg/rpmsg_ns.c
> @@ -31,10 +31,11 @@ EXPORT_SYMBOL(rpmsg_ns_register_device);
> static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
> void *priv, u32 src)
> {
> - struct rpmsg_ns_msg *msg = data;
> struct rpmsg_device *newch;
> struct rpmsg_channel_info chinfo;
> struct device *dev = rpdev->dev.parent;
> + __rpmsg32 ns_addr, ns_flags;
> + char *ns_name;
> int ret;
>
> #if defined(CONFIG_DYNAMIC_DEBUG)
> @@ -42,23 +43,36 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
> data, len, true);
> #endif
>
> - if (len != sizeof(*msg)) {
> + if (len == sizeof(struct rpmsg_ns_msg)) {
> + struct rpmsg_ns_msg *msg = data;
> +
> + ns_addr = msg->addr;
> + ns_flags = msg->flags;
> + ns_name = msg->name;
> + } else if (len == sizeof(struct __rpmsg_ns_msg_ti)) {
> + struct __rpmsg_ns_msg_ti *msg = data;
> +
> + ns_addr = msg->addr;
> + ns_flags = msg->flags;
> + ns_name = msg->name;
> + dev_warn(dev, "non-standard ns msg found\n");
> + } else {
> dev_err(dev, "malformed ns msg (%d)\n", len);
> return -EINVAL;
> }
>
> /* don't trust the remote processor for null terminating the name */
> - msg->name[RPMSG_NAME_SIZE - 1] = '\0';
> + ns_name[RPMSG_NAME_SIZE - 1] = '\0';
>
> - strscpy_pad(chinfo.name, msg->name, sizeof(chinfo.name));
> + strscpy_pad(chinfo.name, ns_name, sizeof(chinfo.name));
> chinfo.src = RPMSG_ADDR_ANY;
> - chinfo.dst = rpmsg32_to_cpu(rpdev, msg->addr);
> + chinfo.dst = rpmsg32_to_cpu(rpdev, ns_addr);
>
> dev_info(dev, "%sing channel %s addr 0x%x\n",
> - rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY ?
> - "destroy" : "creat", msg->name, chinfo.dst);
> + rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY ?
> + "destroy" : "creat", ns_name, chinfo.dst);
>
> - if (rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY) {
> + if (rpmsg32_to_cpu(rpdev, ns_flags) & RPMSG_NS_DESTROY) {
> ret = rpmsg_release_channel(rpdev, &chinfo);
> if (ret)
> dev_err(dev, "rpmsg_destroy_channel failed: %d\n", ret);
> diff --git a/include/linux/rpmsg/ns.h b/include/linux/rpmsg/ns.h
> index a7804edd6d58f..60fca84ad4cea 100644
> --- a/include/linux/rpmsg/ns.h
> +++ b/include/linux/rpmsg/ns.h
> @@ -26,6 +26,14 @@ struct rpmsg_ns_msg {
> __rpmsg32 flags;
> } __packed;
>
> +/* Non-standard extended ns message by Texas Instruments */
> +struct __rpmsg_ns_msg_ti {
> + char name[RPMSG_NAME_SIZE];
> + char desc[RPMSG_NAME_SIZE]; /* ignored */
> + u32 addr;
> + u32 flags;
> +} __packed;
> +
> /**
> * enum rpmsg_ns_flags - dynamic name service announcement flags
> *
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-15 16:48 ` Mathieu Poirier
@ 2024-10-15 16:58 ` Richard Weinberger
2024-10-15 17:56 ` Mathieu Poirier
0 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2024-10-15 16:58 UTC (permalink / raw)
To: Richard Weinberger, upstream
Cc: linux-remoteproc, linux-kernel, andersson, upstream+rproc, ohad,
s-anna, t-kristo, Mathieu Poirier
Mathieu,
Am Dienstag, 15. Oktober 2024, 18:48:08 CEST schrieb Mathieu Poirier:
> Good morning Richard,
>
> On Fri, Oct 11, 2024 at 02:39:22PM +0200, Richard Weinberger wrote:
> > Texas Instruments ships a patch in their vendor kernels,
> > which adds a new NS message that includes a description field.
> > While TI is free to do whatever they want in their copy of the kernel,
> > it becomes a mess when people switch to a mainline kernel and want
> > to use their existing DSP programs with it.
>
> I suspect there is a lot more things to change when going from downstream to a
> mainline kernel.
Not really.
I had to revert c6aed238b7a9b ("remoteproc: modify vring allocation to rely on centralized carveout allocator")
because the DSP has a sub-optimal resource table, and this workaround.
With that the DSP program worked as-is on kernel 6.6.
Downstream was 4.19 TI.
> >
> > To make it easier to migrate to a mainline kernel,
> > let's make the kernel aware of their non-standard extension but
> > briefly ignore the description field.
>
> In my opinion the real fix here is to get TI to use the standard message
> announcement structure. The ->desc field doesn't seem to be that useful since
> it gets discarted.
This is for the future, the goal of my patch is helping people to
get existing DSP programs work with mainline.
Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
kernel.
Thanks,
//richard
--
sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-15 16:58 ` Richard Weinberger
@ 2024-10-15 17:56 ` Mathieu Poirier
2024-10-15 18:00 ` Richard Weinberger
0 siblings, 1 reply; 10+ messages in thread
From: Mathieu Poirier @ 2024-10-15 17:56 UTC (permalink / raw)
To: Richard Weinberger
Cc: Richard Weinberger, upstream, linux-remoteproc, linux-kernel,
andersson, upstream+rproc, ohad, s-anna, t-kristo
On Tue, Oct 15, 2024 at 06:58:33PM +0200, Richard Weinberger wrote:
> Mathieu,
>
> Am Dienstag, 15. Oktober 2024, 18:48:08 CEST schrieb Mathieu Poirier:
> > Good morning Richard,
> >
> > On Fri, Oct 11, 2024 at 02:39:22PM +0200, Richard Weinberger wrote:
> > > Texas Instruments ships a patch in their vendor kernels,
> > > which adds a new NS message that includes a description field.
> > > While TI is free to do whatever they want in their copy of the kernel,
> > > it becomes a mess when people switch to a mainline kernel and want
> > > to use their existing DSP programs with it.
> >
> > I suspect there is a lot more things to change when going from downstream to a
> > mainline kernel.
>
> Not really.
> I had to revert c6aed238b7a9b ("remoteproc: modify vring allocation to rely on centralized carveout allocator")
> because the DSP has a sub-optimal resource table, and this workaround.
> With that the DSP program worked as-is on kernel 6.6.
> Downstream was 4.19 TI.
>
> > >
> > > To make it easier to migrate to a mainline kernel,
> > > let's make the kernel aware of their non-standard extension but
> > > briefly ignore the description field.
> >
> > In my opinion the real fix here is to get TI to use the standard message
> > announcement structure. The ->desc field doesn't seem to be that useful since
> > it gets discarted.
>
> This is for the future, the goal of my patch is helping people to
> get existing DSP programs work with mainline.
> Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
> kernel.
That's an even better argument to adopt the standard structure as soon as
possible. Modifying the mainline kernel to adapt to vendors' quirks doesn't
scale.
>
> Thanks,
> //richard
>
> --
> sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
> UID/VAT Nr: ATU 66964118 | FN: 374287y
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-15 17:56 ` Mathieu Poirier
@ 2024-10-15 18:00 ` Richard Weinberger
2024-10-29 16:15 ` Romain Naour
2024-12-03 15:19 ` Andrew Davis
0 siblings, 2 replies; 10+ messages in thread
From: Richard Weinberger @ 2024-10-15 18:00 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Richard Weinberger, upstream, linux-remoteproc, linux-kernel,
andersson, upstream+rproc, ohad, s-anna, t-kristo
Am Dienstag, 15. Oktober 2024, 19:56:08 CEST schrieb Mathieu Poirier:
> > > In my opinion the real fix here is to get TI to use the standard message
> > > announcement structure. The ->desc field doesn't seem to be that useful since
> > > it gets discarted.
> >
> > This is for the future, the goal of my patch is helping people to
> > get existing DSP programs work with mainline.
> > Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
> > kernel.
>
> That's an even better argument to adopt the standard structure as soon as
> possible. Modifying the mainline kernel to adapt to vendors' quirks doesn't
> scale.
Well, I can't speak for TI.
But I have little hope.
Thanks,
//richard
--
sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-15 18:00 ` Richard Weinberger
@ 2024-10-29 16:15 ` Romain Naour
2024-12-03 15:19 ` Andrew Davis
1 sibling, 0 replies; 10+ messages in thread
From: Romain Naour @ 2024-10-29 16:15 UTC (permalink / raw)
To: Richard Weinberger, Mathieu Poirier
Cc: Richard Weinberger, upstream, linux-remoteproc, linux-kernel,
andersson, upstream+rproc, ohad, s-anna, t-kristo
Hello Richard, All,
Le 15/10/2024 à 20:00, Richard Weinberger a écrit :
> Am Dienstag, 15. Oktober 2024, 19:56:08 CEST schrieb Mathieu Poirier:
>>>> In my opinion the real fix here is to get TI to use the standard message
>>>> announcement structure. The ->desc field doesn't seem to be that useful since
>>>> it gets discarted.
>>>
>>> This is for the future, the goal of my patch is helping people to
>>> get existing DSP programs work with mainline.
>>> Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
>>> kernel.
>>
>> That's an even better argument to adopt the standard structure as soon as
>> possible. Modifying the mainline kernel to adapt to vendors' quirks doesn't
>> scale.
>
> Well, I can't speak for TI.
> But I have little hope.
I'm also using an AM57xx SoC with DSP firmware and I had the same issue while
updating the kernel from 5.10 to a newer version.
remoteproc rpmsg description field changes [1] is required by the DSP firmware
based on TI-RTOS that is loaded by remoteproc using the new (as of 2013) but
still experimental RPMSG_NS_2_0 [2].
RPMSG_NS_2_0 broke compatibility with existing rpmsg structs [3] (defined in
upstream kernel) for all devices except OMAP5 and newer SoC (Newer 64bits SoC
DRA8 and Jacinto doesn’t need this change thanks to the new IPC-lld implementation).
This rpmsg description field has been added to ipcdev long time ago (14/03/2013)
and requires this kernel vendor change since then (all DSP firmware generated by
AM57xx TI SDK need it).
Note:
RPMSG_NS_2_0 is not the only vendor changes you need to rebase on newer kernels...
DSP firmware generated by TI SDK are using the vendor MessageQ API [4] that
requires AF_RPMSG socket support [5] in the kernel.
This driver was never upstreamed since the IPC 3.x is deprecated nowadays and
replaced by the IPC-lld on newer SoC (IPC-lld uses the upstream generic
rpmg-char driver).
All theses patches were moved out of ti-linux-kernel (6.1 since) to a meta-tisdk
yocto layer that is used only for am57xx vendor kernel. See the latest SDK
release for AM57xx [7].
So, DSP firmwares on AM57xx will requires theses two vendor patches since it's
how the TI IPC stack was designed more than 10 years ago.
It's nice to see newer TI SoC able to use upstream kernel using the standard
structure.
[1]
https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-rt-linux-5.10.y&id=7e3ea0d62a4bf0ca04be9bc320d13f564aab0a92
[2]
https://git.ti.com/cgit/ipc/ipcdev/commit/?id=e8a33844cd2faa404e457d13f9d54478ec8129e7
[3]
https://git.ti.com/cgit/ipc/ipcdev/commit/?id=1264ed112ef8c0eed6ff30503b14f81b8ff11dd7
[4]
http://downloads.ti.com/dsps/dsps_public_sw/sdo_sb/targetcontent/ipc/latest/docs/doxygen/html/_message_q_8h.html
[5]
https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-rt-linux-5.10.y&id=f4b978a978c38149f712ddd137f12ed5fb914161
[6]
https://git.ti.com/cgit/ti-sdk-linux/meta-tisdk/commit/?h=am57x-9.x&id=25e56a0615fb8e973e516b5a225ee81f655f98db
[7] https://www.ti.com/tool/download/PROCESSOR-SDK-LINUX-AM57X/09.02.00.133
Best regards,
Romain
>
> Thanks,
> //richard
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-10-15 18:00 ` Richard Weinberger
2024-10-29 16:15 ` Romain Naour
@ 2024-12-03 15:19 ` Andrew Davis
2024-12-03 16:40 ` Richard Weinberger
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Davis @ 2024-12-03 15:19 UTC (permalink / raw)
To: Richard Weinberger, Mathieu Poirier
Cc: Richard Weinberger, upstream, linux-remoteproc, linux-kernel,
andersson, upstream+rproc, ohad, s-anna, t-kristo
On 10/15/24 1:00 PM, Richard Weinberger wrote:
> Am Dienstag, 15. Oktober 2024, 19:56:08 CEST schrieb Mathieu Poirier:
>>>> In my opinion the real fix here is to get TI to use the standard message
>>>> announcement structure. The ->desc field doesn't seem to be that useful since
>>>> it gets discarted.
>>>
>>> This is for the future, the goal of my patch is helping people to
>>> get existing DSP programs work with mainline.
>>> Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
>>> kernel.
>>
>> That's an even better argument to adopt the standard structure as soon as
>> possible. Modifying the mainline kernel to adapt to vendors' quirks doesn't
>> scale.
>
> Well, I can't speak for TI.
> But I have little hope.
>
RPMSG_NS_2_0 is a compile time option, you can turn that off and rebuild
the firmware to go back to the standard message structure. Our new firmware
doesn't use that anyway (and it was only introduced to support some OpenMA
firmware that had multiple channels. It was left on by default in some old
firmware SDKs which is why NS_2.0 is in more firmware than it really needed
to have been). Is there some specific firmware you a working with that cannot
be rebuilt?
Andrew
> Thanks,
> //richard
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rpmsg_ns: Work around TI non-standard message
2024-12-03 15:19 ` Andrew Davis
@ 2024-12-03 16:40 ` Richard Weinberger
0 siblings, 0 replies; 10+ messages in thread
From: Richard Weinberger @ 2024-12-03 16:40 UTC (permalink / raw)
To: Mathieu Poirier, Andrew Davis
Cc: Richard Weinberger, upstream, linux-remoteproc, linux-kernel,
andersson, upstream+rproc, ohad, s-anna, t-kristo
Andrew,
Am Dienstag, 3. Dezember 2024, 16:19:31 CET schrieb Andrew Davis:
> On 10/15/24 1:00 PM, Richard Weinberger wrote:
> > Am Dienstag, 15. Oktober 2024, 19:56:08 CEST schrieb Mathieu Poirier:
> >>>> In my opinion the real fix here is to get TI to use the standard message
> >>>> announcement structure. The ->desc field doesn't seem to be that useful since
> >>>> it gets discarted.
> >>>
> >>> This is for the future, the goal of my patch is helping people to
> >>> get existing DSP programs work with mainline.
> >>> Not everyone can or want to rebuild theirs DSP programs when moving to a mainline
> >>> kernel.
> >>
> >> That's an even better argument to adopt the standard structure as soon as
> >> possible. Modifying the mainline kernel to adapt to vendors' quirks doesn't
> >> scale.
> >
> > Well, I can't speak for TI.
> > But I have little hope.
> >
>
> RPMSG_NS_2_0 is a compile time option, you can turn that off and rebuild
> the firmware to go back to the standard message structure. Our new firmware
> doesn't use that anyway (and it was only introduced to support some OpenMA
> firmware that had multiple channels. It was left on by default in some old
> firmware SDKs which is why NS_2.0 is in more firmware than it really needed
> to have been). Is there some specific firmware you a working with that cannot
> be rebuilt?
Cool, that's valuable information!
In the meanwhile I got contact to the right people to be able to rebuild the
firmware. The overall software stack is complicated and I'm just the Linux guy...
So, with some luck, the DSP application can work on mainline with
almost no kernel patches. :-)
Thanks,
//richard
--
sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-12-03 16:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-11 12:39 [PATCH] rpmsg_ns: Work around TI non-standard message Richard Weinberger
2024-10-12 15:53 ` kernel test robot
2024-10-14 9:24 ` Richard Weinberger
2024-10-15 16:48 ` Mathieu Poirier
2024-10-15 16:58 ` Richard Weinberger
2024-10-15 17:56 ` Mathieu Poirier
2024-10-15 18:00 ` Richard Weinberger
2024-10-29 16:15 ` Romain Naour
2024-12-03 15:19 ` Andrew Davis
2024-12-03 16:40 ` Richard Weinberger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox