In message: Re: [linux-yocto][linux-yocto v5.15/standard/ti-sdk-5.10/ti-j72xx][PATCH] net: update the AF_MAX value to the latest on 07/04/2022 Xulin Sun wrote:
> On 4/7/22 11:01 AM, Bruce Ashfield wrote: > > > > On Wed, Apr 6, 2022 at 8:39 AM Paul Gortmaker > <[email protected] > > wrote: > > [[linux-yocto][linux-yocto v5.15/standard/ti-sdk-5.10/ti-j72xx][PATCH] > net: update the AF_MAX value to the latest] On 06/04/2022 (Wed 12:52) > Xulin Sun wrote: > > > commit c6f441a33dd (net/rpmsg: add support for new rpmsg sockets) > adds > > a new socket address AF_RPMSG, the AF_MAX should be incremented > accordingly. > > > > To avoid below call trace: > > net/core/sock.c:233:21: error: excess elements in array initializer > [-Werror] > > | 233 | _sock_locks("sk_lock-") > > This is not a call trace. A call trace is what comes from a runtime > BUG() or WARN() etc etc. This is a compile error. > > Let us look at c6f441a33dd - which it would be helpful in the future > if > you indicate when a commit ID is yocto specific; otherwise the > convention is to assume a "reference free" ID is a mainline ID/SHA. > > ----------------- > commit c6f441a33dde0e4fad634be1aa4fc145e893880c > Author: Suman Anna <[email protected]> > Date: Tue Apr 13 16:20:19 2021 -0500 > > net/rpmsg: add support for new rpmsg sockets > > commit f4b978a978c38149f712ddd137f12ed5fb914161 from > git://git.ti.com/ti-linux-kernel/ti-linux-kernel.git > ----------------- > > This is a classic example of what should NOT EVER be in a BSP branch. > There is NOTHING hardware specific about this AT ALL. By having these > kinds of random feature and random non-hardware changes in an SDK you > then open the door for these kinds of annoying whack-a-mole compile > issues and other regressions. > > Now, I realize you didn't put it there, and may not have even worked > on > the SDK integration into v5.15 kernel, but someone has to push back on > on the originators of the SDK content and act as a 1st pass filter for > removing obvious unrelated pet projects that are tagging along before > they make to to Yocto. > > If your BSP strays outside of arch/ and drivers/ then you REALLY need > to > look hard at those changes. If a BSP touches fs/ or net/ or lib/ or > any > other non-hardware dirs, it is probably inappropriate and the > change(s) > should be dropped. > > If you don't, you make things a future merge-hell for Bruce for the > next > two years that we maintain v5.15 kernels because we have SDK BSPs that > contain a truck load of random unrelated things. > > Alternatively, if in this case, this new rpmsg *feature* is super > important to the overall userspace use case you have in mind for this > hardware, then submit it as a proper kernel feature/addition, > independent of the BSP, and then have the BSP include it. > > Bruce, given the above, I suggest we revert c6f441a33dd from the > branch > v5.15/standard/ti-sdk-5.10/ti-j72xx and let the original BSP submitter > revisit the question of whether it was needed or just simply shoved > forward because "it was there already". > > > Thanks for the detailed analysis Paul! > > I agree, and I also need to be a bit more precise when looking at some of > the initial BSP merges. When a BSP strays outside of the directories you > mention, it really should raise red flags and ring alarm bells for > everyone. > > I'm inclined to do the revert, and take at least a small step to bringing > the BSP back into something more easily maintained .. but I'd also like to > hear from Xuilin about what the original feature in the BSP was trying to > do, and if anything important will break with a revert (and by "break", I > mean some sort of demo, or h/w specific functionality .. since hopefully > with the revert, it compiles :)) > > Hi Bruce & Paul, > > This original patch mainly created a standalone directory "net/rpmsg" and it > had minor modifications to public files "net/core/sock.c" & "include/linux/ > socket.h" to add the newly address type AF_RPMSG. > > At the same time there was a newly added kernel config option > "CONFIG_RPMSG_PROTO" to include this feature or not. > > The feature implemented an IPC service using the remoteproc/rpmsg framework > for > TI's series SOC. > Usually TI's SOC contains a variety of DSP and R5 cores on it. The IPC is used > between threads running > on different asymmetric processors. The IPC service is provided to user > threads > via socket APIs. If this patch is removed, it will interrupt this IPC > communication > service between master cores (normally running Linux) and slave cores > (normally > running RTOS) for TI's series SOC. Ah yes, and rpmsg is still at the point where there are SOC specific requirements (that is changing upstream, but we aren't there yet). > > The patch existed in TI's SDK kernel v5.4 & v5.10, and it also existed in > previous yocto kernel version v5.4 & v5.10. > > I took it to kernel v5.15, in case of interrupting user-dependent application. Given the SOC specific needs, and this, I'll grab the patch and merge it shortly. Bruce > > Thanks > Xulin > > > Bruce > > > > > Thanks, > Paul. > -- > > ` > > > > Signed-off-by: Xulin Sun <[email protected]> > > --- > > include/linux/socket.h | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/socket.h b/include/linux/socket.h > > index 6ec661deb432..ef57dbba6c5f 100644 > > --- a/include/linux/socket.h > > +++ b/include/linux/socket.h > > @@ -223,12 +223,11 @@ struct ucred { > > * reuses AF_INET address family > > */ > > #define AF_XDP 44 /* XDP sockets > > */ > > -#define AF_RPMSG 45 /* Remote-processor messaging */ > > -#define AF_MAX 46 /* For now.. */ > > - > > -#define AF_MCTP 47 /* Management component > > +#define AF_MCTP 45 /* Management component > > * transport protocol > > */ > > +#define AF_RPMSG 46 /* Remote-processor messaging */ > > +#define AF_MAX 47 /* For now.. */ > > > > /* Protocol families, same as address families. */ > > #define PF_UNSPEC AF_UNSPEC > > -- > > 2.35.1 > > > > > > > > > > > > > > -- > - Thou shalt not follow the NULL pointer, for chaos and madness await thee > at its end > - "Use the force Harry" - Gandalf, Star Trek II > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#11138): https://lists.yoctoproject.org/g/linux-yocto/message/11138 Mute This Topic: https://lists.yoctoproject.org/mt/90283473/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
