[PATCH 2/2] device: rust: change the name function

[PATCH 2/2] device: rust: change the name function

[Guilherme Giácomo Simões]

This function increments the refcount by this command
"bindings::get_device(prt)"
This can be confused because the function Arc::from_raw() from
standard library, doesn't
increment the refcount.
Then, this function "Device::from_raw()" will be renamed for don't
make confusion
in the future.

Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
---
 rust/kernel/device.rs   | 2 +-
 rust/kernel/firmware.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 851018eef885..ecffaff041e0 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -51,7 +51,7 @@ 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> {
+    pub unsafe fn get_device(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.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index dee5b4b18aec..13a374a5cdb7 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -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();
-- 
2.46.1

Re: [PATCH 2/2] device: rust: change the name function

[gregkh]

On Sun, Sep 29, 2024 at 10:38:47AM -0300, Guilherme Giácomo Simões wrote:
> This function increments the refcount by this command
> "bindings::get_device(prt)"
> This can be confused because the function Arc::from_raw() from
> standard library, doesn't
> increment the refcount.
> Then, this function "Device::from_raw()" will be renamed for don't
> make confusion
> in the future.

Please wrap the lines here properly so they show up in a sane way :)

> 
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
> ---
>  rust/kernel/device.rs   | 2 +-
>  rust/kernel/firmware.rs | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 851018eef885..ecffaff041e0 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -51,7 +51,7 @@ impl Device {
>      ///
>      /// It must also be ensured that `bindings::device::release` can
> be called from any thread.

Your patch is line-wrapped and can not be applied :(

Please read the email documentation in the kernel for how to use gmail
to send patches out (hint, almost never do so, but you can use git
send-email through it), that will help you in sending changes that can
be applied.

>      /// While not officially documented, this should be the case for
> any `struct device`.
> -    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
> +    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {

With this change, nothing broke?  Does nothing call this code yet?  I
thought the firmware interface did that, but I could be wrong...

thanks,

greg k-h

Re: [PATCH 2/2] device: rust: change the name function

[Guilherme Giácomo Simões]

gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> writes:
>
> On Sun, Sep 29, 2024 at 10:38:47AM -0300, Guilherme Giácomo Simões wrote:
> > This function increments the refcount by this command
> > "bindings::get_device(prt)"
> > This can be confused because the function Arc::from_raw() from
> > standard library, doesn't
> > increment the refcount.
> > Then, this function "Device::from_raw()" will be renamed for don't
> > make confusion
> > in the future.
>
> Please wrap the lines here properly so they show up in a sane way :)
>
> >
> > Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
> > ---
> >  rust/kernel/device.rs   | 2 +-
> >  rust/kernel/firmware.rs | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 851018eef885..ecffaff041e0 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -51,7 +51,7 @@ impl Device {
> >      ///
> >      /// It must also be ensured that `bindings::device::release` can
> > be called from any thread.
>
> Your patch is line-wrapped and can not be applied :(
>
> Please read the email documentation in the kernel for how to use gmail
> to send patches out (hint, almost never do so, but you can use git
> send-email through it), that will help you in sending changes that can
> be applied.

So, I really send the patch with gmail. But, I make this because my
git send-email have any bug and I don't fix this.
I'll see what's happening with my send-email and send the next patch through it.


>
> >      /// While not officially documented, this should be the case for
> > any `struct device`.
> > -    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
> > +    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
>
> With this change, nothing broke?  Does nothing call this code yet?  I
> thought the firmware interface did that, but I could be wrong...
>
> thanks,
>
> greg k-h

This change don't broke nothing. The Device::from_raw() don't is used
yet. The firmware only have a Documentation for calling the
Device::from_raw()
and I change this Doc too.

Re: [PATCH 2/2] device: rust: change the name function

[Danilo Krummrich]

On Mon, Sep 30, 2024 at 01:35:09PM +0200, gregkh@linuxfoundation.org wrote:
> 
> >      /// While not officially documented, this should be the case for
> > any `struct device`.
> > -    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
> > +    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
> 
> With this change, nothing broke?  Does nothing call this code yet?  I
> thought the firmware interface did that, but I could be wrong...

The firmware code uses the `Device` structure, but it doesn't create a
reference from a raw pointer.

This function should probably only ever be called from bus abstractions. I
thought the PHY layer needed this urgently (which also was the reason we merged
it already), so I'd expect the PHY code to use it.

Though, they might just use `as_ref`.

- Danilo

Re: [PATCH 2/2] device: rust: change the name function

[gregkh]

On Mon, Sep 30, 2024 at 02:11:37PM +0200, Danilo Krummrich wrote:
> On Mon, Sep 30, 2024 at 01:35:09PM +0200, gregkh@linuxfoundation.org wrote:
> > 
> > >      /// While not officially documented, this should be the case for
> > > any `struct device`.
> > > -    pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef<Self> {
> > > +    pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
> > 
> > With this change, nothing broke?  Does nothing call this code yet?  I
> > thought the firmware interface did that, but I could be wrong...
> 
> The firmware code uses the `Device` structure, but it doesn't create a
> reference from a raw pointer.
> 
> This function should probably only ever be called from bus abstractions. I
> thought the PHY layer needed this urgently (which also was the reason we merged
> it already), so I'd expect the PHY code to use it.
> 
> Though, they might just use `as_ref`.

Ah, then no harm in renaming it now, great!

When it's resent in a way we can apply it, I'll be glad to queue it up.

thanks,

greg k-h