netdev - Re: [PATCH net-next v3 3/3] net: phy: add Rust Asix PHY driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 9 Oct 2023 15:15:29 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
	rust-for-linux@...r.kernel.org, miguel.ojeda.sandonis@...il.com,
	greg@...ah.com, tmgross@...ch.edu
Subject: Re: [PATCH net-next v3 3/3] net: phy: add Rust Asix PHY driver

> > +        if ret as u32 & uapi::BMCR_SPEED100 != 0 {
> > +            dev.set_speed(uapi::SPEED_100);
> > +        } else {
> > +            dev.set_speed(uapi::SPEED_10);
> > +        }
> 
> Maybe refactor to only have one `dev.set_speed` call?

This is a common pattern in the C code. This is basically a
re-implementation of

https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy_device.c#L2432

because this PHY is broken. Being one of the maintainers of the PHY
subsystem, it helps me review this code if it happens to look like the
existing code it is adding a workaround to.

Is there a Rust reason to only have one call?

   Andrew