* [PATCH 1/3] si2165: Add demod driver for DVB-T only
@ 2014-04-26 20:21 Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 2/3] cx231xx: Add [2040:b130] Hauppauge WinTV 930C-hd 1113xx Matthias Schwarzott
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Matthias Schwarzott @ 2014-04-26 20:21 UTC (permalink / raw)
To: linux-media; +Cc: zzam, crope, xpert-reactos
DVB-T was tested with 8MHz BW channels in germany
This driver is the simplest possible, it uses automatic mode for all
parameters (TPS).
Firmware file can be extracted via get_dvb_firmware.
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
---
Documentation/dvb/get_dvb_firmware | 33 +-
drivers/media/dvb-frontends/Kconfig | 9 +
drivers/media/dvb-frontends/Makefile | 1 +
drivers/media/dvb-frontends/si2165.c | 890 ++++++++++++++++++++++++++++++
drivers/media/dvb-frontends/si2165.h | 61 ++
drivers/media/dvb-frontends/si2165_priv.h | 23 +
6 files changed, 1016 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/dvb-frontends/si2165.c
create mode 100644 drivers/media/dvb-frontends/si2165.h
create mode 100644 drivers/media/dvb-frontends/si2165_priv.h
diff --git a/Documentation/dvb/get_dvb_firmware b/Documentation/dvb/get_dvb_firmware
index d91b8be..26c623d 100755
--- a/Documentation/dvb/get_dvb_firmware
+++ b/Documentation/dvb/get_dvb_firmware
@@ -29,7 +29,7 @@ use IO::Handle;
"af9015", "ngene", "az6027", "lme2510_lg", "lme2510c_s7395",
"lme2510c_s7395_old", "drxk", "drxk_terratec_h5",
"drxk_hauppauge_hvr930c", "tda10071", "it9135", "drxk_pctv",
- "drxk_terratec_htc_stick", "sms1xxx_hcw");
+ "drxk_terratec_htc_stick", "sms1xxx_hcw", "si2165");
# Check args
syntax() if (scalar(@ARGV) != 1);
@@ -783,6 +783,37 @@ sub sms1xxx_hcw {
$allfiles;
}
+sub si2165 {
+ my $sourcefile = "model_111xxx_122xxx_driver_6_0_119_31191_WHQL.zip";
+ my $url = "http://www.hauppauge.de/files/drivers/";
+ my $hash = "76633e7c76b0edee47c3ba18ded99336";
+ my $fwfile = "dvb-demod-si2165.fw";
+ my $tmpdir = tempdir(DIR => "/tmp", CLEANUP => 1);
+
+ checkstandard();
+
+ wgetfile($sourcefile, $url . $sourcefile);
+ verify($sourcefile, $hash);
+ unzip($sourcefile, $tmpdir);
+ extract("$tmpdir/Driver10/Hcw10bda.sys", 0x80788, 0x81E08-0x80788, "$tmpdir/fw1");
+
+ delzero("$tmpdir/fw1","$tmpdir/fw1-1");
+ #verify("$tmpdir/fw1","5e0909858fdf0b5b09ad48b9fe622e70");
+
+ my $CRC="\x0A\xCC";
+ my $BLOCKS_MAIN="\x27";
+ open FW,">$fwfile";
+ print FW "\x01\x00"; # just a version id for the driver itself
+ print FW "\x9A"; # fw version
+ print FW "\x00"; # padding
+ print FW "$BLOCKS_MAIN"; # number of blocks of main part
+ print FW "\x00"; # padding
+ print FW "$CRC"; # 16bit crc value of main part
+ appendfile(FW,"$tmpdir/fw1");
+
+ "$fwfile";
+}
+
# ---------------------------------------------------------------
# Utilities
diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index 1469d44..0da53c2 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -63,6 +63,15 @@ config DVB_TDA18271C2DD
Say Y when you want to support this tuner.
+config DVB_SI2165
+ tristate "Silicon Labs si2165 based"
+ depends on DVB_CORE && I2C
+ default m if !MEDIA_SUBDRV_AUTOSELECT
+ help
+ A DVB-C/T demodulator.
+
+ Say Y when you want to support this frontend.
+
comment "DVB-S (satellite) frontends"
depends on DVB_CORE
diff --git a/drivers/media/dvb-frontends/Makefile b/drivers/media/dvb-frontends/Makefile
index dda0bee..595dd8d 100644
--- a/drivers/media/dvb-frontends/Makefile
+++ b/drivers/media/dvb-frontends/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_DVB_STV0367) += stv0367.o
obj-$(CONFIG_DVB_CXD2820R) += cxd2820r.o
obj-$(CONFIG_DVB_DRXK) += drxk.o
obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
+obj-$(CONFIG_DVB_SI2165) += si2165.o
obj-$(CONFIG_DVB_A8293) += a8293.o
obj-$(CONFIG_DVB_TDA10071) += tda10071.o
obj-$(CONFIG_DVB_RTL2830) += rtl2830.o
diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
new file mode 100644
index 0000000..3e69a32
--- /dev/null
+++ b/drivers/media/dvb-frontends/si2165.c
@@ -0,0 +1,890 @@
+/*
+ Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
+
+ Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
+
+ 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.
+
+ References:
+ http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
+*/
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/firmware.h>
+
+#include "dvb_frontend.h"
+#include "dvb_math.h"
+#include "si2165_priv.h"
+#include "si2165.h"
+
+/* Hauppauge WinTV-HVR-930C-HD 1113xx uses 16.000 MHz xtal */
+
+struct si2165_state {
+ struct i2c_adapter *i2c;
+
+ struct dvb_frontend frontend;
+
+ struct si2165_config config;
+
+ /* chip revision */
+ u8 revcode;
+ /* chip type */
+ u8 chip_type;
+
+ /* calculated by xtal and div settings */
+ u32 fvco_hz;
+ u32 sys_clk;
+ u32 adc_clk;
+
+ /* depends on adc_clk and Ovr mode */
+ u32 fe_clk;
+
+ bool m_has_dvbc;
+ bool m_has_dvbt;
+ bool firmware_loaded;
+};
+
+#define DEBUG_OTHER 0x01
+#define DEBUG_I2C_WRITE 0x02
+#define DEBUG_I2C_READ 0x04
+#define DEBUG_REG_READ 0x08
+#define DEBUG_REG_WRITE 0x10
+#define DEBUG_FW_LOAD 0x20
+
+static int debug = 0x00;
+
+#define dprintk(args...) \
+ do { \
+ if (debug & DEBUG_OTHER) \
+ printk(KERN_DEBUG "si2165: " args); \
+ } while (0)
+
+#define deb_i2c_write(args...) \
+ do { \
+ if (debug & DEBUG_I2C_WRITE) \
+ printk(KERN_DEBUG "si2165: i2c write: " args); \
+ } while (0)
+
+#define deb_i2c_read(args...) \
+ do { \
+ if (debug & DEBUG_I2C_READ) \
+ printk(KERN_DEBUG "si2165: i2c read: " args); \
+ } while (0)
+
+#define deb_readreg(args...) \
+ do { \
+ if (debug & DEBUG_REG_READ) \
+ printk(KERN_DEBUG "si2165: reg read: " args); \
+ } while (0)
+
+#define deb_writereg(args...) \
+ do { \
+ if (debug & DEBUG_REG_WRITE) \
+ printk(KERN_DEBUG "si2165: reg write: " args); \
+ } while (0)
+
+#define deb_fw_load(args...) \
+ do { \
+ if (debug & DEBUG_FW_LOAD) \
+ printk(KERN_DEBUG "si2165: fw load: " args); \
+ } while (0)
+
+static int si2165_write(struct si2165_state *state, const u16 reg,
+ const u8 *src, const int count)
+{
+ int ret;
+ struct i2c_msg msg;
+ u8 buf[2 + 4]; /* write a maximum of 4 bytes of data */
+ if (count + 2 > sizeof(buf)) {
+ dev_warn(&state->i2c->dev,
+ "%s: i2c wr reg=%04x: count=%d is too big!\n",
+ KBUILD_MODNAME, reg, count);
+ return -EINVAL;
+ }
+ buf[0] = reg >> 8;
+ buf[1] = reg & 0xff;
+ memcpy(buf + 2, src, count);
+
+ msg.addr = state->config.i2c_addr;
+ msg.flags = 0;
+ msg.buf = buf;
+ msg.len = count + 2;
+
+ if (debug & DEBUG_I2C_WRITE) {
+ int i;
+ deb_i2c_write("reg: 0x%04x, data:", reg);
+ for (i = 0; i < count; i++)
+ printk(KERN_CONT " %02x", src[i]);
+ printk(KERN_CONT "\n");
+ }
+
+ ret = i2c_transfer(state->i2c, &msg, 1);
+
+ if (ret != 1) {
+ dev_err(&state->i2c->dev, "%s: ret == %d\n", __func__, ret);
+ return -EREMOTEIO;
+ }
+
+ return 0;
+}
+
+static int si2165_read(struct si2165_state *state,
+ const u16 reg, u8 *val, const size_t count)
+{
+ int ret;
+ u8 reg_buf[] = { reg >> 8, reg & 0xff };
+ struct i2c_msg msg[] = {
+ { .addr = state->config.i2c_addr,
+ .flags = 0, .buf = reg_buf, .len = 2 },
+ { .addr = state->config.i2c_addr,
+ .flags = I2C_M_RD, .buf = val, .len = count },
+ };
+
+ ret = i2c_transfer(state->i2c, msg, 2);
+
+ if (ret != 2) {
+ dev_err(&state->i2c->dev, "%s: error (addr %02x reg %04x error (ret == %i)\n",
+ __func__, state->config.i2c_addr, reg, ret);
+ if (ret < 0)
+ return ret;
+ else
+ return -EREMOTEIO;
+ }
+
+ if (debug & DEBUG_I2C_READ) {
+ int i;
+ deb_i2c_read("reg: 0x%04x, data:", reg);
+ for (i = 0; i < count; i++)
+ printk(KERN_CONT " %02x", val[i]);
+ printk(KERN_CONT "\n");
+ }
+ return 0;
+}
+
+static int si2165_readreg8(struct si2165_state *state,
+ const u16 reg, u8 *val)
+{
+ int ret;
+ ret = si2165_read(state, reg, val, 1);
+ deb_readreg("R(0x%04x)=0x%02x\n", reg, *val);
+ return ret;
+}
+
+static int si2165_readreg16(struct si2165_state *state,
+ const u16 reg, u16 *val)
+{
+ u8 buf[2];
+ int ret = si2165_read(state, reg, buf, 2);
+ *val = buf[0] | buf[1] << 8;
+ deb_readreg("R(0x%04x)=0x%04x\n", reg, *val);
+ return ret;
+}
+
+#if 0
+static int si2165_readreg24(struct si2165_state *state,
+ const u16 reg, u32 *val)
+{
+ u8 buf[3];
+ int ret = si2165_read(state, reg, buf, 3);
+ *val = buf[0] | buf[1] << 8 | buf[2] << 16;
+ deb_readreg("R(0x%04x)=0x%06x\n", reg, *val);
+ return ret;
+}
+
+static int si2165_readreg32(struct si2165_state *state,
+ const u16 reg, u32 *val)
+{
+ u8 buf[4];
+ int ret = si2165_read(state, reg, buf, 4);
+ *val = buf[0] | buf[1] << 8 | buf[2] << 16 | buf[3] << 24;
+ deb_readreg("R(0x%04x)=0x%08x\n", reg, *val);
+ return ret;
+}
+#endif
+
+
+static int si2165_writereg8(struct si2165_state *state, const u16 reg, u8 val)
+{
+ return si2165_write(state, reg, &val, 1);
+}
+
+static int si2165_writereg16(struct si2165_state *state, const u16 reg, u16 val)
+{
+ u8 buf[2] = { val & 0xff, (val >> 8) & 0xff };
+ return si2165_write(state, reg, buf, 2);
+}
+
+static int si2165_writereg24(struct si2165_state *state, const u16 reg, u32 val)
+{
+ u8 buf[3] = { val & 0xff, (val >> 8) & 0xff, (val >> 16) & 0xff };
+ return si2165_write(state, reg, buf, 3);
+}
+
+static int si2165_writereg32(struct si2165_state *state, const u16 reg, u32 val)
+{
+ u8 buf[4] = {
+ val & 0xff,
+ (val >> 8) & 0xff,
+ (val >> 16) & 0xff,
+ (val >> 24) & 0xff
+ };
+ return si2165_write(state, reg, buf, 4);
+}
+
+static int si2165_writereg_mask8(struct si2165_state *state, const u16 reg,
+ u8 val, u8 mask)
+{
+ int ret;
+ u8 tmp;
+
+ if (mask != 0xff) {
+ ret = si2165_readreg8(state, reg, &tmp);
+ if (ret < 0)
+ goto err;
+
+ val &= mask;
+ tmp &= ~mask;
+ val |= tmp;
+ }
+
+ ret = si2165_writereg8(state, reg, val);
+err:
+ return ret;
+}
+
+static int si2165_get_tune_settings(struct dvb_frontend *fe,
+ struct dvb_frontend_tune_settings *s)
+{
+ s->min_delay_ms = 1000;
+ return 0;
+}
+
+static int si2165_init_pll(struct si2165_state *state)
+{
+ u8 ref_freq_MHz = state->config.ref_freq_MHz;
+ /* u8 val; */
+ u8 divr = 1; /* 1..7 */
+ u8 divp = 1; /* only 1 or 4 */
+ u8 divn = 56; /* 1..63 */
+
+ u8 divm = 8;
+ u8 divl = 12;
+ u8 buf[4];
+
+ /* ref_freq / divr must be between 4 and 16 MHz */
+ if (ref_freq_MHz > 16)
+ divr = 2;
+
+ /* now select divn and divp such that fvco is in 1624..1824 MHz */
+ if (1624 * divr > ref_freq_MHz * 2 * 63)
+ divp = 4;
+
+ /* to get exactly the same as the windows driver does */
+ if (ref_freq_MHz == 16)
+ divn = 56;
+ else {
+ /* is this already correct regarding rounding? */
+ divn = 1624 * divr / (ref_freq_MHz * 2 * divp);
+ }
+
+ /* adc_clk and sys_clk depend on xtal and pll settings */
+ /* only calculate once as long as the pll settings are not modified */
+ if (state->adc_clk == 0) {
+ u32 fvco_hz = ref_freq_MHz * 1000000ull / divr
+ * 2 * divn * divp;
+ state->adc_clk = fvco_hz / (divm * 4);
+ state->sys_clk = fvco_hz / (divl * 2);
+ }
+
+ /* write pll registers 0x00a0..0x00a3 at once */
+ buf[0] = divl;
+ buf[1] = divm;
+ buf[2] = (divn & 0x3f) | ((divp == 1) ? 0x40 : 0x00) | 0x80;
+ buf[3] = divr;
+ si2165_write(state, 0x00a0 /* first pll reg */, buf, 4);
+
+ return 0;
+}
+
+static bool si2165_wait_init_done(struct si2165_state *state)
+{
+ int ret = false;
+ u8 val;
+ int i;
+ for (i = 0; i < 10; i++) {
+ si2165_readreg8(state, 0x0054 /* init_done */, &val);
+ if (val == 0x01) {
+ ret = true;
+ break;
+ }
+ msleep(1);
+ }
+ return ret;
+}
+
+static int si2165_upload_firmware_block(struct si2165_state *state, const u8 *data, u32 len, u32 *poffset, u32 block_count)
+{
+ u8 buf_ctrl[4] = { 0x00, 0x00, 0x00, 0xc0 };
+ u8 wordcount;
+ u32 cur_block = 0;
+ u32 offset = poffset ? *poffset : 0;
+ if (len < 4)
+ return -EINVAL;
+ if (len % 4 != 0)
+ return -EINVAL;
+
+ deb_fw_load("si2165_upload_firmware_block called with len=0x%x offset=0x%x blockcount=0x%x\n",
+ len, offset, block_count);
+ while (offset+12 <= len && cur_block < block_count) {
+ deb_fw_load("si2165_upload_firmware_block in while len=0x%x offset=0x%x cur_block=0x%x blockcount=0x%x\n",
+ len, offset, cur_block, block_count);
+ wordcount = data[offset];
+ if (wordcount < 1 || data[offset+1] || data[offset+2] || data[offset+3]) {
+ dev_warn(&state->i2c->dev, "%s: bad fw data[0..3] = %02x %02x %02x %02x\n", KBUILD_MODNAME, data[0], data[1], data[2], data[3]);
+ return -EINVAL;
+ }
+
+ if (offset + 8 + wordcount * 4 > len) {
+ dev_warn(&state->i2c->dev, "%s: len is too small for block len=%d, wordcount=%d\n", KBUILD_MODNAME, len, wordcount);
+ return -EINVAL;
+ }
+
+ buf_ctrl[0] = wordcount - 1;
+
+ si2165_write(state, 0x0364, buf_ctrl, 4);
+ si2165_write(state, 0x0368, data+offset+4, 4);
+
+ offset += 8;
+
+ while (wordcount > 0) {
+ si2165_write(state, 0x36c, data+offset, 4);
+ wordcount--;
+ offset += 4;
+ }
+ cur_block++;
+ }
+
+ deb_fw_load("si2165_upload_firmware_block after while len=0x%x offset=0x%x cur_block=0x%x blockcount=0x%x\n",
+ len, offset, cur_block, block_count);
+
+ if (poffset)
+ *poffset = offset;
+
+ deb_fw_load("si2165_upload_firmware_block returned offset=0x%x\n",
+ offset);
+
+ return 0;
+}
+
+static int si2165_upload_firmware(struct si2165_state *state)
+{
+ /* int ret; */
+ u8 val[3];
+ u16 val16;
+ int ret;
+
+ const struct firmware *fw = NULL;
+ u8 *fw_file = SI2165_FIRMWARE;
+ const u8 *data;
+ u32 len;
+ u32 offset;
+ u8 patch_version;
+ u8 block_count;
+ u16 crc_expected;
+
+ /* request the firmware, this will block and timeout */
+ ret = request_firmware(&fw, fw_file, state->i2c->dev.parent);
+ if (ret) {
+ dev_warn(&state->i2c->dev, "%s: firmare file '%s' not found\n",
+ KBUILD_MODNAME, fw_file);
+ goto err;
+ }
+
+ data = fw->data;
+ len = fw->size;
+
+ dev_info(&state->i2c->dev, "%s: downloading firmware from file '%s' size=%d\n",
+ KBUILD_MODNAME, fw_file, len);
+
+ if (len % 4 != 0) {
+ dev_warn(&state->i2c->dev, "%s: firmware size is not multiple of 4\n",
+ KBUILD_MODNAME);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ /* check header (8 bytes) */
+ if (len < 8) {
+ dev_warn(&state->i2c->dev, "%s: firmware header is missing\n",
+ KBUILD_MODNAME);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (data[0] != 1 || data[1] != 0) {
+ dev_warn(&state->i2c->dev, "%s: firmware file version is wrong\n",
+ KBUILD_MODNAME);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ patch_version = data[2];
+ block_count = data[4];
+ crc_expected = data[7] << 8 | data[6];
+
+ /* start uploading fw */
+ si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
+ si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+
+ si2165_readreg8(state, 0x035c /* en_rst_error */, val); /* returned 0x03 */
+ si2165_readreg8(state, 0x035c /* en_rst_error */, val); /* returned 0x03 */
+ si2165_writereg8(state, 0x035c /* en_rst_error */, 0x02);
+
+ /* start right after the header */
+ offset = 8;
+
+ dev_info(&state->i2c->dev, "%s: si2165_upload_firmware extracted patch_version=0x%02x, block_count=0x%02x, crc_expected=0x%04x\n",
+ KBUILD_MODNAME, patch_version, block_count, crc_expected);
+
+ ret = si2165_upload_firmware_block(state, data, len, &offset, 1);
+
+ si2165_writereg8(state, 0x0344 /* patch_version */, patch_version);
+
+ ret = si2165_writereg8(state, 0x0379 /* rst_crc */, 0x01);
+ if (ret)
+ return ret;
+
+ ret = si2165_upload_firmware_block(state, data, len, &offset, block_count);
+
+ if (ret) {
+ dev_err(&state->i2c->dev, "%s: firmare could not be uploaded\n",
+ KBUILD_MODNAME);
+ goto err;
+ }
+
+ ret = si2165_readreg16(state, 0x037a /* crc */, &val16); /* returned 0xcc0a */
+ if (ret)
+ goto err;
+
+ if (val16 != crc_expected) {
+ dev_err(&state->i2c->dev, "%s: firmware crc mismatch %04x != %04x\n", KBUILD_MODNAME, val16, crc_expected);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ ret = si2165_upload_firmware_block(state, data, len, &offset, 5);
+ if (ret)
+ goto err;
+
+ if (len != offset) {
+ dev_err(&state->i2c->dev, "%s: firmare len mismatch %04x != %04x\n", KBUILD_MODNAME, len, offset);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ /* reset watchdog error register, using auto return value rst_wdog_error */
+ si2165_writereg_mask8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x02, 0x02);
+
+ /* enable reset on error */
+ si2165_writereg_mask8(state, 0x035c /* en_rst_error */, 0x01, 0x01);
+
+ dev_info(&state->i2c->dev, "%s: fw load finished\n", KBUILD_MODNAME);
+
+ ret = 0;
+ state->firmware_loaded = true;
+err:
+ if (fw) {
+ release_firmware(fw);
+ fw = NULL;
+ }
+
+ return ret;
+}
+
+static int si2165_init_dsp(struct si2165_state *state)
+{
+ u8 val;
+ u8 patch_version = 0x00;
+
+ si2165_readreg8(state, 0x0344 /* patch_version */, &patch_version); /* returned 0x00 */
+
+ si2165_writereg8(state, 0x00cb, 0x00);
+ if (patch_version == 0x00)
+ si2165_writereg8(state, 0x0344 /* patch_version */, 0x00);
+
+ si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, &val); /* returned 0x01 */
+
+ if (patch_version == 0x00)
+ si2165_upload_firmware(state);
+
+ return 0;
+}
+
+static int si2165_init(struct dvb_frontend *fe)
+{
+ struct si2165_state *state = fe->demodulator_priv;
+ u8 val;
+
+ dprintk("%s: called\n", __func__);
+
+ /* powerup */
+ si2165_writereg8(state, 0x0000 /* chip_mode */, state->config.chip_mode);
+ si2165_writereg8(state, 0x0104 /* dsp_clock_enable */, 0x01);
+ si2165_readreg8(state, 0x0000 /* chip_mode */, &val);
+ if (val != state->config.chip_mode) {
+ dev_err(&state->i2c->dev, "%s: could not set chip_mode\n",
+ KBUILD_MODNAME);
+ return -EINVAL;
+ }
+
+ si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
+ si2165_writereg8(state, 0x0190 /* agc_if_slr */, 0x01);
+ si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, &val); /* returned 0x00 */
+ si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, 0x00);
+ si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, &val); /* returned 0x00 */
+ si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, 0x00);
+ si2165_writereg8(state, 0x0171 /* agc2_clkdiv */, 0x07);
+ si2165_writereg8(state, 0x0646 /* rssi_pad_ctrl */, 0x00);
+ si2165_readreg8(state, 0x0641 /* en_rssi,start_rssi,rssi_update_time */, &val); /* returned 0x00 */
+ si2165_writereg8(state, 0x0641 /* en_rssi,start_rssi,rssi_update_time */, 0x00);
+ si2165_writereg8(state, 0x00e0 /* adc_sampling_mode */, 0x00);
+
+ si2165_init_pll(state);
+
+ si2165_writereg8(state, 0x0050 /* chip_init */, 0x01);
+ si2165_writereg8(state, 0x0096 /* start_init */, 0x01);
+ if (!si2165_wait_init_done(state)) {
+ dev_err(&state->i2c->dev, "%s: init_done was not set\n",
+ KBUILD_MODNAME);
+ return -EINVAL;
+ }
+
+ si2165_writereg8(state, 0x0050 /* chip_init */, 0x00);
+
+ si2165_writereg16(state, 0x0470 /* ber_pkt */, 0x7530);
+
+ si2165_init_dsp(state);
+
+ si2165_writereg8(state, 0x012a /* adc_ri1 */, 0x46);
+ si2165_writereg8(state, 0x012c /* adc_ri3 */, 0x00);
+ si2165_writereg8(state, 0x012e /* adc_ri5 */, 0x0a);
+ si2165_writereg8(state, 0x012f /* adc_ri6 */, 0xff);
+ si2165_writereg8(state, 0x0123 /* adc_ri8 */, 0x70);
+
+ return 0;
+}
+
+static int si2165_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
+{
+ struct si2165_state *state = fe->demodulator_priv;
+ u8 val = enable ? 0x01 : 0x00;
+ return si2165_writereg8(state, 0x0001 /* i2c passthru */, val);
+}
+
+static int si2165_sleep(struct dvb_frontend *fe)
+{
+ struct si2165_state *state = fe->demodulator_priv;
+ dprintk("%s: called\n", __func__);
+ si2165_writereg8(state, 0x0104 /* dsp clock enable */, 0x00);
+ si2165_writereg8(state, 0x0000 /* chip mode */, SI2165_MODE_OFF);
+ return 0;
+}
+
+static int si2165_read_status(struct dvb_frontend *fe, fe_status_t *status)
+{
+ u8 fec_lock = 0;
+ struct si2165_state *state = fe->demodulator_priv;
+
+ if (!state->m_has_dvbt)
+ return -EINVAL;
+
+ /* seq1 */
+ si2165_readreg8(state, 0x4e0 /* fec_lock */, &fec_lock);
+ *status = 0;
+ if (fec_lock & 0x01) {
+ *status |= FE_HAS_SIGNAL;
+ *status |= FE_HAS_CARRIER;
+ *status |= FE_HAS_VITERBI;
+ *status |= FE_HAS_SYNC;
+ *status |= FE_HAS_LOCK;
+ }
+
+ return 0;
+}
+
+static int si2165_set_parameters(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+ struct si2165_state *state = fe->demodulator_priv;
+ u8 val[3];
+ u32 IF;
+ u32 dvb_rate = 0;
+
+ dprintk("%s: called\n", __func__);
+
+ if (!fe->ops.tuner_ops.get_if_frequency) {
+ pr_err("Error: get_if_frequency() not defined at tuner. Can't work without it!\n");
+ return -EINVAL;
+ }
+
+ /* If Oversampling mode Ovr4 is used */
+ state->fe_clk = state->adc_clk;
+
+ if (state->fe_clk == 0) {
+ pr_err("Error: fe_clk is 0\n");
+ return -EINVAL;
+ }
+
+ if (!state->m_has_dvbt)
+ return -EINVAL;
+
+ if (p->bandwidth_hz > 0)
+ dvb_rate = p->bandwidth_hz * 8 / 7;
+ else
+ dvb_rate = 8 * 8 / 7;
+
+ si2165_writereg8(state, 0x00ec /* standard */, 0x01);
+ si2165_writereg8(state, 0x00a0 /* pll_divl */, 0x0c);
+
+ si2165_readreg8(state, 0x00e0 /* adc_sampling_mode */, val); /* returned 0x00 */
+ si2165_writereg32(state, 0x00e8 /* if_freq_shift */, 0x00000000);
+ si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
+ si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x21 */
+ si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
+ si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
+ si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
+ si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
+ si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
+ si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x03 */
+ si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x03);
+ si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x03 */
+ si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
+ si2165_writereg16(state, 0x0308 /* bandwidth */, 0x0320);
+ si2165_writereg32(state, 0x00e4 /* oversamp */, 0x03100000);
+ si2165_writereg8(state, 0x031c /* impulsive_noise_remover */, 0x01);
+ si2165_writereg8(state, 0x00cb /* unknown_wr8 */, 0x00);
+ si2165_writereg8(state, 0x016e /* agc2_min */, 0x41);
+ si2165_writereg8(state, 0x016c /* agc2_kacq */, 0x0e);
+ si2165_writereg8(state, 0x016d /* agc2_kloc */, 0x10);
+ si2165_writereg8(state, 0x015b /* agc_unfreeze_thr */, 0x03);
+ si2165_writereg8(state, 0x0150 /* agc_crestf_dbx8 */, 0x78);
+ si2165_writereg8(state, 0x01a0 /* aaf_crestf_dbx8 */, 0x78);
+ si2165_writereg8(state, 0x01c8 /* aci_crestf_dbx8 */, 0x68);
+ si2165_writereg16(state, 0x030c /* freq_sync_range */, 0x0064);
+ si2165_readreg8(state, 0x0387 /* gp_reg0 */, val); /* returned 0x00 */
+ si2165_writereg8(state, 0x0387 /* gp_reg0 */, 0x00);
+ si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+ si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
+ si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
+ si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
+ si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+ si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
+ si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
+ si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
+ si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+ si2165_readreg8(state, 0x0118 /* dvb-c standard support */, val); /* returned 0x07 */
+ si2165_readreg8(state, 0x0023 /* hardware_rev */, val); /* returned 0x03 */
+ si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
+ si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
+ si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
+ si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
+ si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
+ si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
+ si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
+ si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
+ si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x01 */
+ si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
+ si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x01 */
+ si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
+
+ if (dvb_rate == 0) {
+ pr_err("Error: dvb_rate is 0\n");
+ return -EINVAL;
+ }
+
+ if (fe->ops.tuner_ops.set_params) {
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 1);
+ fe->ops.tuner_ops.set_params(fe);
+ if (fe->ops.i2c_gate_ctrl)
+ fe->ops.i2c_gate_ctrl(fe, 0);
+ }
+
+ {
+ s64 if_freq_shift;
+ u32 reg_value;
+ fe->ops.tuner_ops.get_if_frequency(fe, &IF);
+
+ if_freq_shift = IF;
+ if_freq_shift <<= 29;
+ if (state->fe_clk > 0)
+ if_freq_shift /= (u64)state->fe_clk;
+ reg_value = ((u32)if_freq_shift) & 0x1fffffff;
+
+ si2165_writereg32(state, 0x00e8 /* if_freq_shift */, reg_value);
+ }
+
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+ si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
+ si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
+ si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
+ si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
+ si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
+
+ return 0;
+}
+
+static void si2165_release(struct dvb_frontend *fe)
+{
+ struct si2165_state *state = fe->demodulator_priv;
+ dprintk("%s: called\n", __func__);
+ kfree(state);
+}
+
+static struct dvb_frontend_ops si2165_ops = {
+ .info = {
+ .name = "SI2165",
+ .caps = FE_CAN_FEC_1_2 |
+ FE_CAN_FEC_2_3 |
+ FE_CAN_FEC_3_4 |
+ FE_CAN_FEC_5_6 |
+ FE_CAN_FEC_7_8 |
+ FE_CAN_FEC_AUTO |
+ FE_CAN_QPSK |
+ FE_CAN_QAM_16 |
+ FE_CAN_QAM_32 |
+ FE_CAN_QAM_64 |
+ FE_CAN_QAM_128 |
+ FE_CAN_QAM_256 |
+ FE_CAN_QAM_AUTO |
+ FE_CAN_TRANSMISSION_MODE_AUTO |
+ FE_CAN_GUARD_INTERVAL_AUTO |
+ FE_CAN_HIERARCHY_AUTO |
+ FE_CAN_MUTE_TS |
+ FE_CAN_TRANSMISSION_MODE_AUTO |
+ FE_CAN_RECOVER
+ },
+
+ .get_tune_settings = si2165_get_tune_settings,
+
+ .init = si2165_init,
+ .sleep = si2165_sleep,
+
+ .i2c_gate_ctrl = si2165_i2c_gate_ctrl,
+
+ .set_frontend = si2165_set_parameters,
+ .read_status = si2165_read_status,
+
+ .release = si2165_release,
+};
+
+struct dvb_frontend *si2165_attach(const struct si2165_config *config, struct i2c_adapter *i2c)
+{
+ struct si2165_state *state = NULL;
+ int n;
+
+ if (config == NULL)
+ goto error;
+
+ /* allocate memory for the internal state */
+ state = kzalloc(sizeof(struct si2165_state), GFP_KERNEL);
+ if (state == NULL)
+ goto error;
+
+ /* setup the state */
+ state->i2c = i2c;
+ state->config = *config;
+
+ if (state->config.ref_freq_MHz < 4 || state->config.ref_freq_MHz > 27) {
+ dev_info(&state->i2c->dev, "%s: ref_freq of %d MHz not supported by this driver\n",
+ KBUILD_MODNAME, state->config.ref_freq_MHz);
+ goto error;
+ }
+
+ /* create dvb_frontend */
+ memcpy(&state->frontend.ops, &si2165_ops,
+ sizeof(struct dvb_frontend_ops));
+ state->frontend.demodulator_priv = state;
+
+ /* powerup */
+ if (si2165_writereg8(state, 0x0000 /* chip_mode */, state->config.chip_mode) < 0)
+ goto error;
+
+ if (si2165_readreg8(state, 0x0023 /* rev code */, &state->revcode) < 0)
+ goto error;
+
+ if (si2165_readreg8(state, 0x0118 /* chip type */, &state->chip_type) < 0)
+ goto error;
+
+ /* powerdown */
+ if (si2165_writereg8(state, 0x0000 /* chip_mode */, SI2165_MODE_OFF) < 0)
+ goto error;
+
+ dev_info(&state->i2c->dev, "%s: hardware revision 0x%02x, chip type 0x%02x\n",
+ KBUILD_MODNAME, state->revcode, state->chip_type);
+
+ if (state->revcode != 0x03) {
+ dev_err(&state->i2c->dev, "%s: Unsupported hardware revision.\n",
+ KBUILD_MODNAME);
+ goto error;
+ }
+
+ /* It is a guess that register 0x0118 (chip type?) can be used to
+ * differ between si2161, si2163 and si2165
+ * Only si2165 has been tested.
+ */
+ if (state->chip_type == 0x07) {
+ state->m_has_dvbt = true;
+ state->m_has_dvbc = true;
+ strcpy(state->frontend.ops.info.name, "SI2165");
+ } else {
+ dev_err(&state->i2c->dev, "%s: Unsupported Chip type.\n",
+ KBUILD_MODNAME);
+ goto error;
+ }
+
+ n = 0;
+ if (state->m_has_dvbt) {
+ state->frontend.ops.delsys[n++] = SYS_DVBT;
+ strlcat(state->frontend.ops.info.name, " DVB-T",
+ sizeof(state->frontend.ops.info.name));
+ }
+ if (state->m_has_dvbc)
+ dev_warn(&state->i2c->dev, "%s: DVB-C is not yet supported.\n",
+ KBUILD_MODNAME);
+
+ return &state->frontend;
+
+error:
+ kfree(state);
+ return NULL;
+}
+EXPORT_SYMBOL(si2165_attach);
+
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Turn on/off frontend debugging (default:off).");
+
+MODULE_DESCRIPTION("Silicon Labs si2165 DVB-C/-T Demodulator driver");
+MODULE_AUTHOR("Matthias Schwarzott <zzam@gentoo.org>");
+MODULE_LICENSE("GPL");
+MODULE_FIRMWARE(SI2165_FIRMWARE);
diff --git a/drivers/media/dvb-frontends/si2165.h b/drivers/media/dvb-frontends/si2165.h
new file mode 100644
index 0000000..cdc12e7
--- /dev/null
+++ b/drivers/media/dvb-frontends/si2165.h
@@ -0,0 +1,61 @@
+/*
+ Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
+
+ Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
+
+ 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.
+
+ References:
+ http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
+*/
+
+#ifndef _DVB_SI2165_H
+#define _DVB_SI2165_H
+
+#include <linux/dvb/frontend.h>
+
+#if IS_ENABLED(CONFIG_DVB_SI2165)
+
+enum {
+ SI2165_MODE_OFF = 0x00,
+ SI2165_MODE_PLL_EXT = 0x20,
+ SI2165_MODE_PLL_XTAL = 0x21
+};
+
+struct si2165_config {
+ /* i2c addr
+ * possible values: 0x64,0x65,0x66,0x67 */
+ u8 i2c_addr;
+
+ /* external clock or XTAL */
+ u8 chip_mode;
+
+ /* frequency of external clock or xtal in Mhz
+ * possible values: 4,16,20,24,27 in
+ */
+ u8 ref_freq_MHz;
+};
+
+/* Addresses: 0x64,0x65,0x66,0x67 */
+struct dvb_frontend *si2165_attach(
+ const struct si2165_config *config,
+ struct i2c_adapter *i2c);
+#else
+static inline struct dvb_frontend *si2165_attach(
+ const struct si2165_config *config,
+ struct i2c_adapter *i2c)
+{
+ pr_warn("%s: driver disabled by Kconfig\n", __func__);
+ return NULL;
+}
+#endif /* CONFIG_DVB_SI2165 */
+
+#endif /* _DVB_SI2165_H */
diff --git a/drivers/media/dvb-frontends/si2165_priv.h b/drivers/media/dvb-frontends/si2165_priv.h
new file mode 100644
index 0000000..d4cc93f
--- /dev/null
+++ b/drivers/media/dvb-frontends/si2165_priv.h
@@ -0,0 +1,23 @@
+/*
+ Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
+
+ Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
+
+ 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.
+
+*/
+
+#ifndef _DVB_SI2165_PRIV
+#define _DVB_SI2165_PRIV
+
+#define SI2165_FIRMWARE "dvb-demod-si2165.fw"
+
+#endif /* _DVB_SI2165_PRIV */
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] cx231xx: Add [2040:b130] Hauppauge WinTV 930C-hd 1113xx
2014-04-26 20:21 [PATCH 1/3] si2165: Add demod driver for DVB-T only Matthias Schwarzott
@ 2014-04-26 20:21 ` Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 3/3] cx23885: Add si2165 support for HVR-5500 Matthias Schwarzott
2014-05-17 2:14 ` [PATCH 1/3] si2165: Add demod driver for DVB-T only Antti Palosaari
2 siblings, 0 replies; 6+ messages in thread
From: Matthias Schwarzott @ 2014-04-26 20:21 UTC (permalink / raw)
To: linux-media; +Cc: zzam, crope, xpert-reactos
Reading eeprom works
Analog is untested
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
---
drivers/media/usb/cx231xx/Kconfig | 1 +
drivers/media/usb/cx231xx/cx231xx-avcore.c | 1 +
drivers/media/usb/cx231xx/cx231xx-cards.c | 92 ++++++++++++++++++++++++++++++
drivers/media/usb/cx231xx/cx231xx-core.c | 3 +
drivers/media/usb/cx231xx/cx231xx-dvb.c | 34 +++++++++++
drivers/media/usb/cx231xx/cx231xx.h | 1 +
6 files changed, 132 insertions(+)
diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index f14c5e8..036454e 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -47,6 +47,7 @@ config VIDEO_CX231XX_DVB
select DVB_MB86A20S if MEDIA_SUBDRV_AUTOSELECT
select DVB_LGDT3305 if MEDIA_SUBDRV_AUTOSELECT
select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
+ select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
---help---
This adds support for DVB cards based on the
diff --git a/drivers/media/usb/cx231xx/cx231xx-avcore.c b/drivers/media/usb/cx231xx/cx231xx-avcore.c
index 89de00b..a428c10 100644
--- a/drivers/media/usb/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/usb/cx231xx/cx231xx-avcore.c
@@ -352,6 +352,7 @@ int cx231xx_afe_update_power_control(struct cx231xx *dev,
case CX231XX_BOARD_CNXT_RDU_253S:
case CX231XX_BOARD_CNXT_VIDEO_GRABBER:
case CX231XX_BOARD_HAUPPAUGE_EXETER:
+ case CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx:
case CX231XX_BOARD_HAUPPAUGE_USBLIVE2:
case CX231XX_BOARD_PV_PLAYTV_USB_HYBRID:
case CX231XX_BOARD_HAUPPAUGE_USB2_FM_PAL:
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 2ee03e4..ba4c4cb 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -704,6 +704,45 @@ struct cx231xx_board cx231xx_boards[] = {
}
},
},
+ [CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx] = {
+ .name = "Hauppauge WinTV 930C-HD (1113xx)",
+ .tuner_type = TUNER_NXP_TDA18271,
+ .tuner_addr = 0x60,
+ .tuner_gpio = RDE250_XCV_TUNER,
+ .tuner_sif_gpio = 0x05,
+ .tuner_scl_gpio = 0x1a,
+ .tuner_sda_gpio = 0x1b,
+ .decoder = CX231XX_AVDECODER,
+ .output_mode = OUT_MODE_VIP11,
+ .demod_xfer_mode = 0,
+ .ctl_pin_status_mask = 0xFFFFFFC4,
+ .agc_analog_digital_select_gpio = 0x0c,
+ .gpio_pin_status_mask = 0x4001000,
+ .tuner_i2c_master = 1,
+ .demod_i2c_master = 2,
+ .has_dvb = 1,
+ .demod_addr = 0x0e,
+ .norm = V4L2_STD_PAL,
+
+ .input = {{
+ .type = CX231XX_VMUX_TELEVISION,
+ .vmux = CX231XX_VIN_3_1,
+ .amux = CX231XX_AMUX_VIDEO,
+ .gpio = NULL,
+ }, {
+ .type = CX231XX_VMUX_COMPOSITE1,
+ .vmux = CX231XX_VIN_2_1,
+ .amux = CX231XX_AMUX_LINE_IN,
+ .gpio = NULL,
+ }, {
+ .type = CX231XX_VMUX_SVIDEO,
+ .vmux = CX231XX_VIN_1_1 |
+ (CX231XX_VIN_1_2 << 8) |
+ CX25840_SVIDEO_ON,
+ .amux = CX231XX_AMUX_LINE_IN,
+ .gpio = NULL,
+ } },
+ },
};
const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
@@ -733,6 +772,8 @@ struct usb_device_id cx231xx_id_table[] = {
.driver_info = CX231XX_BOARD_HAUPPAUGE_USB2_FM_NTSC},
{USB_DEVICE(0x2040, 0xb120),
.driver_info = CX231XX_BOARD_HAUPPAUGE_EXETER},
+ {USB_DEVICE(0x2040, 0xb130),
+ .driver_info = CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx},
{USB_DEVICE(0x2040, 0xb140),
.driver_info = CX231XX_BOARD_HAUPPAUGE_EXETER},
{USB_DEVICE(0x2040, 0xc200),
@@ -886,6 +927,48 @@ static void cx231xx_config_tuner(struct cx231xx *dev)
}
+static int read_eeprom(struct cx231xx *dev, u8 *eedata, int len)
+{
+ int ret = 0;
+ u8 addr = 0xa0 >> 1;
+ u8 start_offset = 0;
+ int len_todo = len;
+ u8 *eedata_cur = eedata;
+ int i;
+ struct i2c_msg msg_write = { .addr = addr, .flags = 0,
+ .buf = &start_offset, .len = 1 };
+ struct i2c_msg msg_read = { .addr = addr, .flags = I2C_M_RD };
+
+ /* mutex_lock(&dev->i2c_lock); */
+ cx231xx_enable_i2c_port_3(dev, false);
+
+ /* start reading at offset 0 */
+ ret = i2c_transfer(&dev->i2c_bus[1].i2c_adap, &msg_write, 1);
+
+ while (len_todo > 0) {
+ msg_read.len = (len_todo > 64) ? 64 : len_todo;
+ msg_read.buf = eedata_cur;
+
+ ret = i2c_transfer(&dev->i2c_bus[1].i2c_adap, &msg_read, 1);
+
+ eedata_cur += msg_read.len;
+ len_todo -= msg_read.len;
+ }
+
+ cx231xx_enable_i2c_port_3(dev, true);
+ /* mutex_unlock(&dev->i2c_lock); */
+
+ for (i = 0; i < len; i++) {
+ if (0 == (i % 16))
+ printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
+ printk(KERN_CONT " %02x", eedata[i]);
+ if (15 == (i % 16))
+ printk(KERN_CONT "\n");
+ }
+
+ return 0;
+}
+
void cx231xx_card_setup(struct cx231xx *dev)
{
@@ -917,6 +1000,15 @@ void cx231xx_card_setup(struct cx231xx *dev)
else
cx231xx_config_tuner(dev);
}
+
+ if (dev->model == CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx) {
+ struct tveeprom tvee;
+ static u8 eeprom[256];
+ read_eeprom(dev, eeprom, sizeof(eeprom));
+ tveeprom_hauppauge_analog(&dev->i2c_bus[1].i2c_client,
+ &tvee, eeprom + 0xc0);
+ }
+
}
/*
diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
index 4ba3ce0..513194a 100644
--- a/drivers/media/usb/cx231xx/cx231xx-core.c
+++ b/drivers/media/usb/cx231xx/cx231xx-core.c
@@ -726,6 +726,7 @@ int cx231xx_set_mode(struct cx231xx *dev, enum cx231xx_mode set_mode)
errCode = cx231xx_set_agc_analog_digital_mux_select(dev, 1);
break;
case CX231XX_BOARD_HAUPPAUGE_EXETER:
+ case CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx:
errCode = cx231xx_set_power_mode(dev,
POLARIS_AVMODE_DIGITAL);
break;
@@ -744,6 +745,7 @@ int cx231xx_set_mode(struct cx231xx *dev, enum cx231xx_mode set_mode)
case CX231XX_BOARD_CNXT_RDE_253S:
case CX231XX_BOARD_CNXT_RDU_253S:
case CX231XX_BOARD_HAUPPAUGE_EXETER:
+ case CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx:
case CX231XX_BOARD_PV_PLAYTV_USB_HYBRID:
case CX231XX_BOARD_HAUPPAUGE_USB2_FM_PAL:
case CX231XX_BOARD_HAUPPAUGE_USB2_FM_NTSC:
@@ -1379,6 +1381,7 @@ int cx231xx_dev_init(struct cx231xx *dev)
case CX231XX_BOARD_CNXT_RDE_253S:
case CX231XX_BOARD_CNXT_RDU_253S:
case CX231XX_BOARD_HAUPPAUGE_EXETER:
+ case CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx:
case CX231XX_BOARD_PV_PLAYTV_USB_HYBRID:
case CX231XX_BOARD_HAUPPAUGE_USB2_FM_PAL:
case CX231XX_BOARD_HAUPPAUGE_USB2_FM_NTSC:
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 4504bc6..d367796 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -32,6 +32,7 @@
#include "tda18271.h"
#include "s5h1411.h"
#include "lgdt3305.h"
+#include "si2165.h"
#include "mb86a20s.h"
MODULE_DESCRIPTION("driver for cx231xx based DVB cards");
@@ -151,6 +152,12 @@ static struct tda18271_config pv_tda18271_config = {
.small_i2c = TDA18271_03_BYTE_CHUNK_INIT,
};
+static const struct si2165_config hauppauge_930C_HD_1113xx_si2165_config = {
+ .i2c_addr = 0x64,
+ .chip_mode = SI2165_MODE_PLL_XTAL,
+ .ref_freq_MHz = 16,
+};
+
static inline void print_err_status(struct cx231xx *dev, int packet, int status)
{
char *errmsg = "Unknown";
@@ -704,6 +711,33 @@ static int dvb_init(struct cx231xx *dev)
&hcw_tda18271_config);
break;
+ case CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx:
+
+ printk(KERN_INFO "%s: looking for tuner / demod on i2c bus: %d\n",
+ __func__, i2c_adapter_id(&dev->i2c_bus[dev->board.tuner_i2c_master].i2c_adap));
+
+ dev->dvb->frontend = dvb_attach(si2165_attach,
+ &hauppauge_930C_HD_1113xx_si2165_config,
+ &dev->i2c_bus[dev->board.tuner_i2c_master].i2c_adap
+ );
+
+ if (dev->dvb->frontend == NULL) {
+ printk(DRIVER_NAME
+ ": Failed to attach SI2165 front end\n");
+ result = -EINVAL;
+ goto out_free;
+ }
+
+ dev->dvb->frontend->ops.i2c_gate_ctrl = 0;
+
+ /* define general-purpose callback pointer */
+ dvb->frontend->callback = cx231xx_tuner_callback;
+
+ dvb_attach(tda18271_attach, dev->dvb->frontend,
+ 0x60, &dev->i2c_bus[dev->board.tuner_i2c_master].i2c_adap,
+ &hcw_tda18271_config);
+ break;
+
case CX231XX_BOARD_PV_PLAYTV_USB_HYBRID:
case CX231XX_BOARD_KWORLD_UB430_USB_HYBRID:
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index babca7f..a6373ba 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -73,6 +73,7 @@
#define CX231XX_BOARD_ELGATO_VIDEO_CAPTURE_V2 16
#define CX231XX_BOARD_OTG102 17
#define CX231XX_BOARD_KWORLD_UB445_USB_HYBRID 18
+#define CX231XX_BOARD_HAUPPAUGE_930C_HD_1113xx 19
/* Limits minimum and default number of buffers */
#define CX231XX_MIN_BUF 4
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] cx23885: Add si2165 support for HVR-5500
2014-04-26 20:21 [PATCH 1/3] si2165: Add demod driver for DVB-T only Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 2/3] cx231xx: Add [2040:b130] Hauppauge WinTV 930C-hd 1113xx Matthias Schwarzott
@ 2014-04-26 20:21 ` Matthias Schwarzott
2014-05-17 2:14 ` [PATCH 1/3] si2165: Add demod driver for DVB-T only Antti Palosaari
2 siblings, 0 replies; 6+ messages in thread
From: Matthias Schwarzott @ 2014-04-26 20:21 UTC (permalink / raw)
To: linux-media; +Cc: zzam, crope, xpert-reactos
The same card entry is used for HVR-4400 and HVR-5500.
Only HVR-5500 has been tested.
Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
---
drivers/media/pci/cx23885/Kconfig | 1 +
drivers/media/pci/cx23885/cx23885-cards.c | 17 +++++++++---
drivers/media/pci/cx23885/cx23885-dvb.c | 43 +++++++++++++++++++++++++++----
3 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/drivers/media/pci/cx23885/Kconfig b/drivers/media/pci/cx23885/Kconfig
index d1dcb1d..6cd1db2 100644
--- a/drivers/media/pci/cx23885/Kconfig
+++ b/drivers/media/pci/cx23885/Kconfig
@@ -31,6 +31,7 @@ config VIDEO_CX23885
select DVB_TDA10071 if MEDIA_SUBDRV_AUTOSELECT
select DVB_A8293 if MEDIA_SUBDRV_AUTOSELECT
select DVB_MB86A20S if MEDIA_SUBDRV_AUTOSELECT
+ select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_MT2063 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_MT2131 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_XC2028 if MEDIA_SUBDRV_AUTOSELECT
diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 79f20c8..6ed0551 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -619,7 +619,12 @@ struct cx23885_board cx23885_boards[] = {
},
[CX23885_BOARD_HAUPPAUGE_HVR4400] = {
.name = "Hauppauge WinTV-HVR4400",
+ .porta = CX23885_ANALOG_VIDEO,
.portb = CX23885_MPEG_DVB,
+ .portc = CX23885_MPEG_DVB,
+ .tuner_type = TUNER_NXP_TDA18271,
+ .tuner_addr = 0x60, /* 0xc0 >> 1 */
+ .tuner_bus = 1,
},
[CX23885_BOARD_AVERMEDIA_HC81R] = {
.name = "AVerTV Hybrid Express Slim HC81R",
@@ -1449,13 +1454,16 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_HAUPPAUGE_HVR4400:
/* GPIO-8 tda10071 demod reset */
+ /* GPIO-9 si2165 demod reset */
/* Put the parts into reset and back */
- cx23885_gpio_enable(dev, GPIO_8, 1);
- cx23885_gpio_clear(dev, GPIO_8);
+ cx23885_gpio_enable(dev, GPIO_8 | GPIO_9, 1);
+
+ cx23885_gpio_clear(dev, GPIO_8 | GPIO_9);
mdelay(100);
- cx23885_gpio_set(dev, GPIO_8);
+ cx23885_gpio_set(dev, GPIO_8 | GPIO_9);
mdelay(100);
+
break;
case CX23885_BOARD_AVERMEDIA_HC81R:
cx_clear(MC417_CTL, 1);
@@ -1799,6 +1807,9 @@ void cx23885_card_setup(struct cx23885_dev *dev)
ts1->gen_ctrl_val = 0xc; /* Serial bus + punctured clock */
ts1->ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts1->src_sel_val = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
+ ts2->gen_ctrl_val = 0xc; /* Serial bus + punctured clock */
+ ts2->ts_clk_en_val = 0x1; /* Enable TS_CLK */
+ ts2->src_sel_val = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
break;
case CX23885_BOARD_HAUPPAUGE_HVR1250:
case CX23885_BOARD_HAUPPAUGE_HVR1500:
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 4be01b3..ddb0e82 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -71,6 +71,7 @@
#include "tda10071.h"
#include "a8293.h"
#include "mb86a20s.h"
+#include "si2165.h"
static unsigned int debug;
@@ -302,6 +303,11 @@ static struct tda18271_config hauppauge_hvr1210_tuner_config = {
.output_opt = TDA18271_OUTPUT_LT_OFF,
};
+static struct tda18271_config hauppauge_hvr4400_tuner_config = {
+ .gate = TDA18271_GATE_DIGITAL,
+ .output_opt = TDA18271_OUTPUT_LT_OFF,
+};
+
static struct tda18271_std_map hauppauge_hvr127x_std_map = {
.atsc_6 = { .if_freq = 3250, .agc_mode = 3, .std = 4,
.if_lvl = 1, .rfagc_top = 0x58 },
@@ -702,6 +708,12 @@ static const struct a8293_config hauppauge_a8293_config = {
.i2c_addr = 0x0b,
};
+static const struct si2165_config hauppauge_hvr4400_si2165_config = {
+ .i2c_addr = 0x64,
+ .chip_mode = SI2165_MODE_PLL_XTAL,
+ .ref_freq_MHz = 16,
+};
+
static int netup_altera_fpga_rw(void *device, int flag, int data, int read)
{
struct cx23885_dev *dev = (struct cx23885_dev *)device;
@@ -1331,13 +1343,34 @@ static int dvb_register(struct cx23885_tsport *port)
break;
case CX23885_BOARD_HAUPPAUGE_HVR4400:
i2c_bus = &dev->i2c_bus[0];
- fe0->dvb.frontend = dvb_attach(tda10071_attach,
+ i2c_bus2 = &dev->i2c_bus[1];
+ switch (port->nr) {
+ /* port b */
+ case 1:
+ fe0->dvb.frontend = dvb_attach(tda10071_attach,
&hauppauge_tda10071_config,
&i2c_bus->i2c_adap);
- if (fe0->dvb.frontend != NULL) {
- dvb_attach(a8293_attach, fe0->dvb.frontend,
- &i2c_bus->i2c_adap,
- &hauppauge_a8293_config);
+ if (fe0->dvb.frontend != NULL) {
+ if (!dvb_attach(a8293_attach, fe0->dvb.frontend,
+ &i2c_bus->i2c_adap,
+ &hauppauge_a8293_config))
+ goto frontend_detach;
+ }
+ break;
+ /* port c */
+ case 2:
+ fe0->dvb.frontend = dvb_attach(si2165_attach,
+ &hauppauge_hvr4400_si2165_config,
+ &i2c_bus->i2c_adap);
+ if (fe0->dvb.frontend != NULL) {
+ fe0->dvb.frontend->ops.i2c_gate_ctrl = 0;
+ if (!dvb_attach(tda18271_attach,
+ fe0->dvb.frontend,
+ 0x60, &i2c_bus2->i2c_adap,
+ &hauppauge_hvr4400_tuner_config))
+ goto frontend_detach;
+ }
+ break;
}
break;
default:
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] si2165: Add demod driver for DVB-T only
2014-04-26 20:21 [PATCH 1/3] si2165: Add demod driver for DVB-T only Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 2/3] cx231xx: Add [2040:b130] Hauppauge WinTV 930C-hd 1113xx Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 3/3] cx23885: Add si2165 support for HVR-5500 Matthias Schwarzott
@ 2014-05-17 2:14 ` Antti Palosaari
2014-07-01 19:01 ` Matthias Schwarzott
2 siblings, 1 reply; 6+ messages in thread
From: Antti Palosaari @ 2014-05-17 2:14 UTC (permalink / raw)
To: Matthias Schwarzott, linux-media; +Cc: xpert-reactos
Sorry for the late review. I think that will be skipped over 2.16 as too
late...
There is also many other DTV drivers coming from Earthsoft PT3 support
waiting for review. Anyone else willing to review? I wonder how we could
improve situation, we are simply lack of reviewers. Mike, Devin, Mauro?
On 04/26/2014 11:21 PM, Matthias Schwarzott wrote:
> DVB-T was tested with 8MHz BW channels in germany
> This driver is the simplest possible, it uses automatic mode for all
> parameters (TPS).
>
> Firmware file can be extracted via get_dvb_firmware.
>
> Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
> ---
> Documentation/dvb/get_dvb_firmware | 33 +-
> drivers/media/dvb-frontends/Kconfig | 9 +
> drivers/media/dvb-frontends/Makefile | 1 +
> drivers/media/dvb-frontends/si2165.c | 890 ++++++++++++++++++++++++++++++
> drivers/media/dvb-frontends/si2165.h | 61 ++
> drivers/media/dvb-frontends/si2165_priv.h | 23 +
> 6 files changed, 1016 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/dvb-frontends/si2165.c
> create mode 100644 drivers/media/dvb-frontends/si2165.h
> create mode 100644 drivers/media/dvb-frontends/si2165_priv.h
>
> diff --git a/Documentation/dvb/get_dvb_firmware b/Documentation/dvb/get_dvb_firmware
> index d91b8be..26c623d 100755
> --- a/Documentation/dvb/get_dvb_firmware
> +++ b/Documentation/dvb/get_dvb_firmware
> @@ -29,7 +29,7 @@ use IO::Handle;
> "af9015", "ngene", "az6027", "lme2510_lg", "lme2510c_s7395",
> "lme2510c_s7395_old", "drxk", "drxk_terratec_h5",
> "drxk_hauppauge_hvr930c", "tda10071", "it9135", "drxk_pctv",
> - "drxk_terratec_htc_stick", "sms1xxx_hcw");
> + "drxk_terratec_htc_stick", "sms1xxx_hcw", "si2165");
>
> # Check args
> syntax() if (scalar(@ARGV) != 1);
> @@ -783,6 +783,37 @@ sub sms1xxx_hcw {
> $allfiles;
> }
>
> +sub si2165 {
> + my $sourcefile = "model_111xxx_122xxx_driver_6_0_119_31191_WHQL.zip";
> + my $url = "http://www.hauppauge.de/files/drivers/";
> + my $hash = "76633e7c76b0edee47c3ba18ded99336";
> + my $fwfile = "dvb-demod-si2165.fw";
> + my $tmpdir = tempdir(DIR => "/tmp", CLEANUP => 1);
> +
> + checkstandard();
> +
> + wgetfile($sourcefile, $url . $sourcefile);
> + verify($sourcefile, $hash);
> + unzip($sourcefile, $tmpdir);
> + extract("$tmpdir/Driver10/Hcw10bda.sys", 0x80788, 0x81E08-0x80788, "$tmpdir/fw1");
> +
> + delzero("$tmpdir/fw1","$tmpdir/fw1-1");
> + #verify("$tmpdir/fw1","5e0909858fdf0b5b09ad48b9fe622e70");
> +
> + my $CRC="\x0A\xCC";
> + my $BLOCKS_MAIN="\x27";
> + open FW,">$fwfile";
> + print FW "\x01\x00"; # just a version id for the driver itself
> + print FW "\x9A"; # fw version
> + print FW "\x00"; # padding
> + print FW "$BLOCKS_MAIN"; # number of blocks of main part
> + print FW "\x00"; # padding
> + print FW "$CRC"; # 16bit crc value of main part
> + appendfile(FW,"$tmpdir/fw1");
> +
> + "$fwfile";
> +}
> +
> # ---------------------------------------------------------------
> # Utilities
Separate that firmware extractor to own patch.
>
> diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
> index 1469d44..0da53c2 100644
> --- a/drivers/media/dvb-frontends/Kconfig
> +++ b/drivers/media/dvb-frontends/Kconfig
> @@ -63,6 +63,15 @@ config DVB_TDA18271C2DD
>
> Say Y when you want to support this tuner.
>
> +config DVB_SI2165
> + tristate "Silicon Labs si2165 based"
> + depends on DVB_CORE && I2C
> + default m if !MEDIA_SUBDRV_AUTOSELECT
> + help
> + A DVB-C/T demodulator.
> +
> + Say Y when you want to support this frontend.
> +
> comment "DVB-S (satellite) frontends"
> depends on DVB_CORE
>
> diff --git a/drivers/media/dvb-frontends/Makefile b/drivers/media/dvb-frontends/Makefile
> index dda0bee..595dd8d 100644
> --- a/drivers/media/dvb-frontends/Makefile
> +++ b/drivers/media/dvb-frontends/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_DVB_STV0367) += stv0367.o
> obj-$(CONFIG_DVB_CXD2820R) += cxd2820r.o
> obj-$(CONFIG_DVB_DRXK) += drxk.o
> obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
> +obj-$(CONFIG_DVB_SI2165) += si2165.o
> obj-$(CONFIG_DVB_A8293) += a8293.o
> obj-$(CONFIG_DVB_TDA10071) += tda10071.o
> obj-$(CONFIG_DVB_RTL2830) += rtl2830.o
> diff --git a/drivers/media/dvb-frontends/si2165.c b/drivers/media/dvb-frontends/si2165.c
> new file mode 100644
> index 0000000..3e69a32
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/si2165.c
> @@ -0,0 +1,890 @@
> +/*
> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
> +
> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
> +
> + 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.
> +
> + References:
> + http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
> +*/
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/firmware.h>
> +
> +#include "dvb_frontend.h"
> +#include "dvb_math.h"
> +#include "si2165_priv.h"
> +#include "si2165.h"
> +
> +/* Hauppauge WinTV-HVR-930C-HD 1113xx uses 16.000 MHz xtal */
> +
> +struct si2165_state {
> + struct i2c_adapter *i2c;
> +
> + struct dvb_frontend frontend;
> +
> + struct si2165_config config;
> +
> + /* chip revision */
> + u8 revcode;
> + /* chip type */
> + u8 chip_type;
> +
> + /* calculated by xtal and div settings */
> + u32 fvco_hz;
> + u32 sys_clk;
> + u32 adc_clk;
> +
> + /* depends on adc_clk and Ovr mode */
> + u32 fe_clk;
> +
> + bool m_has_dvbc;
> + bool m_has_dvbt;
m_ prefix is for Hungarian notation of member variables. It is not
suitable for Kernel style.
> + bool firmware_loaded;
> +};
> +
> +#define DEBUG_OTHER 0x01
> +#define DEBUG_I2C_WRITE 0x02
> +#define DEBUG_I2C_READ 0x04
> +#define DEBUG_REG_READ 0x08
> +#define DEBUG_REG_WRITE 0x10
> +#define DEBUG_FW_LOAD 0x20
> +
> +static int debug = 0x00;
> +
> +#define dprintk(args...) \
> + do { \
> + if (debug & DEBUG_OTHER) \
> + printk(KERN_DEBUG "si2165: " args); \
> + } while (0)
> +
> +#define deb_i2c_write(args...) \
> + do { \
> + if (debug & DEBUG_I2C_WRITE) \
> + printk(KERN_DEBUG "si2165: i2c write: " args); \
> + } while (0)
> +
> +#define deb_i2c_read(args...) \
> + do { \
> + if (debug & DEBUG_I2C_READ) \
> + printk(KERN_DEBUG "si2165: i2c read: " args); \
> + } while (0)
> +
> +#define deb_readreg(args...) \
> + do { \
> + if (debug & DEBUG_REG_READ) \
> + printk(KERN_DEBUG "si2165: reg read: " args); \
> + } while (0)
> +
> +#define deb_writereg(args...) \
> + do { \
> + if (debug & DEBUG_REG_WRITE) \
> + printk(KERN_DEBUG "si2165: reg write: " args); \
> + } while (0)
> +
> +#define deb_fw_load(args...) \
> + do { \
> + if (debug & DEBUG_FW_LOAD) \
> + printk(KERN_DEBUG "si2165: fw load: " args); \
> + } while (0)
Could you consider kernel dynamic debugs which are todays norm? See for
example si2168 driver as a example. IMHO it is not hard requirement, but
still...
> +
> +static int si2165_write(struct si2165_state *state, const u16 reg,
> + const u8 *src, const int count)
> +{
> + int ret;
> + struct i2c_msg msg;
> + u8 buf[2 + 4]; /* write a maximum of 4 bytes of data */
Here should be empty line between variable definitions and code. IIRC
latest checkpatch.pl catch that.
> + if (count + 2 > sizeof(buf)) {
> + dev_warn(&state->i2c->dev,
> + "%s: i2c wr reg=%04x: count=%d is too big!\n",
> + KBUILD_MODNAME, reg, count);
> + return -EINVAL;
> + }
> + buf[0] = reg >> 8;
> + buf[1] = reg & 0xff;
> + memcpy(buf + 2, src, count);
> +
> + msg.addr = state->config.i2c_addr;
> + msg.flags = 0;
> + msg.buf = buf;
> + msg.len = count + 2;
> +
> + if (debug & DEBUG_I2C_WRITE) {
> + int i;
> + deb_i2c_write("reg: 0x%04x, data:", reg);
> + for (i = 0; i < count; i++)
> + printk(KERN_CONT " %02x", src[i]);
> + printk(KERN_CONT "\n");
> + }
> +
> + ret = i2c_transfer(state->i2c, &msg, 1);
> +
> + if (ret != 1) {
> + dev_err(&state->i2c->dev, "%s: ret == %d\n", __func__, ret);
> + return -EREMOTEIO;
> + }
Could be nice to return error from i2c_transfer().
> +
> + return 0;
> +}
> +
> +static int si2165_read(struct si2165_state *state,
> + const u16 reg, u8 *val, const size_t count)
You used int on si2165_write(), try to make decision which one is
correct and keep it.
> +{
> + int ret;
> + u8 reg_buf[] = { reg >> 8, reg & 0xff };
> + struct i2c_msg msg[] = {
> + { .addr = state->config.i2c_addr,
> + .flags = 0, .buf = reg_buf, .len = 2 },
> + { .addr = state->config.i2c_addr,
> + .flags = I2C_M_RD, .buf = val, .len = count },
> + };
> +
> + ret = i2c_transfer(state->i2c, msg, 2);
> +
> + if (ret != 2) {
> + dev_err(&state->i2c->dev, "%s: error (addr %02x reg %04x error (ret == %i)\n",
> + __func__, state->config.i2c_addr, reg, ret);
> + if (ret < 0)
> + return ret;
> + else
> + return -EREMOTEIO;
> + }
> +
> + if (debug & DEBUG_I2C_READ) {
> + int i;
> + deb_i2c_read("reg: 0x%04x, data:", reg);
> + for (i = 0; i < count; i++)
> + printk(KERN_CONT " %02x", val[i]);
> + printk(KERN_CONT "\n");
> + }
There is formatter %pH (or something like) for that.
> + return 0;
> +}
> +
> +static int si2165_readreg8(struct si2165_state *state,
> + const u16 reg, u8 *val)
> +{
> + int ret;
> + ret = si2165_read(state, reg, val, 1);
> + deb_readreg("R(0x%04x)=0x%02x\n", reg, *val);
> + return ret;
> +}
> +
> +static int si2165_readreg16(struct si2165_state *state,
> + const u16 reg, u16 *val)
> +{
> + u8 buf[2];
> + int ret = si2165_read(state, reg, buf, 2);
> + *val = buf[0] | buf[1] << 8;
> + deb_readreg("R(0x%04x)=0x%04x\n", reg, *val);
> + return ret;
> +}
> +
> +#if 0
> +static int si2165_readreg24(struct si2165_state *state,
> + const u16 reg, u32 *val)
> +{
> + u8 buf[3];
> + int ret = si2165_read(state, reg, buf, 3);
> + *val = buf[0] | buf[1] << 8 | buf[2] << 16;
> + deb_readreg("R(0x%04x)=0x%06x\n", reg, *val);
> + return ret;
> +}
> +
> +static int si2165_readreg32(struct si2165_state *state,
> + const u16 reg, u32 *val)
> +{
> + u8 buf[4];
> + int ret = si2165_read(state, reg, buf, 4);
> + *val = buf[0] | buf[1] << 8 | buf[2] << 16 | buf[3] << 24;
> + deb_readreg("R(0x%04x)=0x%08x\n", reg, *val);
> + return ret;
> +}
> +#endif
Personally, I don't like presenting shorthand functions for multi-byte
readings like that. But if you think it is good then leave it.
> +
> +
> +static int si2165_writereg8(struct si2165_state *state, const u16 reg, u8 val)
> +{
> + return si2165_write(state, reg, &val, 1);
> +}
> +
> +static int si2165_writereg16(struct si2165_state *state, const u16 reg, u16 val)
> +{
> + u8 buf[2] = { val & 0xff, (val >> 8) & 0xff };
> + return si2165_write(state, reg, buf, 2);
> +}
> +
> +static int si2165_writereg24(struct si2165_state *state, const u16 reg, u32 val)
> +{
> + u8 buf[3] = { val & 0xff, (val >> 8) & 0xff, (val >> 16) & 0xff };
> + return si2165_write(state, reg, buf, 3);
> +}
> +
> +static int si2165_writereg32(struct si2165_state *state, const u16 reg, u32 val)
> +{
> + u8 buf[4] = {
> + val & 0xff,
> + (val >> 8) & 0xff,
> + (val >> 16) & 0xff,
> + (val >> 24) & 0xff
> + };
> + return si2165_write(state, reg, buf, 4);
> +}
> +
> +static int si2165_writereg_mask8(struct si2165_state *state, const u16 reg,
> + u8 val, u8 mask)
> +{
> + int ret;
> + u8 tmp;
> +
> + if (mask != 0xff) {
> + ret = si2165_readreg8(state, reg, &tmp);
> + if (ret < 0)
> + goto err;
> +
> + val &= mask;
> + tmp &= ~mask;
> + val |= tmp;
> + }
> +
> + ret = si2165_writereg8(state, reg, val);
> +err:
> + return ret;
> +}
> +
> +static int si2165_get_tune_settings(struct dvb_frontend *fe,
> + struct dvb_frontend_tune_settings *s)
> +{
> + s->min_delay_ms = 1000;
> + return 0;
> +}
> +
> +static int si2165_init_pll(struct si2165_state *state)
> +{
> + u8 ref_freq_MHz = state->config.ref_freq_MHz;
1MHz is quite big reference frequency step?
> + /* u8 val; */
excessive comment
> + u8 divr = 1; /* 1..7 */
> + u8 divp = 1; /* only 1 or 4 */
> + u8 divn = 56; /* 1..63 */
> +
excessive newline
> + u8 divm = 8;
> + u8 divl = 12;
> + u8 buf[4];
> +
> + /* ref_freq / divr must be between 4 and 16 MHz */
> + if (ref_freq_MHz > 16)
> + divr = 2;
> +
> + /* now select divn and divp such that fvco is in 1624..1824 MHz */
> + if (1624 * divr > ref_freq_MHz * 2 * 63)
> + divp = 4;
> +
> + /* to get exactly the same as the windows driver does */
> + if (ref_freq_MHz == 16)
> + divn = 56;
> + else {
> + /* is this already correct regarding rounding? */
> + divn = 1624 * divr / (ref_freq_MHz * 2 * divp);
> + }
That logic does not look correct. PLL dividers are usually calculated in
a order:
1) calculate reference divider (divREF)
2) calculate output divider (divOUT)
3) calculate VCO freq (fVCO = fOUT * divOUT)
4) calculate N & NF, pllN = fVCO * divREF / fREF;
N = interger part, NF = fractional part. NF only when PLL is Fractional N.
Biggest thing looks wrong is that: IF (fREF == 16MHz) THEN pllN = 56.
There should be parenthesis for none or both conditions. checkpatch.pl?
> +
> + /* adc_clk and sys_clk depend on xtal and pll settings */
> + /* only calculate once as long as the pll settings are not modified */
> + if (state->adc_clk == 0) {
> + u32 fvco_hz = ref_freq_MHz * 1000000ull / divr
> + * 2 * divn * divp;
> + state->adc_clk = fvco_hz / (divm * 4);
> + state->sys_clk = fvco_hz / (divl * 2);
> + }
> +
> + /* write pll registers 0x00a0..0x00a3 at once */
> + buf[0] = divl;
> + buf[1] = divm;
> + buf[2] = (divn & 0x3f) | ((divp == 1) ? 0x40 : 0x00) | 0x80;
> + buf[3] = divr;
> + si2165_write(state, 0x00a0 /* first pll reg */, buf, 4);
That kinf of comments in a parameter list do not look typical for kernel
style.
> +
> + return 0;
> +}
Also that function is called from init(), maybe you could inline all
that stuff to that function instead.
> +
> +static bool si2165_wait_init_done(struct si2165_state *state)
> +{
> + int ret = false;
> + u8 val;
> + int i;
> + for (i = 0; i < 10; i++) {
> + si2165_readreg8(state, 0x0054 /* init_done */, &val);
> + if (val == 0x01) {
> + ret = true;
> + break;
> + }
> + msleep(1);
Too small msleep(), see timers howto. Also checkpatch.pl should be able
to find that too, you haven't ran it.
> + }
> + return ret;
> +}
Whole function is called from one place and it is very trivial, so that
function is mostly reduntant. Move logic to where it is now called.
Also, use jiffies for timeouts like that. See si2168 for example.
> +
> +static int si2165_upload_firmware_block(struct si2165_state *state, const u8 *data, u32 len, u32 *poffset, u32 block_count)
> +{
> + u8 buf_ctrl[4] = { 0x00, 0x00, 0x00, 0xc0 };
> + u8 wordcount;
> + u32 cur_block = 0;
> + u32 offset = poffset ? *poffset : 0;
> + if (len < 4)
> + return -EINVAL;
> + if (len % 4 != 0)
> + return -EINVAL;
> +
> + deb_fw_load("si2165_upload_firmware_block called with len=0x%x offset=0x%x blockcount=0x%x\n",
> + len, offset, block_count);
> + while (offset+12 <= len && cur_block < block_count) {
> + deb_fw_load("si2165_upload_firmware_block in while len=0x%x offset=0x%x cur_block=0x%x blockcount=0x%x\n",
> + len, offset, cur_block, block_count);
> + wordcount = data[offset];
> + if (wordcount < 1 || data[offset+1] || data[offset+2] || data[offset+3]) {
> + dev_warn(&state->i2c->dev, "%s: bad fw data[0..3] = %02x %02x %02x %02x\n", KBUILD_MODNAME, data[0], data[1], data[2], data[3]);
> + return -EINVAL;
> + }
> +
> + if (offset + 8 + wordcount * 4 > len) {
> + dev_warn(&state->i2c->dev, "%s: len is too small for block len=%d, wordcount=%d\n", KBUILD_MODNAME, len, wordcount);
> + return -EINVAL;
> + }
> +
> + buf_ctrl[0] = wordcount - 1;
> +
> + si2165_write(state, 0x0364, buf_ctrl, 4);
> + si2165_write(state, 0x0368, data+offset+4, 4);
CodingStyle. That driver has a lot of coding style mistakes ==> you must
run checkpatch.pl and fix all.
> +
> + offset += 8;
> +
> + while (wordcount > 0) {
> + si2165_write(state, 0x36c, data+offset, 4);
> + wordcount--;
> + offset += 4;
> + }
> + cur_block++;
> + }
> +
> + deb_fw_load("si2165_upload_firmware_block after while len=0x%x offset=0x%x cur_block=0x%x blockcount=0x%x\n",
> + len, offset, cur_block, block_count);
> +
> + if (poffset)
> + *poffset = offset;
> +
> + deb_fw_load("si2165_upload_firmware_block returned offset=0x%x\n",
> + offset);
> +
> + return 0;
> +}
> +
> +static int si2165_upload_firmware(struct si2165_state *state)
> +{
> + /* int ret; */
> + u8 val[3];
> + u16 val16;
> + int ret;
> +
> + const struct firmware *fw = NULL;
> + u8 *fw_file = SI2165_FIRMWARE;
> + const u8 *data;
> + u32 len;
> + u32 offset;
> + u8 patch_version;
> + u8 block_count;
> + u16 crc_expected;
> +
> + /* request the firmware, this will block and timeout */
> + ret = request_firmware(&fw, fw_file, state->i2c->dev.parent);
> + if (ret) {
> + dev_warn(&state->i2c->dev, "%s: firmare file '%s' not found\n",
> + KBUILD_MODNAME, fw_file);
> + goto err;
> + }
> +
> + data = fw->data;
> + len = fw->size;
> +
> + dev_info(&state->i2c->dev, "%s: downloading firmware from file '%s' size=%d\n",
> + KBUILD_MODNAME, fw_file, len);
> +
> + if (len % 4 != 0) {
> + dev_warn(&state->i2c->dev, "%s: firmware size is not multiple of 4\n",
> + KBUILD_MODNAME);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + /* check header (8 bytes) */
> + if (len < 8) {
> + dev_warn(&state->i2c->dev, "%s: firmware header is missing\n",
> + KBUILD_MODNAME);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + if (data[0] != 1 || data[1] != 0) {
> + dev_warn(&state->i2c->dev, "%s: firmware file version is wrong\n",
> + KBUILD_MODNAME);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + patch_version = data[2];
> + block_count = data[4];
> + crc_expected = data[7] << 8 | data[6];
> +
> + /* start uploading fw */
> + si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> +
> + si2165_readreg8(state, 0x035c /* en_rst_error */, val); /* returned 0x03 */
> + si2165_readreg8(state, 0x035c /* en_rst_error */, val); /* returned 0x03 */
> + si2165_writereg8(state, 0x035c /* en_rst_error */, 0x02);
> +
> + /* start right after the header */
> + offset = 8;
> +
> + dev_info(&state->i2c->dev, "%s: si2165_upload_firmware extracted patch_version=0x%02x, block_count=0x%02x, crc_expected=0x%04x\n",
> + KBUILD_MODNAME, patch_version, block_count, crc_expected);
> +
> + ret = si2165_upload_firmware_block(state, data, len, &offset, 1);
> +
> + si2165_writereg8(state, 0x0344 /* patch_version */, patch_version);
> +
> + ret = si2165_writereg8(state, 0x0379 /* rst_crc */, 0x01);
> + if (ret)
> + return ret;
> +
> + ret = si2165_upload_firmware_block(state, data, len, &offset, block_count);
> +
> + if (ret) {
> + dev_err(&state->i2c->dev, "%s: firmare could not be uploaded\n",
> + KBUILD_MODNAME);
> + goto err;
> + }
> +
> + ret = si2165_readreg16(state, 0x037a /* crc */, &val16); /* returned 0xcc0a */
> + if (ret)
> + goto err;
> +
> + if (val16 != crc_expected) {
> + dev_err(&state->i2c->dev, "%s: firmware crc mismatch %04x != %04x\n", KBUILD_MODNAME, val16, crc_expected);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = si2165_upload_firmware_block(state, data, len, &offset, 5);
> + if (ret)
> + goto err;
> +
> + if (len != offset) {
> + dev_err(&state->i2c->dev, "%s: firmare len mismatch %04x != %04x\n", KBUILD_MODNAME, len, offset);
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + /* reset watchdog error register, using auto return value rst_wdog_error */
> + si2165_writereg_mask8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x02, 0x02);
> +
> + /* enable reset on error */
> + si2165_writereg_mask8(state, 0x035c /* en_rst_error */, 0x01, 0x01);
> +
> + dev_info(&state->i2c->dev, "%s: fw load finished\n", KBUILD_MODNAME);
> +
> + ret = 0;
> + state->firmware_loaded = true;
> +err:
> + if (fw) {
> + release_firmware(fw);
> + fw = NULL;
> + }
> +
> + return ret;
> +}
That firmware downloading looks overall very long a also a bit complex.
I didn't looked it through carefully. Maybe you could take a look to
si2168 or af9035 to see if that could be shorten.
> +
> +static int si2165_init_dsp(struct si2165_state *state)
> +{
> + u8 val;
> + u8 patch_version = 0x00;
> +
> + si2165_readreg8(state, 0x0344 /* patch_version */, &patch_version); /* returned 0x00 */
> +
> + si2165_writereg8(state, 0x00cb, 0x00);
> + if (patch_version == 0x00)
> + si2165_writereg8(state, 0x0344 /* patch_version */, 0x00);
> +
> + si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, &val); /* returned 0x01 */
> +
> + if (patch_version == 0x00)
> + si2165_upload_firmware(state);
> +
> + return 0;
> +}
Maybe that trivial function could be integrated to where it is called.
> +
> +static int si2165_init(struct dvb_frontend *fe)
> +{
> + struct si2165_state *state = fe->demodulator_priv;
> + u8 val;
> +
> + dprintk("%s: called\n", __func__);
> +
> + /* powerup */
> + si2165_writereg8(state, 0x0000 /* chip_mode */, state->config.chip_mode);
> + si2165_writereg8(state, 0x0104 /* dsp_clock_enable */, 0x01);
> + si2165_readreg8(state, 0x0000 /* chip_mode */, &val);
> + if (val != state->config.chip_mode) {
> + dev_err(&state->i2c->dev, "%s: could not set chip_mode\n",
> + KBUILD_MODNAME);
> + return -EINVAL;
> + }
> +
> + si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
> + si2165_writereg8(state, 0x0190 /* agc_if_slr */, 0x01);
> + si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, &val); /* returned 0x00 */
> + si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, 0x00);
> + si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, &val); /* returned 0x00 */
> + si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */, 0x00);
> + si2165_writereg8(state, 0x0171 /* agc2_clkdiv */, 0x07);
> + si2165_writereg8(state, 0x0646 /* rssi_pad_ctrl */, 0x00);
> + si2165_readreg8(state, 0x0641 /* en_rssi,start_rssi,rssi_update_time */, &val); /* returned 0x00 */
> + si2165_writereg8(state, 0x0641 /* en_rssi,start_rssi,rssi_update_time */, 0x00);
> + si2165_writereg8(state, 0x00e0 /* adc_sampling_mode */, 0x00);
> +
> + si2165_init_pll(state);
> +
> + si2165_writereg8(state, 0x0050 /* chip_init */, 0x01);
> + si2165_writereg8(state, 0x0096 /* start_init */, 0x01);
> + if (!si2165_wait_init_done(state)) {
> + dev_err(&state->i2c->dev, "%s: init_done was not set\n",
> + KBUILD_MODNAME);
> + return -EINVAL;
> + }
> +
> + si2165_writereg8(state, 0x0050 /* chip_init */, 0x00);
> +
> + si2165_writereg16(state, 0x0470 /* ber_pkt */, 0x7530);
> +
> + si2165_init_dsp(state);
> +
> + si2165_writereg8(state, 0x012a /* adc_ri1 */, 0x46);
> + si2165_writereg8(state, 0x012c /* adc_ri3 */, 0x00);
> + si2165_writereg8(state, 0x012e /* adc_ri5 */, 0x0a);
> + si2165_writereg8(state, 0x012f /* adc_ri6 */, 0xff);
> + si2165_writereg8(state, 0x0123 /* adc_ri8 */, 0x70);
> +
> + return 0;
> +}
> +
> +static int si2165_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
> +{
> + struct si2165_state *state = fe->demodulator_priv;
> + u8 val = enable ? 0x01 : 0x00;
> + return si2165_writereg8(state, 0x0001 /* i2c passthru */, val);
> +}
> +
> +static int si2165_sleep(struct dvb_frontend *fe)
> +{
> + struct si2165_state *state = fe->demodulator_priv;
> + dprintk("%s: called\n", __func__);
> + si2165_writereg8(state, 0x0104 /* dsp clock enable */, 0x00);
> + si2165_writereg8(state, 0x0000 /* chip mode */, SI2165_MODE_OFF);
> + return 0;
> +}
> +
> +static int si2165_read_status(struct dvb_frontend *fe, fe_status_t *status)
> +{
> + u8 fec_lock = 0;
> + struct si2165_state *state = fe->demodulator_priv;
> +
> + if (!state->m_has_dvbt)
> + return -EINVAL;
> +
> + /* seq1 */
> + si2165_readreg8(state, 0x4e0 /* fec_lock */, &fec_lock);
> + *status = 0;
> + if (fec_lock & 0x01) {
> + *status |= FE_HAS_SIGNAL;
> + *status |= FE_HAS_CARRIER;
> + *status |= FE_HAS_VITERBI;
> + *status |= FE_HAS_SYNC;
> + *status |= FE_HAS_LOCK;
> + }
> +
> + return 0;
> +}
> +
> +static int si2165_set_parameters(struct dvb_frontend *fe)
> +{
> + struct dtv_frontend_properties *p = &fe->dtv_property_cache;
> + struct si2165_state *state = fe->demodulator_priv;
> + u8 val[3];
> + u32 IF;
> + u32 dvb_rate = 0;
> +
> + dprintk("%s: called\n", __func__);
> +
> + if (!fe->ops.tuner_ops.get_if_frequency) {
> + pr_err("Error: get_if_frequency() not defined at tuner. Can't work without it!\n");
You should use dev_ not pr_.
> + return -EINVAL;
> + }
> +
> + /* If Oversampling mode Ovr4 is used */
> + state->fe_clk = state->adc_clk;
> +
> + if (state->fe_clk == 0) {
> + pr_err("Error: fe_clk is 0\n");
> + return -EINVAL;
> + }
> +
> + if (!state->m_has_dvbt)
> + return -EINVAL;
> +
> + if (p->bandwidth_hz > 0)
> + dvb_rate = p->bandwidth_hz * 8 / 7;
> + else
> + dvb_rate = 8 * 8 / 7;
> +
> + si2165_writereg8(state, 0x00ec /* standard */, 0x01);
> + si2165_writereg8(state, 0x00a0 /* pll_divl */, 0x0c);
> +
> + si2165_readreg8(state, 0x00e0 /* adc_sampling_mode */, val); /* returned 0x00 */
> + si2165_writereg32(state, 0x00e8 /* if_freq_shift */, 0x00000000);
> + si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x21 */
> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
> + si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
> + si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x03 */
> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x03);
> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x03 */
> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
> + si2165_writereg16(state, 0x0308 /* bandwidth */, 0x0320);
> + si2165_writereg32(state, 0x00e4 /* oversamp */, 0x03100000);
> + si2165_writereg8(state, 0x031c /* impulsive_noise_remover */, 0x01);
> + si2165_writereg8(state, 0x00cb /* unknown_wr8 */, 0x00);
> + si2165_writereg8(state, 0x016e /* agc2_min */, 0x41);
> + si2165_writereg8(state, 0x016c /* agc2_kacq */, 0x0e);
> + si2165_writereg8(state, 0x016d /* agc2_kloc */, 0x10);
> + si2165_writereg8(state, 0x015b /* agc_unfreeze_thr */, 0x03);
> + si2165_writereg8(state, 0x0150 /* agc_crestf_dbx8 */, 0x78);
> + si2165_writereg8(state, 0x01a0 /* aaf_crestf_dbx8 */, 0x78);
> + si2165_writereg8(state, 0x01c8 /* aci_crestf_dbx8 */, 0x68);
> + si2165_writereg16(state, 0x030c /* freq_sync_range */, 0x0064);
> + si2165_readreg8(state, 0x0387 /* gp_reg0 */, val); /* returned 0x00 */
> + si2165_writereg8(state, 0x0387 /* gp_reg0 */, 0x00);
> + si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> + si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> + si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> + si2165_readreg8(state, 0x0118 /* dvb-c standard support */, val); /* returned 0x07 */
> + si2165_readreg8(state, 0x0023 /* hardware_rev */, val); /* returned 0x03 */
> + si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
> + si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
> + si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
> + si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /* returned 0x20 */
> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x01 */
> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned 0x01 */
> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
OK, maybe those own functions for 8/16/24/32 are fine as there is that
many cases.
> +
> + if (dvb_rate == 0) {
> + pr_err("Error: dvb_rate is 0\n");
> + return -EINVAL;
> + }
Dead code. Please check all the other error checks too and consider if
those are really needed or not. It is good idea to check all needed
parameters just beginning of function and jump out before any I/O if
invalid params.
> +
> + if (fe->ops.tuner_ops.set_params) {
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 1);
> + fe->ops.tuner_ops.set_params(fe);
> + if (fe->ops.i2c_gate_ctrl)
> + fe->ops.i2c_gate_ctrl(fe, 0);
> + }
Get the rid of those i2c_gate_ctrl() stuff. It is tuner who is
responsible of calling those. Even better if you could implement proper
I2C adapder (I2C mux) which handles gating transparently.
> +
> + {
> + s64 if_freq_shift;
Is signed needed?
> + u32 reg_value;
> + fe->ops.tuner_ops.get_if_frequency(fe, &IF);
> +
> + if_freq_shift = IF;
> + if_freq_shift <<= 29;
> + if (state->fe_clk > 0)
Can that clock be zero at all?
> + if_freq_shift /= (u64)state->fe_clk;
> + reg_value = ((u32)if_freq_shift) & 0x1fffffff;
Just thinking what happens here. So you have a signed number, which is
cast to unsigned and masked to X bit len. I think there is no need for
signed at all.
1) s_if = u_if;
2) s_if <<= 29;
3) s_if /= clk;
4) u_32tmp = s_if;
5) u_32val = u_32tmp & 0x1fffffff;
> +
> + si2165_writereg32(state, 0x00e8 /* if_freq_shift */, reg_value);
> + }
> +
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> + si2165_writereg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, 0x00);
> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
> + si2165_readreg8(state, 0x0341 /* boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
> +
> + return 0;
> +}
> +
> +static void si2165_release(struct dvb_frontend *fe)
> +{
> + struct si2165_state *state = fe->demodulator_priv;
> + dprintk("%s: called\n", __func__);
> + kfree(state);
> +}
> +
> +static struct dvb_frontend_ops si2165_ops = {
> + .info = {
> + .name = "SI2165",
Nit, but that chip is Si2165 not SI2165. Same everywhere.
Also use full name "Silicon Labs Si2165" as that string is shown to
application and user.
Also, delsys are missing.
> + .caps = FE_CAN_FEC_1_2 |
> + FE_CAN_FEC_2_3 |
> + FE_CAN_FEC_3_4 |
> + FE_CAN_FEC_5_6 |
> + FE_CAN_FEC_7_8 |
> + FE_CAN_FEC_AUTO |
> + FE_CAN_QPSK |
> + FE_CAN_QAM_16 |
> + FE_CAN_QAM_32 |
> + FE_CAN_QAM_64 |
> + FE_CAN_QAM_128 |
> + FE_CAN_QAM_256 |
> + FE_CAN_QAM_AUTO |
> + FE_CAN_TRANSMISSION_MODE_AUTO |
> + FE_CAN_GUARD_INTERVAL_AUTO |
> + FE_CAN_HIERARCHY_AUTO |
> + FE_CAN_MUTE_TS |
> + FE_CAN_TRANSMISSION_MODE_AUTO |
> + FE_CAN_RECOVER
> + },
> +
> + .get_tune_settings = si2165_get_tune_settings,
> +
> + .init = si2165_init,
> + .sleep = si2165_sleep,
> +
> + .i2c_gate_ctrl = si2165_i2c_gate_ctrl,
> +
> + .set_frontend = si2165_set_parameters,
> + .read_status = si2165_read_status,
> +
> + .release = si2165_release,
> +};
> +
> +struct dvb_frontend *si2165_attach(const struct si2165_config *config, struct i2c_adapter *i2c)
> +{
> + struct si2165_state *state = NULL;
> + int n;
> +
> + if (config == NULL)
> + goto error;
> +
> + /* allocate memory for the internal state */
> + state = kzalloc(sizeof(struct si2165_state), GFP_KERNEL);
> + if (state == NULL)
> + goto error;
> +
> + /* setup the state */
> + state->i2c = i2c;
> + state->config = *config;
> +
> + if (state->config.ref_freq_MHz < 4 || state->config.ref_freq_MHz > 27) {
> + dev_info(&state->i2c->dev, "%s: ref_freq of %d MHz not supported by this driver\n",
> + KBUILD_MODNAME, state->config.ref_freq_MHz);
> + goto error;
> + }
That is error case, dev_err.
> +
> + /* create dvb_frontend */
> + memcpy(&state->frontend.ops, &si2165_ops,
> + sizeof(struct dvb_frontend_ops));
> + state->frontend.demodulator_priv = state;
> +
> + /* powerup */
> + if (si2165_writereg8(state, 0x0000 /* chip_mode */, state->config.chip_mode) < 0)
> + goto error;
That kind of functionality is not allowed inside if () condition.
Checkpatch.pl should complain that too.
Do you really need powerup chip to read chip ID?
> +
> + if (si2165_readreg8(state, 0x0023 /* rev code */, &state->revcode) < 0)
> + goto error;
Bail out immediately if wrong rev.
> +
> + if (si2165_readreg8(state, 0x0118 /* chip type */, &state->chip_type) < 0)
> + goto error;
> +
Bail out immediately if wrong type.
> + /* powerdown */
> + if (si2165_writereg8(state, 0x0000 /* chip_mode */, SI2165_MODE_OFF) < 0)
> + goto error;
> +
> + dev_info(&state->i2c->dev, "%s: hardware revision 0x%02x, chip type 0x%02x\n",
> + KBUILD_MODNAME, state->revcode, state->chip_type);
> +
> + if (state->revcode != 0x03) {
> + dev_err(&state->i2c->dev, "%s: Unsupported hardware revision.\n",
> + KBUILD_MODNAME);
> + goto error;
> + }
> +
> + /* It is a guess that register 0x0118 (chip type?) can be used to
> + * differ between si2161, si2163 and si2165
> + * Only si2165 has been tested.
> + */
> + if (state->chip_type == 0x07) {
> + state->m_has_dvbt = true;
> + state->m_has_dvbc = true;
> + strcpy(state->frontend.ops.info.name, "SI2165");
> + } else {
> + dev_err(&state->i2c->dev, "%s: Unsupported Chip type.\n",
> + KBUILD_MODNAME);
> + goto error;
> + }
> +
> + n = 0;
> + if (state->m_has_dvbt) {
> + state->frontend.ops.delsys[n++] = SYS_DVBT;
> + strlcat(state->frontend.ops.info.name, " DVB-T",
> + sizeof(state->frontend.ops.info.name));
> + }
> + if (state->m_has_dvbc)
> + dev_warn(&state->i2c->dev, "%s: DVB-C is not yet supported.\n",
> + KBUILD_MODNAME);
> +
> + return &state->frontend;
> +
> +error:
> + kfree(state);
> + return NULL;
> +}
> +EXPORT_SYMBOL(si2165_attach);
> +
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Turn on/off frontend debugging (default:off).");
> +
> +MODULE_DESCRIPTION("Silicon Labs si2165 DVB-C/-T Demodulator driver");
> +MODULE_AUTHOR("Matthias Schwarzott <zzam@gentoo.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_FIRMWARE(SI2165_FIRMWARE);
> diff --git a/drivers/media/dvb-frontends/si2165.h b/drivers/media/dvb-frontends/si2165.h
> new file mode 100644
> index 0000000..cdc12e7
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/si2165.h
> @@ -0,0 +1,61 @@
> +/*
> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
> +
> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
> +
> + 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.
> +
> + References:
> + http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
> +*/
> +
> +#ifndef _DVB_SI2165_H
> +#define _DVB_SI2165_H
> +
> +#include <linux/dvb/frontend.h>
> +
> +#if IS_ENABLED(CONFIG_DVB_SI2165)
> +
> +enum {
> + SI2165_MODE_OFF = 0x00,
> + SI2165_MODE_PLL_EXT = 0x20,
> + SI2165_MODE_PLL_XTAL = 0x21
> +};
> +
> +struct si2165_config {
> + /* i2c addr
> + * possible values: 0x64,0x65,0x66,0x67 */
> + u8 i2c_addr;
> +
> + /* external clock or XTAL */
> + u8 chip_mode;
> +
> + /* frequency of external clock or xtal in Mhz
> + * possible values: 4,16,20,24,27 in
> + */
> + u8 ref_freq_MHz;
> +};
> +
> +/* Addresses: 0x64,0x65,0x66,0x67 */
> +struct dvb_frontend *si2165_attach(
> + const struct si2165_config *config,
> + struct i2c_adapter *i2c);
> +#else
> +static inline struct dvb_frontend *si2165_attach(
> + const struct si2165_config *config,
> + struct i2c_adapter *i2c)
> +{
> + pr_warn("%s: driver disabled by Kconfig\n", __func__);
> + return NULL;
> +}
> +#endif /* CONFIG_DVB_SI2165 */
> +
> +#endif /* _DVB_SI2165_H */
> diff --git a/drivers/media/dvb-frontends/si2165_priv.h b/drivers/media/dvb-frontends/si2165_priv.h
> new file mode 100644
> index 0000000..d4cc93f
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/si2165_priv.h
> @@ -0,0 +1,23 @@
> +/*
> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
> +
> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
> +
> + 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.
> +
> +*/
> +
> +#ifndef _DVB_SI2165_PRIV
> +#define _DVB_SI2165_PRIV
> +
> +#define SI2165_FIRMWARE "dvb-demod-si2165.fw"
> +
> +#endif /* _DVB_SI2165_PRIV */
>
There is almost none I/O error checks, but it is not so big issue.
That driver could be a little bit modern in a following ways:
1) dynamic debugs
2) I2C client driver model
3) RegMap API
4) I2C mux adapter for tuner I2C bus / gate
Maybe 30% less LOC.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] si2165: Add demod driver for DVB-T only
2014-05-17 2:14 ` [PATCH 1/3] si2165: Add demod driver for DVB-T only Antti Palosaari
@ 2014-07-01 19:01 ` Matthias Schwarzott
2014-07-01 19:07 ` Antti Palosaari
0 siblings, 1 reply; 6+ messages in thread
From: Matthias Schwarzott @ 2014-07-01 19:01 UTC (permalink / raw)
To: Antti Palosaari, linux-media; +Cc: xpert-reactos
On 17.05.2014 04:14, Antti Palosaari wrote:
> Sorry for the late review. I think that will be skipped over 2.16 as too
> late...
>
> There is also many other DTV drivers coming from Earthsoft PT3 support
> waiting for review. Anyone else willing to review? I wonder how we could
> improve situation, we are simply lack of reviewers. Mike, Devin, Mauro?
>
>
>
> On 04/26/2014 11:21 PM, Matthias Schwarzott wrote:
>> DVB-T was tested with 8MHz BW channels in germany
>> This driver is the simplest possible, it uses automatic mode for all
>> parameters (TPS).
>>
>> Firmware file can be extracted via get_dvb_firmware.
>>
>> Signed-off-by: Matthias Schwarzott <zzam@gentoo.org>
>> ---
>> Documentation/dvb/get_dvb_firmware | 33 +-
>> drivers/media/dvb-frontends/Kconfig | 9 +
>> drivers/media/dvb-frontends/Makefile | 1 +
>> drivers/media/dvb-frontends/si2165.c | 890
>> ++++++++++++++++++++++++++++++
>> drivers/media/dvb-frontends/si2165.h | 61 ++
>> drivers/media/dvb-frontends/si2165_priv.h | 23 +
>> 6 files changed, 1016 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/media/dvb-frontends/si2165.c
>> create mode 100644 drivers/media/dvb-frontends/si2165.h
>> create mode 100644 drivers/media/dvb-frontends/si2165_priv.h
>>
>> diff --git a/Documentation/dvb/get_dvb_firmware
>> b/Documentation/dvb/get_dvb_firmware
>> index d91b8be..26c623d 100755
>> --- a/Documentation/dvb/get_dvb_firmware
>> +++ b/Documentation/dvb/get_dvb_firmware
>> @@ -29,7 +29,7 @@ use IO::Handle;
>> "af9015", "ngene", "az6027", "lme2510_lg", "lme2510c_s7395",
>> "lme2510c_s7395_old", "drxk", "drxk_terratec_h5",
>> "drxk_hauppauge_hvr930c", "tda10071", "it9135", "drxk_pctv",
>> - "drxk_terratec_htc_stick", "sms1xxx_hcw");
>> + "drxk_terratec_htc_stick", "sms1xxx_hcw", "si2165");
>>
>> # Check args
>> syntax() if (scalar(@ARGV) != 1);
>> @@ -783,6 +783,37 @@ sub sms1xxx_hcw {
>> $allfiles;
>> }
>>
>> +sub si2165 {
>> + my $sourcefile =
>> "model_111xxx_122xxx_driver_6_0_119_31191_WHQL.zip";
>> + my $url = "http://www.hauppauge.de/files/drivers/";
>> + my $hash = "76633e7c76b0edee47c3ba18ded99336";
>> + my $fwfile = "dvb-demod-si2165.fw";
>> + my $tmpdir = tempdir(DIR => "/tmp", CLEANUP => 1);
>> +
>> + checkstandard();
>> +
>> + wgetfile($sourcefile, $url . $sourcefile);
>> + verify($sourcefile, $hash);
>> + unzip($sourcefile, $tmpdir);
>> + extract("$tmpdir/Driver10/Hcw10bda.sys", 0x80788,
>> 0x81E08-0x80788, "$tmpdir/fw1");
>> +
>> + delzero("$tmpdir/fw1","$tmpdir/fw1-1");
>> + #verify("$tmpdir/fw1","5e0909858fdf0b5b09ad48b9fe622e70");
>> +
>> + my $CRC="\x0A\xCC";
>> + my $BLOCKS_MAIN="\x27";
>> + open FW,">$fwfile";
>> + print FW "\x01\x00"; # just a version id for the driver itself
>> + print FW "\x9A"; # fw version
>> + print FW "\x00"; # padding
>> + print FW "$BLOCKS_MAIN"; # number of blocks of main part
>> + print FW "\x00"; # padding
>> + print FW "$CRC"; # 16bit crc value of main part
>> + appendfile(FW,"$tmpdir/fw1");
>> +
>> + "$fwfile";
>> +}
>> +
>> # ---------------------------------------------------------------
>> # Utilities
>
> Separate that firmware extractor to own patch.
>
done
>>
>> diff --git a/drivers/media/dvb-frontends/Kconfig
>> b/drivers/media/dvb-frontends/Kconfig
>> index 1469d44..0da53c2 100644
>> --- a/drivers/media/dvb-frontends/Kconfig
>> +++ b/drivers/media/dvb-frontends/Kconfig
>> @@ -63,6 +63,15 @@ config DVB_TDA18271C2DD
>>
>> Say Y when you want to support this tuner.
>>
>> +config DVB_SI2165
>> + tristate "Silicon Labs si2165 based"
>> + depends on DVB_CORE && I2C
>> + default m if !MEDIA_SUBDRV_AUTOSELECT
>> + help
>> + A DVB-C/T demodulator.
>> +
>> + Say Y when you want to support this frontend.
>> +
>> comment "DVB-S (satellite) frontends"
>> depends on DVB_CORE
>>
>> diff --git a/drivers/media/dvb-frontends/Makefile
>> b/drivers/media/dvb-frontends/Makefile
>> index dda0bee..595dd8d 100644
>> --- a/drivers/media/dvb-frontends/Makefile
>> +++ b/drivers/media/dvb-frontends/Makefile
>> @@ -100,6 +100,7 @@ obj-$(CONFIG_DVB_STV0367) += stv0367.o
>> obj-$(CONFIG_DVB_CXD2820R) += cxd2820r.o
>> obj-$(CONFIG_DVB_DRXK) += drxk.o
>> obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
>> +obj-$(CONFIG_DVB_SI2165) += si2165.o
>> obj-$(CONFIG_DVB_A8293) += a8293.o
>> obj-$(CONFIG_DVB_TDA10071) += tda10071.o
>> obj-$(CONFIG_DVB_RTL2830) += rtl2830.o
>> diff --git a/drivers/media/dvb-frontends/si2165.c
>> b/drivers/media/dvb-frontends/si2165.c
>> new file mode 100644
>> index 0000000..3e69a32
>> --- /dev/null
>> +++ b/drivers/media/dvb-frontends/si2165.c
>> @@ -0,0 +1,890 @@
>> +/*
>> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
>> +
>> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
>> +
>> + 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.
>> +
>> + References:
>> +
>> http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
>> +*/
>> +
>> +#include <linux/delay.h>
>> +#include <linux/errno.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/string.h>
>> +#include <linux/slab.h>
>> +#include <linux/firmware.h>
>> +
>> +#include "dvb_frontend.h"
>> +#include "dvb_math.h"
>> +#include "si2165_priv.h"
>> +#include "si2165.h"
>> +
>> +/* Hauppauge WinTV-HVR-930C-HD 1113xx uses 16.000 MHz xtal */
>> +
>> +struct si2165_state {
>> + struct i2c_adapter *i2c;
>> +
>> + struct dvb_frontend frontend;
>> +
>> + struct si2165_config config;
>> +
>> + /* chip revision */
>> + u8 revcode;
>> + /* chip type */
>> + u8 chip_type;
>> +
>> + /* calculated by xtal and div settings */
>> + u32 fvco_hz;
>> + u32 sys_clk;
>> + u32 adc_clk;
>> +
>> + /* depends on adc_clk and Ovr mode */
>> + u32 fe_clk;
>> +
>> + bool m_has_dvbc;
>> + bool m_has_dvbt;
>
> m_ prefix is for Hungarian notation of member variables. It is not
> suitable for Kernel style.
>
removed. I only use this part of hungarian notation. Sometimes its bad
to switch between C and C++ :)
>
>> + bool firmware_loaded;
>> +};
>> +
>> +#define DEBUG_OTHER 0x01
>> +#define DEBUG_I2C_WRITE 0x02
>> +#define DEBUG_I2C_READ 0x04
>> +#define DEBUG_REG_READ 0x08
>> +#define DEBUG_REG_WRITE 0x10
>> +#define DEBUG_FW_LOAD 0x20
>> +
>> +static int debug = 0x00;
>> +
>> +#define dprintk(args...) \
>> + do { \
>> + if (debug & DEBUG_OTHER) \
>> + printk(KERN_DEBUG "si2165: " args); \
>> + } while (0)
>> +
>> +#define deb_i2c_write(args...) \
>> + do { \
>> + if (debug & DEBUG_I2C_WRITE) \
>> + printk(KERN_DEBUG "si2165: i2c write: " args); \
>> + } while (0)
>> +
>> +#define deb_i2c_read(args...) \
>> + do { \
>> + if (debug & DEBUG_I2C_READ) \
>> + printk(KERN_DEBUG "si2165: i2c read: " args); \
>> + } while (0)
>> +
>> +#define deb_readreg(args...) \
>> + do { \
>> + if (debug & DEBUG_REG_READ) \
>> + printk(KERN_DEBUG "si2165: reg read: " args); \
>> + } while (0)
>> +
>> +#define deb_writereg(args...) \
>> + do { \
>> + if (debug & DEBUG_REG_WRITE) \
>> + printk(KERN_DEBUG "si2165: reg write: " args); \
>> + } while (0)
>> +
>> +#define deb_fw_load(args...) \
>> + do { \
>> + if (debug & DEBUG_FW_LOAD) \
>> + printk(KERN_DEBUG "si2165: fw load: " args); \
>> + } while (0)
>
> Could you consider kernel dynamic debugs which are todays norm? See for
> example si2168 driver as a example. IMHO it is not hard requirement, but
> still...
>
TODO
>> +
>> +static int si2165_write(struct si2165_state *state, const u16 reg,
>> + const u8 *src, const int count)
>> +{
>> + int ret;
>> + struct i2c_msg msg;
>> + u8 buf[2 + 4]; /* write a maximum of 4 bytes of data */
>
> Here should be empty line between variable definitions and code. IIRC
> latest checkpatch.pl catch that.
>
done.
The version from git://linuxtv.org/media_tree.git does not have this
warning.
>> + if (count + 2 > sizeof(buf)) {
>> + dev_warn(&state->i2c->dev,
>> + "%s: i2c wr reg=%04x: count=%d is too big!\n",
>> + KBUILD_MODNAME, reg, count);
>> + return -EINVAL;
>> + }
>> + buf[0] = reg >> 8;
>> + buf[1] = reg & 0xff;
>> + memcpy(buf + 2, src, count);
>> +
>> + msg.addr = state->config.i2c_addr;
>> + msg.flags = 0;
>> + msg.buf = buf;
>> + msg.len = count + 2;
>> +
>> + if (debug & DEBUG_I2C_WRITE) {
>> + int i;
>> + deb_i2c_write("reg: 0x%04x, data:", reg);
>> + for (i = 0; i < count; i++)
>> + printk(KERN_CONT " %02x", src[i]);
>> + printk(KERN_CONT "\n");
>> + }
>> +
>> + ret = i2c_transfer(state->i2c, &msg, 1);
>> +
>> + if (ret != 1) {
>> + dev_err(&state->i2c->dev, "%s: ret == %d\n", __func__, ret);
>> + return -EREMOTEIO;
>> + }
>
> Could be nice to return error from i2c_transfer().
>
done.
>> +
>> + return 0;
>> +}
>> +
>> +static int si2165_read(struct si2165_state *state,
>> + const u16 reg, u8 *val, const size_t count)
>
> You used int on si2165_write(), try to make decision which one is
> correct and keep it.
>
changed to int.
>> +{
>> + int ret;
>> + u8 reg_buf[] = { reg >> 8, reg & 0xff };
>> + struct i2c_msg msg[] = {
>> + { .addr = state->config.i2c_addr,
>> + .flags = 0, .buf = reg_buf, .len = 2 },
>> + { .addr = state->config.i2c_addr,
>> + .flags = I2C_M_RD, .buf = val, .len = count },
>> + };
>> +
>> + ret = i2c_transfer(state->i2c, msg, 2);
>> +
>> + if (ret != 2) {
>> + dev_err(&state->i2c->dev, "%s: error (addr %02x reg %04x
>> error (ret == %i)\n",
>> + __func__, state->config.i2c_addr, reg, ret);
>> + if (ret < 0)
>> + return ret;
>> + else
>> + return -EREMOTEIO;
>> + }
>> +
>> + if (debug & DEBUG_I2C_READ) {
>> + int i;
>> + deb_i2c_read("reg: 0x%04x, data:", reg);
>> + for (i = 0; i < count; i++)
>> + printk(KERN_CONT " %02x", val[i]);
>> + printk(KERN_CONT "\n");
>> + }
>
> There is formatter %pH (or something like) for that.
>
Done, it is "%*ph".
>> + return 0;
>> +}
>> +
>> +static int si2165_readreg8(struct si2165_state *state,
>> + const u16 reg, u8 *val)
>> +{
>> + int ret;
>> + ret = si2165_read(state, reg, val, 1);
>> + deb_readreg("R(0x%04x)=0x%02x\n", reg, *val);
>> + return ret;
>> +}
>> +
>> +static int si2165_readreg16(struct si2165_state *state,
>> + const u16 reg, u16 *val)
>> +{
>> + u8 buf[2];
>> + int ret = si2165_read(state, reg, buf, 2);
>> + *val = buf[0] | buf[1] << 8;
>> + deb_readreg("R(0x%04x)=0x%04x\n", reg, *val);
>> + return ret;
>> +}
>> +
>> +#if 0
>> +static int si2165_readreg24(struct si2165_state *state,
>> + const u16 reg, u32 *val)
>> +{
>> + u8 buf[3];
>> + int ret = si2165_read(state, reg, buf, 3);
>> + *val = buf[0] | buf[1] << 8 | buf[2] << 16;
>> + deb_readreg("R(0x%04x)=0x%06x\n", reg, *val);
>> + return ret;
>> +}
>> +
>> +static int si2165_readreg32(struct si2165_state *state,
>> + const u16 reg, u32 *val)
>> +{
>> + u8 buf[4];
>> + int ret = si2165_read(state, reg, buf, 4);
>> + *val = buf[0] | buf[1] << 8 | buf[2] << 16 | buf[3] << 24;
>> + deb_readreg("R(0x%04x)=0x%08x\n", reg, *val);
>> + return ret;
>> +}
>> +#endif
>
> Personally, I don't like presenting shorthand functions for multi-byte
> readings like that. But if you think it is good then leave it.
>
This is for combining the read bytes to correct order.
>> +
>> +
>> +static int si2165_writereg8(struct si2165_state *state, const u16
>> reg, u8 val)
>> +{
>> + return si2165_write(state, reg, &val, 1);
>> +}
>> +
>> +static int si2165_writereg16(struct si2165_state *state, const u16
>> reg, u16 val)
>> +{
>> + u8 buf[2] = { val & 0xff, (val >> 8) & 0xff };
>> + return si2165_write(state, reg, buf, 2);
>> +}
>> +
>> +static int si2165_writereg24(struct si2165_state *state, const u16
>> reg, u32 val)
>> +{
>> + u8 buf[3] = { val & 0xff, (val >> 8) & 0xff, (val >> 16) & 0xff };
>> + return si2165_write(state, reg, buf, 3);
>> +}
>> +
>> +static int si2165_writereg32(struct si2165_state *state, const u16
>> reg, u32 val)
>> +{
>> + u8 buf[4] = {
>> + val & 0xff,
>> + (val >> 8) & 0xff,
>> + (val >> 16) & 0xff,
>> + (val >> 24) & 0xff
>> + };
>> + return si2165_write(state, reg, buf, 4);
>> +}
>> +
>> +static int si2165_writereg_mask8(struct si2165_state *state, const
>> u16 reg,
>> + u8 val, u8 mask)
>> +{
>> + int ret;
>> + u8 tmp;
>> +
>> + if (mask != 0xff) {
>> + ret = si2165_readreg8(state, reg, &tmp);
>> + if (ret < 0)
>> + goto err;
>> +
>> + val &= mask;
>> + tmp &= ~mask;
>> + val |= tmp;
>> + }
>> +
>> + ret = si2165_writereg8(state, reg, val);
>> +err:
>> + return ret;
>> +}
>> +
>> +static int si2165_get_tune_settings(struct dvb_frontend *fe,
>> + struct dvb_frontend_tune_settings *s)
>> +{
>> + s->min_delay_ms = 1000;
>> + return 0;
>> +}
>> +
>> +static int si2165_init_pll(struct si2165_state *state)
>> +{
>> + u8 ref_freq_MHz = state->config.ref_freq_MHz;
>
> 1MHz is quite big reference frequency step?
It was because short datasheet of si2165 says: "4, 16, 20, 24, or 27 MHz
clock/crystal reference"
I thought 1MHz precision is enough.
The two devices I currently support both use a 16MHz chrystal.
But I wanted to have it because the followup device (930C-HD with
vid=0xb131) has in the inf file: "24MHz clock from Si2158 to Si2165 demod".
Now I changed it to 1Hz precision as some other drivers do.
>
>> + /* u8 val; */
>
> excessive comment
removed.
>
>> + u8 divr = 1; /* 1..7 */
>> + u8 divp = 1; /* only 1 or 4 */
>> + u8 divn = 56; /* 1..63 */
>> +
>
> excessive newline
removed.
>
>> + u8 divm = 8;
>> + u8 divl = 12;
>> + u8 buf[4];
>> +
>> + /* ref_freq / divr must be between 4 and 16 MHz */
>> + if (ref_freq_MHz > 16)
>> + divr = 2;
>> +
>> + /* now select divn and divp such that fvco is in 1624..1824 MHz */
>> + if (1624 * divr > ref_freq_MHz * 2 * 63)
>> + divp = 4;
>> +
>> + /* to get exactly the same as the windows driver does */
>> + if (ref_freq_MHz == 16)
>> + divn = 56;
>> + else {
>> + /* is this already correct regarding rounding? */
>> + divn = 1624 * divr / (ref_freq_MHz * 2 * divp);
>> + }
>
> That logic does not look correct. PLL dividers are usually calculated in
> a order:
> 1) calculate reference divider (divREF)
> 2) calculate output divider (divOUT)
> 3) calculate VCO freq (fVCO = fOUT * divOUT)
> 4) calculate N & NF, pllN = fVCO * divREF / fREF;
>
> N = interger part, NF = fractional part. NF only when PLL is Fractional N.
>
> Biggest thing looks wrong is that: IF (fREF == 16MHz) THEN pllN = 56.
>
that was just modifying the value in the valid range to yield exactly
the same result and not just a valid one.
For now I return the exact same values as windows hardcoded. The
calculation still is there.
>
> There should be parenthesis for none or both conditions. checkpatch.pl?
>
Removed them.
It did not say anything about this.
>
>> +
>> + /* adc_clk and sys_clk depend on xtal and pll settings */
>> + /* only calculate once as long as the pll settings are not
>> modified */
>> + if (state->adc_clk == 0) {
>> + u32 fvco_hz = ref_freq_MHz * 1000000ull / divr
>> + * 2 * divn * divp;
>> + state->adc_clk = fvco_hz / (divm * 4);
>> + state->sys_clk = fvco_hz / (divl * 2);
>> + }
>> +
>> + /* write pll registers 0x00a0..0x00a3 at once */
>> + buf[0] = divl;
>> + buf[1] = divm;
>> + buf[2] = (divn & 0x3f) | ((divp == 1) ? 0x40 : 0x00) | 0x80;
>> + buf[3] = divr;
>> + si2165_write(state, 0x00a0 /* first pll reg */, buf, 4);
>
> That kinf of comments in a parameter list do not look typical for kernel
> style.
>
I changed it.
>> +
>> + return 0;
>> +}
>
> Also that function is called from init(), maybe you could inline all
> that stuff to that function instead.
>
I tried to keep the functions somewhat easy to overview.
>> +
>> +static bool si2165_wait_init_done(struct si2165_state *state)
>> +{
>> + int ret = false;
>> + u8 val;
>> + int i;
>> + for (i = 0; i < 10; i++) {
>> + si2165_readreg8(state, 0x0054 /* init_done */, &val);
>> + if (val == 0x01) {
>> + ret = true;
>> + break;
>> + }
>> + msleep(1);
>
> Too small msleep(), see timers howto. Also checkpatch.pl should be able
> to find that too, you haven't ran it.
>
well, this was not listed.
I don't yet know exactly how to solve it here.
Datasheet says to either poll or wait for around 5ms (depending on
sys_clk, but I have no clue about the dependency).
checkpatch had only these kind of warnings:
WARNING: line over 80 characters
WARNING: msleep < 20ms can sleep for up to 20ms; see
Documentation/timers/timers-howto.txt
WARNING: Prefer [subsystem eg: netdev]_cont([subsystem]dev, ... then
dev_cont(dev, ... then pr_cont(... to printk(KERN_CONT ...
WARNING: Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then
dev_dbg(dev, ... then pr_debug(... to printk(KERN_DEBUG ...
WARNING: suspect code indent for conditional statements (8, 18)
yeah, I replace msleep by usleep_range.
>> + }
>> + return ret;
>> +}
>
> Whole function is called from one place and it is very trivial, so that
> function is mostly reduntant. Move logic to where it is now called.
> Also, use jiffies for timeouts like that. See si2168 for example.
>
>
>> +
>> +static int si2165_upload_firmware_block(struct si2165_state *state,
>> const u8 *data, u32 len, u32 *poffset, u32 block_count)
>> +{
>> + u8 buf_ctrl[4] = { 0x00, 0x00, 0x00, 0xc0 };
>> + u8 wordcount;
>> + u32 cur_block = 0;
>> + u32 offset = poffset ? *poffset : 0;
>> + if (len < 4)
>> + return -EINVAL;
>> + if (len % 4 != 0)
>> + return -EINVAL;
>> +
>> + deb_fw_load("si2165_upload_firmware_block called with len=0x%x
>> offset=0x%x blockcount=0x%x\n",
>> + len, offset, block_count);
>> + while (offset+12 <= len && cur_block < block_count) {
>> + deb_fw_load("si2165_upload_firmware_block in while len=0x%x
>> offset=0x%x cur_block=0x%x blockcount=0x%x\n",
>> + len, offset, cur_block, block_count);
>> + wordcount = data[offset];
>> + if (wordcount < 1 || data[offset+1] || data[offset+2] ||
>> data[offset+3]) {
>> + dev_warn(&state->i2c->dev, "%s: bad fw data[0..3] = %02x
>> %02x %02x %02x\n", KBUILD_MODNAME, data[0], data[1], data[2], data[3]);
>> + return -EINVAL;
>> + }
>> +
>> + if (offset + 8 + wordcount * 4 > len) {
>> + dev_warn(&state->i2c->dev, "%s: len is too small for
>> block len=%d, wordcount=%d\n", KBUILD_MODNAME, len, wordcount);
>> + return -EINVAL;
>> + }
>> +
>> + buf_ctrl[0] = wordcount - 1;
>> +
>> + si2165_write(state, 0x0364, buf_ctrl, 4);
>> + si2165_write(state, 0x0368, data+offset+4, 4);
>
> CodingStyle. That driver has a lot of coding style mistakes ==> you must
> run checkpatch.pl and fix all.
>
>> +
>> + offset += 8;
>> +
>> + while (wordcount > 0) {
>> + si2165_write(state, 0x36c, data+offset, 4);
>> + wordcount--;
>> + offset += 4;
>> + }
>> + cur_block++;
>> + }
>> +
>> + deb_fw_load("si2165_upload_firmware_block after while len=0x%x
>> offset=0x%x cur_block=0x%x blockcount=0x%x\n",
>> + len, offset, cur_block, block_count);
>> +
>> + if (poffset)
>> + *poffset = offset;
>> +
>> + deb_fw_load("si2165_upload_firmware_block returned offset=0x%x\n",
>> + offset);
>> +
>> + return 0;
>> +}
>> +
>> +static int si2165_upload_firmware(struct si2165_state *state)
>> +{
>> + /* int ret; */
>> + u8 val[3];
>> + u16 val16;
>> + int ret;
>> +
>> + const struct firmware *fw = NULL;
>> + u8 *fw_file = SI2165_FIRMWARE;
>> + const u8 *data;
>> + u32 len;
>> + u32 offset;
>> + u8 patch_version;
>> + u8 block_count;
>> + u16 crc_expected;
>> +
>> + /* request the firmware, this will block and timeout */
>> + ret = request_firmware(&fw, fw_file, state->i2c->dev.parent);
>> + if (ret) {
>> + dev_warn(&state->i2c->dev, "%s: firmare file '%s' not found\n",
>> + KBUILD_MODNAME, fw_file);
>> + goto err;
>> + }
>> +
>> + data = fw->data;
>> + len = fw->size;
>> +
>> + dev_info(&state->i2c->dev, "%s: downloading firmware from file
>> '%s' size=%d\n",
>> + KBUILD_MODNAME, fw_file, len);
>> +
>> + if (len % 4 != 0) {
>> + dev_warn(&state->i2c->dev, "%s: firmware size is not multiple
>> of 4\n",
>> + KBUILD_MODNAME);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + /* check header (8 bytes) */
>> + if (len < 8) {
>> + dev_warn(&state->i2c->dev, "%s: firmware header is missing\n",
>> + KBUILD_MODNAME);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + if (data[0] != 1 || data[1] != 0) {
>> + dev_warn(&state->i2c->dev, "%s: firmware file version is
>> wrong\n",
>> + KBUILD_MODNAME);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + patch_version = data[2];
>> + block_count = data[4];
>> + crc_expected = data[7] << 8 | data[6];
>> +
>> + /* start uploading fw */
>> + si2165_writereg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, 0x00);
>> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> +
>> + si2165_readreg8(state, 0x035c /* en_rst_error */, val); /*
>> returned 0x03 */
>> + si2165_readreg8(state, 0x035c /* en_rst_error */, val); /*
>> returned 0x03 */
>> + si2165_writereg8(state, 0x035c /* en_rst_error */, 0x02);
>> +
>> + /* start right after the header */
>> + offset = 8;
>> +
>> + dev_info(&state->i2c->dev, "%s: si2165_upload_firmware extracted
>> patch_version=0x%02x, block_count=0x%02x, crc_expected=0x%04x\n",
>> + KBUILD_MODNAME, patch_version, block_count,
>> crc_expected);
>> +
>> + ret = si2165_upload_firmware_block(state, data, len, &offset, 1);
>> +
>> + si2165_writereg8(state, 0x0344 /* patch_version */, patch_version);
>> +
>> + ret = si2165_writereg8(state, 0x0379 /* rst_crc */, 0x01);
>> + if (ret)
>> + return ret;
>> +
>> + ret = si2165_upload_firmware_block(state, data, len, &offset,
>> block_count);
>> +
>> + if (ret) {
>> + dev_err(&state->i2c->dev, "%s: firmare could not be uploaded\n",
>> + KBUILD_MODNAME);
>> + goto err;
>> + }
>> +
>> + ret = si2165_readreg16(state, 0x037a /* crc */, &val16); /*
>> returned 0xcc0a */
>> + if (ret)
>> + goto err;
>> +
>> + if (val16 != crc_expected) {
>> + dev_err(&state->i2c->dev, "%s: firmware crc mismatch %04x !=
>> %04x\n", KBUILD_MODNAME, val16, crc_expected);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + ret = si2165_upload_firmware_block(state, data, len, &offset, 5);
>> + if (ret)
>> + goto err;
>> +
>> + if (len != offset) {
>> + dev_err(&state->i2c->dev, "%s: firmare len mismatch %04x !=
>> %04x\n", KBUILD_MODNAME, len, offset);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + /* reset watchdog error register, using auto return value
>> rst_wdog_error */
>> + si2165_writereg_mask8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, 0x02, 0x02);
>> +
>> + /* enable reset on error */
>> + si2165_writereg_mask8(state, 0x035c /* en_rst_error */, 0x01, 0x01);
>> +
>> + dev_info(&state->i2c->dev, "%s: fw load finished\n",
>> KBUILD_MODNAME);
>> +
>> + ret = 0;
>> + state->firmware_loaded = true;
>> +err:
>> + if (fw) {
>> + release_firmware(fw);
>> + fw = NULL;
>> + }
>> +
>> + return ret;
>> +}
>
>
> That firmware downloading looks overall very long a also a bit complex.
> I didn't looked it through carefully. Maybe you could take a look to
> si2168 or af9035 to see if that could be shorten.
>
>
>> +
>> +static int si2165_init_dsp(struct si2165_state *state)
>> +{
>> + u8 val;
>> + u8 patch_version = 0x00;
>> +
>> + si2165_readreg8(state, 0x0344 /* patch_version */,
>> &patch_version); /* returned 0x00 */
>> +
>> + si2165_writereg8(state, 0x00cb, 0x00);
>> + if (patch_version == 0x00)
>> + si2165_writereg8(state, 0x0344 /* patch_version */, 0x00);
>> +
>> + si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, &val); /* returned 0x01 */
>> +
>> + if (patch_version == 0x00)
>> + si2165_upload_firmware(state);
>> +
>> + return 0;
>> +}
>
> Maybe that trivial function could be integrated to where it is called.
>
inlined
>> +
>> +static int si2165_init(struct dvb_frontend *fe)
>> +{
>> + struct si2165_state *state = fe->demodulator_priv;
>> + u8 val;
>> +
>> + dprintk("%s: called\n", __func__);
>> +
>> + /* powerup */
>> + si2165_writereg8(state, 0x0000 /* chip_mode */,
>> state->config.chip_mode);
>> + si2165_writereg8(state, 0x0104 /* dsp_clock_enable */, 0x01);
>> + si2165_readreg8(state, 0x0000 /* chip_mode */, &val);
>> + if (val != state->config.chip_mode) {
>> + dev_err(&state->i2c->dev, "%s: could not set chip_mode\n",
>> + KBUILD_MODNAME);
>> + return -EINVAL;
>> + }
>> +
>> + si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
>> + si2165_writereg8(state, 0x0190 /* agc_if_slr */, 0x01);
>> + si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */,
>> &val); /* returned 0x00 */
>> + si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */,
>> 0x00);
>> + si2165_readreg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */,
>> &val); /* returned 0x00 */
>> + si2165_writereg8(state, 0x0170 /* agc2_{freeze,pola,buftype} */,
>> 0x00);
>> + si2165_writereg8(state, 0x0171 /* agc2_clkdiv */, 0x07);
>> + si2165_writereg8(state, 0x0646 /* rssi_pad_ctrl */, 0x00);
>> + si2165_readreg8(state, 0x0641 /*
>> en_rssi,start_rssi,rssi_update_time */, &val); /* returned 0x00 */
>> + si2165_writereg8(state, 0x0641 /*
>> en_rssi,start_rssi,rssi_update_time */, 0x00);
>> + si2165_writereg8(state, 0x00e0 /* adc_sampling_mode */, 0x00);
>> +
>> + si2165_init_pll(state);
>> +
>> + si2165_writereg8(state, 0x0050 /* chip_init */, 0x01);
>> + si2165_writereg8(state, 0x0096 /* start_init */, 0x01);
>> + if (!si2165_wait_init_done(state)) {
>> + dev_err(&state->i2c->dev, "%s: init_done was not set\n",
>> + KBUILD_MODNAME);
>> + return -EINVAL;
>> + }
>> +
>> + si2165_writereg8(state, 0x0050 /* chip_init */, 0x00);
>> +
>> + si2165_writereg16(state, 0x0470 /* ber_pkt */, 0x7530);
>> +
>> + si2165_init_dsp(state);
>> +
>> + si2165_writereg8(state, 0x012a /* adc_ri1 */, 0x46);
>> + si2165_writereg8(state, 0x012c /* adc_ri3 */, 0x00);
>> + si2165_writereg8(state, 0x012e /* adc_ri5 */, 0x0a);
>> + si2165_writereg8(state, 0x012f /* adc_ri6 */, 0xff);
>> + si2165_writereg8(state, 0x0123 /* adc_ri8 */, 0x70);
>> +
>> + return 0;
>> +}
>> +
>> +static int si2165_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
>> +{
>> + struct si2165_state *state = fe->demodulator_priv;
>> + u8 val = enable ? 0x01 : 0x00;
>> + return si2165_writereg8(state, 0x0001 /* i2c passthru */, val);
>> +}
>> +
>> +static int si2165_sleep(struct dvb_frontend *fe)
>> +{
>> + struct si2165_state *state = fe->demodulator_priv;
>> + dprintk("%s: called\n", __func__);
>> + si2165_writereg8(state, 0x0104 /* dsp clock enable */, 0x00);
>> + si2165_writereg8(state, 0x0000 /* chip mode */, SI2165_MODE_OFF);
>> + return 0;
>> +}
>> +
>> +static int si2165_read_status(struct dvb_frontend *fe, fe_status_t
>> *status)
>> +{
>> + u8 fec_lock = 0;
>> + struct si2165_state *state = fe->demodulator_priv;
>> +
>> + if (!state->m_has_dvbt)
>> + return -EINVAL;
>> +
>> + /* seq1 */
>> + si2165_readreg8(state, 0x4e0 /* fec_lock */, &fec_lock);
>> + *status = 0;
>> + if (fec_lock & 0x01) {
>> + *status |= FE_HAS_SIGNAL;
>> + *status |= FE_HAS_CARRIER;
>> + *status |= FE_HAS_VITERBI;
>> + *status |= FE_HAS_SYNC;
>> + *status |= FE_HAS_LOCK;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int si2165_set_parameters(struct dvb_frontend *fe)
>> +{
>> + struct dtv_frontend_properties *p = &fe->dtv_property_cache;
>> + struct si2165_state *state = fe->demodulator_priv;
>> + u8 val[3];
>> + u32 IF;
>> + u32 dvb_rate = 0;
>> +
>> + dprintk("%s: called\n", __func__);
>> +
>> + if (!fe->ops.tuner_ops.get_if_frequency) {
>> + pr_err("Error: get_if_frequency() not defined at tuner. Can't
>> work without it!\n");
>
> You should use dev_ not pr_.
>
replaced.
>> + return -EINVAL;
>> + }
>> +
>> + /* If Oversampling mode Ovr4 is used */
>> + state->fe_clk = state->adc_clk;
>> +
>> + if (state->fe_clk == 0) {
>> + pr_err("Error: fe_clk is 0\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (!state->m_has_dvbt)
>> + return -EINVAL;
>> +
>> + if (p->bandwidth_hz > 0)
>> + dvb_rate = p->bandwidth_hz * 8 / 7;
>> + else
>> + dvb_rate = 8 * 8 / 7;
>> +
>> + si2165_writereg8(state, 0x00ec /* standard */, 0x01);
>> + si2165_writereg8(state, 0x00a0 /* pll_divl */, 0x0c);
>> +
>> + si2165_readreg8(state, 0x00e0 /* adc_sampling_mode */, val); /*
>> returned 0x00 */
>> + si2165_writereg32(state, 0x00e8 /* if_freq_shift */, 0x00000000);
>> + si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
>> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /*
>> returned 0x21 */
>> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
>> + si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
>> + si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
>> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /*
>> returned 0x20 */
>> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
>> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned
>> 0x03 */
>> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x03);
>> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned
>> 0x03 */
>> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
>> + si2165_writereg16(state, 0x0308 /* bandwidth */, 0x0320);
>> + si2165_writereg32(state, 0x00e4 /* oversamp */, 0x03100000);
>> + si2165_writereg8(state, 0x031c /* impulsive_noise_remover */, 0x01);
>> + si2165_writereg8(state, 0x00cb /* unknown_wr8 */, 0x00);
>> + si2165_writereg8(state, 0x016e /* agc2_min */, 0x41);
>> + si2165_writereg8(state, 0x016c /* agc2_kacq */, 0x0e);
>> + si2165_writereg8(state, 0x016d /* agc2_kloc */, 0x10);
>> + si2165_writereg8(state, 0x015b /* agc_unfreeze_thr */, 0x03);
>> + si2165_writereg8(state, 0x0150 /* agc_crestf_dbx8 */, 0x78);
>> + si2165_writereg8(state, 0x01a0 /* aaf_crestf_dbx8 */, 0x78);
>> + si2165_writereg8(state, 0x01c8 /* aci_crestf_dbx8 */, 0x68);
>> + si2165_writereg16(state, 0x030c /* freq_sync_range */, 0x0064);
>> + si2165_readreg8(state, 0x0387 /* gp_reg0 */, val); /* returned
>> 0x00 */
>> + si2165_writereg8(state, 0x0387 /* gp_reg0 */, 0x00);
>> + si2165_writereg32(state, 0x0348 /* dsp_addr_jump */, 0xf4000000);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> + si2165_writereg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, 0x00);
>> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
>> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
>> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> + si2165_writereg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, 0x00);
>> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
>> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
>> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> + si2165_readreg8(state, 0x0118 /* dvb-c standard support */, val);
>> /* returned 0x07 */
>> + si2165_readreg8(state, 0x0023 /* hardware_rev */, val); /*
>> returned 0x03 */
>> + si2165_writereg8(state, 0x018b /* agc_if_tri */, 0x00);
>> + si2165_writereg8(state, 0x08f8 /* unknown_wr8 */, 0x00);
>> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /*
>> returned 0x20 */
>> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
>> + si2165_writereg16(state, 0x04ef /* ts_data_tri */, 0x00fe);
>> + si2165_writereg24(state, 0x04f4 /* ts_data0-3_slr */, 0x555555);
>> + si2165_readreg8(state, 0x04e4 /* ts_output_mode */, val); /*
>> returned 0x20 */
>> + si2165_writereg8(state, 0x04e4 /* ts_output_mode */, 0x20);
>> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned
>> 0x01 */
>> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
>> + si2165_readreg8(state, 0x04e5 /* ts_clock */, val); /* returned
>> 0x01 */
>> + si2165_writereg8(state, 0x04e5 /* ts_clock */, 0x01);
>
> OK, maybe those own functions for 8/16/24/32 are fine as there is that
> many cases.
>
>> +
>> + if (dvb_rate == 0) {
>> + pr_err("Error: dvb_rate is 0\n");
>> + return -EINVAL;
>> + }
>
> Dead code. Please check all the other error checks too and consider if
> those are really needed or not. It is good idea to check all needed
> parameters just beginning of function and jump out before any I/O if
> invalid params.
>
removed.
>> +
>> + if (fe->ops.tuner_ops.set_params) {
>> + if (fe->ops.i2c_gate_ctrl)
>> + fe->ops.i2c_gate_ctrl(fe, 1);
>> + fe->ops.tuner_ops.set_params(fe);
>> + if (fe->ops.i2c_gate_ctrl)
>> + fe->ops.i2c_gate_ctrl(fe, 0);
>> + }
>
> Get the rid of those i2c_gate_ctrl() stuff. It is tuner who is
> responsible of calling those. Even better if you could implement proper
> I2C adapder (I2C mux) which handles gating transparently.
>
removed the call to i2c_gate_ctrl.
>> +
>> + {
>> + s64 if_freq_shift;
> Is signed needed?
>
yes, the register is defined signed. And in some cases signed numbers
need to be written.
For example a spectrum inversion in some cases is handled by multiplying
this freq_shift by -1.
>> + u32 reg_value;
>> + fe->ops.tuner_ops.get_if_frequency(fe, &IF);
>> +
>> + if_freq_shift = IF;
>> + if_freq_shift <<= 29;
>> + if (state->fe_clk > 0)
> Can that clock be zero at all?
not if the code is correct
>
>> + if_freq_shift /= (u64)state->fe_clk;
>> + reg_value = ((u32)if_freq_shift) & 0x1fffffff;
>
> Just thinking what happens here. So you have a signed number, which is
> cast to unsigned and masked to X bit len. I think there is no need for
> signed at all.
> 1) s_if = u_if;
> 2) s_if <<= 29;
> 3) s_if /= clk;
> 4) u_32tmp = s_if;
> 5) u_32val = u_32tmp & 0x1fffffff;
>
Not yet, but see comment above.
>
>> +
>> + si2165_writereg32(state, 0x00e8 /* if_freq_shift */, reg_value);
>> + }
>> +
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> + si2165_writereg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, 0x00);
>> + si2165_writereg8(state, 0x00c0 /* rst_all */, 0x00);
>> + si2165_writereg32(state, 0x0384 /* gp_reg0 */, 0x00000000);
>> + si2165_writereg8(state, 0x02e0 /* start_synchro */, 0x01);
>> + si2165_readreg8(state, 0x0341 /*
>> boot_done,rst_wdog_error,wdog_error */, val); /* returned 0x01 */
>> +
>> + return 0;
>> +}
>> +
>> +static void si2165_release(struct dvb_frontend *fe)
>> +{
>> + struct si2165_state *state = fe->demodulator_priv;
>> + dprintk("%s: called\n", __func__);
>> + kfree(state);
>> +}
>> +
>> +static struct dvb_frontend_ops si2165_ops = {
>> + .info = {
>> + .name = "SI2165",
>
> Nit, but that chip is Si2165 not SI2165. Same everywhere.
> Also use full name "Silicon Labs Si2165" as that string is shown to
> application and user.
>
> Also, delsys are missing.
>
delsys is filled in later in preparation for si2161 support.
>
>> + .caps = FE_CAN_FEC_1_2 |
>> + FE_CAN_FEC_2_3 |
>> + FE_CAN_FEC_3_4 |
>> + FE_CAN_FEC_5_6 |
>> + FE_CAN_FEC_7_8 |
>> + FE_CAN_FEC_AUTO |
>> + FE_CAN_QPSK |
>> + FE_CAN_QAM_16 |
>> + FE_CAN_QAM_32 |
>> + FE_CAN_QAM_64 |
>> + FE_CAN_QAM_128 |
>> + FE_CAN_QAM_256 |
>> + FE_CAN_QAM_AUTO |
>> + FE_CAN_TRANSMISSION_MODE_AUTO |
>> + FE_CAN_GUARD_INTERVAL_AUTO |
>> + FE_CAN_HIERARCHY_AUTO |
>> + FE_CAN_MUTE_TS |
>> + FE_CAN_TRANSMISSION_MODE_AUTO |
>> + FE_CAN_RECOVER
>> + },
>> +
>> + .get_tune_settings = si2165_get_tune_settings,
>> +
>> + .init = si2165_init,
>> + .sleep = si2165_sleep,
>> +
>> + .i2c_gate_ctrl = si2165_i2c_gate_ctrl,
>> +
>> + .set_frontend = si2165_set_parameters,
>> + .read_status = si2165_read_status,
>> +
>> + .release = si2165_release,
>> +};
>> +
>> +struct dvb_frontend *si2165_attach(const struct si2165_config
>> *config, struct i2c_adapter *i2c)
>> +{
>> + struct si2165_state *state = NULL;
>> + int n;
>> +
>> + if (config == NULL)
>> + goto error;
>> +
>> + /* allocate memory for the internal state */
>> + state = kzalloc(sizeof(struct si2165_state), GFP_KERNEL);
>> + if (state == NULL)
>> + goto error;
>> +
>> + /* setup the state */
>> + state->i2c = i2c;
>> + state->config = *config;
>> +
>> + if (state->config.ref_freq_MHz < 4 || state->config.ref_freq_MHz
>> > 27) {
>> + dev_info(&state->i2c->dev, "%s: ref_freq of %d MHz not
>> supported by this driver\n",
>> + KBUILD_MODNAME, state->config.ref_freq_MHz);
>> + goto error;
>> + }
>
> That is error case, dev_err.
changed.
>
>> +
>> + /* create dvb_frontend */
>> + memcpy(&state->frontend.ops, &si2165_ops,
>> + sizeof(struct dvb_frontend_ops));
>> + state->frontend.demodulator_priv = state;
>
>> +
>> + /* powerup */
>> + if (si2165_writereg8(state, 0x0000 /* chip_mode */,
>> state->config.chip_mode) < 0)
>> + goto error;
>
> That kind of functionality is not allowed inside if () condition.
> Checkpatch.pl should complain that too.
fixed. It apparently here does not warn here.
>
> Do you really need powerup chip to read chip ID?
Yes, only the mode and i2c gate register can be accessed without powerup.
>
>> +
>> + if (si2165_readreg8(state, 0x0023 /* rev code */,
>> &state->revcode) < 0)
>> + goto error;
>
> Bail out immediately if wrong rev.
At least the device should be powered down before.
>
>> +
>> + if (si2165_readreg8(state, 0x0118 /* chip type */,
>> &state->chip_type) < 0)
>> + goto error;
>> +
>
> Bail out immediately if wrong type.
>
>> + /* powerdown */
>> + if (si2165_writereg8(state, 0x0000 /* chip_mode */,
>> SI2165_MODE_OFF) < 0)
>> + goto error;
>> +
>> + dev_info(&state->i2c->dev, "%s: hardware revision 0x%02x, chip
>> type 0x%02x\n",
>> + KBUILD_MODNAME, state->revcode, state->chip_type);
>> +
>> + if (state->revcode != 0x03) {
>> + dev_err(&state->i2c->dev, "%s: Unsupported hardware
>> revision.\n",
>> + KBUILD_MODNAME);
>> + goto error;
>> + }
>> +
>> + /* It is a guess that register 0x0118 (chip type?) can be used to
>> + * differ between si2161, si2163 and si2165
>> + * Only si2165 has been tested.
>> + */
>> + if (state->chip_type == 0x07) {
>> + state->m_has_dvbt = true;
>> + state->m_has_dvbc = true;
>> + strcpy(state->frontend.ops.info.name, "SI2165");
>> + } else {
>> + dev_err(&state->i2c->dev, "%s: Unsupported Chip type.\n",
>> + KBUILD_MODNAME);
>> + goto error;
>> + }
>> +
>> + n = 0;
>> + if (state->m_has_dvbt) {
>> + state->frontend.ops.delsys[n++] = SYS_DVBT;
>> + strlcat(state->frontend.ops.info.name, " DVB-T",
>> + sizeof(state->frontend.ops.info.name));
>> + }
>> + if (state->m_has_dvbc)
>> + dev_warn(&state->i2c->dev, "%s: DVB-C is not yet supported.\n",
>> + KBUILD_MODNAME);
>> +
>> + return &state->frontend;
>> +
>> +error:
>> + kfree(state);
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(si2165_attach);
>> +
>> +module_param(debug, int, 0644);
>> +MODULE_PARM_DESC(debug, "Turn on/off frontend debugging
>> (default:off).");
>> +
>> +MODULE_DESCRIPTION("Silicon Labs si2165 DVB-C/-T Demodulator driver");
>> +MODULE_AUTHOR("Matthias Schwarzott <zzam@gentoo.org>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_FIRMWARE(SI2165_FIRMWARE);
>> diff --git a/drivers/media/dvb-frontends/si2165.h
>> b/drivers/media/dvb-frontends/si2165.h
>> new file mode 100644
>> index 0000000..cdc12e7
>> --- /dev/null
>> +++ b/drivers/media/dvb-frontends/si2165.h
>> @@ -0,0 +1,61 @@
>> +/*
>> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
>> +
>> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
>> +
>> + 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.
>> +
>> + References:
>> +
>> http://www.silabs.com/Support%20Documents/TechnicalDocs/Si2165-short.pdf
>> +*/
>> +
>> +#ifndef _DVB_SI2165_H
>> +#define _DVB_SI2165_H
>> +
>> +#include <linux/dvb/frontend.h>
>> +
>> +#if IS_ENABLED(CONFIG_DVB_SI2165)
>> +
>> +enum {
>> + SI2165_MODE_OFF = 0x00,
>> + SI2165_MODE_PLL_EXT = 0x20,
>> + SI2165_MODE_PLL_XTAL = 0x21
>> +};
>> +
>> +struct si2165_config {
>> + /* i2c addr
>> + * possible values: 0x64,0x65,0x66,0x67 */
>> + u8 i2c_addr;
>> +
>> + /* external clock or XTAL */
>> + u8 chip_mode;
>> +
>> + /* frequency of external clock or xtal in Mhz
>> + * possible values: 4,16,20,24,27 in
>> + */
>> + u8 ref_freq_MHz;
>> +};
>> +
>> +/* Addresses: 0x64,0x65,0x66,0x67 */
>> +struct dvb_frontend *si2165_attach(
>> + const struct si2165_config *config,
>> + struct i2c_adapter *i2c);
>> +#else
>> +static inline struct dvb_frontend *si2165_attach(
>> + const struct si2165_config *config,
>> + struct i2c_adapter *i2c)
>> +{
>> + pr_warn("%s: driver disabled by Kconfig\n", __func__);
>> + return NULL;
>> +}
>> +#endif /* CONFIG_DVB_SI2165 */
>> +
>> +#endif /* _DVB_SI2165_H */
>> diff --git a/drivers/media/dvb-frontends/si2165_priv.h
>> b/drivers/media/dvb-frontends/si2165_priv.h
>> new file mode 100644
>> index 0000000..d4cc93f
>> --- /dev/null
>> +++ b/drivers/media/dvb-frontends/si2165_priv.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + Driver for Silicon Labs SI2165 DVB-C/-T Demodulator
>> +
>> + Copyright (C) 2013-2014 Matthias Schwarzott <zzam@gentoo.org>
>> +
>> + 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.
>> +
>> +*/
>> +
>> +#ifndef _DVB_SI2165_PRIV
>> +#define _DVB_SI2165_PRIV
>> +
>> +#define SI2165_FIRMWARE "dvb-demod-si2165.fw"
>> +
>> +#endif /* _DVB_SI2165_PRIV */
>>
>
> There is almost none I/O error checks, but it is not so big issue.
>
> That driver could be a little bit modern in a following ways:
> 1) dynamic debugs
> 2) I2C client driver model
> 3) RegMap API
> 4) I2C mux adapter for tuner I2C bus / gate
>
> Maybe 30% less LOC.
>
> regards
> Antti
I hope to reduce LOC by using register data tables instead of long
chains of register writes. But mixing 8 bits and larger writes makes
this complicated.
1) I could also write the larger registers byte by byte -> bad performance.
2) store them byte by byte and let the register array write function
collect them.
3) store them together with a size indicator -> wasted space when always
reserving 32bits for the value and normally using only 8bits.
I will send the next version later.
Regards
Matthias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] si2165: Add demod driver for DVB-T only
2014-07-01 19:01 ` Matthias Schwarzott
@ 2014-07-01 19:07 ` Antti Palosaari
0 siblings, 0 replies; 6+ messages in thread
From: Antti Palosaari @ 2014-07-01 19:07 UTC (permalink / raw)
To: Matthias Schwarzott, linux-media; +Cc: xpert-reactos
Moikka Matthias!
On 07/01/2014 10:01 PM, Matthias Schwarzott wrote:
> On 17.05.2014 04:14, Antti Palosaari wrote:
>> That driver could be a little bit modern in a following ways:
>> 1) dynamic debugs
>> 2) I2C client driver model
>> 3) RegMap API
>> 4) I2C mux adapter for tuner I2C bus / gate
>>
>> Maybe 30% less LOC.
>>
>> regards
>> Antti
>
> I hope to reduce LOC by using register data tables instead of long
> chains of register writes. But mixing 8 bits and larger writes makes
> this complicated.
> 1) I could also write the larger registers byte by byte -> bad performance.
> 2) store them byte by byte and let the register array write function
> collect them.
> 3) store them together with a size indicator -> wasted space when always
> reserving 32bits for the value and normally using only 8bits.
>
> I will send the next version later.
Just send your current driver out and improve it later in order to go
ahead. There is many devices waiting that driver... :D
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-01 19:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-26 20:21 [PATCH 1/3] si2165: Add demod driver for DVB-T only Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 2/3] cx231xx: Add [2040:b130] Hauppauge WinTV 930C-hd 1113xx Matthias Schwarzott
2014-04-26 20:21 ` [PATCH 3/3] cx23885: Add si2165 support for HVR-5500 Matthias Schwarzott
2014-05-17 2:14 ` [PATCH 1/3] si2165: Add demod driver for DVB-T only Antti Palosaari
2014-07-01 19:01 ` Matthias Schwarzott
2014-07-01 19:07 ` Antti Palosaari
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).