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

From: Ira Weiny
Date: Wed Oct 23 2024 - 22:33:54 EST


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@xxxxxxxxx>
> > ---
> One minor comment inline; otherwise
>
> Reviewed-by: Fan Ni <fan.ni@xxxxxxxxxxx>
>

[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]