On Thu, Jul 16, 2020 at 02:25:11PM -0400, Jason Baron wrote:
> The Intel uncore driver may claim some of the pci ids from ie31200 which
> means that the ie31200 edac driver will not initialize them as part of
> pci_register_driver().
>
> Let's add a fallback for this case to 'pci_get_device()' to get a
> reference on the device such that it can still be configured. This is
> similar in approach to other edac drivers.
>
> Signed-off-by: Jason Baron <jba...@akamai.com>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> Cc: Tony Luck <tony.l...@intel.com>
> Cc: linux-edac <linux-e...@vger.kernel.org>
> ---
> drivers/edac/ie31200_edac.c | 50
> ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
> index d68346a..ebe5099 100644
> --- a/drivers/edac/ie31200_edac.c
> +++ b/drivers/edac/ie31200_edac.c
> @@ -170,6 +170,8 @@
> (n << (28 + (2 * skl) - PAGE_SHIFT))
>
> static int nr_channels;
> +static struct pci_dev *mci_pdev;
> +static int ie31200_registered = 1;
>
> struct ie31200_priv {
> void __iomem *window;
> @@ -538,12 +540,16 @@ static int ie31200_probe1(struct pci_dev *pdev, int
> dev_idx)
> static int ie31200_init_one(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> {
> - edac_dbg(0, "MC:\n");
> + int rc;
>
> + edac_dbg(0, "MC:\n");
> if (pci_enable_device(pdev) < 0)
> return -EIO;
> + rc = ie31200_probe1(pdev, ent->driver_data);
> + if (rc == 0 && !mci_pdev)
> + mci_pdev = pci_dev_get(pdev);
>
> - return ie31200_probe1(pdev, ent->driver_data);
> + return rc;
> }
>
> static void ie31200_remove_one(struct pci_dev *pdev)
> @@ -552,6 +558,8 @@ static void ie31200_remove_one(struct pci_dev *pdev)
> struct ie31200_priv *priv;
>
> edac_dbg(0, "\n");
> + pci_dev_put(mci_pdev);
> + mci_pdev = NULL;
> mci = edac_mc_del_mc(&pdev->dev);
> if (!mci)
> return;
> @@ -593,17 +601,53 @@ static struct pci_driver ie31200_driver = {
>
> static int __init ie31200_init(void)
> {
> + int pci_rc, i;
> +
> edac_dbg(3, "MC:\n");
> /* Ensure that the OPSTATE is set correctly for POLL or NMI */
> opstate_init();
>
> - return pci_register_driver(&ie31200_driver);
> + pci_rc = pci_register_driver(&ie31200_driver);
> + if (pci_rc < 0)
> + goto fail0;
> +
> + if (!mci_pdev) {
> + ie31200_registered = 0;
> + for (i = 0; ie31200_pci_tbl[i].vendor != 0; i++) {
> + mci_pdev = pci_get_device(ie31200_pci_tbl[i].vendor,
> + ie31200_pci_tbl[i].device,
> + NULL);
> + if (mci_pdev)
> + break;
> + }
> + if (!mci_pdev) {
> + edac_dbg(0, "ie31200 pci_get_device fail\n");
> + pci_rc = -ENODEV;
> + goto fail1;
> + }
> + pci_rc = ie31200_init_one(mci_pdev, &ie31200_pci_tbl[i]);
> + if (pci_rc < 0) {
> + edac_dbg(0, "ie31200 init fail\n");
> + pci_rc = -ENODEV;
> + goto fail1;
> + }
> + }
> + return 0;
> +
> +fail1:
> + pci_unregister_driver(&ie31200_driver);
> +fail0:
> + pci_dev_put(mci_pdev);
> +
> + return pci_rc;
> }
>
> static void __exit ie31200_exit(void)
> {
> edac_dbg(3, "MC:\n");
> pci_unregister_driver(&ie31200_driver);
> + if (!ie31200_registered)
> + ie31200_remove_one(mci_pdev);
> }
>
> module_init(ie31200_init);
We tested this inside in machines having this issue and it works.
Patch looks good to me.
Acked-by: Aristeu Rozanski <a...@redhat.com>
--
Aristeu
