public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Implement Send and Sync for clk
@ 2026-02-23 10:08 Alice Ryhl
  2026-02-23 10:08 ` [PATCH v5 1/3] rust: clk: implement Send and Sync Alice Ryhl
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Alice Ryhl @ 2026-02-23 10:08 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Maarten Lankhorst, Maxime Ripard, Drew Fustini,
	Guo Ren, Fu Wei, Uwe Kleine-König, Michal Wilczynski,
	Viresh Kumar, Miguel Ojeda, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, linux-clk, rust-for-linux, linux-kernel,
	dri-devel, linux-pwm, Alice Ryhl, Boqun Feng

The Clk type is thread-safe, so let's mark it as thread-safe in the type
system. This lets us get rid of hacks in drivers.

For Stephen's clk tree.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v5:
- Rebase on v7.0-rc1.
- Pick up Reviewed-by tags.
- Link to v4: https://lore.kernel.org/r/20260113-clk-send-sync-v4-0-712bc7d94a79@google.com

Changes in v4:
- Pick up Reviewed-by tags.
- Link to v3: https://lore.kernel.org/r/20251218-clk-send-sync-v3-0-e48b2e2f1eac@google.com

Changes in v3:
- Rebase on v6.19-rc1.
- Pick up tags.
- Add fix for pwm driver as well.
- Link to v2: https://lore.kernel.org/r/20251020-clk-send-sync-v2-0-44ab533ae084@google.com

Changes in v2:
- Rebase on v6.18-rc1.
- Add patch to tyr driver.
- Link to v1: https://lore.kernel.org/r/20250904-clk-send-sync-v1-1-48d023320eb8@google.com

---
Alice Ryhl (3):
      rust: clk: implement Send and Sync
      tyr: remove impl Send/Sync for TyrData
      pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData

 drivers/gpu/drm/tyr/driver.rs | 12 ------------
 drivers/pwm/pwm_th1520.rs     | 15 ---------------
 rust/kernel/clk.rs            |  7 +++++++
 3 files changed, 7 insertions(+), 27 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20250904-clk-send-sync-3cfa7f4e1ce2

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v5 1/3] rust: clk: implement Send and Sync
  2026-02-23 10:08 [PATCH v5 0/3] Implement Send and Sync for clk Alice Ryhl
@ 2026-02-23 10:08 ` Alice Ryhl
  2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
  2026-02-23 10:08 ` [PATCH v5 2/3] tyr: remove impl Send/Sync for TyrData Alice Ryhl
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-02-23 10:08 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Maarten Lankhorst, Maxime Ripard, Drew Fustini,
	Guo Ren, Fu Wei, Uwe Kleine-König, Michal Wilczynski,
	Viresh Kumar, Miguel Ojeda, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, linux-clk, rust-for-linux, linux-kernel,
	dri-devel, linux-pwm, Alice Ryhl, Boqun Feng

These traits are required for drivers to embed the Clk type in their own
data structures because driver data structures are usually required to
be Send. Since the Clk type is thread-safe, implement the relevant
traits.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/clk.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 4059aff34d096264bc7f3c8bfcb39eb3f42725be..7abbd0767d8cff9f2561d1783a9a1d774a96d1fc 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -128,6 +128,13 @@ mod common_clk {
     #[repr(transparent)]
     pub struct Clk(*mut bindings::clk);
 
+    // SAFETY: It is safe to call `clk_put` on another thread than where `clk_get` was called.
+    unsafe impl Send for Clk {}
+
+    // SAFETY: It is safe to call any combination of the `&self` methods in parallel, as the
+    // methods are synchronized internally.
+    unsafe impl Sync for Clk {}
+
     impl Clk {
         /// Gets [`Clk`] corresponding to a [`Device`] and a connection id.
         ///

-- 
2.53.0.345.g96ddfc5eaa-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 2/3] tyr: remove impl Send/Sync for TyrData
  2026-02-23 10:08 [PATCH v5 0/3] Implement Send and Sync for clk Alice Ryhl
  2026-02-23 10:08 ` [PATCH v5 1/3] rust: clk: implement Send and Sync Alice Ryhl
@ 2026-02-23 10:08 ` Alice Ryhl
  2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
  2026-02-23 10:08 ` [PATCH v5 3/3] pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData Alice Ryhl
  2026-02-24  0:11 ` Claude review: Implement Send and Sync for clk Claude Code Review Bot
  3 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-02-23 10:08 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Maarten Lankhorst, Maxime Ripard, Drew Fustini,
	Guo Ren, Fu Wei, Uwe Kleine-König, Michal Wilczynski,
	Viresh Kumar, Miguel Ojeda, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, linux-clk, rust-for-linux, linux-kernel,
	dri-devel, linux-pwm, Alice Ryhl, Boqun Feng

Now that clk implements Send and Sync, we no longer need to manually
implement these traits for TyrData. Thus remove the implementations.

The comment also mentions the regulator. However, the regulator had the
traits added in commit 9a200cbdb543 ("rust: regulator: implement Send
and Sync for Regulator<T>"), which is already in mainline.

Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/gpu/drm/tyr/driver.rs | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index beeffe36b6cb068f2a87129872b37cbea4fb2010..e833e9f537b0225368ca10f5bc71b2612b0befa5 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -53,18 +53,6 @@ pub(crate) struct TyrData {
     pub(crate) gpu_info: GpuInfo,
 }
 
-// Both `Clk` and `Regulator` do not implement `Send` or `Sync`, but they
-// should. There are patches on the mailing list to address this, but they have
-// not landed yet.
-//
-// For now, add this workaround so that this patch compiles with the promise
-// that it will be removed in a future patch.
-//
-// SAFETY: This will be removed in a future patch.
-unsafe impl Send for TyrData {}
-// SAFETY: This will be removed in a future patch.
-unsafe impl Sync for TyrData {}
-
 fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
     regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
 

-- 
2.53.0.345.g96ddfc5eaa-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 3/3] pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData
  2026-02-23 10:08 [PATCH v5 0/3] Implement Send and Sync for clk Alice Ryhl
  2026-02-23 10:08 ` [PATCH v5 1/3] rust: clk: implement Send and Sync Alice Ryhl
  2026-02-23 10:08 ` [PATCH v5 2/3] tyr: remove impl Send/Sync for TyrData Alice Ryhl
@ 2026-02-23 10:08 ` Alice Ryhl
  2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
  2026-02-24  0:11 ` Claude review: Implement Send and Sync for clk Claude Code Review Bot
  3 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-02-23 10:08 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Maarten Lankhorst, Maxime Ripard, Drew Fustini,
	Guo Ren, Fu Wei, Uwe Kleine-König, Michal Wilczynski,
	Viresh Kumar, Miguel Ojeda, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, linux-clk, rust-for-linux, linux-kernel,
	dri-devel, linux-pwm, Alice Ryhl, Boqun Feng

Now that clk implements Send and Sync, we no longer need to manually
implement these traits for Th1520PwmDriverData. Thus remove the
implementations.

Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
Reviewed-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/pwm/pwm_th1520.rs | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index b0e24ee724e4501b812e5f9c7904b483bfe5e2dc..6cd6fa3d298484cbd84b88cfdd3244217855fa05 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -99,21 +99,6 @@ struct Th1520PwmDriverData {
     clk: Clk,
 }
 
-// This `unsafe` implementation is a temporary necessity because the underlying `kernel::clk::Clk`
-// type does not yet expose `Send` and `Sync` implementations. This block should be removed
-// as soon as the clock abstraction provides these guarantees directly.
-// TODO: Remove those unsafe impl's when Clk will support them itself.
-
-// SAFETY: The `devres` framework requires the driver's private data to be `Send` and `Sync`.
-// We can guarantee this because the PWM core synchronizes all callbacks, preventing concurrent
-// access to the contained `iomem` and `clk` resources.
-unsafe impl Send for Th1520PwmDriverData {}
-
-// SAFETY: The same reasoning applies as for `Send`. The PWM core's synchronization
-// guarantees that it is safe for multiple threads to have shared access (`&self`)
-// to the driver data during callbacks.
-unsafe impl Sync for Th1520PwmDriverData {}
-
 impl pwm::PwmOps for Th1520PwmDriverData {
     type WfHw = Th1520WfHw;
 

-- 
2.53.0.345.g96ddfc5eaa-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Claude review: Implement Send and Sync for clk
  2026-02-23 10:08 [PATCH v5 0/3] Implement Send and Sync for clk Alice Ryhl
                   ` (2 preceding siblings ...)
  2026-02-23 10:08 ` [PATCH v5 3/3] pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData Alice Ryhl
@ 2026-02-24  0:11 ` Claude Code Review Bot
  3 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-02-24  0:11 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Implement Send and Sync for clk
Author: Alice Ryhl <aliceryhl@google.com>
Patches: 4
Reviewed: 2026-02-24T10:11:35.760888

---

This is a small, well-structured 3-patch series from Alice Ryhl that adds `Send` and `Sync` trait implementations to the Rust `Clk` type in the kernel clock subsystem, then removes the corresponding workaround `unsafe impl` blocks in two drivers that had been manually marking their data structures as `Send`/`Sync` to compensate for the missing impls on `Clk`.

The series is correctly ordered: patch 1 adds the trait implementations to the core type, and patches 2-3 remove the now-redundant workarounds in the tyr DRM driver and the th1520 PWM driver. Each patch compiles correctly on its own -- patches 2 and 3 depend on patch 1, but never introduce a window where correctness is broken. The series has multiple Reviewed-by and Acked-by tags from relevant maintainers and Rust-for-Linux developers.

No significant issues were found. The safety justifications are reasonable: the C clock framework uses internal locking for its operations (prepare/enable/disable/unprepare/get_rate/set_rate are all internally synchronized), and `clk_put` does not need to be called from the same thread as `clk_get`. The removal of the driver-side workarounds is clean -- the auto-trait derivation works correctly because all constituent fields of both `TyrData` and `Th1520PwmDriverData` are now `Send` + `Sync`.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: rust: clk: implement Send and Sync
  2026-02-23 10:08 ` [PATCH v5 1/3] rust: clk: implement Send and Sync Alice Ryhl
@ 2026-02-24  0:11   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-02-24  0:11 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

> +    // SAFETY: It is safe to call `clk_put` on another thread than where `clk_get` was called.
> +    unsafe impl Send for Clk {}
> +
> +    // SAFETY: It is safe to call any combination of the `&self` methods in parallel, as the
> +    // methods are synchronized internally.
> +    unsafe impl Sync for Clk {}

The safety comments are accurate. The `Clk` type wraps a `*mut bindings::clk`, which prevents auto-derivation of `Send`/`Sync`. The C clock framework's `clk_prepare`/`clk_enable`/`clk_disable`/`clk_unprepare`/`clk_get_rate`/`clk_set_rate` operations all use internal locking (prepare_lock mutex and enable_lock spinlock), making concurrent `&self` access safe.

One minor observation: `OptionalClk` is defined as `pub struct OptionalClk(Clk)` and has no explicit `Send`/`Sync` impls. It will correctly auto-derive both traits from `Clk` after this patch. This is fine and requires no changes.

No issues found.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: tyr: remove impl Send/Sync for TyrData
  2026-02-23 10:08 ` [PATCH v5 2/3] tyr: remove impl Send/Sync for TyrData Alice Ryhl
@ 2026-02-24  0:11   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-02-24  0:11 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

> -// Both `Clk` and `Regulator` do not implement `Send` or `Sync`, but they
> -// should. There are patches on the mailing list to address this, but they have
> -// not landed yet.
> -//
> -// For now, add this workaround so that this patch compiles with the promise
> -// that it will be removed in a future patch.
> -//
> -// SAFETY: This will be removed in a future patch.
> -unsafe impl Send for TyrData {}
> -// SAFETY: This will be removed in a future patch.
> -unsafe impl Sync for TyrData {}

Removal is correct. After patch 1, `TyrData`'s fields all auto-derive `Send`/`Sync`: `ARef<platform::Device>` is `Send`+`Sync`, `Mutex<Clocks>` is `Send`+`Sync` (where `Clocks` contains `Clk` and `OptionalClk`, now both `Send`+`Sync`), `Mutex<Regulators>` is `Send`+`Sync` (where `Regulator<T>` already has `Send`+`Sync` per commit 9a200cbdb543), and `GpuInfo` wraps a plain data struct. The commit message correctly explains that the `Regulator` case was already resolved in mainline.

No issues found.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData
  2026-02-23 10:08 ` [PATCH v5 3/3] pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData Alice Ryhl
@ 2026-02-24  0:11   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-02-24  0:11 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

> -// SAFETY: The `devres` framework requires the driver's private data to be `Send` and `Sync`.
> -// We can guarantee this because the PWM core synchronizes all callbacks, preventing concurrent
> -// access to the contained `iomem` and `clk` resources.
> -unsafe impl Send for Th1520PwmDriverData {}
> -
> -// SAFETY: The same reasoning applies as for `Send`. The PWM core's synchronization
> -// guarantees that it is safe for multiple threads to have shared access (`&self`)
> -// to the driver data during callbacks.
> -unsafe impl Sync for Th1520PwmDriverData {}

Removal is correct. `Th1520PwmDriverData` contains `Devres<IoMem<...>>` (which is `Send`+`Sync` when `IoMem` is `Send`, which it should be) and `Clk` (now `Send`+`Sync` after patch 1). Auto-derivation will work.

Note: the quoted-printable encoding in the mbox (`=20`, `=3D`, line continuations with `=`) is a transport artifact and does not affect the actual patch content.

No issues found.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-02-24  0:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 10:08 [PATCH v5 0/3] Implement Send and Sync for clk Alice Ryhl
2026-02-23 10:08 ` [PATCH v5 1/3] rust: clk: implement Send and Sync Alice Ryhl
2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
2026-02-23 10:08 ` [PATCH v5 2/3] tyr: remove impl Send/Sync for TyrData Alice Ryhl
2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
2026-02-23 10:08 ` [PATCH v5 3/3] pwm: th1520: remove impl Send/Sync for Th1520PwmDriverData Alice Ryhl
2026-02-24  0:11   ` Claude review: " Claude Code Review Bot
2026-02-24  0:11 ` Claude review: Implement Send and Sync for clk Claude Code Review Bot

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