Message ID | 20241218-ncv6336-v1-8-b8d973747f7a@gmail.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Regulator driver with I2C/Regmap Rust abstractions | expand |
Hi Fabien, you might have noticed that I'm using your development branches of this since some time. So sending these patches for review is a big step! Many thanks! Just one topic to check below: On 19.12.24 00:36, Fabien Parent wrote: > From: Fabien Parent <fabien.parent@linaro.org> > > This commit adds support for the Onsemi NCV6336 buck down converter. > > Signed-off-by: Fabien Parent <fabien.parent@linaro.org> > --- > drivers/regulator/Kconfig | 7 + > drivers/regulator/Makefile | 1 + > drivers/regulator/ncv6336_regulator.rs | 241 +++++++++++++++++++++++++++++++++ > 3 files changed, 249 insertions(+) > > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig > index 39297f7d8177193e51c99bc2b360c6d9936e62fe..7254a6e1d7539b147b7ba00ebcb6fd92eb6b2616 100644 > --- a/drivers/regulator/Kconfig > +++ b/drivers/regulator/Kconfig > @@ -952,6 +952,13 @@ config REGULATOR_MTK_DVFSRC > of Mediatek. It allows for voting on regulator state > between multiple users. > > +config REGULATOR_NCV6336 > + tristate "Onsemi NCV6336 regulator driver" > + depends on RUST && OF && I2C=y > + select REGMAP_I2C > + help > + Say y here to support the Onsemi NCV6336 buck converter. > + > config REGULATOR_PALMAS > tristate "TI Palmas PMIC Regulators" > depends on MFD_PALMAS > diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile > index 3d5a803dce8a0556ba9557fa069c6e37593b3c69..0309a78b820cc85883c0c129801ab713e08e4391 100644 > --- a/drivers/regulator/Makefile > +++ b/drivers/regulator/Makefile > @@ -113,6 +113,7 @@ obj-$(CONFIG_REGULATOR_MT6370) += mt6370-regulator.o > obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o > obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o > obj-$(CONFIG_REGULATOR_MTK_DVFSRC) += mtk-dvfsrc-regulator.o > +obj-$(CONFIG_REGULATOR_NCV6336) += ncv6336_regulator.o > obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o > obj-$(CONFIG_REGULATOR_QCOM_PM8008) += qcom-pm8008-regulator.o > obj-$(CONFIG_REGULATOR_QCOM_REFGEN) += qcom-refgen-regulator.o > diff --git a/drivers/regulator/ncv6336_regulator.rs b/drivers/regulator/ncv6336_regulator.rs > new file mode 100644 > index 0000000000000000000000000000000000000000..7efd7630792b68fb353ed1b1634980def9e326a1 > --- /dev/null > +++ b/drivers/regulator/ncv6336_regulator.rs > @@ -0,0 +1,241 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Driver for the Onsemi Buck Converter NCV6336 > +//! > +//! Datasheet: https://www.onsemi.com/pdf/datasheet/ncv6336bm-d.pdf > + > +use kernel::{ > + c_str, i2c, of, > + prelude::*, > + regmap::{self, BitFieldReadOps, BitFieldWriteOps, RawFieldWriteOps}, > + regulator::{ > + driver::{Config, Desc, Device, Driver, RegmapHelpers, Status, Type}, > + Mode, > + }, > + sync::{new_mutex, Arc, Mutex}, > +}; > +use register::*; > + > +kernel::module_i2c_driver! { > + type: Ncv6336, > + name: "ncv6336", > + author: "Fabien Parent <fabien.parent@linaro.org>", > + license: "GPL", > +} > + > +kernel::i2c_device_table!( > + I2C_ID_TABLE, > + MODULE_I2C_ID_TABLE, > + <Ncv6336 as i2c::Driver>::IdInfo, > + [(i2c::DeviceId::new(c_str!("ncv6336")), ()),] > +); > + > +kernel::of_device_table!( > + OF_ID_TABLE, > + MODULE_OF_ID_TABLE, > + <Ncv6336 as i2c::Driver>::IdInfo, > + [(of::DeviceId::new(c_str!("onnn,ncv6336")), ()),] > +); > + > +regmap::define_regmap_field_descs!(FIELD_DESCS, { > + (pid, 0x3, READ, { value => raw([7:0], ro) }), > + (rid, 0x4, READ, { value => raw([7:0], ro) }), > + (fid, 0x5, READ, { value => raw([7:0], ro) }), > + (progvsel1, 0x10, RW, { > + voutvsel1 => raw([6:0], rw), > + envsel1 => bit(7, rw), > + }), > + (progvsel0, 0x11, RW, { > + voutvsel0 => raw([6:0], rw), > + envsel0 => bit(7, rw), > + }), > + (pgood, 0x12, RW, { dischg => bit(4, rw) }), > + (command, 0x14, RW, { > + vselgt => bit(0, rw), > + pwmvsel1 => bit(6, rw), > + pwmvsel0 => bit(7, rw), > + }), > + (limconf, 0x16, RW, { > + rearm => bit(0, rw), > + rststatus => bit(1, rw), > + tpwth => enum([5:4], rw, { > + Temp83C = 0x0, > + Temp94C = 0x1, > + Temp105C = 0x2, > + Temp116C = 0x3, > + }), > + ipeak => enum([7:6], rw, { > + Peak3p5A = 0x0, > + Peak4p0A = 0x1, > + Peak4p5A = 0x2, > + Peak5p0A = 0x3, Could you check to read from or write to the tpwth or ipeak (enum) above? I've been under the impression that for that Desc & enum need to Copy & Clone [1]? [1] diff --git a/rust/kernel/regmap.rs b/rust/kernel/regmap.rs index 232fe93df769..d1baf182f53c 100644 --- a/rust/kernel/regmap.rs +++ b/rust/kernel/regmap.rs @@ -623,6 +623,7 @@ macro_rules! regmap_field_enum { kernel::macros::paste! { #[repr(u32)] #[allow(non_camel_case_types)] + #[derive(Copy, Clone)] pub(crate) enum [<$field_name _enum>] { $($k = $v,)+ } diff --git a/rust/kernel/regulator/driver.rs b/rust/kernel/regulator/driver.rs index e79e93122b09..0b6819e46686 100644 --- a/rust/kernel/regulator/driver.rs +++ b/rust/kernel/regulator/driver.rs @@ -256,6 +256,7 @@ fn set_suspend_mode(_rdev: &mut Device<Self::Data>, _mode: Mode) -> Result { /// # Invariants /// /// `self.0` has always valid data. +#[derive(Copy, Clone)] pub struct Desc(bindings::regulator_desc); impl Desc { /// Create a new [`Device`] descriptor > +static NCV6336_DESC: Desc = Desc::new::<Ncv6336>(c_str!("ncv6336"), Type::Voltage) > + .with_owner(&THIS_MODULE) > + .with_of_match(c_str!("buck")) > + .with_active_discharge( > + pgood::addr(), > + pgood::dischg::mask(), > + pgood::dischg::mask(), > + 0, > + ) > + .with_csel( > + limconf::addr(), > + limconf::ipeak::mask(), > + &[3_500_000, 4_000_000, 4_500_000, 5_000_000], > + ) > + .with_enable( > + progvsel0::addr(), > + progvsel0::envsel0::mask(), > + progvsel0::envsel0::mask(), > + 0, > + ) > + .with_linear_mapping( > + progvsel0::addr(), > + progvsel0::voutvsel0::mask(), > + 600_000, > + 6250, > + 128, > + 0, > + ); > + > +struct Ncv6336RegulatorData { > + fields: regmap::Fields<{ FIELD_DESCS.len() }>, > +} > + > +struct Ncv6336(#[expect(dead_code)] Device<<Self as Driver>::Data>); > + > +impl i2c::Driver for Ncv6336 { > + type IdInfo = (); > + > + const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_ID_TABLE); > + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_ID_TABLE); > + > + fn probe(client: &mut i2c::Client, _id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> { > + let config = regmap::Config::<AccessOps>::new(8, 8) > + .with_max_register(0x16) > + .with_cache_type(regmap::CacheType::RbTree); > + let regmap = Arc::new(regmap::Regmap::init_i2c(client, &config)?, GFP_KERNEL)?; > + let fields = regmap::Fields::new(®map, &FIELD_DESCS)?; > + > + let data = Arc::pin_init(new_mutex!(Ncv6336RegulatorData { fields }), GFP_KERNEL)?; > + let config = Config::new(client.as_ref(), data.clone()).with_regmap(regmap.clone()); > + let regulator = Device::register(client.as_ref(), &NCV6336_DESC, config)?; > + > + let drvdata = KBox::new(Self(regulator), GFP_KERNEL)?; > + > + Ok(drvdata.into()) > + } > +} > + > +#[vtable] > +impl Driver for Ncv6336 { > + type Data = Arc<Mutex<Ncv6336RegulatorData>>; > + > + fn list_voltage(reg: &mut Device<Self::Data>, selector: u32) -> Result<i32> { > + reg.list_voltage_linear(selector) > + } > + > + fn enable(reg: &mut Device<Self::Data>) -> Result { > + reg.enable_regmap() > + } > + > + fn disable(reg: &mut Device<Self::Data>) -> Result { > + reg.disable_regmap() > + } > + > + fn is_enabled(reg: &mut Device<Self::Data>) -> Result<bool> { > + reg.is_enabled_regmap() > + } > + > + fn set_active_discharge(reg: &mut Device<Self::Data>, enable: bool) -> Result { > + reg.set_active_discharge_regmap(enable) > + } > + > + fn set_current_limit(reg: &mut Device<Self::Data>, min_ua: i32, max_ua: i32) -> Result { > + reg.set_current_limit_regmap(min_ua, max_ua) > + } > + > + fn get_current_limit(reg: &mut Device<Self::Data>) -> Result<i32> { > + reg.get_current_limit_regmap() > + } > + > + fn set_voltage_sel(reg: &mut Device<Self::Data>, selector: u32) -> Result { > + reg.set_voltage_sel_regmap(selector) > + } > + > + fn get_voltage_sel(reg: &mut Device<Self::Data>) -> Result<i32> { > + reg.get_voltage_sel_regmap() > + } > + > + fn set_mode(reg: &mut Device<Self::Data>, mode: Mode) -> Result { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + match mode { > + Mode::Normal => command::pwmvsel0::clear(fields), > + Mode::Fast => command::pwmvsel0::set(fields), > + _ => Err(ENOTSUPP), > + } > + } > + > + fn get_mode(reg: &mut Device<Self::Data>) -> Mode { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + match command::pwmvsel0::is_set(fields) { > + Ok(true) => Mode::Fast, > + Ok(false) => Mode::Normal, > + Err(_) => Mode::Invalid, > + } > + } > + > + fn get_status(reg: &mut Device<Self::Data>) -> Result<Status> { > + if !Self::is_enabled(reg)? { > + return Ok(Status::Off); > + } > + > + Ok(Self::get_mode(reg).into()) > + } > + > + fn set_suspend_voltage(reg: &mut Device<Self::Data>, uv: i32) -> Result { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + let quot = (uv - 600000) / 6250; > + let rem = (uv - 600000) % 6250; > + let selector = if rem > 0 { quot + 1 } else { quot }; > + > + progvsel1::voutvsel1::write(fields, selector as _) > + } > + > + fn set_suspend_enable(reg: &mut Device<Self::Data>) -> Result { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + progvsel1::envsel1::set(fields)?; > + command::vselgt::clear(fields) > + } > + > + fn set_suspend_disable(reg: &mut Device<Self::Data>) -> Result { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + progvsel1::envsel1::clear(fields)?; > + command::vselgt::set(fields) > + } > + > + fn set_suspend_mode(reg: &mut Device<Self::Data>, mode: Mode) -> Result { > + let data = reg.data(); > + let fields = &mut data.lock().fields; > + > + match mode { > + Mode::Normal => command::pwmvsel1::clear(fields), > + Mode::Fast => command::pwmvsel1::set(fields), > + _ => Err(ENOTSUPP), > + } > + } > +} >
On Wed, Dec 18, 2024 at 03:36:38PM -0800, Fabien Parent wrote: > +regmap::define_regmap_field_descs!(FIELD_DESCS, { > + (pid, 0x3, READ, { value => raw([7:0], ro) }), Looking at this it appears that the whole thing with only exposing regmap fields is actually a deliberate attempt to elide whole register access. This doesn't seem like a good idea, it's quite common to want to operate on a register as a whole. For example there's the very common case where we're updating multiple fields at once, we don't want to do a separate read/modify/write cycle for each field. As I mentioned in review of the regmap patch I'd expect that fields would be layered on the underlying register access interface, and I'd expect that we'd always be able to work with the registers. > +static NCV6336_DESC: Desc = Desc::new::<Ncv6336>(c_str!("ncv6336"), Type::Voltage) > + .with_owner(&THIS_MODULE) > + .with_of_match(c_str!("buck")) > + .with_active_discharge( > + pgood::addr(), > + pgood::dischg::mask(), > + pgood::dischg::mask(), > + 0, > + ) I'm not sure these sequences of unnamed arguments are great for legibility. > +#[vtable] > +impl Driver for Ncv6336 { > + type Data = Arc<Mutex<Ncv6336RegulatorData>>; > + > + fn list_voltage(reg: &mut Device<Self::Data>, selector: u32) -> Result<i32> { > + reg.list_voltage_linear(selector) > + } > + > + fn enable(reg: &mut Device<Self::Data>) -> Result { > + reg.enable_regmap() > + } > + > + fn disable(reg: &mut Device<Self::Data>) -> Result { > + reg.disable_regmap() > + } > + > + fn is_enabled(reg: &mut Device<Self::Data>) -> Result<bool> { > + reg.is_enabled_regmap() > + } I can't help but feel that this isn't a great way to deal with the ops table - AIUI we're mapping the C functions to rust, then bouncing those wrapped functions back to the C API. All of this with no abstraction of the whole fact that this is a data driven way of specifying the regmap. It feels like we're loosing a lot of the point of having these data driven helpers, for most devices where we're data driven it looks like it would be actively worse to write in rust. As with the regmap fields I'd really expect to see this handled in a much more directly rust native manner for drivers, in this case I'd expect that to wind up with us reusing the C ops but handled by the support code rather than directly by each driver. Drivers doing complicated things might want to peer in and work with the ops (eg, some devices need a special write sequence but the read side uses helpers) but it shouldn't be the default. > + fn get_status(reg: &mut Device<Self::Data>) -> Result<Status> { > + if !Self::is_enabled(reg)? { > + return Ok(Status::Off); > + } > + > + Ok(Self::get_mode(reg).into()) > + } This is buggy, it's just returning the values configured by the driver not status from the hardware. If the hardware doesn't report status don't implement this operation.
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 39297f7d8177193e51c99bc2b360c6d9936e62fe..7254a6e1d7539b147b7ba00ebcb6fd92eb6b2616 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -952,6 +952,13 @@ config REGULATOR_MTK_DVFSRC of Mediatek. It allows for voting on regulator state between multiple users. +config REGULATOR_NCV6336 + tristate "Onsemi NCV6336 regulator driver" + depends on RUST && OF && I2C=y + select REGMAP_I2C + help + Say y here to support the Onsemi NCV6336 buck converter. + config REGULATOR_PALMAS tristate "TI Palmas PMIC Regulators" depends on MFD_PALMAS diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 3d5a803dce8a0556ba9557fa069c6e37593b3c69..0309a78b820cc85883c0c129801ab713e08e4391 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -113,6 +113,7 @@ obj-$(CONFIG_REGULATOR_MT6370) += mt6370-regulator.o obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o obj-$(CONFIG_REGULATOR_MTK_DVFSRC) += mtk-dvfsrc-regulator.o +obj-$(CONFIG_REGULATOR_NCV6336) += ncv6336_regulator.o obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o obj-$(CONFIG_REGULATOR_QCOM_PM8008) += qcom-pm8008-regulator.o obj-$(CONFIG_REGULATOR_QCOM_REFGEN) += qcom-refgen-regulator.o diff --git a/drivers/regulator/ncv6336_regulator.rs b/drivers/regulator/ncv6336_regulator.rs new file mode 100644 index 0000000000000000000000000000000000000000..7efd7630792b68fb353ed1b1634980def9e326a1 --- /dev/null +++ b/drivers/regulator/ncv6336_regulator.rs @@ -0,0 +1,241 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Driver for the Onsemi Buck Converter NCV6336 +//! +//! Datasheet: https://www.onsemi.com/pdf/datasheet/ncv6336bm-d.pdf + +use kernel::{ + c_str, i2c, of, + prelude::*, + regmap::{self, BitFieldReadOps, BitFieldWriteOps, RawFieldWriteOps}, + regulator::{ + driver::{Config, Desc, Device, Driver, RegmapHelpers, Status, Type}, + Mode, + }, + sync::{new_mutex, Arc, Mutex}, +}; +use register::*; + +kernel::module_i2c_driver! { + type: Ncv6336, + name: "ncv6336", + author: "Fabien Parent <fabien.parent@linaro.org>", + license: "GPL", +} + +kernel::i2c_device_table!( + I2C_ID_TABLE, + MODULE_I2C_ID_TABLE, + <Ncv6336 as i2c::Driver>::IdInfo, + [(i2c::DeviceId::new(c_str!("ncv6336")), ()),] +); + +kernel::of_device_table!( + OF_ID_TABLE, + MODULE_OF_ID_TABLE, + <Ncv6336 as i2c::Driver>::IdInfo, + [(of::DeviceId::new(c_str!("onnn,ncv6336")), ()),] +); + +regmap::define_regmap_field_descs!(FIELD_DESCS, { + (pid, 0x3, READ, { value => raw([7:0], ro) }), + (rid, 0x4, READ, { value => raw([7:0], ro) }), + (fid, 0x5, READ, { value => raw([7:0], ro) }), + (progvsel1, 0x10, RW, { + voutvsel1 => raw([6:0], rw), + envsel1 => bit(7, rw), + }), + (progvsel0, 0x11, RW, { + voutvsel0 => raw([6:0], rw), + envsel0 => bit(7, rw), + }), + (pgood, 0x12, RW, { dischg => bit(4, rw) }), + (command, 0x14, RW, { + vselgt => bit(0, rw), + pwmvsel1 => bit(6, rw), + pwmvsel0 => bit(7, rw), + }), + (limconf, 0x16, RW, { + rearm => bit(0, rw), + rststatus => bit(1, rw), + tpwth => enum([5:4], rw, { + Temp83C = 0x0, + Temp94C = 0x1, + Temp105C = 0x2, + Temp116C = 0x3, + }), + ipeak => enum([7:6], rw, { + Peak3p5A = 0x0, + Peak4p0A = 0x1, + Peak4p5A = 0x2, + Peak5p0A = 0x3, + }), + }) +}); + +static NCV6336_DESC: Desc = Desc::new::<Ncv6336>(c_str!("ncv6336"), Type::Voltage) + .with_owner(&THIS_MODULE) + .with_of_match(c_str!("buck")) + .with_active_discharge( + pgood::addr(), + pgood::dischg::mask(), + pgood::dischg::mask(), + 0, + ) + .with_csel( + limconf::addr(), + limconf::ipeak::mask(), + &[3_500_000, 4_000_000, 4_500_000, 5_000_000], + ) + .with_enable( + progvsel0::addr(), + progvsel0::envsel0::mask(), + progvsel0::envsel0::mask(), + 0, + ) + .with_linear_mapping( + progvsel0::addr(), + progvsel0::voutvsel0::mask(), + 600_000, + 6250, + 128, + 0, + ); + +struct Ncv6336RegulatorData { + fields: regmap::Fields<{ FIELD_DESCS.len() }>, +} + +struct Ncv6336(#[expect(dead_code)] Device<<Self as Driver>::Data>); + +impl i2c::Driver for Ncv6336 { + type IdInfo = (); + + const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_ID_TABLE); + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_ID_TABLE); + + fn probe(client: &mut i2c::Client, _id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> { + let config = regmap::Config::<AccessOps>::new(8, 8) + .with_max_register(0x16) + .with_cache_type(regmap::CacheType::RbTree); + let regmap = Arc::new(regmap::Regmap::init_i2c(client, &config)?, GFP_KERNEL)?; + let fields = regmap::Fields::new(®map, &FIELD_DESCS)?; + + let data = Arc::pin_init(new_mutex!(Ncv6336RegulatorData { fields }), GFP_KERNEL)?; + let config = Config::new(client.as_ref(), data.clone()).with_regmap(regmap.clone()); + let regulator = Device::register(client.as_ref(), &NCV6336_DESC, config)?; + + let drvdata = KBox::new(Self(regulator), GFP_KERNEL)?; + + Ok(drvdata.into()) + } +} + +#[vtable] +impl Driver for Ncv6336 { + type Data = Arc<Mutex<Ncv6336RegulatorData>>; + + fn list_voltage(reg: &mut Device<Self::Data>, selector: u32) -> Result<i32> { + reg.list_voltage_linear(selector) + } + + fn enable(reg: &mut Device<Self::Data>) -> Result { + reg.enable_regmap() + } + + fn disable(reg: &mut Device<Self::Data>) -> Result { + reg.disable_regmap() + } + + fn is_enabled(reg: &mut Device<Self::Data>) -> Result<bool> { + reg.is_enabled_regmap() + } + + fn set_active_discharge(reg: &mut Device<Self::Data>, enable: bool) -> Result { + reg.set_active_discharge_regmap(enable) + } + + fn set_current_limit(reg: &mut Device<Self::Data>, min_ua: i32, max_ua: i32) -> Result { + reg.set_current_limit_regmap(min_ua, max_ua) + } + + fn get_current_limit(reg: &mut Device<Self::Data>) -> Result<i32> { + reg.get_current_limit_regmap() + } + + fn set_voltage_sel(reg: &mut Device<Self::Data>, selector: u32) -> Result { + reg.set_voltage_sel_regmap(selector) + } + + fn get_voltage_sel(reg: &mut Device<Self::Data>) -> Result<i32> { + reg.get_voltage_sel_regmap() + } + + fn set_mode(reg: &mut Device<Self::Data>, mode: Mode) -> Result { + let data = reg.data(); + let fields = &mut data.lock().fields; + + match mode { + Mode::Normal => command::pwmvsel0::clear(fields), + Mode::Fast => command::pwmvsel0::set(fields), + _ => Err(ENOTSUPP), + } + } + + fn get_mode(reg: &mut Device<Self::Data>) -> Mode { + let data = reg.data(); + let fields = &mut data.lock().fields; + + match command::pwmvsel0::is_set(fields) { + Ok(true) => Mode::Fast, + Ok(false) => Mode::Normal, + Err(_) => Mode::Invalid, + } + } + + fn get_status(reg: &mut Device<Self::Data>) -> Result<Status> { + if !Self::is_enabled(reg)? { + return Ok(Status::Off); + } + + Ok(Self::get_mode(reg).into()) + } + + fn set_suspend_voltage(reg: &mut Device<Self::Data>, uv: i32) -> Result { + let data = reg.data(); + let fields = &mut data.lock().fields; + + let quot = (uv - 600000) / 6250; + let rem = (uv - 600000) % 6250; + let selector = if rem > 0 { quot + 1 } else { quot }; + + progvsel1::voutvsel1::write(fields, selector as _) + } + + fn set_suspend_enable(reg: &mut Device<Self::Data>) -> Result { + let data = reg.data(); + let fields = &mut data.lock().fields; + + progvsel1::envsel1::set(fields)?; + command::vselgt::clear(fields) + } + + fn set_suspend_disable(reg: &mut Device<Self::Data>) -> Result { + let data = reg.data(); + let fields = &mut data.lock().fields; + + progvsel1::envsel1::clear(fields)?; + command::vselgt::set(fields) + } + + fn set_suspend_mode(reg: &mut Device<Self::Data>, mode: Mode) -> Result { + let data = reg.data(); + let fields = &mut data.lock().fields; + + match mode { + Mode::Normal => command::pwmvsel1::clear(fields), + Mode::Fast => command::pwmvsel1::set(fields), + _ => Err(ENOTSUPP), + } + } +}