Re: [PATCH v2 1/3] rust: core abstractions for network PHY drivers

FUJITA Tomonori
2023-10-07T22:33:50Z

On Sat, 07 Oct 2023 19:58:57 +0900 (JST)
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:

>>> +/// # Examples
>>> +///
>>> +/// ```ignore
>>> +///
>>> +/// use kernel::net::phy::{self, DeviceId, Driver};
>>> +/// use kernel::prelude::*;
>>> +///
>>> +/// kernel::module_phy_driver! {
>>> +/// drivers: [PhyAX88772A, PhyAX88772C, PhyAX88796B],
>>> +/// device_table: [
>>> +/// DeviceId::new_with_driver::<PhyAX88772A>(),
>>> +/// DeviceId::new_with_driver::<PhyAX88772C>(),
>>> +/// DeviceId::new_with_driver::<PhyAX88796B>()
>>> +/// ],
>>> +/// type: RustAsixPhy,
>>> +/// name: "rust_asix_phy",
>>> +/// author: "Rust for Linux Contributors",
>>> +/// description: "Rust Asix PHYs driver",
>>> +/// license: "GPL",
>>> +/// }
>>> +/// ```
>>
>> I can't find the discussion we had about this, but you said you have
>> the `type` parameter to be consistent with `module!`, correct?
>
> No, `driver!` in rust branch, which is used by platform, amba, etc.
>
> https://github.com/Rust-for-Linux/linux/blob/rust/samples/rust/rust_platform.rs
>
>> I think that it is more important to be consistent with C's
>> `MODULE_PHY_DRIVER` where you don't need to specify anything extra,
>> since the module doesn't do anything else. And I think it is less
>> confusing for users if they don't wonder why they need to define a
>> type they never use.
>>
>> Why not just remove the field and create an internal type based on
>> `name` for now? We can always make it an optional field later on if it
>> turns out there is a use case.
>
> Sure, I'll try. I have no preference and driver! macro isn't in
> upstream.

To create an internal type based on `name`, we need to unstringify
`name`? I can't find a easy way to do it.