[PATCH v4 06/11] cxl: Refactor adaptor init/teardown

Ian Munsie imunsie at au1.ibm.com
Fri Aug 14 16:12:13 AEST 2015


Excerpts from Daniel Axtens's message of 2015-08-13 14:11:24 +1000:
> +/* This should contain *only* operations that can safely be done in
> + * both creation and recovery.
> + */
> +static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
>  {
> -    struct cxl *adapter;
> -    bool free = true;
>      int rc;
>  
> +    adapter->dev.parent = &dev->dev;
> +    adapter->dev.release = cxl_release_adapter;
> +    pci_set_drvdata(dev, adapter);

These seem a bit odd here (though perfectly harmless) - not sure these
need to be done again on recovery (but maybe I'm wrong?) - seems more
like something that should be done early in cxl_init_adapter?

> -    if ((rc = cxl_update_image_control(adapter)))
> -        goto err2;
> +    rc = cxl_update_image_control(adapter);
> +    if (rc)

These types of coding style changes should really be in a separate patch
to make it easier to see exactly how you have changed the init path in
this one. I know mpe wanted these changed and after looking at the diff
pretty carefully I realise that you haven't actually changed much
functionally so I'll let this pass, but if you happen to do another
respin please move the style changes into a separate patch.

Acked-by: Ian Munsie <imunsie at au1.ibm.com>



More information about the Linuxppc-dev mailing list