rust: device: change the from_raw() function

The function Device::from_raw() increments a refcount by a call to bindings::get_device(ptr). This can be confused because usually from_raw() functions don't increment a refcount.
Hence, rename Device::from_raw() to avoid confuion with other "from_raw" semantics.

The new name of function should be "get_device" to be consistent with the function get_device() already exist in .c files.

This function body also changed, because the into() will convert the &'a Device into ARef<Device> and also call inc_ref from the AlwaysRefCounted trait implemented for Device.

Signed-off-by Guilherme Giacomo Simoes <[email protected]>

---
 rust/kernel/device.rs   | 15 +++------------
 rust/kernel/firmware.rs |  2 +-
 2 files changed, 4 insertions(+), 13 deletions(-)

rust/kernel/device.rs +3 -12

index 851018eef885..c8199ee079ef 100644
@@ -51,18 +51,9 @@ impl Device {
     ///
     /// It must also be ensured that `bindings::device::release` can be called from any thread.
     /// While not officially documented, this should be the case for any `struct device`.
-    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
-        // SAFETY: By the safety requirements, ptr is valid.
-        // Initially increase the reference count by one to compensate for the final decrement once
-        // this newly created `ARef<Device>` instance is dropped.
-        unsafe { bindings::get_device(ptr) };
-
-        // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::device`.
-        let ptr = ptr.cast::<Self>();
-
-        // SAFETY: `ptr` is valid by the safety requirements of this function. By the above call to
-        // `bindings::get_device` we also own a reference to the underlying `struct device`.
-        unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(ptr)) }
+    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
+        // SAFETY: By the safety requirements ptr is valid
+        unsafe { Self::as_ref(ptr) }.into()
     }
 
     /// Obtain the raw `struct device *`.

rust/kernel/firmware.rs +1 -1

index dee5b4b18aec..13a374a5cdb7 100644
@@ -44,7 +44,7 @@ fn request_nowarn() -> Self {
 ///
 /// # fn no_run() -> Result<(), Error> {
 /// # // SAFETY: *NOT* safe, just for the example to get an `ARef<Device>` instance
-/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) };
+/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) };
 ///
 /// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?;
 /// let blob = fw.data();

Signature · 10 lines

differences from v1:
- this version don't have 0/1 patch anymore.
- this version contains a refactor get_device() function

The motivation from this change was will discussion in:
https://rust-for-linux.zulipchat.com/#narrow/stream/291566-Library/topic/Inconsistency.20of.20.60from_raw.60.2E

I would like to thanks for Greg <[email protected]>, Danilo
<[email protected]> and Alice <[email protected]> for help me with this
patch.

Quoted 64 lines · Guilherme Giacomo Simoes

The function Device::from_raw() increments a refcount by a call to bindings::get_device(ptr). This can be confused because usually from_raw() functions don't increment a refcount.
Hence, rename Device::from_raw() to avoid confuion with other "from_raw" semantics.

The new name of function should be "get_device" to be consistent with the function get_device() already exist in .c files.

This function body also changed, because the into() will convert the &'a Device into ARef<Device> and also call inc_ref from the AlwaysRefCounted trait implemented for Device.

Signed-off-by Guilherme Giacomo Simoes <[email protected]>

---
 rust/kernel/device.rs   | 15 +++------------
 rust/kernel/firmware.rs |  2 +-
 2 files changed, 4 insertions(+), 13 deletions(-)

rust/kernel/device.rs +3 -12

index 851018eef885..c8199ee079ef 100644
@@ -51,18 +51,9 @@ impl Device {
     ///
     /// It must also be ensured that `bindings::device::release` can be called from any thread.
     /// While not officially documented, this should be the case for any `struct device`.
-    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
-        // SAFETY: By the safety requirements, ptr is valid.
-        // Initially increase the reference count by one to compensate for the final decrement once
-        // this newly created `ARef<Device>` instance is dropped.
-        unsafe { bindings::get_device(ptr) };
-
-        // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::device`.
-        let ptr = ptr.cast::<Self>();
-
-        // SAFETY: `ptr` is valid by the safety requirements of this function. By the above call to
-        // `bindings::get_device` we also own a reference to the underlying `struct device`.
-        unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(ptr)) }
+    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
+        // SAFETY: By the safety requirements ptr is valid
+        unsafe { Self::as_ref(ptr) }.into()
     }
 
     /// Obtain the raw `struct device *`.

rust/kernel/firmware.rs +1 -2

index dee5b4b18aec..13a374a5cdb7 100644
@@ -44,7 +44,7 @@ fn request_nowarn() -> Self {
 ///
 /// # fn no_run() -> Result<(), Error> {
 /// # // SAFETY: *NOT* safe, just for the example to get an `ARef<Device>` instance
-/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) };
+/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) };
 ///
 /// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?;
 /// let blob = fw.data();
--

differences from v1:
- this version don't have 0/1 patch anymore.
- this version contains a refactor get_device() function

Quoted 6 lines

The motivation from this change was will discussion in:
https://rust-for-linux.zulipchat.com/#narrow/stream/291566-Library/topic/Inconsistency.20of.20.60from_raw.60.2E

I would like to thanks for Greg <[email protected]>, Danilo <[email protected]> and Alice <[email protected]> for help me with this patch.

This information goes below the _first_ --- line, not down here at the bottom of the patch. The documentation should explain all of this, you did read that, right? :)

v3 please?

thanks,

greg k-h