* [PATCH] Various improvements on clock abstractions
@ 2025-06-16 20:01 onur-ozkan
2025-06-16 20:40 ` Miguel Ojeda
2025-06-17 7:52 ` Alexandre Courbot
0 siblings, 2 replies; 7+ messages in thread
From: onur-ozkan @ 2025-06-16 20:01 UTC (permalink / raw)
To: linux-clk, rust-for-linux, linux-kernel
Cc: mturquette, sboyd, ojeda, alex.gaynor, boqun.feng, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr,
onur-ozkan
A few changes to improve the clock abstractions and make them a little
more idiomatic:
1. `impl Hertz` functions are now constant and compile-time evaluable.
2. `Hertz` conversions are now done with constant variables, which should
make them more readable.
3. `con_id` is handled in a single line using `map_or` instead of using
nested if-else blocks.
Signed-off-by: onur-ozkan <work@onurozkan.dev>
---
rust/kernel/clk.rs | 42 +++++++++++++++++++-----------------------
1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 6041c6d07527..fbcea31dbcca 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -30,39 +30,43 @@
pub struct Hertz(pub c_ulong);
impl Hertz {
+ const KHZ_TO_HZ: c_ulong = 1_000;
+ const MHZ_TO_HZ: c_ulong = 1_000_000;
+ const GHZ_TO_HZ: c_ulong = 1_000_000_000;
+
/// Create a new instance from kilohertz (kHz)
- pub fn from_khz(khz: c_ulong) -> Self {
- Self(khz * 1_000)
+ pub const fn from_khz(khz: c_ulong) -> Self {
+ Self(khz * Self::KHZ_TO_HZ)
}
/// Create a new instance from megahertz (MHz)
- pub fn from_mhz(mhz: c_ulong) -> Self {
- Self(mhz * 1_000_000)
+ pub const fn from_mhz(mhz: c_ulong) -> Self {
+ Self(mhz * Self::MHZ_TO_HZ)
}
/// Create a new instance from gigahertz (GHz)
- pub fn from_ghz(ghz: c_ulong) -> Self {
- Self(ghz * 1_000_000_000)
+ pub const fn from_ghz(ghz: c_ulong) -> Self {
+ Self(ghz * Self::GHZ_TO_HZ)
}
/// Get the frequency in hertz
- pub fn as_hz(&self) -> c_ulong {
+ pub const fn as_hz(&self) -> c_ulong {
self.0
}
/// Get the frequency in kilohertz
- pub fn as_khz(&self) -> c_ulong {
- self.0 / 1_000
+ pub const fn as_khz(&self) -> c_ulong {
+ self.0 / Self::KHZ_TO_HZ
}
/// Get the frequency in megahertz
- pub fn as_mhz(&self) -> c_ulong {
- self.0 / 1_000_000
+ pub const fn as_mhz(&self) -> c_ulong {
+ self.0 / Self::MHZ_TO_HZ
}
/// Get the frequency in gigahertz
- pub fn as_ghz(&self) -> c_ulong {
- self.0 / 1_000_000_000
+ pub const fn as_ghz(&self) -> c_ulong {
+ self.0 / Self::GHZ_TO_HZ
}
}
@@ -132,11 +136,7 @@ impl Clk {
///
/// [`clk_get`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_get
pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
- let con_id = if let Some(name) = name {
- name.as_ptr()
- } else {
- ptr::null()
- };
+ let con_id = name.map_or(ptr::null(), |n| n.as_ptr());
// SAFETY: It is safe to call [`clk_get`] for a valid device pointer.
//
@@ -304,11 +304,7 @@ impl OptionalClk {
/// [`clk_get_optional`]:
/// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
- let con_id = if let Some(name) = name {
- name.as_ptr()
- } else {
- ptr::null()
- };
+ let con_id = name.map_or(ptr::null(), |n| n.as_ptr());
// SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
//
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] Various improvements on clock abstractions 2025-06-16 20:01 [PATCH] Various improvements on clock abstractions onur-ozkan @ 2025-06-16 20:40 ` Miguel Ojeda [not found] ` <42151750134012@mail.yandex.com> 2025-06-17 7:52 ` Alexandre Courbot 1 sibling, 1 reply; 7+ messages in thread From: Miguel Ojeda @ 2025-06-16 20:40 UTC (permalink / raw) To: onur-ozkan, Viresh Kumar Cc: linux-clk, rust-for-linux, linux-kernel, mturquette, sboyd, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr On Mon, Jun 16, 2025 at 10:01 PM onur-ozkan <work@onurozkan.dev> wrote: > > Signed-off-by: onur-ozkan <work@onurozkan.dev> Should this be "Onur Özkan"? Apologies if not -- I am just asking because the SoB typically uses the full name as usually written, and I saw it in Zulip spelled differently. Cc'ing Viresh, since he introduced the file. On the changes themselves, the `const fn` one seems obvious. The constants one is OK, though we could perhaps have a `units.h` equivalent eventually. The `map_or` is up to style, though I am not sure why you say "nested if-else blocks" in the commit message, i.e. what is nested? (By the way, this could (or not) have been split into different patches -- it is up to the maintainers/reviewers, and it depends on how complex a given patch is and other factors, but I thought I would mention it just in case you thought it needed to be one.) Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <42151750134012@mail.yandex.com>]
* Re: [PATCH] Various improvements on clock abstractions [not found] ` <42151750134012@mail.yandex.com> @ 2025-06-17 6:55 ` Miguel Ojeda 2025-06-19 6:45 ` Viresh Kumar 0 siblings, 1 reply; 7+ messages in thread From: Miguel Ojeda @ 2025-06-17 6:55 UTC (permalink / raw) To: Onur Özkan Cc: Viresh Kumar, linux-clk@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, mturquette@baylibre.com, sboyd@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, dakr@kernel.org On Tue, Jun 17, 2025 at 6:28 AM Onur Özkan <work@onurozkan.dev> wrote: > > Yes, it should be "Onur Özkan", sorry. Should I update that part and re-send the patch? I would suggest to wait for other feedback, and then you can send a v2 if needed. > where my patch converts this into a single straight line which I think makes it more idiomatic. Up to the maintainers :) So far we have both styles around. Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Various improvements on clock abstractions 2025-06-17 6:55 ` Miguel Ojeda @ 2025-06-19 6:45 ` Viresh Kumar 2025-06-19 10:05 ` Onur 0 siblings, 1 reply; 7+ messages in thread From: Viresh Kumar @ 2025-06-19 6:45 UTC (permalink / raw) To: Miguel Ojeda Cc: Onur Özkan, linux-clk@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, mturquette@baylibre.com, sboyd@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, dakr@kernel.org On 17-06-25, 08:55, Miguel Ojeda wrote: > On Tue, Jun 17, 2025 at 6:28 AM Onur Özkan <work@onurozkan.dev> wrote: > > > > Yes, it should be "Onur Özkan", sorry. Should I update that part and re-send the patch? > > I would suggest to wait for other feedback, and then you can send a v2 > if needed. > > > where my patch converts this into a single straight line which I think makes it more idiomatic. > > Up to the maintainers :) So far we have both styles around. I am okay with all the changes, the commit log can be improved as you mentioned earlier. Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Various improvements on clock abstractions 2025-06-19 6:45 ` Viresh Kumar @ 2025-06-19 10:05 ` Onur 2025-06-19 13:48 ` Alexandre Courbot 0 siblings, 1 reply; 7+ messages in thread From: Onur @ 2025-06-19 10:05 UTC (permalink / raw) To: Viresh Kumar Cc: Miguel Ojeda, linux-clk@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, mturquette@baylibre.com, sboyd@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, dakr@kernel.org On Thu, 19 Jun 2025 12:15:34 +0530 Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 17-06-25, 08:55, Miguel Ojeda wrote: > > On Tue, Jun 17, 2025 at 6:28 AM Onur Özkan <work@onurozkan.dev> > > wrote: > > > > > > Yes, it should be "Onur Özkan", sorry. Should I update that part > > > and re-send the patch? > > > > I would suggest to wait for other feedback, and then you can send a > > v2 if needed. > > > > > where my patch converts this into a single straight line which I > > > think makes it more idiomatic. > > > > Up to the maintainers :) So far we have both styles around. > > I am okay with all the changes, the commit log can be improved as you > mentioned earlier. > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > FWIW I split this patch into 3 parts and sent them separately as it was suggested earlier by Alexandre. Here are the patches I sent yesterday: - https://lore.kernel.org/all/20250618092810.29370-1-work@onurozkan.dev/ - https://lore.kernel.org/all/20250618093508.16343-1-work@onurozkan.dev/ - https://lore.kernel.org/all/20250618091442.29104-1-work@onurozkan.dev/ Regards, Onur ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Various improvements on clock abstractions 2025-06-19 10:05 ` Onur @ 2025-06-19 13:48 ` Alexandre Courbot 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Courbot @ 2025-06-19 13:48 UTC (permalink / raw) To: Onur, Viresh Kumar Cc: Miguel Ojeda, linux-clk@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, mturquette@baylibre.com, sboyd@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, dakr@kernel.org On Thu Jun 19, 2025 at 7:05 PM JST, Onur wrote: > On Thu, 19 Jun 2025 12:15:34 +0530 > Viresh Kumar <viresh.kumar@linaro.org> wrote: > >> On 17-06-25, 08:55, Miguel Ojeda wrote: >> > On Tue, Jun 17, 2025 at 6:28 AM Onur Özkan <work@onurozkan.dev> >> > wrote: >> > > >> > > Yes, it should be "Onur Özkan", sorry. Should I update that part >> > > and re-send the patch? >> > >> > I would suggest to wait for other feedback, and then you can send a >> > v2 if needed. >> > >> > > where my patch converts this into a single straight line which I >> > > think makes it more idiomatic. >> > >> > Up to the maintainers :) So far we have both styles around. >> >> I am okay with all the changes, the commit log can be improved as you >> mentioned earlier. >> >> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> >> > > FWIW I split this patch into 3 parts and sent them separately as it > was suggested earlier by Alexandre. > > Here are the patches I sent yesterday: > > - https://lore.kernel.org/all/20250618092810.29370-1-work@onurozkan.dev/ > - https://lore.kernel.org/all/20250618093508.16343-1-work@onurozkan.dev/ > - https://lore.kernel.org/all/20250618091442.29104-1-work@onurozkan.dev/ These look good! Small nit for next time: since they are closely related, please send them as a series to make review (and merging, as they will conflict if not applied in the right order) easier. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Various improvements on clock abstractions 2025-06-16 20:01 [PATCH] Various improvements on clock abstractions onur-ozkan 2025-06-16 20:40 ` Miguel Ojeda @ 2025-06-17 7:52 ` Alexandre Courbot 1 sibling, 0 replies; 7+ messages in thread From: Alexandre Courbot @ 2025-06-17 7:52 UTC (permalink / raw) To: onur-ozkan, linux-clk, rust-for-linux, linux-kernel Cc: mturquette, sboyd, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr On Tue Jun 17, 2025 at 5:01 AM JST, onur-ozkan wrote: > A few changes to improve the clock abstractions and make them a little > more idiomatic: > > 1. `impl Hertz` functions are now constant and compile-time evaluable. > 2. `Hertz` conversions are now done with constant variables, which should "constant variable" is an oxymoron. :) I think you just want to say "constant" here. > make them more readable. > 3. `con_id` is handled in a single line using `map_or` instead of using > nested if-else blocks. Please split these 3 changes into 3 patches, I agree that they are trivial but a patch should do a single thing. This makes review simpler and allows to apply only part of the changes if e.g. one of them needs further discussion or is rejected. The changes in themselves look good though! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-19 13:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16 20:01 [PATCH] Various improvements on clock abstractions onur-ozkan
2025-06-16 20:40 ` Miguel Ojeda
[not found] ` <42151750134012@mail.yandex.com>
2025-06-17 6:55 ` Miguel Ojeda
2025-06-19 6:45 ` Viresh Kumar
2025-06-19 10:05 ` Onur
2025-06-19 13:48 ` Alexandre Courbot
2025-06-17 7:52 ` Alexandre Courbot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox