Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/panthor: missing and incorrect header dependencies in panthor_device_io.h --
commit 4a15e81aefbe1480e274e929d06e5e1ad5234842 Author: Karunika Choo <[email protected]> drm/panthor: Move register access helpers out of panthor_device.h Move register access helpers out of panthor_device.h to allow reuse by other panthor drivers that will be added to enable v15 support. > diff --git a/drivers/gpu/drm/panthor/panthor_device.h > b/drivers/gpu/drm/panthor/panthor_device.h > index b55a3f9edd414..54031c5252473 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -6,7 +6,6 @@ > #ifndef __PANTHOR_DEVICE_H__ > #define __PANTHOR_DEVICE_H__ > > -#include <linux/atomic.h> [Severity: Low] Is it okay to remove <linux/atomic.h> here? struct panthor_device (defined further down in this file) relies heavily on atomic_t, such as for retry_count and pending reset state. It seems to currently rely on transitive inclusion from the newly added panthor_device_io.h. > #include <linux/io-pgtable.h> > #include <linux/regulator/consumer.h> > #include <linux/pm_runtime.h> [ ... ] > diff --git a/drivers/gpu/drm/panthor/panthor_device_io.h > b/drivers/gpu/drm/panthor/panthor_device_io.h > new file mode 100644 > index 0000000000000..2287a0d9f86c2 > --- /dev/null > +++ b/drivers/gpu/drm/panthor/panthor_device_io.h > @@ -0,0 +1,83 @@ > +/* SPDX-License-Identifier: GPL-2.0 or MIT */ > +/* Copyright 2026 ARM Limited. All rights reserved. */ > + > +#ifndef __PANTHOR_DEVICE_IO_H__ > +#define __PANTHOR_DEVICE_IO_H__ > + > +#include <linux/atomic.h> [Severity: Low] Does this file actually need <linux/atomic.h>? It doesn't appear to use any atomic types. > +#include <linux/io.h> > + > + > +static inline void gpu_write(void __iomem *iomem, u32 reg, u32 data) > +{ > + writel(data, iomem + reg); > +} [ ... ] > +static inline void gpu_write64(void __iomem *iomem, u32 reg, u64 data) > +{ > + gpu_write(iomem, reg, lower_32_bits(data)); > + gpu_write(iomem, reg + 4, upper_32_bits(data)); > +} [Severity: Low] Should <linux/wordpart.h> be included for lower_32_bits() and upper_32_bits()? Files including this header directly might fail to build if the dependency isn't met transitively. > +static inline u64 gpu_read64(void __iomem *iomem, u32 reg) > +{ > + return (gpu_read(iomem, reg) | ((u64)gpu_read(iomem, reg + 4) << 32)); > +} [ ... ] > +static inline u64 gpu_read64_counter(void __iomem *iomem, u32 reg) > +{ > + u32 lo, hi1, hi2; > + do { > + hi1 = gpu_read(iomem, reg + 4); > + lo = gpu_read(iomem, reg); > + hi2 = gpu_read(iomem, reg + 4); > + } while (hi1 != hi2); > + return lo | ((u64)hi2 << 32); > +} > + > +#define gpu_read_poll_timeout(iomem, reg, val, cond, delay_us, timeout_us) > \ > + read_poll_timeout(gpu_read, val, cond, delay_us, timeout_us, false, > \ > + iomem, reg) [Severity: Low] Should <linux/iopoll.h> be included here for the read_poll_timeout() macros? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
