On 4/7/22 11:01 AM, Bruce Ashfield wrote:
On Wed, Apr 6, 2022 at 8:39 AM Paul Gortmaker
<[email protected] <mailto:[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] <mailto:[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
<https://urldefense.com/v3/__http://git.ti.com/ti-linux-kernel/ti-linux-kernel.git__;!!AjveYdw8EvQ!LvsC-Qu2mVWU7kV0C4MkKy8XdXh5u5y1TS_7ojUtk3IZssLEVnH5Rv6B9BOgwJP87Q$>
-----------------
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.
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.
Thanks
Xulin
Bruce
Thanks,
Paul.
--
`
>
> Signed-off-by: Xulin Sun <[email protected]
<mailto:[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 (#11133):
https://lists.yoctoproject.org/g/linux-yocto/message/11133
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]]
-=-=-=-=-=-=-=-=-=-=-=-
