Re: [PATCH v2 12/20] cxl/port: Split endpoint and switch port probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2023-02-10 at 01:06 -0800, Dan Williams wrote:
> Jonathan points out that the shared code between the switch and endpoint
> case is small. Before adding another is_cxl_endpoint() conditional,
> just split the two cases.
> 
> Rather than duplicate the "Couldn't enumerate decoders" error message
> take the opportunity to improve the error messages in
> devm_cxl_enumerate_decoders().
> 
> Reported-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Link: http://lore.kernel.org/r/20230208170724.000067ec@xxxxxxxxxx
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
>  drivers/cxl/core/hdm.c |   11 ++++++--
>  drivers/cxl/port.c     |   69 +++++++++++++++++++++++++++---------------------
>  2 files changed, 47 insertions(+), 33 deletions(-)

Looks good,

Reviewed-by: Vishal Verma <vishal.l.verma@xxxxxxxxx>

> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index dcc16d7cb8f3..a0891c3464f1 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -826,7 +826,8 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm)
>                         cxled = cxl_endpoint_decoder_alloc(port);
>                         if (IS_ERR(cxled)) {
>                                 dev_warn(&port->dev,
> -                                        "Failed to allocate the decoder\n");
> +                                        "Failed to allocate decoder%d.%d\n",
> +                                        port->id, i);
>                                 return PTR_ERR(cxled);
>                         }
>                         cxld = &cxled->cxld;
> @@ -836,7 +837,8 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm)
>                         cxlsd = cxl_switch_decoder_alloc(port, target_count);
>                         if (IS_ERR(cxlsd)) {
>                                 dev_warn(&port->dev,
> -                                        "Failed to allocate the decoder\n");
> +                                        "Failed to allocate decoder%d.%d\n",
> +                                        port->id, i);
>                                 return PTR_ERR(cxlsd);
>                         }
>                         cxld = &cxlsd->cxld;
> @@ -844,13 +846,16 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm)
>  
>                 rc = init_hdm_decoder(port, cxld, target_map, hdm, i, &dpa_base);
>                 if (rc) {
> +                       dev_warn(&port->dev,
> +                                "Failed to initialize decoder%d.%d\n",
> +                                port->id, i);
>                         put_device(&cxld->dev);
>                         return rc;
>                 }
>                 rc = add_hdm_decoder(port, cxld, target_map);
>                 if (rc) {
>                         dev_warn(&port->dev,
> -                                "Failed to add decoder to port\n");
> +                                "Failed to add decoder%d.%d\n", port->id, i);
>                         return rc;
>                 }
>         }
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 5453771bf330..a8d46a67b45e 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -30,55 +30,64 @@ static void schedule_detach(void *cxlmd)
>         schedule_cxl_memdev_detach(cxlmd);
>  }
>  
> -static int cxl_port_probe(struct device *dev)
> +static int cxl_switch_port_probe(struct cxl_port *port)
>  {
> -       struct cxl_port *port = to_cxl_port(dev);
>         struct cxl_hdm *cxlhdm;
>         int rc;
>  
> +       rc = devm_cxl_port_enumerate_dports(port);
> +       if (rc < 0)
> +               return rc;
>  
> -       if (!is_cxl_endpoint(port)) {
> -               rc = devm_cxl_port_enumerate_dports(port);
> -               if (rc < 0)
> -                       return rc;
> -               if (rc == 1)
> -                       return devm_cxl_add_passthrough_decoder(port);
> -       }
> +       if (rc == 1)
> +               return devm_cxl_add_passthrough_decoder(port);
>  
>         cxlhdm = devm_cxl_setup_hdm(port);
>         if (IS_ERR(cxlhdm))
>                 return PTR_ERR(cxlhdm);
>  
> -       if (is_cxl_endpoint(port)) {
> -               struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
> -               struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +       return devm_cxl_enumerate_decoders(cxlhdm);
> +}
>  
> -               /* Cache the data early to ensure is_visible() works */
> -               read_cdat_data(port);
> +static int cxl_endpoint_port_probe(struct cxl_port *port)
> +{
> +       struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
> +       struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +       struct cxl_hdm *cxlhdm;
> +       int rc;
> +
> +       cxlhdm = devm_cxl_setup_hdm(port);
> +       if (IS_ERR(cxlhdm))
> +               return PTR_ERR(cxlhdm);
>  
> -               get_device(&cxlmd->dev);
> -               rc = devm_add_action_or_reset(dev, schedule_detach, cxlmd);
> -               if (rc)
> -                       return rc;
> +       /* Cache the data early to ensure is_visible() works */
> +       read_cdat_data(port);
>  
> -               rc = cxl_hdm_decode_init(cxlds, cxlhdm);
> -               if (rc)
> -                       return rc;
> +       get_device(&cxlmd->dev);
> +       rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
> +       if (rc)
> +               return rc;
>  
> -               rc = cxl_await_media_ready(cxlds);
> -               if (rc) {
> -                       dev_err(dev, "Media not active (%d)\n", rc);
> -                       return rc;
> -               }
> -       }
> +       rc = cxl_hdm_decode_init(cxlds, cxlhdm);
> +       if (rc)
> +               return rc;
>  
> -       rc = devm_cxl_enumerate_decoders(cxlhdm);
> +       rc = cxl_await_media_ready(cxlds);
>         if (rc) {
> -               dev_err(dev, "Couldn't enumerate decoders (%d)\n", rc);
> +               dev_err(&port->dev, "Media not active (%d)\n", rc);
>                 return rc;
>         }
>  
> -       return 0;
> +       return devm_cxl_enumerate_decoders(cxlhdm);
> +}
> +
> +static int cxl_port_probe(struct device *dev)
> +{
> +       struct cxl_port *port = to_cxl_port(dev);
> +
> +       if (is_cxl_endpoint(port))
> +               return cxl_endpoint_port_probe(port);
> +       return cxl_switch_port_probe(port);
>  }
>  
>  static ssize_t CDAT_read(struct file *filp, struct kobject *kobj,
> 





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux