Re: [PATCH REF v3 27/27] gpu: drm: tyr: use lifetime for IoMem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Meaningful mismatch between the commit message and the code regarding the I/O memory type.
- [Low] The `dev` parameter is now unused in `issue_soft_reset` and `l2_power_on`.
--

commit 763cc0c53067cdbb12995b72ad25897a1324b9d9
Author: Danilo Krummrich <dakr@kernel.org>

gpu: drm: tyr: use lifetime for IoMem
    
This commit simplifies the memory mapping during probe by using the mapped
memory directly and eliminating the Arc<Devres<IoMem>> indirection, which
also makes Register::read/write infallible.

> Take advantage of the lifetime-parameterized IoMem<'bound> to use the
> memory mapping directly during probe, eliminating the Arc<Devres<IoMem>>
> indirection.

The commit message mentions using the lifetime-parameterized IoMem<'bound>,
but the code changes the type alias to kernel::io::Mmio<SZ_2M> which lacks
a lifetime parameter. Is this difference intentional?

> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 8c916913e9765..884c20ec18cef 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
[ ... ]
> @@ -65,11 +63,11 @@ pub(crate) struct TyrDrmDeviceData {
>      pub(crate) gpu_info: GpuInfo,
>  }
>  
> -fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
> -    regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
> +fn issue_soft_reset(dev: &Device<Bound>, iomem: &IoMem) -> Result {
> +    regs::GPU_CMD.write(iomem, regs::GPU_CMD_SOFT_RESET);

Does the dev parameter still need to be in the function signature here? It
appears to be unused now that it is no longer passed to regs::GPU_CMD.write().

[ ... ]
> diff --git a/drivers/gpu/drm/tyr/gpu.rs b/drivers/gpu/drm/tyr/gpu.rs
> index a88775160f981..bb0473c85bf78 100644
> --- a/drivers/gpu/drm/tyr/gpu.rs
> +++ b/drivers/gpu/drm/tyr/gpu.rs
[ ... ]
> @@ -208,11 +206,11 @@ fn from(value: u32) -> Self {
>  }
>  
>  /// Powers on the l2 block.
> -pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
> -    regs::L2_PWRON_LO.write(dev, iomem, 1)?;
> +pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &IoMem) -> Result {
> +    regs::L2_PWRON_LO.write(iomem, 1);

Are there any remaining uses for the dev parameter in this function, or
could it be removed from the signature?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517000149.3226762-1-dakr@kernel.org?part=27