Re: [PATCH 4/4] crypto: qat - add pm_status debugfs file

[Andy Shevchenko]

On Fri, Aug 18, 2023 at 01:28:42PM +0800, Herbert Xu wrote:
> On Thu, Aug 17, 2023 at 04:33:17PM +0200, Lucas Segarra Fernandez wrote:
> >
> > +static struct pm_status_row pm_event_rows[] = {
> > +	PM_INFO_REGSET_ENTRY32(event_log[0], EVENT0),
> > +	PM_INFO_REGSET_ENTRY32(event_log[1], EVENT1),
> > +	PM_INFO_REGSET_ENTRY32(event_log[2], EVENT2),
> > +	PM_INFO_REGSET_ENTRY32(event_log[3], EVENT3),
> > +	PM_INFO_REGSET_ENTRY32(event_log[4], EVENT4),
> > +	PM_INFO_REGSET_ENTRY32(event_log[5], EVENT5),
> > +	PM_INFO_REGSET_ENTRY32(event_log[6], EVENT6),
> > +	PM_INFO_REGSET_ENTRY32(event_log[7], EVENT7),
> > +};
> > +
> > +static_assert(ARRAY_SIZE_OF_FIELD(struct icp_qat_fw_init_admin_pm_info, event_log) ==
> > +	      ARRAY_SIZE(pm_event_rows));
> 
> Was all of that churn just for this one line?
> 
> How about simply declaring a macro
> 
> 	#define QAT_NUMBER_OF_PM_EVENTS 8
> 
> and then use it for the two arrays:
> 
> 	static struct pm_status_row pm_event_rows[QAT_NUMBER_OF_PM_EVENTS] = {
> 
> 	__u32 event_log[QAT_NUMBER_OF_PM_EVENTS];
> 
> What am I missing?

Splitting ARRAY_SIZE() is very beneficial on its own.
The static assert is slightly more robust for the big code then defining
something that at some point can be missed or miscalculated. Yet we can
survive with a macro if you thinks it's better.

-- 
With Best Regards,
Andy Shevchenko