Re: [PATCH net-next v3 2/2] net: phy: add Rust reference driver for ET1011C

[Принтер Принтеров]

Re: [PATCH net-next v3 0/2] net: phy: add Rust reference driver for ET1011C
  On Mon, Feb 24, 2026 at ... Miguel Ojeda <ojeda@xxxxxxxxxx> wrote:
  > Since there is already a PHY one, is this meant to bootstrap the two
  > new APIs? Or something else?

Thank you for the feedback, Miguel. You raise a fair point.

  Honestly, the new abstraction here (speed() getter) is quite minor --
  it just reads a struct field. The driver itself doesn't bootstrap
  anything significant beyond what ax88796b_rust already covers.

  I'm looking to contribute to Rust in the kernel and started with PHY
  drivers because the abstraction is well established. But I understand
  now that duplicating C drivers isn't the goal.

  Could you point me toward areas where Rust work would be most
  valuable? I'm happy to work on new abstractions or drivers that
  actually need bootstrapping, rather than adding another reference
  driver to an area that's already covered.

  Thanks,
  Artem


вт, 24 февр. 2026 г. в 19:49, Artem Lytkin <iprintercanon@xxxxxxxxx>:
>
> Add a Rust reference driver for the LSI ET1011C PHY, following the
> pattern established by the existing Rust Asix PHY driver
> (ax88796b_rust.rs).
>
> Unlike the C driver which uses a custom config_aneg that manually
> clears BMCR bits and writes BMCR_RESET without polling, this driver
> uses the soft_reset callback with genphy_soft_reset(). This properly
> polls the self-clearing BMCR_RESET bit and handles both autoneg and
> forced mode, following current best practices.
>
> The read_status callback detects speed changes and reconfigures the
> GMII interface and TX FIFO when switching to gigabit, matching the C
> driver's functionality.
>
> Signed-off-by: Artem Lytkin <iprintercanon@xxxxxxxxx>
> ---
>  drivers/net/phy/Kconfig         |  9 +++++
>  drivers/net/phy/Makefile        |  6 ++-
>  drivers/net/phy/et1011c_rust.rs | 69 +++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/phy/et1011c_rust.rs
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 7b73332a13d9..41abf13662e6 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -254,6 +254,15 @@ config LSI_ET1011C_PHY
>         help
>           Supports the LSI ET1011C PHY.
>
> +config ET1011C_RUST_PHY
> +       bool "Rust reference driver for LSI ET1011C PHY"
> +       depends on RUST_PHYLIB_ABSTRACTIONS && LSI_ET1011C_PHY
> +       help
> +         Uses the Rust reference driver for LSI ET1011C PHY
> +         (et1011c_rust.ko). The features are equivalent.
> +         It supports the LSI ET1011C PHY. If unsure,
> +         say N.
> +
>  config MARVELL_PHY
>         tristate "Marvell Alaska PHYs"
>         help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 3a34917adea7..491469457a67 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -62,7 +62,11 @@ obj-$(CONFIG_DP83TG720_PHY)  += dp83tg720.o
>  obj-$(CONFIG_FIXED_PHY)                += fixed_phy.o
>  obj-$(CONFIG_ICPLUS_PHY)       += icplus.o
>  obj-$(CONFIG_INTEL_XWAY_PHY)   += intel-xway.o
> -obj-$(CONFIG_LSI_ET1011C_PHY)  += et1011c.o
> +ifdef CONFIG_ET1011C_RUST_PHY
> +  obj-$(CONFIG_LSI_ET1011C_PHY)        += et1011c_rust.o
> +else
> +  obj-$(CONFIG_LSI_ET1011C_PHY)        += et1011c.o
> +endif
>  obj-$(CONFIG_LXT_PHY)          += lxt.o
>  obj-$(CONFIG_MARVELL_10G_PHY)  += marvell10g.o
>  obj-$(CONFIG_MARVELL_PHY)      += marvell.o
> diff --git a/drivers/net/phy/et1011c_rust.rs b/drivers/net/phy/et1011c_rust.rs
> new file mode 100644
> index 000000000000..93dbf9586da3
> --- /dev/null
> +++ b/drivers/net/phy/et1011c_rust.rs
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright (C) 2026 Artem Lytkin <iprintercanon@xxxxxxxxx>
> +
> +//! Rust LSI ET1011C PHY driver
> +//!
> +//! C version of this driver: [`drivers/net/phy/et1011c.c`](./et1011c.c)
> +
> +use kernel::{
> +    net::phy::{self, reg::C22, DeviceId, Driver},
> +    prelude::*,
> +};
> +
> +kernel::module_phy_driver! {
> +    drivers: [PhyET1011C],
> +    device_table: [
> +        DeviceId::new_with_driver::<PhyET1011C>()
> +    ],
> +    name: "rust_et1011c_phy",
> +    authors: ["Artem Lytkin <iprintercanon@xxxxxxxxx>"],
> +    description: "Rust LSI ET1011C PHY driver",
> +    license: "GPL",
> +}
> +
> +// Vendor-specific registers
> +const ET1011C_STATUS_REG: C22 = C22::vendor_specific::<0x1A>();
> +const ET1011C_CONFIG_REG: C22 = C22::vendor_specific::<0x16>();
> +
> +// ET1011C status register fields
> +const ET1011C_SPEED_MASK: u16 = 0x0300;
> +const ET1011C_GIGABIT_SPEED: u16 = 0x0200;
> +
> +// ET1011C config register fields
> +const ET1011C_TX_FIFO_MASK: u16 = 0x3000;
> +const ET1011C_TX_FIFO_DEPTH_16: u16 = 0x1000;
> +const ET1011C_GMII_INTERFACE: u16 = 0x0002;
> +const ET1011C_SYS_CLK_EN: u16 = 0x0010;
> +
> +struct PhyET1011C;
> +
> +#[vtable]
> +impl Driver for PhyET1011C {
> +    const NAME: &'static CStr = c"ET1011C";
> +    const PHY_DEVICE_ID: DeviceId = DeviceId::new_with_model_mask(0x0282f014);
> +
> +    fn soft_reset(dev: &mut phy::Device) -> Result {
> +        dev.genphy_soft_reset()
> +    }
> +
> +    fn read_status(dev: &mut phy::Device) -> Result<u16> {
> +        let old_speed = dev.speed();
> +        dev.genphy_read_status::<C22>()?;
> +
> +        if old_speed != dev.speed() {
> +            let val = dev.read(ET1011C_STATUS_REG)?;
> +            if (val & ET1011C_SPEED_MASK) == ET1011C_GIGABIT_SPEED {
> +                let cfg = dev.read(ET1011C_CONFIG_REG)?;
> +                let cfg = cfg & !ET1011C_TX_FIFO_MASK;
> +                dev.write(
> +                    ET1011C_CONFIG_REG,
> +                    cfg | ET1011C_GMII_INTERFACE
> +                        | ET1011C_SYS_CLK_EN
> +                        | ET1011C_TX_FIFO_DEPTH_16,
> +                )?;
> +            }
> +        }
> +
> +        Ok(0)
> +    }
> +}
> --
> 2.43.0
>