* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Danilo Krummrich @ 2026-02-06 15:53 UTC (permalink / raw)
To: Gary Guo
Cc: Joel Fernandes, linux-kernel, 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, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa,
Daniel Almeida, joel, nouveau, dri-devel, rust-for-linux,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG7ZF1UT98RQ.3F42J3ULGV2OC@garyguo.net>
On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>> index 22dd29cd50b5..2c3dec070645 100644
>> --- a/drivers/gpu/Kconfig
>> +++ b/drivers/gpu/Kconfig
>> @@ -1,7 +1,14 @@
>> # SPDX-License-Identifier: GPL-2.0
>>
>> +config RUST_CLIST
>> + bool
>> + depends on RUST
>> + help
>> + Rust abstraction for interfacing with C linked lists.
>
> I am not sure if we need extra config entry. This is fully generic so shouldn't
> generate any code unless there is an user.
I also don't think we need a Kconfig for this.
In any case, it shouln't be in drivers/gpu/Kconfig.
>> +
>> config GPU_BUDDY
>> bool
>> + select RUST_CLIST if RUST
If we will have a Kconfig for this, this belongs in the GPU buddy patch.
>> help
>> A page based buddy allocator for GPU memory.
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 16:05 UTC (permalink / raw)
To: Danilo Krummrich, Gary Guo
Cc: linux-kernel, 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, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DG800TDA6OXQ.275PMMS19F1EX@kernel.org>
On 2/6/2026 10:53 AM, Danilo Krummrich wrote:
> On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
>> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>>> index 22dd29cd50b5..2c3dec070645 100644
>>> --- a/drivers/gpu/Kconfig
>>> +++ b/drivers/gpu/Kconfig
>>> @@ -1,7 +1,14 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>>
>>> +config RUST_CLIST
>>> + bool
>>> + depends on RUST
>>> + help
>>> + Rust abstraction for interfacing with C linked lists.
>>
>> I am not sure if we need extra config entry. This is fully generic so shouldn't
>> generate any code unless there is an user.
>
> I also don't think we need a Kconfig for this.
>
> In any case, it shouln't be in drivers/gpu/Kconfig.
Fair point, I believe I was having trouble compiling this into the kernel crate
without warnings (I believe if !GPU_BUDDY). I'll try to drop it and see if we
can get rid of it.
>
>>> +
>>> config GPU_BUDDY
>>> bool
>>> + select RUST_CLIST if RUST
>
> If we will have a Kconfig for this, this belongs in the GPU buddy patch.
You mean, in the GPU buddy bindings patch right? If so, yes, I will move it there.
Thanks.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-02-06 16:13 UTC (permalink / raw)
To: Joel Fernandes, Danilo Krummrich, Gary Guo
Cc: linux-kernel, 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, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <77ac3274-a962-469d-a2f6-6ccc0670988a@nvidia.com>
On Fri Feb 6, 2026 at 4:05 PM GMT, Joel Fernandes wrote:
>
>
> On 2/6/2026 10:53 AM, Danilo Krummrich wrote:
>> On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
>>> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>>>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>>>> index 22dd29cd50b5..2c3dec070645 100644
>>>> --- a/drivers/gpu/Kconfig
>>>> +++ b/drivers/gpu/Kconfig
>>>> @@ -1,7 +1,14 @@
>>>> # SPDX-License-Identifier: GPL-2.0
>>>>
>>>> +config RUST_CLIST
>>>> + bool
>>>> + depends on RUST
>>>> + help
>>>> + Rust abstraction for interfacing with C linked lists.
>>>
>>> I am not sure if we need extra config entry. This is fully generic so shouldn't
>>> generate any code unless there is an user.
>>
>> I also don't think we need a Kconfig for this.
>>
>> In any case, it shouln't be in drivers/gpu/Kconfig.
>
> Fair point, I believe I was having trouble compiling this into the kernel crate
> without warnings (I believe if !GPU_BUDDY). I'll try to drop it and see if we
> can get rid of it.
If you run into dead code warnings, I think it is fine to just
#[allow(dead_code, reason = "all users might be cfg-ed out")]
the overhead of just let rustc type-checking this module isn't worth the extra
Kconfig plumbing, I think.
Best,
Gary
>
>>
>>>> +
>>>> config GPU_BUDDY
>>>> bool
>>>> + select RUST_CLIST if RUST
>>
>> If we will have a Kconfig for this, this belongs in the GPU buddy patch.
>
> You mean, in the GPU buddy bindings patch right? If so, yes, I will move it there.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Danilo Krummrich @ 2026-02-06 17:13 UTC (permalink / raw)
To: Gary Guo
Cc: Joel Fernandes, linux-kernel, 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, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa,
Daniel Almeida, joel, nouveau, dri-devel, rust-for-linux,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG80FV3VTT6P.2ZP18EM8605GT@garyguo.net>
On Fri Feb 6, 2026 at 5:13 PM CET, Gary Guo wrote:
> On Fri Feb 6, 2026 at 4:05 PM GMT, Joel Fernandes wrote:
>>
>>
>> On 2/6/2026 10:53 AM, Danilo Krummrich wrote:
>>> On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
>>>> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>>>>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>>>>> index 22dd29cd50b5..2c3dec070645 100644
>>>>> --- a/drivers/gpu/Kconfig
>>>>> +++ b/drivers/gpu/Kconfig
>>>>> @@ -1,7 +1,14 @@
>>>>> # SPDX-License-Identifier: GPL-2.0
>>>>>
>>>>> +config RUST_CLIST
>>>>> + bool
>>>>> + depends on RUST
>>>>> + help
>>>>> + Rust abstraction for interfacing with C linked lists.
>>>>
>>>> I am not sure if we need extra config entry. This is fully generic so shouldn't
>>>> generate any code unless there is an user.
>>>
>>> I also don't think we need a Kconfig for this.
>>>
>>> In any case, it shouln't be in drivers/gpu/Kconfig.
>>
>> Fair point, I believe I was having trouble compiling this into the kernel crate
>> without warnings (I believe if !GPU_BUDDY). I'll try to drop it and see if we
>> can get rid of it.
>
> If you run into dead code warnings, I think it is fine to just
>
> #[allow(dead_code, reason = "all users might be cfg-ed out")]
>
> the overhead of just let rustc type-checking this module isn't worth the extra
> Kconfig plumbing, I think.
You mean because there are pub(crate) in clist.rs? I don't think the Kconfig
would help with that, nothing prevents people from enabling RUST_CLIST, but none
of the users.
Besides that, once we have the new build system, the users of CList are likely
in other crates anyways, so I think we should just change things to pub.
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-02-06 17:20 UTC (permalink / raw)
To: Danilo Krummrich, Gary Guo
Cc: Joel Fernandes, linux-kernel, 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, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa,
Daniel Almeida, joel, nouveau, dri-devel, rust-for-linux,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG81PJ9QD8FC.2NF6VEKDD3F2Q@kernel.org>
On Fri Feb 6, 2026 at 5:13 PM GMT, Danilo Krummrich wrote:
> On Fri Feb 6, 2026 at 5:13 PM CET, Gary Guo wrote:
>> On Fri Feb 6, 2026 at 4:05 PM GMT, Joel Fernandes wrote:
>>>
>>>
>>> On 2/6/2026 10:53 AM, Danilo Krummrich wrote:
>>>> On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
>>>>> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>>>>>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>>>>>> index 22dd29cd50b5..2c3dec070645 100644
>>>>>> --- a/drivers/gpu/Kconfig
>>>>>> +++ b/drivers/gpu/Kconfig
>>>>>> @@ -1,7 +1,14 @@
>>>>>> # SPDX-License-Identifier: GPL-2.0
>>>>>>
>>>>>> +config RUST_CLIST
>>>>>> + bool
>>>>>> + depends on RUST
>>>>>> + help
>>>>>> + Rust abstraction for interfacing with C linked lists.
>>>>>
>>>>> I am not sure if we need extra config entry. This is fully generic so shouldn't
>>>>> generate any code unless there is an user.
>>>>
>>>> I also don't think we need a Kconfig for this.
>>>>
>>>> In any case, it shouln't be in drivers/gpu/Kconfig.
>>>
>>> Fair point, I believe I was having trouble compiling this into the kernel crate
>>> without warnings (I believe if !GPU_BUDDY). I'll try to drop it and see if we
>>> can get rid of it.
>>
>> If you run into dead code warnings, I think it is fine to just
>>
>> #[allow(dead_code, reason = "all users might be cfg-ed out")]
>>
>> the overhead of just let rustc type-checking this module isn't worth the extra
>> Kconfig plumbing, I think.
>
> You mean because there are pub(crate) in clist.rs? I don't think the Kconfig
> would help with that, nothing prevents people from enabling RUST_CLIST, but none
> of the users.
>
> Besides that, once we have the new build system, the users of CList are likely
> in other crates anyways, so I think we should just change things to pub.
I asked for this to be changed to `pub(crate)` because I think this isn't
something that should be used by drivers.
As you said, tt might be tricky to enforce that with new build system when
subsystems are inside different crates. But until then I think it's better to
limit visibility.
Best,
Gary
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Danilo Krummrich @ 2026-02-06 17:27 UTC (permalink / raw)
To: Gary Guo
Cc: Joel Fernandes, linux-kernel, 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, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa,
Daniel Almeida, joel, nouveau, dri-devel, rust-for-linux,
linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG81V8NG5RC7.38EYLHQZXKTTO@garyguo.net>
On Fri Feb 6, 2026 at 6:20 PM CET, Gary Guo wrote:
> I asked for this to be changed to `pub(crate)` because I think this isn't
> something that should be used by drivers.
>
> As you said, tt might be tricky to enforce that with new build system when
> subsystems are inside different crates. But until then I think it's better to
> limit visibility.
It should *usually* not be used by drivers, but there are exceptions. For
instance, it is perfectly valid to be used by Rust drivers that interact with C
drivers.
Besides that, my take on this is that we know that the new build system will
come and that pub(crate) won't work in the long term, so why bother. I'd only
use it in cases where you want to keep things local to the subsystem.
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 17:49 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260206004110.1914814-2-joelagnelf@nvidia.com>
Hi Joel,
> On 5 Feb 2026, at 21:41, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Add a new module `clist` 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.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> MAINTAINERS | 7 +
> drivers/gpu/Kconfig | 7 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/list.c | 21 +++
> rust/kernel/clist.rs | 315 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 2 +
> 6 files changed, 353 insertions(+)
> create mode 100644 rust/helpers/list.c
> create mode 100644 rust/kernel/clist.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 900fc00b73e6..310bb479260c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23204,6 +23204,13 @@ S: Maintained
> T: git https://github.com/Rust-for-Linux/linux.git rust-analyzer-next
> F: scripts/generate_rust_analyzer.py
>
> +RUST TO C LIST INTERFACES
> +M: Joel Fernandes <joelagnelf@nvidia.com>
> +M: Alexandre Courbot <acourbot@nvidia.com>
> +L: rust-for-linux@vger.kernel.org
> +S: Maintained
> +F: rust/kernel/clist.rs
> +
> RXRPC SOCKETS (AF_RXRPC)
> M: David Howells <dhowells@redhat.com>
> M: Marc Dionne <marc.dionne@auristor.com>
> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
> index 22dd29cd50b5..2c3dec070645 100644
> --- a/drivers/gpu/Kconfig
> +++ b/drivers/gpu/Kconfig
> @@ -1,7 +1,14 @@
> # SPDX-License-Identifier: GPL-2.0
>
> +config RUST_CLIST
> + bool
> + depends on RUST
> + help
> + Rust abstraction for interfacing with C linked lists.
> +
> config GPU_BUDDY
> bool
> + select RUST_CLIST if RUST
> help
> A page based buddy allocator for GPU memory.
>
> 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..3390b154fa36
> --- /dev/null
> +++ b/rust/helpers/list.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for C Circular doubly linked list implementation.
> + */
> +
> +#include <linux/list.h>
> +
> +#ifndef __rust_helper
> +#define __rust_helper
> +#endif
> +
> +__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/clist.rs b/rust/kernel/clist.rs
> new file mode 100644
> index 000000000000..1f6d4db13c1d
> --- /dev/null
> +++ b/rust/kernel/clist.rs
> @@ -0,0 +1,315 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A C doubly circular intrusive linked list interface for rust code.
> +//!
> +//! # 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(crate) 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: pointers are to allocated test objects with a list_head field.
> +//! # unsafe {
> +//! # (*ptr).value = i as i32 * 10;
> +//! # // addr_of_mut!() computes address of link directly as link is uninitialized.
> +//! # bindings::INIT_LIST_HEAD(core::ptr::addr_of_mut!((*ptr).link));
Shoudn’t this be &raw mut?
> +//! # 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:
> +//! // struct SampleItemC {
> +//! // int value;
> +//! // struct list_head link;
> +//! // };
> +//! //
> +//! #[repr(transparent)]
> +//! pub(crate) struct Item(Opaque<SampleItemC>);
> +//!
> +//! impl Item {
> +//! pub(crate) 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, items are [`SampleItemC`] with embedded `link` field.
> +//! let list = unsafe { clist_create!(head, Item, SampleItemC, link) };
> +//!
> +//! // Iterate directly over typed items.
> +//! let mut found_0 = false;
> +//! let mut found_10 = false;
> +//! let mut found_20 = false;
> +//!
> +//! for item in list.iter() {
> +//! let val = item.value();
> +//! if val == 0 { found_0 = true; }
> +//! if val == 10 { found_10 = true; }
> +//! if val == 20 { found_20 = true; }
> +//! }
> +//!
> +//! assert!(found_0 && found_10 && found_20);
> +//! ```
> +
> +use core::{
> + iter::FusedIterator,
> + marker::PhantomData, //
> +};
> +
> +use crate::{
> + bindings,
> + types::Opaque, //
> +};
> +
> +use pin_init::PinInit;
> +
> +/// Wraps a `list_head` object for use in intrusive linked lists.
> +///
> +/// # Invariants
> +///
> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
Can you expand on the two points above?
> +#[repr(transparent)]
> +pub(crate) struct CListHead(Opaque<bindings::list_head>);
> +
> +impl CListHead {
> + /// Create a `&CListHead` reference from a raw `list_head` pointer.
> + ///
> + /// # Safety
> + ///
> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure.
> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> + #[inline]
> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
> + // SAFETY:
> + // - [`CListHead`] has same layout as `list_head`.
> + // - `ptr` is valid and unmodified for 'a.
> + unsafe { &*ptr.cast() }
> + }
> +
> + /// Get the raw `list_head` pointer.
> + #[inline]
> + pub(crate) fn as_raw(&self) -> *mut bindings::list_head {
> + self.0.get()
> + }
> +
> + /// Get the next [`CListHead`] in the list.
> + #[inline]
> + pub(crate) fn next(&self) -> &Self {
> + let raw = self.as_raw();
> + // SAFETY:
> + // - `self.as_raw()` is valid per type invariants.
> + // - The `next` pointer is guaranteed to be non-NULL.
> + unsafe { Self::from_raw((*raw).next) }
> + }
> +
> + /// Check if this node is linked in a list (not isolated).
> + #[inline]
> + pub(crate) fn is_linked(&self) -> bool {
> + let raw = self.as_raw();
> + // SAFETY: self.as_raw() is valid per type invariants.
> + unsafe { (*raw).next != raw && (*raw).prev != raw }
I wonder if this is duplicating some C helper?
> + }
> +
> + /// Pin-initializer that initializes the list head.
> + pub(crate) fn new() -> impl PinInit<Self> {
> + // SAFETY: `INIT_LIST_HEAD` initializes `slot` to a valid empty list.
> + unsafe {
> + pin_init::pin_init_from_closure(move |slot: *mut Self| {
> + bindings::INIT_LIST_HEAD(slot.cast());
> + Ok(())
> + })
> + }
> + }
> +}
> +
> +// SAFETY: [`CListHead`] can be sent to any thread.
> +unsafe impl Send for CListHead {}
> +
> +// SAFETY: [`CListHead`] can be shared among threads as it is not modified
> +// by non-Rust code per type invariants.
> +unsafe impl Sync for CListHead {}
> +
> +impl PartialEq for CListHead {
> + fn eq(&self, other: &Self) -> bool {
> + core::ptr::eq(self, other)
> + }
> +}
> +
> +impl Eq for CListHead {}
> +
> +/// Low-level iterator over `list_head` nodes.
> +///
> +/// An iterator used to iterate over a C intrusive linked list (`list_head`). Caller has to
> +/// perform conversion of returned [`CListHead`] to an item (using `container_of` macro or similar).
> +///
> +/// # Invariants
> +///
> +/// [`CListHeadIter`] is iterating over an allocated, initialized and valid list.
> +struct CListHeadIter<'a> {
> + /// Current position in the list.
> + current: &'a CListHead,
> + /// The sentinel head (used to detect end of iteration).
> + sentinel: &'a CListHead,
> +}
> +
> +impl<'a> Iterator for CListHeadIter<'a> {
> + type Item = &'a CListHead;
> +
> + #[inline]
> + fn next(&mut self) -> Option<Self::Item> {
> + // Check if we've reached the sentinel (end of list).
> + if core::ptr::eq(self.current, self.sentinel) {
> + return None;
> + }
I was under the impression that CListHeads implemented PartialEq/Eq?
> +
> + let item = self.current;
> + self.current = item.next();
> + Some(item)
> + }
> +}
> +
> +impl<'a> FusedIterator for CListHeadIter<'a> {}
> +
> +/// A typed C linked list with a sentinel head.
> +///
> +/// A sentinel head represents the entire linked list and can be used for
> +/// iteration over items of type `T`, it is not associated with a specific item.
> +///
> +/// The const generic `OFFSET` specifies the byte offset of the `list_head` field within
> +/// the struct that `T` wraps.
> +///
> +/// # Invariants
> +///
> +/// - The [`CListHead`] is an allocated and valid sentinel C `list_head` structure.
> +/// - `OFFSET` is the byte offset of the `list_head` field within the struct that `T` wraps.
> +/// - All the list's `list_head` nodes are allocated and have valid next/prev pointers.
> +#[repr(transparent)]
> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
> +
> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
> + ///
> + /// # Safety
> + ///
> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
> + /// representing a list sentinel.
> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
> + /// - `T` must be `#[repr(transparent)]` over the C struct.
> + #[inline]
> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
> + // SAFETY:
> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
> + unsafe { &*ptr.cast() }
> + }
> +
> + /// Check if the list is empty.
> + #[inline]
> + #[expect(dead_code)]
> + pub(crate) fn is_empty(&self) -> bool {
Why can’t this be pub?
> + !self.0.is_linked()
> + }
> +
> + /// Create an iterator over typed items.
> + #[inline]
> + pub(crate) fn iter(&self) -> CListIter<'_, T, OFFSET> {
> + let head = &self.0;
> + CListIter {
> + head_iter: CListHeadIter {
> + current: head.next(),
> + sentinel: head,
> + },
> + _phantom: PhantomData,
> + }
> + }
> +}
> +
> +/// High-level iterator over typed list items.
> +pub(crate) struct CListIter<'a, T, const OFFSET: usize> {
> + head_iter: CListHeadIter<'a>,
> + _phantom: PhantomData<&'a T>,
> +}
> +
> +impl<'a, T, const OFFSET: usize> Iterator for CListIter<'a, T, OFFSET> {
> + type Item = &'a T;
> +
> + fn next(&mut self) -> Option<Self::Item> {
> + let head = self.head_iter.next()?;
> +
> + // Convert to item using OFFSET.
> + // SAFETY: `item_ptr` calculation from `OFFSET` (calculated using offset_of!)
> + // is valid per invariants.
> + Some(unsafe { &*head.as_raw().byte_sub(OFFSET).cast::<T>() })
> + }
> +}
> +
> +impl<'a, T, const OFFSET: usize> FusedIterator for CListIter<'a, T, OFFSET> {}
> +
> +/// 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
> +///
> +/// This is an unsafe macro. The caller must ensure:
> +///
> +/// - `$head` is a valid, initialized sentinel `list_head` pointing to a list that remains
> +/// unmodified for the lifetime of the rust `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 this module's documentation.
> +#[macro_export]
> +macro_rules! clist_create {
> + ($head:expr, $rust_type:ty, $c_type:ty, $($field:tt).+) => {{
> + // Compile-time check that field path is a list_head.
> + let _: fn(*const $c_type) -> *const $crate::bindings::list_head =
> + |p| &raw const (*p).$($field).+;
> +
> + // Calculate offset and create `CList`.
> + const OFFSET: usize = ::core::mem::offset_of!($c_type, $($field).+);
> + $crate::clist::CList::<$rust_type, OFFSET>::from_raw($head)
> + }};
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3da92f18f4ee..8439c30f40b5 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -75,6 +75,8 @@
> pub mod bug;
> #[doc(hidden)]
> pub mod build_assert;
> +#[cfg(CONFIG_RUST_CLIST)]
> +pub(crate) mod clist;
> pub mod clk;
> #[cfg(CONFIG_CONFIGFS_FS)]
> pub mod configfs;
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH] fbdev: au1200fb: Fix a memory leak in au1200fb_drv_probe()
From: Helge Deller @ 2026-02-06 19:32 UTC (permalink / raw)
To: Felix Gu, Zhang Shurong; +Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260203-au1200fb-v1-1-7889c4061337@gmail.com>
On 2/3/26 13:14, Felix Gu wrote:
> In au1200fb_drv_probe(), when platform_get_irq fails(), it directly
> returns from the function with an error code, which causes a memory
> leak.
>
> Replace it with a goto label to ensure proper cleanup.
>
> Fixes: 4e88761f5f8c ("fbdev: au1200fb: Fix missing IRQ check in au1200fb_drv_probe")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> drivers/video/fbdev/au1200fb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 20:44 UTC (permalink / raw)
To: Danilo Krummrich, Gary Guo
Cc: linux-kernel, 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, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, Daniel Almeida, joel, nouveau,
dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DG81PJ9QD8FC.2NF6VEKDD3F2Q@kernel.org>
On 2/6/2026 12:13 PM, Danilo Krummrich wrote:
> On Fri Feb 6, 2026 at 5:13 PM CET, Gary Guo wrote:
>> On Fri Feb 6, 2026 at 4:05 PM GMT, Joel Fernandes wrote:
>>>
>>>
>>> On 2/6/2026 10:53 AM, Danilo Krummrich wrote:
>>>> On Fri Feb 6, 2026 at 4:25 PM CET, Gary Guo wrote:
>>>>> On Fri Feb 6, 2026 at 12:41 AM GMT, Joel Fernandes wrote:
>>>>>> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
>>>>>> index 22dd29cd50b5..2c3dec070645 100644
>>>>>> --- a/drivers/gpu/Kconfig
>>>>>> +++ b/drivers/gpu/Kconfig
>>>>>> @@ -1,7 +1,14 @@
>>>>>> # SPDX-License-Identifier: GPL-2.0
>>>>>>
>>>>>> +config RUST_CLIST
>>>>>> + bool
>>>>>> + depends on RUST
>>>>>> + help
>>>>>> + Rust abstraction for interfacing with C linked lists.
>>>>>
>>>>> I am not sure if we need extra config entry. This is fully generic so shouldn't
>>>>> generate any code unless there is an user.
>>>>
>>>> I also don't think we need a Kconfig for this.
>>>>
>>>> In any case, it shouln't be in drivers/gpu/Kconfig.
>>>
>>> Fair point, I believe I was having trouble compiling this into the kernel crate
>>> without warnings (I believe if !GPU_BUDDY). I'll try to drop it and see if we
>>> can get rid of it.
>>
>> If you run into dead code warnings, I think it is fine to just
>>
>> #[allow(dead_code, reason = "all users might be cfg-ed out")]
>>
>> the overhead of just let rustc type-checking this module isn't worth the extra
>> Kconfig plumbing, I think.
>
> You mean because there are pub(crate) in clist.rs? I don't think the Kconfig
> would help with that, nothing prevents people from enabling RUST_CLIST, but none
> of the users.
I think he means add the alloc annotation to suppress deadcode warnings and get
rid of the Kconfig?
> Besides that, once we have the new build system, the users of CList are likely
> in other crates anyways, so I think we should just change things to pub.
I agree with both approaches. Perhaps changing to pub is better to avoid churn
in the future when other crates use it.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 20:46 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <73B64D35-6574-4776-962D-865465C40226@collabora.com>
On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>> +#[repr(transparent)]
>> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
>> +
>> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
>> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>> + /// representing a list sentinel.
>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>> + /// - `T` must be `#[repr(transparent)]` over the C struct.
>> + #[inline]
>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>> + // SAFETY:
>> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
>> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>> + unsafe { &*ptr.cast() }
>> + }
>> +
>> + /// Check if the list is empty.
>> + #[inline]
>> + #[expect(dead_code)]
>> + pub(crate) fn is_empty(&self) -> bool {
>
> Why can’t this be pub?
I believe this was suggested by Gary. See the other thread where we are
discussing it (with Gary and Danilo) and let us discuss there.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Gary Guo @ 2026-02-06 20:51 UTC (permalink / raw)
To: Joel Fernandes, Daniel Almeida
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <8dde5b79-29d6-4840-be9a-31bc4af27cf9@nvidia.com>
On Fri Feb 6, 2026 at 8:46 PM GMT, Joel Fernandes wrote:
> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>> +#[repr(transparent)]
>>> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
>>> +
>>> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
>>> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>>> + /// representing a list sentinel.
>>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>>> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>>> + /// - `T` must be `#[repr(transparent)]` over the C struct.
>>> + #[inline]
>>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>>> + // SAFETY:
>>> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
>>> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>>> + unsafe { &*ptr.cast() }
>>> + }
>>> +
>>> + /// Check if the list is empty.
>>> + #[inline]
>>> + #[expect(dead_code)]
>>> + pub(crate) fn is_empty(&self) -> bool {
>>
>> Why can’t this be pub?
>
> I believe this was suggested by Gary. See the other thread where we are
> discussing it (with Gary and Danilo) and let us discuss there.
I suggested the module to be `pub(crate)`. For the individual item it is not
necessary if the module itself already have limited visibility.
Best,
Gary
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 20:51 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <73B64D35-6574-4776-962D-865465C40226@collabora.com>
Hi Daniel,
Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
you for all the comments.
On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>> +use crate::{
>> + bindings,
>> + types::Opaque, //
>> +};
>> +
>> +use pin_init::PinInit;
>> +
>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>> +///
>> +/// # Invariants
>> +///
>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>
> Can you expand on the two points above?
This is basically saying that a C `list_head` that is wrapped by a `CListHead`
is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
That is the invariant.
Or did I miss something?
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 21:12 UTC (permalink / raw)
To: Gary Guo, Daniel Almeida
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Balbir Singh, Philipp Stanner, Elle Rhumsaa, joel,
nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DG86CJXCM7DB.1A4F4JTMSS9ZZ@garyguo.net>
On 2/6/2026 3:51 PM, Gary Guo wrote:
> On Fri Feb 6, 2026 at 8:46 PM GMT, Joel Fernandes wrote:
>> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>>> +#[repr(transparent)]
>>>> +pub(crate) struct CList<T, const OFFSET: usize>(CListHead, PhantomData<T>);
>>>> +
>>>> +impl<T, const OFFSET: usize> CList<T, OFFSET> {
>>>> + /// Create a typed [`CList`] reference from a raw sentinel `list_head` pointer.
>>>> + ///
>>>> + /// # Safety
>>>> + ///
>>>> + /// - `ptr` must be a valid pointer to an allocated and initialized `list_head` structure
>>>> + /// representing a list sentinel.
>>>> + /// - `ptr` must remain valid and unmodified for the lifetime `'a`.
>>>> + /// - The list must contain items where the `list_head` field is at byte offset `OFFSET`.
>>>> + /// - `T` must be `#[repr(transparent)]` over the C struct.
>>>> + #[inline]
>>>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::list_head) -> &'a Self {
>>>> + // SAFETY:
>>>> + // - [`CList`] has same layout as [`CListHead`] due to repr(transparent).
>>>> + // - Caller guarantees `ptr` is a valid, sentinel `list_head` object.
>>>> + unsafe { &*ptr.cast() }
>>>> + }
>>>> +
>>>> + /// Check if the list is empty.
>>>> + #[inline]
>>>> + #[expect(dead_code)]
>>>> + pub(crate) fn is_empty(&self) -> bool {
>>>
>>> Why can’t this be pub?
>>
>> I believe this was suggested by Gary. See the other thread where we are
>> discussing it (with Gary and Danilo) and let us discuss there.
>
> I suggested the module to be `pub(crate)`. For the individual item it is not
> necessary if the module itself already have limited visibility.
>
Sure, I can change it to module-level pub then, and drop the pub(crate) if
everyone agrees.
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 21:21 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <7ed85eca-2a5e-4e8f-8356-e7fbbf7d3a8f@nvidia.com>
> On 6 Feb 2026, at 17:51, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Hi Daniel,
> Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
> you for all the comments.
>
> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>> +use crate::{
>>> + bindings,
>>> + types::Opaque, //
>>> +};
>>> +
>>> +use pin_init::PinInit;
>>> +
>>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>>
>> Can you expand on the two points above?
>
> This is basically saying that a C `list_head` that is wrapped by a `CListHead`
> is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
> That is the invariant.
>
> Or did I miss something?
>
> --
> Joel Fernandes
>
>
Yeah, but my point being: is there a reason why the underlying list has to
remain read-only? Is this a safety requirement or an invariant that is established
by the code above?
— Daniel
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Joel Fernandes @ 2026-02-06 21:26 UTC (permalink / raw)
To: Daniel Almeida
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <E846F3BB-DE64-4E6B-ACA3-00F965038478@collabora.com>
On 2/6/2026 4:21 PM, Daniel Almeida wrote:
>
>
>> On 6 Feb 2026, at 17:51, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>
>> Hi Daniel,
>> Hope you do not mind me replying piecemeal as I can reply more quickly. Thank
>> you for all the comments.
>>
>> On 2/6/2026 12:49 PM, Daniel Almeida wrote:
>>>> +use crate::{
>>>> + bindings,
>>>> + types::Opaque, //
>>>> +};
>>>> +
>>>> +use pin_init::PinInit;
>>>> +
>>>> +/// Wraps a `list_head` object for use in intrusive linked lists.
>>>> +///
>>>> +/// # Invariants
>>>> +///
>>>> +/// - [`CListHead`] represents an allocated and valid `list_head` structure.
>>>> +/// - Once a [`CListHead`] is created in Rust, it will not be modified by non-Rust code.
>>>> +/// - All `list_head` for individual items are not modified for the lifetime of [`CListHead`].
>>>
>>> Can you expand on the two points above?
>>
>> This is basically saying that a C `list_head` that is wrapped by a `CListHead`
>> is read-only for the lifetime of `ClistHead`. modifying the pointers anymore.
>> That is the invariant.
>>
>> Or did I miss something?
>>
>> --
>> Joel Fernandes
>>
>>
>
>
> Yeah, but my point being: is there a reason why the underlying list has to
> remain read-only? Is this a safety requirement or an invariant that is established
> by the code above?
I'm not fully sure if it's an invariant or a safety requirement, but anyone
creating a C list head on the rust side must guarantee that it is not modified.
Since rust has no visibility on the C side, I believe it is a Rust invariant
here that the existence of CListHead assumes that the list cannot be modified
once Rust has access over it. That is up to the creator (user) of the CListHead
to guarantee. In the DRM buddy case, once the list is allocated and accessible
from Rust, C code will not modify it while the Rust object exists.
Does that make sense, or is there a better way to document this?
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 21:30 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Gary Guo, Joel Fernandes, linux-kernel, 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, Alice Ryhl, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG820GM5YHJS.11E92OR824CWM@kernel.org>
> On 6 Feb 2026, at 14:27, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri Feb 6, 2026 at 6:20 PM CET, Gary Guo wrote:
>> I asked for this to be changed to `pub(crate)` because I think this isn't
>> something that should be used by drivers.
>>
>> As you said, tt might be tricky to enforce that with new build system when
>> subsystems are inside different crates. But until then I think it's better to
>> limit visibility.
>
> It should *usually* not be used by drivers, but there are exceptions. For
> instance, it is perfectly valid to be used by Rust drivers that interact with C
> drivers.
I agree with what Danilo said here.
I don’t see a reason to forbid drivers from using this. If the reason is
the unsafe bits, then isn’t it the same pattern used by impl_has_work!()
anyways? i.e.: a macro that implements an unsafe trait so long as the driver
gives it the right Work field. Seems equivalent in spirit to the clist_create macro
introduced by this patch.
— Daniel
^ permalink raw reply
* Re: [PATCH -next v7 1/2] rust: clist: Add support to interface with C linked lists
From: Daniel Almeida @ 2026-02-06 22:33 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, 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,
Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Alex Gaynor,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, joel, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <983b7db3-b0e6-45d6-866e-f001b64abde1@nvidia.com>
>>
>>
>> Yeah, but my point being: is there a reason why the underlying list has to
>> remain read-only? Is this a safety requirement or an invariant that is established
>> by the code above?
> I'm not fully sure if it's an invariant or a safety requirement, but anyone
> creating a C list head on the rust side must guarantee that it is not modified.
> Since rust has no visibility on the C side, I believe it is a Rust invariant
> here that the existence of CListHead assumes that the list cannot be modified
> once Rust has access over it. That is up to the creator (user) of the CListHead
> to guarantee. In the DRM buddy case, once the list is allocated and accessible
> from Rust, C code will not modify it while the Rust object exists.
>
> Does that make sense, or is there a better way to document this?
>
> --
> Joel Fernandes
In which case, I recommend moving this to a safety requirement when
creating the list.
I assume the purpose of not modifying the list on the C side is to avoid
corrupting the list in Rust somehow?
— Daniel
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: fix CamelCase and Hungarian prefix in variable names
From: Greg KH @ 2026-02-07 12:43 UTC (permalink / raw)
To: Shreyas Ravi
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204054753.3137479-1-shreyasravi320@gmail.com>
On Tue, Feb 03, 2026 at 09:47:53PM -0800, Shreyas Ravi wrote:
> Fix multiple coding style issues:
> - Rename CamelCase variables to snake_case
> - Drop Hungarian prefixes on variable names
>
> No functional changes.
>
> Signed-off-by: Shreyas Ravi <shreyasravi320@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 132 +++++++++++++-------------
> drivers/staging/sm750fb/sm750.h | 16 ++--
> drivers/staging/sm750fb/sm750_accel.c | 132 +++++++++++++-------------
> drivers/staging/sm750fb/sm750_accel.h | 44 ++++-----
> drivers/staging/sm750fb/sm750_hw.c | 26 ++---
> 5 files changed, 175 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index dec1f6b88a7d..67cba3eb0422 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -30,14 +30,14 @@
> */
>
> /* common var for all device */
> -static int g_hwcursor = 1;
> -static int g_noaccel;
> -static int g_nomtrr;
> -static const char *g_fbmode[] = {NULL, NULL};
> -static const char *g_def_fbmode = "1024x768-32@60";
> -static char *g_settings;
> -static int g_dualview;
> -static char *g_option;
> +static int sm750_hwcursor = 1;
> +static int sm750_noaccel;
> +static int sm750_nomtrr;
> +static const char *sm750_fbmode[] = {NULL, NULL};
> +static const char *sm750_def_fbmode = "1024x768-32@60";
> +static char *sm750_settings;
> +static int sm750_dualview;
> +static char *sm750_option;
Why are static variables prefixed with the driver name? They previously
were not, why add the prefix here?
And you are doing multiple things here, changing Hungarian prefixes AND
CamelCase names, please only do one logical change per patch.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: rename pvMem to pv_mem
From: Greg Kroah-Hartman @ 2026-02-07 13:25 UTC (permalink / raw)
To: dhyaan19022009-hue
Cc: Sudip Mukherjee, Teddy Wang, linux-staging, linux-kernel,
linux-fbdev, dhyaan19022009-hue
In-Reply-To: <20260204062240.20293-1-dhyaan19022009@gmail.com>
On Wed, Feb 04, 2026 at 11:52:40AM +0530, dhyaan19022009-hue wrote:
> Rename the CamelCase variable pvMem to the snake_case pv_mem to
> comply with the Linux kernel coding style. This fixes multiple
> warnings reported by checkpatch.pl.
>
> Signed-off-by: dhyaan19022009-hue <dhyaan19022009@gmail.com>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- It looks like you did not use your "real" name for the patch on either
the Signed-off-by: line, or the From: line (both of which have to
match). Please read the kernel file,
Documentation/process/submitting-patches.rst for how to do this
correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH v3 1/5] staging: sm750fb: replace strcat() with memcpy() in lynxfb_setup()
From: Greg Kroah-Hartman @ 2026-02-07 13:27 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-1-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:05:58PM +0000, Artem Lytkin wrote:
> As part of kernel hardening, I am auditing calls to strcat(). This
> code works but it is a bit ugly.
>
> This function takes a string "options" and allocates "g_settings"
> which is large enough to hold a copy of "options". It copies all the
> options from "options" to "g_settings" except "noaccel", "nomtrr" and
> "dual". The new buffer is large enough to fit all the options so
> there is no buffer overflow in using strcat() here.
>
> However, using strcat() is misleading because "tmp" always points
> to the next unused character in the "g_settings" buffer and it's
> always the NUL character. Use memcpy() instead to make the code
> easier to read. This also removes an instance of strcat() which
> is a #NiceBonus.
>
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index fecd7457e..4c6e84c03 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -1163,7 +1163,7 @@ static int __init lynxfb_setup(char *options)
> } else if (!strncmp(opt, "dual", strlen("dual"))) {
> g_dualview = 1;
> } else {
> - strcat(tmp, opt);
> + memcpy(tmp, opt, strlen(opt));
You are open-coding a call to strcat() here :(
Please don't replace one "warning" with another, this will just cause
code churn over time. If the original code is fine, just leave it
as-is, your change here did not actually do anything at all.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 2/5] staging: sm750fb: use strcmp() for exact option matching
From: Greg Kroah-Hartman @ 2026-02-07 13:31 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-2-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:05:59PM +0000, Artem Lytkin wrote:
> Replace strncmp(opt, "...", strlen("...")) with strcmp() in option
> parsing functions. Options from strsep() are complete null-terminated
> tokens, so prefix matching via strncmp() could cause false positives
> for options like "noaccelXYZ" matching "noaccel".
>
> Fixes: 81dee67e215b ("staging: sm750fb: add sm750 to staging")
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 4c6e84c03..bd2d4a290 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -937,21 +937,21 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
> dev_info(&sm750_dev->pdev->dev, "opt=%s\n", opt);
> dev_info(&sm750_dev->pdev->dev, "src=%s\n", src);
>
> - if (!strncmp(opt, "swap", strlen("swap"))) {
> + if (!strcmp(opt, "swap")) {
While I understand the feeling, again, this really isn't doing anything
except cause other code checkers to go "Wait, we can't call strcmp() we
must replace that with strncmp()!"
Please don't replace one warning with another. Option parsing is a
pain, let's not make it any more of a pain than it is. Ideally all of
the framebuffer drivers could make some "simple" helper functions to
handle this crazy logic for them, instead of forcing them to all do it
manually :(
Yet another reason all of us want to just delete all of these drivers...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 3/5] staging: sm750fb: remove debug and diagnostic prints
From: Greg Kroah-Hartman @ 2026-02-07 13:32 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260204120602.6715-3-iprintercanon@gmail.com>
On Wed, Feb 04, 2026 at 12:06:00PM +0000, Artem Lytkin wrote:
> @@ -811,11 +802,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
> g_fbmode[index],
> mdb_desc[i]);
> break;
> - } else if (ret == 3) {
> - pr_warn("wanna use default mode\n");
> - /*break;*/
> - } else if (ret == 4) {
> - pr_warn("fall back to any valid mode\n");
> + } else if (ret == 3 || ret == 4) {
> + continue;
> } else {
> pr_warn("ret = %d,fb_find_mode failed,with %s\n",
> ret,
Why delete some of these but not all? Why delete any of them?
Consistancy matters :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: rename CamelCase variables in sm750_accel.c
From: Greg Kroah-Hartman @ 2026-02-07 13:34 UTC (permalink / raw)
To: Artem Lytkin
Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260205234808.2232-1-iprintercanon@gmail.com>
On Thu, Feb 05, 2026 at 11:48:08PM +0000, Artem Lytkin wrote:
> Rename CamelCase function parameters and local variables to
> snake_case to comply with kernel coding style:
>
> sBase -> src_base dBase -> dst_base
> sPitch -> src_pitch dPitch -> dst_pitch
> Bpp -> bpp nDirection -> direction
> pSrcbuf -> src_buf srcDelta -> src_delta
> startBit -> start_bit bytePerPixel -> bytes_per_pixel
> fColor -> fg_color bColor -> bg_color
> ulBytesPerScan -> bytes_per_scan
> ul4BytesPerScan -> bytes_per_scan_aligned
> ulBytesRemain -> bytes_remain
> ajRemain -> remain_buf
> write_dpPort -> write_dp_port
>
> Update the corresponding kernel-doc comments and function
> declarations in sm750_accel.h.
>
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
> drivers/staging/sm750fb/sm750_accel.c | 134 +++++++++++++-------------
> drivers/staging/sm750fb/sm750_accel.h | 46 ++++-----
> 2 files changed, 90 insertions(+), 90 deletions(-)
Does not apply to my tree due to others doing the same sort of changes
right at the same time as you were, but their changes came in first,
sorry.
greg k-h
^ permalink raw reply
* [PATCH v4 0/3] staging: sm750fb: clean up logging
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
This series cleans up the logging in the sm750fb driver.
Changes since v3:
- Dropped the strcat and strncmp patches as requested by Greg KH
- Consistently removed all pr_info/pr_debug/pr_warn diagnostic prints
in one patch (previously some fb_find_mode prints were kept)
- Split dev_err conversions into two patches: one for sm750.c
and one for sm750_hw.c
Patch 1 removes all pr_info/pr_debug/pr_warn diagnostic prints
from both sm750.c and sm750_hw.c. These include address dumps,
debug variable prints, fb_find_mode result logging, and CH7301
DVI chip status messages.
Patches 2-3 convert the remaining pr_err() calls to dev_err()
for proper device context.
Artem Lytkin (3):
staging: sm750fb: remove debug and diagnostic prints
staging: sm750fb: convert logging to device-based in sm750.c
staging: sm750fb: convert logging to device-based in sm750_hw.c
drivers/staging/sm750fb/sm750.c | 88 +++---------------------------
drivers/staging/sm750fb/sm750_hw.c | 26 ++-------
2 files changed, 14 insertions(+), 100 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v4 1/3] staging: sm750fb: remove debug and diagnostic prints
From: Artem Lytkin @ 2026-02-07 15:37 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
Cc: linux-fbdev, linux-staging, linux-kernel, Artem Lytkin
In-Reply-To: <20260207153703.2049-1-iprintercanon@gmail.com>
Remove all pr_info, pr_debug, and pr_warn calls that dump internal
variable values, pointer addresses, and structure contents not useful
for production use. This includes the complete fb_find_mode() result
logging in lynxfb_set_fbinfo(), the CH7301 DVI chip status messages
in hw_sm750_inithw(), and various debug prints throughout the driver.
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 76 +-----------------------------
drivers/staging/sm750fb/sm750_hw.c | 18 +------
2 files changed, 3 insertions(+), 91 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index fecd7457e..987ba8772 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -375,7 +375,6 @@ static int lynxfb_ops_set_par(struct fb_info *info)
line_length = var->xres_virtual * var->bits_per_pixel / 8;
line_length = ALIGN(line_length, crtc->line_pad);
fix->line_length = line_length;
- pr_info("fix->line_length = %d\n", fix->line_length);
/*
* var->red,green,blue,transp are need to be set by driver
@@ -485,11 +484,6 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
par = info->par;
crtc = &par->crtc;
- pr_debug("check var:%dx%d-%d\n",
- var->xres,
- var->yres,
- var->bits_per_pixel);
-
ret = lynxfb_set_color_offsets(info);
if (ret) {
@@ -580,7 +574,6 @@ static int lynxfb_ops_blank(int blank, struct fb_info *info)
struct lynxfb_par *par;
struct lynxfb_output *output;
- pr_debug("blank = %d.\n", blank);
par = info->par;
output = &par->output;
sm750_dev = par->dev;
@@ -625,7 +618,6 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
crtc->channel = sm750_primary;
crtc->o_screen = 0;
crtc->v_screen = sm750_dev->pvMem;
- pr_info("use simul primary mode\n");
break;
case sm750_simul_sec:
output->paths = sm750_pnc;
@@ -734,12 +726,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
lynx750_ext, NULL, vesa_modes,
};
int cdb[] = {ARRAY_SIZE(lynx750_ext), 0, VESA_MODEDB_SIZE};
- static const char * const mdb_desc[] = {
- "driver prepared modes",
- "kernel prepared default modedb",
- "kernel HELPERS prepared vesa_modes",
- };
-
static const char *fixId[2] = {
"sm750_fb1", "sm750_fb2",
};
@@ -767,7 +753,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
crtc->cursor.mmio = sm750_dev->pvReg +
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;
@@ -801,47 +786,10 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
ret = fb_find_mode(var, info, g_fbmode[index],
pdb[i], cdb[i], NULL, 8);
- if (ret == 1) {
- pr_info("success! use specified mode:%s in %s\n",
- g_fbmode[index],
- mdb_desc[i]);
+ if (ret == 1 || ret == 2)
break;
- } else if (ret == 2) {
- pr_warn("use specified mode:%s in %s,with an ignored refresh rate\n",
- g_fbmode[index],
- mdb_desc[i]);
- break;
- } else if (ret == 3) {
- pr_warn("wanna use default mode\n");
- /*break;*/
- } else if (ret == 4) {
- pr_warn("fall back to any valid mode\n");
- } else {
- pr_warn("ret = %d,fb_find_mode failed,with %s\n",
- ret,
- mdb_desc[i]);
- }
}
- /* some member of info->var had been set by fb_find_mode */
-
- pr_info("Member of info->var is :\n"
- "xres=%d\n"
- "yres=%d\n"
- "xres_virtual=%d\n"
- "yres_virtual=%d\n"
- "xoffset=%d\n"
- "yoffset=%d\n"
- "bits_per_pixel=%d\n"
- " ...\n",
- var->xres,
- var->yres,
- var->xres_virtual,
- var->yres_virtual,
- var->xoffset,
- var->yoffset,
- var->bits_per_pixel);
-
/* set par */
par->info = info;
@@ -851,7 +799,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
info->pseudo_palette = &par->pseudo_palette[0];
info->screen_base = crtc->v_screen;
- pr_debug("screen_base vaddr = %p\n", info->screen_base);
info->screen_size = line_length * var->yres_virtual;
/* set info->fix */
@@ -865,7 +812,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
strscpy(fix->id, fixId[index], sizeof(fix->id));
fix->smem_start = crtc->o_screen + sm750_dev->vidmem_start;
- pr_info("fix->smem_start = %lx\n", fix->smem_start);
/*
* according to mmap experiment from user space application,
* fix->mmio_len should not larger than virtual size
@@ -874,13 +820,10 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
* data into the bound over virtual size
*/
fix->smem_len = crtc->vidmem_size;
- pr_info("fix->smem_len = %x\n", fix->smem_len);
info->screen_size = fix->smem_len;
fix->line_length = line_length;
fix->mmio_start = sm750_dev->vidreg_start;
- pr_info("fix->mmio_start = %lx\n", fix->mmio_start);
fix->mmio_len = sm750_dev->vidreg_size;
- pr_info("fix->mmio_len = %x\n", fix->mmio_len);
lynxfb_set_visual_mode(info);
@@ -889,22 +832,12 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
var->accel_flags = 0;
var->vmode = FB_VMODE_NONINTERLACED;
- pr_debug("#1 show info->cmap :\nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
- info->cmap.start, info->cmap.len,
- info->cmap.red, info->cmap.green, info->cmap.blue,
- info->cmap.transp);
-
ret = fb_alloc_cmap(&info->cmap, 256, 0);
if (ret < 0) {
pr_err("Could not allocate memory for cmap.\n");
goto exit;
}
- pr_debug("#2 show info->cmap :\nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
- info->cmap.start, info->cmap.len,
- info->cmap.red, info->cmap.green, info->cmap.blue,
- info->cmap.transp);
-
exit:
lynxfb_ops_check_var(var, info);
return ret;
@@ -1131,12 +1064,8 @@ static int __init lynxfb_setup(char *options)
int len;
char *opt, *tmp;
- if (!options || !*options) {
- pr_warn("no options.\n");
+ if (!options || !*options)
return 0;
- }
-
- pr_info("options:%s\n", options);
len = strlen(options) + 1;
g_settings = kzalloc(len, GFP_KERNEL);
@@ -1173,7 +1102,6 @@ static int __init lynxfb_setup(char *options)
}
/* misc g_settings are transport to chip specific routines */
- pr_info("parameter left for chip specific analysis:%s\n", g_settings);
return 0;
}
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index ce46f240c..983b51164 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -34,8 +34,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
sm750_dev->vidreg_start = pci_resource_start(pdev, 1);
sm750_dev->vidreg_size = SZ_2M;
- pr_info("mmio phyAddr = %lx\n", sm750_dev->vidreg_start);
-
/*
* reserve the vidreg space of smi adaptor
* if you do this, you need to add release region code
@@ -56,7 +54,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
ret = -EFAULT;
goto exit;
}
- pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
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;
@@ -72,8 +69,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
* @ddk750_get_vm_size function can be safe.
*/
sm750_dev->vidmem_size = ddk750_get_vm_size();
- pr_info("video memory phyAddr = %lx, size = %u bytes\n",
- sm750_dev->vidmem_start, sm750_dev->vidmem_size);
/* reserve the vidmem space of smi adaptor */
sm750_dev->pvMem =
@@ -84,7 +79,6 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
ret = -EFAULT;
goto exit;
}
- pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
exit:
return ret;
}
@@ -163,11 +157,9 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
* The following register values for CH7301 are from
* Chrontel app note and our experiment.
*/
- pr_info("yes,CH7301 DVI chip found\n");
sm750_sw_i2c_write_reg(0xec, 0x1d, 0x16);
sm750_sw_i2c_write_reg(0xec, 0x21, 0x9);
sm750_sw_i2c_write_reg(0xec, 0x49, 0xC0);
- pr_info("okay,CH7301 DVI chip setup done\n");
}
}
@@ -192,14 +184,12 @@ int hw_sm750_output_set_mode(struct lynxfb_output *output,
if (sm750_get_chip_type() != SM750LE) {
if (channel == sm750_primary) {
- pr_info("primary channel\n");
if (output->paths & sm750_panel)
disp_set |= do_LCD1_PRI;
if (output->paths & sm750_crt)
disp_set |= do_CRT_PRI;
} else {
- pr_info("secondary channel\n");
if (output->paths & sm750_panel)
disp_set |= do_LCD1_SEC;
if (output->paths & sm750_crt)
@@ -215,7 +205,6 @@ int hw_sm750_output_set_mode(struct lynxfb_output *output,
poke32(DISPLAY_CONTROL_750LE, reg);
}
- pr_info("ddk setlogicdispout done\n");
return ret;
}
@@ -232,10 +221,8 @@ int hw_sm750_crtc_check_mode(struct lynxfb_crtc *crtc,
case 16:
break;
case 32:
- if (sm750_dev->revid == SM750LE_REVISION_ID) {
- pr_debug("750le do not support 32bpp\n");
+ if (sm750_dev->revid == SM750LE_REVISION_ID)
return -EINVAL;
- }
break;
default:
return -EINVAL;
@@ -302,7 +289,6 @@ int hw_sm750_crtc_set_mode(struct lynxfb_crtc *crtc,
else
clock = SECONDARY_PLL;
- pr_debug("Request pixel clock = %lu\n", modparm.pixel_clock);
ret = ddk750_set_mode_timing(&modparm, clock);
if (ret) {
pr_err("Set mode timing failed\n");
@@ -431,12 +417,10 @@ int hw_sm750_set_blank(struct lynxfb_output *output, int blank)
switch (blank) {
case FB_BLANK_UNBLANK:
- pr_debug("flag = FB_BLANK_UNBLANK\n");
dpms = SYSTEM_CTRL_DPMS_VPHP;
pps = PANEL_DISPLAY_CTRL_DATA;
break;
case FB_BLANK_NORMAL:
- pr_debug("flag = FB_BLANK_NORMAL\n");
dpms = SYSTEM_CTRL_DPMS_VPHP;
crtdb = CRT_DISPLAY_CTRL_BLANK;
break;
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox