On Fri, Jul 17, 2026 at 10:56:37AM +0000, [email protected] wrote:
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +
> > +/* Registers */
> > +#define CLA_REG_DATA(i) (0x00 + (8 * (i)))
> [ ... ]
> > +#define CLA_PLxCTRL_AVAIL BIT(0)
> > +#define CLA_PLxCTRL_DBGPERM GENMASK(3, 1)
> > +#define CLA_PLxCTRL_PREP(accid, v) ((u64)(v) << (8 * (accid)))
>
> [Severity: Low]
> Should linux/types.h be included in drivers/misc/arm-cla/arm-cla-regs.h?
>
> The CLA_PLxCTRL_PREP macro casts the value to u64, but the header doesn't
> include linux/types.h. This could lead to build failures if the header is
> included directly in new source files without prior type inclusions.
linux/types.h gets included, but 4 levels deep starting from
bitfields.h. I'll move the include from arm-cla.h here.
> > +/* Common register offsets */
> > +#define CLA_MTC_PSTATE 0
> > +#define CLA_MTC_TTBR0 1
> > +#define CLA_MTC_TTBR1 2
> > +#define CLA_MTC_TCR 3
> > +#define CLA_MTC_SCTLR 4
> > +#define CLA_MTC_MAIR 5
> > +#define CLA_MTC_TCR2 8
> > +
> > +/* EL2 specific register offsets */
> > +#define CLA_MTC_HCR_EL2 80
> > +#define CLA_MTC_VTTBR_EL2 81
> > +#define CLA_MTC_VTCR_EL2 82
>
> [Severity: Medium]
> Are these EL2 specific register offsets intended to be relative offsets like
> the
> common ones (0-8)?
>
> They are currently defined as absolute indices (80, 81, 82). If the driver
> applies these by adding the base index (e.g., CLA_MTC_REGIDX_PL2, which is 64)
> to the offset, using CLA_MTC_HCR_EL2 (80) would yield an index of 144. This
> would exceed the defined CLA_MTC_PL_SIZE (64) and target the wrong hardware
> address space.
The generic offsets are for both EL1 and EL2. These are differentiated
by their _EL2 suffix. Targetting the EL3 frame or beyond causes
cla_op_setctx() to fail loudly.
> > diff --git a/drivers/misc/arm-cla/arm-cla.h b/drivers/misc/arm-cla/arm-cla.h
> > +struct cla_dev {
> > + unsigned int cpu;
> > + struct device *dev;
> > +};
> > +
> > +#define cla_dbg(dev, fmt, ...) \
> > + dev_dbg((dev)->dev, "[%u] " fmt, (dev)->cpu, ##__VA_ARGS__)
>
> [Severity: Low]
> Does this header need to include linux/device.h and linux/dev_printk.h?
>
> The cla_dbg macro uses dev_dbg() and dereferences struct device. Relying on
> implicit inclusions can cause build issues if arm-cla.h is included in
> a different order or in new files.
We include device.h in a later patch. It includes dev_prink.h
