s390/pci: refactor zpci_create_device()

Currently zpci_create_device() is only called in clp_add_pci_device()
which allocates the memory for the struct zpci_dev being created. There
is little separation of concerns as only both functions together can
create a zpci_dev and the only CLP specific code in
clp_add_pci_device() is a call to clp_query_pci_fn().

Improve this by removing clp_add_pci_device() and refactor
zpci_create_device() such that it alone creates and initializes the
zpci_dev given the FID and Function Handle. For this we need to make
clp_query_pci_fn() non-static. While at it remove the function handle
parameter since we can just take that from the zpci_dev. Also move
adding to the zpci_list to after the zdev has been fully created which
eliminates a window where a partially initialized zdev can be found by
get_zdev_by_fid().

Acked-by: Pierre Morel <pmorel@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 2126289..053fe8b 100644 (file)
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -201,7 +201,7 @@ extern unsigned int s390_pci_no_rid;
   Prototypes
 ----------------------------------------------------------------------------- */
 /* Base stuff */
-int zpci_create_device(struct zpci_dev *);
+int zpci_create_device(u32 fid, u32 fh, enum zpci_state state);
 void zpci_remove_device(struct zpci_dev *zdev);
 int zpci_enable_device(struct zpci_dev *);
 int zpci_disable_device(struct zpci_dev *);
@@ -212,7 +212,7 @@ void zpci_remove_reserved_devices(void);
 /* CLP */
 int clp_setup_writeback_mio(void);
 int clp_scan_pci_devices(void);
-int clp_add_pci_device(u32, u32, int);
+int clp_query_pci_fn(struct zpci_dev *zdev);
 int clp_enable_fh(struct zpci_dev *, u8);
 int clp_disable_fh(struct zpci_dev *);
 int clp_get_state(u32 fid, enum zpci_state *state);

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 41df8fc..600881d 100644 (file)
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -695,43 +695,68 @@ void zpci_remove_device(struct zpci_dev *zdev)
        }
 }
 
-int zpci_create_device(struct zpci_dev *zdev)
+/**
+ * zpci_create_device() - Create a new zpci_dev and add it to the zbus
+ * @fid: Function ID of the device to be created
+ * @fh: Current Function Handle of the device to be created
+ * @state: Initial state after creation either Standby or Configured
+ *
+ * Creates a new zpci device and adds it to its, possibly newly created, zbus
+ * as well as zpci_list.
+ *
+ * Returns: 0 on success, an error value otherwise
+ */
+int zpci_create_device(u32 fid, u32 fh, enum zpci_state state)
 {
+       struct zpci_dev *zdev;
        int rc;
 
-       kref_init(&zdev->kref);
+       zpci_dbg(3, "add fid:%x, fh:%x, c:%d\n", fid, fh, state);
+       zdev = kzalloc(sizeof(*zdev), GFP_KERNEL);
+       if (!zdev)
+               return -ENOMEM;
 
-       spin_lock(&zpci_list_lock);
-       list_add_tail(&zdev->entry, &zpci_list);
-       spin_unlock(&zpci_list_lock);
+       /* FID and Function Handle are the static/dynamic identifiers */
+       zdev->fid = fid;
+       zdev->fh = fh;
 
-       rc = zpci_init_iommu(zdev);
+       /* Query function properties and update zdev */
+       rc = clp_query_pci_fn(zdev);
        if (rc)
-               goto out;
+               goto error;
+       zdev->state =  state;
 
+       kref_init(&zdev->kref);
        mutex_init(&zdev->lock);
+
+       rc = zpci_init_iommu(zdev);
+       if (rc)
+               goto error;
+
        if (zdev->state == ZPCI_FN_STATE_CONFIGURED) {
                rc = zpci_enable_device(zdev);
                if (rc)
-                       goto out_destroy_iommu;
+                       goto error_destroy_iommu;
        }
 
        rc = zpci_bus_device_register(zdev, &pci_root_ops);
        if (rc)
-               goto out_disable;
+               goto error_disable;
+
+       spin_lock(&zpci_list_lock);
+       list_add_tail(&zdev->entry, &zpci_list);
+       spin_unlock(&zpci_list_lock);
 
        return 0;
 
-out_disable:
+error_disable:
        if (zdev->state == ZPCI_FN_STATE_ONLINE)
                zpci_disable_device(zdev);
-
-out_destroy_iommu:
+error_destroy_iommu:
        zpci_destroy_iommu(zdev);
-out:
-       spin_lock(&zpci_list_lock);
-       list_del(&zdev->entry);
-       spin_unlock(&zpci_list_lock);
+error:
+       zpci_dbg(0, "add fid:%x, rc:%d\n", fid, rc);
+       kfree(zdev);
        return rc;
 }
 

diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index 153720d..d333159 100644 (file)
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -181,7 +181,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev,
        return 0;
 }
 
-static int clp_query_pci_fn(struct zpci_dev *zdev, u32 fh)
+int clp_query_pci_fn(struct zpci_dev *zdev)
 {
        struct clp_req_rsp_query_pci *rrb;
        int rc;
@@ -194,7 +194,7 @@ static int clp_query_pci_fn(struct zpci_dev *zdev, u32 fh)
        rrb->request.hdr.len = sizeof(rrb->request);
        rrb->request.hdr.cmd = CLP_QUERY_PCI_FN;
        rrb->response.hdr.len = sizeof(rrb->response);
-       rrb->request.fh = fh;
+       rrb->request.fh = zdev->fh;
 
        rc = clp_req(rrb, CLP_LPS_PCI);
        if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) {
@@ -212,40 +212,6 @@ out:
        return rc;
 }
 
-int clp_add_pci_device(u32 fid, u32 fh, int configured)
-{
-       struct zpci_dev *zdev;
-       int rc = -ENOMEM;
-
-       zpci_dbg(3, "add fid:%x, fh:%x, c:%d\n", fid, fh, configured);
-       zdev = kzalloc(sizeof(*zdev), GFP_KERNEL);
-       if (!zdev)
-               goto error;
-
-       zdev->fh = fh;
-       zdev->fid = fid;
-
-       /* Query function properties and update zdev */
-       rc = clp_query_pci_fn(zdev, fh);
-       if (rc)
-               goto error;
-
-       if (configured)
-               zdev->state = ZPCI_FN_STATE_CONFIGURED;
-       else
-               zdev->state = ZPCI_FN_STATE_STANDBY;
-
-       rc = zpci_create_device(zdev);
-       if (rc)
-               goto error;
-       return 0;
-
-error:
-       zpci_dbg(0, "add fid:%x, rc:%d\n", fid, rc);
-       kfree(zdev);
-       return rc;
-}
-
 static int clp_refresh_fh(u32 fid);
 /*
  * Enable/Disable a given PCI function and update its function handle if
@@ -408,7 +374,7 @@ static void __clp_add(struct clp_fh_list_entry *entry, void *data)
 
        zdev = get_zdev_by_fid(entry->fid);
        if (!zdev)
-               clp_add_pci_device(entry->fid, entry->fh, entry->config_state);
+               zpci_create_device(entry->fid, entry->fh, entry->config_state);
 }
 
 int clp_scan_pci_devices(void)

diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 5c6407f..b4162da 100644 (file)
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -89,7 +89,7 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
        switch (ccdf->pec) {
        case 0x0301: /* Reserved|Standby -> Configured */
                if (!zdev) {
-                       ret = clp_add_pci_device(ccdf->fid, ccdf->fh, 1);
+                       zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
                        break;
                }
                /* the configuration request may be stale */
@@ -116,7 +116,7 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
                break;
        case 0x0302: /* Reserved -> Standby */
                if (!zdev) {
-                       clp_add_pci_device(ccdf->fid, ccdf->fh, 0);
+                       zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
                        break;
                }
                zdev->fh = ccdf->fh;