[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20231205.124407.1028275511920165940.fujita.tomonori@gmail.com>
Date: Tue, 05 Dec 2023 12:44:07 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: jarkko@...nel.org
Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org,
rust-for-linux@...r.kernel.org, andrew@...n.ch, tmgross@...ch.edu,
miguel.ojeda.sandonis@...il.com, benno.lossin@...ton.me,
wedsonaf@...il.com, aliceryhl@...gle.com, boqun.feng@...il.com
Subject: Re: [PATCH net-next v9 2/4] rust: net::phy add module_phy_driver
macro
On Tue, 05 Dec 2023 03:48:32 +0200
"Jarkko Sakkinen" <jarkko@...nel.org> wrote:
> On Tue Dec 5, 2023 at 3:14 AM EET, FUJITA Tomonori wrote:
>> This macro creates an array of kernel's `struct phy_driver` and
>> registers it. This also corresponds to the kernel's
>> `MODULE_DEVICE_TABLE` macro, which embeds the information for module
>> loading into the module binary file.
>>
>> A PHY driver should use this macro.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
>> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
>> ---
>> rust/kernel/net/phy.rs | 146 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 146 insertions(+)
>>
>> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
>> index 5d220187eec9..d9cec139324a 100644
>> --- a/rust/kernel/net/phy.rs
>> +++ b/rust/kernel/net/phy.rs
>> @@ -752,3 +752,149 @@ const fn as_int(&self) -> u32 {
>> }
>> }
>> }
>> +
>> +/// Declares a kernel module for PHYs drivers.
>> +///
>> +/// This creates a static array of kernel's `struct phy_driver` and registers it.
>
> s/This creates a static array/Creates a static array/
>
> Suggestion for better formulation:
>
> "Creates a static array of `struct phy driver` instances, and registers them.""
I follow Rust comment style here. Maybe I should do:
s/This creates/This macro creates/
>> +/// This also corresponds to the kernel's `MODULE_DEVICE_TABLE` macro, which embeds the information
>> +/// for module loading into the module binary file. Every driver needs an entry in `device_table`.
>
> s/This/`kernel::module_phy_driver`/
>
> Or at least I did not see it introduced earlier in the text.
s/This/This macro/ works for you?
Likely `kernel::` part would be changed in the future.
Powered by blists - more mailing lists