Re: [spacemit:for-next 4/6] drivers/clk/spacemit/ccu-k1.c:241:1: error: initializer element is not a compile-time constant

[Haylen Chu <heylenay@4d2.org>]

On Tue, Apr 15, 2025 at 11:03:12AM -0500, Alex Elder wrote:
> On 4/15/25 9:31 AM, Yixun Lan wrote:
> > Hi Alex,
> > 
> > On 09:16 Tue 15 Apr     , Alex Elder wrote:
> > > On 4/15/25 8:44 AM, kernel test robot wrote:
> > > > tree:   https://github.com/spacemit-com/linux for-next
> > > > head:   9ead3efbc88a21e9a1f38948cc0902518022a2e5
> > > > commit: e2d238ba301f25a5c8a7d490a36f9cd014d7aecc [4/6] clk: spacemit: Add clock support for SpacemiT K1 SoC
> > > > config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250415/202504152107.7ekGPIrF-lkp@intel.com/config)
> > > > compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250415/202504152107.7ekGPIrF-lkp@intel.com/reproduce)
> > > > 
> > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > > the same patch/commit), kindly add following tags
> > > > | Reported-by: kernel test robot <lkp@intel.com>
> > > > | Closes: https://lore.kernel.org/oe-kbuild-all/202504152107.7ekGPIrF-lkp@intel.com/
> > > > 
> > > > All errors (new ones prefixed by >>):
> > > 
> > > If I'm interpreting this right, this seems to be a problem on
> > > a S390 build (under COMPILE_TEST), because there is no constant
> > > version of the __ffs() for that architecture.  This might be the
> > > case on other architectures as well.
> > > 
> > > On RISC-V, this is fine.
> > > 
> > > Any recommendations on how to address this?
> > > 
> > it occur to me the __ffs() isn't really necessary, since all input
> > is constant value (GENMASK here), we then should know the shift..
> 
> I asked Haylen to make this change.  Yes, we know the
> shift, but I much prefer specifying only a constant mask
> (which implies both the shift and the width) rather than
> a constant mask and shift (or width and shift).
> 
> If there's no way to avoid this S390 compile error, then
> adding the separate (and redundant) shift value back
> into all these definitions is a solution.

We could avoid the redundancy by specifying only width and shift and
move the call to GENMASK() into the macro, see my previous reply. To me
it doesn't look very different, it's just about where to invoke
GENMASK().

> 					-Alex

Thanks,
Haylen Chu

> 
> > 
> > > 
> > > 
> > > > 
> > > > > > drivers/clk/spacemit/ccu-k1.c:241:1: error: initializer element is not a compile-time constant
> > > >        241 | CCU_DDN_DEFINE(slow_uart1_14p74, pll1_d16_153p6, MPMU_SUCCR, GENMASK(28, 16), GENMASK(12, 0), 0);
> > > >            | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >      drivers/clk/spacemit/ccu_ddn.h:33:15: note: expanded from macro 'CCU_DDN_DEFINE'
> > > >         33 |         .num_shift      = __ffs(_num_mask),                                     \
> > > >            |                           ^~~~~~~~~~~~~~~~
> > > >      drivers/clk/spacemit/ccu-k1.c:242:1: error: initializer element is not a compile-time constant
> > > >        242 | CCU_DDN_DEFINE(slow_uart2_48, pll1_d4_614p4, MPMU_SUCCR_1, GENMASK(28, 16), GENMASK(12, 0), 0);
> > > >            | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >      drivers/clk/spacemit/ccu_ddn.h:33:15: note: expanded from macro 'CCU_DDN_DEFINE'
> > > >         33 |         .num_shift      = __ffs(_num_mask),                                     \
> > > >            |                           ^~~~~~~~~~~~~~~~
> > > >      2 errors generated.
> > > > 
> > > > 
> > > > vim +241 drivers/clk/spacemit/ccu-k1.c
> > > > 
> > > >      239	
> > > >      240	CCU_GATE_DEFINE(slow_uart, CCU_PARENT_NAME(osc), MPMU_ACGR, BIT(1), CLK_IGNORE_UNUSED);
> > > >    > 241	CCU_DDN_DEFINE(slow_uart1_14p74, pll1_d16_153p6, MPMU_SUCCR, GENMASK(28, 16), GENMASK(12, 0), 0);
> > > >      242	CCU_DDN_DEFINE(slow_uart2_48, pll1_d4_614p4, MPMU_SUCCR_1, GENMASK(28, 16), GENMASK(12, 0), 0);
> > > >      243	
> > > > 
> > > 
> > 
>