All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: Export an unlocked flavor of i2c_transfer
@ 2012-06-29 10:47 Jean Delvare
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Jean Delvare @ 2012-06-29 10:47 UTC (permalink / raw)
  To: Linux I2C, Mauro Carvalho Chehab; +Cc: LMML

Some drivers (in particular for TV cards) need exclusive access to
their I2C buses for specific operations. Export an unlocked flavor
of i2c_transfer to give them full control.

The unlocked flavor has the following limitations:
* Obviously, caller must hold the i2c adapter lock.
* No debug messages are logged. We don't want to log messages while
  holding a rt_mutex.
* No check is done on the existence of adap->algo->master_xfer. It
  is thus the caller's responsibility to ensure that the function is
  OK to call.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
---
Mauro, would this be OK with you?

 drivers/i2c/i2c-core.c |   44 +++++++++++++++++++++++++++++++++-----------
 include/linux/i2c.h    |    3 +++
 2 files changed, 36 insertions(+), 11 deletions(-)

--- linux-3.5-rc4.orig/drivers/i2c/i2c-core.c	2012-06-05 16:22:59.000000000 +0200
+++ linux-3.5-rc4/drivers/i2c/i2c-core.c	2012-06-29 12:41:04.707793937 +0200
@@ -1312,6 +1312,37 @@ module_exit(i2c_exit);
  */
 
 /**
+ * __i2c_transfer - unlocked flavor of i2c_transfer
+ * @adap: Handle to I2C bus
+ * @msgs: One or more messages to execute before STOP is issued to
+ *	terminate the operation; each message begins with a START.
+ * @num: Number of messages to be executed.
+ *
+ * Returns negative errno, else the number of messages executed.
+ *
+ * Adapter lock must be held when calling this function. No debug logging
+ * takes place. adap->algo->master_xfer existence isn't checked.
+ */
+int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	unsigned long orig_jiffies;
+	int ret, try;
+
+	/* Retry automatically on arbitration loss */
+	orig_jiffies = jiffies;
+	for (ret = 0, try = 0; try <= adap->retries; try++) {
+		ret = adap->algo->master_xfer(adap, msgs, num);
+		if (ret != -EAGAIN)
+			break;
+		if (time_after(jiffies, orig_jiffies + adap->timeout))
+			break;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(__i2c_transfer);
+
+/**
  * i2c_transfer - execute a single or combined I2C message
  * @adap: Handle to I2C bus
  * @msgs: One or more messages to execute before STOP is issued to
@@ -1325,8 +1356,7 @@ module_exit(i2c_exit);
  */
 int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
-	unsigned long orig_jiffies;
-	int ret, try;
+	int ret;
 
 	/* REVISIT the fault reporting model here is weak:
 	 *
@@ -1364,15 +1394,7 @@ int i2c_transfer(struct i2c_adapter *ada
 			i2c_lock_adapter(adap);
 		}
 
-		/* Retry automatically on arbitration loss */
-		orig_jiffies = jiffies;
-		for (ret = 0, try = 0; try <= adap->retries; try++) {
-			ret = adap->algo->master_xfer(adap, msgs, num);
-			if (ret != -EAGAIN)
-				break;
-			if (time_after(jiffies, orig_jiffies + adap->timeout))
-				break;
-		}
+		ret = __i2c_transfer(adap, msgs, num);
 		i2c_unlock_adapter(adap);
 
 		return ret;
--- linux-3.5-rc4.orig/include/linux/i2c.h	2012-06-05 16:23:05.000000000 +0200
+++ linux-3.5-rc4/include/linux/i2c.h	2012-06-29 10:29:47.865621249 +0200
@@ -68,6 +68,9 @@ extern int i2c_master_recv(const struct
  */
 extern int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			int num);
+/* Unlocked flavor */
+extern int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			  int num);
 
 /* This is the very generalized SMBus access routine. You probably do not
    want to use this, though; one of the functions below may be much easier,

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 0/4] drxk: use request_firmware_nowait()
  2012-06-29 10:47 [PATCH] i2c: Export an unlocked flavor of i2c_transfer Jean Delvare
@ 2012-06-29 21:51 ` Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 1/4] [media] drxk: change it to " Mauro Carvalho Chehab
                     ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 21:51 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List

This patch series should be applied after "i2c: Export an unlocked 
flavor of i2c_transfer". It converts the drxk driver to use
request_firmware_nowait() and prevents I2C bus usage during firmware
load.

If firmware load doesn't happen and the device cannot be reset due
to that, -ENODEV will be returned to all dvb callbacks.

Mauro Carvalho Chehab (4):
  [media] drxk: change it to use request_firmware_nowait()
  [media] drxk: pass drxk priv struct instead of I2C adapter to i2c
    calls
  [media] drxk: Lock I2C bus during firmware load
  [media] drxk: prevent doing something wrong when init is not ok

 drivers/media/dvb/frontends/drxk_hard.c |  228 +++++++++++++++++++++++--------
 drivers/media/dvb/frontends/drxk_hard.h |   16 ++-
 2 files changed, 187 insertions(+), 57 deletions(-)

-- 
1.7.10.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] [media] drxk: change it to use request_firmware_nowait()
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
@ 2012-06-29 21:51   ` Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 2/4] [media] drxk: pass drxk priv struct instead of I2C adapter to i2c calls Mauro Carvalho Chehab
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 21:51 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List

The firmware blob may not be available when the driver probes.

Instead of blocking the whole kernel use request_firmware_nowait() and
continue without firmware.

This shouldn't be that bad on drx-k devices, as they all seem to have an
internal firmware. So, only the firmware update will take a little longer
to happen.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c |  109 +++++++++++++++++++------------
 drivers/media/dvb/frontends/drxk_hard.h |    3 +
 2 files changed, 72 insertions(+), 40 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 60b868f..4cb8d1e 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -5968,29 +5968,9 @@ error:
 	return status;
 }
 
-static int load_microcode(struct drxk_state *state, const char *mc_name)
-{
-	const struct firmware *fw = NULL;
-	int err = 0;
-
-	dprintk(1, "\n");
-
-	err = request_firmware(&fw, mc_name, state->i2c->dev.parent);
-	if (err < 0) {
-		printk(KERN_ERR
-		       "drxk: Could not load firmware file %s.\n", mc_name);
-		printk(KERN_INFO
-		       "drxk: Copy %s to your hotplug directory!\n", mc_name);
-		return err;
-	}
-	err = DownloadMicrocode(state, fw->data, fw->size);
-	release_firmware(fw);
-	return err;
-}
-
 static int init_drxk(struct drxk_state *state)
 {
-	int status = 0;
+	int status = 0, n = 0;
 	enum DRXPowerMode powerMode = DRXK_POWER_DOWN_OFDM;
 	u16 driverVersion;
 
@@ -6073,8 +6053,12 @@ static int init_drxk(struct drxk_state *state)
 		if (status < 0)
 			goto error;
 
-		if (state->microcode_name)
-			load_microcode(state, state->microcode_name);
+		if (state->fw) {
+			status = DownloadMicrocode(state, state->fw->data,
+						   state->fw->size);
+			if (status < 0)
+				goto error;
+		}
 
 		/* disable token-ring bus through OFDM block for possible ucode upload */
 		status = write16(state, SIO_OFDM_SH_OFDM_RING_ENABLE__A, SIO_OFDM_SH_OFDM_RING_ENABLE_OFF);
@@ -6167,6 +6151,20 @@ static int init_drxk(struct drxk_state *state)
 			state->m_DrxkState = DRXK_POWERED_DOWN;
 		} else
 			state->m_DrxkState = DRXK_STOPPED;
+
+		/* Initialize the supported delivery systems */
+		n = 0;
+		if (state->m_hasDVBC) {
+			state->frontend.ops.delsys[n++] = SYS_DVBC_ANNEX_A;
+			state->frontend.ops.delsys[n++] = SYS_DVBC_ANNEX_C;
+			strlcat(state->frontend.ops.info.name, " DVB-C",
+				sizeof(state->frontend.ops.info.name));
+		}
+		if (state->m_hasDVBT) {
+			state->frontend.ops.delsys[n++] = SYS_DVBT;
+			strlcat(state->frontend.ops.info.name, " DVB-T",
+				sizeof(state->frontend.ops.info.name));
+		}
 	}
 error:
 	if (status < 0)
@@ -6175,11 +6173,44 @@ error:
 	return status;
 }
 
+static void load_firmware_cb(const struct firmware *fw,
+			     void *context)
+{
+	struct drxk_state *state = context;
+
+	if (!fw) {
+		printk(KERN_ERR
+		       "drxk: Could not load firmware file %s.\n",
+			state->microcode_name);
+		printk(KERN_INFO
+		       "drxk: Copy %s to your hotplug directory!\n",
+			state->microcode_name);
+		state->microcode_name = NULL;
+
+		/*
+		 * As firmware is now load asynchronous, it is not possible
+		 * anymore to fail at frontend attach. We might silently
+		 * return here, and hope that the driver won't crash.
+		 * We might also change all DVB callbacks to return -ENODEV
+		 * if the device is not initialized.
+		 * As the DRX-K devices have their own internal firmware,
+		 * let's just hope that it will match a firmware revision
+		 * compatible with this driver and proceed.
+		 */
+	}
+	state->fw = fw;
+
+	init_drxk(state);
+}
+
 static void drxk_release(struct dvb_frontend *fe)
 {
 	struct drxk_state *state = fe->demodulator_priv;
 
 	dprintk(1, "\n");
+	if (state->fw)
+		release_firmware(state->fw);
+
 	kfree(state);
 }
 
@@ -6371,10 +6402,9 @@ static struct dvb_frontend_ops drxk_ops = {
 struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 				 struct i2c_adapter *i2c)
 {
-	int n;
-
 	struct drxk_state *state = NULL;
 	u8 adr = config->adr;
+	int status;
 
 	dprintk(1, "\n");
 	state = kzalloc(sizeof(struct drxk_state), GFP_KERNEL);
@@ -6425,22 +6455,21 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 	state->frontend.demodulator_priv = state;
 
 	init_state(state);
-	if (init_drxk(state) < 0)
-		goto error;
 
-	/* Initialize the supported delivery systems */
-	n = 0;
-	if (state->m_hasDVBC) {
-		state->frontend.ops.delsys[n++] = SYS_DVBC_ANNEX_A;
-		state->frontend.ops.delsys[n++] = SYS_DVBC_ANNEX_C;
-		strlcat(state->frontend.ops.info.name, " DVB-C",
-			sizeof(state->frontend.ops.info.name));
-	}
-	if (state->m_hasDVBT) {
-		state->frontend.ops.delsys[n++] = SYS_DVBT;
-		strlcat(state->frontend.ops.info.name, " DVB-T",
-			sizeof(state->frontend.ops.info.name));
-	}
+	/* Load firmware and initialize DRX-K */
+	if (state->microcode_name) {
+		status = request_firmware_nowait(THIS_MODULE, 1,
+					      state->microcode_name,
+					      state->i2c->dev.parent,
+					      GFP_KERNEL,
+					      state, load_firmware_cb);
+		if (status < 0) {
+			printk(KERN_ERR
+			"drxk: failed to request a firmware\n");
+			return NULL;
+		}
+	} else if (init_drxk(state) < 0)
+		goto error;
 
 	printk(KERN_INFO "drxk: frontend initialized.\n");
 	return &state->frontend;
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index 4bbf841..36677cd 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -338,7 +338,10 @@ struct drxk_state {
 	bool	antenna_dvbt;
 	u16	antenna_gpio;
 
+	/* Firmware */
 	const char *microcode_name;
+	struct completion fw_wait_load;
+	const struct firmware *fw;
 };
 
 #define NEVER_LOCK 0
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] [media] drxk: pass drxk priv struct instead of I2C adapter to i2c calls
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 1/4] [media] drxk: change it to " Mauro Carvalho Chehab
@ 2012-06-29 21:51   ` Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 3/4] [media] drxk: Lock I2C bus during firmware load Mauro Carvalho Chehab
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 21:51 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List

As it will be using the unlocked version of i2c_transfer during
firmware loads, make sure that the priv state routine will be
used on all I2C calls, in preparation for the next patch that
will implement an exclusive lock mode to be used during firmware
load, at drxk_init.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c |   34 ++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 4cb8d1e..5b3a17c 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -308,16 +308,22 @@ static u32 Log10Times100(u32 x)
 /* I2C **********************************************************************/
 /****************************************************************************/
 
-static int i2c_read1(struct i2c_adapter *adapter, u8 adr, u8 *val)
+static int drxk_i2c_transfer(struct drxk_state *state, struct i2c_msg *msgs,
+			     unsigned len)
+{
+	return i2c_transfer(state->i2c, msgs, len);
+}
+
+static int i2c_read1(struct drxk_state *state, u8 adr, u8 *val)
 {
 	struct i2c_msg msgs[1] = { {.addr = adr, .flags = I2C_M_RD,
 				    .buf = val, .len = 1}
 	};
 
-	return i2c_transfer(adapter, msgs, 1);
+	return drxk_i2c_transfer(state, msgs, 1);
 }
 
-static int i2c_write(struct i2c_adapter *adap, u8 adr, u8 *data, int len)
+static int i2c_write(struct drxk_state *state, u8 adr, u8 *data, int len)
 {
 	int status;
 	struct i2c_msg msg = {
@@ -330,7 +336,7 @@ static int i2c_write(struct i2c_adapter *adap, u8 adr, u8 *data, int len)
 			printk(KERN_CONT " %02x", data[i]);
 		printk(KERN_CONT "\n");
 	}
-	status = i2c_transfer(adap, &msg, 1);
+	status = drxk_i2c_transfer(state, &msg, 1);
 	if (status >= 0 && status != 1)
 		status = -EIO;
 
@@ -340,7 +346,7 @@ static int i2c_write(struct i2c_adapter *adap, u8 adr, u8 *data, int len)
 	return status;
 }
 
-static int i2c_read(struct i2c_adapter *adap,
+static int i2c_read(struct drxk_state *state,
 		    u8 adr, u8 *msg, int len, u8 *answ, int alen)
 {
 	int status;
@@ -351,7 +357,7 @@ static int i2c_read(struct i2c_adapter *adap,
 		 .buf = answ, .len = alen}
 	};
 
-	status = i2c_transfer(adap, msgs, 2);
+	status = drxk_i2c_transfer(state, msgs, 2);
 	if (status != 2) {
 		if (debug > 2)
 			printk(KERN_CONT ": ERROR!\n");
@@ -394,7 +400,7 @@ static int read16_flags(struct drxk_state *state, u32 reg, u16 *data, u8 flags)
 		len = 2;
 	}
 	dprintk(2, "(0x%08x, 0x%02x)\n", reg, flags);
-	status = i2c_read(state->i2c, adr, mm1, len, mm2, 2);
+	status = i2c_read(state, adr, mm1, len, mm2, 2);
 	if (status < 0)
 		return status;
 	if (data)
@@ -428,7 +434,7 @@ static int read32_flags(struct drxk_state *state, u32 reg, u32 *data, u8 flags)
 		len = 2;
 	}
 	dprintk(2, "(0x%08x, 0x%02x)\n", reg, flags);
-	status = i2c_read(state->i2c, adr, mm1, len, mm2, 4);
+	status = i2c_read(state, adr, mm1, len, mm2, 4);
 	if (status < 0)
 		return status;
 	if (data)
@@ -464,7 +470,7 @@ static int write16_flags(struct drxk_state *state, u32 reg, u16 data, u8 flags)
 	mm[len + 1] = (data >> 8) & 0xff;
 
 	dprintk(2, "(0x%08x, 0x%04x, 0x%02x)\n", reg, data, flags);
-	return i2c_write(state->i2c, adr, mm, len + 2);
+	return i2c_write(state, adr, mm, len + 2);
 }
 
 static int write16(struct drxk_state *state, u32 reg, u16 data)
@@ -495,7 +501,7 @@ static int write32_flags(struct drxk_state *state, u32 reg, u32 data, u8 flags)
 	mm[len + 3] = (data >> 24) & 0xff;
 	dprintk(2, "(0x%08x, 0x%08x, 0x%02x)\n", reg, data, flags);
 
-	return i2c_write(state->i2c, adr, mm, len + 4);
+	return i2c_write(state, adr, mm, len + 4);
 }
 
 static int write32(struct drxk_state *state, u32 reg, u32 data)
@@ -542,7 +548,7 @@ static int write_block(struct drxk_state *state, u32 Address,
 					printk(KERN_CONT " %02x", pBlock[i]);
 			printk(KERN_CONT "\n");
 		}
-		status = i2c_write(state->i2c, state->demod_address,
+		status = i2c_write(state, state->demod_address,
 				   &state->Chunk[0], Chunk + AdrLength);
 		if (status < 0) {
 			printk(KERN_ERR "drxk: %s: i2c write error at addr 0x%02x\n",
@@ -568,17 +574,17 @@ int PowerUpDevice(struct drxk_state *state)
 
 	dprintk(1, "\n");
 
-	status = i2c_read1(state->i2c, state->demod_address, &data);
+	status = i2c_read1(state, state->demod_address, &data);
 	if (status < 0) {
 		do {
 			data = 0;
-			status = i2c_write(state->i2c, state->demod_address,
+			status = i2c_write(state, state->demod_address,
 					   &data, 1);
 			msleep(10);
 			retryCount++;
 			if (status < 0)
 				continue;
-			status = i2c_read1(state->i2c, state->demod_address,
+			status = i2c_read1(state, state->demod_address,
 					   &data);
 		} while (status < 0 &&
 			 (retryCount < DRXK_MAX_RETRIES_POWERUP));
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] [media] drxk: Lock I2C bus during firmware load
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 1/4] [media] drxk: change it to " Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 2/4] [media] drxk: pass drxk priv struct instead of I2C adapter to i2c calls Mauro Carvalho Chehab
@ 2012-06-29 21:51   ` Mauro Carvalho Chehab
  2012-06-29 21:51   ` [PATCH 4/4] [media] drxk: prevent doing something wrong when init is not ok Mauro Carvalho Chehab
  2012-07-03  7:43   ` [PATCH 0/4] drxk: use request_firmware_nowait() Hans Verkuil
  4 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 21:51 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List

Don't allow other devices at the same I2C bus to use it during
firmware load, in order to prevent using the device while it is
not on a sane state.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c |   29 +++++++++++++++++++++++++++--
 drivers/media/dvb/frontends/drxk_hard.h |    3 +++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 5b3a17c..87cb3f0 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -28,6 +28,7 @@
 #include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/i2c.h>
+#include <linux/hardirq.h>
 #include <asm/div64.h>
 
 #include "dvb_frontend.h"
@@ -308,10 +309,30 @@ static u32 Log10Times100(u32 x)
 /* I2C **********************************************************************/
 /****************************************************************************/
 
+static int drxk_i2c_lock(struct drxk_state *state)
+{
+	i2c_lock_adapter(state->i2c);
+	state->drxk_i2c_exclusive_lock = true;
+
+	return 0;
+}
+
+static void drxk_i2c_unlock(struct drxk_state *state)
+{
+	if (!state->drxk_i2c_exclusive_lock)
+		return;
+
+	i2c_unlock_adapter(state->i2c);
+	state->drxk_i2c_exclusive_lock = false;
+}
+
 static int drxk_i2c_transfer(struct drxk_state *state, struct i2c_msg *msgs,
 			     unsigned len)
 {
-	return i2c_transfer(state->i2c, msgs, len);
+	if (state->drxk_i2c_exclusive_lock)
+		return __i2c_transfer(state->i2c, msgs, len);
+	else
+		return i2c_transfer(state->i2c, msgs, len);
 }
 
 static int i2c_read1(struct drxk_state *state, u8 adr, u8 *val)
@@ -5982,6 +6003,7 @@ static int init_drxk(struct drxk_state *state)
 
 	dprintk(1, "\n");
 	if ((state->m_DrxkState == DRXK_UNINITIALIZED)) {
+		drxk_i2c_lock(state);
 		status = PowerUpDevice(state);
 		if (status < 0)
 			goto error;
@@ -6171,10 +6193,13 @@ static int init_drxk(struct drxk_state *state)
 			strlcat(state->frontend.ops.info.name, " DVB-T",
 				sizeof(state->frontend.ops.info.name));
 		}
+		drxk_i2c_unlock(state);
 	}
 error:
-	if (status < 0)
+	if (status < 0) {
+		drxk_i2c_unlock(state);
 		printk(KERN_ERR "drxk: Error %d on %s\n", status, __func__);
+	}
 
 	return status;
 }
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index 36677cd..c35ab2b 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -325,6 +325,9 @@ struct drxk_state {
 
 	enum DRXPowerMode m_currentPowerMode;
 
+	/* when true, avoids other devices to use the I2C bus */
+	bool		  drxk_i2c_exclusive_lock;
+
 	/*
 	 * Configurable parameters at the driver. They stores the values found
 	 * at struct drxk_config.
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] [media] drxk: prevent doing something wrong when init is not ok
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
                     ` (2 preceding siblings ...)
  2012-06-29 21:51   ` [PATCH 3/4] [media] drxk: Lock I2C bus during firmware load Mauro Carvalho Chehab
@ 2012-06-29 21:51   ` Mauro Carvalho Chehab
  2012-07-03  7:43   ` [PATCH 0/4] drxk: use request_firmware_nowait() Hans Verkuil
  4 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-06-29 21:51 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List

If firmware is not loaded for some reason, or if it is not ready
yet, it makes no sense to honour to any DVB callbacks.

So, return -EAGAIN, as the error condition may be temporary.
If the device doesn't initialize, either because it requires a
firmware or because there's an error during init_drxk, returns
-ENODEV, to indicate such error, on all DVB callbacks.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c |   58 ++++++++++++++++++++++++++++++-
 drivers/media/dvb/frontends/drxk_hard.h |   10 +++++-
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 87cb3f0..8fa28bb 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -2851,7 +2851,7 @@ static int ConfigureI2CBridge(struct drxk_state *state, bool bEnableBridge)
 	dprintk(1, "\n");
 
 	if (state->m_DrxkState == DRXK_UNINITIALIZED)
-		goto error;
+		return 0;
 	if (state->m_DrxkState == DRXK_POWERED_DOWN)
 		goto error;
 
@@ -6197,6 +6197,7 @@ static int init_drxk(struct drxk_state *state)
 	}
 error:
 	if (status < 0) {
+		state->m_DrxkState = DRXK_NO_DEV;
 		drxk_i2c_unlock(state);
 		printk(KERN_ERR "drxk: Error %d on %s\n", status, __func__);
 	}
@@ -6209,6 +6210,7 @@ static void load_firmware_cb(const struct firmware *fw,
 {
 	struct drxk_state *state = context;
 
+	dprintk(1, ": %s\n", fw ? "firmware loaded" : "firmware not loaded");
 	if (!fw) {
 		printk(KERN_ERR
 		       "drxk: Could not load firmware file %s.\n",
@@ -6250,6 +6252,12 @@ static int drxk_sleep(struct dvb_frontend *fe)
 	struct drxk_state *state = fe->demodulator_priv;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return 0;
+
 	ShutDown(state);
 	return 0;
 }
@@ -6259,6 +6267,10 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 	struct drxk_state *state = fe->demodulator_priv;
 
 	dprintk(1, "%s\n", enable ? "enable" : "disable");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+
 	return ConfigureI2CBridge(state, enable ? true : false);
 }
 
@@ -6271,6 +6283,12 @@ static int drxk_set_parameters(struct dvb_frontend *fe)
 
 	dprintk(1, "\n");
 
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	if (!fe->ops.tuner_ops.get_if_frequency) {
 		printk(KERN_ERR
 		       "drxk: Error: get_if_frequency() not defined at tuner. Can't work without it!\n");
@@ -6324,6 +6342,12 @@ static int drxk_read_status(struct dvb_frontend *fe, fe_status_t *status)
 	u32 stat;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	*status = 0;
 	GetLockStatus(state, &stat, 0);
 	if (stat == MPEG_LOCK)
@@ -6337,8 +6361,15 @@ static int drxk_read_status(struct dvb_frontend *fe, fe_status_t *status)
 
 static int drxk_read_ber(struct dvb_frontend *fe, u32 *ber)
 {
+	struct drxk_state *state = fe->demodulator_priv;
+
 	dprintk(1, "\n");
 
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	*ber = 0;
 	return 0;
 }
@@ -6350,6 +6381,12 @@ static int drxk_read_signal_strength(struct dvb_frontend *fe,
 	u32 val = 0;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	ReadIFAgc(state, &val);
 	*strength = val & 0xffff;
 	return 0;
@@ -6361,6 +6398,12 @@ static int drxk_read_snr(struct dvb_frontend *fe, u16 *snr)
 	s32 snr2;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	GetSignalToNoise(state, &snr2);
 	*snr = snr2 & 0xffff;
 	return 0;
@@ -6372,6 +6415,12 @@ static int drxk_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
 	u16 err;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	DVBTQAMGetAccPktErr(state, &err);
 	*ucblocks = (u32) err;
 	return 0;
@@ -6380,9 +6429,16 @@ static int drxk_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
 static int drxk_get_tune_settings(struct dvb_frontend *fe, struct dvb_frontend_tune_settings
 				    *sets)
 {
+	struct drxk_state *state = fe->demodulator_priv;
 	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
 
 	dprintk(1, "\n");
+
+	if (state->m_DrxkState == DRXK_NO_DEV)
+		return -ENODEV;
+	if (state->m_DrxkState == DRXK_UNINITIALIZED)
+		return -EAGAIN;
+
 	switch (p->delivery_system) {
 	case SYS_DVBC_ANNEX_A:
 	case SYS_DVBC_ANNEX_C:
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index c35ab2b..f417797 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -94,7 +94,15 @@ enum DRXPowerMode {
 
 
 enum AGC_CTRL_MODE { DRXK_AGC_CTRL_AUTO = 0, DRXK_AGC_CTRL_USER, DRXK_AGC_CTRL_OFF };
-enum EDrxkState { DRXK_UNINITIALIZED = 0, DRXK_STOPPED, DRXK_DTV_STARTED, DRXK_ATV_STARTED, DRXK_POWERED_DOWN };
+enum EDrxkState {
+	DRXK_UNINITIALIZED = 0,
+	DRXK_STOPPED,
+	DRXK_DTV_STARTED,
+	DRXK_ATV_STARTED,
+	DRXK_POWERED_DOWN,
+	DRXK_NO_DEV			/* If drxk init failed */
+};
+
 enum EDrxkCoefArrayIndex {
 	DRXK_COEF_IDX_MN = 0,
 	DRXK_COEF_IDX_FM    ,
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] drxk: use request_firmware_nowait()
  2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
                     ` (3 preceding siblings ...)
  2012-06-29 21:51   ` [PATCH 4/4] [media] drxk: prevent doing something wrong when init is not ok Mauro Carvalho Chehab
@ 2012-07-03  7:43   ` Hans Verkuil
  4 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2012-07-03  7:43 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List

Hi Mauro!

On Fri 29 June 2012 23:51:53 Mauro Carvalho Chehab wrote:
> This patch series should be applied after "i2c: Export an unlocked 
> flavor of i2c_transfer". It converts the drxk driver to use
> request_firmware_nowait() and prevents I2C bus usage during firmware
> load.

Can you take a look at media_build? After this change it fails to build drxk
for kernels <= 3.4 (i.e., all kernels :-) ).

Regards,

	Hans

> 
> If firmware load doesn't happen and the device cannot be reset due
> to that, -ENODEV will be returned to all dvb callbacks.
> 
> Mauro Carvalho Chehab (4):
>   [media] drxk: change it to use request_firmware_nowait()
>   [media] drxk: pass drxk priv struct instead of I2C adapter to i2c
>     calls
>   [media] drxk: Lock I2C bus during firmware load
>   [media] drxk: prevent doing something wrong when init is not ok
> 
>  drivers/media/dvb/frontends/drxk_hard.c |  228 +++++++++++++++++++++++--------
>  drivers/media/dvb/frontends/drxk_hard.h |   16 ++-
>  2 files changed, 187 insertions(+), 57 deletions(-)
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-07-03  7:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-29 10:47 [PATCH] i2c: Export an unlocked flavor of i2c_transfer Jean Delvare
2012-06-29 21:51 ` [PATCH 0/4] drxk: use request_firmware_nowait() Mauro Carvalho Chehab
2012-06-29 21:51   ` [PATCH 1/4] [media] drxk: change it to " Mauro Carvalho Chehab
2012-06-29 21:51   ` [PATCH 2/4] [media] drxk: pass drxk priv struct instead of I2C adapter to i2c calls Mauro Carvalho Chehab
2012-06-29 21:51   ` [PATCH 3/4] [media] drxk: Lock I2C bus during firmware load Mauro Carvalho Chehab
2012-06-29 21:51   ` [PATCH 4/4] [media] drxk: prevent doing something wrong when init is not ok Mauro Carvalho Chehab
2012-07-03  7:43   ` [PATCH 0/4] drxk: use request_firmware_nowait() Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.