All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add fc0011 tuner driver
@ 2012-04-02 16:14 Michael Büsch
  2012-04-02 16:56 ` Antti Palosaari
  2012-05-07 19:02 ` Antti Palosaari
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Büsch @ 2012-04-02 16:14 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 17550 bytes --]

This adds support for the Fitipower fc0011 DVB-t tuner.

Signed-off-by: Michael Buesch <m@bues.ch>

---


Index: linux/drivers/media/common/tuners/Kconfig
===================================================================
--- linux.orig/drivers/media/common/tuners/Kconfig	2012-04-02 15:55:51.155296579 +0200
+++ linux/drivers/media/common/tuners/Kconfig	2012-04-02 16:00:14.464066789 +0200
@@ -204,6 +204,13 @@
 	help
 	  NXP TDA18218 silicon tuner driver.
 
+config MEDIA_TUNER_FC0011
+	tristate "Fitipower FC0011 silicon tuner"
+	depends on VIDEO_MEDIA && I2C
+	default m if MEDIA_TUNER_CUSTOMISE
+	help
+	  Fitipower FC0011 silicon tuner driver.
+
 config MEDIA_TUNER_TDA18212
 	tristate "NXP TDA18212 silicon tuner"
 	depends on VIDEO_MEDIA && I2C
Index: linux/drivers/media/common/tuners/Makefile
===================================================================
--- linux.orig/drivers/media/common/tuners/Makefile	2012-04-02 15:55:51.155296579 +0200
+++ linux/drivers/media/common/tuners/Makefile	2012-04-02 16:00:14.464066789 +0200
@@ -29,6 +29,7 @@
 obj-$(CONFIG_MEDIA_TUNER_TDA18218) += tda18218.o
 obj-$(CONFIG_MEDIA_TUNER_TDA18212) += tda18212.o
 obj-$(CONFIG_MEDIA_TUNER_TUA9001) += tua9001.o
+obj-$(CONFIG_MEDIA_TUNER_FC0011) += fc0011.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb/frontends
Index: linux/drivers/media/common/tuners/fc0011.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/media/common/tuners/fc0011.c	2012-04-02 18:01:25.978477833 +0200
@@ -0,0 +1,524 @@
+/*
+ * Fitipower FC0011 tuner driver
+ *
+ * Copyright (C) 2012 Michael Buesch <m@bues.ch>
+ *
+ * Derived from FC0012 tuner driver:
+ * Copyright (C) 2012 Hans-Frieder Vogt <hfvogt@gmx.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include "fc0011.h"
+
+
+/* Tuner registers */
+enum {
+	FC11_REG_0,
+	FC11_REG_FA,		/* FA */
+	FC11_REG_FP,		/* FP */
+	FC11_REG_XINHI,		/* XIN high 8 bit */
+	FC11_REG_XINLO,		/* XIN low 8 bit */
+	FC11_REG_VCO,		/* VCO */
+	FC11_REG_VCOSEL,	/* VCO select */
+	FC11_REG_7,		/* Unknown tuner reg 7 */
+	FC11_REG_8,		/* Unknown tuner reg 8 */
+	FC11_REG_9,
+	FC11_REG_10,		/* Unknown tuner reg 10 */
+	FC11_REG_11,		/* Unknown tuner reg 11 */
+	FC11_REG_12,
+	FC11_REG_RCCAL,		/* RC calibrate */
+	FC11_REG_VCOCAL,	/* VCO calibrate */
+	FC11_REG_15,
+	FC11_REG_16,		/* Unknown tuner reg 16 */
+	FC11_REG_17,
+
+	FC11_NR_REGS,		/* Number of registers */
+};
+
+enum FC11_REG_VCOSEL_bits {
+	FC11_VCOSEL_2		= 0x08, /* VCO select 2 */
+	FC11_VCOSEL_1		= 0x10, /* VCO select 1 */
+	FC11_VCOSEL_CLKOUT	= 0x20, /* Fix clock out */
+	FC11_VCOSEL_BW7M	= 0x40, /* 7MHz bw */
+	FC11_VCOSEL_BW6M	= 0x80, /* 6MHz bw */
+};
+
+enum FC11_REG_RCCAL_bits {
+	FC11_RCCAL_FORCE	= 0x10, /* force */
+};
+
+enum FC11_REG_VCOCAL_bits {
+	FC11_VCOCAL_RUN		= 0,	/* VCO calibration run */
+	FC11_VCOCAL_VALUEMASK	= 0x3F,	/* VCO calibration value mask */
+	FC11_VCOCAL_OK		= 0x40,	/* VCO calibration Ok */
+	FC11_VCOCAL_RESET	= 0x80, /* VCO calibration reset */
+};
+
+
+struct fc0011_priv {
+	struct i2c_adapter *i2c;
+	u8 addr;
+
+	u32 frequency;
+	u32 bandwidth;
+};
+
+
+static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
+{
+	u8 buf[2] = { reg, val };
+	struct i2c_msg msg = { .addr = priv->addr,
+		.flags = 0, .buf = buf, .len = 2 };
+
+	if (i2c_transfer(priv->i2c, &msg, 1) != 1) {
+		dev_err(&priv->i2c->dev,
+			"I2C write reg failed, reg: %02x, val: %02x\n",
+			reg, val);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int fc0011_readreg(struct fc0011_priv *priv, u8 reg, u8 *val)
+{
+	u8 dummy;
+	struct i2c_msg msg[2] = {
+		{ .addr = priv->addr,
+		  .flags = 0, .buf = &reg, .len = 1 },
+		{ .addr = priv->addr,
+		  .flags = I2C_M_RD, .buf = val ? : &dummy, .len = 1 },
+	};
+
+	if (i2c_transfer(priv->i2c, msg, 2) != 2) {
+		dev_err(&priv->i2c->dev,
+			"I2C read failed, reg: %02x\n", reg);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int fc0011_release(struct dvb_frontend *fe)
+{
+	kfree(fe->tuner_priv);
+	fe->tuner_priv = NULL;
+
+	return 0;
+}
+
+static int fc0011_init(struct dvb_frontend *fe)
+{
+	struct fc0011_priv *priv = fe->tuner_priv;
+	int err;
+
+	if (WARN_ON(!fe->callback))
+		return -EINVAL;
+
+	err = fe->callback(priv->i2c, DVB_FRONTEND_COMPONENT_TUNER,
+			   FC0011_FE_CALLBACK_POWER, priv->addr);
+	if (err) {
+		dev_err(&priv->i2c->dev, "Power-on callback failed\n");
+		return err;
+	}
+	err = fe->callback(priv->i2c, DVB_FRONTEND_COMPONENT_TUNER,
+			   FC0011_FE_CALLBACK_RESET, priv->addr);
+	if (err) {
+		dev_err(&priv->i2c->dev, "Reset callback failed\n");
+		return err;
+	}
+
+	return 0;
+}
+
+/* Initiate VCO calibration */
+static int fc0011_vcocal_trigger(struct fc0011_priv *priv)
+{
+	int err;
+
+	err = fc0011_writereg(priv, FC11_REG_VCOCAL, FC11_VCOCAL_RESET);
+	if (err)
+		return err;
+	err = fc0011_writereg(priv, FC11_REG_VCOCAL, FC11_VCOCAL_RUN);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+/* Read VCO calibration value */
+static int fc0011_vcocal_read(struct fc0011_priv *priv, u8 *value)
+{
+	int err;
+
+	err = fc0011_writereg(priv, FC11_REG_VCOCAL, FC11_VCOCAL_RUN);
+	if (err)
+		return err;
+	msleep(10);
+	err = fc0011_readreg(priv, FC11_REG_VCOCAL, value);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int fc0011_set_params(struct dvb_frontend *fe)
+{
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+	struct fc0011_priv *priv = fe->tuner_priv;
+	int err;
+	unsigned int i, vco_retries;
+	u32 freq = p->frequency / 1000;
+	u32 bandwidth = p->bandwidth_hz / 1000;
+	u32 fvco, xin, xdiv, xdivr;
+	u16 frac;
+	u8 fa, fp, vco_sel, vco_cal;
+	u8 regs[FC11_NR_REGS] = { };
+
+	regs[FC11_REG_7] = 0x0F;
+	regs[FC11_REG_8] = 0x3E;
+	regs[FC11_REG_10] = 0xB8;
+	regs[FC11_REG_11] = 0x80;
+	regs[FC11_REG_RCCAL] = 0x04;
+	err = fc0011_writereg(priv, FC11_REG_7, regs[FC11_REG_7]);
+	err |= fc0011_writereg(priv, FC11_REG_8, regs[FC11_REG_8]);
+	err |= fc0011_writereg(priv, FC11_REG_10, regs[FC11_REG_10]);
+	err |= fc0011_writereg(priv, FC11_REG_11, regs[FC11_REG_11]);
+	err |= fc0011_writereg(priv, FC11_REG_RCCAL, regs[FC11_REG_RCCAL]);
+	if (err)
+		return -EIO;
+
+	/* Set VCO freq and VCO div */
+	if (freq < 54000) {
+		fvco = freq * 64;
+		regs[FC11_REG_VCO] = 0x82;
+	} else if (freq < 108000) {
+		fvco = freq * 32;
+		regs[FC11_REG_VCO] = 0x42;
+	} else if (freq < 216000) {
+		fvco = freq * 16;
+		regs[FC11_REG_VCO] = 0x22;
+	} else if (freq < 432000) {
+		fvco = freq * 8;
+		regs[FC11_REG_VCO] = 0x12;
+	} else {
+		fvco = freq * 4;
+		regs[FC11_REG_VCO] = 0x0A;
+	}
+
+	/* Calc XIN. The PLL reference frequency is 18 MHz. */
+	xdiv = fvco / 18000;
+	frac = fvco - xdiv * 18000;
+	frac = (frac << 15) / 18000;
+	if (frac >= 16384)
+		frac += 32786;
+	if (!frac)
+		xin = 0;
+	else if (frac < 511)
+		xin = 512;
+	else if (frac < 65026)
+		xin = frac;
+	else
+		xin = 65024;
+	regs[FC11_REG_XINHI] = xin >> 8;
+	regs[FC11_REG_XINLO] = xin;
+
+	/* Calc FP and FA */
+	xdivr = xdiv;
+	if (fvco - xdiv * 18000 >= 9000)
+		xdivr += 1; /* round */
+	fp = xdivr / 8;
+	fa = xdivr - fp * 8;
+	if (fa < 2) {
+		fp -= 1;
+		fa += 8;
+	}
+	if (fp > 0x1F) {
+		fp &= 0x1F;
+		fa &= 0xF;
+	}
+	if (fa >= fp) {
+		dev_warn(&priv->i2c->dev,
+			 "fa %02X >= fp %02X, but trying to continue\n",
+			 (unsigned int)(u8)fa, (unsigned int)(u8)fp);
+	}
+	regs[FC11_REG_FA] = fa;
+	regs[FC11_REG_FP] = fp;
+
+	/* Select bandwidth */
+	switch (bandwidth) {
+	case 8000:
+		break;
+	case 7000:
+		regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_BW7M;
+		break;
+	default:
+		dev_warn(&priv->i2c->dev, "Unsupported bandwidth %u kHz. "
+			 "Using 6000 kHz.\n",
+			 bandwidth);
+		bandwidth = 6000;
+		/* fallthrough */
+	case 6000:
+		regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_BW6M;
+		break;
+	}
+
+	/* Pre VCO select */
+	if (fvco < 2320000) {
+		vco_sel = 0;
+		regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+	} else if (fvco < 3080000) {
+		vco_sel = 1;
+		regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+		regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_1;
+	} else {
+		vco_sel = 2;
+		regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+		regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_2;
+	}
+
+	/* Fix for low freqs */
+	if (freq < 45000) {
+		regs[FC11_REG_FA] = 0x6;
+		regs[FC11_REG_FP] = 0x11;
+	}
+
+	/* Clock out fix */
+	regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_CLKOUT;
+
+	/* Write the cached registers */
+	for (i = FC11_REG_FA; i <= FC11_REG_VCOSEL; i++) {
+		err = fc0011_writereg(priv, i, regs[i]);
+		if (err)
+			return err;
+	}
+
+	/* VCO calibration */
+	err = fc0011_vcocal_trigger(priv);
+	if (err)
+		return err;
+	err = fc0011_vcocal_read(priv, &vco_cal);
+	if (err)
+		return err;
+	vco_retries = 0;
+	while (!(vco_cal & FC11_VCOCAL_OK) && vco_retries < 6) {
+		/* Reset the tuner and try again */
+		err = fe->callback(priv->i2c, DVB_FRONTEND_COMPONENT_TUNER,
+				   FC0011_FE_CALLBACK_RESET, priv->addr);
+		if (err) {
+			dev_err(&priv->i2c->dev, "Failed to reset tuner\n");
+			return err;
+		}
+		/* Reinit tuner config */
+		err = 0;
+		for (i = FC11_REG_FA; i <= FC11_REG_VCOSEL; i++)
+			err |= fc0011_writereg(priv, i, regs[i]);
+		err |= fc0011_writereg(priv, FC11_REG_7, regs[FC11_REG_7]);
+		err |= fc0011_writereg(priv, FC11_REG_8, regs[FC11_REG_8]);
+		err |= fc0011_writereg(priv, FC11_REG_10, regs[FC11_REG_10]);
+		err |= fc0011_writereg(priv, FC11_REG_11, regs[FC11_REG_11]);
+		err |= fc0011_writereg(priv, FC11_REG_RCCAL, regs[FC11_REG_RCCAL]);
+		if (err)
+			return -EIO;
+		/* VCO calibration */
+		err = fc0011_vcocal_trigger(priv);
+		if (err)
+			return err;
+		err = fc0011_vcocal_read(priv, &vco_cal);
+		if (err)
+			return err;
+		vco_retries++;
+	}
+	if (!(vco_cal & FC11_VCOCAL_OK)) {
+		dev_err(&priv->i2c->dev,
+			"Failed to read VCO calibration value (got %02X)\n",
+			(unsigned int)vco_cal);
+		return -EIO;
+	}
+	vco_cal &= FC11_VCOCAL_VALUEMASK;
+
+	switch (vco_sel) {
+	case 0:
+		if (vco_cal < 8) {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_1;
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+			err = fc0011_vcocal_trigger(priv);
+			if (err)
+				return err;
+		} else {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+		}
+		break;
+	case 1:
+		if (vco_cal < 5) {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_2;
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+			err = fc0011_vcocal_trigger(priv);
+			if (err)
+				return err;
+		} else if (vco_cal <= 48) {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_1;
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+		} else {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+			err = fc0011_vcocal_trigger(priv);
+			if (err)
+				return err;
+		}
+		break;
+	case 2:
+		if (vco_cal > 53) {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_1;
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+			err = fc0011_vcocal_trigger(priv);
+			if (err)
+				return err;
+		} else {
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);
+			regs[FC11_REG_VCOSEL] |= FC11_VCOSEL_2;
+			err = fc0011_writereg(priv, FC11_REG_VCOSEL,
+					      regs[FC11_REG_VCOSEL]);
+			if (err)
+				return err;
+		}
+		break;
+	}
+	err = fc0011_vcocal_read(priv, NULL);
+	if (err)
+		return err;
+	msleep(10);
+
+	err = fc0011_readreg(priv, FC11_REG_RCCAL, &regs[FC11_REG_RCCAL]);
+	if (err)
+		return err;
+	regs[FC11_REG_RCCAL] |= FC11_RCCAL_FORCE;
+	err = fc0011_writereg(priv, FC11_REG_RCCAL, regs[FC11_REG_RCCAL]);
+	if (err)
+		return err;
+	err = fc0011_writereg(priv, FC11_REG_16, 0xB);
+	if (err)
+		return err;
+
+	dev_dbg(&priv->i2c->dev, "Tuned to "
+		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "
+		"vcocal=%02X(%u) bw=%u\n",
+		(unsigned int)regs[FC11_REG_FA],
+		(unsigned int)regs[FC11_REG_FP],
+		(unsigned int)regs[FC11_REG_XINHI],
+		(unsigned int)regs[FC11_REG_XINLO],
+		(unsigned int)regs[FC11_REG_VCO],
+		(unsigned int)regs[FC11_REG_VCOSEL],
+		(unsigned int)vco_cal, vco_retries,
+		(unsigned int)bandwidth);
+
+	priv->frequency = p->frequency;
+	priv->bandwidth = p->bandwidth_hz;
+
+	return 0;
+}
+
+static int fc0011_get_frequency(struct dvb_frontend *fe, u32 *frequency)
+{
+	struct fc0011_priv *priv = fe->tuner_priv;
+
+	*frequency = priv->frequency;
+
+	return 0;
+}
+
+static int fc0011_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
+{
+	*frequency = 0;
+
+	return 0;
+}
+
+static int fc0011_get_bandwidth(struct dvb_frontend *fe, u32 *bandwidth)
+{
+	struct fc0011_priv *priv = fe->tuner_priv;
+
+	*bandwidth = priv->bandwidth;
+
+	return 0;
+}
+
+static const struct dvb_tuner_ops fc0011_tuner_ops = {
+	.info = {
+		.name		= "Fitipower FC0011",
+
+		.frequency_min	= 45000000,
+		.frequency_max	= 1000000000,
+	},
+
+	.release		= fc0011_release,
+	.init			= fc0011_init,
+
+	.set_params		= fc0011_set_params,
+
+	.get_frequency		= fc0011_get_frequency,
+	.get_if_frequency	= fc0011_get_if_frequency,
+	.get_bandwidth		= fc0011_get_bandwidth,
+};
+
+struct dvb_frontend *fc0011_attach(struct dvb_frontend *fe,
+				   struct i2c_adapter *i2c,
+				   const struct fc0011_config *config)
+{
+	struct fc0011_priv *priv;
+
+	priv = kzalloc(sizeof(struct fc0011_priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+
+	priv->i2c = i2c;
+	priv->addr = config->i2c_address;
+
+	fe->tuner_priv = priv;
+	fe->ops.tuner_ops = fc0011_tuner_ops;
+
+	dev_info(&priv->i2c->dev, "Fitipower FC0011 tuner attached\n");
+
+	return fe;
+}
+EXPORT_SYMBOL(fc0011_attach);
+
+MODULE_DESCRIPTION("Fitipower FC0011 silicon tuner driver");
+MODULE_AUTHOR("Michael Buesch <m@bues.ch>");
+MODULE_LICENSE("GPL");
Index: linux/drivers/media/common/tuners/fc0011.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/media/common/tuners/fc0011.h	2012-04-02 16:00:14.480067083 +0200
@@ -0,0 +1,41 @@
+#ifndef LINUX_FC0011_H_
+#define LINUX_FC0011_H_
+
+#include "dvb_frontend.h"
+
+
+/** struct fc0011_config - fc0011 hardware config
+ *
+ * @i2c_address: I2C bus address.
+ */
+struct fc0011_config {
+	u8 i2c_address;
+};
+
+/** enum fc0011_fe_callback_commands - Frontend callbacks
+ *
+ * @FC0011_FE_CALLBACK_POWER: Power on tuner hardware.
+ * @FC0011_FE_CALLBACK_RESET: Request a tuner reset.
+ */
+enum fc0011_fe_callback_commands {
+	FC0011_FE_CALLBACK_POWER,
+	FC0011_FE_CALLBACK_RESET,
+};
+
+#if defined(CONFIG_MEDIA_TUNER_FC0011) ||\
+    defined(CONFIG_MEDIA_TUNER_FC0011_MODULE)
+struct dvb_frontend *fc0011_attach(struct dvb_frontend *fe,
+				   struct i2c_adapter *i2c,
+				   const struct fc0011_config *config);
+#else
+static inline
+struct dvb_frontend *fc0011_attach(struct dvb_frontend *fe,
+				   struct i2c_adapter *i2c,
+				   const struct fc0011_config *config)
+{
+	dev_err(&i2c->dev, "fc0011 driver disabled in Kconfig\n");
+	return NULL;
+}
+#endif
+
+#endif /* LINUX_FC0011_H_ */
Index: linux/MAINTAINERS
===================================================================
--- linux.orig/MAINTAINERS	2012-04-02 15:55:51.155296579 +0200
+++ linux/MAINTAINERS	2012-04-02 16:00:14.500067443 +0200
@@ -2697,6 +2697,13 @@
 F:	Documentation/hwmon/f71805f
 F:	drivers/hwmon/f71805f.c
 
+FC0011 TUNER DRIVER
+M:	Michael Buesch <m@bues.ch>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	drivers/media/common/tuners/fc0011.h
+F:	drivers/media/common/tuners/fc0011.c
+
 FANOTIFY
 M:	Eric Paris <eparis@redhat.com>
 S:	Maintained


-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 16:14 [PATCH] Add fc0011 tuner driver Michael Büsch
@ 2012-04-02 16:56 ` Antti Palosaari
  2012-04-02 17:20   ` Michael Büsch
  2012-05-07 19:02 ` Antti Palosaari
  1 sibling, 1 reply; 10+ messages in thread
From: Antti Palosaari @ 2012-04-02 16:56 UTC (permalink / raw)
  To: Michael Büsch; +Cc: linux-media

On 02.04.2012 19:14, Michael Büsch wrote:
> This adds support for the Fitipower fc0011 DVB-t tuner.
>
> Signed-off-by: Michael Buesch<m@bues.ch>

Applied, thanks!
http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/af9035_experimental

I looked it through quickly, no big issues. Anyhow, when I ran 
checkpatch.pl and it complains rather much. All Kernel developers must 
use checkpatch.pl before sent patches and fix findings if possible, 
errors must be fixed and warnings too if there is no good reason to 
leave as it is.

Send new patch that fixes those issues or sent same patches again but fixed.

And one note about tuner driver, my AF9035 + FC0011 device founds only 1 
mux of 4. Looks like some performance issues still to resolve.

regards
Antti
-- 
http://palosaari.fi/

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 16:56 ` Antti Palosaari
@ 2012-04-02 17:20   ` Michael Büsch
  2012-04-02 17:40     ` Antti Palosaari
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Büsch @ 2012-04-02 17:20 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Mon, 02 Apr 2012 19:56:50 +0300
Antti Palosaari <crope@iki.fi> wrote:

> On 02.04.2012 19:14, Michael Büsch wrote:
> > This adds support for the Fitipower fc0011 DVB-t tuner.
> >
> > Signed-off-by: Michael Buesch<m@bues.ch>
> 
> Applied, thanks!
> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/af9035_experimental
> 
> I looked it through quickly, no big issues. Anyhow, when I ran 
> checkpatch.pl and it complains rather much. All Kernel developers must 
> use checkpatch.pl before sent patches and fix findings if possible, 
> errors must be fixed and warnings too if there is no good reason to 
> leave as it is.

Well, I _did_ run it on the patch.
There is no error. Only (IMO bogus) warnings. Most of them
are about the 80 char limit. Which isn't really a hard limit. And those lines
only exceed the 80 char limit by a few chars (one, two or such). Splitting
those line serves no readability purpose. In fact, it just worsens it.

> And one note about tuner driver, my AF9035 + FC0011 device founds only 1 
> mux of 4. Looks like some performance issues still to resolve.

I have no idea what this means.
So I have no remote idea of what could possibly be wrong here.
Is this a bug on af903x or the tuner driver?

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 17:20   ` Michael Büsch
@ 2012-04-02 17:40     ` Antti Palosaari
  2012-04-02 17:51       ` Michael Büsch
  0 siblings, 1 reply; 10+ messages in thread
From: Antti Palosaari @ 2012-04-02 17:40 UTC (permalink / raw)
  To: Michael Büsch, Mauro Carvalho Chehab; +Cc: linux-media

On 02.04.2012 20:20, Michael Büsch wrote:
> On Mon, 02 Apr 2012 19:56:50 +0300
> Antti Palosaari<crope@iki.fi>  wrote:
>
>> On 02.04.2012 19:14, Michael Büsch wrote:
>>> This adds support for the Fitipower fc0011 DVB-t tuner.
>>>
>>> Signed-off-by: Michael Buesch<m@bues.ch>
>>
>> Applied, thanks!
>> http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/af9035_experimental
>>
>> I looked it through quickly, no big issues. Anyhow, when I ran
>> checkpatch.pl and it complains rather much. All Kernel developers must
>> use checkpatch.pl before sent patches and fix findings if possible,
>> errors must be fixed and warnings too if there is no good reason to
>> leave as it is.
>
> Well, I _did_ run it on the patch.
> There is no error. Only (IMO bogus) warnings. Most of them
> are about the 80 char limit. Which isn't really a hard limit. And those lines
> only exceed the 80 char limit by a few chars (one, two or such). Splitting
> those line serves no readability purpose. In fact, it just worsens it.


git show --pretty=email b650f4d701859ccff76870208c9fd8093cc64209 | 
./scripts/checkpatch.pl -
WARNING: please write a paragraph that describes the config symbol fully
#37: FILE: drivers/media/common/tuners/Kconfig:207:
+config MEDIA_TUNER_FC0011

WARNING: msleep < 20ms can sleep for up to 20ms; see 
Documentation/timers/timers-howto.txt
+	msleep(10);
WARNING: quoted string split across lines
#334: FILE: drivers/media/common/tuners/fc0011.c:270:
+		dev_warn(&priv->i2c->dev, "Unsupported bandwidth %u kHz. "
+			 "Using 6000 kHz.\n",

WARNING: line over 80 characters
#397: FILE: drivers/media/common/tuners/fc0011.c:333:
+		err |= fc0011_writereg(priv, FC11_REG_RCCAL, regs[FC11_REG_RCCAL]);

WARNING: line over 80 characters
#420: FILE: drivers/media/common/tuners/fc0011.c:356:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#430: FILE: drivers/media/common/tuners/fc0011.c:366:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#439: FILE: drivers/media/common/tuners/fc0011.c:375:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#449: FILE: drivers/media/common/tuners/fc0011.c:385:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#456: FILE: drivers/media/common/tuners/fc0011.c:392:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#468: FILE: drivers/media/common/tuners/fc0011.c:404:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: line over 80 characters
#478: FILE: drivers/media/common/tuners/fc0011.c:414:
+			regs[FC11_REG_VCOSEL] &= ~(FC11_VCOSEL_1 | FC11_VCOSEL_2);

WARNING: msleep < 20ms can sleep for up to 20ms; see 
Documentation/timers/timers-howto.txt
+	msleep(10);
WARNING: quoted string split across lines
#504: FILE: drivers/media/common/tuners/fc0011.c:440:
+	dev_dbg(&priv->i2c->dev, "Tuned to "
+		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "

WARNING: please, no spaces at the start of a line
#620: FILE: drivers/media/common/tuners/fc0011.h:26:
+    defined(CONFIG_MEDIA_TUNER_FC0011_MODULE)$

total: 0 errors, 14 warnings, 598 lines checked

Your patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


hmmmm, I think Mauro will at least complain when I ask he to PULL that 
master. Personally I would like to see line len something more than 80 
chars, but as checkpatch.pl complains it I have shortened lines despite 
very few cases.

@Mauro what do you think are that kind of WARNINGs, those are not 
errors, allowed?

>> And one note about tuner driver, my AF9035 + FC0011 device founds only 1
>> mux of 4. Looks like some performance issues still to resolve.
>
> I have no idea what this means.
> So I have no remote idea of what could possibly be wrong here.
> Is this a bug on af903x or the tuner driver?

Likely tuner driver, or demod driver. But as demod tuner initialization 
tables are likely correct I suspect it is tuner issue at first hand. And 
secondly my other hardware with TUA9001 performs very well, better than 
old AF9015 sticks.


regards
Antti
-- 
http://palosaari.fi/

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 17:40     ` Antti Palosaari
@ 2012-04-02 17:51       ` Michael Büsch
  2012-04-02 21:52         ` Antti Palosaari
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Büsch @ 2012-04-02 17:51 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, linux-media

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

On Mon, 02 Apr 2012 20:40:45 +0300
Antti Palosaari <crope@iki.fi> wrote:

> hmmmm, I think Mauro will at least complain when I ask he to PULL that 
> master. Personally I would like to see line len something more than 80 
> chars, but as checkpatch.pl complains it I have shortened lines despite 
> very few cases.

I'm not a friend of long lines. In fact, I'm developing on a Netbook
with split screen. So long lines will absolutely kill readability for me.
But there is "long" as in 90 or 100 chars and there is "long" as in
"uh let's stretch the 80 chars limit by a few chars, so that it's more readable".

I already worked on code from other kernel subsystems for quite some time
and the 80 char limit never was a hard limit there. For good reasons.

That said, iff the 80 char limit _is_ a hard limit for the DVB subsystem,
I'll honor it. I just think it would worsen the code.

> Likely tuner driver, or demod driver. But as demod tuner initialization 
> tables are likely correct I suspect it is tuner issue at first hand. And 
> secondly my other hardware with TUA9001 performs very well, better than 
> old AF9015 sticks.

Well the fc0011 tuner driver still works worse on this af9035 driver
than on Hans' driver. I have absolutely no idea why this is the case.
I'm almost certain that it is not a timing issue of some sort. I tried
a zillion of delays and such.

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 17:51       ` Michael Büsch
@ 2012-04-02 21:52         ` Antti Palosaari
  2012-04-03  6:36           ` Michael Büsch
  0 siblings, 1 reply; 10+ messages in thread
From: Antti Palosaari @ 2012-04-02 21:52 UTC (permalink / raw)
  To: Michael Büsch
  Cc: Mauro Carvalho Chehab, linux-media, Hans-Frieder Vogt, Gianluca Gennari

On 02.04.2012 20:51, Michael Büsch wrote:
>> Likely tuner driver, or demod driver. But as demod tuner initialization
>> tables are likely correct I suspect it is tuner issue at first hand. And
>> secondly my other hardware with TUA9001 performs very well, better than
>> old AF9015 sticks.
>
> Well the fc0011 tuner driver still works worse on this af9035 driver
> than on Hans' driver. I have absolutely no idea why this is the case.
> I'm almost certain that it is not a timing issue of some sort. I tried
> a zillion of delays and such.

And after taking sniffs and comparing those I found the reason. It is 
I2C adapter code. It writes one byte too much => as a FC0011 is 
auto-increment (as almost every I2C client) registers it means it writes 
next register too. Fixing that gives normal tuner sensitivity. I will 
try to make patch for that soon.

regards
Antti
-- 
http://palosaari.fi/

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 21:52         ` Antti Palosaari
@ 2012-04-03  6:36           ` Michael Büsch
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Büsch @ 2012-04-03  6:36 UTC (permalink / raw)
  To: Antti Palosaari
  Cc: Mauro Carvalho Chehab, linux-media, Hans-Frieder Vogt, Gianluca Gennari

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Tue, 03 Apr 2012 00:52:08 +0300
Antti Palosaari <crope@iki.fi> wrote:

> > Well the fc0011 tuner driver still works worse on this af9035 driver
> > than on Hans' driver. I have absolutely no idea why this is the case.
> > I'm almost certain that it is not a timing issue of some sort. I tried
> > a zillion of delays and such.
> 
> And after taking sniffs and comparing those I found the reason. It is 
> I2C adapter code. It writes one byte too much => as a FC0011 is 
> auto-increment (as almost every I2C client) registers it means it writes 
> next register too. Fixing that gives normal tuner sensitivity. I will 
> try to make patch for that soon.

Awesome! Thanks a lot!

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-04-02 16:14 [PATCH] Add fc0011 tuner driver Michael Büsch
  2012-04-02 16:56 ` Antti Palosaari
@ 2012-05-07 19:02 ` Antti Palosaari
  2012-05-07 21:00   ` Michael Büsch
  1 sibling, 1 reply; 10+ messages in thread
From: Antti Palosaari @ 2012-05-07 19:02 UTC (permalink / raw)
  To: Michael Büsch; +Cc: linux-media

On 02.04.2012 19:14, Michael Büsch wrote:
> This adds support for the Fitipower fc0011 DVB-t tuner.
>
> Signed-off-by: Michael Buesch<m@bues.ch>

> +	unsigned int i, vco_retries;
> +	u32 freq = p->frequency / 1000;
> +	u32 bandwidth = p->bandwidth_hz / 1000;
> +	u32 fvco, xin, xdiv, xdivr;
> +	u16 frac;
> +	u8 fa, fp, vco_sel, vco_cal;
> +	u8 regs[FC11_NR_REGS] = { };

> +
> +	dev_dbg(&priv->i2c->dev, "Tuned to "
> +		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "
> +		"vcocal=%02X(%u) bw=%u\n",
> +		(unsigned int)regs[FC11_REG_FA],
> +		(unsigned int)regs[FC11_REG_FP],
> +		(unsigned int)regs[FC11_REG_XINHI],
> +		(unsigned int)regs[FC11_REG_XINLO],
> +		(unsigned int)regs[FC11_REG_VCO],
> +		(unsigned int)regs[FC11_REG_VCOSEL],
> +		(unsigned int)vco_cal, vco_retries,
> +		(unsigned int)bandwidth);

Just for the interest, is there any reason you use so much casting or is 
that only your style?

I removed some similar castings from the AF9035 log writings you added 
as I did not see need for those. I even think casting should be avoided 
as it can hide possible meaningful compiler warnings on bad case.

regards
Antti
-- 
http://palosaari.fi/

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-05-07 19:02 ` Antti Palosaari
@ 2012-05-07 21:00   ` Michael Büsch
  2012-05-07 21:13     ` Rémi Denis-Courmont
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Büsch @ 2012-05-07 21:00 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

On Mon, 07 May 2012 22:02:18 +0300
Antti Palosaari <crope@iki.fi> wrote:

> On 02.04.2012 19:14, Michael Büsch wrote:
> > This adds support for the Fitipower fc0011 DVB-t tuner.
> >
> > Signed-off-by: Michael Buesch<m@bues.ch>
> 
> > +	unsigned int i, vco_retries;
> > +	u32 freq = p->frequency / 1000;
> > +	u32 bandwidth = p->bandwidth_hz / 1000;
> > +	u32 fvco, xin, xdiv, xdivr;
> > +	u16 frac;
> > +	u8 fa, fp, vco_sel, vco_cal;
> > +	u8 regs[FC11_NR_REGS] = { };
> 
> > +
> > +	dev_dbg(&priv->i2c->dev, "Tuned to "
> > +		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "
> > +		"vcocal=%02X(%u) bw=%u\n",
> > +		(unsigned int)regs[FC11_REG_FA],
> > +		(unsigned int)regs[FC11_REG_FP],
> > +		(unsigned int)regs[FC11_REG_XINHI],
> > +		(unsigned int)regs[FC11_REG_XINLO],
> > +		(unsigned int)regs[FC11_REG_VCO],
> > +		(unsigned int)regs[FC11_REG_VCOSEL],
> > +		(unsigned int)vco_cal, vco_retries,
> > +		(unsigned int)bandwidth);
> 
> Just for the interest, is there any reason you use so much casting or is 
> that only your style?

Well it makes sure the types are what the format string and thus vararg code expects.
it is true that most (probably all) of those explicit casts could be removed and instead
rely on implicit casts and promotions. But I personally prefer explicit casts
in this case (and only this case).

-- 
Greetings, Michael.

PGP encryption is encouraged / 908D8B0E

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH] Add fc0011 tuner driver
  2012-05-07 21:00   ` Michael Büsch
@ 2012-05-07 21:13     ` Rémi Denis-Courmont
  0 siblings, 0 replies; 10+ messages in thread
From: Rémi Denis-Courmont @ 2012-05-07 21:13 UTC (permalink / raw)
  To: linux-media

Le mardi 8 mai 2012 00:00:31 Michael Büsch, vous avez écrit :
> > > +	dev_dbg(&priv->i2c->dev, "Tuned to "
> > > +		"fa=%02X fp=%02X xin=%02X%02X vco=%02X vcosel=%02X "
> > > +		"vcocal=%02X(%u) bw=%u\n",
> > > +		(unsigned int)regs[FC11_REG_FA],
> > > +		(unsigned int)regs[FC11_REG_FP],
> > > +		(unsigned int)regs[FC11_REG_XINHI],
> > > +		(unsigned int)regs[FC11_REG_XINLO],
> > > +		(unsigned int)regs[FC11_REG_VCO],
> > > +		(unsigned int)regs[FC11_REG_VCOSEL],
> > > +		(unsigned int)vco_cal, vco_retries,
> > > +		(unsigned int)bandwidth);
> > 
> > Just for the interest, is there any reason you use so much casting or is
> > that only your style?
> 
> Well it makes sure the types are what the format string and thus vararg
> code expects. it is true that most (probably all) of those explicit casts
> could be removed and instead rely on implicit casts and promotions. But I
> personally prefer explicit casts in this case (and only this case).

Not sure Linux printk supports it, but C specifies the "hh" prefix for 'char', 
and the "h" prefix for 'short' for instance "%02hhX", and this would also work 
for u8 and u16.

The pedantic in me needs to add that the official prefixes for uint8_t and 
uint16_t are the PRIX8 and PRIX16 macros from <inttypes.h>, e.g.:
	printf("%02"PRIX8"\n", regs[0]);
...but that's definitely not valid in kernel.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

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

end of thread, other threads:[~2012-05-07 21:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-02 16:14 [PATCH] Add fc0011 tuner driver Michael Büsch
2012-04-02 16:56 ` Antti Palosaari
2012-04-02 17:20   ` Michael Büsch
2012-04-02 17:40     ` Antti Palosaari
2012-04-02 17:51       ` Michael Büsch
2012-04-02 21:52         ` Antti Palosaari
2012-04-03  6:36           ` Michael Büsch
2012-05-07 19:02 ` Antti Palosaari
2012-05-07 21:00   ` Michael Büsch
2012-05-07 21:13     ` Rémi Denis-Courmont

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