Re: [PATCH v4 17/28] cxl/events: Split event msgnum configuration from irq setup

[Ira Weiny <ira.weiny@intel.com>]

Fan Ni wrote:
> On Mon, Oct 07, 2024 at 06:16:23PM -0500, Ira Weiny wrote:
> > Dynamic Capacity Devices (DCD) require event interrupts to process
> > memory addition or removal.  BIOS may have control over non-DCD event
> > processing.  DCD interrupt configuration needs to be separate from
> > memory event interrupt configuration.
> > 
> > Split cxl_event_config_msgnums() from irq setup in preparation for
> > separate DCD interrupts configuration.
> > 
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> One minor comment inline; otherwise
> 
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> 

[snip]

> >  
> > -	rc = cxl_event_req_irq(cxlds, policy.fatal_settings);
> > +	rc = cxl_event_req_irq(cxlds, policy->fatal_settings);
> >  	if (rc) {
> >  		dev_err(cxlds->dev, "Failed to get interrupt for event Fatal log\n");
> >  		return rc;
> 
> There is a lot of duplicate code here, can we simplify it by
> iteratting all setttings in cxl_event_interrrupt_policy like 
> 
> for setting in policy:
>     rc = cxl_event_req_irq(cxlds, setting);
>     if (rc) {
>         ...
>     }
>

Do you mean by treating struct cxl_event_interrupt_policy as an u8 array?

I'm not sure that is super beneficial.

Ira

> 
> For DCD, handle the setup separately afterwards.
> 
> Fan

[snip]