From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/tyr: Clarify driver/device type names
Date: Tue, 24 Feb 2026 09:38:58 +1000
Message-ID: <review-patch1-20260223204648.208886-1-deborah.brouwer@collabora.com>
In-Reply-To: <20260223204648.208886-1-deborah.brouwer@collabora.com>
References: <20260223204648.208886-1-deborah.brouwer@collabora.com> <20260223204648.208886-1-deborah.brouwer@collabora.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

The commit message is clear and accurately describes the changes. The "no functional changes intended" claim is correct.

**Type alias inconsistency across files:**

In driver.rs, the patch defines the type alias:

> +pub(crate) type TyrDrmDevice = drm::Device<TyrDrmDriver>;

But in file.rs, the `dev_query` parameter is changed to spell out the full type instead of using the alias:

> +        ddev: &drm::Device<TyrDrmDriver>,

And in gem.rs similarly:

> +    fn new(_dev: &kernel::drm::Device<TyrDrmDriver>, _size: usize) -> impl PinInit<Self, Error> {

The old code used the `TyrDevice` alias in both file.rs and gem.rs (via `use crate::driver::TyrDevice`). The new code drops those imports and writes out the full type. If the `TyrDrmDevice` alias is going to be defined, it would be more consistent to use it everywhere as the old code did with `TyrDevice`. Otherwise the alias only serves one site (the `_device` field in `TyrPlatformDeviceData`). This is not a correctness issue but it defeats the purpose of the convenience alias.

Also note that gem.rs spells it `kernel::drm::Device<TyrDrmDriver>` while file.rs spells it `drm::Device<TyrDrmDriver>` -- this is because file.rs has `use kernel::drm` at the top while gem.rs uses `use kernel::drm::gem` and doesn't import the `drm` module directly. If the `TyrDrmDevice` alias were used, this inconsistency would go away.

**Naming convention (already noted by Danilo):**

> +pub(crate) struct TyrPlatformDeviceData {

Danilo's point about `TyrPlatformDriverData` being a better name is well-taken. The C subsystem uses `platform_set_drvdata()`/`platform_get_drvdata()` -- it is *driver* private data stored in the platform device, not device data. This is the main item to address in a v3.

**Everything else looks correct.** The `Send`/`Sync` impls, `PinnedDrop` impls, `platform::Driver` impl, `drm::Driver` impl, `module_platform_driver!` macro, ioctl declaration, and `DriverFile` impl are all updated consistently to the new names. The probe function correctly constructs `TyrDrmDeviceData` and `TyrPlatformDeviceData` in the right order. No functional behavior is changed.

---
Generated by Claude Code Patch Reviewer