Skip to contentSkip to conversation

rust: device: support `dev_printk` on all devices

PATCH Superseded Gary Guo 7 participants 15 messages Jan 20 – Jan 27

Subsystems (12)
DRM drivers PCI RISC-V Driver core +8 more subsystems

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Gary Guo

author
#0 lore raw

PATCH 1/4 rust: device: support `dev_printk` on all devices

From: Gary Guo <[email protected]>

Currently, dev_* only works on the core Device, but not on any other bus or class device objects. This causes a pattern of dev_info!(pdev.as_ref()) which is not ideal.

This adds support of using these devices directly with dev_* macros, by adding AsRef call inside the macro. To make sure we can still use just kernel::device::Device, as AsRef implementation is added for it; this is typical for types that is designed to use with AsRef anyway, for example, str implements AsRef<str> and Path implements AsRef<Path>.

Signed-off-by Gary Guo <[email protected]>
---
 rust/kernel/device.rs | 9 ++++++++-
 rust/kernel/pci.rs    | 2 +-
 rust/kernel/pci/id.rs | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)
rust/kernel/device.rs +8 -1
index c7b5db9dcca1..94e0548e7687 100644
@@ -601,6 +601,13 @@ impl DeviceContext for Core {}
 impl DeviceContext for CoreInternal {}
 impl DeviceContext for Normal {}
 
+impl<Ctx: DeviceContext> AsRef<Device<Ctx>> for Device<Ctx> {
+    #[inline]
+    fn as_ref(&self) -> &Device<Ctx> {
+        self
+    }
+}
+
 /// Convert device references to bus device references.
 ///
 /// Bus devices can implement this trait to allow abstractions to provide the bus device in

@@ -720,7 +727,7 @@ macro_rules! impl_device_context_into_aref {
 macro_rules! dev_printk {
     ($method:ident, $dev:expr, $($f:tt)*) => {
         {
-            ($dev).$method($crate::prelude::fmt!($($f)*));
+            $crate::device::Device::$method($dev.as_ref(), $crate::prelude::fmt!($($f)*))
         }
     }
 }
rust/kernel/pci.rs +1 -1
index bea76ca9c3da..3946ca919332 100644
@@ -351,7 +351,7 @@ impl Device {
     ///     // Get an instance of `Vendor`.
     ///     let vendor = pdev.vendor_id();
     ///     dev_info!(
-    ///         pdev.as_ref(),
+    ///         pdev,
     ///         "Device: Vendor={}, Device=0x{:x}\n",
     ///         vendor,
     ///         pdev.device_id()
rust/kernel/pci/id.rs +1 -1
index c09125946d9e..e2d9e8804347 100644
@@ -22,7 +22,7 @@
 /// fn probe_device(pdev: &pci::Device<Core>) -> Result {
 ///     let pci_class = pdev.pci_class();
 ///     dev_info!(
-///         pdev.as_ref(),
+///         pdev,
 ///         "Detected PCI class: {}\n",
 ///         pci_class
 ///     );

base-commit: 053966c344dbd346e71305f530e91ea77916189f

Signature · 1 line
2.51.2
author
#1 lore raw

PATCH 2/4 rust: samples: remove redundant `.as_ref()` for `dev_*` print

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
 samples/rust/rust_dma.rs              |  2 +-
 samples/rust/rust_driver_auxiliary.rs |  2 +-
 samples/rust/rust_driver_faux.rs      |  2 +-
 samples/rust/rust_driver_i2c.rs       |  4 ++--
 samples/rust/rust_driver_pci.rs       |  6 +++---
 samples/rust/rust_driver_platform.rs  |  2 +-
 samples/rust/rust_i2c_client.rs       | 10 ++--------
 7 files changed, 11 insertions(+), 17 deletions(-)
samples/rust/rust_dma.rs +1 -1
index f53bce2a73e3..a9ac7937ebaf 100644
@@ -57,7 +57,7 @@ impl pci::Driver for DmaSampleDriver {
 
     fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         pin_init::pin_init_scope(move || {
-            dev_info!(pdev.as_ref(), "Probe DMA test driver.\n");
+            dev_info!(pdev, "Probe DMA test driver.\n");
 
             let mask = DmaMask::new::<64>();
samples/rust/rust_driver_auxiliary.rs +1 -1
index f148124fe81f..c20961f16835 100644
@@ -39,7 +39,7 @@ impl auxiliary::Driver for AuxiliaryDriver {
 
     fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         dev_info!(
-            adev.as_ref(),
+            adev,
             "Probing auxiliary driver for auxiliary device with id={}\n",
             adev.id()
         );
samples/rust/rust_driver_faux.rs +1 -1
index 5330b77ea986..99876c8e3743 100644
@@ -26,7 +26,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
 
         let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
 
-        dev_info!(reg.as_ref(), "Hello from faux device!\n");
+        dev_info!(reg, "Hello from faux device!\n");
 
         Ok(Self { _reg: reg })
     }
samples/rust/rust_driver_i2c.rs +2 -2
index 6be79f9e9fb5..e71b77196163 100644
@@ -56,11 +56,11 @@ fn probe(
     }
 
     fn shutdown(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+        dev_info!(idev, "Shutdown Rust I2C driver sample.\n");
     }
 
     fn unbind(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Unbind Rust I2C driver sample.\n");
+        dev_info!(idev, "Unbind Rust I2C driver sample.\n");
     }
 }
samples/rust/rust_driver_pci.rs +3 -3
index ef04c6401e6a..d50828b642e5 100644
@@ -75,7 +75,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
         pin_init::pin_init_scope(move || {
             let vendor = pdev.vendor_id();
             dev_dbg!(
-                pdev.as_ref(),
+                pdev,
                 "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
                 vendor,
                 pdev.device_id()

@@ -91,7 +91,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
                     let bar = bar.access(pdev.as_ref())?;
 
                     dev_info!(
-                        pdev.as_ref(),
+                        pdev,
                         "pci-testdev data-match count: {}\n",
                         Self::testdev(info, bar)?
                     );

@@ -112,7 +112,7 @@ fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
 #[pinned_drop]
 impl PinnedDrop for SampleDriver {
     fn drop(self: Pin<&mut Self>) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust PCI driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
     }
 }
samples/rust/rust_driver_platform.rs +1 -1
index 9537dc38c563..f2229d176fb9 100644
@@ -180,7 +180,7 @@ fn properties_parse(dev: &device::Device) -> Result {
 
 impl Drop for SampleDriver {
     fn drop(&mut self) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust Platform driver sample.\n");
     }
 }
samples/rust/rust_i2c_client.rs +2 -8
index 8d2c12e535b0..72da5499f150 100644
@@ -113,10 +113,7 @@ fn probe(
         pdev: &platform::Device<device::Core>,
         _info: Option<&Self::IdInfo>,
     ) -> impl PinInit<Self, Error> {
-        dev_info!(
-            pdev.as_ref(),
-            "Probe Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Probe Rust I2C Client registration sample.\n");
 
         kernel::try_pin_init!( Self {
             parent_dev: pdev.into(),

@@ -130,10 +127,7 @@ fn probe(
     }
 
     fn unbind(pdev: &platform::Device<device::Core>, _this: Pin<&Self>) {
-        dev_info!(
-            pdev.as_ref(),
-            "Unbind Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Unbind Rust I2C Client registration sample.\n");
     }
 }
Signature · 1 line
2.51.2
#2 lore raw

Re: [PATCH 2/4] rust: samples: remove redundant `.as_ref()` for `dev_*` print

Hi Gary

Quoted 133 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
  samples/rust/rust_dma.rs              |  2 +-
  samples/rust/rust_driver_auxiliary.rs |  2 +-
  samples/rust/rust_driver_faux.rs      |  2 +-
  samples/rust/rust_driver_i2c.rs       |  4 ++--
  samples/rust/rust_driver_pci.rs       |  6 +++---
  samples/rust/rust_driver_platform.rs  |  2 +-
  samples/rust/rust_i2c_client.rs       | 10 ++--------
  7 files changed, 11 insertions(+), 17 deletions(-)
samples/rust/rust_dma.rs +1 -1
index f53bce2a73e3..a9ac7937ebaf 100644
@@ -57,7 +57,7 @@ impl pci::Driver for DmaSampleDriver {
 
      fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          pin_init::pin_init_scope(move || {
-            dev_info!(pdev.as_ref(), "Probe DMA test driver.\n");
+            dev_info!(pdev, "Probe DMA test driver.\n");
 
              let mask = DmaMask::new::<64>();
samples/rust/rust_driver_auxiliary.rs +1 -1
index f148124fe81f..c20961f16835 100644
@@ -39,7 +39,7 @@ impl auxiliary::Driver for AuxiliaryDriver {
 
      fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          dev_info!(
-            adev.as_ref(),
+            adev,
              "Probing auxiliary driver for auxiliary device with id={}\n",
              adev.id()
          );
samples/rust/rust_driver_faux.rs +1 -1
index 5330b77ea986..99876c8e3743 100644
@@ -26,7 +26,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
 
          let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
 
-        dev_info!(reg.as_ref(), "Hello from faux device!\n");
+        dev_info!(reg, "Hello from faux device!\n");
 
          Ok(Self { _reg: reg })
      }
samples/rust/rust_driver_i2c.rs +2 -2
index 6be79f9e9fb5..e71b77196163 100644
@@ -56,11 +56,11 @@ fn probe(
      }
 
      fn shutdown(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+        dev_info!(idev, "Shutdown Rust I2C driver sample.\n");
      }
 
      fn unbind(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Unbind Rust I2C driver sample.\n");
+        dev_info!(idev, "Unbind Rust I2C driver sample.\n");
      }
  }
samples/rust/rust_driver_pci.rs +3 -3
index ef04c6401e6a..d50828b642e5 100644
@@ -75,7 +75,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
          pin_init::pin_init_scope(move || {
              let vendor = pdev.vendor_id();
              dev_dbg!(
-                pdev.as_ref(),
+                pdev,
                  "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
                  vendor,
                  pdev.device_id()

@@ -91,7 +91,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
                      let bar = bar.access(pdev.as_ref())?;
 
                      dev_info!(
-                        pdev.as_ref(),
+                        pdev,
                          "pci-testdev data-match count: {}\n",
                          Self::testdev(info, bar)?
                      );

@@ -112,7 +112,7 @@ fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
  #[pinned_drop]
  impl PinnedDrop for SampleDriver {
      fn drop(self: Pin<&mut Self>) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust PCI driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
      }
  }
samples/rust/rust_driver_platform.rs +1 -1
index 9537dc38c563..f2229d176fb9 100644
@@ -180,7 +180,7 @@ fn properties_parse(dev: &device::Device) -> Result {
 
  impl Drop for SampleDriver {
      fn drop(&mut self) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust Platform driver sample.\n");
      }
  }
samples/rust/rust_i2c_client.rs +1 -4
index 8d2c12e535b0..72da5499f150 100644
@@ -113,10 +113,7 @@ fn probe(
          pdev: &platform::Device<device::Core>,
          _info: Option<&Self::IdInfo>,
      ) -> impl PinInit<Self, Error> {
-        dev_info!(
-            pdev.as_ref(),
-            "Probe Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Probe Rust I2C Client registration sample.\n");
 
          kernel::try_pin_init!( Self {
              parent_dev: pdev.into(),

samples/rust/rust_i2c_client.rs fn probe( · line 132

Quoted 10 lines
samples/rust/rust_i2c_client.rs +1 -4
@@ -130,10 +127,7 @@ fn probe(
      }
 
      fn unbind(pdev: &platform::Device<device::Core>, _this: Pin<&Self>) {
-        dev_info!(
-            pdev.as_ref(),
-            "Unbind Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Unbind Rust I2C Client registration sample.\n");
      }
  }

At least in samples/rust we have several places where we added helper variables for dev. For example in rust_driver_platform.rs [1] we have

let dev = pdev.as_ref();
dev_dbg!(dev, "Probe Rust Platform driver sample.\n");

Do we want to keep this?

I think there are places where this dev helper variable is used for printing, only. And there are places, like in the probe() of rust_driver_platform.rs, where the dev helper is additionally used elsewhere:

... Self::properties_parse(dev)?;

Hmm, I'm somehow under the impression what to use when in the end becomes slightly confusing in sum?

Best regards

Dirk

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/rust/rust_driver_platform.rs#n106

reviewer
#3 lore raw

Re: [PATCH 2/4] rust: samples: remove redundant `.as_ref()` for `dev_*` print

Quoted 161 lines · Dirk Behme

Hi Gary

Quoted 133 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
  samples/rust/rust_dma.rs              |  2 +-
  samples/rust/rust_driver_auxiliary.rs |  2 +-
  samples/rust/rust_driver_faux.rs      |  2 +-
  samples/rust/rust_driver_i2c.rs       |  4 ++--
  samples/rust/rust_driver_pci.rs       |  6 +++---
  samples/rust/rust_driver_platform.rs  |  2 +-
  samples/rust/rust_i2c_client.rs       | 10 ++--------
  7 files changed, 11 insertions(+), 17 deletions(-)
samples/rust/rust_dma.rs +1 -1
index f53bce2a73e3..a9ac7937ebaf 100644
@@ -57,7 +57,7 @@ impl pci::Driver for DmaSampleDriver {
 
      fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          pin_init::pin_init_scope(move || {
-            dev_info!(pdev.as_ref(), "Probe DMA test driver.\n");
+            dev_info!(pdev, "Probe DMA test driver.\n");
 
              let mask = DmaMask::new::<64>();
samples/rust/rust_driver_auxiliary.rs +1 -1
index f148124fe81f..c20961f16835 100644
@@ -39,7 +39,7 @@ impl auxiliary::Driver for AuxiliaryDriver {
 
      fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          dev_info!(
-            adev.as_ref(),
+            adev,
              "Probing auxiliary driver for auxiliary device with id={}\n",
              adev.id()
          );
samples/rust/rust_driver_faux.rs +1 -1
index 5330b77ea986..99876c8e3743 100644
@@ -26,7 +26,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
 
          let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
 
-        dev_info!(reg.as_ref(), "Hello from faux device!\n");
+        dev_info!(reg, "Hello from faux device!\n");
 
          Ok(Self { _reg: reg })
      }
samples/rust/rust_driver_i2c.rs +2 -2
index 6be79f9e9fb5..e71b77196163 100644
@@ -56,11 +56,11 @@ fn probe(
      }
 
      fn shutdown(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+        dev_info!(idev, "Shutdown Rust I2C driver sample.\n");
      }
 
      fn unbind(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Unbind Rust I2C driver sample.\n");
+        dev_info!(idev, "Unbind Rust I2C driver sample.\n");
      }
  }
samples/rust/rust_driver_pci.rs +3 -3
index ef04c6401e6a..d50828b642e5 100644
@@ -75,7 +75,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
          pin_init::pin_init_scope(move || {
              let vendor = pdev.vendor_id();
              dev_dbg!(
-                pdev.as_ref(),
+                pdev,
                  "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
                  vendor,
                  pdev.device_id()

@@ -91,7 +91,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
                      let bar = bar.access(pdev.as_ref())?;
 
                      dev_info!(
-                        pdev.as_ref(),
+                        pdev,
                          "pci-testdev data-match count: {}\n",
                          Self::testdev(info, bar)?
                      );

@@ -112,7 +112,7 @@ fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
  #[pinned_drop]
  impl PinnedDrop for SampleDriver {
      fn drop(self: Pin<&mut Self>) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust PCI driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
      }
  }
samples/rust/rust_driver_platform.rs +1 -1
index 9537dc38c563..f2229d176fb9 100644
@@ -180,7 +180,7 @@ fn properties_parse(dev: &device::Device) -> Result {
 
  impl Drop for SampleDriver {
      fn drop(&mut self) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust Platform driver sample.\n");
      }
  }
samples/rust/rust_i2c_client.rs +1 -4
index 8d2c12e535b0..72da5499f150 100644
@@ -113,10 +113,7 @@ fn probe(
          pdev: &platform::Device<device::Core>,
          _info: Option<&Self::IdInfo>,
      ) -> impl PinInit<Self, Error> {
-        dev_info!(
-            pdev.as_ref(),
-            "Probe Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Probe Rust I2C Client registration sample.\n");
 
          kernel::try_pin_init!( Self {
              parent_dev: pdev.into(),

samples/rust/rust_i2c_client.rs fn probe( · line 132

Quoted 10 lines
samples/rust/rust_i2c_client.rs +1 -4
@@ -130,10 +127,7 @@ fn probe(
      }
 
      fn unbind(pdev: &platform::Device<device::Core>, _this: Pin<&Self>) {
-        dev_info!(
-            pdev.as_ref(),
-            "Unbind Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Unbind Rust I2C Client registration sample.\n");
      }
  }

At least in samples/rust we have several places where we added helper variables for dev. For example in rust_driver_platform.rs [1] we have

let dev = pdev.as_ref();
dev_dbg!(dev, "Probe Rust Platform driver sample.\n");

Do we want to keep this?

I think there are places where this dev helper variable is used for printing, only. And there are places, like in the probe() of rust_driver_platform.rs, where the dev helper is additionally used elsewhere:

Quoted 4 lines

... Self::properties_parse(dev)?;

Hmm, I'm somehow under the impression what to use when in the end becomes slightly confusing in sum?

Hi Dirk,

If you already need to be obtain &Device, then I would say either is fine. The AsRef impl of devices are zero-cost operations anyway.

The motivation behind the patch is that in a lot of cases we only need &Device specifically to print, which is something that I want to avoid.

Best,
Gary

maintainer Acked-by on v1 2/4
#4 lore raw

Re: [PATCH 2/4] rust: samples: remove redundant `.as_ref()` for `dev_*` print

In reply to Gary Guo

Hi

Quoted 134 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
  samples/rust/rust_dma.rs              |  2 +-
  samples/rust/rust_driver_auxiliary.rs |  2 +-
  samples/rust/rust_driver_faux.rs      |  2 +-
  samples/rust/rust_driver_i2c.rs       |  4 ++--
  samples/rust/rust_driver_pci.rs       |  6 +++---
  samples/rust/rust_driver_platform.rs  |  2 +-
  samples/rust/rust_i2c_client.rs       | 10 ++--------
  7 files changed, 11 insertions(+), 17 deletions(-)
samples/rust/rust_dma.rs +1 -1
index f53bce2a73e3..a9ac7937ebaf 100644
@@ -57,7 +57,7 @@ impl pci::Driver for DmaSampleDriver {
 
      fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          pin_init::pin_init_scope(move || {
-            dev_info!(pdev.as_ref(), "Probe DMA test driver.\n");
+            dev_info!(pdev, "Probe DMA test driver.\n");
 
              let mask = DmaMask::new::<64>();
samples/rust/rust_driver_auxiliary.rs +1 -1
index f148124fe81f..c20961f16835 100644
@@ -39,7 +39,7 @@ impl auxiliary::Driver for AuxiliaryDriver {
 
      fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
          dev_info!(
-            adev.as_ref(),
+            adev,
              "Probing auxiliary driver for auxiliary device with id={}\n",
              adev.id()
          );
samples/rust/rust_driver_faux.rs +1 -1
index 5330b77ea986..99876c8e3743 100644
@@ -26,7 +26,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
 
          let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
 
-        dev_info!(reg.as_ref(), "Hello from faux device!\n");
+        dev_info!(reg, "Hello from faux device!\n");
 
          Ok(Self { _reg: reg })
      }
samples/rust/rust_driver_i2c.rs +2 -2
index 6be79f9e9fb5..e71b77196163 100644
@@ -56,11 +56,11 @@ fn probe(
      }
 
      fn shutdown(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+        dev_info!(idev, "Shutdown Rust I2C driver sample.\n");
      }
 
      fn unbind(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
-        dev_info!(idev.as_ref(), "Unbind Rust I2C driver sample.\n");
+        dev_info!(idev, "Unbind Rust I2C driver sample.\n");
      }
  }
samples/rust/rust_driver_pci.rs +3 -3
index ef04c6401e6a..d50828b642e5 100644
@@ -75,7 +75,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
          pin_init::pin_init_scope(move || {
              let vendor = pdev.vendor_id();
              dev_dbg!(
-                pdev.as_ref(),
+                pdev,
                  "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
                  vendor,
                  pdev.device_id()

@@ -91,7 +91,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
                      let bar = bar.access(pdev.as_ref())?;
 
                      dev_info!(
-                        pdev.as_ref(),
+                        pdev,
                          "pci-testdev data-match count: {}\n",
                          Self::testdev(info, bar)?
                      );

@@ -112,7 +112,7 @@ fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
  #[pinned_drop]
  impl PinnedDrop for SampleDriver {
      fn drop(self: Pin<&mut Self>) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust PCI driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust PCI driver sample.\n");
      }
  }
samples/rust/rust_driver_platform.rs +1 -1
index 9537dc38c563..f2229d176fb9 100644
@@ -180,7 +180,7 @@ fn properties_parse(dev: &device::Device) -> Result {
 
  impl Drop for SampleDriver {
      fn drop(&mut self) {
-        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
+        dev_dbg!(self.pdev, "Remove Rust Platform driver sample.\n");
      }
  }
samples/rust/rust_i2c_client.rs +1 -4
index 8d2c12e535b0..72da5499f150 100644
@@ -113,10 +113,7 @@ fn probe(
          pdev: &platform::Device<device::Core>,
          _info: Option<&Self::IdInfo>,
      ) -> impl PinInit<Self, Error> {
-        dev_info!(
-            pdev.as_ref(),
-            "Probe Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Probe Rust I2C Client registration sample.\n");
 
          kernel::try_pin_init!( Self {
              parent_dev: pdev.into(),

@@ -130,10 +127,7 @@ fn probe(
      }

samples/rust/rust_i2c_client.rs fn probe( · line 133

Quoted 10 lines
samples/rust/rust_i2c_client.rs +1 -4
 
      fn unbind(pdev: &platform::Device<device::Core>, _this: Pin<&Self>) {
-        dev_info!(
-            pdev.as_ref(),
-            "Unbind Rust I2C Client registration sample.\n"
-        );
+        dev_info!(pdev, "Unbind Rust I2C Client registration sample.\n");
      }
  }
Acked-by Igor Korotin <[email protected]>

Thanks
Igor

author
#5 lore raw

PATCH 3/4 gpu: nova-core: remove redundant `.as_ref()` for `dev_*` print

In reply to Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
 drivers/gpu/nova-core/driver.rs   |  2 +-
 drivers/gpu/nova-core/gpu.rs      |  4 ++--
 drivers/gpu/nova-core/gsp/boot.rs | 32 +++++++------------------------
 3 files changed, 10 insertions(+), 28 deletions(-)
drivers/gpu/nova-core/driver.rs +1 -1
index 5a4cc047bcfc..e39885c0d5ca 100644
@@ -70,7 +70,7 @@ impl pci::Driver for NovaCore {
 
     fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         pin_init::pin_init_scope(move || {
-            dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
+            dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
 
             pdev.enable_device_mem()?;
             pdev.set_master();
drivers/gpu/nova-core/gpu.rs +2 -2
index 9b042ef1a308..60c85fffaeaf 100644
@@ -262,13 +262,13 @@ pub(crate) fn new<'a>(
     ) -> impl PinInit<Self, Error> + 'a {
         try_pin_init!(Self {
             spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
-                dev_info!(pdev.as_ref(),"NVIDIA ({})\n", spec);
+                dev_info!(pdev,"NVIDIA ({})\n", spec);
             })?,
 
             // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
             _: {
                 gfw::wait_gfw_boot_completion(bar)
-                    .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did not complete\n"))?;
+                    .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;
             },
 
             sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
drivers/gpu/nova-core/gsp/boot.rs +7 -25
index 581b412554dc..1582e1a65274 100644
@@ -170,15 +170,10 @@ pub(crate) fn boot(
             Some(libos_handle as u32),
             Some((libos_handle >> 32) as u32),
         )?;
-        dev_dbg!(
-            pdev.as_ref(),
-            "GSP MBOX0: {:#x}, MBOX1: {:#x}\n",
-            mbox0,
-            mbox1
-        );
+        dev_dbg!(pdev, "GSP MBOX0: {:#x}, MBOX1: {:#x}\n", mbox0, mbox1);
 
         dev_dbg!(
-            pdev.as_ref(),
+            pdev,
             "Using SEC2 to load and run the booter_load firmware...\n"
         );
 

@@ -190,19 +185,10 @@ pub(crate) fn boot(
             Some(wpr_handle as u32),
             Some((wpr_handle >> 32) as u32),
         )?;
-        dev_dbg!(
-            pdev.as_ref(),
-            "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n",
-            mbox0,
-            mbox1
-        );
+        dev_dbg!(pdev, "SEC2 MBOX0: {:#x}, MBOX1{:#x}\n", mbox0, mbox1);
 
         if mbox0 != 0 {
-            dev_err!(
-                pdev.as_ref(),
-                "Booter-load failed with error {:#x}\n",
-                mbox0
-            );
+            dev_err!(pdev, "Booter-load failed with error {:#x}\n", mbox0);
             return Err(ENODEV);
         }
 

@@ -216,11 +202,7 @@ pub(crate) fn boot(
             Delta::from_secs(5),
         )?;
 
-        dev_dbg!(
-            pdev.as_ref(),
-            "RISC-V active? {}\n",
-            gsp_falcon.is_riscv_active(bar),
-        );
+        dev_dbg!(pdev, "RISC-V active? {}\n", gsp_falcon.is_riscv_active(bar),);
 
         // Create and run the GSP sequencer.
         let seq_params = GspSequencerParams {

@@ -239,8 +221,8 @@ pub(crate) fn boot(
         // Obtain and display basic GPU information.
         let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
         match info.gpu_name() {
-            Ok(name) => dev_info!(pdev.as_ref(), "GPU name: {}\n", name),
-            Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: {:?}\n", e),
+            Ok(name) => dev_info!(pdev, "GPU name: {}\n", name),
+            Err(e) => dev_warn!(pdev, "GPU name unavailable: {:?}\n", e),
         }
 
         Ok(())
Signature · 1 line
2.51.2
author
#6 lore raw

PATCH 4/4 gpu: tyr: remove redundant `.as_ref()` for `dev_*` print

In reply to Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
---
 drivers/gpu/drm/tyr/driver.rs | 2 +-
 drivers/gpu/drm/tyr/gpu.rs    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/tyr/driver.rs +1 -1
index 2a45d0288825..fe991d8cbb4a 100644
@@ -140,7 +140,7 @@ fn probe(
 
         // We need this to be dev_info!() because dev_dbg!() does not work at
         // all in Rust for now, and we need to see whether probe succeeded.
-        dev_info!(pdev.as_ref(), "Tyr initialized correctly.\n");
+        dev_info!(pdev, "Tyr initialized correctly.\n");
         Ok(driver)
     }
 }
drivers/gpu/drm/tyr/gpu.rs +3 -3
index bfbf2a1d80e6..04a0a5d5ef3a 100644
@@ -98,7 +98,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
         };
 
         dev_info!(
-            pdev.as_ref(),
+            pdev,
             "mali-{} id 0x{:x} major 0x{:x} minor 0x{:x} status 0x{:x}",
             model_name,
             self.gpu_id >> 16,

@@ -108,7 +108,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
         );
 
         dev_info!(
-            pdev.as_ref(),
+            pdev,
             "Features: L2:{:#x} Tiler:{:#x} Mem:{:#x} MMU:{:#x} AS:{:#x}",
             self.l2_features,
             self.tiler_features,

@@ -118,7 +118,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
         );
 
         dev_info!(
-            pdev.as_ref(),
+            pdev,
             "shader_present=0x{:016x} l2_present=0x{:016x} tiler_present=0x{:016x}",
             self.shader_present,
             self.l2_present,
Signature · 1 line
2.51.2
maintainer Reviewed-by on v1 4/4
#7 lore raw

Re: [PATCH 4/4] gpu: tyr: remove redundant `.as_ref()` for `dev_*` print

Quoted 62 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>

---
drivers/gpu/drm/tyr/driver.rs | 2 +-
drivers/gpu/drm/tyr/gpu.rs | 6 +++---

2 files changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/tyr/driver.rs +1 -1
index 2a45d0288825..fe991d8cbb4a 100644
@@ -140,7 +140,7 @@ fn probe(
 
        // We need this to be dev_info!() because dev_dbg!() does not work at
        // all in Rust for now, and we need to see whether probe succeeded.
-        dev_info!(pdev.as_ref(), "Tyr initialized correctly.\n");
+        dev_info!(pdev, "Tyr initialized correctly.\n");
        Ok(driver)
    }
 }
drivers/gpu/drm/tyr/gpu.rs +3 -3
index bfbf2a1d80e6..04a0a5d5ef3a 100644
@@ -98,7 +98,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
        };
 
        dev_info!(
-            pdev.as_ref(),
+            pdev,
            "mali-{} id 0x{:x} major 0x{:x} minor 0x{:x} status 0x{:x}",
            model_name,
            self.gpu_id >> 16,

@@ -108,7 +108,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
        );
 
        dev_info!(
-            pdev.as_ref(),
+            pdev,
            "Features: L2:{:#x} Tiler:{:#x} Mem:{:#x} MMU:{:#x} AS:{:#x}",
            self.l2_features,
            self.tiler_features,

@@ -118,7 +118,7 @@ pub(crate) fn log(&self, pdev: &platform::Device) {
        );
 
        dev_info!(
-            pdev.as_ref(),
+            pdev,
            "shader_present=0x{:016x} l2_present=0x{:016x} tiler_present=0x{:016x}",
            self.shader_present,
            self.l2_present,
Signature · 1 line
2.51.2
Reviewed-by Daniel Almeida <[email protected]>
maintainer
#9 lore raw

Re: [PATCH 4/4] gpu: tyr: remove redundant `.as_ref()` for `dev_*` print

Quoted 8 lines · Alice Ryhl
Quoted 5 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Signed-off-by Gary Guo <[email protected]>
Acked-by Alice Ryhl <[email protected]>

Given your ACK, do you want me to take this one through driver-core for this cycle? For the nova-core one I will wait for -rc1 to be out and take it through the drm-rust tree to avoid conflicts, Tyr should be less prone to conflicts though.

maintainer
#10 lore raw

Re: [PATCH 4/4] gpu: tyr: remove redundant `.as_ref()` for `dev_*` print

Quoted 7 lines · Danilo Krummrich
Quoted 5 lines · Alice Ryhl
Quoted 4 lines · Gary Guo

From: Gary Guo <[email protected]>

This is now handled by the macro itself.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Gary Guo <[email protected]>
Acked-by Alice Ryhl <[email protected]>

Given your ACK, do you want me to take this one through driver-core for this cycle? For the nova-core one I will wait for -rc1 to be out and take it through the drm-rust tree to avoid conflicts, Tyr should be less prone to conflicts though.

I don't mind either way, but if there are no conflicts, it would be nice to take it off our plate now.

Alice

maintainer
#11 lore raw

Re: [PATCH 4/4] gpu: tyr: remove redundant `.as_ref()` for `dev_*` print

Quoted 2 lines · Alice Ryhl

I don't mind either way, but if there are no conflicts, it would be nice to take it off our plate now.

Applied to driver-core-testing, thanks!

maintainer
#12 lore raw
In reply to Gary Guo
Quoted 19 lines · Gary Guo
rust/kernel/device.rs +7 -0
index c7b5db9dcca1..94e0548e7687 100644
@@ -601,6 +601,13 @@ impl DeviceContext for Core {}
 impl DeviceContext for CoreInternal {}
 impl DeviceContext for Normal {}
 
+impl<Ctx: DeviceContext> AsRef<Device<Ctx>> for Device<Ctx> {
+    #[inline]
+    fn as_ref(&self) -> &Device<Ctx> {
+        self
+    }
+}
+
 /// Convert device references to bus device references.
 ///
 /// Bus devices can implement this trait to allow abstractions to provide the bus device in

rust/kernel/device.rs macro_rules! impl_device_context_into_aref { · line 730

Quoted 5 lines
rust/kernel/device.rs +1 -1
@@ -720,7 +727,7 @@ macro_rules! impl_device_context_into_aref {
 macro_rules! dev_printk {
     ($method:ident, $dev:expr, $($f:tt)*) => {
         {
-            ($dev).$method($crate::prelude::fmt!($($f)*));
+            $crate::device::Device::$method($dev.as_ref(), $crate::prelude::fmt!($($f)*))

At a first glance I wasn't sure if we really want to rely on AsRef when hide it in the macro. But on the other hand, if someone would implement AsRef on a class or bus device to return some other (unrelated) &Device it would be wrong anyways. So I think using AsRef is fine.

I think a lot of people will be very happy about this patch. :)

I will pick up patches 1 and 2 this cycle, patch 3 and 4 can go through the corresponding subsystem tree next cycle.

Quoted 18 lines
rust/kernel/pci.rs +1 -1
index bea76ca9c3da..3946ca919332 100644
@@ -351,7 +351,7 @@ impl Device {
     ///     // Get an instance of `Vendor`.
     ///     let vendor = pdev.vendor_id();
     ///     dev_info!(
-    ///         pdev.as_ref(),
+    ///         pdev,
     ///         "Device: Vendor={}, Device=0x{:x}\n",
     ///         vendor,
     ///         pdev.device_id()
rust/kernel/pci/id.rs +0 -0
index c09125946d9e..e2d9e8804347 100644

rust/kernel/pci/id.rs line 28

Quoted 8 lines
rust/kernel/pci/id.rs +1 -1
@@ -22,7 +22,7 @@
 /// fn probe_device(pdev: &pci::Device<Core>) -> Result {
 ///     let pci_class = pdev.pci_class();
 ///     dev_info!(
-///         pdev.as_ref(),
+///         pdev,
 ///         "Detected PCI class: {}\n",
 ///         pci_class
 ///     );

Can you please send a separate patch for those?

reviewer
#13 lore raw
Quoted 31 lines · Danilo Krummrich
Quoted 19 lines · Gary Guo
rust/kernel/device.rs +7 -0
index c7b5db9dcca1..94e0548e7687 100644
@@ -601,6 +601,13 @@ impl DeviceContext for Core {}
 impl DeviceContext for CoreInternal {}
 impl DeviceContext for Normal {}
 
+impl<Ctx: DeviceContext> AsRef<Device<Ctx>> for Device<Ctx> {
+    #[inline]
+    fn as_ref(&self) -> &Device<Ctx> {
+        self
+    }
+}
+
 /// Convert device references to bus device references.
 ///
 /// Bus devices can implement this trait to allow abstractions to provide the bus device in

rust/kernel/device.rs macro_rules! impl_device_context_into_aref { · line 730

Quoted 5 lines
rust/kernel/device.rs +1 -1
@@ -720,7 +727,7 @@ macro_rules! impl_device_context_into_aref {
 macro_rules! dev_printk {
     ($method:ident, $dev:expr, $($f:tt)*) => {
         {
-            ($dev).$method($crate::prelude::fmt!($($f)*));
+            $crate::device::Device::$method($dev.as_ref(), $crate::prelude::fmt!($($f)*))

At a first glance I wasn't sure if we really want to rely on AsRef when hide it in the macro. But on the other hand, if someone would implement AsRef on a class or bus device to return some other (unrelated) &Device it would be wrong anyways. So I think using AsRef is fine.

Quoted 1 line

I think a lot of people will be very happy about this patch. :)

^ this is basically me getting annoyed by everything being pdev.as_ref() when reviewing code and want to fix the pain :)

Quoted 24 lines

I will pick up patches 1 and 2 this cycle, patch 3 and 4 can go through the corresponding subsystem tree next cycle.

Quoted 20 lines
rust/kernel/pci.rs +1 -1
index bea76ca9c3da..3946ca919332 100644
@@ -351,7 +351,7 @@ impl Device {
     ///     // Get an instance of `Vendor`.
     ///     let vendor = pdev.vendor_id();
     ///     dev_info!(
-    ///         pdev.as_ref(),
+    ///         pdev,
     ///         "Device: Vendor={}, Device=0x{:x}\n",
     ///         vendor,
     ///         pdev.device_id()
rust/kernel/pci/id.rs +0 -0
index c09125946d9e..e2d9e8804347 100644
@@ -22,7 +22,7 @@
 /// fn probe_device(pdev: &pci::Device<Core>) -> Result {
 ///     let pci_class = pdev.pci_class();
Quoted 8 lines

rust/kernel/pci/id.rs line 28

Quoted 6 lines
rust/kernel/pci/id.rs +1 -1
 ///     dev_info!(
-///         pdev.as_ref(),
+///         pdev,
 ///         "Detected PCI class: {}\n",
 ///         pci_class
 ///     );

Can you please send a separate patch for those?

I converted them as they're the users that benefit from this behaviour in the kernel crate (i.e. "convert the users"). Do you want them separate so you can take it via a different tree?

Best,
Gary

maintainer
#14 lore raw
Quoted 3 lines · Gary Guo

I converted them as they're the users that benefit from this behaviour in the kernel crate (i.e. "convert the users"). Do you want them separate so you can take it via a different tree?

No it eventually goes through the same tree, but in general patches should be split up per subsystem / component / driver if possible.

For instance, the split should go as follows:

1. DMA
- samples/rust/rust_dma.rs

2. DRIVER CORE
- samples/rust/rust_driver_auxiliary.rs
- samples/rust/rust_driver_faux.rs
- samples/rust/rust_driver_platform.rs

3. PCI
- rust/kernel/pci.rs
- samples/rust/rust_driver_pci.rs

4. I2C
- samples/rust/rust_driver_i2c.rs
- samples/rust/rust_i2c_client.rs