On Tue, Mar 04 2025 at 22:23, Andre Przywara wrote:
>
> -struct sunxi_sc_nmi_reg_offs {
> +struct sunxi_sc_nmi_data {
> u32 ctrl;
> u32 pend;
> u32 enable;
> + u32 enable_val;
The data structure name and the corresponding variable/argument name
were making the code pretty obvious, but now this is opaque and
incomprehensible.
data::ctrl does not even give the slightest hint what this is about. You
need to read up in the code to figure out what it means. Something like:
struct sunxi_sc_nmi_data {
u32 reg_offs_ctrl;
u32 reg_offs_pend;
u32 reg_offs_enable;
u32 enable_val;
};
or even better:
struct sunxi_sc_nmi_data {
struct {
u32 ctrl;
u32 pend;
u32 enable;
} reg_offs;
u32 enable_val;
};
makes it clear and obvious, no?
> +static const struct sunxi_sc_nmi_data sun55i_a523_data __initconst = {
> + .ctrl = SUN9I_NMI_CTRL,
> + .pend = SUN9I_NMI_PENDING,
> + .enable = SUN9I_NMI_ENABLE,
> + .enable_val = BIT(31),
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
Thanks,
tglx
