[PATCH] Introduce Tyr

[Miguel Ojeda]

Hi Daniel,

Some procedural notes and general comments, and please note that some
may apply several times.

On Sat, Jun 28, 2025 at 12:35 AM Daniel Almeida
<daniel.almeida at collabora.com> wrote:
>
> Signed-off-by: Rob Herring <robh at kernel.org>
>
> Signed-off-by: Daniel Almeida <daniel.almeida at collabora.com>

No newline.

> [2]: https://gitlab.freedesktop.org/panfrost/linux/-/tree/tyr?ref_type=heads

The base commit seems to be the one in this branch, but the branch is
a custom one that is not intended to land as-is, right?

If all the patches are in the list already (like the regulator ones),
then I would suggest putting the links to those instead. Otherwise, I
would mark the patch as RFC, since it is not meant to be applied
as-is.

Maybe I am just missing context, and this is all crystal clear for
everyone else, but normally patches are supposed to be candidates to
be applied, possibly with other dependencies, all coming from the
list.

> +use core::pin::Pin

This should already be able to come from the prelude.

> +/// Convienence type alias for the DRM device type for this driver

"Convenience"

Also, please end comments/docs with periods.

> +unsafe impl Send for TyrData {}
> +unsafe impl Sync for TyrData {}

Clippy should catch this (orthogonal to what Danilo mentioned).

> +use kernel::alloc::flags::*;

Prelude covers this.

> +// SAFETY:
> +//
> +// This type is the same type exposed by Panthor's uAPI. As it's declared as
> +// #repr(C), we can be sure that the layout is the same. Therefore, it is safe
> +// to expose this to userspace.

If they are not bullets, please don't add newlines, i.e. you can start
in the same line.

Also, `#[repr(C)]`.

Regarding the safety comment, it should explain how it covers the
requirements of `AsBytes`:

    Values of this type may not contain any uninitialized bytes. This
type must not have interior mutability.

> +#[allow(dead_code)]

Could it be `expect`?

> +/// Powers on the l2 block.
> +pub(crate) fn l2_power_on(iomem: &Devres<IoMem>) -> Result<()> {

-> Result

> +#![allow(dead_code)]

Could it be `expect`?

> +    author: "The Tyr driver authors",

Please use the `authors` key (this one is going away) -- with it now
you could mention each author.

> +#include<uapi/drm/panthor_drm.h>

Missing space.

Cheers,
Miguel