Re: [PATCH 3/4] edac: Add APM X-Gene SoC EDAC driver

Hi,

quoted

+static int xgene_edac_mc_is_active(struct xgene_edac_mc_ctx *ctx, int mc_idx)
+{
+     u32 reg;
+     u32 mcu_mask;
+
+     reg = readl(ctx->csw_csr + CSW_CSWCR);
+     if (reg & CSW_CSWCR_DUALMCB_MASK) {
+             /*
+              * Dual MCB active - Determine if all 4 activie or just MCU0

Spellcheck: "activie"

The proper spelling is "active" from online dictionary.

quoted

+     ctx = mci->pvt_info;
+     ctx->name = "xgene_edac_mc_err";
+     mci->pdev = &pdev->dev;
+     dev_set_drvdata(mci->pdev, mci);
+     mci->ctl_name = ctx->name;
+     mci->dev_name = ctx->name;
+
+     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+     ctx->pcp_csr = devm_ioremap(&pdev->dev, res->start,
+                                 resource_size(res));
+     if (IS_ERR(ctx->pcp_csr)) {
+             dev_err(&pdev->dev, "no PCP resource address\n");

edac_printk(KERN_ERR, "X-Gene", ... Ditto for all other dev_err calls.

Are you sure about this? With dev_err, it prints the device node info
as well. This allows one to determine which instance that has issue.
X-Gene has 4 instance of memory controllers.

quoted

+
+     res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
+     ctx->mcu_csr = devm_ioremap_resource(&pdev->dev, res);
+     if (IS_ERR(ctx->mcu_csr)) {
+             dev_err(&pdev->dev, "no MCU resource address\n");
+             rc = PTR_ERR(ctx->mcu_csr);
+             goto err_free;
+     }
+     /* Ignore non-active MCU */
+     if (!xgene_edac_mc_is_active(ctx, ((res->start >> 16) & 0xF) / 4)) {
+             rc = -ENODEV;
+             goto err_free;
+     }

You could move edac_mc_alloc() here, after the resources have been
collected successfully and simplify the unwinding path.

I need the ctx pointer which is part of the edac_mc_alloc. Can't move it here.

-Loc
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html