Re: [PATCH v2 3/3] net: phy: add Rust Asix PHY driver

Andrew Lunn
2023-10-06T16:55:27Z

> 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.

> 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_EXCLUDE
> depends 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?

> 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