On 18/12/2023 12:32, Dikshita Agarwal wrote: > Registers are defined differently for different VPUs. > Define ops for VPU specific handling to accommodate > different VPUs. Implement boot sequence of firmware and interrupt > programming. > ... > + > +int write_register(struct iris_core *core, u32 reg, u32 value) > +{ > + void __iomem *base_addr; > + int ret; > + > + ret = check_core_lock(core); > + if (ret) > + return ret; > + > + base_addr = core->reg_base; > + base_addr += reg; > + writel_relaxed(value, base_addr); > + > + /* Make sure value is written into the register */ > + wmb(); Just don't use relaxed method. The same applies to other places like that. > + > + return ret; > +} > + > +int read_register(struct iris_core *core, u32 reg, u32 *value) > +{ > + void __iomem *base_addr; > + > + base_addr = core->reg_base; > + > + *value = readl_relaxed(base_addr + reg); > + > + /* Make sure value is read correctly from the register */ > + rmb(); > + > + return 0; > +} > + > +static const struct compat_handle compat_handle[] = { > + { > + .compat = "qcom,sm8550-iris", > + .init = init_iris3, Uh... > + }, > +}; > + > +int init_vpu(struct iris_core *core) > +{ > + struct device *dev = NULL; > + int i, ret = 0; > + > + dev = core->dev; > + > + for (i = 0; i < ARRAY_SIZE(compat_handle); i++) { > + if (of_device_is_compatible(dev->of_node, compat_handle[i].compat)) { > + ret = compat_handle[i].init(core); This does not look good. Use flags, quirks, type, pointer ops in structures. Just look at existing code in Linux kernel. Do not reimplement driver match data. Best regards, Krzysztof