[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALNs47svLHD3=LFrhi=zf4hdr--e6hGjTzT5X_U9yd8q5r7G7g@mail.gmail.com>
Date: Thu, 25 Jan 2024 19:03:58 -0600
From: Trevor Gross <tmgross@...ch.edu>
To: Andrew Lunn <andrew@...n.ch>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH net-next] rust: phy: use VTABLE_DEFAULT_ERROR
On Thu, Jan 25, 2024 at 11:42 AM Andrew Lunn <andrew@...n.ch> wrote:
> > @@ -580,12 +580,12 @@ pub trait Driver {
> >
> > /// Issues a PHY software reset.
> > fn soft_reset(_dev: &mut Device) -> Result {
> > - Err(code::ENOTSUPP)
> > + kernel::build_error(VTABLE_DEFAULT_ERROR)
> > }
>
> Dumb question, which i should probably duckduckgo myself.
>
> Why is it called VTABLE_DEFAULT_ERROR and not
> VTABLE_NOT_SUPPORTED_ERROR?
>
> Looking through the C code my guess would be -EINVAL would be the
> default, or maybe 0.
>
> The semantics of ENOTSUPP can also vary. Its often not an actual
> error, it just a means to indicate its not supported, and the caller
> might decide to do something different as a result. One typical use in
> the network stack is offloading functionality to hardware.
> e.g. blinking the LEDs on the RJ45 connected. The driver could be
> asked to blink to show activity of a received packet. Some hardware
> can only indicate activity for both receive and transmit, so the
> driver would return -EOPNOTSUPP. Software blinking would then be used
> instead of hardware blinking.
>
> Andrew
`build_error` is a bit special and is implemented as follows:
1. If called in a `const fn`, it will fail the build
2. If the build_error symbol is in a final binary, tooling should
detect this and raise some kind of error (config
RUST_BUILD_ASSERT_ALLOW, not exactly positive how this is implemented)
3. If called at runtime, something is very unexpected so we panic
To make a trait function optional in rust, you need to provide a
default (since all functions in a trait must always be callable). But
if the C side can handle a null function pointer, it's better to just
set that instead of reimplementing the default from Rust.
So it's up to the abstraction to set these fields to NULL/None if not
implemented, e.g. from create_phy_driver in phy.rs:
soft_reset: if T::HAS_SOFT_RESET {
Some(Adapter::<T>::soft_reset_callback)
} else {
None
},
If an abstraction does this wrong and tries to assign a default
function when it would be better to get a null pointer, then we get
the build error. So this is something that would pop up if the
abstraction is done wrong, not just if somebody writes a bad phy
driver.
VTABLE_DEFAULT_ERROR isn't a kernel errno, it's a string printed to be
printed by build_error or the tooling.
- Trevor
Powered by blists - more mailing lists