Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Helge Deller @ 2026-03-19 12:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jason Yan, Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abu9DKMN660yd3Sd@ashevche-desk.local>

On 3/19/26 10:08, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
>> On 3/19/26 09:21, Andy Shevchenko wrote:
>>> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>>>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> 
> ...
> 
>>>>> Helge, can we revert this change and start over, please? (I can also send
>>>>> revert if you think it's a better way)
>>
>> Andy, all points you make against removing relevant code is absolutely right.
>>
>> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
>> set but not used variable") I have to agree with Jason, that the patch is ok.
>> I don't see any build errors either.
> 
> Just run on today's Linux Next (since the driver has not been changed in that
> sense for a few years, the very same issue is present for a long time):
> 
> drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
>    412 |                                 unsigned int mnp;
>        |                                              ^
> 1 error generated.

Ok.

  
>> Are we mixing up things here maybe?
> 
> ...
> 
> FWIW, I dove to the history of the driver.
> 
> So, the code, that was guarded by #if 0 was introduced in the original commit
> 213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded in the
> commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on G450/G550.
> Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC from older
> DAC1064 handling. Modify PLL setting when both CRTCs use same pixel clocks.").
> 
> Even without that guard the modern compilers may see that the pixel_vco wasn't
> ever used and seems a leftover after some debug or review made 25 years ago.
> 
> The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem to have
> any side effects either than some unneeded CPU processing during runtime. I
> agree that's unlikely that timeout (or heating up the CPU) has any effect on
> the HW (GPU/display) functionality.
> 
> So, since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
> and set but not used variable") the 'mnp' became unused, but eliminating that
> code might have side-effects. The question here is what should we do with mnp?
> The easiest way out is just mark it with __maybe_unused which will shut the
> compiler up and won't change any possible IO flow.

Yes, I think that's what I'd prefer.
Do you mind sending a patch?

Helge

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Gary Guo @ 2026-03-19 12:21 UTC (permalink / raw)
  To: Danilo Krummrich, Gary Guo
  Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH6QUO2T941E.2S1UP7EABOP42@kernel.org>

On Thu Mar 19, 2026 at 12:05 PM GMT, Danilo Krummrich wrote:
> On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
>> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>>> Add a new module `kernel::interop::list` for working with C's doubly
>>> circular linked lists. Provide low-level iteration over list nodes.
>>>
>>> Typed iteration over actual items is provided with a `clist_create`
>>> macro to assist in creation of the `CList` type.
>>>
>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Gary Guo <gary@garyguo.net>
>>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>>  MAINTAINERS                 |   8 +
>>>  rust/helpers/helpers.c      |   1 +
>>>  rust/helpers/list.c         |  17 ++
>>>  rust/kernel/interop.rs      |   9 +
>>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>>  rust/kernel/lib.rs          |   2 +
>>>  6 files changed, 379 insertions(+)
>>>  create mode 100644 rust/helpers/list.c
>>>  create mode 100644 rust/kernel/interop.rs
>>>  create mode 100644 rust/kernel/interop/list.rs
>>>
>>> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
>>> +///
>>> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
>>> +/// linked via the `$field` field in the underlying C struct `$c_type`.
>>> +///
>>> +/// # Arguments
>>> +///
>>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>>> +/// - `$rust_type`: Each item's rust wrapper type.
>>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>>> +/// - `$field`: The name of the `list_head` field within the C struct.
>>> +///
>>> +/// # Safety
>>> +///
>>> +/// The caller must ensure:
>>> +///
>>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
>>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
>>> +#[macro_export]
>>> +macro_rules! clist_create {
>>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>>> +        // Compile-time check that field path is a `list_head`.
>>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>>> +            |p| unsafe { &raw const (*p).$($field).+ };
>>
>> Actually, this check is insufficient, you should create a reference instead
>> (just in case people put this inside `repr(packed)`.
>>
>> This could be something like
>>
>>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>>
>> ?
>>
>>> +
>>> +        // Calculate offset and create `CList`.
>>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>>
>> Given that this is unsafe, I am not sure why the macro should have unsafe
>> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
>> user write unsafe.
>
> Either you are proposing to not wrap unsafe code within unsafe {} within the
> macro, such that the user is forced to write an unsafe {} around the macro, but
> then they calls within the macro are not justified individually, or you propose
> to let the user write an unsafe {} around the macro regardless of the inner
> unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
> and nothing forces the user to actually wrap it in unsafe {}.

The former.

"The caller of this macro is responsible for ensuring safety" justification is
not really useful here IMO.

If there're cases where we do want to justify unsafe code that's not immediately
deferring to the user inside the macro, we could use the SAFETY* trick proposed
in the thread, without writing an actual `unsafe {}` block.

>
> Is there a third option I'm not aware of? I.e. for the above reason
> impl_device_context_deref!() was designed the same way.

impl_device_context_deref!() expands to an item, so the user couldn't put an
`unsafe {}` on the outside. This macro expands to an expression, so users can
add `unsafe` themselves.

Best,
Gary

>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650


^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Danilo Krummrich @ 2026-03-19 12:05 UTC (permalink / raw)
  To: Gary Guo
  Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Alex Gaynor, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH6QAR1HHXRV.1Y7IZ22HC9FZ3@garyguo.net>

On Thu Mar 19, 2026 at 12:39 PM CET, Gary Guo wrote:
> On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
>> Add a new module `kernel::interop::list` for working with C's doubly
>> circular linked lists. Provide low-level iteration over list nodes.
>>
>> Typed iteration over actual items is provided with a `clist_create`
>> macro to assist in creation of the `CList` type.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Gary Guo <gary@garyguo.net>
>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  MAINTAINERS                 |   8 +
>>  rust/helpers/helpers.c      |   1 +
>>  rust/helpers/list.c         |  17 ++
>>  rust/kernel/interop.rs      |   9 +
>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs          |   2 +
>>  6 files changed, 379 insertions(+)
>>  create mode 100644 rust/helpers/list.c
>>  create mode 100644 rust/kernel/interop.rs
>>  create mode 100644 rust/kernel/interop/list.rs
>>
>> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
>> +///
>> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
>> +/// linked via the `$field` field in the underlying C struct `$c_type`.
>> +///
>> +/// # Arguments
>> +///
>> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
>> +/// - `$rust_type`: Each item's rust wrapper type.
>> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
>> +/// - `$field`: The name of the `list_head` field within the C struct.
>> +///
>> +/// # Safety
>> +///
>> +/// The caller must ensure:
>> +///
>> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
>> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
>> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
>> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
>> +///
>> +/// # Examples
>> +///
>> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
>> +#[macro_export]
>> +macro_rules! clist_create {
>> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
>> +        // Compile-time check that field path is a `list_head`.
>> +        // SAFETY: `p` is a valid pointer to `$c_type`.
>> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
>> +            |p| unsafe { &raw const (*p).$($field).+ };
>
> Actually, this check is insufficient, you should create a reference instead
> (just in case people put this inside `repr(packed)`.
>
> This could be something like
>
>     let _ = |p: &$c_type| { _ = &p.$($field).+ }
>
> ?
>
>> +
>> +        // Calculate offset and create `CList`.
>> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
>> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
>
> Given that this is unsafe, I am not sure why the macro should have unsafe
> keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
> user write unsafe.

Either you are proposing to not wrap unsafe code within unsafe {} within the
macro, such that the user is forced to write an unsafe {} around the macro, but
then they calls within the macro are not justified individually, or you propose
to let the user write an unsafe {} around the macro regardless of the inner
unsafe {} blocks, but then then the compiler warns about an unnecessary unsafe
and nothing forces the user to actually wrap it in unsafe {}.

Is there a third option I'm not aware of? I.e. for the above reason
impl_device_context_deref!() was designed the same way.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n650

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Alexandre Courbot @ 2026-03-19 11:59 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Miguel Ojeda, Alejandra González, Alice Ryhl, linux-kernel,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Alex Gaynor,
	Danilo Krummrich, Dave Airlie, David Airlie, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Daniel Almeida,
	Koen Koning, Nikola Djukic, Philipp Stanner, Elle Rhumsaa,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, alexeyi, Eliot Courtney,
	dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <20260318192446.GA591541@joelbox2>

On Thu Mar 19, 2026 at 4:24 AM JST, Joel Fernandes wrote:
> On Wed, Mar 18, 2026 at 07:57:14PM +0100, Miguel Ojeda wrote:
>> On Wed, Mar 18, 2026 at 7:31 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>> >
>> > Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
>> > here, instead of // SAFETY:
>> > https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/
>> 
>> So, to clarify, I suggested it as a temporary thing we could do if we
>> want to use that "fake `unsafe` block in macro matcher" pattern more
>> and more.
>> 
>> i.e. if we plan to use the pattern more, then I am happy to ask
>> upstream if it would make sense for Clippy to recognize it (or perhaps
>> it is just a false negative instead of a false positive, given
>> `impl_device_context_deref`), so that we don't need a hacked safety
>> tag (Cc'ing Alejandra).
>> 
>> But if we could put it outside, then we wouldn't need any of that.
>> Unsafe macros support could help perhaps here, which I have had it in
>> our wishlist too (https://github.com/Rust-for-Linux/linux/issues/354),
>> but I guess the fake block could still be useful to make only certain
>> macro arms unsafe? (Perhaps Rust could allow `unsafe` just at the
>> start of each arm for that...).
>
> Even if I reworked the macro to be outisde, it doesn't work as below, still
> need the 'disabled' comment on the macro's generate unsafe { } block below.
>
> If we don't want the SAFETY*: hack, we could do the following.
>
> Perhaps, we can file the github bug and also do the below. Once the
> github bug is fixed, we could remove the 'disable lint' below.
>
> Thoughts? 
>
> ---8<-----------------------
>
> diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
> index 495497f0405e..dfa2e1490202 100644
> --- a/rust/kernel/interop/list.rs
> +++ b/rust/kernel/interop/list.rs
> @@ -73,7 +73,7 @@
>  //!
>  //!
>  //! // Create typed [`CList`] from sentinel head.
> -//! // SAFETY*: `head` is valid and initialized, items are `SampleItemC` with
> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>  //! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
>  //! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
>  //!
> @@ -328,17 +328,19 @@ impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
>  /// Refer to the examples in the [`crate::interop::list`] module documentation.
>  #[macro_export]
>  macro_rules! clist_create {
> -    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => (
> +        // SAFETY: disable lint.
> +        unsafe { {{
>          // Compile-time check that field path is a `list_head`.
>          let _: fn(*const $c_type) -> *const $crate::bindings::list_head = |p| {
>              // SAFETY: `p` is a valid pointer to `$c_type`.
> -            unsafe { &raw const (*p).$($field).+ }
> +            &raw const (*p).$($field).+
>          };
>  
>          // Calculate offset and create `CList`.
>          const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
>          // SAFETY: The caller of this macro is responsible for ensuring safety.
> -        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
> -    }};
> +        $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head)
> +    } }});
>  }
>  pub use clist_create;

I think I like this, it preserves the expected use of `SAFETY:` without
that confusing `*`. The unsafe blocks is a bit larger that it should, be
we are in a controlled environment.

Even after using the `SAFETY*:` I was still getting errors because the
in-macro SAFETY comment wasn't at the right place:

    warning: unsafe block missing a safety comment
      --> ../rust/kernel/interop/list.rs:335:17
        |
    335 |               |p| unsafe { &raw const (*p).$($field).+ };
        |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
      ::: ../rust/kernel/gpu/buddy.rs:527:21
        |
    527 |           let clist = clist_create!(unsafe {
        |  _____________________-
    528 | |             head,
    529 | |             Block,
    530 | |             bindings::gpu_buddy_block,
    531 | |             __bindgen_anon_1.link
    532 | |         });
        | |__________- in this macro invocation

The diff below fixes that, but I believe your proposal should as well on
top of letting callers use the expected SAFETY statement.

--- a/rust/kernel/interop/list.rs
+++ b/rust/kernel/interop/list.rs
@@ -330,8 +330,8 @@ impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
 macro_rules! clist_create {
     (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
         // Compile-time check that field path is a `list_head`.
-        // SAFETY: `p` is a valid pointer to `$c_type`.
         let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
+            // SAFETY: `p` is a valid pointer to `$c_type`.
             |p| unsafe { &raw const (*p).$($field).+ };

         // Calculate offset and create `CList`.


^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Gary Guo @ 2026-03-19 11:39 UTC (permalink / raw)
  To: Joel Fernandes, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Alex Gaynor, Danilo Krummrich, Dave Airlie, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260317201710.934932-2-joelagnelf@nvidia.com>

On Tue Mar 17, 2026 at 8:17 PM GMT, Joel Fernandes wrote:
> Add a new module `kernel::interop::list` for working with C's doubly
> circular linked lists. Provide low-level iteration over list nodes.
>
> Typed iteration over actual items is provided with a `clist_create`
> macro to assist in creation of the `CList` type.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Gary Guo <gary@garyguo.net>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  MAINTAINERS                 |   8 +
>  rust/helpers/helpers.c      |   1 +
>  rust/helpers/list.c         |  17 ++
>  rust/kernel/interop.rs      |   9 +
>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs          |   2 +
>  6 files changed, 379 insertions(+)
>  create mode 100644 rust/helpers/list.c
>  create mode 100644 rust/kernel/interop.rs
>  create mode 100644 rust/kernel/interop/list.rs
>
> +/// Create a C doubly-circular linked list interface [`CList`] from a raw `list_head` pointer.
> +///
> +/// This macro creates a `CList<T, OFFSET>` that can iterate over items of type `$rust_type`
> +/// linked via the `$field` field in the underlying C struct `$c_type`.
> +///
> +/// # Arguments
> +///
> +/// - `$head`: Raw pointer to the sentinel `list_head` object (`*mut bindings::list_head`).
> +/// - `$rust_type`: Each item's rust wrapper type.
> +/// - `$c_type`: Each item's C struct type that contains the embedded `list_head`.
> +/// - `$field`: The name of the `list_head` field within the C struct.
> +///
> +/// # Safety
> +///
> +/// The caller must ensure:
> +///
> +/// - `$head` is a valid, initialized sentinel `list_head` (e.g. via `INIT_LIST_HEAD()`)
> +///   pointing to a list that is not concurrently modified for the lifetime of the [`CList`].
> +/// - The list contains items of type `$c_type` linked via an embedded `$field`.
> +/// - `$rust_type` is `#[repr(transparent)]` over `$c_type` or has compatible layout.
> +///
> +/// # Examples
> +///
> +/// Refer to the examples in the [`crate::interop::list`] module documentation.
> +#[macro_export]
> +macro_rules! clist_create {
> +    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> +        // Compile-time check that field path is a `list_head`.
> +        // SAFETY: `p` is a valid pointer to `$c_type`.
> +        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
> +            |p| unsafe { &raw const (*p).$($field).+ };

Actually, this check is insufficient, you should create a reference instead
(just in case people put this inside `repr(packed)`.

This could be something like

    let _ = |p: &$c_type| { _ = &p.$($field).+ }

?

> +
> +        // Calculate offset and create `CList`.
> +        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> +        // SAFETY: The caller of this macro is responsible for ensuring safety.
> +        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }

Given that this is unsafe, I am not sure why the macro should have unsafe
keyword in it, rather than just being `clist_create(a, b, c, d)` and just have
user write unsafe.

The offset could probably benefit from similar strategies we used for other
embedded data structure like Rust list and work items. Boqun has a series
unifying these and clist could probably use it, too. (As a follow-up)

Best,
Gary

> +    }};
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index d93292d47420..bdcf632050ee 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -29,6 +29,7 @@
>  #![feature(lint_reasons)]
>  //
>  // Stable since Rust 1.82.0.
> +#![feature(offset_of_nested)]
>  #![feature(raw_ref_op)]
>  //
>  // Stable since Rust 1.83.0.
> @@ -107,6 +108,7 @@
>  #[doc(hidden)]
>  pub mod impl_flags;
>  pub mod init;
> +pub mod interop;
>  pub mod io;
>  pub mod ioctl;
>  pub mod iommu;
G

^ permalink raw reply

* [syzbot] Monthly fbdev report (Mar 2026)
From: syzbot @ 2026-03-19 10:23 UTC (permalink / raw)
  To: deller, dri-devel, linux-fbdev, linux-kernel, syzkaller-bugs

Hello fbdev maintainers/developers,

This is a 31-day syzbot report for the fbdev subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/fbdev

During the period, 0 new issues were detected and 0 were fixed.
In total, 5 issues are still open and 29 have already been fixed.

Some of the still happening issues:

Ref Crashes Repro Title
<1> 2249    Yes   KASAN: vmalloc-out-of-bounds Write in imageblit (6)
                  https://syzkaller.appspot.com/bug?extid=5a40432dfe8f86ee657a
<2> 1510    Yes   KASAN: slab-out-of-bounds Read in fbcon_prepare_logo
                  https://syzkaller.appspot.com/bug?extid=0c815b25cdb3678e7083
<3> 195     No    KASAN: vmalloc-out-of-bounds Write in fillrect
                  https://syzkaller.appspot.com/bug?extid=7a63ce155648954e749b

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-19  9:08 UTC (permalink / raw)
  To: Helge Deller
  Cc: Jason Yan, Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <c717b7b6-ffb6-47f9-b345-de0eddcfe7a4@gmx.de>

On Thu, Mar 19, 2026 at 09:35:33AM +0100, Helge Deller wrote:
> On 3/19/26 09:21, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> > > 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > > > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:

...

> > > > Helge, can we revert this change and start over, please? (I can also send
> > > > revert if you think it's a better way)
> 
> Andy, all points you make against removing relevant code is absolutely right.
> 
> But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
> set but not used variable") I have to agree with Jason, that the patch is ok.
> I don't see any build errors either.

Just run on today's Linux Next (since the driver has not been changed in that
sense for a few years, the very same issue is present for a long time):

drivers/video/fbdev/matrox/g450_pll.c:412:18: error: variable 'mnp' set but not used [-Werror,-Wunused-but-set-variable]
  412 |                                 unsigned int mnp;
      |                                              ^
1 error generated.

> Are we mixing up things here maybe?

...

FWIW, I dove to the history of the driver.

So, the code, that was guarded by #if 0 was introduced in the original commit
213d22146d1f ("[PATCH] (1/3) matroxfb for 2.5.3"). And then guarded in the
commit 705e41f82988 ("matroxfb DVI updates: Handle DVI output on G450/G550.
Powerdown unused portions of G450/G550 DAC. Split G450/G550 DAC from older
DAC1064 handling. Modify PLL setting when both CRTCs use same pixel clocks.").

Even without that guard the modern compilers may see that the pixel_vco wasn't
ever used and seems a leftover after some debug or review made 25 years ago.

The g450_mnp2vco() doesn't have any IO and as Jason said doesn't seem to have
any side effects either than some unneeded CPU processing during runtime. I
agree that's unlikely that timeout (or heating up the CPU) has any effect on
the HW (GPU/display) functionality.

So, since the commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code
and set but not used variable") the 'mnp' became unused, but eliminating that
code might have side-effects. The question here is what should we do with mnp?
The easiest way out is just mark it with __maybe_unused which will shut the
compiler up and won't change any possible IO flow.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Helge Deller @ 2026-03-19  8:35 UTC (permalink / raw)
  To: Andy Shevchenko, Jason Yan
  Cc: Sam Ravnborg, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abuyJDK6E2aHA4rC@ashevche-desk.local>

Hi Andy,

On 3/19/26 09:21, Andy Shevchenko wrote:
> On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
>> 在 2026/3/19 15:38, Andy Shevchenko 写道:
>>> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
>>>> 在 2026/3/18 15:45, Andy Shevchenko 写道:
>>>>> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>>>>>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>>>>>> Fix the following gcc warning:
>>>>>>>
>>>>>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>>>>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>>>>>      unsigned int pixel_vco;
>>>>>>>                   ^~~~~~~~~
>>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>>>>>
>>>>>> Thanks, committed and pushed to drm-misc-next.
>>>>>> The fix will show up in upstream kernel at the next
>>>>>> merge window.
>>>>>
>>>>> The most of the patches from so called Hulk Robot appeared to be controversial.
>>>>>
>>>>> First of all, even so called "dead code" may have side effects on the registers
>>>>> in HW which may lead to other issues. Second, the mentioned dead code elimination
>>>>> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
>>>>> I got into that, I still have a build error).
>>>>
>>>> Sorry, I do not understand what side effects this may cause. Would you
>>>> please explain it in detail?
>>>
>>> Any IO (and specifically on PCI bus) may have side effects. If the removed code
>>> did some HW accesses (especially reads), it affects the state of HW. You can
>>> read more about PCI bus and what read from it does.
>>
>> No, the removed code did not do any IO and will not affect the state of HW.
> 
> Excellent, but it leaves the code that does IO and now still breaks the
> compilation.
> 
>>> Helge, can we revert this change and start over, please? (I can also send
>>> revert if you think it's a better way)

Andy, all points you make against removing relevant code is absolutely right.

But for this specific commit 7b987887f97b ("video: fbdev: matroxfb: remove dead code and
set but not used variable") I have to agree with Jason, that the patch is ok.
I don't see any build errors either.

Are we mixing up things here maybe?

Helge

^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-19  8:21 UTC (permalink / raw)
  To: Jason Yan
  Cc: Sam Ravnborg, Helge Deller, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <e5f7611f-f087-4835-99e3-4fddc927aab8@huawei.com>

On Thu, Mar 19, 2026 at 04:06:44PM +0800, Jason Yan wrote:
> 在 2026/3/19 15:38, Andy Shevchenko 写道:
> > On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> > > 在 2026/3/18 15:45, Andy Shevchenko 写道:
> > > > On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> > > > > On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > > > > > Fix the following gcc warning:
> > > > > > 
> > > > > > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > > > > > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> > > > > >     unsigned int pixel_vco;
> > > > > >                  ^~~~~~~~~
> > > > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > > > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> > > > > 
> > > > > Thanks, committed and pushed to drm-misc-next.
> > > > > The fix will show up in upstream kernel at the next
> > > > > merge window.
> > > > 
> > > > The most of the patches from so called Hulk Robot appeared to be controversial.
> > > > 
> > > > First of all, even so called "dead code" may have side effects on the registers
> > > > in HW which may lead to other issues. Second, the mentioned dead code elimination
> > > > patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> > > > I got into that, I still have a build error).
> > > 
> > > Sorry, I do not understand what side effects this may cause. Would you
> > > please explain it in detail?
> > 
> > Any IO (and specifically on PCI bus) may have side effects. If the removed code
> > did some HW accesses (especially reads), it affects the state of HW. You can
> > read more about PCI bus and what read from it does.
> 
> No, the removed code did not do any IO and will not affect the state of HW.

Excellent, but it leaves the code that does IO and now still breaks the
compilation.

> > Helge, can we revert this change and start over, please? (I can also send
> > revert if you think it's a better way)
> 
> I don't think we need to revert this patch. Please provide proof that this
> modification will lead to real issues.

It's not me, who should provide an evidence, it had to be the author
who must had provided testing and everything when submitting the change.

> > I do not believe this change was ever tested on real HW, and I see an evidence
> > that this most likely was blindly made as it led to the similar issue after
> > the change.

No answer here? That's what I expected based on my observations on the quality
of the patches from so called Hulk Robot.

> > > > That said, for the starter I suggest to revert this change. After one need go
> > > > carefully through code to understand if it's exactly the case and what to do with
> > > > 'mnp' which involves some IO.
> > > > 
> > > > +Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Jason Yan @ 2026-03-19  8:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sam Ravnborg, Helge Deller, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abun9dwCYAWd1nIB@ashevche-desk.local>

在 2026/3/19 15:38, Andy Shevchenko 写道:
> On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
>> 在 2026/3/18 15:45, Andy Shevchenko 写道:
>>> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>>>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>>>> Fix the following gcc warning:
>>>>>
>>>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>>>     unsigned int pixel_vco;
>>>>>                  ^~~~~~~~~
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>>>
>>>> Thanks, committed and pushed to drm-misc-next.
>>>> The fix will show up in upstream kernel at the next
>>>> merge window.
>>>
>>> The most of the patches from so called Hulk Robot appeared to be controversial.
>>>
>>> First of all, even so called "dead code" may have side effects on the registers
>>> in HW which may lead to other issues. Second, the mentioned dead code elimination
>>> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
>>> I got into that, I still have a build error).
>>
>> Sorry, I do not understand what side effects this may cause. Would you
>> please explain it in detail?
> 
> Any IO (and specifically on PCI bus) may have side effects. If the removed code
> did some HW accesses (especially reads), it affects the state of HW. You can
> read more about PCI bus and what read from it does.

No, the removed code did not do any IO and will not affect the state of HW.

> 
> Helge, can we revert this change and start over, please? (I can also send
> revert if you think it's a better way)

I don't think we need to revert this patch. Please provide proof that 
this modification will lead to real issues.

Thanks,
Jason

> 
> I do not believe this change was ever tested on real HW, and I see an evidence
> that this most likely was blindly made as it led to the similar issue after
> the change.
> 
>>> That said, for the starter I suggest to revert this change. After one need go
>>> carefully through code to understand if it's exactly the case and what to do with
>>> 'mnp' which involves some IO.
>>>
>>> +Cc: current fbdev maintainers
> 

^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Andy Shevchenko @ 2026-03-19  7:38 UTC (permalink / raw)
  To: Jason Yan
  Cc: Sam Ravnborg, Helge Deller, linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <17605e19-065c-4b71-abb2-a9c9a7b9ddc6@huawei.com>

On Thu, Mar 19, 2026 at 10:22:08AM +0800, Jason Yan wrote:
> 在 2026/3/18 15:45, Andy Shevchenko 写道:
> > On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
> > > On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
> > > > Fix the following gcc warning:
> > > > 
> > > > drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
> > > > ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
> > > >    unsigned int pixel_vco;
> > > >                 ^~~~~~~~~
> > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > Signed-off-by: Jason Yan <yanaijie@huawei.com>
> > > 
> > > Thanks, committed and pushed to drm-misc-next.
> > > The fix will show up in upstream kernel at the next
> > > merge window.
> > 
> > The most of the patches from so called Hulk Robot appeared to be controversial.
> > 
> > First of all, even so called "dead code" may have side effects on the registers
> > in HW which may lead to other issues. Second, the mentioned dead code elimination
> > patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> > I got into that, I still have a build error).
> 
> Sorry, I do not understand what side effects this may cause. Would you
> please explain it in detail?

Any IO (and specifically on PCI bus) may have side effects. If the removed code
did some HW accesses (especially reads), it affects the state of HW. You can
read more about PCI bus and what read from it does.

Helge, can we revert this change and start over, please? (I can also send
revert if you think it's a better way)

I do not believe this change was ever tested on real HW, and I see an evidence
that this most likely was blindly made as it led to the similar issue after
the change.

> > That said, for the starter I suggest to revert this change. After one need go
> > carefully through code to understand if it's exactly the case and what to do with
> > 'mnp' which involves some IO.
> > 
> > +Cc: current fbdev maintainers

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH] video: fbdev: matroxfb: remove dead code and set but not used variable
From: Jason Yan @ 2026-03-19  2:22 UTC (permalink / raw)
  To: Andy Shevchenko, Sam Ravnborg, Helge Deller
  Cc: linux-fbdev, dri-devel, b.zolnierkie
In-Reply-To: <abpX_2PUWie03GdG@black.igk.intel.com>

在 2026/3/18 15:45, Andy Shevchenko 写道:
> On Wed, Apr 08, 2020 at 10:18:52AM +0000, Sam Ravnborg wrote:
>> On Fri, Apr 03, 2020 at 10:16:09AM +0800, Jason Yan wrote:
>>> Fix the following gcc warning:
>>>
>>> drivers/video/fbdev/matrox/g450_pll.c:336:15: warning: variable
>>> ‘pixel_vco’ set but not used [-Wunused-but-set-variable]
>>>    unsigned int pixel_vco;
>>>                 ^~~~~~~~~
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>
>> Thanks, committed and pushed to drm-misc-next.
>> The fix will show up in upstream kernel at the next
>> merge window.
> 
> The most of the patches from so called Hulk Robot appeared to be controversial.
> 
> First of all, even so called "dead code" may have side effects on the registers
> in HW which may lead to other issues. Second, the mentioned dead code elimination
> patch doesn't improve anything as now the dead code is 'mnp' variable (that's how
> I got into that, I still have a build error).

Sorry, I do not understand what side effects this may cause. Would you 
please explain it in detail?

Thanks,
Jason

> 
> That said, for the starter I suggest to revert this change. After one need go
> carefully through code to understand if it's exactly the case and what to do with
> 'mnp' which involves some IO.
> 
> +Cc: current fbdev maintainers
> 

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-18 19:24 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Alejandra González, Alice Ryhl, linux-kernel, Miguel Ojeda,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Alex Gaynor, Danilo Krummrich,
	Dave Airlie, David Airlie, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Simona Vetter, Daniel Almeida, Koen Koning,
	Nikola Djukic, Alexandre Courbot, Philipp Stanner, Elle Rhumsaa,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, alexeyi, Eliot Courtney,
	dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <CANiq72nZKx7pw_rZK2mHHvR=TaeGvMRvg5GTHOd58X17oyxieg@mail.gmail.com>

On Wed, Mar 18, 2026 at 07:57:14PM +0100, Miguel Ojeda wrote:
> On Wed, Mar 18, 2026 at 7:31 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
> >
> > Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
> > here, instead of // SAFETY:
> > https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/
> 
> So, to clarify, I suggested it as a temporary thing we could do if we
> want to use that "fake `unsafe` block in macro matcher" pattern more
> and more.
> 
> i.e. if we plan to use the pattern more, then I am happy to ask
> upstream if it would make sense for Clippy to recognize it (or perhaps
> it is just a false negative instead of a false positive, given
> `impl_device_context_deref`), so that we don't need a hacked safety
> tag (Cc'ing Alejandra).
> 
> But if we could put it outside, then we wouldn't need any of that.
> Unsafe macros support could help perhaps here, which I have had it in
> our wishlist too (https://github.com/Rust-for-Linux/linux/issues/354),
> but I guess the fake block could still be useful to make only certain
> macro arms unsafe? (Perhaps Rust could allow `unsafe` just at the
> start of each arm for that...).

Even if I reworked the macro to be outisde, it doesn't work as below, still
need the 'disabled' comment on the macro's generate unsafe { } block below.

If we don't want the SAFETY*: hack, we could do the following.

Perhaps, we can file the github bug and also do the below. Once the
github bug is fixed, we could remove the 'disable lint' below.

Thoughts? 

---8<-----------------------

diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
index 495497f0405e..dfa2e1490202 100644
--- a/rust/kernel/interop/list.rs
+++ b/rust/kernel/interop/list.rs
@@ -73,7 +73,7 @@
 //!
 //!
 //! // Create typed [`CList`] from sentinel head.
-//! // SAFETY*: `head` is valid and initialized, items are `SampleItemC` with
+//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
 //! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
 //! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
 //!
@@ -328,17 +328,19 @@ impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
 /// Refer to the examples in the [`crate::interop::list`] module documentation.
 #[macro_export]
 macro_rules! clist_create {
-    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
+    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => (
+        // SAFETY: disable lint.
+        unsafe { {{
         // Compile-time check that field path is a `list_head`.
         let _: fn(*const $c_type) -> *const $crate::bindings::list_head = |p| {
             // SAFETY: `p` is a valid pointer to `$c_type`.
-            unsafe { &raw const (*p).$($field).+ }
+            &raw const (*p).$($field).+
         };
 
         // Calculate offset and create `CList`.
         const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
         // SAFETY: The caller of this macro is responsible for ensuring safety.
-        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
-    }};
+        $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head)
+    } }});
 }
 pub use clist_create;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Miguel Ojeda @ 2026-03-18 18:57 UTC (permalink / raw)
  To: Joel Fernandes, Alejandra González
  Cc: Alice Ryhl, linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alex Gaynor, Danilo Krummrich, Dave Airlie,
	David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <71b6a115-98f1-4b09-9c04-a99349f51e49@nvidia.com>

On Wed, Mar 18, 2026 at 7:31 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
> here, instead of // SAFETY:
> https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/

So, to clarify, I suggested it as a temporary thing we could do if we
want to use that "fake `unsafe` block in macro matcher" pattern more
and more.

i.e. if we plan to use the pattern more, then I am happy to ask
upstream if it would make sense for Clippy to recognize it (or perhaps
it is just a false negative instead of a false positive, given
`impl_device_context_deref`), so that we don't need a hacked safety
tag (Cc'ing Alejandra).

But if we could put it outside, then we wouldn't need any of that.
Unsafe macros support could help perhaps here, which I have had it in
our wishlist too (https://github.com/Rust-for-Linux/linux/issues/354),
but I guess the fake block could still be useful to make only certain
macro arms unsafe? (Perhaps Rust could allow `unsafe` just at the
start of each arm for that...).

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Miguel Ojeda @ 2026-03-18 18:57 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Alice Ryhl, Alexandre Courbot, Joel Fernandes, linux-kernel,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Alex Gaynor,
	Dave Airlie, David Airlie, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Simona Vetter, Daniel Almeida, Koen Koning,
	Nikola Djukic, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DH5ZP3IUEWMD.TOOZSEUTLV85@kernel.org>

On Wed, Mar 18, 2026 at 3:49 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> What do you mean with extra file?

Right, sorry -- I was probably overfocused on the `mod.rs` symlink for
some reason. We may have talked about generating them on the fly in
the past too, I don't recall anymore.

Let's assume "s/an extra file/extra lines" on my message... If folks
are OK writing manually them, or that we generate them on the fly,
then I am happy I can get the option I always liked! :P

We can chat about it in one of the calls.

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-18 18:55 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alex Gaynor, Danilo Krummrich, Dave Airlie,
	David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <abpr65jD6esHysWO@google.com>



On 3/18/2026 5:10 AM, Alice Ryhl wrote:
> On Tue, Mar 17, 2026 at 04:17:10PM -0400, Joel Fernandes wrote:
>> Add a new module `kernel::interop::list` for working with C's doubly
>> circular linked lists. Provide low-level iteration over list nodes.
>>
>> Typed iteration over actual items is provided with a `clist_create`
>> macro to assist in creation of the `CList` type.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Gary Guo <gary@garyguo.net>
>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> 
> I have a few nits below. But overall I think this looks ok:
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> Please do consider my mod.rs suggestion too, though.

sure.

> 
>> +//! ```
>> +//! use kernel::{
>> +//!     bindings,
>> +//!     clist_create,
> 
> IMO the automatic re-exports of macros at the root shouldn't be used.
> Import it from kernel::interop::list::clist_create instead.
> 
> Note that you need to put a re-export below macro definition to do this.
> 
> 	macro_rules! clist_create {
> 	    (unsafe { $head:ident, $rust_type:ty, $c_type:ty, $($field:tt).+ }) => {{
> 	        // Compile-time check that field path is a `list_head`.
> 	        // SAFETY: `p` is a valid pointer to `$c_type`.
> 	        let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
> 	            |p| unsafe { &raw const (*p).$($field).+ };
> 	
> 	        // Calculate offset and create `CList`.
> 	        const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> 	        // SAFETY: The caller of this macro is responsible for ensuring safety.
> 	        unsafe { $crate::interop::list::CList::<$rust_type, OFFSET>::from_raw($head) }
> 	    }};
> 	}
> 	pub use clist_create; // <-- you need this
> 
> See tracepoint.rs or any of the other macros for an example.

Ok.

> 
>> +//! // Create typed [`CList`] from sentinel head.
>> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>> +//! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
>> +//! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
> 
> Did you try using this in your real use-case? You require `head` to be
> an :ident, but I think for any 'struct list_head' not stored on the
> stack, accepting an :expr would be easier to use so that you can just
> pass `&raw mut my_c_struct.the_list_head` directly to the macro. Right
> now you have to put the raw pointer in a local variable first.
For buddy.rs usecase, currently we do put it in a local variable so it
works fine. Not doing :ident was causing clippy errors.


warning: this macro expands metavariables in an unsafe block
   --> rust/kernel/interop/list.rs:341:9
    |
341 |         unsafe { $crate::interop::list::CList::<$rust_type,
OFFSET>::from_raw($head) }
    |
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this allows the user of the macro to write unsafe code outside
of an unsafe block
    = help: consider expanding any metavariables outside of this block,
e.g. by storing them in a variable
    = help: ... or also expand referenced metavariables in a safe context
to require an unsafe block at callsite
    = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#macro_metavars_in_unsafe
    = note: `-W clippy::macro-metavars-in-unsafe` implied by `-W clippy::all`
    = help: to override `-W clippy::all` add
`#[allow(clippy::macro_metavars_in_unsafe)]`


thanks,

--
Joel Fernandes


^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-18 18:43 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alex Gaynor, Danilo Krummrich, Dave Airlie,
	David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <71b6a115-98f1-4b09-9c04-a99349f51e49@nvidia.com>



On 3/18/2026 2:31 PM, Joel Fernandes wrote:
> 
> 
> On 3/18/2026 8:40 AM, Alice Ryhl wrote:
>> On Tue, Mar 17, 2026 at 04:17:10PM -0400, Joel Fernandes wrote:
>>> Add a new module `kernel::interop::list` for working with C's doubly
>>> circular linked lists. Provide low-level iteration over list nodes.
>>>
>>> Typed iteration over actual items is provided with a `clist_create`
>>> macro to assist in creation of the `CList` type.
>>>
>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Acked-by: Gary Guo <gary@garyguo.net>
>>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> ---
>>>  MAINTAINERS                 |   8 +
>>>  rust/helpers/helpers.c      |   1 +
>>>  rust/helpers/list.c         |  17 ++
>>>  rust/kernel/interop.rs      |   9 +
>>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>>  rust/kernel/lib.rs          |   2 +
>>>  6 files changed, 379 insertions(+)
>>>  create mode 100644 rust/helpers/list.c
>>>  create mode 100644 rust/kernel/interop.rs
>>>  create mode 100644 rust/kernel/interop/list.rs
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 4bd6b538a51f..e847099efcc2 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -23251,6 +23251,14 @@ T:	git https://github.com/Rust-for-Linux/linux.git alloc-next
>>>  F:	rust/kernel/alloc.rs
>>>  F:	rust/kernel/alloc/
>>>  
>>> +RUST [INTEROP]
>>> +M:	Joel Fernandes <joelagnelf@nvidia.com>
>>> +M:	Alexandre Courbot <acourbot@nvidia.com>
>>> +L:	rust-for-linux@vger.kernel.org
>>> +S:	Maintained
>>> +T:	git https://github.com/Rust-for-Linux/linux.git interop-next
>>> +F:	rust/kernel/interop/
>>> +
>>>  RUST [NUM]
>>>  M:	Alexandre Courbot <acourbot@nvidia.com>
>>>  R:	Yury Norov <yury.norov@gmail.com>
>>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>>> index a3c42e51f00a..724fcb8240ac 100644
>>> --- a/rust/helpers/helpers.c
>>> +++ b/rust/helpers/helpers.c
>>> @@ -35,6 +35,7 @@
>>>  #include "io.c"
>>>  #include "jump_label.c"
>>>  #include "kunit.c"
>>> +#include "list.c"
>>>  #include "maple_tree.c"
>>>  #include "mm.c"
>>>  #include "mutex.c"
>>> diff --git a/rust/helpers/list.c b/rust/helpers/list.c
>>> new file mode 100644
>>> index 000000000000..18095a5593c5
>>> --- /dev/null
>>> +++ b/rust/helpers/list.c
>>> @@ -0,0 +1,17 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +/*
>>> + * Helpers for C circular doubly linked list implementation.
>>> + */
>>> +
>>> +#include <linux/list.h>
>>> +
>>> +__rust_helper void rust_helper_INIT_LIST_HEAD(struct list_head *list)
>>> +{
>>> +	INIT_LIST_HEAD(list);
>>> +}
>>> +
>>> +__rust_helper void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
>>> +{
>>> +	list_add_tail(new, head);
>>> +}
>>> diff --git a/rust/kernel/interop.rs b/rust/kernel/interop.rs
>>> new file mode 100644
>>> index 000000000000..b88140cf76dc
>>> --- /dev/null
>>> +++ b/rust/kernel/interop.rs
>>> @@ -0,0 +1,9 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Infrastructure for interfacing Rust code with C kernel subsystems.
>>> +//!
>>> +//! This module is intended for low-level, unsafe Rust infrastructure code
>>> +//! that interoperates between Rust and C. It is NOT for use directly in
>>> +//! Rust drivers.
>>> +
>>> +pub mod list;
>>> diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
>>> new file mode 100644
>>> index 000000000000..328f6b0de2ce
>>> --- /dev/null
>>> +++ b/rust/kernel/interop/list.rs
>>> @@ -0,0 +1,342 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Rust interface for C doubly circular intrusive linked lists.
>>> +//!
>>> +//! This module provides Rust abstractions for iterating over C `list_head`-based
>>> +//! linked lists. It should only be used for cases where C and Rust code share
>>> +//! direct access to the same linked list through a C interop interface.
>>> +//!
>>> +//! Note: This *must not* be used by Rust components that just need a linked list
>>> +//! primitive. Use [`kernel::list::List`] instead.
>>> +//!
>>> +//! # Examples
>>> +//!
>>> +//! ```
>>> +//! use kernel::{
>>> +//!     bindings,
>>> +//!     clist_create,
>>> +//!     types::Opaque,
>>> +//! };
>>> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
>>> +//! # // emulated here for doctests using the C bindings.
>>> +//! # use core::mem::MaybeUninit;
>>> +//! #
>>> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
>>> +//! # #[repr(C)]
>>> +//! # pub struct SampleItemC {
>>> +//! #     pub value: i32,
>>> +//! #     pub link: bindings::list_head,
>>> +//! # }
>>> +//! #
>>> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
>>> +//! #
>>> +//! # let head = head.as_mut_ptr();
>>> +//! # // SAFETY: `head` and all the items are test objects allocated in this scope.
>>> +//! # unsafe { bindings::INIT_LIST_HEAD(head) };
>>> +//! #
>>> +//! # let mut items = [
>>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>>> +//! # ];
>>> +//! #
>>> +//! # for (i, item) in items.iter_mut().enumerate() {
>>> +//! #     let ptr = item.as_mut_ptr();
>>> +//! #     // SAFETY: `ptr` points to a valid `MaybeUninit<SampleItemC>`.
>>> +//! #     unsafe { (*ptr).value = i as i32 * 10 };
>>> +//! #     // SAFETY: `&raw mut` creates a pointer valid for `INIT_LIST_HEAD`.
>>> +//! #     unsafe { bindings::INIT_LIST_HEAD(&raw mut (*ptr).link) };
>>> +//! #     // SAFETY: `link` was just initialized and `head` is a valid list head.
>>> +//! #     unsafe { bindings::list_add_tail(&mut (*ptr).link, head) };
>>> +//! # }
>>> +//!
>>> +//! //
>>> +//! /// Rust wrapper for the C struct.
>>> +//! ///
>>> +//! /// The list item struct in this example is defined in C code as:
>>> +//! ///
>>> +//! /// ```c
>>> +//! /// struct SampleItemC {
>>> +//! ///     int value;
>>> +//! ///     struct list_head link;
>>> +//! /// };
>>> +//! /// ```
>>> +//! #[repr(transparent)]
>>> +//! pub struct Item(Opaque<SampleItemC>);
>>> +//!
>>> +//! impl Item {
>>> +//!     pub fn value(&self) -> i32 {
>>> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
>>> +//!         unsafe { (*self.0.get()).value }
>>> +//!     }
>>> +//! }
>>> +//!
>>> +//!
>>> +//! // Create typed [`CList`] from sentinel head.
>>> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>>> +//! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
>>> +//! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
>>
>> Bad news.
>>
>> My build triggers this warning:
>>
>> error: statement has unnecessary safety comment
>>     --> rust/doctests_kernel_generated.rs:7103:1
>>      |
>> 7103 | let list = clist_create!(unsafe { head, Item, SampleItemC, link });
>>      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      |
>> help: consider removing the safety comment
>>     --> rust/doctests_kernel_generated.rs:7101:4
>>      |
>> 7101 | // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>>      |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#unnecessary_safety_comment
>>      = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`
>>      = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]`
>>
>> This probably needs to be:
>>
>> 	unsafe { clist_create!(head, Item, SampleItemC, link) }
> 
> Oops, I sometimes run clippy wrong. I did: "CLIPPY=1 make" instead of "make
> CLIPPY=1", the reason is habit. I have been doing "LLVM=1 make" for llvm
> builds since many years.
> 
> Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
> here, instead of // SAFETY:
> https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/
> 
> Should be easy to do on apply if Danilo doesn't mind. :) I verified on my
> setup that it fixes it.
Here is the clist fixup fixing both the clippy errors in list.rs:

https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=nova/mm&id=226e1c75932aec3fde66b4b4a36abd8de919c6da

Thanks.


^ permalink raw reply

* Re: [PATCH v13 1/2] rust: gpu: Add GPU buddy allocator bindings
From: Joel Fernandes @ 2026-03-18 18:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
	dri-devel, rust-for-linux, Nikola Djukic, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Jonathan Corbet, Alex Deucher, Christian König, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Huang Rui,
	Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, Alex Gaynor, Boqun Feng,
	John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi,
	Eliot Courtney, joel, linux-doc, amd-gfx, intel-gfx, intel-xe,
	linux-fbdev
In-Reply-To: <20260317220323.1909618-2-joelagnelf@nvidia.com>



On 3/17/2026 6:03 PM, Joel Fernandes wrote:
> +impl AllocatedBlocks {
> +    /// Check if the block list is empty.
> +    pub fn is_empty(&self) -> bool {
> +        // An empty list head points to itself.
> +        !self.list.is_linked()
> +    }
> +
> +    /// Iterate over allocated blocks.
> +    ///
> +    /// Returns an iterator yielding [`AllocatedBlock`] values. Each [`AllocatedBlock`]
> +    /// borrows `self` and is only valid for the duration of that borrow.
> +    pub fn iter(&self) -> impl Iterator<Item = AllocatedBlock<'_>> + '_ {
> +        // SAFETY:
> +        // - Per the type invariant, `list` is an initialized sentinel `list_head`

Sorry that there is a clippy error here, this SAFETY needs to be SAFETY* per:
https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/
https://lore.kernel.org/all/71b6a115-98f1-4b09-9c04-a99349f51e49@nvidia.com/

here is a fixup:
https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=nova/mm&id=8ee92de5549324cba602c0e7a491596056f55153



^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-18 18:31 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Alex Gaynor, Danilo Krummrich, Dave Airlie,
	David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Simona Vetter, Daniel Almeida, Koen Koning, Nikola Djukic,
	Alexandre Courbot, Philipp Stanner, Elle Rhumsaa, Jonathan Corbet,
	Alex Deucher, Christian König, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellström,
	Helge Deller, John Hubbard, Alistair Popple, Timur Tabi,
	Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	alexeyi, Eliot Courtney, dri-devel, rust-for-linux, linux-doc,
	amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <abqdUBqchnVFo7Qk@google.com>



On 3/18/2026 8:40 AM, Alice Ryhl wrote:
> On Tue, Mar 17, 2026 at 04:17:10PM -0400, Joel Fernandes wrote:
>> Add a new module `kernel::interop::list` for working with C's doubly
>> circular linked lists. Provide low-level iteration over list nodes.
>>
>> Typed iteration over actual items is provided with a `clist_create`
>> macro to assist in creation of the `CList` type.
>>
>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>> Acked-by: Gary Guo <gary@garyguo.net>
>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  MAINTAINERS                 |   8 +
>>  rust/helpers/helpers.c      |   1 +
>>  rust/helpers/list.c         |  17 ++
>>  rust/kernel/interop.rs      |   9 +
>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs          |   2 +
>>  6 files changed, 379 insertions(+)
>>  create mode 100644 rust/helpers/list.c
>>  create mode 100644 rust/kernel/interop.rs
>>  create mode 100644 rust/kernel/interop/list.rs
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4bd6b538a51f..e847099efcc2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -23251,6 +23251,14 @@ T:	git https://github.com/Rust-for-Linux/linux.git alloc-next
>>  F:	rust/kernel/alloc.rs
>>  F:	rust/kernel/alloc/
>>  
>> +RUST [INTEROP]
>> +M:	Joel Fernandes <joelagnelf@nvidia.com>
>> +M:	Alexandre Courbot <acourbot@nvidia.com>
>> +L:	rust-for-linux@vger.kernel.org
>> +S:	Maintained
>> +T:	git https://github.com/Rust-for-Linux/linux.git interop-next
>> +F:	rust/kernel/interop/
>> +
>>  RUST [NUM]
>>  M:	Alexandre Courbot <acourbot@nvidia.com>
>>  R:	Yury Norov <yury.norov@gmail.com>
>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>> index a3c42e51f00a..724fcb8240ac 100644
>> --- a/rust/helpers/helpers.c
>> +++ b/rust/helpers/helpers.c
>> @@ -35,6 +35,7 @@
>>  #include "io.c"
>>  #include "jump_label.c"
>>  #include "kunit.c"
>> +#include "list.c"
>>  #include "maple_tree.c"
>>  #include "mm.c"
>>  #include "mutex.c"
>> diff --git a/rust/helpers/list.c b/rust/helpers/list.c
>> new file mode 100644
>> index 000000000000..18095a5593c5
>> --- /dev/null
>> +++ b/rust/helpers/list.c
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * Helpers for C circular doubly linked list implementation.
>> + */
>> +
>> +#include <linux/list.h>
>> +
>> +__rust_helper void rust_helper_INIT_LIST_HEAD(struct list_head *list)
>> +{
>> +	INIT_LIST_HEAD(list);
>> +}
>> +
>> +__rust_helper void rust_helper_list_add_tail(struct list_head *new, struct list_head *head)
>> +{
>> +	list_add_tail(new, head);
>> +}
>> diff --git a/rust/kernel/interop.rs b/rust/kernel/interop.rs
>> new file mode 100644
>> index 000000000000..b88140cf76dc
>> --- /dev/null
>> +++ b/rust/kernel/interop.rs
>> @@ -0,0 +1,9 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Infrastructure for interfacing Rust code with C kernel subsystems.
>> +//!
>> +//! This module is intended for low-level, unsafe Rust infrastructure code
>> +//! that interoperates between Rust and C. It is NOT for use directly in
>> +//! Rust drivers.
>> +
>> +pub mod list;
>> diff --git a/rust/kernel/interop/list.rs b/rust/kernel/interop/list.rs
>> new file mode 100644
>> index 000000000000..328f6b0de2ce
>> --- /dev/null
>> +++ b/rust/kernel/interop/list.rs
>> @@ -0,0 +1,342 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Rust interface for C doubly circular intrusive linked lists.
>> +//!
>> +//! This module provides Rust abstractions for iterating over C `list_head`-based
>> +//! linked lists. It should only be used for cases where C and Rust code share
>> +//! direct access to the same linked list through a C interop interface.
>> +//!
>> +//! Note: This *must not* be used by Rust components that just need a linked list
>> +//! primitive. Use [`kernel::list::List`] instead.
>> +//!
>> +//! # Examples
>> +//!
>> +//! ```
>> +//! use kernel::{
>> +//!     bindings,
>> +//!     clist_create,
>> +//!     types::Opaque,
>> +//! };
>> +//! # // Create test list with values (0, 10, 20) - normally done by C code but it is
>> +//! # // emulated here for doctests using the C bindings.
>> +//! # use core::mem::MaybeUninit;
>> +//! #
>> +//! # /// C struct with embedded `list_head` (typically will be allocated by C code).
>> +//! # #[repr(C)]
>> +//! # pub struct SampleItemC {
>> +//! #     pub value: i32,
>> +//! #     pub link: bindings::list_head,
>> +//! # }
>> +//! #
>> +//! # let mut head = MaybeUninit::<bindings::list_head>::uninit();
>> +//! #
>> +//! # let head = head.as_mut_ptr();
>> +//! # // SAFETY: `head` and all the items are test objects allocated in this scope.
>> +//! # unsafe { bindings::INIT_LIST_HEAD(head) };
>> +//! #
>> +//! # let mut items = [
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! #     MaybeUninit::<SampleItemC>::uninit(),
>> +//! # ];
>> +//! #
>> +//! # for (i, item) in items.iter_mut().enumerate() {
>> +//! #     let ptr = item.as_mut_ptr();
>> +//! #     // SAFETY: `ptr` points to a valid `MaybeUninit<SampleItemC>`.
>> +//! #     unsafe { (*ptr).value = i as i32 * 10 };
>> +//! #     // SAFETY: `&raw mut` creates a pointer valid for `INIT_LIST_HEAD`.
>> +//! #     unsafe { bindings::INIT_LIST_HEAD(&raw mut (*ptr).link) };
>> +//! #     // SAFETY: `link` was just initialized and `head` is a valid list head.
>> +//! #     unsafe { bindings::list_add_tail(&mut (*ptr).link, head) };
>> +//! # }
>> +//!
>> +//! //
>> +//! /// Rust wrapper for the C struct.
>> +//! ///
>> +//! /// The list item struct in this example is defined in C code as:
>> +//! ///
>> +//! /// ```c
>> +//! /// struct SampleItemC {
>> +//! ///     int value;
>> +//! ///     struct list_head link;
>> +//! /// };
>> +//! /// ```
>> +//! #[repr(transparent)]
>> +//! pub struct Item(Opaque<SampleItemC>);
>> +//!
>> +//! impl Item {
>> +//!     pub fn value(&self) -> i32 {
>> +//!         // SAFETY: `Item` has same layout as `SampleItemC`.
>> +//!         unsafe { (*self.0.get()).value }
>> +//!     }
>> +//! }
>> +//!
>> +//!
>> +//! // Create typed [`CList`] from sentinel head.
>> +//! // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>> +//! // embedded `link` field, and `Item` is `#[repr(transparent)]` over `SampleItemC`.
>> +//! let list = clist_create!(unsafe { head, Item, SampleItemC, link });
> 
> Bad news.
> 
> My build triggers this warning:
> 
> error: statement has unnecessary safety comment
>     --> rust/doctests_kernel_generated.rs:7103:1
>      |
> 7103 | let list = clist_create!(unsafe { head, Item, SampleItemC, link });
>      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>      |
> help: consider removing the safety comment
>     --> rust/doctests_kernel_generated.rs:7101:4
>      |
> 7101 | // SAFETY: `head` is valid and initialized, items are `SampleItemC` with
>      |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>      = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.94.0/index.html#unnecessary_safety_comment
>      = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`
>      = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]`
> 
> This probably needs to be:
> 
> 	unsafe { clist_create!(head, Item, SampleItemC, link) }

Oops, I sometimes run clippy wrong. I did: "CLIPPY=1 make" instead of "make
CLIPPY=1", the reason is habit. I have been doing "LLVM=1 make" for llvm
builds since many years.

Anyway, the fix is simple, just need to do // SAFETY*: as Miguel suggests
here, instead of // SAFETY:
https://lore.kernel.org/all/CANiq72kEnDyUpnWMZmheJytjioeiJUK_C-yQJk77dPid89LExw@mail.gmail.com/

Should be easy to do on apply if Danilo doesn't mind. :) I verified on my
setup that it fixes it.

thanks,

--
Joel Fernandes

^ permalink raw reply

* Re: [PATCH v13 1/1] rust: interop: Add list module for C linked list interface
From: Joel Fernandes @ 2026-03-18 17:58 UTC (permalink / raw)
  To: Gary Guo, Alice Ryhl
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Alex Gaynor,
	Danilo Krummrich, Dave Airlie, David Airlie, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Simona Vetter, Daniel Almeida,
	Koen Koning, Nikola Djukic, Alexandre Courbot, Philipp Stanner,
	Elle Rhumsaa, Jonathan Corbet, Alex Deucher, Christian König,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Huang Rui, Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellström, Helge Deller, John Hubbard,
	Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, alexeyi, Eliot Courtney,
	dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <DH5Y1UR318WP.VNY18WXEZHDI@garyguo.net>



On 3/18/2026 9:31 AM, Gary Guo wrote:
> On Wed Mar 18, 2026 at 8:59 AM GMT, Alice Ryhl wrote:
>> On Tue, Mar 17, 2026 at 04:18:46PM -0400, Joel Fernandes wrote:
>>>
>>>
>>> On 3/17/2026 4:17 PM, Joel Fernandes wrote:
>>>> Add a new module `kernel::interop::list` for working with C's doubly
>>>> circular linked lists. Provide low-level iteration over list nodes.
>>>>
>>>> Typed iteration over actual items is provided with a `clist_create`
>>>> macro to assist in creation of the `CList` type.
>>>>
>>>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> Acked-by: Gary Guo <gary@garyguo.net>
>>>> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>>> ---
>>>>  MAINTAINERS                 |   8 +
>>>>  rust/helpers/helpers.c      |   1 +
>>>>  rust/helpers/list.c         |  17 ++
>>>>  rust/kernel/interop.rs      |   9 +
>>>>  rust/kernel/interop/list.rs | 342 ++++++++++++++++++++++++++++++++++++
>>>>  rust/kernel/lib.rs          |   2 +
>>>>  6 files changed, 379 insertions(+)
>>>>  create mode 100644 rust/helpers/list.c
>>>>  create mode 100644 rust/kernel/interop.rs
>>>>  create mode 100644 rust/kernel/interop/list.rs
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 4bd6b538a51f..e847099efcc2 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -23251,6 +23251,14 @@ T:	git https://github.com/Rust-for-Linux/linux.git alloc-next
>>>>  F:	rust/kernel/alloc.rs
>>>>  F:	rust/kernel/alloc/
>>>>  
>>>> +RUST [INTEROP]
>>>> +M:	Joel Fernandes <joelagnelf@nvidia.com>
>>>> +M:	Alexandre Courbot <acourbot@nvidia.com>
>>>> +L:	rust-for-linux@vger.kernel.org
>>>> +S:	Maintained
>>>> +T:	git https://github.com/Rust-for-Linux/linux.git interop-next
>>>> +F:	rust/kernel/interop/
>>>
>>> Sorry, I forgot to add an additional F: for the rust/kernel/interop.rs file.
>>> Danilo/Miguel, do you mind adding this when applying?
>>
>> I think you should consider a mod.rs file to avoid this. It's tiny, and
>> just re-exports submodules, so I don't think the "mod.rs name in file
>> view" concern is that big, and IMO having files related to interop/
>> inside the directory is much better than having them outside.
>>
>> Alice
> 
> I wanted this for all modules in general. For modules that grow into multiple
> files we should really use mod.rs and avoid both module_name.rs and the
> module_name directory.
> 

This is how it was, and I changed it based on Alex's feedback:

https://lore.kernel.org/all/DH3XD8NUDJNG.2IMPYC40D8DXI@nvidia.com/

I am Ok with both approaches. I would request Danilo if he's applying it,
that if he could do so without my additional re-send.

thanks,

--
Joel Fernandes




^ permalink raw reply

* [PATCH v2 3/3] staging: sm750fb: Rename struct sm750_dev members to snake_case
From: Shubham Chakraborty @ 2026-03-18 17:34 UTC (permalink / raw)
  To: gregkh, sudipm.mukherjee, teddy.wang
  Cc: linux-fbdev, linux-staging, linux-kernel, Shubham Chakraborty
In-Reply-To: <20260318173440.9457-1-chakrabortyshubham66@gmail.com>

Rename CamelCase members pvReg and pvMem to follow the Linux kernel
coding style.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 drivers/staging/sm750fb/sm750.c    | 22 +++++++++++-----------
 drivers/staging/sm750fb/sm750.h    |  4 ++--
 drivers/staging/sm750fb/sm750_hw.c | 20 ++++++++++----------
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c30ffab8a5f3..1c60ba056719 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -624,27 +624,27 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
 		output->paths = sm750_pnc;
 		crtc->channel = sm750_primary;
 		crtc->o_screen = 0;
-		crtc->v_screen = sm750_dev->pvMem;
+		crtc->v_screen = sm750_dev->pv_mem;
 		pr_info("use simul primary mode\n");
 		break;
 	case sm750_simul_sec:
 		output->paths = sm750_pnc;
 		crtc->channel = sm750_secondary;
 		crtc->o_screen = 0;
-		crtc->v_screen = sm750_dev->pvMem;
+		crtc->v_screen = sm750_dev->pv_mem;
 		break;
 	case sm750_dual_normal:
 		if (par->index == 0) {
 			output->paths = sm750_panel;
 			crtc->channel = sm750_primary;
 			crtc->o_screen = 0;
-			crtc->v_screen = sm750_dev->pvMem;
+			crtc->v_screen = sm750_dev->pv_mem;
 		} else {
 			output->paths = sm750_crt;
 			crtc->channel = sm750_secondary;
 			/* not consider of padding stuffs for o_screen,need fix */
 			crtc->o_screen = sm750_dev->vidmem_size >> 1;
-			crtc->v_screen = sm750_dev->pvMem + crtc->o_screen;
+			crtc->v_screen = sm750_dev->pv_mem + crtc->o_screen;
 		}
 		break;
 	case sm750_dual_swap:
@@ -652,7 +652,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
 			output->paths = sm750_panel;
 			crtc->channel = sm750_secondary;
 			crtc->o_screen = 0;
-			crtc->v_screen = sm750_dev->pvMem;
+			crtc->v_screen = sm750_dev->pv_mem;
 		} else {
 			output->paths = sm750_crt;
 			crtc->channel = sm750_primary;
@@ -660,7 +660,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
 			 * need fix
 			 */
 			crtc->o_screen = sm750_dev->vidmem_size >> 1;
-			crtc->v_screen = sm750_dev->pvMem + crtc->o_screen;
+			crtc->v_screen = sm750_dev->pv_mem + crtc->o_screen;
 		}
 		break;
 	default:
@@ -764,14 +764,14 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
 	 * must be set after crtc member initialized
 	 */
 	crtc->cursor.offset = crtc->o_screen + crtc->vidmem_size - 1024;
-	crtc->cursor.mmio = sm750_dev->pvReg +
+	crtc->cursor.mmio = sm750_dev->pv_reg +
 		0x800f0 + (int)crtc->channel * 0x140;
 
 	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
 	crtc->cursor.max_h = 64;
 	crtc->cursor.max_w = 64;
 	crtc->cursor.size = crtc->cursor.max_h * crtc->cursor.max_w * 2 / 8;
-	crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;
+	crtc->cursor.vstart = sm750_dev->pv_mem + crtc->cursor.offset;
 
 	memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
 	if (!g_hwcursor)
@@ -1090,7 +1090,7 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 		sm750_dev->mtrr.vram = arch_phys_wc_add(sm750_dev->vidmem_start,
 							sm750_dev->vidmem_size);
 
-	memset_io(sm750_dev->pvMem, 0, sm750_dev->vidmem_size);
+	memset_io(sm750_dev->pv_mem, 0, sm750_dev->vidmem_size);
 
 	pci_set_drvdata(pdev, sm750_dev);
 
@@ -1121,8 +1121,8 @@ static void lynxfb_pci_remove(struct pci_dev *pdev)
 	sm750fb_framebuffer_release(sm750_dev);
 	arch_phys_wc_del(sm750_dev->mtrr.vram);
 
-	iounmap(sm750_dev->pvReg);
-	iounmap(sm750_dev->pvMem);
+	iounmap(sm750_dev->pv_reg);
+	iounmap(sm750_dev->pv_mem);
 	kfree(g_settings);
 }
 
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index b683a82af349..3f6570fc8f08 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -97,8 +97,8 @@ struct sm750_dev {
 	unsigned long vidreg_start;
 	__u32 vidmem_size;
 	__u32 vidreg_size;
-	void __iomem *pvReg;
-	unsigned char __iomem *pvMem;
+	void __iomem *pv_reg;
+	unsigned char __iomem *pv_mem;
 	/* locks*/
 	spinlock_t slock;
 
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 0e594734a8b9..32b3813d0d8b 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -49,19 +49,19 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
 	}
 
 	/* now map mmio and vidmem */
-	sm750_dev->pvReg =
+	sm750_dev->pv_reg =
 		ioremap(sm750_dev->vidreg_start, sm750_dev->vidreg_size);
-	if (!sm750_dev->pvReg) {
+	if (!sm750_dev->pv_reg) {
 		pr_err("mmio failed\n");
 		ret = -EFAULT;
 		goto exit;
 	}
-	pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
+	pr_info("mmio virtual addr = %p\n", sm750_dev->pv_reg);
 
-	sm750_dev->accel.dpr_base = sm750_dev->pvReg + DE_BASE_ADDR_TYPE1;
-	sm750_dev->accel.dp_port_base = sm750_dev->pvReg + DE_PORT_ADDR_TYPE1;
+	sm750_dev->accel.dpr_base = sm750_dev->pv_reg + DE_BASE_ADDR_TYPE1;
+	sm750_dev->accel.dp_port_base = sm750_dev->pv_reg + DE_PORT_ADDR_TYPE1;
 
-	mmio750 = sm750_dev->pvReg;
+	mmio750 = sm750_dev->pv_reg;
 	sm750_set_chip_type(sm750_dev->devid, sm750_dev->revid);
 
 	sm750_dev->vidmem_start = pci_resource_start(pdev, 0);
@@ -76,15 +76,15 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
 		sm750_dev->vidmem_start, sm750_dev->vidmem_size);
 
 	/* reserve the vidmem space of smi adaptor */
-	sm750_dev->pvMem =
+	sm750_dev->pv_mem =
 		ioremap_wc(sm750_dev->vidmem_start, sm750_dev->vidmem_size);
-	if (!sm750_dev->pvMem) {
-		iounmap(sm750_dev->pvReg);
+	if (!sm750_dev->pv_mem) {
+		iounmap(sm750_dev->pv_reg);
 		pr_err("Map video memory failed\n");
 		ret = -EFAULT;
 		goto exit;
 	}
-	pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
+	pr_info("video memory vaddr = %p\n", sm750_dev->pv_mem);
 exit:
 	return ret;
 }
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 2/3] staging: sm750fb: Rename struct init_status members to snake_case
From: Shubham Chakraborty @ 2026-03-18 17:34 UTC (permalink / raw)
  To: gregkh, sudipm.mukherjee, teddy.wang
  Cc: linux-fbdev, linux-staging, linux-kernel, Shubham Chakraborty
In-Reply-To: <20260318173440.9457-1-chakrabortyshubham66@gmail.com>

Rename CamelCase members powerMode, setAllEngOff, and resetMemory
to follow the Linux kernel coding style.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 6 +++---
 drivers/staging/sm750fb/sm750.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 729c34372a1e..c30ffab8a5f3 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -921,9 +921,9 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
 	sm750_dev->init_parm.chip_clk = 0;
 	sm750_dev->init_parm.mem_clk = 0;
 	sm750_dev->init_parm.master_clk = 0;
-	sm750_dev->init_parm.powerMode = 0;
-	sm750_dev->init_parm.setAllEngOff = 0;
-	sm750_dev->init_parm.resetMemory = 1;
+	sm750_dev->init_parm.power_mode = 0;
+	sm750_dev->init_parm.set_all_eng_off = 0;
+	sm750_dev->init_parm.reset_memory = 1;
 
 	/* defaultly turn g_hwcursor on for both view */
 	g_hwcursor = 3;
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 49a79d0a8a2e..b683a82af349 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -39,13 +39,13 @@ enum sm750_path {
 };
 
 struct init_status {
-	ushort powerMode;
+	ushort power_mode;
 	/* below three clocks are in unit of MHZ*/
 	ushort chip_clk;
 	ushort mem_clk;
 	ushort master_clk;
-	ushort setAllEngOff;
-	ushort resetMemory;
+	ushort set_all_eng_off;
+	ushort reset_memory;
 };
 
 struct lynx_accel {
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 1/3] staging: sm750fb: Rename enum sm750_pnltype values to snake_case
From: Shubham Chakraborty @ 2026-03-18 17:34 UTC (permalink / raw)
  To: gregkh, sudipm.mukherjee, teddy.wang
  Cc: linux-fbdev, linux-staging, linux-kernel, Shubham Chakraborty
In-Reply-To: <20260318173440.9457-1-chakrabortyshubham66@gmail.com>

This patch renames the CamelCase enum values in sm750_pnltype to
follow the Linux kernel coding style.

Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
---
 drivers/staging/sm750fb/sm750.c    | 6 +++---
 drivers/staging/sm750fb/sm750.h    | 6 +++---
 drivers/staging/sm750fb/sm750_hw.c | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index dec1f6b88a7d..729c34372a1e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -942,11 +942,11 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
 		} else if (!strncmp(opt, "nocrt", strlen("nocrt"))) {
 			sm750_dev->nocrt = 1;
 		} else if (!strncmp(opt, "36bit", strlen("36bit"))) {
-			sm750_dev->pnltype = sm750_doubleTFT;
+			sm750_dev->pnltype = SM750_DOUBLE_TFT;
 		} else if (!strncmp(opt, "18bit", strlen("18bit"))) {
-			sm750_dev->pnltype = sm750_dualTFT;
+			sm750_dev->pnltype = SM750_DUAL_TFT;
 		} else if (!strncmp(opt, "24bit", strlen("24bit"))) {
-			sm750_dev->pnltype = sm750_24TFT;
+			sm750_dev->pnltype = SM750_24TFT;
 		} else if (!strncmp(opt, "nohwc0", strlen("nohwc0"))) {
 			g_hwcursor &= ~0x1;
 		} else if (!strncmp(opt, "nohwc1", strlen("nohwc1"))) {
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 67b9bfa23f41..49a79d0a8a2e 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -13,9 +13,9 @@
 #endif
 
 enum sm750_pnltype {
-	sm750_24TFT = 0,	/* 24bit tft */
-	sm750_dualTFT = 2,	/* dual 18 bit tft */
-	sm750_doubleTFT = 1,	/* 36 bit double pixel tft */
+	SM750_24TFT = 0,	/* 24bit tft */
+	SM750_DUAL_TFT = 2,	/* dual 18 bit tft */
+	SM750_DOUBLE_TFT = 1,	/* 36 bit double pixel tft */
 };
 
 /* vga channel is not concerned  */
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index a29faee91c78..0e594734a8b9 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -134,12 +134,12 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
 		      ~(PANEL_DISPLAY_CTRL_DUAL_DISPLAY |
 			PANEL_DISPLAY_CTRL_DOUBLE_PIXEL);
 		switch (sm750_dev->pnltype) {
-		case sm750_24TFT:
+		case SM750_24TFT:
 			break;
-		case sm750_doubleTFT:
+		case SM750_DOUBLE_TFT:
 			val |= PANEL_DISPLAY_CTRL_DOUBLE_PIXEL;
 			break;
-		case sm750_dualTFT:
+		case SM750_DUAL_TFT:
 			val |= PANEL_DISPLAY_CTRL_DUAL_DISPLAY;
 			break;
 		}
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2 0/3] staging: sm750fb: Clean up CamelCase names
From: Shubham Chakraborty @ 2026-03-18 17:34 UTC (permalink / raw)
  To: gregkh, sudipm.mukherjee, teddy.wang
  Cc: linux-fbdev, linux-staging, linux-kernel, Shubham Chakraborty

This patch series renames various CamelCase enums and struct members in
the sm750fb driver to standard snake_case to comply with the Linux
kernel coding style.

Changes in v2:
- Wrapped all commit message changelogs to 72 columns to fix 
  formatting as requested by Greg K-H.
- No changes to the actual code.

Shubham Chakraborty (3):
  staging: sm750fb: Rename enum sm750_pnltype values to snake_case
  staging: sm750fb: Rename struct init_status members to snake_case
  staging: sm750fb: Rename struct sm750_dev members to snake_case

 drivers/staging/sm750fb/sm750.c    | 34 +++++++++++++++---------------
 drivers/staging/sm750fb/sm750.h    | 16 +++++++-------
 drivers/staging/sm750fb/sm750_hw.c | 26 +++++++++++------------
 3 files changed, 38 insertions(+), 38 deletions(-)

-- 
2.53.0


^ permalink raw reply

* Re: [PATCH 1/3] staging: sm750fb: Rename enum sm750_pnltype values to snake_case
From: Greg Kroah-Hartman @ 2026-03-18 15:24 UTC (permalink / raw)
  To: Shubham Chakraborty
  Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
	linux-kernel
In-Reply-To: <20260314080934.135457-1-chakrabortyshubham66@gmail.com>

On Sat, Mar 14, 2026 at 01:39:30PM +0530, Shubham Chakraborty wrote:
> This patch renames the CamelCase enum values in sm750_pnltype to follow the Linux kernel coding style.

Please wrap your changelogs at 72 columns.

thanks,

greg k-h

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox