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.we...@intel.com>
> ---
One minor comment inline; otherwise

Reviewed-by: Fan Ni <fan...@samsung.com>

>  drivers/cxl/pci.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index fc5ab74448cc..29a863331bec 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -702,35 +702,31 @@ static int cxl_event_config_msgnums(struct 
> cxl_memdev_state *mds,
>       return cxl_event_get_int_policy(mds, policy);
>  }
>  
> -static int cxl_event_irqsetup(struct cxl_memdev_state *mds)
> +static int cxl_event_irqsetup(struct cxl_memdev_state *mds,
> +                           struct cxl_event_interrupt_policy *policy)
>  {
>       struct cxl_dev_state *cxlds = &mds->cxlds;
> -     struct cxl_event_interrupt_policy policy;
>       int rc;
>  
> -     rc = cxl_event_config_msgnums(mds, &policy);
> -     if (rc)
> -             return rc;
> -
> -     rc = cxl_event_req_irq(cxlds, policy.info_settings);
> +     rc = cxl_event_req_irq(cxlds, policy->info_settings);
>       if (rc) {
>               dev_err(cxlds->dev, "Failed to get interrupt for event Info 
> log\n");
>               return rc;
>       }
>  
> -     rc = cxl_event_req_irq(cxlds, policy.warn_settings);
> +     rc = cxl_event_req_irq(cxlds, policy->warn_settings);
>       if (rc) {
>               dev_err(cxlds->dev, "Failed to get interrupt for event Warn 
> log\n");
>               return rc;
>       }
>  
> -     rc = cxl_event_req_irq(cxlds, policy.failure_settings);
> +     rc = cxl_event_req_irq(cxlds, policy->failure_settings);
>       if (rc) {
>               dev_err(cxlds->dev, "Failed to get interrupt for event Failure 
> log\n");
>               return rc;
>       }
>  
> -     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) {
        ...
    }

For DCD, handle the setup separately afterwards.

Fan

> @@ -749,7 +745,7 @@ static bool cxl_event_int_is_fw(u8 setting)
>  static int cxl_event_config(struct pci_host_bridge *host_bridge,
>                           struct cxl_memdev_state *mds, bool irq_avail)
>  {
> -     struct cxl_event_interrupt_policy policy;
> +     struct cxl_event_interrupt_policy policy = { 0 };
>       int rc;
>  
>       /*
> @@ -777,11 +773,15 @@ static int cxl_event_config(struct pci_host_bridge 
> *host_bridge,
>               return -EBUSY;
>       }
>  
> +     rc = cxl_event_config_msgnums(mds, &policy);
> +     if (rc)
> +             return rc;
> +
>       rc = cxl_mem_alloc_event_buf(mds);
>       if (rc)
>               return rc;
>  
> -     rc = cxl_event_irqsetup(mds);
> +     rc = cxl_event_irqsetup(mds, &policy);
>       if (rc)
>               return rc;
>  
> 
> -- 
> 2.46.0
> 

-- 
Fan Ni

Reply via email to