[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7e54be7d-9c67-45ad-819c-fba6d2da16fc@redhat.com>
Date: Wed, 16 Apr 2025 15:18:24 +0200
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Miguel Ojeda <ojeda@...nel.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Alex Gaynor <alex.gaynor@...il.com>
Cc: dri-devel@...ts.freedesktop.org, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
patches@...ts.linux.dev
Subject: Re: [PATCH] drm/panic: use `///` for private items too
On 16/04/2025 14:21, Miguel Ojeda wrote:
> `///` should still be used for private items [1]. Some of the items in
> this file do so already, so do it for a few other clear candidates in
> the file.
Thanks, it looks good to me.
As it's only comments, I think it's not worth to mark it as a fix, and
to backport it to older releases.
I plan to push it to drm-misc-next tomorrow.
Reviewed-by: Jocelyn Falempe <jfalempe@...hat.com>
Best regards,
--
Jocelyn
>
> Link: https://lore.kernel.org/rust-for-linux/20250416112454.2503872-1-ojeda@kernel.org/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---
> Not sure if you would consider it a fix, but please feel free to add Fixes: etc.
> if so. Thanks!
>
> drivers/gpu/drm/drm_panic_qr.rs | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
> index f2a99681b998..642e3240a014 100644
> --- a/drivers/gpu/drm/drm_panic_qr.rs
> +++ b/drivers/gpu/drm/drm_panic_qr.rs
> @@ -315,7 +315,7 @@ fn get_header(&self) -> (u16, usize) {
> }
> }
>
> - // Returns the size of the length field in bits, depending on QR Version.
> + /// Returns the size of the length field in bits, depending on QR Version.
> fn length_bits_count(&self, version: Version) -> usize {
> let Version(v) = version;
> match self {
> @@ -331,7 +331,7 @@ fn length_bits_count(&self, version: Version) -> usize {
> }
> }
>
> - // Number of characters in the segment.
> + /// Number of characters in the segment.
> fn character_count(&self) -> usize {
> match self {
> Segment::Binary(data) => data.len(),
> @@ -569,8 +569,8 @@ struct EncodedMsgIterator<'a> {
> impl Iterator for EncodedMsgIterator<'_> {
> type Item = u8;
>
> - // Send the bytes in interleaved mode, first byte of first block of group1,
> - // then first byte of second block of group1, ...
> + /// Send the bytes in interleaved mode, first byte of first block of group1,
> + /// then first byte of second block of group1, ...
> fn next(&mut self) -> Option<Self::Item> {
> let em = self.em;
> let blocks = em.g1_blocks + em.g2_blocks;
> @@ -638,7 +638,7 @@ fn clear(&mut self) {
> self.data.fill(0);
> }
>
> - // Set pixel to light color.
> + /// Set pixel to light color.
> fn set(&mut self, x: u8, y: u8) {
> let off = y as usize * self.stride as usize + x as usize / 8;
> let mut v = self.data[off];
> @@ -646,13 +646,13 @@ fn set(&mut self, x: u8, y: u8) {
> self.data[off] = v;
> }
>
> - // Invert a module color.
> + /// Invert a module color.
> fn xor(&mut self, x: u8, y: u8) {
> let off = y as usize * self.stride as usize + x as usize / 8;
> self.data[off] ^= 0x80 >> (x % 8);
> }
>
> - // Draw a light square at (x, y) top left corner.
> + /// Draw a light square at (x, y) top left corner.
> fn draw_square(&mut self, x: u8, y: u8, size: u8) {
> for k in 0..size {
> self.set(x + k, y);
> @@ -784,7 +784,7 @@ fn is_version_info(&self, x: u8, y: u8) -> bool {
> vinfo != 0 && ((x >= pos && x < pos + 3 && y < 6) || (y >= pos && y < pos + 3 && x < 6))
> }
>
> - // Returns true if the module is reserved (Not usable for data and EC).
> + /// Returns true if the module is reserved (Not usable for data and EC).
> fn is_reserved(&self, x: u8, y: u8) -> bool {
> self.is_alignment(x, y)
> || self.is_finder(x, y)
> @@ -793,13 +793,14 @@ fn is_reserved(&self, x: u8, y: u8) -> bool {
> || self.is_version_info(x, y)
> }
>
> - // Last module to draw, at bottom left corner.
> + /// Last module to draw, at bottom left corner.
> fn is_last(&self, x: u8, y: u8) -> bool {
> x == 0 && y == self.width - 1
> }
>
> - // Move to the next module according to QR code order.
> - // From bottom right corner, to bottom left corner.
> + /// Move to the next module according to QR code order.
> + ///
> + /// From bottom right corner, to bottom left corner.
> fn next(&self, x: u8, y: u8) -> (u8, u8) {
> let x_adj = if x <= 6 { x + 1 } else { x };
> let column_type = (self.width - x_adj) % 4;
> @@ -812,7 +813,7 @@ fn next(&self, x: u8, y: u8) -> (u8, u8) {
> }
> }
>
> - // Find next module that can hold data.
> + /// Find next module that can hold data.
> fn next_available(&self, x: u8, y: u8) -> (u8, u8) {
> let (mut x, mut y) = self.next(x, y);
> while self.is_reserved(x, y) && !self.is_last(x, y) {
> @@ -841,7 +842,7 @@ fn draw_data(&mut self, data: impl Iterator<Item = u8>) {
> }
> }
>
> - // Apply checkerboard mask to all non-reserved modules.
> + /// Apply checkerboard mask to all non-reserved modules.
> fn apply_mask(&mut self) {
> for x in 0..self.width {
> for y in 0..self.width {
> @@ -852,7 +853,7 @@ fn apply_mask(&mut self) {
> }
> }
>
> - // Draw the QR code with the provided data iterator.
> + /// Draw the QR code with the provided data iterator.
> fn draw_all(&mut self, data: impl Iterator<Item = u8>) {
> // First clear the table, as it may have already some data.
> self.clear();
>
> base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
> --
> 2.49.0
>
Powered by blists - more mailing lists