Skip to content

Commit 1fc9a0d

Browse files
CohenArthurSkallwar
authored andcommitted
spi: Apply review comments
Co-developed-by: Martin Schmidt <[email protected]> Signed-off-by: Martin Schmidt <[email protected]> Co-developed-by: Arthur Cohen <[email protected]> Signed-off-by: Arthur Cohen <[email protected]> spi: Fix Spi::write_then_read spi: Mark SpiDevice as unsafe and document safety Fix out of scope pointer to spi_driver_register spi: Remove useless mut spi: Fix SAFETY doc for DriverRegistration spi: Run rustfmt Signed-off-by: Esteban Blanc <[email protected]>
1 parent 6d05942 commit 1fc9a0d

File tree

1 file changed

+18
-21
lines changed

1 file changed

+18
-21
lines changed

rust/kernel/spi.rs

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use core::pin::Pin;
1111
pub struct SpiDevice(*mut bindings::spi_device);
1212

1313
impl SpiDevice {
14-
pub fn from_ptr(dev: *mut bindings::spi_device) -> Self {
14+
pub unsafe fn from_ptr(dev: *mut bindings::spi_device) -> Self {
1515
SpiDevice(dev)
1616
}
1717

@@ -85,7 +85,9 @@ impl DriverRegistration {
8585

8686
this.spi_driver = Some(spi_driver);
8787

88-
let res = unsafe { bindings::__spi_register_driver(this.this_module.0, &mut spi_driver) };
88+
let res = unsafe {
89+
bindings::__spi_register_driver(this.this_module.0, this.spi_driver.as_mut().unwrap())
90+
};
8991

9092
match res {
9193
0 => {
@@ -99,7 +101,7 @@ impl DriverRegistration {
99101

100102
impl Drop for DriverRegistration {
101103
fn drop(&mut self) {
102-
unsafe { bindings::driver_unregister(&mut self.spi_driver.unwrap().driver) }
104+
unsafe { bindings::driver_unregister(&mut self.spi_driver.as_mut().unwrap().driver) }
103105
// FIXME: No unwrap? But it's safe?
104106
}
105107
}
@@ -110,8 +112,7 @@ impl Drop for DriverRegistration {
110112
// is safe to pass `&Registration` to multiple threads because it offers no interior mutability.
111113
unsafe impl Sync for DriverRegistration {}
112114

113-
// SAFETY: The only method is `register()`, which requires a (pinned) mutable `Registration`, so it
114-
// is safe to pass `&Registration` to multiple threads because it offers no interior mutability.
115+
// SAFETY: All functions work from any thread.
115116
unsafe impl Send for DriverRegistration {}
116117

117118
type SpiMethod = unsafe extern "C" fn(*mut bindings::spi_device) -> c_types::c_int;
@@ -125,7 +126,8 @@ macro_rules! spi_method {
125126

126127
fn inner(mut $device_name: SpiDevice) -> Result $block
127128

128-
match inner(SpiDevice::from_ptr(dev)) {
129+
// SAFETY: The dev pointer is provided by the kernel and is sure to be valid
130+
match inner(unsafe { SpiDevice::from_ptr(dev) }) {
129131
Ok(_) => 0,
130132
Err(e) => e.to_kernel_errno(),
131133
}
@@ -137,28 +139,23 @@ macro_rules! spi_method {
137139

138140
fn inner(mut $device_name: SpiDevice) $block
139141

140-
inner(SpiDevice::from_ptr(dev))
142+
// SAFETY: The dev pointer is provided by the kernel and is sure to be valid
143+
inner(unsafe { SpiDevice::from_ptr(dev) })
141144
}
142145
};
143146
}
144147

145148
pub struct Spi;
146149

147150
impl Spi {
148-
pub fn write_then_read(
149-
dev: &mut SpiDevice,
150-
tx_buf: &[u8],
151-
n_tx: usize,
152-
rx_buf: &mut [u8],
153-
n_rx: usize,
154-
) -> Result {
151+
pub fn write_then_read(dev: &mut SpiDevice, tx_buf: &[u8], rx_buf: &mut [u8]) -> Result {
155152
let res = unsafe {
156153
bindings::spi_write_then_read(
157154
dev.to_ptr(),
158155
tx_buf.as_ptr() as *const c_types::c_void,
159-
n_tx as c_types::c_uint,
160-
rx_buf.as_ptr() as *mut c_types::c_void,
161-
n_rx as c_types::c_uint,
156+
tx_buf.len() as c_types::c_uint,
157+
rx_buf.as_mut_ptr() as *mut c_types::c_void,
158+
rx_buf.len() as c_types::c_uint,
162159
)
163160
};
164161

@@ -169,12 +166,12 @@ impl Spi {
169166
}
170167

171168
#[inline]
172-
pub fn write(dev: &mut SpiDevice, tx_buf: &[u8], n_tx: usize) -> Result {
173-
Spi::write_then_read(dev, tx_buf, n_tx, &mut [0u8; 0], 0)
169+
pub fn write(dev: &mut SpiDevice, tx_buf: &[u8]) -> Result {
170+
Spi::write_then_read(dev, tx_buf, &mut [0u8; 0])
174171
}
175172

176173
#[inline]
177-
pub fn read(dev: &mut SpiDevice, rx_buf: &mut [u8], n_rx: usize) -> Result {
178-
Spi::write_then_read(dev, &[0u8; 0], 0, rx_buf, n_rx)
174+
pub fn read(dev: &mut SpiDevice, rx_buf: &mut [u8]) -> Result {
175+
Spi::write_then_read(dev, &[0u8; 0], rx_buf)
179176
}
180177
}

0 commit comments

Comments
 (0)