linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan <alan@linux.intel.com>
To: vincent.hervieux@gmail.com, sakari.ailus@linux.intel.com,
	linux-media@vger.kernel.org
Subject: [PATCH 1/3] atomisp: Fix up the open v load race
Date: Mon, 06 Nov 2017 23:36:36 +0000	[thread overview]
Message-ID: <151001137594.77201.4306351721772580664.stgit@alans-desktop> (raw)

This isn't the ideal final solution but it stops the main problem for now
where an open (often from udev) races the device initialization and we try
and load the firmware twice at the same time. This needless to say doesn't
usually end well.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 .../media/atomisp/pci/atomisp2/atomisp_fops.c      |   12 ++++++++++++
 .../media/atomisp/pci/atomisp2/atomisp_internal.h  |    5 +++++
 .../media/atomisp/pci/atomisp2/atomisp_v4l2.c      |    6 ++++++
 3 files changed, 23 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
index dd7596d8763d..b82c53cee32c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
@@ -771,6 +771,18 @@ static int atomisp_open(struct file *file)
 
 	dev_dbg(isp->dev, "open device %s\n", vdev->name);
 
+	/* Ensure that if we are still loading we block. Once the loading
+	   is over we can proceed. We can't blindly hold the lock until
+	   that occurs as if the load fails we'll deadlock the unload */
+	rt_mutex_lock(&isp->loading);
+	/* Revisit this with a better check once the code structure is
+	   cleaned up a bit more FIXME */
+	if (!isp->ready) {
+		rt_mutex_unlock(&isp->loading);
+		return -ENXIO;
+	}
+	rt_mutex_unlock(&isp->loading);
+
 	rt_mutex_lock(&isp->mutex);
 
 	acc_node = !strncmp(vdev->name, "ATOMISP ISP ACC",
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h
index 52a6f8002048..808d79c840d4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h
@@ -252,6 +252,11 @@ struct atomisp_device {
 	/* Purpose of mutex is to protect and serialize use of isp data
 	 * structures and css API calls. */
 	struct rt_mutex mutex;
+	/* This mutex ensures that we don't allow an open to succeed while
+	 * the initialization process is incomplete */
+	struct rt_mutex loading;
+	/* Set once the ISP is ready to allow opens */
+	bool ready;
 	/*
 	 * Serialise streamoff: mutex is dropped during streamoff to
 	 * cancel the watchdog queue. MUST be acquired BEFORE
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index 3c260f8b52e2..350e298bc3a6 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1220,6 +1220,7 @@ static int atomisp_pci_probe(struct pci_dev *dev,
 	isp->saved_regs.ispmmadr = start;
 
 	rt_mutex_init(&isp->mutex);
+	rt_mutex_init(&isp->loading);
 	mutex_init(&isp->streamoff_mutex);
 	spin_lock_init(&isp->lock);
 
@@ -1393,6 +1394,8 @@ static int atomisp_pci_probe(struct pci_dev *dev,
 				      csi_afe_trim);
 	}
 
+	rt_mutex_lock(&isp->loading);
+
 	err = atomisp_initialize_modules(isp);
 	if (err < 0) {
 		dev_err(&dev->dev, "atomisp_initialize_modules (%d)\n", err);
@@ -1450,6 +1453,8 @@ static int atomisp_pci_probe(struct pci_dev *dev,
 	release_firmware(isp->firmware);
 	isp->firmware = NULL;
 	isp->css_env.isp_css_fw.data = NULL;
+	isp->ready = true;
+	rt_mutex_unlock(&isp->loading);
 
 	atomisp_drvfs_init(&atomisp_pci_driver, isp);
 
@@ -1468,6 +1473,7 @@ static int atomisp_pci_probe(struct pci_dev *dev,
 register_entities_fail:
 	atomisp_uninitialize_modules(isp);
 initialize_modules_fail:
+	rt_mutex_unlock(&isp->loading);
 	pm_qos_remove_request(&isp->pm_qos);
 	atomisp_msi_irq_uninit(isp, dev);
 enable_msi_fail:

             reply	other threads:[~2017-11-06 23:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-06 23:36 Alan [this message]
2017-11-06 23:36 ` [PATCH 2/3] atomisp: fix vfree of bogus data on unload Alan
2017-11-13 22:05   ` Sakari Ailus
2017-11-14  0:16     ` Alan Cox
2017-11-14 14:10       ` Sakari Ailus
2017-11-06 23:37 ` [PATCH 3/3] atomisp: hmm gives a bogus warning " Alan
2017-12-12 11:03 ` [PATCH 1/3] atomisp: Fix up the open v load race Mauro Carvalho Chehab
2017-12-12 15:55   ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=151001137594.77201.4306351721772580664.stgit@alans-desktop \
    --to=alan@linux.intel.com \
    --cc=linux-media@vger.kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=vincent.hervieux@gmail.com \
    --subject='Re: [PATCH 1/3] atomisp: Fix up the open v load race' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).