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 :))
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 (#11130):
https://lists.yoctoproject.org/g/linux-yocto/message/11130
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]]
-=-=-=-=-=-=-=-=-=-=-=-
