On 1/6/26 5:24 PM, Timur Tabi wrote: > Although the dev_xx!() macro calls do not technically require terminating > newlines for the format strings, they should be added any way to maintain > consistency, both within Rust code and with the C versions.
I'd *tentatively* prefer to *omit* the \n's, because they are stripped out (and then re-added) as part of the printing call stack, and so it seems nice to have less dead code on the screen. On the other hand, the output always gets a newline, so seeing it on screen is actually informative in a way. And yes consistency is important. But which way to head toward? I checked the Rust for Linux code in drm-rust-next, and we have: Codebase WITH \n WITHOUT \n % with \n ---------------------------------------------------------------- samples/rust (dev_*!) 30 10 75% samples/rust (pr_*!) 20 7 74% nova-core (dev_*!) [after patch] 39 32 55% nova-core (dev_*!) [before patch] 32 39 45% nova (dev_*!) 0 0 -- tyr (dev_*!) 3 5 38% pwm (dev_*!) 3 6 33% binder (pr_*!) 17 62 22% So there is not yet a clear convention visible in the code. Miguel, Alice, Gary et al, is there actually a preference already? thanks, -- John Hubbard > > Signed-off-by: Timur Tabi <[email protected]> > --- > drivers/gpu/nova-core/falcon.rs | 6 +++--- > drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++-- > drivers/gpu/nova-core/gpu.rs | 2 +- > drivers/gpu/nova-core/gsp/sequencer.rs | 6 +++--- > drivers/gpu/nova-core/vbios.rs | 2 +- > 5 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs > index fe5abf64dd2b..c1cb31c856b5 100644 > --- a/drivers/gpu/nova-core/falcon.rs > +++ b/drivers/gpu/nova-core/falcon.rs > @@ -466,7 +466,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>( > if dma_start % DmaAddress::from(DMA_LEN) > 0 { > dev_err!( > self.dev, > - "DMA transfer start addresses must be a multiple of {}", > + "DMA transfer start addresses must be a multiple of {}\n", > DMA_LEN > ); > return Err(EINVAL); > @@ -483,11 +483,11 @@ fn dma_wr<F: FalconFirmware<Target = E>>( > .and_then(|size| size.checked_add(load_offsets.src_start)) > { > None => { > - dev_err!(self.dev, "DMA transfer length overflow"); > + dev_err!(self.dev, "DMA transfer length overflow\n"); > return Err(EOVERFLOW); > } > Some(upper_bound) if usize::from_safe_cast(upper_bound) > > fw.size() => { > - dev_err!(self.dev, "DMA transfer goes beyond range of DMA > object"); > + dev_err!(self.dev, "DMA transfer goes beyond range of DMA > object\n"); > return Err(EINVAL); > } > Some(_) => (), > diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs > b/drivers/gpu/nova-core/falcon/hal/ga102.rs > index 69a7a95cac16..0bdfe45a2d03 100644 > --- a/drivers/gpu/nova-core/falcon/hal/ga102.rs > +++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs > @@ -52,7 +52,7 @@ fn signature_reg_fuse_version_ga102( > let ucode_idx = match usize::from(ucode_id) { > ucode_id @ 1..=regs::NV_FUSE_OPT_FPF_SIZE => ucode_id - 1, > _ => { > - dev_err!(dev, "invalid ucode id {:#x}", ucode_id); > + dev_err!(dev, "invalid ucode id {:#x}\n", ucode_id); > return Err(EINVAL); > } > }; > @@ -66,7 +66,7 @@ fn signature_reg_fuse_version_ga102( > } else if engine_id_mask & 0x0400 != 0 { > regs::NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION::read(bar, ucode_idx).data() > } else { > - dev_err!(dev, "unexpected engine_id_mask {:#x}", engine_id_mask); > + dev_err!(dev, "unexpected engine_id_mask {:#x}\n", engine_id_mask); > return Err(EINVAL); > }; > > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs > index 50d76092fbdd..9b042ef1a308 100644 > --- a/drivers/gpu/nova-core/gpu.rs > +++ b/drivers/gpu/nova-core/gpu.rs > @@ -268,7 +268,7 @@ pub(crate) fn new<'a>( > // 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"))?; > + .inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did > not complete\n"))?; > }, > > sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, > spec.chipset)?, > diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs > b/drivers/gpu/nova-core/gsp/sequencer.rs > index d78a30fbb70f..d965ffe9ba89 100644 > --- a/drivers/gpu/nova-core/gsp/sequencer.rs > +++ b/drivers/gpu/nova-core/gsp/sequencer.rs > @@ -121,7 +121,7 @@ pub(crate) fn new(data: &[u8], dev: &device::Device) -> > Result<(Self, usize)> { > }; > > if data.len() < size { > - dev_err!(dev, "Data is not enough for command"); > + dev_err!(dev, "Data is not enough for command\n"); > return Err(EINVAL); > } > > @@ -320,7 +320,7 @@ fn next(&mut self) -> Option<Self::Item> { > > cmd_result.map_or_else( > |_err| { > - dev_err!(self.dev, "Error parsing command at offset {}", > offset); > + dev_err!(self.dev, "Error parsing command at offset {}\n", > offset); > None > }, > |(cmd, size)| { > @@ -382,7 +382,7 @@ pub(crate) fn run(cmdq: &mut Cmdq, params: > GspSequencerParams<'a>) -> Result { > dev: params.dev, > }; > > - dev_dbg!(sequencer.dev, "Running CPU Sequencer commands"); > + dev_dbg!(sequencer.dev, "Running CPU Sequencer commands\n"); > > for cmd_result in sequencer.iter() { > match cmd_result { > diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs > index 7c26e4a2d61c..e4eae9385f47 100644 > --- a/drivers/gpu/nova-core/vbios.rs > +++ b/drivers/gpu/nova-core/vbios.rs > @@ -790,7 +790,7 @@ fn falcon_data_ptr(&self) -> Result<u32> { > // read the 4 bytes at the offset specified in the token > let offset = usize::from(token.data_offset); > let bytes: [u8; 4] = self.base.data[offset..offset + > 4].try_into().map_err(|_| { > - dev_err!(self.base.dev, "Failed to convert data slice to array"); > + dev_err!(self.base.dev, "Failed to convert data slice to > array\n"); > EINVAL > })?; >
