Thread (60 messages) 60 messages, 13 authors, 2023-02-10

Re: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object

From: Alexander Lobakin <hidden>
Date: 2022-11-23 21:48:28
Also in: linux-kbuild, lkml

From: Colin Foster <colin.foster@in-advantage.com>,
Date: Mon, 21 Nov 2022 10:12:59 -0800
On Mon, Nov 21, 2022 at 07:55:04PM +0200, Vladimir Oltean wrote:
quoted
On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
quoted
With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
(or vice versa), felix.o are linked to a module and also to vmlinux
even though the expected CFLAGS are different between builtins and
modules.
This is the same situation as fixed by
commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
There's also no need to duplicate relatively big piece of object
code into two modules.

Introduce the new module, mscc_core, to provide the common functions
to both mscc_felix and mscc_seville.

Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Alexander Lobakin <redacted>
---
I don't disagree with the patch, but I dislike the name chosen.
How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
it would be good to use that word here, since the code you're moving is
no more than a thin glue layer with some DSA specific code.
Sure. I usually barely work with the code touched by the series, so
often the names can make no sense -- I'm open for better namings
from the real developers.
_dsa_lib sounds good, I like it.
quoted
Adding Colin for a second opinion on the naming. I'm sure things could
have been done better in the first place, just not sure how.
Good catch on this patch. "mscc_ocelot_dsa_lib" makes sense. The only
other option that might be considered would be along the lines of
"felix_lib". While I know "Felix" is the chip, in the dsa directory it
seems to represent the DSA lib in general.
The thing confused me is that one chip is named Felix and the other
one is Seville, but the shared code is named felix as well. So at
first I thought maybe Felix is a family of chips and Seville is a
chip from that family, dunno :D
Either one seems fine for me. And thanks for the heads up, as I'll need
to make the same changes for ocelot_ext when it is ready.
Something interesting is coming, nice <.<

(re "pls prefix with "net: dsa: ..."" -- roger that)

Thanks,
Olek
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help