diff mbox series

[v2] mtd: rawnand: pasemi: Don't use static data to track per-device state

Message ID 20230102124051.1508424-1-u.kleine-koenig@pengutronix.de
State Accepted
Headers show
Series [v2] mtd: rawnand: pasemi: Don't use static data to track per-device state | expand

Commit Message

Uwe Kleine-König Jan. 2, 2023, 12:40 p.m. UTC 
Up to now the pasemi nand driver only supported a single device
instance. However the check for that was racy because two parallel calls
of pasemi_nand_probe() could pass the check

	if (pasemi_nand_mtd)
		return -ENODEV;

before any of them assigns a non-NULL value to it.

So rework the driver to make use of per-device driver data.

As an intended side effect the driver can bind more than one device and
also gets rid of the check

	if (!pasemi_nand_mtd)
		return 0;

in the remove callback that could only ever trigger after the above race
happened.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

Changes since (implicit) v1:

 - drop Fixes line (846fc31d06e5 ("[MTD] [NAND] pasemi_nand driver"))
 - Cc a mailing list (*sigh*)

Best regards
Uwe

 drivers/mtd/nand/raw/pasemi_nand.c | 63 +++++++++++++++++-------------
 1 file changed, 35 insertions(+), 28 deletions(-)

Comments

Miquel Raynal Jan. 7, 2023, 3:38 p.m. UTC | #1 
On Mon, 2023-01-02 at 12:40:51 UTC, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= wrote:
> Up to now the pasemi nand driver only supported a single device
> instance. However the check for that was racy because two parallel calls
> of pasemi_nand_probe() could pass the check
> 
> 	if (pasemi_nand_mtd)
> 		return -ENODEV;
> 
> before any of them assigns a non-NULL value to it.
> 
> So rework the driver to make use of per-device driver data.
> 
> As an intended side effect the driver can bind more than one device and
> also gets rid of the check
> 
> 	if (!pasemi_nand_mtd)
> 		return 0;
> 
> in the remove callback that could only ever trigger after the above race
> happened.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/pasemi_nand.c b/drivers/mtd/nand/raw/pasemi_nand.c
index c176036453ed..f7ef6ca06ca9 100644
--- a/drivers/mtd/nand/raw/pasemi_nand.c
+++ b/drivers/mtd/nand/raw/pasemi_nand.c
@@ -26,9 +26,12 @@ 
 #define CLE_PIN_CTL			15
 #define ALE_PIN_CTL			14
 
-static unsigned int lpcctl;
-static struct mtd_info *pasemi_nand_mtd;
-static struct nand_controller controller;
+struct pasemi_ddata {
+	struct nand_chip chip;
+	unsigned int lpcctl;
+	struct nand_controller controller;
+};
+
 static const char driver_name[] = "pasemi-nand";
 
 static void pasemi_read_buf(struct nand_chip *chip, u_char *buf, int len)
@@ -55,6 +58,8 @@  static void pasemi_write_buf(struct nand_chip *chip, const u_char *buf,
 static void pasemi_hwcontrol(struct nand_chip *chip, int cmd,
 			     unsigned int ctrl)
 {
+	struct pasemi_ddata *ddata = container_of(chip, struct pasemi_ddata, chip);
+
 	if (cmd == NAND_CMD_NONE)
 		return;
 
@@ -65,12 +70,14 @@  static void pasemi_hwcontrol(struct nand_chip *chip, int cmd,
 
 	/* Push out posted writes */
 	eieio();
-	inl(lpcctl);
+	inl(ddata->lpcctl);
 }
 
 static int pasemi_device_ready(struct nand_chip *chip)
 {
-	return !!(inl(lpcctl) & LBICTRL_LPCCTL_NR);
+	struct pasemi_ddata *ddata = container_of(chip, struct pasemi_ddata, chip);
+
+	return !!(inl(ddata->lpcctl) & LBICTRL_LPCCTL_NR);
 }
 
 static int pasemi_attach_chip(struct nand_chip *chip)
@@ -93,29 +100,31 @@  static int pasemi_nand_probe(struct platform_device *ofdev)
 	struct device_node *np = dev->of_node;
 	struct resource res;
 	struct nand_chip *chip;
+	struct nand_controller *controller;
 	int err = 0;
+	struct pasemi_ddata *ddata;
+	struct mtd_info *pasemi_nand_mtd;
 
 	err = of_address_to_resource(np, 0, &res);
 
 	if (err)
 		return -EINVAL;
 
-	/* We only support one device at the moment */
-	if (pasemi_nand_mtd)
-		return -ENODEV;
-
 	dev_dbg(dev, "pasemi_nand at %pR\n", &res);
 
 	/* Allocate memory for MTD device structure and private data */
-	chip = kzalloc(sizeof(struct nand_chip), GFP_KERNEL);
-	if (!chip) {
+	ddata = kzalloc(sizeof(*ddata), GFP_KERNEL);
+	if (!ddata) {
 		err = -ENOMEM;
 		goto out;
 	}
+	platform_set_drvdata(ofdev, ddata);
+	chip = &ddata->chip;
+	controller = &ddata->controller;
 
-	controller.ops = &pasemi_ops;
-	nand_controller_init(&controller);
-	chip->controller = &controller;
+	controller->ops = &pasemi_ops;
+	nand_controller_init(controller);
+	chip->controller = controller;
 
 	pasemi_nand_mtd = nand_to_mtd(chip);
 
@@ -136,10 +145,10 @@  static int pasemi_nand_probe(struct platform_device *ofdev)
 		goto out_ior;
 	}
 
-	lpcctl = pci_resource_start(pdev, 0);
+	ddata->lpcctl = pci_resource_start(pdev, 0);
 	pci_dev_put(pdev);
 
-	if (!request_region(lpcctl, 4, driver_name)) {
+	if (!request_region(ddata->lpcctl, 4, driver_name)) {
 		err = -EBUSY;
 		goto out_ior;
 	}
@@ -172,45 +181,43 @@  static int pasemi_nand_probe(struct platform_device *ofdev)
 	}
 
 	dev_info(dev, "PA Semi NAND flash at %pR, control at I/O %x\n", &res,
-		 lpcctl);
+		 ddata->lpcctl);
 
 	return 0;
 
  out_cleanup_nand:
 	nand_cleanup(chip);
  out_lpc:
-	release_region(lpcctl, 4);
+	release_region(ddata->lpcctl, 4);
  out_ior:
 	iounmap(chip->legacy.IO_ADDR_R);
  out_mtd:
-	kfree(chip);
+	kfree(ddata);
  out:
 	return err;
 }
 
 static int pasemi_nand_remove(struct platform_device *ofdev)
 {
-	struct nand_chip *chip;
+	struct pasemi_ddata *ddata = platform_get_drvdata(ofdev);
+	struct mtd_info *pasemi_nand_mtd;
 	int ret;
+	struct nand_chip *chip;
 
-	if (!pasemi_nand_mtd)
-		return 0;
-
-	chip = mtd_to_nand(pasemi_nand_mtd);
+	chip = &ddata->chip;
+	pasemi_nand_mtd = nand_to_mtd(chip);
 
 	/* Release resources, unregister device */
 	ret = mtd_device_unregister(pasemi_nand_mtd);
 	WARN_ON(ret);
 	nand_cleanup(chip);
 
-	release_region(lpcctl, 4);
+	release_region(ddata->lpcctl, 4);
 
 	iounmap(chip->legacy.IO_ADDR_R);
 
 	/* Free the MTD device structure */
-	kfree(chip);
-
-	pasemi_nand_mtd = NULL;
+	kfree(ddata);
 
 	return 0;
 }