lkml.org 
[lkml]   [2020]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 06/11] EDAC/ghes: Carve out MC device handling into separate functions
    Date
    The functions are too long, carve out code that handles MC devices
    into the new functions ghes_mc_create(), ghes_mc_add_or_free() and
    ghes_mc_free(). Apart from better code readability the functions can
    be reused and the implementation of the error paths becomes easier.

    Signed-off-by: Robert Richter <rrichter@marvell.com>
    ---
    drivers/edac/ghes_edac.c | 133 +++++++++++++++++++++++----------------
    1 file changed, 79 insertions(+), 54 deletions(-)

    diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
    index 358519e8c2e9..5a4c9694bbff 100644
    --- a/drivers/edac/ghes_edac.c
    +++ b/drivers/edac/ghes_edac.c
    @@ -462,16 +462,81 @@ static struct acpi_platform_list plat_list[] = {
    { } /* End */
    };

    -int ghes_edac_register(struct ghes *ghes, struct device *dev)
    +static struct mem_ctl_info *ghes_mc_create(struct device *dev, int mc_idx,
    + int num_dimm)
    {
    - bool fake = false;
    - int rc = 0, num_dimm = 0;
    + struct edac_mc_layer layers[1];
    struct mem_ctl_info *mci;
    struct ghes_mci *pvt;
    - struct edac_mc_layer layers[1];
    - struct ghes_dimm_fill dimm_fill;
    +
    + layers[0].type = EDAC_MC_LAYER_ALL_MEM;
    + layers[0].size = num_dimm;
    + layers[0].is_virt_csrow = true;
    +
    + mci = edac_mc_alloc(mc_idx, ARRAY_SIZE(layers), layers, sizeof(*pvt));
    + if (!mci)
    + return NULL;
    +
    + pvt = mci->pvt_info;
    + pvt->mci = mci;
    +
    + mci->pdev = dev;
    + mci->mtype_cap = MEM_FLAG_EMPTY;
    + mci->edac_ctl_cap = EDAC_FLAG_NONE;
    + mci->edac_cap = EDAC_FLAG_NONE;
    + mci->mod_name = "ghes_edac.c";
    + mci->ctl_name = "ghes_edac";
    + mci->dev_name = "ghes";
    +
    + return mci;
    +}
    +
    +static int ghes_mc_add_or_free(struct mem_ctl_info *mci)
    +{
    unsigned long flags;
    - int idx = -1;
    + int rc;
    +
    + rc = edac_mc_add_mc(mci);
    + if (rc < 0) {
    + edac_mc_free(mci);
    + return rc;
    + }
    +
    + spin_lock_irqsave(&ghes_lock, flags);
    + ghes_pvt = mci->pvt_info;
    + spin_unlock_irqrestore(&ghes_lock, flags);
    +
    + return 0;
    +}
    +
    +static void ghes_mc_free(void)
    +{
    + struct mem_ctl_info *mci;
    + unsigned long flags;
    +
    + /*
    + * Wait for the irq handler being finished.
    + */
    + spin_lock_irqsave(&ghes_lock, flags);
    + mci = ghes_pvt ? ghes_pvt->mci : NULL;
    + ghes_pvt = NULL;
    + spin_unlock_irqrestore(&ghes_lock, flags);
    +
    + if (!mci)
    + return;
    +
    + mci = edac_mc_del_mc(mci->pdev);
    + if (mci)
    + edac_mc_free(mci);
    +}
    +
    +int ghes_edac_register(struct ghes *ghes, struct device *dev)
    +{
    + struct ghes_dimm_fill dimm_fill;
    + int rc = 0, num_dimm = 0;
    + struct mem_ctl_info *mci;
    + bool fake = false;
    + int idx;

    if (IS_ENABLED(CONFIG_X86)) {
    /* Check if safe to enable on this system */
    @@ -500,28 +565,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
    num_dimm = 1;
    }

    - layers[0].type = EDAC_MC_LAYER_ALL_MEM;
    - layers[0].size = num_dimm;
    - layers[0].is_virt_csrow = true;
    -
    - mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt));
    + mci = ghes_mc_create(dev, 0, num_dimm);
    if (!mci) {
    - pr_info("Can't allocate memory for EDAC data\n");
    rc = -ENOMEM;
    goto unlock;
    }

    - pvt = mci->pvt_info;
    - pvt->mci = mci;
    -
    - mci->pdev = dev;
    - mci->mtype_cap = MEM_FLAG_EMPTY;
    - mci->edac_ctl_cap = EDAC_FLAG_NONE;
    - mci->edac_cap = EDAC_FLAG_NONE;
    - mci->mod_name = "ghes_edac.c";
    - mci->ctl_name = "ghes_edac";
    - mci->dev_name = "ghes";
    -
    if (fake) {
    pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
    pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
    @@ -549,22 +598,17 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
    dimm->edac_mode = EDAC_SECDED;
    }

    - rc = edac_mc_add_mc(mci);
    - if (rc < 0) {
    - pr_info("Can't register at EDAC core\n");
    - edac_mc_free(mci);
    - rc = -ENODEV;
    + rc = ghes_mc_add_or_free(mci);
    + if (rc)
    goto unlock;
    - }
    -
    - spin_lock_irqsave(&ghes_lock, flags);
    - ghes_pvt = pvt;
    - spin_unlock_irqrestore(&ghes_lock, flags);

    /* only set on success */
    refcount_set(&ghes_refcount, 1);

    unlock:
    + if (rc)
    + pr_err("Can't register at EDAC core: %d\n", rc);
    +
    mutex_unlock(&ghes_reg_mutex);

    return rc;
    @@ -572,29 +616,10 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)

    void ghes_edac_unregister(struct ghes *ghes)
    {
    - struct mem_ctl_info *mci;
    - unsigned long flags;
    -
    mutex_lock(&ghes_reg_mutex);

    - if (!refcount_dec_and_test(&ghes_refcount))
    - goto unlock;
    -
    - /*
    - * Wait for the irq handler being finished.
    - */
    - spin_lock_irqsave(&ghes_lock, flags);
    - mci = ghes_pvt ? ghes_pvt->mci : NULL;
    - ghes_pvt = NULL;
    - spin_unlock_irqrestore(&ghes_lock, flags);
    -
    - if (!mci)
    - goto unlock;
    -
    - mci = edac_mc_del_mc(mci->pdev);
    - if (mci)
    - edac_mc_free(mci);
    + if (refcount_dec_and_test(&ghes_refcount))
    + ghes_mc_free();

    -unlock:
    mutex_unlock(&ghes_reg_mutex);
    }
    --
    2.20.1
    \
     
     \ /
      Last update: 2020-03-06 16:15    [W:2.276 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site