[PATCH] drm/panic: use `///` for private items too

[PATCH] drm/panic: use `///` for private items too

[Miguel Ojeda]

`///` 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.

Link: https://lore.kernel.org/rust-for-linux/20250416112454.2503872-1-ojeda@kernel.org/ [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.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

Re: [PATCH] drm/panic: use `///` for private items too

[Miguel Ojeda]

On Wed, Apr 16, 2025 at 2:21 PM Miguel Ojeda <ojeda@kernel.org> 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.
>
> Link: https://lore.kernel.org/rust-for-linux/20250416112454.2503872-1-ojeda@kernel.org/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Sorry, I forgot the Cc: Jocelyn (it could be nice to have a
`MAINTAINERS` entry for this one).

Cheers,
Miguel

Re: [PATCH] drm/panic: use `///` for private items too

[Jocelyn Falempe]

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@redhat.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@kernel.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
> 

Re: [PATCH] drm/panic: use `///` for private items too

[Christian Schrefl]

On 16.04.25 2:21 PM, 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.
> 
> Link: https://lore.kernel.org/rust-for-linux/20250416112454.2503872-1-ojeda@kernel.org/ [1]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> Not sure if you would consider it a fix, but please feel free to add Fixes: etc.
> if so. Thanks!
> 

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Re: [PATCH] drm/panic: use `///` for private items too

[Jocelyn Falempe]

On 16/04/2025 14:24, Miguel Ojeda wrote:
> On Wed, Apr 16, 2025 at 2:21 PM Miguel Ojeda <ojeda@kernel.org> 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.

I just pushed it to drm-misc-next

Thanks,

-- 

Jocelyn



>>
>> Link: https://lore.kernel.org/rust-for-linux/20250416112454.2503872-1-ojeda@kernel.org/ [1]
>> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> 
> Sorry, I forgot the Cc: Jocelyn (it could be nice to have a
> `MAINTAINERS` entry for this one).
> 
> Cheers,
> Miguel
>