Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH 01/22] si2168: define symbol rate limits
@ 2014-12-06 21:34 Antti Palosaari
  2014-12-06 21:34 ` [PATCH 02/22] si2168: rename device state variable from 's' to 'dev' Antti Palosaari
                   ` (21 more replies)
  0 siblings, 22 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

w_scan complains about missing symbol rate limits:
This dvb driver is *buggy*: the symbol rate limits are undefined - please report to linuxtv.org

Chip supports 1 to 7.2 MSymbol/s on DVB-C.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index ce9ab44..acf0fc3 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -635,6 +635,8 @@ static const struct dvb_frontend_ops si2168_ops = {
 	.delsys = {SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A},
 	.info = {
 		.name = "Silicon Labs Si2168",
+		.symbol_rate_min = 1000000,
+		.symbol_rate_max = 7200000,
 		.caps =	FE_CAN_FEC_1_2 |
 			FE_CAN_FEC_2_3 |
 			FE_CAN_FEC_3_4 |
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 02/22] si2168: rename device state variable from 's' to 'dev'
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 03/22] si2168: carry pointer to client instead of state Antti Palosaari
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

'dev' is most common name in kernel for structure containing device
state instance, so rename it.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c      | 202 +++++++++++++++---------------
 drivers/media/dvb-frontends/si2168_priv.h |   2 +-
 2 files changed, 102 insertions(+), 102 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index acf0fc3..e989bd4 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -19,16 +19,16 @@
 static const struct dvb_frontend_ops si2168_ops;
 
 /* execute firmware command */
-static int si2168_cmd_execute(struct si2168 *s, struct si2168_cmd *cmd)
+static int si2168_cmd_execute(struct si2168_dev *dev, struct si2168_cmd *cmd)
 {
 	int ret;
 	unsigned long timeout;
 
-	mutex_lock(&s->i2c_mutex);
+	mutex_lock(&dev->i2c_mutex);
 
 	if (cmd->wlen) {
 		/* write cmd and args for firmware */
-		ret = i2c_master_send(s->client, cmd->args, cmd->wlen);
+		ret = i2c_master_send(dev->client, cmd->args, cmd->wlen);
 		if (ret < 0) {
 			goto err_mutex_unlock;
 		} else if (ret != cmd->wlen) {
@@ -42,7 +42,7 @@ static int si2168_cmd_execute(struct si2168 *s, struct si2168_cmd *cmd)
 		#define TIMEOUT 50
 		timeout = jiffies + msecs_to_jiffies(TIMEOUT);
 		while (!time_after(jiffies, timeout)) {
-			ret = i2c_master_recv(s->client, cmd->args, cmd->rlen);
+			ret = i2c_master_recv(dev->client, cmd->args, cmd->rlen);
 			if (ret < 0) {
 				goto err_mutex_unlock;
 			} else if (ret != cmd->rlen) {
@@ -55,7 +55,7 @@ static int si2168_cmd_execute(struct si2168 *s, struct si2168_cmd *cmd)
 				break;
 		}
 
-		dev_dbg(&s->client->dev, "cmd execution took %d ms\n",
+		dev_dbg(&dev->client->dev, "cmd execution took %d ms\n",
 				jiffies_to_msecs(jiffies) -
 				(jiffies_to_msecs(timeout) - TIMEOUT));
 
@@ -68,26 +68,26 @@ static int si2168_cmd_execute(struct si2168 *s, struct si2168_cmd *cmd)
 	ret = 0;
 
 err_mutex_unlock:
-	mutex_unlock(&s->i2c_mutex);
+	mutex_unlock(&dev->i2c_mutex);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 {
-	struct si2168 *s = fe->demodulator_priv;
+	struct si2168_dev *dev = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2168_cmd cmd;
 
 	*status = 0;
 
-	if (!s->active) {
+	if (!dev->active) {
 		ret = -EAGAIN;
 		goto err;
 	}
@@ -113,7 +113,7 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		goto err;
 	}
 
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -138,7 +138,7 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		break;
 	}
 
-	s->fe_status = *status;
+	dev->fe_status = *status;
 
 	if (*status & FE_HAS_LOCK) {
 		c->cnr.len = 1;
@@ -149,30 +149,30 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 	}
 
-	dev_dbg(&s->client->dev, "status=%02x args=%*ph\n",
+	dev_dbg(&dev->client->dev, "status=%02x args=%*ph\n",
 			*status, cmd.rlen, cmd.args);
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2168_set_frontend(struct dvb_frontend *fe)
 {
-	struct si2168 *s = fe->demodulator_priv;
+	struct si2168_dev *dev = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2168_cmd cmd;
 	u8 bandwidth, delivery_system;
 
-	dev_dbg(&s->client->dev,
+	dev_dbg(&dev->client->dev,
 			"delivery_system=%u modulation=%u frequency=%u bandwidth_hz=%u symbol_rate=%u inversion=%u, stream_id=%d\n",
 			c->delivery_system, c->modulation,
 			c->frequency, c->bandwidth_hz, c->symbol_rate,
 			c->inversion, c->stream_id);
 
-	if (!s->active) {
+	if (!dev->active) {
 		ret = -EAGAIN;
 		goto err;
 	}
@@ -217,7 +217,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x88\x02\x02\x02\x02", 5);
 	cmd.wlen = 5;
 	cmd.rlen = 5;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -230,7 +230,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		memcpy(cmd.args, "\x89\x21\x06\x11\x89\x20", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 3;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -241,7 +241,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		cmd.args[2] = c->stream_id == NO_STREAM_ID_FILTER ? 0 : 1;
 		cmd.wlen = 3;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(s, &cmd);
+		ret = si2168_cmd_execute(dev, &cmd);
 		if (ret)
 			goto err;
 	}
@@ -249,35 +249,35 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x51\x03", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 12;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x12\x08\x04", 3);
 	cmd.wlen = 3;
 	cmd.rlen = 3;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x0c\x10\x12\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x06\x10\x24\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x07\x10\x00\x24", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -285,7 +285,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	cmd.args[4] = delivery_system | bandwidth;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -296,7 +296,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		cmd.args[5] = ((c->symbol_rate / 1000) >> 8) & 0xff;
 		cmd.wlen = 6;
 		cmd.rlen = 4;
-		ret = si2168_cmd_execute(s, &cmd);
+		ret = si2168_cmd_execute(dev, &cmd);
 		if (ret)
 			goto err;
 	}
@@ -304,58 +304,58 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x14\x00\x0f\x10\x10\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x09\x10\xe3\x08", 6);
-	cmd.args[5] |= s->ts_clock_inv ? 0x00 : 0x10;
+	cmd.args[5] |= dev->ts_clock_inv ? 0x00 : 0x10;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x08\x10\xd7\x05", 6);
-	cmd.args[5] |= s->ts_clock_inv ? 0x00 : 0x10;
+	cmd.args[5] |= dev->ts_clock_inv ? 0x00 : 0x10;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x01\x12\x00\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x01\x03\x0c\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x85", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	s->delivery_system = c->delivery_system;
+	dev->delivery_system = c->delivery_system;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2168_init(struct dvb_frontend *fe)
 {
-	struct si2168 *s = fe->demodulator_priv;
+	struct si2168_dev *dev = fe->demodulator_priv;
 	int ret, len, remaining;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
@@ -363,29 +363,29 @@ static int si2168_init(struct dvb_frontend *fe)
 	struct si2168_cmd cmd;
 	unsigned int chip_id;
 
-	dev_dbg(&s->client->dev, "\n");
+	dev_dbg(&dev->client->dev, "\n");
 
 	/* initialize */
 	memcpy(cmd.args, "\xc0\x12\x00\x0c\x00\x0d\x16\x00\x00\x00\x00\x00\x00", 13);
 	cmd.wlen = 13;
 	cmd.rlen = 0;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	if (s->fw_loaded) {
+	if (dev->fw_loaded) {
 		/* resume */
 		memcpy(cmd.args, "\xc0\x06\x08\x0f\x00\x20\x21\x01", 8);
 		cmd.wlen = 8;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(s, &cmd);
+		ret = si2168_cmd_execute(dev, &cmd);
 		if (ret)
 			goto err;
 
 		memcpy(cmd.args, "\x85", 1);
 		cmd.wlen = 1;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(s, &cmd);
+		ret = si2168_cmd_execute(dev, &cmd);
 		if (ret)
 			goto err;
 
@@ -396,7 +396,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\xc0\x06\x01\x0f\x00\x20\x20\x01", 8);
 	cmd.wlen = 8;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -404,7 +404,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x02", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 13;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -426,7 +426,7 @@ static int si2168_init(struct dvb_frontend *fe)
 		fw_file = SI2168_B40_FIRMWARE;
 		break;
 	default:
-		dev_err(&s->client->dev,
+		dev_err(&dev->client->dev,
 				"unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
 				cmd.args[3], cmd.args[4]);
@@ -435,31 +435,31 @@ static int si2168_init(struct dvb_frontend *fe)
 	}
 
 	/* cold state - try to download firmware */
-	dev_info(&s->client->dev, "found a '%s' in cold state\n",
+	dev_info(&dev->client->dev, "found a '%s' in cold state\n",
 			si2168_ops.info.name);
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &s->client->dev);
+	ret = request_firmware(&fw, fw_file, &dev->client->dev);
 	if (ret) {
 		/* fallback mechanism to handle old name for Si2168 B40 fw */
 		if (chip_id == SI2168_B40) {
 			fw_file = SI2168_B40_FIRMWARE_FALLBACK;
-			ret = request_firmware(&fw, fw_file, &s->client->dev);
+			ret = request_firmware(&fw, fw_file, &dev->client->dev);
 		}
 
 		if (ret == 0) {
-			dev_notice(&s->client->dev,
+			dev_notice(&dev->client->dev,
 					"please install firmware file '%s'\n",
 					SI2168_B40_FIRMWARE);
 		} else {
-			dev_err(&s->client->dev,
+			dev_err(&dev->client->dev,
 					"firmware file '%s' not found\n",
 					fw_file);
 			goto error_fw_release;
 		}
 	}
 
-	dev_info(&s->client->dev, "downloading firmware from file '%s'\n",
+	dev_info(&dev->client->dev, "downloading firmware from file '%s'\n",
 			fw_file);
 
 	if ((fw->size % 17 == 0) && (fw->data[0] > 5)) {
@@ -469,9 +469,9 @@ static int si2168_init(struct dvb_frontend *fe)
 			memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
 			cmd.wlen = len;
 			cmd.rlen = 1;
-			ret = si2168_cmd_execute(s, &cmd);
+			ret = si2168_cmd_execute(dev, &cmd);
 			if (ret) {
-				dev_err(&s->client->dev,
+				dev_err(&dev->client->dev,
 						"firmware download failed=%d\n",
 						ret);
 				goto error_fw_release;
@@ -487,9 +487,9 @@ static int si2168_init(struct dvb_frontend *fe)
 			memcpy(cmd.args, &fw->data[fw->size - remaining], len);
 			cmd.wlen = len;
 			cmd.rlen = 1;
-			ret = si2168_cmd_execute(s, &cmd);
+			ret = si2168_cmd_execute(dev, &cmd);
 			if (ret) {
-				dev_err(&s->client->dev,
+				dev_err(&dev->client->dev,
 						"firmware download failed=%d\n",
 						ret);
 				goto error_fw_release;
@@ -503,7 +503,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x01\x01", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -511,58 +511,58 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x11", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 10;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	dev_dbg(&s->client->dev, "firmware version: %c.%c.%d\n",
+	dev_dbg(&dev->client->dev, "firmware version: %c.%c.%d\n",
 			cmd.args[6], cmd.args[7], cmd.args[8]);
 
 	/* set ts mode */
 	memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
-	cmd.args[4] |= s->ts_mode;
+	cmd.args[4] |= dev->ts_mode;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	s->fw_loaded = true;
+	dev->fw_loaded = true;
 
-	dev_info(&s->client->dev, "found a '%s' in warm state\n",
+	dev_info(&dev->client->dev, "found a '%s' in warm state\n",
 			si2168_ops.info.name);
 warm:
-	s->active = true;
+	dev->active = true;
 
 	return 0;
 
 error_fw_release:
 	release_firmware(fw);
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2168_sleep(struct dvb_frontend *fe)
 {
-	struct si2168 *s = fe->demodulator_priv;
+	struct si2168_dev *dev = fe->demodulator_priv;
 	int ret;
 	struct si2168_cmd cmd;
 
-	dev_dbg(&s->client->dev, "\n");
+	dev_dbg(&dev->client->dev, "\n");
 
-	s->active = false;
+	dev->active = false;
 
 	memcpy(cmd.args, "\x13", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 0;
-	ret = si2168_cmd_execute(s, &cmd);
+	ret = si2168_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -581,21 +581,21 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
  */
 static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 {
-	struct si2168 *s = mux_priv;
+	struct si2168_dev *dev = mux_priv;
 	int ret;
 	struct i2c_msg gate_open_msg = {
-		.addr = s->client->addr,
+		.addr = dev->client->addr,
 		.flags = 0,
 		.len = 3,
 		.buf = "\xc0\x0d\x01",
 	};
 
-	mutex_lock(&s->i2c_mutex);
+	mutex_lock(&dev->i2c_mutex);
 
 	/* open tuner I2C gate */
-	ret = __i2c_transfer(s->client->adapter, &gate_open_msg, 1);
+	ret = __i2c_transfer(dev->client->adapter, &gate_open_msg, 1);
 	if (ret != 1) {
-		dev_warn(&s->client->dev, "i2c write failed=%d\n", ret);
+		dev_warn(&dev->client->dev, "i2c write failed=%d\n", ret);
 		if (ret >= 0)
 			ret = -EREMOTEIO;
 	} else {
@@ -607,26 +607,26 @@ static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 
 static int si2168_deselect(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 {
-	struct si2168 *s = mux_priv;
+	struct si2168_dev *dev = mux_priv;
 	int ret;
 	struct i2c_msg gate_close_msg = {
-		.addr = s->client->addr,
+		.addr = dev->client->addr,
 		.flags = 0,
 		.len = 3,
 		.buf = "\xc0\x0d\x00",
 	};
 
 	/* close tuner I2C gate */
-	ret = __i2c_transfer(s->client->adapter, &gate_close_msg, 1);
+	ret = __i2c_transfer(dev->client->adapter, &gate_close_msg, 1);
 	if (ret != 1) {
-		dev_warn(&s->client->dev, "i2c write failed=%d\n", ret);
+		dev_warn(&dev->client->dev, "i2c write failed=%d\n", ret);
 		if (ret >= 0)
 			ret = -EREMOTEIO;
 	} else {
 		ret = 0;
 	}
 
-	mutex_unlock(&s->i2c_mutex);
+	mutex_unlock(&dev->i2c_mutex);
 
 	return ret;
 }
@@ -672,62 +672,62 @@ static int si2168_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
 	struct si2168_config *config = client->dev.platform_data;
-	struct si2168 *s;
+	struct si2168_dev *dev;
 	int ret;
 
 	dev_dbg(&client->dev, "\n");
 
-	s = kzalloc(sizeof(struct si2168), GFP_KERNEL);
-	if (!s) {
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
 		ret = -ENOMEM;
 		dev_err(&client->dev, "kzalloc() failed\n");
 		goto err;
 	}
 
-	s->client = client;
-	mutex_init(&s->i2c_mutex);
+	dev->client = client;
+	mutex_init(&dev->i2c_mutex);
 
 	/* create mux i2c adapter for tuner */
-	s->adapter = i2c_add_mux_adapter(client->adapter, &client->dev, s,
+	dev->adapter = i2c_add_mux_adapter(client->adapter, &client->dev, dev,
 			0, 0, 0, si2168_select, si2168_deselect);
-	if (s->adapter == NULL) {
+	if (dev->adapter == NULL) {
 		ret = -ENODEV;
 		goto err;
 	}
 
 	/* create dvb_frontend */
-	memcpy(&s->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
-	s->fe.demodulator_priv = s;
+	memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
+	dev->fe.demodulator_priv = dev;
 
-	*config->i2c_adapter = s->adapter;
-	*config->fe = &s->fe;
-	s->ts_mode = config->ts_mode;
-	s->ts_clock_inv = config->ts_clock_inv;
-	s->fw_loaded = false;
+	*config->i2c_adapter = dev->adapter;
+	*config->fe = &dev->fe;
+	dev->ts_mode = config->ts_mode;
+	dev->ts_clock_inv = config->ts_clock_inv;
+	dev->fw_loaded = false;
 
-	i2c_set_clientdata(client, s);
+	i2c_set_clientdata(client, dev);
 
-	dev_info(&s->client->dev,
+	dev_info(&dev->client->dev,
 			"Silicon Labs Si2168 successfully attached\n");
 	return 0;
 err:
-	kfree(s);
+	kfree(dev);
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2168_remove(struct i2c_client *client)
 {
-	struct si2168 *s = i2c_get_clientdata(client);
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 
 	dev_dbg(&client->dev, "\n");
 
-	i2c_del_mux_adapter(s->adapter);
+	i2c_del_mux_adapter(dev->adapter);
 
-	s->fe.ops.release = NULL;
-	s->fe.demodulator_priv = NULL;
+	dev->fe.ops.release = NULL;
+	dev->fe.demodulator_priv = NULL;
 
-	kfree(s);
+	kfree(dev);
 
 	return 0;
 }
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 60bc334..cb8827a 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -28,7 +28,7 @@
 #define SI2168_B40_FIRMWARE_FALLBACK "dvb-demod-si2168-02.fw"
 
 /* state struct */
-struct si2168 {
+struct si2168_dev {
 	struct i2c_client *client;
 	struct i2c_adapter *adapter;
 	struct mutex i2c_mutex;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 03/22] si2168: carry pointer to client instead of state
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
  2014-12-06 21:34 ` [PATCH 02/22] si2168: rename device state variable from 's' to 'dev' Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 04/22] si2168: get rid of own struct i2c_client pointer Antti Palosaari
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Carry struct i2c_client pointer inside struct dvb_frontend private
pointer. This driver is I2C driver, which is represented as a
struct i2c_client, so better to carry this top level structure for
each routine in order to unify things. This allows further
simplification.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 83 +++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index e989bd4..50674d4 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -19,8 +19,9 @@
 static const struct dvb_frontend_ops si2168_ops;
 
 /* execute firmware command */
-static int si2168_cmd_execute(struct si2168_dev *dev, struct si2168_cmd *cmd)
+static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
 {
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	unsigned long timeout;
 
@@ -80,7 +81,8 @@ err:
 
 static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 {
-	struct si2168_dev *dev = fe->demodulator_priv;
+	struct i2c_client *client = fe->demodulator_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2168_cmd cmd;
@@ -113,7 +115,7 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		goto err;
 	}
 
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -160,7 +162,8 @@ err:
 
 static int si2168_set_frontend(struct dvb_frontend *fe)
 {
-	struct si2168_dev *dev = fe->demodulator_priv;
+	struct i2c_client *client = fe->demodulator_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2168_cmd cmd;
@@ -217,7 +220,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x88\x02\x02\x02\x02", 5);
 	cmd.wlen = 5;
 	cmd.rlen = 5;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -230,7 +233,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		memcpy(cmd.args, "\x89\x21\x06\x11\x89\x20", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 3;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -241,7 +244,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		cmd.args[2] = c->stream_id == NO_STREAM_ID_FILTER ? 0 : 1;
 		cmd.wlen = 3;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(dev, &cmd);
+		ret = si2168_cmd_execute(client, &cmd);
 		if (ret)
 			goto err;
 	}
@@ -249,35 +252,35 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x51\x03", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 12;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x12\x08\x04", 3);
 	cmd.wlen = 3;
 	cmd.rlen = 3;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x0c\x10\x12\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x06\x10\x24\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x07\x10\x00\x24", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -285,7 +288,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	cmd.args[4] = delivery_system | bandwidth;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -296,7 +299,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 		cmd.args[5] = ((c->symbol_rate / 1000) >> 8) & 0xff;
 		cmd.wlen = 6;
 		cmd.rlen = 4;
-		ret = si2168_cmd_execute(dev, &cmd);
+		ret = si2168_cmd_execute(client, &cmd);
 		if (ret)
 			goto err;
 	}
@@ -304,7 +307,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x14\x00\x0f\x10\x10\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -312,7 +315,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	cmd.args[5] |= dev->ts_clock_inv ? 0x00 : 0x10;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -320,28 +323,28 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	cmd.args[5] |= dev->ts_clock_inv ? 0x00 : 0x10;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x01\x12\x00\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x14\x00\x01\x03\x0c\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	memcpy(cmd.args, "\x85", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -355,7 +358,8 @@ err:
 
 static int si2168_init(struct dvb_frontend *fe)
 {
-	struct si2168_dev *dev = fe->demodulator_priv;
+	struct i2c_client *client = fe->demodulator_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret, len, remaining;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
@@ -369,7 +373,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\xc0\x12\x00\x0c\x00\x0d\x16\x00\x00\x00\x00\x00\x00", 13);
 	cmd.wlen = 13;
 	cmd.rlen = 0;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -378,14 +382,14 @@ static int si2168_init(struct dvb_frontend *fe)
 		memcpy(cmd.args, "\xc0\x06\x08\x0f\x00\x20\x21\x01", 8);
 		cmd.wlen = 8;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(dev, &cmd);
+		ret = si2168_cmd_execute(client, &cmd);
 		if (ret)
 			goto err;
 
 		memcpy(cmd.args, "\x85", 1);
 		cmd.wlen = 1;
 		cmd.rlen = 1;
-		ret = si2168_cmd_execute(dev, &cmd);
+		ret = si2168_cmd_execute(client, &cmd);
 		if (ret)
 			goto err;
 
@@ -396,7 +400,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\xc0\x06\x01\x0f\x00\x20\x20\x01", 8);
 	cmd.wlen = 8;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -404,7 +408,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x02", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 13;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -469,7 +473,7 @@ static int si2168_init(struct dvb_frontend *fe)
 			memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
 			cmd.wlen = len;
 			cmd.rlen = 1;
-			ret = si2168_cmd_execute(dev, &cmd);
+			ret = si2168_cmd_execute(client, &cmd);
 			if (ret) {
 				dev_err(&dev->client->dev,
 						"firmware download failed=%d\n",
@@ -487,7 +491,7 @@ static int si2168_init(struct dvb_frontend *fe)
 			memcpy(cmd.args, &fw->data[fw->size - remaining], len);
 			cmd.wlen = len;
 			cmd.rlen = 1;
-			ret = si2168_cmd_execute(dev, &cmd);
+			ret = si2168_cmd_execute(client, &cmd);
 			if (ret) {
 				dev_err(&dev->client->dev,
 						"firmware download failed=%d\n",
@@ -503,7 +507,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x01\x01", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -511,7 +515,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x11", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 10;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -523,7 +527,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	cmd.args[4] |= dev->ts_mode;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -545,7 +549,8 @@ err:
 
 static int si2168_sleep(struct dvb_frontend *fe)
 {
-	struct si2168_dev *dev = fe->demodulator_priv;
+	struct i2c_client *client = fe->demodulator_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct si2168_cmd cmd;
 
@@ -556,7 +561,7 @@ static int si2168_sleep(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x13", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 0;
-	ret = si2168_cmd_execute(dev, &cmd);
+	ret = si2168_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -581,7 +586,8 @@ static int si2168_get_tune_settings(struct dvb_frontend *fe,
  */
 static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 {
-	struct si2168_dev *dev = mux_priv;
+	struct i2c_client *client = mux_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct i2c_msg gate_open_msg = {
 		.addr = dev->client->addr,
@@ -607,7 +613,8 @@ static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 
 static int si2168_deselect(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 {
-	struct si2168_dev *dev = mux_priv;
+	struct i2c_client *client = mux_priv;
+	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct i2c_msg gate_close_msg = {
 		.addr = dev->client->addr,
@@ -688,8 +695,8 @@ static int si2168_probe(struct i2c_client *client,
 	mutex_init(&dev->i2c_mutex);
 
 	/* create mux i2c adapter for tuner */
-	dev->adapter = i2c_add_mux_adapter(client->adapter, &client->dev, dev,
-			0, 0, 0, si2168_select, si2168_deselect);
+	dev->adapter = i2c_add_mux_adapter(client->adapter, &client->dev,
+			client, 0, 0, 0, si2168_select, si2168_deselect);
 	if (dev->adapter == NULL) {
 		ret = -ENODEV;
 		goto err;
@@ -697,7 +704,7 @@ static int si2168_probe(struct i2c_client *client,
 
 	/* create dvb_frontend */
 	memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
-	dev->fe.demodulator_priv = dev;
+	dev->fe.demodulator_priv = client;
 
 	*config->i2c_adapter = dev->adapter;
 	*config->fe = &dev->fe;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 04/22] si2168: get rid of own struct i2c_client pointer
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
  2014-12-06 21:34 ` [PATCH 02/22] si2168: rename device state variable from 's' to 'dev' Antti Palosaari
  2014-12-06 21:34 ` [PATCH 03/22] si2168: carry pointer to client instead of state Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 05/22] si2168: simplify si2168_cmd_execute() error path Antti Palosaari
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

We don't need that anymore as same pointer is passed to each
routine via struct dvb_frontend private field.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c      | 61 +++++++++++++++----------------
 drivers/media/dvb-frontends/si2168_priv.h |  1 -
 2 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 50674d4..db06bb7 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -29,7 +29,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
 
 	if (cmd->wlen) {
 		/* write cmd and args for firmware */
-		ret = i2c_master_send(dev->client, cmd->args, cmd->wlen);
+		ret = i2c_master_send(client, cmd->args, cmd->wlen);
 		if (ret < 0) {
 			goto err_mutex_unlock;
 		} else if (ret != cmd->wlen) {
@@ -43,7 +43,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
 		#define TIMEOUT 50
 		timeout = jiffies + msecs_to_jiffies(TIMEOUT);
 		while (!time_after(jiffies, timeout)) {
-			ret = i2c_master_recv(dev->client, cmd->args, cmd->rlen);
+			ret = i2c_master_recv(client, cmd->args, cmd->rlen);
 			if (ret < 0) {
 				goto err_mutex_unlock;
 			} else if (ret != cmd->rlen) {
@@ -56,7 +56,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
 				break;
 		}
 
-		dev_dbg(&dev->client->dev, "cmd execution took %d ms\n",
+		dev_dbg(&client->dev, "cmd execution took %d ms\n",
 				jiffies_to_msecs(jiffies) -
 				(jiffies_to_msecs(timeout) - TIMEOUT));
 
@@ -75,7 +75,7 @@ err_mutex_unlock:
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -151,12 +151,12 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 	}
 
-	dev_dbg(&dev->client->dev, "status=%02x args=%*ph\n",
+	dev_dbg(&client->dev, "status=%02x args=%*ph\n",
 			*status, cmd.rlen, cmd.args);
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -169,7 +169,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	struct si2168_cmd cmd;
 	u8 bandwidth, delivery_system;
 
-	dev_dbg(&dev->client->dev,
+	dev_dbg(&client->dev,
 			"delivery_system=%u modulation=%u frequency=%u bandwidth_hz=%u symbol_rate=%u inversion=%u, stream_id=%d\n",
 			c->delivery_system, c->modulation,
 			c->frequency, c->bandwidth_hz, c->symbol_rate,
@@ -352,7 +352,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -367,7 +367,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	struct si2168_cmd cmd;
 	unsigned int chip_id;
 
-	dev_dbg(&dev->client->dev, "\n");
+	dev_dbg(&client->dev, "\n");
 
 	/* initialize */
 	memcpy(cmd.args, "\xc0\x12\x00\x0c\x00\x0d\x16\x00\x00\x00\x00\x00\x00", 13);
@@ -430,7 +430,7 @@ static int si2168_init(struct dvb_frontend *fe)
 		fw_file = SI2168_B40_FIRMWARE;
 		break;
 	default:
-		dev_err(&dev->client->dev,
+		dev_err(&client->dev,
 				"unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
 				cmd.args[3], cmd.args[4]);
@@ -439,31 +439,31 @@ static int si2168_init(struct dvb_frontend *fe)
 	}
 
 	/* cold state - try to download firmware */
-	dev_info(&dev->client->dev, "found a '%s' in cold state\n",
+	dev_info(&client->dev, "found a '%s' in cold state\n",
 			si2168_ops.info.name);
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &dev->client->dev);
+	ret = request_firmware(&fw, fw_file, &client->dev);
 	if (ret) {
 		/* fallback mechanism to handle old name for Si2168 B40 fw */
 		if (chip_id == SI2168_B40) {
 			fw_file = SI2168_B40_FIRMWARE_FALLBACK;
-			ret = request_firmware(&fw, fw_file, &dev->client->dev);
+			ret = request_firmware(&fw, fw_file, &client->dev);
 		}
 
 		if (ret == 0) {
-			dev_notice(&dev->client->dev,
+			dev_notice(&client->dev,
 					"please install firmware file '%s'\n",
 					SI2168_B40_FIRMWARE);
 		} else {
-			dev_err(&dev->client->dev,
+			dev_err(&client->dev,
 					"firmware file '%s' not found\n",
 					fw_file);
 			goto error_fw_release;
 		}
 	}
 
-	dev_info(&dev->client->dev, "downloading firmware from file '%s'\n",
+	dev_info(&client->dev, "downloading firmware from file '%s'\n",
 			fw_file);
 
 	if ((fw->size % 17 == 0) && (fw->data[0] > 5)) {
@@ -475,7 +475,7 @@ static int si2168_init(struct dvb_frontend *fe)
 			cmd.rlen = 1;
 			ret = si2168_cmd_execute(client, &cmd);
 			if (ret) {
-				dev_err(&dev->client->dev,
+				dev_err(&client->dev,
 						"firmware download failed=%d\n",
 						ret);
 				goto error_fw_release;
@@ -493,7 +493,7 @@ static int si2168_init(struct dvb_frontend *fe)
 			cmd.rlen = 1;
 			ret = si2168_cmd_execute(client, &cmd);
 			if (ret) {
-				dev_err(&dev->client->dev,
+				dev_err(&client->dev,
 						"firmware download failed=%d\n",
 						ret);
 				goto error_fw_release;
@@ -519,7 +519,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	if (ret)
 		goto err;
 
-	dev_dbg(&dev->client->dev, "firmware version: %c.%c.%d\n",
+	dev_dbg(&client->dev, "firmware version: %c.%c.%d\n",
 			cmd.args[6], cmd.args[7], cmd.args[8]);
 
 	/* set ts mode */
@@ -533,7 +533,7 @@ static int si2168_init(struct dvb_frontend *fe)
 
 	dev->fw_loaded = true;
 
-	dev_info(&dev->client->dev, "found a '%s' in warm state\n",
+	dev_info(&client->dev, "found a '%s' in warm state\n",
 			si2168_ops.info.name);
 warm:
 	dev->active = true;
@@ -543,7 +543,7 @@ warm:
 error_fw_release:
 	release_firmware(fw);
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -554,7 +554,7 @@ static int si2168_sleep(struct dvb_frontend *fe)
 	int ret;
 	struct si2168_cmd cmd;
 
-	dev_dbg(&dev->client->dev, "\n");
+	dev_dbg(&client->dev, "\n");
 
 	dev->active = false;
 
@@ -567,7 +567,7 @@ static int si2168_sleep(struct dvb_frontend *fe)
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -590,7 +590,7 @@ static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct i2c_msg gate_open_msg = {
-		.addr = dev->client->addr,
+		.addr = client->addr,
 		.flags = 0,
 		.len = 3,
 		.buf = "\xc0\x0d\x01",
@@ -599,9 +599,9 @@ static int si2168_select(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 	mutex_lock(&dev->i2c_mutex);
 
 	/* open tuner I2C gate */
-	ret = __i2c_transfer(dev->client->adapter, &gate_open_msg, 1);
+	ret = __i2c_transfer(client->adapter, &gate_open_msg, 1);
 	if (ret != 1) {
-		dev_warn(&dev->client->dev, "i2c write failed=%d\n", ret);
+		dev_warn(&client->dev, "i2c write failed=%d\n", ret);
 		if (ret >= 0)
 			ret = -EREMOTEIO;
 	} else {
@@ -617,16 +617,16 @@ static int si2168_deselect(struct i2c_adapter *adap, void *mux_priv, u32 chan)
 	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct i2c_msg gate_close_msg = {
-		.addr = dev->client->addr,
+		.addr = client->addr,
 		.flags = 0,
 		.len = 3,
 		.buf = "\xc0\x0d\x00",
 	};
 
 	/* close tuner I2C gate */
-	ret = __i2c_transfer(dev->client->adapter, &gate_close_msg, 1);
+	ret = __i2c_transfer(client->adapter, &gate_close_msg, 1);
 	if (ret != 1) {
-		dev_warn(&dev->client->dev, "i2c write failed=%d\n", ret);
+		dev_warn(&client->dev, "i2c write failed=%d\n", ret);
 		if (ret >= 0)
 			ret = -EREMOTEIO;
 	} else {
@@ -691,7 +691,6 @@ static int si2168_probe(struct i2c_client *client,
 		goto err;
 	}
 
-	dev->client = client;
 	mutex_init(&dev->i2c_mutex);
 
 	/* create mux i2c adapter for tuner */
@@ -714,7 +713,7 @@ static int si2168_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, dev);
 
-	dev_info(&dev->client->dev,
+	dev_info(&client->dev,
 			"Silicon Labs Si2168 successfully attached\n");
 	return 0;
 err:
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index cb8827a..aadd136 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -29,7 +29,6 @@
 
 /* state struct */
 struct si2168_dev {
-	struct i2c_client *client;
 	struct i2c_adapter *adapter;
 	struct mutex i2c_mutex;
 	struct dvb_frontend fe;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 05/22] si2168: simplify si2168_cmd_execute() error path
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (2 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 04/22] si2168: get rid of own struct i2c_client pointer Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 06/22] si2168: rename few things Antti Palosaari
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Remove if () from firmware command error path as there should not be
any error prone conditional logic there. Use goto labels instead.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index db06bb7..2df3a27 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -66,15 +66,11 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd)
 		}
 	}
 
-	ret = 0;
+	mutex_unlock(&dev->i2c_mutex);
+	return 0;
 
 err_mutex_unlock:
 	mutex_unlock(&dev->i2c_mutex);
-	if (ret)
-		goto err;
-
-	return 0;
-err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 06/22] si2168: rename few things
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (3 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 05/22] si2168: simplify si2168_cmd_execute() error path Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 07/22] si2168: change firmware version print from debug to info Antti Palosaari
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Rename some goto labels and more. No functionality changes.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 38 ++++++++++++------------------------
 drivers/media/dvb-frontends/si2168.h |  6 ++----
 2 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 2df3a27..a9486ef 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -115,17 +115,6 @@ static int si2168_read_status(struct dvb_frontend *fe, fe_status_t *status)
 	if (ret)
 		goto err;
 
-	/*
-	 * Possible values seen, in order from strong signal to weak:
-	 * 16 0001 0110 full lock
-	 * 1e 0001 1110 partial lock
-	 * 1a 0001 1010 partial lock
-	 * 18 0001 1000 no lock
-	 *
-	 * [b3:b1] lock bits
-	 * [b4] statistics ready? Set in a few secs after lock is gained.
-	 */
-
 	switch ((cmd.args[2] >> 1) & 0x03) {
 	case 0x01:
 		*status = FE_HAS_SIGNAL | FE_HAS_CARRIER;
@@ -291,7 +280,7 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	/* set DVB-C symbol rate */
 	if (c->delivery_system == SYS_DVBC_ANNEX_A) {
 		memcpy(cmd.args, "\x14\x00\x02\x11", 4);
-		cmd.args[4] = (c->symbol_rate / 1000) & 0xff;
+		cmd.args[4] = ((c->symbol_rate / 1000) >> 0) & 0xff;
 		cmd.args[5] = ((c->symbol_rate / 1000) >> 8) & 0xff;
 		cmd.wlen = 6;
 		cmd.rlen = 4;
@@ -455,7 +444,7 @@ static int si2168_init(struct dvb_frontend *fe)
 			dev_err(&client->dev,
 					"firmware file '%s' not found\n",
 					fw_file);
-			goto error_fw_release;
+			goto err_release_firmware;
 		}
 	}
 
@@ -474,7 +463,7 @@ static int si2168_init(struct dvb_frontend *fe)
 				dev_err(&client->dev,
 						"firmware download failed=%d\n",
 						ret);
-				goto error_fw_release;
+				goto err_release_firmware;
 			}
 		}
 	} else {
@@ -492,7 +481,7 @@ static int si2168_init(struct dvb_frontend *fe)
 				dev_err(&client->dev,
 						"firmware download failed=%d\n",
 						ret);
-				goto error_fw_release;
+				goto err_release_firmware;
 			}
 		}
 	}
@@ -535,8 +524,7 @@ warm:
 	dev->active = true;
 
 	return 0;
-
-error_fw_release:
+err_release_firmware:
 	release_firmware(fw);
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
@@ -684,7 +672,7 @@ static int si2168_probe(struct i2c_client *client,
 	if (!dev) {
 		ret = -ENOMEM;
 		dev_err(&client->dev, "kzalloc() failed\n");
-		goto err;
+		goto err_kfree;
 	}
 
 	mutex_init(&dev->i2c_mutex);
@@ -694,13 +682,12 @@ static int si2168_probe(struct i2c_client *client,
 			client, 0, 0, 0, si2168_select, si2168_deselect);
 	if (dev->adapter == NULL) {
 		ret = -ENODEV;
-		goto err;
+		goto err_kfree;
 	}
 
 	/* create dvb_frontend */
 	memcpy(&dev->fe.ops, &si2168_ops, sizeof(struct dvb_frontend_ops));
 	dev->fe.demodulator_priv = client;
-
 	*config->i2c_adapter = dev->adapter;
 	*config->fe = &dev->fe;
 	dev->ts_mode = config->ts_mode;
@@ -709,10 +696,9 @@ static int si2168_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, dev);
 
-	dev_info(&client->dev,
-			"Silicon Labs Si2168 successfully attached\n");
+	dev_info(&client->dev, "Silicon Labs Si2168 successfully attached\n");
 	return 0;
-err:
+err_kfree:
 	kfree(dev);
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
@@ -734,11 +720,11 @@ static int si2168_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct i2c_device_id si2168_id[] = {
+static const struct i2c_device_id si2168_id_table[] = {
 	{"si2168", 0},
 	{}
 };
-MODULE_DEVICE_TABLE(i2c, si2168_id);
+MODULE_DEVICE_TABLE(i2c, si2168_id_table);
 
 static struct i2c_driver si2168_driver = {
 	.driver = {
@@ -747,7 +733,7 @@ static struct i2c_driver si2168_driver = {
 	},
 	.probe		= si2168_probe,
 	.remove		= si2168_remove,
-	.id_table	= si2168_id,
+	.id_table	= si2168_id_table,
 };
 
 module_i2c_driver(si2168_driver);
diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index 87bc121..70d702a 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -36,14 +36,12 @@ struct si2168_config {
 	struct i2c_adapter **i2c_adapter;
 
 	/* TS mode */
+#define SI2168_TS_PARALLEL	0x06
+#define SI2168_TS_SERIAL	0x03
 	u8 ts_mode;
 
 	/* TS clock inverted */
 	bool ts_clock_inv;
-
 };
 
-#define SI2168_TS_PARALLEL	0x06
-#define SI2168_TS_SERIAL	0x03
-
 #endif
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 07/22] si2168: change firmware version print from debug to info
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (4 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 06/22] si2168: rename few things Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 08/22] si2168: change stream id debug log formatter Antti Palosaari
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Even firmware version is not needed to know, it is still interesting
and useful to know some cases. Due to that increase its printing to
info log level.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index a9486ef..e51676c 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -504,7 +504,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	if (ret)
 		goto err;
 
-	dev_dbg(&client->dev, "firmware version: %c.%c.%d\n",
+	dev_info(&client->dev, "firmware version: %c.%c.%d\n",
 			cmd.args[6], cmd.args[7], cmd.args[8]);
 
 	/* set ts mode */
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 08/22] si2168: change stream id debug log formatter
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (5 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 07/22] si2168: change firmware version print from debug to info Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 09/22] si2168: add own goto label for kzalloc failure Antti Palosaari
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Change formatter from signed to unsigned as stream_id is 32bit
unsigned variable.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index e51676c..b4a6096 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -155,10 +155,10 @@ static int si2168_set_frontend(struct dvb_frontend *fe)
 	u8 bandwidth, delivery_system;
 
 	dev_dbg(&client->dev,
-			"delivery_system=%u modulation=%u frequency=%u bandwidth_hz=%u symbol_rate=%u inversion=%u, stream_id=%d\n",
-			c->delivery_system, c->modulation,
-			c->frequency, c->bandwidth_hz, c->symbol_rate,
-			c->inversion, c->stream_id);
+			"delivery_system=%u modulation=%u frequency=%u bandwidth_hz=%u symbol_rate=%u inversion=%u stream_id=%u\n",
+			c->delivery_system, c->modulation, c->frequency,
+			c->bandwidth_hz, c->symbol_rate, c->inversion,
+			c->stream_id);
 
 	if (!dev->active) {
 		ret = -EAGAIN;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 09/22] si2168: add own goto label for kzalloc failure
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (6 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 08/22] si2168: change stream id debug log formatter Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 10/22] si2168: enhance firmware download routine Antti Palosaari
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Use own label for kzalloc failure in which does not call kfree().
kfree() could be called with NULL, but it is still better to have
own label which skips unnecessary kfree().

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index b4a6096..1fab088 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -672,7 +672,7 @@ static int si2168_probe(struct i2c_client *client,
 	if (!dev) {
 		ret = -ENOMEM;
 		dev_err(&client->dev, "kzalloc() failed\n");
-		goto err_kfree;
+		goto err;
 	}
 
 	mutex_init(&dev->i2c_mutex);
@@ -700,6 +700,7 @@ static int si2168_probe(struct i2c_client *client,
 	return 0;
 err_kfree:
 	kfree(dev);
+err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 10/22] si2168: enhance firmware download routine
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (7 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 09/22] si2168: add own goto label for kzalloc failure Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 11/22] si2168: remove unneeded fw variable initialization Antti Palosaari
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari, Olli Salonen

All known old firmware firmware formats are downloaded using 8 byte
chunks. Reject firmware if it could not be divided to 8 byte chunks
and because of that we could simplify some calculations. Now both
supported firmware download routines are rather similar.

Cc: Olli Salonen <olli.salonen@iki.fi>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 1fab088..e8e715f 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -348,7 +348,6 @@ static int si2168_init(struct dvb_frontend *fe)
 	int ret, len, remaining;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
-	const unsigned int i2c_wr_max = 8;
 	struct si2168_cmd cmd;
 	unsigned int chip_id;
 
@@ -459,31 +458,28 @@ static int si2168_init(struct dvb_frontend *fe)
 			cmd.wlen = len;
 			cmd.rlen = 1;
 			ret = si2168_cmd_execute(client, &cmd);
-			if (ret) {
-				dev_err(&client->dev,
-						"firmware download failed=%d\n",
-						ret);
-				goto err_release_firmware;
-			}
+			if (ret)
+				break;
 		}
-	} else {
+	} else if (fw->size % 8 == 0) {
 		/* firmware is in the old format */
-		for (remaining = fw->size; remaining > 0; remaining -= i2c_wr_max) {
-			len = remaining;
-			if (len > i2c_wr_max)
-				len = i2c_wr_max;
-
+		for (remaining = fw->size; remaining > 0; remaining -= 8) {
+			len = 8;
 			memcpy(cmd.args, &fw->data[fw->size - remaining], len);
 			cmd.wlen = len;
 			cmd.rlen = 1;
 			ret = si2168_cmd_execute(client, &cmd);
-			if (ret) {
-				dev_err(&client->dev,
-						"firmware download failed=%d\n",
-						ret);
-				goto err_release_firmware;
-			}
+			if (ret)
+				break;
 		}
+	} else {
+		/* bad or unknown firmware format */
+		ret = -EINVAL;
+	}
+
+	if (ret) {
+		dev_err(&client->dev, "firmware download failed %d\n", ret);
+		goto err_release_firmware;
 	}
 
 	release_firmware(fw);
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 11/22] si2168: remove unneeded fw variable initialization
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (8 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 10/22] si2168: enhance firmware download routine Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 12/22] si2168: print chip version Antti Palosaari
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

commit 034e1ec0ce299b9e90056793dcb3187e7add6b62
si2168: One function call less in si2168_init() after error detection

That commit added goto label for error path to release firmware,
but forgets to remove variable NULL set. Remove those now.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index e8e715f..7f20fd0 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -346,7 +346,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	struct i2c_client *client = fe->demodulator_priv;
 	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret, len, remaining;
-	const struct firmware *fw = NULL;
+	const struct firmware *fw;
 	u8 *fw_file;
 	struct si2168_cmd cmd;
 	unsigned int chip_id;
@@ -483,7 +483,6 @@ static int si2168_init(struct dvb_frontend *fe)
 	}
 
 	release_firmware(fw);
-	fw = NULL;
 
 	memcpy(cmd.args, "\x01\x01", 2);
 	cmd.wlen = 2;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 12/22] si2168: print chip version
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (9 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 11/22] si2168: remove unneeded fw variable initialization Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 13/22] si2168: change firmware variable name and type Antti Palosaari
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Print chip version once using log level into when init() is called.
Remove cold/warm state printing as those are not very useful.

old printing:
si2168 6-0064: found a 'Silicon Labs Si2168' in cold state
si2168 6-0064: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
si2168 6-0064: firmware version: 4.0.11
si2168 6-0064: found a 'Silicon Labs Si2168' in warm state

new printing:
si2168 6-0064: found a 'Silicon Labs Si2168-B40'
si2168 6-0064: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
si2168 6-0064: firmware version: 4.0.11

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 7f20fd0..46a919b 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -414,17 +414,15 @@ static int si2168_init(struct dvb_frontend *fe)
 		fw_file = SI2168_B40_FIRMWARE;
 		break;
 	default:
-		dev_err(&client->dev,
-				"unknown chip version Si21%d-%c%c%c\n",
+		dev_err(&client->dev, "unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
 				cmd.args[3], cmd.args[4]);
 		ret = -EINVAL;
 		goto err;
 	}
 
-	/* cold state - try to download firmware */
-	dev_info(&client->dev, "found a '%s' in cold state\n",
-			si2168_ops.info.name);
+	dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
+			cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
 
 	/* request the firmware, this will block and timeout */
 	ret = request_firmware(&fw, fw_file, &client->dev);
@@ -512,13 +510,11 @@ static int si2168_init(struct dvb_frontend *fe)
 		goto err;
 
 	dev->fw_loaded = true;
-
-	dev_info(&client->dev, "found a '%s' in warm state\n",
-			si2168_ops.info.name);
 warm:
 	dev->active = true;
 
 	return 0;
+
 err_release_firmware:
 	release_firmware(fw);
 err:
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 13/22] si2168: change firmware variable name and type
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (10 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 12/22] si2168: print chip version Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 14/22] si2157: rename device state variable from 's' to 'dev' Antti Palosaari
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Rename firmware variable from fw_file to fw_name and change its type
from u8 to const char as request_firmware() input defines.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/si2168.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 46a919b..7f966f3 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -347,7 +347,7 @@ static int si2168_init(struct dvb_frontend *fe)
 	struct si2168_dev *dev = i2c_get_clientdata(client);
 	int ret, len, remaining;
 	const struct firmware *fw;
-	u8 *fw_file;
+	const char *fw_name;
 	struct si2168_cmd cmd;
 	unsigned int chip_id;
 
@@ -405,13 +405,13 @@ static int si2168_init(struct dvb_frontend *fe)
 
 	switch (chip_id) {
 	case SI2168_A20:
-		fw_file = SI2168_A20_FIRMWARE;
+		fw_name = SI2168_A20_FIRMWARE;
 		break;
 	case SI2168_A30:
-		fw_file = SI2168_A30_FIRMWARE;
+		fw_name = SI2168_A30_FIRMWARE;
 		break;
 	case SI2168_B40:
-		fw_file = SI2168_B40_FIRMWARE;
+		fw_name = SI2168_B40_FIRMWARE;
 		break;
 	default:
 		dev_err(&client->dev, "unknown chip version Si21%d-%c%c%c\n",
@@ -425,12 +425,12 @@ static int si2168_init(struct dvb_frontend *fe)
 			cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &client->dev);
+	ret = request_firmware(&fw, fw_name, &client->dev);
 	if (ret) {
 		/* fallback mechanism to handle old name for Si2168 B40 fw */
 		if (chip_id == SI2168_B40) {
-			fw_file = SI2168_B40_FIRMWARE_FALLBACK;
-			ret = request_firmware(&fw, fw_file, &client->dev);
+			fw_name = SI2168_B40_FIRMWARE_FALLBACK;
+			ret = request_firmware(&fw, fw_name, &client->dev);
 		}
 
 		if (ret == 0) {
@@ -440,13 +440,13 @@ static int si2168_init(struct dvb_frontend *fe)
 		} else {
 			dev_err(&client->dev,
 					"firmware file '%s' not found\n",
-					fw_file);
+					fw_name);
 			goto err_release_firmware;
 		}
 	}
 
 	dev_info(&client->dev, "downloading firmware from file '%s'\n",
-			fw_file);
+			fw_name);
 
 	if ((fw->size % 17 == 0) && (fw->data[0] > 5)) {
 		/* firmware is in the new format */
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 14/22] si2157: rename device state variable from 's' to 'dev'
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (11 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 13/22] si2168: change firmware variable name and type Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 15/22] si2157: simplify si2157_cmd_execute() error path Antti Palosaari
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

'dev' is likely most common name in kernel for structure containing
device state instance, so rename it in order to keep things
consistent.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c      | 118 ++++++++++++++++++-------------------
 drivers/media/tuners/si2157_priv.h |   2 +-
 2 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 2180de9..14d2f73 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -19,16 +19,16 @@
 static const struct dvb_tuner_ops si2157_ops;
 
 /* execute firmware command */
-static int si2157_cmd_execute(struct si2157 *s, struct si2157_cmd *cmd)
+static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 {
 	int ret;
 	unsigned long timeout;
 
-	mutex_lock(&s->i2c_mutex);
+	mutex_lock(&dev->i2c_mutex);
 
 	if (cmd->wlen) {
 		/* write cmd and args for firmware */
-		ret = i2c_master_send(s->client, cmd->args, cmd->wlen);
+		ret = i2c_master_send(dev->client, cmd->args, cmd->wlen);
 		if (ret < 0) {
 			goto err_mutex_unlock;
 		} else if (ret != cmd->wlen) {
@@ -42,7 +42,7 @@ static int si2157_cmd_execute(struct si2157 *s, struct si2157_cmd *cmd)
 		#define TIMEOUT 80
 		timeout = jiffies + msecs_to_jiffies(TIMEOUT);
 		while (!time_after(jiffies, timeout)) {
-			ret = i2c_master_recv(s->client, cmd->args, cmd->rlen);
+			ret = i2c_master_recv(dev->client, cmd->args, cmd->rlen);
 			if (ret < 0) {
 				goto err_mutex_unlock;
 			} else if (ret != cmd->rlen) {
@@ -55,7 +55,7 @@ static int si2157_cmd_execute(struct si2157 *s, struct si2157_cmd *cmd)
 				break;
 		}
 
-		dev_dbg(&s->client->dev, "cmd execution took %d ms\n",
+		dev_dbg(&dev->client->dev, "cmd execution took %d ms\n",
 				jiffies_to_msecs(jiffies) -
 				(jiffies_to_msecs(timeout) - TIMEOUT));
 
@@ -68,32 +68,32 @@ static int si2157_cmd_execute(struct si2157 *s, struct si2157_cmd *cmd)
 	ret = 0;
 
 err_mutex_unlock:
-	mutex_unlock(&s->i2c_mutex);
+	mutex_unlock(&dev->i2c_mutex);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_init(struct dvb_frontend *fe)
 {
-	struct si2157 *s = fe->tuner_priv;
+	struct si2157_dev *dev = fe->tuner_priv;
 	int ret, len, remaining;
 	struct si2157_cmd cmd;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
 	unsigned int chip_id;
 
-	dev_dbg(&s->client->dev, "\n");
+	dev_dbg(&dev->client->dev, "\n");
 
-	if (s->fw_loaded)
+	if (dev->fw_loaded)
 		goto warm;
 
 	/* power up */
-	if (s->chiptype == SI2157_CHIPTYPE_SI2146) {
+	if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
 		memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
 		cmd.wlen = 9;
 	} else {
@@ -101,7 +101,7 @@ static int si2157_init(struct dvb_frontend *fe)
 		cmd.wlen = 15;
 	}
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -109,7 +109,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x02", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 13;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -132,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	case SI2146_A10:
 		goto skip_fw_download;
 	default:
-		dev_err(&s->client->dev,
+		dev_err(&dev->client->dev,
 				"unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
 				cmd.args[3], cmd.args[4]);
@@ -141,26 +141,26 @@ static int si2157_init(struct dvb_frontend *fe)
 	}
 
 	/* cold state - try to download firmware */
-	dev_info(&s->client->dev, "found a '%s' in cold state\n",
+	dev_info(&dev->client->dev, "found a '%s' in cold state\n",
 			si2157_ops.info.name);
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &s->client->dev);
+	ret = request_firmware(&fw, fw_file, &dev->client->dev);
 	if (ret) {
-		dev_err(&s->client->dev, "firmware file '%s' not found\n",
+		dev_err(&dev->client->dev, "firmware file '%s' not found\n",
 				fw_file);
 		goto err;
 	}
 
 	/* firmware should be n chunks of 17 bytes */
 	if (fw->size % 17 != 0) {
-		dev_err(&s->client->dev, "firmware file '%s' is invalid\n",
+		dev_err(&dev->client->dev, "firmware file '%s' is invalid\n",
 				fw_file);
 		ret = -EINVAL;
 		goto fw_release_exit;
 	}
 
-	dev_info(&s->client->dev, "downloading firmware from file '%s'\n",
+	dev_info(&dev->client->dev, "downloading firmware from file '%s'\n",
 			fw_file);
 
 	for (remaining = fw->size; remaining > 0; remaining -= 17) {
@@ -168,9 +168,9 @@ static int si2157_init(struct dvb_frontend *fe)
 		memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
 		cmd.wlen = len;
 		cmd.rlen = 1;
-		ret = si2157_cmd_execute(s, &cmd);
+		ret = si2157_cmd_execute(dev, &cmd);
 		if (ret) {
-			dev_err(&s->client->dev,
+			dev_err(&dev->client->dev,
 					"firmware download failed=%d\n",
 					ret);
 			goto fw_release_exit;
@@ -185,61 +185,61 @@ skip_fw_download:
 	memcpy(cmd.args, "\x01\x01", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	s->fw_loaded = true;
+	dev->fw_loaded = true;
 
 warm:
-	s->active = true;
+	dev->active = true;
 	return 0;
 
 fw_release_exit:
 	release_firmware(fw);
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_sleep(struct dvb_frontend *fe)
 {
-	struct si2157 *s = fe->tuner_priv;
+	struct si2157_dev *dev = fe->tuner_priv;
 	int ret;
 	struct si2157_cmd cmd;
 
-	dev_dbg(&s->client->dev, "\n");
+	dev_dbg(&dev->client->dev, "\n");
 
-	s->active = false;
+	dev->active = false;
 
 	/* standby */
 	memcpy(cmd.args, "\x16\x00", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_set_params(struct dvb_frontend *fe)
 {
-	struct si2157 *s = fe->tuner_priv;
+	struct si2157_dev *dev = fe->tuner_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2157_cmd cmd;
 	u8 bandwidth, delivery_system;
 
-	dev_dbg(&s->client->dev,
+	dev_dbg(&dev->client->dev,
 			"delivery_system=%d frequency=%u bandwidth_hz=%u\n",
 			c->delivery_system, c->frequency,
 			c->bandwidth_hz);
 
-	if (!s->active) {
+	if (!dev->active) {
 		ret = -EAGAIN;
 		goto err;
 	}
@@ -274,21 +274,21 @@ static int si2157_set_params(struct dvb_frontend *fe)
 
 	memcpy(cmd.args, "\x14\x00\x03\x07\x00\x00", 6);
 	cmd.args[4] = delivery_system | bandwidth;
-	if (s->inversion)
+	if (dev->inversion)
 		cmd.args[5] = 0x01;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	if (s->chiptype == SI2157_CHIPTYPE_SI2146)
+	if (dev->chiptype == SI2157_CHIPTYPE_SI2146)
 		memcpy(cmd.args, "\x14\x00\x02\x07\x00\x01", 6);
 	else
 		memcpy(cmd.args, "\x14\x00\x02\x07\x01\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
@@ -300,13 +300,13 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	cmd.args[7] = (c->frequency >> 24) & 0xff;
 	cmd.wlen = 8;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&s->client->dev, "failed=%d\n", ret);
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -334,60 +334,58 @@ static int si2157_probe(struct i2c_client *client,
 {
 	struct si2157_config *cfg = client->dev.platform_data;
 	struct dvb_frontend *fe = cfg->fe;
-	struct si2157 *s;
+	struct si2157_dev *dev;
 	struct si2157_cmd cmd;
 	int ret;
 
-	s = kzalloc(sizeof(struct si2157), GFP_KERNEL);
-	if (!s) {
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
 		ret = -ENOMEM;
 		dev_err(&client->dev, "kzalloc() failed\n");
 		goto err;
 	}
 
-	s->client = client;
-	s->fe = cfg->fe;
-	s->inversion = cfg->inversion;
-	s->fw_loaded = false;
-	s->chiptype = (u8)id->driver_data;
-	mutex_init(&s->i2c_mutex);
+	dev->client = client;
+	dev->fe = cfg->fe;
+	dev->inversion = cfg->inversion;
+	dev->fw_loaded = false;
+	dev->chiptype = (u8)id->driver_data;
+	mutex_init(&dev->i2c_mutex);
 
 	/* check if the tuner is there */
 	cmd.wlen = 0;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(s, &cmd);
+	ret = si2157_cmd_execute(dev, &cmd);
 	if (ret)
 		goto err;
 
-	fe->tuner_priv = s;
-	memcpy(&fe->ops.tuner_ops, &si2157_ops,
-			sizeof(struct dvb_tuner_ops));
+	fe->tuner_priv = dev;
+	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
 
-	i2c_set_clientdata(client, s);
+	i2c_set_clientdata(client, dev);
 
-	dev_info(&s->client->dev,
+	dev_info(&dev->client->dev,
 			"Silicon Labs %s successfully attached\n",
-			s->chiptype == SI2157_CHIPTYPE_SI2146 ?
+			dev->chiptype == SI2157_CHIPTYPE_SI2146 ?
 			"Si2146" : "Si2147/2148/2157/2158");
 
 	return 0;
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
-	kfree(s);
-
+	kfree(dev);
 	return ret;
 }
 
 static int si2157_remove(struct i2c_client *client)
 {
-	struct si2157 *s = i2c_get_clientdata(client);
-	struct dvb_frontend *fe = s->fe;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
+	struct dvb_frontend *fe = dev->fe;
 
 	dev_dbg(&client->dev, "\n");
 
 	memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
 	fe->tuner_priv = NULL;
-	kfree(s);
+	kfree(dev);
 
 	return 0;
 }
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index d6e07cd..8f6cfc0 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -21,7 +21,7 @@
 #include "si2157.h"
 
 /* state struct */
-struct si2157 {
+struct si2157_dev {
 	struct mutex i2c_mutex;
 	struct i2c_client *client;
 	struct dvb_frontend *fe;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 15/22] si2157: simplify si2157_cmd_execute() error path
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (12 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 14/22] si2157: rename device state variable from 's' to 'dev' Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 16/22] si2157: carry pointer to client instead of state in tuner_priv Antti Palosaari
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Remove if () from firmware command error path as there should not be
any error prone conditional logic there. Use goto labels instead.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 14d2f73..f7c3867 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -65,15 +65,11 @@ static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 		}
 	}
 
-	ret = 0;
+	mutex_unlock(&dev->i2c_mutex);
+	return 0;
 
 err_mutex_unlock:
 	mutex_unlock(&dev->i2c_mutex);
-	if (ret)
-		goto err;
-
-	return 0;
-err:
 	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 16/22] si2157: carry pointer to client instead of state in tuner_priv
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (13 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 15/22] si2157: simplify si2157_cmd_execute() error path Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 17/22] si2157: change firmware download error handling Antti Palosaari
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Carry struct i2c_client pointer in tuner_priv. This driver is I2C driver,
which is represented as a struct i2c_client, so better to carry this top
level structure for each routine in order to unify things.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c      | 78 +++++++++++++++++++-------------------
 drivers/media/tuners/si2157_priv.h |  1 -
 2 files changed, 38 insertions(+), 41 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index f7c3867..88afb2a 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -19,8 +19,9 @@
 static const struct dvb_tuner_ops si2157_ops;
 
 /* execute firmware command */
-static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
+static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
 {
+	struct si2157_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	unsigned long timeout;
 
@@ -28,7 +29,7 @@ static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 
 	if (cmd->wlen) {
 		/* write cmd and args for firmware */
-		ret = i2c_master_send(dev->client, cmd->args, cmd->wlen);
+		ret = i2c_master_send(client, cmd->args, cmd->wlen);
 		if (ret < 0) {
 			goto err_mutex_unlock;
 		} else if (ret != cmd->wlen) {
@@ -42,7 +43,7 @@ static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 		#define TIMEOUT 80
 		timeout = jiffies + msecs_to_jiffies(TIMEOUT);
 		while (!time_after(jiffies, timeout)) {
-			ret = i2c_master_recv(dev->client, cmd->args, cmd->rlen);
+			ret = i2c_master_recv(client, cmd->args, cmd->rlen);
 			if (ret < 0) {
 				goto err_mutex_unlock;
 			} else if (ret != cmd->rlen) {
@@ -55,7 +56,7 @@ static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 				break;
 		}
 
-		dev_dbg(&dev->client->dev, "cmd execution took %d ms\n",
+		dev_dbg(&client->dev, "cmd execution took %d ms\n",
 				jiffies_to_msecs(jiffies) -
 				(jiffies_to_msecs(timeout) - TIMEOUT));
 
@@ -70,20 +71,21 @@ static int si2157_cmd_execute(struct si2157_dev *dev, struct si2157_cmd *cmd)
 
 err_mutex_unlock:
 	mutex_unlock(&dev->i2c_mutex);
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_init(struct dvb_frontend *fe)
 {
-	struct si2157_dev *dev = fe->tuner_priv;
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
 	int ret, len, remaining;
 	struct si2157_cmd cmd;
 	const struct firmware *fw = NULL;
 	u8 *fw_file;
 	unsigned int chip_id;
 
-	dev_dbg(&dev->client->dev, "\n");
+	dev_dbg(&client->dev, "\n");
 
 	if (dev->fw_loaded)
 		goto warm;
@@ -97,7 +99,7 @@ static int si2157_init(struct dvb_frontend *fe)
 		cmd.wlen = 15;
 	}
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -105,7 +107,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x02", 1);
 	cmd.wlen = 1;
 	cmd.rlen = 13;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -128,8 +130,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	case SI2146_A10:
 		goto skip_fw_download;
 	default:
-		dev_err(&dev->client->dev,
-				"unknown chip version Si21%d-%c%c%c\n",
+		dev_err(&client->dev, "unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
 				cmd.args[3], cmd.args[4]);
 		ret = -EINVAL;
@@ -137,26 +138,26 @@ static int si2157_init(struct dvb_frontend *fe)
 	}
 
 	/* cold state - try to download firmware */
-	dev_info(&dev->client->dev, "found a '%s' in cold state\n",
+	dev_info(&client->dev, "found a '%s' in cold state\n",
 			si2157_ops.info.name);
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &dev->client->dev);
+	ret = request_firmware(&fw, fw_file, &client->dev);
 	if (ret) {
-		dev_err(&dev->client->dev, "firmware file '%s' not found\n",
+		dev_err(&client->dev, "firmware file '%s' not found\n",
 				fw_file);
 		goto err;
 	}
 
 	/* firmware should be n chunks of 17 bytes */
 	if (fw->size % 17 != 0) {
-		dev_err(&dev->client->dev, "firmware file '%s' is invalid\n",
+		dev_err(&client->dev, "firmware file '%s' is invalid\n",
 				fw_file);
 		ret = -EINVAL;
 		goto fw_release_exit;
 	}
 
-	dev_info(&dev->client->dev, "downloading firmware from file '%s'\n",
+	dev_info(&client->dev, "downloading firmware from file '%s'\n",
 			fw_file);
 
 	for (remaining = fw->size; remaining > 0; remaining -= 17) {
@@ -164,10 +165,9 @@ static int si2157_init(struct dvb_frontend *fe)
 		memcpy(cmd.args, &fw->data[(fw->size - remaining) + 1], len);
 		cmd.wlen = len;
 		cmd.rlen = 1;
-		ret = si2157_cmd_execute(dev, &cmd);
+		ret = si2157_cmd_execute(client, &cmd);
 		if (ret) {
-			dev_err(&dev->client->dev,
-					"firmware download failed=%d\n",
+			dev_err(&client->dev, "firmware download failed %d\n",
 					ret);
 			goto fw_release_exit;
 		}
@@ -181,7 +181,7 @@ skip_fw_download:
 	memcpy(cmd.args, "\x01\x01", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -194,17 +194,18 @@ warm:
 fw_release_exit:
 	release_firmware(fw);
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_sleep(struct dvb_frontend *fe)
 {
-	struct si2157_dev *dev = fe->tuner_priv;
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
 	int ret;
 	struct si2157_cmd cmd;
 
-	dev_dbg(&dev->client->dev, "\n");
+	dev_dbg(&client->dev, "\n");
 
 	dev->active = false;
 
@@ -212,28 +213,28 @@ static int si2157_sleep(struct dvb_frontend *fe)
 	memcpy(cmd.args, "\x16\x00", 2);
 	cmd.wlen = 2;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
 static int si2157_set_params(struct dvb_frontend *fe)
 {
-	struct si2157_dev *dev = fe->tuner_priv;
+	struct i2c_client *client = fe->tuner_priv;
+	struct si2157_dev *dev = i2c_get_clientdata(client);
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
 	struct si2157_cmd cmd;
 	u8 bandwidth, delivery_system;
 
-	dev_dbg(&dev->client->dev,
+	dev_dbg(&client->dev,
 			"delivery_system=%d frequency=%u bandwidth_hz=%u\n",
-			c->delivery_system, c->frequency,
-			c->bandwidth_hz);
+			c->delivery_system, c->frequency, c->bandwidth_hz);
 
 	if (!dev->active) {
 		ret = -EAGAIN;
@@ -274,7 +275,7 @@ static int si2157_set_params(struct dvb_frontend *fe)
 		cmd.args[5] = 0x01;
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -284,7 +285,7 @@ static int si2157_set_params(struct dvb_frontend *fe)
 		memcpy(cmd.args, "\x14\x00\x02\x07\x01\x00", 6);
 	cmd.wlen = 6;
 	cmd.rlen = 4;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
@@ -296,13 +297,13 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	cmd.args[7] = (c->frequency >> 24) & 0xff;
 	cmd.wlen = 8;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
 	return 0;
 err:
-	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	dev_dbg(&client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -341,7 +342,7 @@ static int si2157_probe(struct i2c_client *client,
 		goto err;
 	}
 
-	dev->client = client;
+	i2c_set_clientdata(client, dev);
 	dev->fe = cfg->fe;
 	dev->inversion = cfg->inversion;
 	dev->fw_loaded = false;
@@ -351,17 +352,14 @@ static int si2157_probe(struct i2c_client *client,
 	/* check if the tuner is there */
 	cmd.wlen = 0;
 	cmd.rlen = 1;
-	ret = si2157_cmd_execute(dev, &cmd);
+	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
 		goto err;
 
-	fe->tuner_priv = dev;
 	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
+	fe->tuner_priv = client;
 
-	i2c_set_clientdata(client, dev);
-
-	dev_info(&dev->client->dev,
-			"Silicon Labs %s successfully attached\n",
+	dev_info(&client->dev, "Silicon Labs %s successfully attached\n",
 			dev->chiptype == SI2157_CHIPTYPE_SI2146 ?
 			"Si2146" : "Si2147/2148/2157/2158");
 
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 8f6cfc0..7aa53bc 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -23,7 +23,6 @@
 /* state struct */
 struct si2157_dev {
 	struct mutex i2c_mutex;
-	struct i2c_client *client;
 	struct dvb_frontend *fe;
 	bool active;
 	bool fw_loaded;
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 17/22] si2157: change firmware download error handling
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (14 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 16/22] si2157: carry pointer to client instead of state in tuner_priv Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 18/22] si2157: trivial ID table changes Antti Palosaari
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari, Olli Salonen

Rename firmare download error path goto label. Remove firmware NULL
set as NULL value is not needed anymore, due to recent change which
started using goto labels for firmware error handling.

Cc: Olli Salonen <olli.salonen@iki.fi>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 88afb2a..6174c8e 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -81,7 +81,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	struct si2157_dev *dev = i2c_get_clientdata(client);
 	int ret, len, remaining;
 	struct si2157_cmd cmd;
-	const struct firmware *fw = NULL;
+	const struct firmware *fw;
 	u8 *fw_file;
 	unsigned int chip_id;
 
@@ -154,7 +154,7 @@ static int si2157_init(struct dvb_frontend *fe)
 		dev_err(&client->dev, "firmware file '%s' is invalid\n",
 				fw_file);
 		ret = -EINVAL;
-		goto fw_release_exit;
+		goto err_release_firmware;
 	}
 
 	dev_info(&client->dev, "downloading firmware from file '%s'\n",
@@ -169,12 +169,11 @@ static int si2157_init(struct dvb_frontend *fe)
 		if (ret) {
 			dev_err(&client->dev, "firmware download failed %d\n",
 					ret);
-			goto fw_release_exit;
+			goto err_release_firmware;
 		}
 	}
 
 	release_firmware(fw);
-	fw = NULL;
 
 skip_fw_download:
 	/* reboot the tuner with new firmware? */
@@ -191,7 +190,7 @@ warm:
 	dev->active = true;
 	return 0;
 
-fw_release_exit:
+err_release_firmware:
 	release_firmware(fw);
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 18/22] si2157: trivial ID table changes
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (15 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 17/22] si2157: change firmware download error handling Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 19/22] si2157: add own goto label for kfree() on probe error Antti Palosaari
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

- Rename ID table.
- Remove magic numbers from ID table driver data field.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 6174c8e..211d500 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -383,12 +383,12 @@ static int si2157_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct i2c_device_id si2157_id[] = {
-	{"si2157", 0},
-	{"si2146", 1},
+static const struct i2c_device_id si2157_id_table[] = {
+	{"si2157", SI2157_CHIPTYPE_SI2157},
+	{"si2146", SI2157_CHIPTYPE_SI2146},
 	{}
 };
-MODULE_DEVICE_TABLE(i2c, si2157_id);
+MODULE_DEVICE_TABLE(i2c, si2157_id_table);
 
 static struct i2c_driver si2157_driver = {
 	.driver = {
@@ -397,7 +397,7 @@ static struct i2c_driver si2157_driver = {
 	},
 	.probe		= si2157_probe,
 	.remove		= si2157_remove,
-	.id_table	= si2157_id,
+	.id_table	= si2157_id_table,
 };
 
 module_i2c_driver(si2157_driver);
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 19/22] si2157: add own goto label for kfree() on probe error
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (16 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 18/22] si2157: trivial ID table changes Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 20/22] si2157: print firmware version Antti Palosaari
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Use own goto label for error case mem free is needed, even kfree could
be called with NULL. I think it is better to have it, even not required.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 211d500..3f9aa7a 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -353,7 +353,7 @@ static int si2157_probe(struct i2c_client *client,
 	cmd.rlen = 1;
 	ret = si2157_cmd_execute(client, &cmd);
 	if (ret)
-		goto err;
+		goto err_kfree;
 
 	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
 	fe->tuner_priv = client;
@@ -363,9 +363,11 @@ static int si2157_probe(struct i2c_client *client,
 			"Si2146" : "Si2147/2148/2157/2158");
 
 	return 0;
+
+err_kfree:
+	kfree(dev);
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
-	kfree(dev);
 	return ret;
 }
 
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 20/22] si2157: print firmware version
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (17 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 19/22] si2157: add own goto label for kfree() on probe error Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 21/22] si2157: print chip version Antti Palosaari
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Firmware version could be printed similarly than si2168 driver does.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 3f9aa7a..6ae7620 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -184,6 +184,17 @@ skip_fw_download:
 	if (ret)
 		goto err;
 
+	/* query firmware version */
+	memcpy(cmd.args, "\x11", 1);
+	cmd.wlen = 1;
+	cmd.rlen = 10;
+	ret = si2157_cmd_execute(client, &cmd);
+	if (ret)
+		goto err;
+
+	dev_info(&client->dev, "firmware version: %c.%c.%d\n",
+			cmd.args[6], cmd.args[7], cmd.args[8]);
+
 	dev->fw_loaded = true;
 
 warm:
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 21/22] si2157: print chip version
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (18 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 20/22] si2157: print firmware version Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2014-12-06 21:34 ` [PATCH 22/22] si2157: change firmware variable name and type Antti Palosaari
  2015-01-19 13:24 ` [PATCH 01/22] si2168: define symbol rate limits Hans Verkuil
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Print chip version once using log level into when init() is called.
Remove cold/warm state printing as those are not very useful.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 6ae7620..27b488b 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -128,7 +128,8 @@ static int si2157_init(struct dvb_frontend *fe)
 	case SI2157_A30:
 	case SI2147_A30:
 	case SI2146_A10:
-		goto skip_fw_download;
+		fw_file = NULL;
+		break;
 	default:
 		dev_err(&client->dev, "unknown chip version Si21%d-%c%c%c\n",
 				cmd.args[2], cmd.args[1],
@@ -137,9 +138,11 @@ static int si2157_init(struct dvb_frontend *fe)
 		goto err;
 	}
 
-	/* cold state - try to download firmware */
-	dev_info(&client->dev, "found a '%s' in cold state\n",
-			si2157_ops.info.name);
+	dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
+			cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
+
+	if (fw_file == NULL)
+		goto skip_fw_download;
 
 	/* request the firmware, this will block and timeout */
 	ret = request_firmware(&fw, fw_file, &client->dev);
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 22/22] si2157: change firmware variable name and type
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (19 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 21/22] si2157: print chip version Antti Palosaari
@ 2014-12-06 21:34 ` Antti Palosaari
  2015-01-19 13:24 ` [PATCH 01/22] si2168: define symbol rate limits Hans Verkuil
  21 siblings, 0 replies; 26+ messages in thread
From: Antti Palosaari @ 2014-12-06 21:34 UTC (permalink / raw)
  To: linux-media; +Cc: Antti Palosaari

Rename firmware variable from fw_file to fw_name and change its
type from u8 to const char as request_firmware() input is.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/tuners/si2157.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 27b488b..fcf139d 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -82,7 +82,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	int ret, len, remaining;
 	struct si2157_cmd cmd;
 	const struct firmware *fw;
-	u8 *fw_file;
+	const char *fw_name;
 	unsigned int chip_id;
 
 	dev_dbg(&client->dev, "\n");
@@ -123,12 +123,12 @@ static int si2157_init(struct dvb_frontend *fe)
 	switch (chip_id) {
 	case SI2158_A20:
 	case SI2148_A20:
-		fw_file = SI2158_A20_FIRMWARE;
+		fw_name = SI2158_A20_FIRMWARE;
 		break;
 	case SI2157_A30:
 	case SI2147_A30:
 	case SI2146_A10:
-		fw_file = NULL;
+		fw_name = NULL;
 		break;
 	default:
 		dev_err(&client->dev, "unknown chip version Si21%d-%c%c%c\n",
@@ -141,27 +141,27 @@ static int si2157_init(struct dvb_frontend *fe)
 	dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
 			cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
 
-	if (fw_file == NULL)
+	if (fw_name == NULL)
 		goto skip_fw_download;
 
 	/* request the firmware, this will block and timeout */
-	ret = request_firmware(&fw, fw_file, &client->dev);
+	ret = request_firmware(&fw, fw_name, &client->dev);
 	if (ret) {
 		dev_err(&client->dev, "firmware file '%s' not found\n",
-				fw_file);
+				fw_name);
 		goto err;
 	}
 
 	/* firmware should be n chunks of 17 bytes */
 	if (fw->size % 17 != 0) {
 		dev_err(&client->dev, "firmware file '%s' is invalid\n",
-				fw_file);
+				fw_name);
 		ret = -EINVAL;
 		goto err_release_firmware;
 	}
 
 	dev_info(&client->dev, "downloading firmware from file '%s'\n",
-			fw_file);
+			fw_name);
 
 	for (remaining = fw->size; remaining > 0; remaining -= 17) {
 		len = fw->data[fw->size - remaining];
-- 
http://palosaari.fi/


^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [PATCH 01/22] si2168: define symbol rate limits
  2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
                   ` (20 preceding siblings ...)
  2014-12-06 21:34 ` [PATCH 22/22] si2157: change firmware variable name and type Antti Palosaari
@ 2015-01-19 13:24 ` Hans Verkuil
  2015-01-19 13:27   ` Hans Verkuil
  2015-01-19 13:30   ` Antti Palosaari
  21 siblings, 2 replies; 26+ messages in thread
From: Hans Verkuil @ 2015-01-19 13:24 UTC (permalink / raw)
  To: Antti Palosaari, linux-media

On 12/06/2014 10:34 PM, Antti Palosaari wrote:
> w_scan complains about missing symbol rate limits:
> This dvb driver is *buggy*: the symbol rate limits are undefined - please report to linuxtv.org
> 
> Chip supports 1 to 7.2 MSymbol/s on DVB-C.
> 
> Signed-off-by: Antti Palosaari <crope@iki.fi>

Antti,

Are you planning to make a pull request of this patch series?

It looks good to me, so for this patch series:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

BTW, please add a cover letter whenever you post a patch series (git send-email --compose).
It makes it easier to get an overview of what the patch series is all about.

Regards,

	Hans

> ---
>  drivers/media/dvb-frontends/si2168.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> index ce9ab44..acf0fc3 100644
> --- a/drivers/media/dvb-frontends/si2168.c
> +++ b/drivers/media/dvb-frontends/si2168.c
> @@ -635,6 +635,8 @@ static const struct dvb_frontend_ops si2168_ops = {
>  	.delsys = {SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A},
>  	.info = {
>  		.name = "Silicon Labs Si2168",
> +		.symbol_rate_min = 1000000,
> +		.symbol_rate_max = 7200000,
>  		.caps =	FE_CAN_FEC_1_2 |
>  			FE_CAN_FEC_2_3 |
>  			FE_CAN_FEC_3_4 |
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 01/22] si2168: define symbol rate limits
  2015-01-19 13:24 ` [PATCH 01/22] si2168: define symbol rate limits Hans Verkuil
@ 2015-01-19 13:27   ` Hans Verkuil
  2015-01-19 13:30   ` Antti Palosaari
  1 sibling, 0 replies; 26+ messages in thread
From: Hans Verkuil @ 2015-01-19 13:27 UTC (permalink / raw)
  To: Antti Palosaari, linux-media

On 01/19/2015 02:24 PM, Hans Verkuil wrote:
> On 12/06/2014 10:34 PM, Antti Palosaari wrote:
>> w_scan complains about missing symbol rate limits:
>> This dvb driver is *buggy*: the symbol rate limits are undefined - please report to linuxtv.org
>>
>> Chip supports 1 to 7.2 MSymbol/s on DVB-C.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
> 
> Antti,
> 
> Are you planning to make a pull request of this patch series?

Never mind, I found the pull request :-)

I'll mark these patches as accepted in patchwork, to keep patchwork clean.

Regards,

	Hans

> 
> It looks good to me, so for this patch series:
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> BTW, please add a cover letter whenever you post a patch series (git send-email --compose).
> It makes it easier to get an overview of what the patch series is all about.
> 
> Regards,
> 
> 	Hans
> 
>> ---
>>  drivers/media/dvb-frontends/si2168.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
>> index ce9ab44..acf0fc3 100644
>> --- a/drivers/media/dvb-frontends/si2168.c
>> +++ b/drivers/media/dvb-frontends/si2168.c
>> @@ -635,6 +635,8 @@ static const struct dvb_frontend_ops si2168_ops = {
>>  	.delsys = {SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A},
>>  	.info = {
>>  		.name = "Silicon Labs Si2168",
>> +		.symbol_rate_min = 1000000,
>> +		.symbol_rate_max = 7200000,
>>  		.caps =	FE_CAN_FEC_1_2 |
>>  			FE_CAN_FEC_2_3 |
>>  			FE_CAN_FEC_3_4 |
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 01/22] si2168: define symbol rate limits
  2015-01-19 13:24 ` [PATCH 01/22] si2168: define symbol rate limits Hans Verkuil
  2015-01-19 13:27   ` Hans Verkuil
@ 2015-01-19 13:30   ` Antti Palosaari
  2015-01-19 13:37     ` Hans Verkuil
  1 sibling, 1 reply; 26+ messages in thread
From: Antti Palosaari @ 2015-01-19 13:30 UTC (permalink / raw)
  To: Hans Verkuil, linux-media

Moikka!

On 01/19/2015 03:24 PM, Hans Verkuil wrote:
> On 12/06/2014 10:34 PM, Antti Palosaari wrote:
>> w_scan complains about missing symbol rate limits:
>> This dvb driver is *buggy*: the symbol rate limits are undefined - please report to linuxtv.org
>>
>> Chip supports 1 to 7.2 MSymbol/s on DVB-C.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>
> Antti,
>
> Are you planning to make a pull request of this patch series?
>
> It looks good to me, so for this patch series:
>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> BTW, please add a cover letter whenever you post a patch series (git send-email --compose).
> It makes it easier to get an overview of what the patch series is all about.

PULL request is here:
https://patchwork.linuxtv.org/patch/27416/

I could send new one if needed, there is missing branch name (new Git 
version has started blaming it).

Are you applying these pull request now? I was expecting Mauro...

regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 01/22] si2168: define symbol rate limits
  2015-01-19 13:30   ` Antti Palosaari
@ 2015-01-19 13:37     ` Hans Verkuil
  0 siblings, 0 replies; 26+ messages in thread
From: Hans Verkuil @ 2015-01-19 13:37 UTC (permalink / raw)
  To: Antti Palosaari, linux-media

On 01/19/2015 02:30 PM, Antti Palosaari wrote:
> Moikka!
> 
> On 01/19/2015 03:24 PM, Hans Verkuil wrote:
>> On 12/06/2014 10:34 PM, Antti Palosaari wrote:
>>> w_scan complains about missing symbol rate limits:
>>> This dvb driver is *buggy*: the symbol rate limits are undefined - please report to linuxtv.org
>>>
>>> Chip supports 1 to 7.2 MSymbol/s on DVB-C.
>>>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>
>> Antti,
>>
>> Are you planning to make a pull request of this patch series?
>>
>> It looks good to me, so for this patch series:
>>
>> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> BTW, please add a cover letter whenever you post a patch series (git send-email --compose).
>> It makes it easier to get an overview of what the patch series is all about.
> 
> PULL request is here:
> https://patchwork.linuxtv.org/patch/27416/
> 
> I could send new one if needed, there is missing branch name (new Git 
> version has started blaming it).

It's probably wise to do that (and rebase at the same time).

> 
> Are you applying these pull request now? I was expecting Mauro...

I'm cleaning up patchwork, and your (very long) patch series were making it
hard to work with patchwork.

Regards,

	Hans

> 
> regards
> Antti
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-01-19 13:37 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-06 21:34 [PATCH 01/22] si2168: define symbol rate limits Antti Palosaari
2014-12-06 21:34 ` [PATCH 02/22] si2168: rename device state variable from 's' to 'dev' Antti Palosaari
2014-12-06 21:34 ` [PATCH 03/22] si2168: carry pointer to client instead of state Antti Palosaari
2014-12-06 21:34 ` [PATCH 04/22] si2168: get rid of own struct i2c_client pointer Antti Palosaari
2014-12-06 21:34 ` [PATCH 05/22] si2168: simplify si2168_cmd_execute() error path Antti Palosaari
2014-12-06 21:34 ` [PATCH 06/22] si2168: rename few things Antti Palosaari
2014-12-06 21:34 ` [PATCH 07/22] si2168: change firmware version print from debug to info Antti Palosaari
2014-12-06 21:34 ` [PATCH 08/22] si2168: change stream id debug log formatter Antti Palosaari
2014-12-06 21:34 ` [PATCH 09/22] si2168: add own goto label for kzalloc failure Antti Palosaari
2014-12-06 21:34 ` [PATCH 10/22] si2168: enhance firmware download routine Antti Palosaari
2014-12-06 21:34 ` [PATCH 11/22] si2168: remove unneeded fw variable initialization Antti Palosaari
2014-12-06 21:34 ` [PATCH 12/22] si2168: print chip version Antti Palosaari
2014-12-06 21:34 ` [PATCH 13/22] si2168: change firmware variable name and type Antti Palosaari
2014-12-06 21:34 ` [PATCH 14/22] si2157: rename device state variable from 's' to 'dev' Antti Palosaari
2014-12-06 21:34 ` [PATCH 15/22] si2157: simplify si2157_cmd_execute() error path Antti Palosaari
2014-12-06 21:34 ` [PATCH 16/22] si2157: carry pointer to client instead of state in tuner_priv Antti Palosaari
2014-12-06 21:34 ` [PATCH 17/22] si2157: change firmware download error handling Antti Palosaari
2014-12-06 21:34 ` [PATCH 18/22] si2157: trivial ID table changes Antti Palosaari
2014-12-06 21:34 ` [PATCH 19/22] si2157: add own goto label for kfree() on probe error Antti Palosaari
2014-12-06 21:34 ` [PATCH 20/22] si2157: print firmware version Antti Palosaari
2014-12-06 21:34 ` [PATCH 21/22] si2157: print chip version Antti Palosaari
2014-12-06 21:34 ` [PATCH 22/22] si2157: change firmware variable name and type Antti Palosaari
2015-01-19 13:24 ` [PATCH 01/22] si2168: define symbol rate limits Hans Verkuil
2015-01-19 13:27   ` Hans Verkuil
2015-01-19 13:30   ` Antti Palosaari
2015-01-19 13:37     ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox