Re: [PATCH 1/2] clk: renesas: tidyup DEF_GEN4_Z()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] clk: renesas: DEF_GEN4_Z macro arguments lack parentheses
--

commit ad51061672812b3bccf2c0614e480e134d8fc4e5
Author: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

clk: renesas: tidyup DEF_GEN4_Z()

This commit updates the DEF_GEN4_Z macro to take the register offset
and bit position explicitly rather than deriving them from a single
flat offset. It simplifies the offset calculation logic inside
cpg_z_clk_register().

> diff --git a/drivers/clk/renesas/rcar-gen4-cpg.h b/drivers/clk/renesas/rcar-gen4-cpg.h
> index 6c8280b37c37b..cfc150750dc4d 100644
> --- a/drivers/clk/renesas/rcar-gen4-cpg.h
> +++ b/drivers/clk/renesas/rcar-gen4-cpg.h
> @@ -58,8 +58,14 @@ enum rcar_gen4_clk_types {
>  #define DEF_GEN4_PLL_V9_24(_name, _idx, _id, _parent)	\
>  	DEF_BASE(_name, _id, CLK_TYPE_GEN4_PLL_V9_24, _parent, .offset = _idx)
>  
> -#define DEF_GEN4_Z(_name, _id, _type, _parent, _div, _offset)	\
> -	DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)
> +/*
> + * offset 0xAAABB
> + * AAA : reg
> + * BB  : pos
> + */
> +#define FRQCR_offset	8
> +#define DEF_GEN4_Z(_name, _id, _type, _parent, _div, reg, pos)	\
> +	DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = (reg << FRQCR_offset) + pos)

[Severity: Low]
This isn't a bug introduced by this patch since current callers pass simple
constants, but could the lack of parentheses around the reg and pos arguments
cause issues in the future?

If an expression is ever passed for these arguments, operator precedence
rules might cause the offset to be miscalculated (e.g., if reg is passed
as BASE + 4, it would expand to BASE + 4 << 8 + pos). Could parentheses
be added around (reg) and (pos) to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/87cxufs0yi.wl-kuninori.morimoto.gx@renesas.com?part=1