* Re: [PATCH] rust: opp: use to_result for error handling
2025-08-21 9:16 [PATCH] rust: opp: use to_result for error handling Onur Özkan
@ 2025-08-21 9:21 ` Viresh Kumar
2025-08-22 5:13 ` Onur Özkan
2025-08-22 5:53 ` Viresh Kumar
2025-08-25 22:56 ` Elle Rhumsaa
2 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2025-08-21 9:21 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, vireshk, nm, sboyd, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
tmgross, dakr, linux-pm, linux-kernel
On 21-08-25, 12:16, Onur Özkan wrote:
> Simplifies error handling by replacing the manual check
> of the return value with the `to_result` helper.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/opp.rs | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index 08126035d2c6..9d79c2816af5 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -12,7 +12,7 @@
> clk::Hertz,
> cpumask::{Cpumask, CpumaskVar},
> device::Device,
> - error::{code::*, from_err_ptr, from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> + error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
> ffi::c_ulong,
> prelude::*,
> str::CString,
> @@ -500,11 +500,8 @@ pub fn set(self, dev: &Device) -> Result<ConfigToken> {
> // requirements. The OPP core guarantees not to access fields of [`Config`] after this call
> // and so we don't need to save a copy of them for future use.
> let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
> - if ret < 0 {
> - Err(Error::from_errno(ret))
> - } else {
> - Ok(ConfigToken(ret))
> - }
> +
> + to_result(ret).map(|()| ConfigToken(ret))
> }
>
> /// Config's clk callback.
> @@ -713,11 +710,8 @@ pub fn opp_count(&self) -> Result<u32> {
> // SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
> // requirements.
> let ret = unsafe { bindings::dev_pm_opp_get_opp_count(self.dev.as_raw()) };
> - if ret < 0 {
> - Err(Error::from_errno(ret))
> - } else {
> - Ok(ret as u32)
> - }
> +
> + to_result(ret).map(|()| ret as u32)
> }
I would get rid of `ret` as well in both the cases, like it is done by
other users of `to_result()`.
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: opp: use to_result for error handling
2025-08-21 9:21 ` Viresh Kumar
@ 2025-08-22 5:13 ` Onur Özkan
2025-08-22 5:52 ` Viresh Kumar
0 siblings, 1 reply; 6+ messages in thread
From: Onur Özkan @ 2025-08-22 5:13 UTC (permalink / raw)
To: Viresh Kumar
Cc: rust-for-linux, vireshk, nm, sboyd, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
tmgross, dakr, linux-pm, linux-kernel
On Thu, 21 Aug 2025 14:51:37 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 21-08-25, 12:16, Onur Özkan wrote:
> > Simplifies error handling by replacing the manual check
> > of the return value with the `to_result` helper.
> >
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
> > ---
> > rust/kernel/opp.rs | 16 +++++-----------
> > 1 file changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> > index 08126035d2c6..9d79c2816af5 100644
> > --- a/rust/kernel/opp.rs
> > +++ b/rust/kernel/opp.rs
> > @@ -12,7 +12,7 @@
> > clk::Hertz,
> > cpumask::{Cpumask, CpumaskVar},
> > device::Device,
> > - error::{code::*, from_err_ptr, from_result, to_result, Error,
> > Result, VTABLE_DEFAULT_ERROR},
> > + error::{code::*, from_err_ptr, from_result, to_result, Result,
> > VTABLE_DEFAULT_ERROR}, ffi::c_ulong,
> > prelude::*,
> > str::CString,
> > @@ -500,11 +500,8 @@ pub fn set(self, dev: &Device) ->
> > Result<ConfigToken> { // requirements. The OPP core guarantees not
> > to access fields of [`Config`] after this call // and so we don't
> > need to save a copy of them for future use. let ret = unsafe {
> > bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
> > - if ret < 0 {
> > - Err(Error::from_errno(ret))
> > - } else {
> > - Ok(ConfigToken(ret))
> > - }
> > +
> > + to_result(ret).map(|()| ConfigToken(ret))
> > }
> >
> > /// Config's clk callback.
> > @@ -713,11 +710,8 @@ pub fn opp_count(&self) -> Result<u32> {
> > // SAFETY: The requirements are satisfied by the existence
> > of [`Device`] and its safety // requirements.
> > let ret = unsafe {
> > bindings::dev_pm_opp_get_opp_count(self.dev.as_raw()) };
> > - if ret < 0 {
> > - Err(Error::from_errno(ret))
> > - } else {
> > - Ok(ret as u32)
> > - }
> > +
> > + to_result(ret).map(|()| ret as u32)
> > }
>
> I would get rid of `ret` as well in both the cases, like it is done by
> other users of `to_result()`.
>
`ret` is returned at the end unlike other `to_result` usages in other
functions.
-Onur
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rust: opp: use to_result for error handling
2025-08-22 5:13 ` Onur Özkan
@ 2025-08-22 5:52 ` Viresh Kumar
0 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2025-08-22 5:52 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, vireshk, nm, sboyd, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
tmgross, dakr, linux-pm, linux-kernel
On 22-08-25, 08:13, Onur Özkan wrote:
> `ret` is returned at the end unlike other `to_result` usages in other
> functions.
Ahh, I missed that to_result() doesn't return `ret` at all.
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: opp: use to_result for error handling
2025-08-21 9:16 [PATCH] rust: opp: use to_result for error handling Onur Özkan
2025-08-21 9:21 ` Viresh Kumar
@ 2025-08-22 5:53 ` Viresh Kumar
2025-08-25 22:56 ` Elle Rhumsaa
2 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2025-08-22 5:53 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, vireshk, nm, sboyd, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
tmgross, dakr, linux-pm, linux-kernel
On 21-08-25, 12:16, Onur Özkan wrote:
> Simplifies error handling by replacing the manual check
> of the return value with the `to_result` helper.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/opp.rs | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
Applied. Thanks.
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: opp: use to_result for error handling
2025-08-21 9:16 [PATCH] rust: opp: use to_result for error handling Onur Özkan
2025-08-21 9:21 ` Viresh Kumar
2025-08-22 5:53 ` Viresh Kumar
@ 2025-08-25 22:56 ` Elle Rhumsaa
2 siblings, 0 replies; 6+ messages in thread
From: Elle Rhumsaa @ 2025-08-25 22:56 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, vireshk, nm, sboyd, ojeda, alex.gaynor,
boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
tmgross, dakr, linux-pm, linux-kernel
On Thu, Aug 21, 2025 at 12:16:05PM +0300, Onur Özkan wrote:
> Simplifies error handling by replacing the manual check
> of the return value with the `to_result` helper.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/opp.rs | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index 08126035d2c6..9d79c2816af5 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -12,7 +12,7 @@
> clk::Hertz,
> cpumask::{Cpumask, CpumaskVar},
> device::Device,
> - error::{code::*, from_err_ptr, from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> + error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
> ffi::c_ulong,
> prelude::*,
> str::CString,
> @@ -500,11 +500,8 @@ pub fn set(self, dev: &Device) -> Result<ConfigToken> {
> // requirements. The OPP core guarantees not to access fields of [`Config`] after this call
> // and so we don't need to save a copy of them for future use.
> let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
> - if ret < 0 {
> - Err(Error::from_errno(ret))
> - } else {
> - Ok(ConfigToken(ret))
> - }
> +
> + to_result(ret).map(|()| ConfigToken(ret))
> }
>
> /// Config's clk callback.
> @@ -713,11 +710,8 @@ pub fn opp_count(&self) -> Result<u32> {
> // SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
> // requirements.
> let ret = unsafe { bindings::dev_pm_opp_get_opp_count(self.dev.as_raw()) };
> - if ret < 0 {
> - Err(Error::from_errno(ret))
> - } else {
> - Ok(ret as u32)
> - }
> +
> + to_result(ret).map(|()| ret as u32)
> }
>
> /// Returns max clock latency (in nanoseconds) of the [`OPP`]s in the [`Table`].
> --
> 2.50.0
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread