Re: [PATCH v2 3/3] net: phy: add Rust Asix PHY driver
From: Andrew Lunn <andrew@lunn.ch>
Date: 2023-10-06 16:55:27
Also in:
rust-for-linux
How about adding CONFIG_RUST_PHYLIB to the first patch. Not selectable, it's just a flag for Rust PHYLIB support.
We have to be careful with names. To some extent, CONFIG_PHYLIB means the core of phylib. So it could be that CONFIG_RUST_PHYLIB means the core of phylib written in rust? I doubt that will ever happen, but we are setting a naming scheme here which i expect others will blindly cut/paste. What we actually want is a symbol which represents the Rust binding onto the phylib core. So i think it should have BINDING, or WRAPPER or something like that in the name.
quoted hunk ↗ jump to hunk
diff --git a/init/Kconfig b/init/Kconfig index 4b4e3df1658d..2b6627aeb98c 100644 --- a/init/Kconfig +++ b/init/Kconfig@@ -1889,7 +1889,7 @@ config RUST depends on !GCC_PLUGINS depends on !RANDSTRUCT depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDEdepends on PHYLIB=y + select RUST_PHYLIB
I know the rust build system is rather limited at the moment, but is this required? Is it possible to build the rust code without the phylib binding? Can your `RUST_PHYLIB` add phylib.rs to a Makefile target only if it is enabled?
quoted hunk ↗ jump to hunk
select CONSTRUCTORS help Enables Rust support in the kernel.@@ -1904,6 +1904,10 @@ config RUST If unsure, say N. +config RUST_PHYLIB + bool
This is where the depends on PHYLIB should be. It is the Rust binding
on phylib which has the dependency on phylib, not the core rust code.
What i think the end state should be, once the Rust build system is
better is that in drivers/net/phy/Kconfig we have:
if PHYLIB
config RUST_PHYLIB_BINDING
bool
depends on RUST
help
Adds support needed for PHY drivers written in Rust. It provides
a wrapper around the C phlib core.
and the Makefile when uses this to build the binding as a kernel
module.
Andrew