On Sat, Apr 11, 2020 at 11:39:08PM +0300, Dmitry Osipenko wrote:
> > ...
> >> +#define TRIM_REG(chan, rank, reg, byte)					\
> >> +	(((EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ## reg ##	\
> >> +	   _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte ## _MASK &	\
> >> +	   next->trim_regs[EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ##		\
> >> +				 rank ## _ ## reg ## _INDEX]) >>	\
> >> +	  EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ## reg ##	\
> >> +	  _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte ## _SHIFT)	\
> >> +	 +								\
> >> +	 (((EMC_DATA_BRLSHFT_ ## rank ## _RANK ## rank ## _BYTE ##	\
> >> +	    byte ## _DATA_BRLSHFT_MASK &				\
> >> +	    next->trim_perch_regs[EMC ## chan ##			\
> >> +			      _EMC_DATA_BRLSHFT_ ## rank ## _INDEX]) >>	\
> >> +	   EMC_DATA_BRLSHFT_ ## rank ## _RANK ## rank ## _BYTE ##	\
> >> +	   byte ## _DATA_BRLSHFT_SHIFT) * 64))
> >> +
> >> +#define CALC_TEMP(rank, reg, byte1, byte2, n)				\
> >> +	(((new[n] << EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ##	\
> >> +	   reg ## _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte1 ## _SHIFT) & \
> >> +	  EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ## reg ##	\
> >> +	  _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte1 ## _MASK)	\
> >> +	 |								\
> >> +	 ((new[n + 1] << EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ##\
> >> +	   reg ## _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte2 ## _SHIFT) & \
> >> +	  EMC_PMACRO_OB_DDLL_LONG_DQ_RANK ## rank ## _ ## reg ##	\
> >> +	  _OB_DDLL_LONG_DQ_RANK ## rank ## _BYTE ## byte2 ## _MASK))
> 
> What about replacing those barely readable concatenated macros with a
> raw values?
> 
> Like:
> 
> TRIM_REG(brlshft_idx, ob_ddll_long_dq_rank_mask, ...)

That's just going to move the complexity from the macros to the
callsites, isn't it?

I suppose I could spend a few cycles trying to make this a little more
readable, but to be frank, the complexity in this driver is already so
high that this doesn't really make much of a difference, in my opinion.

Thierry