* [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge)
@ 2011-07-03 21:21 Oliver Endriss
2011-07-03 21:23 ` PATCH 1/5] ddbridge: Initial check-in Oliver Endriss
` (5 more replies)
0 siblings, 6 replies; 45+ messages in thread
From: Oliver Endriss @ 2011-07-03 21:21 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab
[PATCH 1/5] ddbridge: Initial check-in
[PATCH 2/5] ddbridge: Codingstyle fixes
[PATCH 3/5] ddbridge: Allow compiling of the driver
[PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n
[PATCH 5/5] cxd2099: Update Kconfig descrition (ddbridge support)
Note:
This patch series depends on the previous one:
[PATCH 00/16] New drivers: DRX-K, TDA18271c2, Updates: CXD2099 and ngene
CU
Oliver
--
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
----------------------------------------------------------------
^ permalink raw reply [flat|nested] 45+ messages in thread* PATCH 1/5] ddbridge: Initial check-in 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss @ 2011-07-03 21:23 ` Oliver Endriss 2011-07-03 21:24 ` [PATCH 2/5] ddbridge: Codingstyle fixes Oliver Endriss ` (4 subsequent siblings) 5 siblings, 0 replies; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 21:23 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab From: Ralph Metzler <rmetzler@digitaldevices.de> Driver support for Digital Devices ddbridge-based cards: Octopus, Octopus mini, Octopus LE, cineS2(v6) with DuoFlex S2 and/or DuoFlex CT tuners. Driver was taken from ddbridge-0.6.1.tar.bz2. Signed-off-by: Ralph Metzler <rmetzler@digitaldevices.de> Signed-off-by: Oliver Endriss <o.endriss@gmx.de> --- drivers/media/dvb/ddbridge/ddbridge-core.c | 1690 ++++++++++++++++++++++++++++ drivers/media/dvb/ddbridge/ddbridge-regs.h | 151 +++ drivers/media/dvb/ddbridge/ddbridge.h | 187 +++ 3 files changed, 2028 insertions(+), 0 deletions(-) create mode 100644 drivers/media/dvb/ddbridge/ddbridge-core.c create mode 100644 drivers/media/dvb/ddbridge/ddbridge-regs.h create mode 100644 drivers/media/dvb/ddbridge/ddbridge.h diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c new file mode 100644 index 0000000..ba9974b --- /dev/null +++ b/drivers/media/dvb/ddbridge/ddbridge-core.c @@ -0,0 +1,1690 @@ +/* + * ddbridge.c: Digital Devices PCIe bridge driver + * + * Copyright (C) 2010-2011 Digital Devices GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 only, as published by the Free Software Foundation. + * + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA + * Or, point your browser to http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/module.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/delay.h> +#include <linux/slab.h> +#include <linux/poll.h> +#include <asm/io.h> +#include <linux/pci.h> +#include <linux/pci_ids.h> +#include <linux/timer.h> +#include <linux/version.h> +#include <linux/i2c.h> +#include <linux/swab.h> +#include <linux/vmalloc.h> +#include "ddbridge.h" + +#include "ddbridge-regs.h" + +#include "tda18271c2dd.h" +#include "stv6110x.h" +#include "stv090x.h" +#include "lnbh24.h" +#include "drxk.h" + +DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); + +/* MSI had problems with lost interrupts, fixed but needs testing */ +#undef CONFIG_PCI_MSI + +/******************************************************************************/ + +static int i2c_read(struct i2c_adapter *adapter, u8 adr, u8 *val) +{ + struct i2c_msg msgs[1] = {{.addr = adr, .flags = I2C_M_RD, + .buf = val, .len = 1 }}; + return (i2c_transfer(adapter, msgs, 1) == 1) ? 0 : -1; +} + +static int i2c_read_reg(struct i2c_adapter *adapter, u8 adr, u8 reg, u8 *val) +{ + struct i2c_msg msgs[2] = {{.addr = adr, .flags = 0, + .buf = ®, .len = 1 }, + {.addr = adr, .flags = I2C_M_RD, + .buf = val, .len = 1 }}; + return (i2c_transfer(adapter, msgs, 2) == 2) ? 0 : -1; +} + +static int i2c_read_reg16(struct i2c_adapter *adapter, u8 adr, + u16 reg, u8 *val) +{ + u8 msg[2] = {reg>>8, reg&0xff}; + struct i2c_msg msgs[2] = {{.addr = adr, .flags = 0, + .buf = msg, .len = 2}, + {.addr = adr, .flags = I2C_M_RD, + .buf = val, .len = 1}}; + return (i2c_transfer(adapter, msgs, 2) == 2) ? 0 : -1; +} + +static int ddb_i2c_cmd(struct ddb_i2c *i2c, u32 adr, u32 cmd) +{ + struct ddb *dev = i2c->dev; + int stat; + u32 val; + + i2c->done = 0; + ddbwritel((adr << 9) | cmd, i2c->regs + I2C_COMMAND); + stat = wait_event_timeout(i2c->wq, i2c->done == 1, HZ); + if (stat <= 0) { + printk("I2C timeout\n"); + { /* MSI debugging*/ + u32 istat = ddbreadl(INTERRUPT_STATUS); + printk("IRS %08x\n", istat); + ddbwritel(istat, INTERRUPT_ACK); + } + return -EIO; + } + val=ddbreadl(i2c->regs+I2C_COMMAND); + if (val & 0x70000) + return -EIO; + return 0; +} + +static int ddb_i2c_master_xfer(struct i2c_adapter *adapter, + struct i2c_msg msg[], int num) +{ + struct ddb_i2c *i2c = (struct ddb_i2c *)i2c_get_adapdata(adapter); + struct ddb *dev = i2c->dev; + u8 addr=0; + + if (num) + addr = msg[0].addr; + + if (num == 2 && msg[1].flags & I2C_M_RD && + !(msg[0].flags & I2C_M_RD)) { + memcpy_toio(dev->regs + I2C_TASKMEM_BASE + i2c->wbuf, + msg[0].buf,msg[0].len); + ddbwritel(msg[0].len|(msg[1].len << 16), + i2c->regs+I2C_TASKLENGTH); + if (!ddb_i2c_cmd(i2c, addr, 1)) { + memcpy_fromio(msg[1].buf, + dev->regs + I2C_TASKMEM_BASE + i2c->rbuf, + msg[1].len); + return num; + } + } + + if (num == 1 && !(msg[0].flags & I2C_M_RD)) { + ddbcpyto(I2C_TASKMEM_BASE + i2c->wbuf,msg[0].buf, msg[0].len); + ddbwritel(msg[0].len, i2c->regs + I2C_TASKLENGTH); + if (!ddb_i2c_cmd(i2c, addr, 2)) + return num; + } + if (num == 1 && (msg[0].flags & I2C_M_RD)) { + ddbwritel(msg[0].len << 16, i2c->regs + I2C_TASKLENGTH); + if (!ddb_i2c_cmd(i2c, addr, 3)) { + ddbcpyfrom(msg[0].buf, + I2C_TASKMEM_BASE + i2c->rbuf, msg[0].len); + return num; + } + } + return -EIO; +} + + +static u32 ddb_i2c_functionality(struct i2c_adapter *adap) +{ + return I2C_FUNC_SMBUS_EMUL; +} + +struct i2c_algorithm ddb_i2c_algo = { + .master_xfer = ddb_i2c_master_xfer, + .functionality = ddb_i2c_functionality, +}; + +static void ddb_i2c_release(struct ddb *dev) +{ + int i; + struct ddb_i2c *i2c; + struct i2c_adapter *adap; + + for (i = 0; i < dev->info->port_num; i++) { + i2c = &dev->i2c[i]; + adap = &i2c->adap; + i2c_del_adapter(adap); + } +} + +static int ddb_i2c_init(struct ddb *dev) +{ + int i, j, stat = 0; + struct ddb_i2c *i2c; + struct i2c_adapter *adap; + + for (i = 0; i < dev->info->port_num; i++) { + i2c = &dev->i2c[i]; + i2c->dev = dev; + i2c->nr = i; + i2c->wbuf = i * (I2C_TASKMEM_SIZE / 4); + i2c->rbuf = i2c->wbuf + (I2C_TASKMEM_SIZE / 8); + i2c->regs = 0x80 + i * 0x20; + ddbwritel(I2C_SPEED_100, i2c->regs + I2C_TIMING); + ddbwritel((i2c->rbuf << 16) | i2c->wbuf, + i2c->regs + I2C_TASKADDRESS); + init_waitqueue_head(&i2c->wq); + + adap = &i2c->adap; + i2c_set_adapdata(adap, i2c); +#ifdef I2C_ADAP_CLASS_TV_DIGITAL + adap->class = I2C_ADAP_CLASS_TV_DIGITAL|I2C_CLASS_TV_ANALOG; +#else +#ifdef I2C_CLASS_TV_ANALOG + adap->class = I2C_CLASS_TV_ANALOG; +#endif +#endif + strcpy(adap->name, "ddbridge"); + adap->algo = &ddb_i2c_algo; + adap->algo_data = (void *)i2c; + adap->dev.parent = &dev->pdev->dev; + stat = i2c_add_adapter(adap); + if (stat) + break; + } + if (stat) + for (j = 0; j < i; j++) { + i2c = &dev->i2c[j]; + adap = &i2c->adap; + i2c_del_adapter(adap); + } + return stat; +} + + +/******************************************************************************/ +/******************************************************************************/ +/******************************************************************************/ + +static void set_table(struct ddb *dev, u32 off, + dma_addr_t *pbuf, u32 num) +{ + u32 i, base; + u64 mem; + + base = DMA_BASE_ADDRESS_TABLE + off; + for (i = 0; i < num; i++) { + mem = pbuf[i]; + ddbwritel(mem & 0xffffffff, base + i * 8); + ddbwritel(mem >> 32, base + i * 8 + 4); + } +} + +static void ddb_address_table(struct ddb *dev) +{ + u32 i, j, base; + u64 mem; + dma_addr_t *pbuf; + + for (i = 0; i < dev->info->port_num * 2; i++) { + base = DMA_BASE_ADDRESS_TABLE + i * 0x100; + pbuf = dev->input[i].pbuf; + for (j = 0; j < dev->input[i].dma_buf_num; j++) { + mem = pbuf[j]; + ddbwritel(mem & 0xffffffff, base + j * 8); + ddbwritel(mem >> 32, base + j * 8 + 4); + } + } + for (i = 0; i < dev->info->port_num; i++) { + base = DMA_BASE_ADDRESS_TABLE + 0x800 + i * 0x100; + pbuf = dev->output[i].pbuf; + for (j = 0; j < dev->output[i].dma_buf_num; j++) { + mem = pbuf[j]; + ddbwritel(mem & 0xffffffff, base + j * 8); + ddbwritel(mem >> 32, base + j * 8 + 4); + } + } +} + +static void io_free(struct pci_dev *pdev, u8 **vbuf, + dma_addr_t *pbuf, u32 size, int num) +{ + int i; + + for (i = 0; i < num; i++) { + if (vbuf[i]) { + pci_free_consistent(pdev, size, vbuf[i], pbuf[i]); + vbuf[i] = 0; + } + } +} + +static int io_alloc(struct pci_dev *pdev, u8 **vbuf, + dma_addr_t *pbuf, u32 size, int num) +{ + int i; + + for (i = 0; i < num; i++) { + vbuf[i] = pci_alloc_consistent(pdev, size, &pbuf[i]); + if (!vbuf[i]) + return -ENOMEM; + } + return 0; +} + +static int ddb_buffers_alloc(struct ddb *dev) +{ + int i; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + switch (port->class) { + case DDB_PORT_TUNER: + if (io_alloc(dev->pdev, port->input[0]->vbuf, + port->input[0]->pbuf, + port->input[0]->dma_buf_size, + port->input[0]->dma_buf_num) < 0) + return -1; + if (io_alloc(dev->pdev, port->input[1]->vbuf, + port->input[1]->pbuf, + port->input[1]->dma_buf_size, + port->input[1]->dma_buf_num) < 0) + return -1; + break; + case DDB_PORT_CI: + if (io_alloc(dev->pdev, port->input[0]->vbuf, + port->input[0]->pbuf, + port->input[0]->dma_buf_size, + port->input[0]->dma_buf_num) < 0) + return -1; + if (io_alloc(dev->pdev, port->output->vbuf, + port->output->pbuf, + port->output->dma_buf_size, + port->output->dma_buf_num) < 0) + return -1; + break; + default: + break; + } + } + ddb_address_table(dev); + return 0; +} + +static void ddb_buffers_free(struct ddb *dev) +{ + int i; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + io_free(dev->pdev, port->input[0]->vbuf, + port->input[0]->pbuf, + port->input[0]->dma_buf_size, + port->input[0]->dma_buf_num); + io_free(dev->pdev, port->input[1]->vbuf, + port->input[1]->pbuf, + port->input[1]->dma_buf_size, + port->input[1]->dma_buf_num); + io_free(dev->pdev, port->output->vbuf, + port->output->pbuf, + port->output->dma_buf_size, + port->output->dma_buf_num); + } +} + +static void ddb_input_start(struct ddb_input *input) +{ + struct ddb *dev = input->port->dev; + + spin_lock_irq(&input->lock); + input->cbuf = 0; + input->coff = 0; + + /* reset */ + ddbwritel(0, TS_INPUT_CONTROL(input->nr)); + ddbwritel(2, TS_INPUT_CONTROL(input->nr)); + ddbwritel(0, TS_INPUT_CONTROL(input->nr)); + + ddbwritel((1 << 16) | + (input->dma_buf_num << 11) | + (input->dma_buf_size >> 7), + DMA_BUFFER_SIZE(input->nr)); + ddbwritel(0, DMA_BUFFER_ACK(input->nr)); + + ddbwritel(1, DMA_BASE_WRITE); + ddbwritel(3, DMA_BUFFER_CONTROL(input->nr)); + ddbwritel(9, TS_INPUT_CONTROL(input->nr)); + input->running = 1; + spin_unlock_irq(&input->lock); +} + +static void ddb_input_stop(struct ddb_input *input) +{ + struct ddb *dev = input->port->dev; + + spin_lock_irq(&input->lock); + ddbwritel(0, TS_INPUT_CONTROL(input->nr)); + ddbwritel(0, DMA_BUFFER_CONTROL(input->nr)); + input->running = 0; + spin_unlock_irq(&input->lock); +} + +static void ddb_output_start(struct ddb_output *output) +{ + struct ddb *dev = output->port->dev; + + spin_lock_irq(&output->lock); + output->cbuf = 0; + output->coff = 0; + ddbwritel(0, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel(2, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel(0, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel(0x3c, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel((1 << 16) | + (output->dma_buf_num << 11) | + (output->dma_buf_size >> 7), + DMA_BUFFER_SIZE(output->nr + 8)); + ddbwritel(0, DMA_BUFFER_ACK(output->nr + 8)); + + ddbwritel(1, DMA_BASE_READ); + ddbwritel(3, DMA_BUFFER_CONTROL(output->nr + 8)); + //ddbwritel(0xbd, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel(0x1d, TS_OUTPUT_CONTROL(output->nr)); + output->running = 1; + spin_unlock_irq(&output->lock); +} + +static void ddb_output_stop(struct ddb_output *output) +{ + struct ddb *dev = output->port->dev; + + spin_lock_irq(&output->lock); + ddbwritel(0, TS_OUTPUT_CONTROL(output->nr)); + ddbwritel(0, DMA_BUFFER_CONTROL(output->nr + 8)); + output->running = 0; + spin_unlock_irq(&output->lock); +} + +static u32 ddb_output_free(struct ddb_output *output) +{ + u32 idx, off, stat = output->stat; + s32 diff; + + idx = (stat >> 11) & 0x1f; + off = (stat & 0x7ff) << 7; + + if (output->cbuf != idx) { + if ((((output->cbuf + 1) % output->dma_buf_num) == idx) && + (output->dma_buf_size - output->coff <= 188)) + return 0; + return 188; + } + diff = off - output->coff; + if (diff <= 0 || diff > 188) + return 188; + return 0; +} + +static ssize_t ddb_output_write(struct ddb_output* output, + const u8 *buf, size_t count) +{ + struct ddb *dev = output->port->dev; + u32 idx, off, stat = output->stat; + u32 left = count, len; + + idx = (stat >> 11) & 0x1f; + off = (stat & 0x7ff) << 7; + + while (left) { + len = output->dma_buf_size - output->coff; + if ((((output->cbuf + 1) % output->dma_buf_num) == idx) && + (off == 0)) { + if (len<=188) + break; + len-=188; + } + if (output->cbuf == idx) { + if (off > output->coff) { +#if 1 + len = off - output->coff; + len -= (len % 188); + if (len <= 188) + +#endif + break; + len -= 188; + } + } + if (len > left) + len = left; + if (copy_from_user(output->vbuf[output->cbuf] + output->coff, + buf, len)) + return -EIO; + left -= len; + buf += len; + output->coff += len; + if (output->coff == output->dma_buf_size) { + output->coff = 0; + output->cbuf = ((output->cbuf + 1) % output->dma_buf_num); + } + ddbwritel((output->cbuf << 11) | (output->coff >> 7), + DMA_BUFFER_ACK(output->nr + 8)); + } + return count - left; +} + +static u32 ddb_input_avail(struct ddb_input *input) +{ + struct ddb *dev = input->port->dev; + u32 idx, off, stat = input->stat; + u32 ctrl = ddbreadl(DMA_BUFFER_CONTROL(input->nr)); + + idx = (stat >> 11) & 0x1f; + off = (stat & 0x7ff) << 7; + + if (ctrl & 4) { + printk("IA %d %d %08x\n", idx, off, ctrl); + ddbwritel(input->stat, DMA_BUFFER_ACK(input->nr)); + return 0; + } + if (input->cbuf != idx) + return 188; + return 0; +} + +static size_t ddb_input_read(struct ddb_input *input, u8 *buf, size_t count) +{ + struct ddb *dev = input->port->dev; + u32 left = count; + u32 idx, off, free, stat = input->stat; + int ret; + + idx = (stat >> 11) & 0x1f; + off = (stat & 0x7ff) << 7; + + while (left) { + if (input->cbuf == idx) + return count - left; + free = input->dma_buf_size - input->coff; + if (free > left) + free = left; + ret = copy_to_user(buf, input->vbuf[input->cbuf] + + input->coff, free); + input->coff += free; + if (input->coff == input->dma_buf_size) { + input->coff = 0; + input->cbuf = (input->cbuf+1) % input->dma_buf_num; + } + left -= free; + ddbwritel((input->cbuf << 11) | (input->coff >> 7), + DMA_BUFFER_ACK(input->nr)); + } + return count; +} + +/******************************************************************************/ +/******************************************************************************/ +/******************************************************************************/ + +#if 0 +static struct ddb_input *fe2input(struct ddb *dev, struct dvb_frontend *fe) +{ + int i; + + for (i = 0; i < dev->info->port_num * 2; i++) { + if (dev->input[i].fe==fe) + return &dev->input[i]; + } + return NULL; +} +#endif + +static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable) +{ + struct ddb_input *input = fe->sec_priv; + struct ddb_port *port = input->port; + int status; + + if (enable) { + mutex_lock(&port->i2c_gate_lock); + status = input->gate_ctrl(fe, 1); + } else { + status = input->gate_ctrl(fe, 0); + mutex_unlock(&port->i2c_gate_lock); + } + return status; +} + +static int demod_attach_drxk(struct ddb_input *input) +{ + struct i2c_adapter *i2c = &input->port->i2c->adap; + struct dvb_frontend *fe; + + fe=input->fe = dvb_attach(drxk_attach, + i2c, 0x29 + (input->nr&1), + &input->fe2); + if (!input->fe) { + printk("No DRXK found!\n"); + return -ENODEV; + } + fe->sec_priv = input; + input->gate_ctrl = fe->ops.i2c_gate_ctrl; + fe->ops.i2c_gate_ctrl = drxk_gate_ctrl; + return 0; +} + +static int tuner_attach_tda18271(struct ddb_input *input) +{ + struct i2c_adapter *i2c = &input->port->i2c->adap; + struct dvb_frontend *fe; + + if (input->fe->ops.i2c_gate_ctrl) + input->fe->ops.i2c_gate_ctrl(input->fe, 1); + fe = dvb_attach(tda18271c2dd_attach, input->fe, i2c, 0x60); + if (!fe) { + printk("No TDA18271 found!\n"); + return -ENODEV; + } + if (input->fe->ops.i2c_gate_ctrl) + input->fe->ops.i2c_gate_ctrl(input->fe, 0); + return 0; +} + +/******************************************************************************/ +/******************************************************************************/ +/******************************************************************************/ + +static struct stv090x_config stv0900 = { + .device = STV0900, + .demod_mode = STV090x_DUAL, + .clk_mode = STV090x_CLK_EXT, + + .xtal = 27000000, + .address = 0x69, + + .ts1_mode = STV090x_TSMODE_SERIAL_PUNCTURED, + .ts2_mode = STV090x_TSMODE_SERIAL_PUNCTURED, + + .repeater_level = STV090x_RPTLEVEL_16, + + .adc1_range = STV090x_ADC_1Vpp, + .adc2_range = STV090x_ADC_1Vpp, + + .diseqc_envelope_mode = true, +}; + +static struct stv090x_config stv0900_aa = { + .device = STV0900, + .demod_mode = STV090x_DUAL, + .clk_mode = STV090x_CLK_EXT, + + .xtal = 27000000, + .address = 0x68, + + .ts1_mode = STV090x_TSMODE_SERIAL_PUNCTURED, + .ts2_mode = STV090x_TSMODE_SERIAL_PUNCTURED, + + .repeater_level = STV090x_RPTLEVEL_16, + + .adc1_range = STV090x_ADC_1Vpp, + .adc2_range = STV090x_ADC_1Vpp, + + .diseqc_envelope_mode = true, +}; + +static struct stv6110x_config stv6110a = { + .addr = 0x60, + .refclk = 27000000, + .clk_div = 1, +}; + +static struct stv6110x_config stv6110b = { + .addr = 0x63, + .refclk = 27000000, + .clk_div = 1, +}; + +static int demod_attach_stv0900(struct ddb_input *input, int type) +{ + struct i2c_adapter *i2c = &input->port->i2c->adap; + struct stv090x_config *feconf = type ? &stv0900_aa : &stv0900; + + input->fe=dvb_attach(stv090x_attach, feconf, i2c, + (input->nr & 1) ? STV090x_DEMODULATOR_1 + : STV090x_DEMODULATOR_0); + if (!input->fe) { + printk("No STV0900 found!\n"); + return -ENODEV; + } + if (!dvb_attach(lnbh24_attach, input->fe, i2c, 0, + 0, (input->nr & 1) ? + (0x09 - type) : (0x0b - type))) { + printk("No LNBH24 found!\n"); + return -ENODEV; + } + return 0; +} + +static int tuner_attach_stv6110(struct ddb_input *input, int type) +{ + struct i2c_adapter *i2c = &input->port->i2c->adap; + struct stv090x_config *feconf = type ? &stv0900_aa : &stv0900; + struct stv6110x_config *tunerconf = (input->nr & 1) ? + &stv6110b : &stv6110a; + struct stv6110x_devctl *ctl; + + ctl = dvb_attach(stv6110x_attach, input->fe, tunerconf, i2c); + if (!ctl) { + printk("No STV6110X found!\n"); + return -ENODEV; + } + printk("attach tuner input %d adr %02x\n", input->nr, tunerconf->addr); + + feconf->tuner_init = ctl->tuner_init; + feconf->tuner_sleep = ctl->tuner_sleep; + feconf->tuner_set_mode = ctl->tuner_set_mode; + feconf->tuner_set_frequency = ctl->tuner_set_frequency; + feconf->tuner_get_frequency = ctl->tuner_get_frequency; + feconf->tuner_set_bandwidth = ctl->tuner_set_bandwidth; + feconf->tuner_get_bandwidth = ctl->tuner_get_bandwidth; + feconf->tuner_set_bbgain = ctl->tuner_set_bbgain; + feconf->tuner_get_bbgain = ctl->tuner_get_bbgain; + feconf->tuner_set_refclk = ctl->tuner_set_refclk; + feconf->tuner_get_status = ctl->tuner_get_status; + + return 0; +} + +int my_dvb_dmx_ts_card_init(struct dvb_demux *dvbdemux, char *id, + int (*start_feed)(struct dvb_demux_feed *), + int (*stop_feed)(struct dvb_demux_feed *), + void *priv) +{ + dvbdemux->priv = priv; + + dvbdemux->filternum = 256; + dvbdemux->feednum = 256; + dvbdemux->start_feed = start_feed; + dvbdemux->stop_feed = stop_feed; + dvbdemux->write_to_decoder = NULL; + dvbdemux->dmx.capabilities = (DMX_TS_FILTERING | + DMX_SECTION_FILTERING | + DMX_MEMORY_BASED_FILTERING); + return dvb_dmx_init(dvbdemux); +} + +int my_dvb_dmxdev_ts_card_init(struct dmxdev *dmxdev, + struct dvb_demux *dvbdemux, + struct dmx_frontend *hw_frontend, + struct dmx_frontend *mem_frontend, + struct dvb_adapter *dvb_adapter) +{ + int ret; + + dmxdev->filternum = 256; + dmxdev->demux = &dvbdemux->dmx; + dmxdev->capabilities = 0; + ret = dvb_dmxdev_init(dmxdev, dvb_adapter); + if (ret < 0) + return ret; + + hw_frontend->source = DMX_FRONTEND_0; + dvbdemux->dmx.add_frontend(&dvbdemux->dmx, hw_frontend); + mem_frontend->source = DMX_MEMORY_FE; + dvbdemux->dmx.add_frontend(&dvbdemux->dmx, mem_frontend); + return dvbdemux->dmx.connect_frontend(&dvbdemux->dmx, hw_frontend); +} + +static int start_feed(struct dvb_demux_feed *dvbdmxfeed) +{ + struct dvb_demux *dvbdmx = dvbdmxfeed->demux; + struct ddb_input *input = dvbdmx->priv; + + if (!input->users) + ddb_input_start(input); + + return ++input->users; +} + +static int stop_feed(struct dvb_demux_feed *dvbdmxfeed) +{ + struct dvb_demux *dvbdmx = dvbdmxfeed->demux; + struct ddb_input *input = dvbdmx->priv; + + if (--input->users) + return input->users; + + ddb_input_stop(input); + return 0; +} + + +static void dvb_input_detach(struct ddb_input *input) +{ + struct dvb_adapter *adap = &input->adap; + struct dvb_demux *dvbdemux = &input->demux; + + switch (input->attached) { + case 5: + if (input->fe2) + dvb_unregister_frontend(input->fe2); + if (input->fe) { + dvb_unregister_frontend(input->fe); + dvb_frontend_detach(input->fe); + input->fe = NULL; + } + case 4: + dvb_net_release(&input->dvbnet); + + case 3: + dvbdemux->dmx.close(&dvbdemux->dmx); + dvbdemux->dmx.remove_frontend(&dvbdemux->dmx, + &input->hw_frontend); + dvbdemux->dmx.remove_frontend(&dvbdemux->dmx, + &input->mem_frontend); + dvb_dmxdev_release(&input->dmxdev); + + case 2: + dvb_dmx_release(&input->demux); + + case 1: + dvb_unregister_adapter(adap); + } + input->attached = 0; +} + +static int dvb_input_attach(struct ddb_input *input) +{ + int ret; + struct ddb_port *port = input->port; + struct dvb_adapter *adap = &input->adap; + struct dvb_demux *dvbdemux = &input->demux; + + ret=dvb_register_adapter(adap, "DDBridge",THIS_MODULE, + &input->port->dev->pdev->dev, + adapter_nr); + if (ret < 0) { + printk("ddbridge: Could not register adapter." + "Check if you enabled enough adapters in dvb-core!\n"); + return ret; + } + input->attached = 1; + + ret = my_dvb_dmx_ts_card_init(dvbdemux, "SW demux", + start_feed, + stop_feed, input); + if (ret < 0) + return ret; + input->attached = 2; + + ret = my_dvb_dmxdev_ts_card_init(&input->dmxdev, &input->demux, + &input->hw_frontend, + &input->mem_frontend, adap); + if (ret < 0) + return ret; + input->attached = 3; + + ret = dvb_net_init(adap, &input->dvbnet, input->dmxdev.demux); + if (ret < 0) + return ret; + input->attached = 4; + + input->fe = 0; + switch (port->type) { + case DDB_TUNER_DVBS_ST: + if (demod_attach_stv0900(input, 0) < 0) + return -ENODEV; + if (tuner_attach_stv6110(input, 0) < 0) + return -ENODEV; + if (input->fe) { + if (dvb_register_frontend(adap, input->fe) < 0) + return -ENODEV; + } + break; + case DDB_TUNER_DVBS_ST_AA: + if (demod_attach_stv0900(input, 1) < 0) + return -ENODEV; + if (tuner_attach_stv6110(input, 1) < 0) + return -ENODEV; + if (input->fe) { + if (dvb_register_frontend(adap, input->fe) < 0) + return -ENODEV; + } + break; + case DDB_TUNER_DVBCT_TR: + if (demod_attach_drxk(input) < 0) + return -ENODEV; + if (tuner_attach_tda18271(input) < 0) + return -ENODEV; + if (input->fe) { + if (dvb_register_frontend(adap, input->fe) < 0) + return -ENODEV; + } + if (input->fe2) { + if (dvb_register_frontend(adap, input->fe2) < 0) + return -ENODEV; + input->fe2->tuner_priv=input->fe->tuner_priv; + memcpy(&input->fe2->ops.tuner_ops, + &input->fe->ops.tuner_ops, + sizeof(struct dvb_tuner_ops)); + } + break; + } + input->attached = 5; + return 0; +} + +/****************************************************************************/ +/****************************************************************************/ + +static ssize_t ts_write(struct file *file, const char *buf, + size_t count, loff_t *ppos) +{ + struct dvb_device *dvbdev = file->private_data; + struct ddb_output *output = dvbdev->priv; + size_t left = count; + int stat; + + while (left) { + if (ddb_output_free(output) < 188) { + if (file->f_flags & O_NONBLOCK) + break; + if (wait_event_interruptible( + output->wq, ddb_output_free(output) >= 188) < 0) + break; + } + stat = ddb_output_write(output, buf, left); + if (stat < 0) + break; + buf += stat; + left -= stat; + } + return (left == count) ? -EAGAIN : (count - left); +} + +static ssize_t ts_read(struct file *file, char *buf, + size_t count, loff_t *ppos) +{ + struct dvb_device *dvbdev = file->private_data; + struct ddb_output *output = dvbdev->priv; + struct ddb_input *input = output->port->input[0]; + int left, read; + + count -= count % 188; + left = count; + while (left) { + if (ddb_input_avail(input) < 188) { + if (file->f_flags & O_NONBLOCK) + break; + if (wait_event_interruptible( + input->wq, ddb_input_avail(input) >= 188) < 0) + break; + } + read = ddb_input_read(input, buf, left); + left -= read; + buf += read; + } + return (left == count) ? -EAGAIN : (count - left); +} + +static unsigned int ts_poll(struct file *file, poll_table *wait) +{ + struct dvb_device *dvbdev = file->private_data; + struct ddb_output *output = dvbdev->priv; + struct ddb_input *input = output->port->input[0]; + unsigned int mask = 0; + +#if 0 + if (data_avail_to_read) + mask |= POLLIN | POLLRDNORM; + if (data_avail_to_write) + mask |= POLLOUT | POLLWRNORM; + + poll_wait(file, &read_queue, wait); + poll_wait(file, &write_queue, wait); +#endif + return mask; +} + +static struct file_operations ci_fops = { + .owner = THIS_MODULE, + .read = ts_read, + .write = ts_write, + .open = dvb_generic_open, + .release = dvb_generic_release, + .poll = ts_poll, + .mmap = 0, +}; + +static struct dvb_device dvbdev_ci = { + .priv = 0, + .readers = -1, + .writers = -1, + .users = -1, + .fops = &ci_fops, +}; + +/****************************************************************************/ +/****************************************************************************/ +/****************************************************************************/ + +static void input_tasklet(unsigned long data) +{ + struct ddb_input *input = (struct ddb_input *) data; + struct ddb *dev = input->port->dev; + + spin_lock(&input->lock); + if (!input->running) { + spin_unlock(&input->lock); + return; + } + input->stat = ddbreadl(DMA_BUFFER_CURRENT(input->nr)); + + if (input->port->class == DDB_PORT_TUNER) { + if (4&ddbreadl(DMA_BUFFER_CONTROL(input->nr))) + printk("Overflow input %d\n", input->nr); + while (input->cbuf != ((input->stat >> 11) & 0x1f) + || (4&ddbreadl(DMA_BUFFER_CONTROL(input->nr)))) { + dvb_dmx_swfilter_packets(&input->demux, + input->vbuf[input->cbuf], + input->dma_buf_size / 188); + + input->cbuf = (input->cbuf + 1) % input->dma_buf_num; + ddbwritel((input->cbuf << 11), + DMA_BUFFER_ACK(input->nr)); + input->stat = ddbreadl(DMA_BUFFER_CURRENT(input->nr)); + } + } + if (input->port->class == DDB_PORT_CI) + wake_up(&input->wq); + spin_unlock(&input->lock); +} + +static void output_tasklet(unsigned long data) +{ + struct ddb_output *output = (struct ddb_output *) data; + struct ddb *dev = output->port->dev; + + spin_lock(&output->lock); + if (!output->running) { + spin_unlock(&output->lock); + return; + } + output->stat = ddbreadl(DMA_BUFFER_CURRENT(output->nr + 8)); + wake_up(&output->wq); + spin_unlock(&output->lock); +} + + +struct cxd2099_cfg cxd_cfg = { + .bitrate = 62000, + .adr = 0x40, + .polarity = 1, + .clock_mode = 1, +}; + +static int ddb_ci_attach(struct ddb_port *port) +{ + int ret; + + ret = dvb_register_adapter(&port->output->adap, + "DDBridge", + THIS_MODULE, + &port->dev->pdev->dev, + adapter_nr); + if (ret < 0) + return ret; + port->en = cxd2099_attach(&cxd_cfg, port, &port->i2c->adap); + if (!port->en) { + dvb_unregister_adapter(&port->output->adap); + return -ENODEV; + } + ddb_input_start(port->input[0]); + ddb_output_start(port->output); + dvb_ca_en50221_init(&port->output->adap, + port->en, 0, 1); + ret=dvb_register_device(&port->output->adap, &port->output->dev, + &dvbdev_ci, (void *) port->output, + DVB_DEVICE_SEC); + return ret; +} + +static int ddb_port_attach(struct ddb_port *port) +{ + int ret = 0; + + switch (port->class) { + case DDB_PORT_TUNER: + ret = dvb_input_attach(port->input[0]); + if (ret<0) + break; + ret = dvb_input_attach(port->input[1]); + break; + case DDB_PORT_CI: + ret = ddb_ci_attach(port); + break; + default: + break; + } + if (ret < 0) + printk("port_attach on port %d failed\n", port->nr); + return ret; +} + +static int ddb_ports_attach(struct ddb *dev) +{ + int i, ret = 0; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + ret = ddb_port_attach(port); + if (ret < 0) + break; + } + return ret; +} + +static void ddb_ports_detach(struct ddb *dev) +{ + int i; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + switch (port->class) { + case DDB_PORT_TUNER: + dvb_input_detach(port->input[0]); + dvb_input_detach(port->input[1]); + break; + case DDB_PORT_CI: + if (port->output->dev) + dvb_unregister_device(port->output->dev); + if (port->en) { + ddb_input_stop(port->input[0]); + ddb_output_stop(port->output); + dvb_ca_en50221_release(port->en); + kfree(port->en); + port->en = 0; + dvb_unregister_adapter(&port->output->adap); + } + break; + } + } +} + +/****************************************************************************/ +/****************************************************************************/ + +static int port_has_ci(struct ddb_port *port) +{ + u8 val; + return (i2c_read_reg(&port->i2c->adap, 0x40, 0, &val) ? 0 : 1); +} + +static int port_has_stv0900(struct ddb_port *port) +{ + u8 val; + if (i2c_read_reg16(&port->i2c->adap, 0x69, 0xf100, &val) < 0) + return 0; + return 1; +} + +static int port_has_stv0900_aa(struct ddb_port *port) +{ + u8 val; + if (i2c_read_reg16(&port->i2c->adap, 0x68, 0xf100, &val) < 0) + return 0; + return 1; +} + +static int port_has_drxks(struct ddb_port *port) +{ + u8 val; + if (i2c_read(&port->i2c->adap, 0x29, &val) < 0) + return 0; + if (i2c_read(&port->i2c->adap, 0x2a, &val) < 0) + return 0; + return 1; +} + +static void ddb_port_probe(struct ddb_port *port) +{ + struct ddb *dev = port->dev; + char *modname = "NO MODULE"; + + port->class = DDB_PORT_NONE; + + if (port_has_ci(port)) { + modname = "CI"; + port->class = DDB_PORT_CI; + ddbwritel(I2C_SPEED_400, port->i2c->regs + I2C_TIMING); + } else if (port_has_stv0900(port)) { + modname = "DUAL DVB-S2"; + port->class = DDB_PORT_TUNER; + port->type = DDB_TUNER_DVBS_ST; + ddbwritel(I2C_SPEED_100, port->i2c->regs + I2C_TIMING); + } else if (port_has_stv0900_aa(port)) { + modname = "DUAL DVB-S2"; + port->class = DDB_PORT_TUNER; + port->type = DDB_TUNER_DVBS_ST_AA; + ddbwritel(I2C_SPEED_100, port->i2c->regs + I2C_TIMING); + } else if (port_has_drxks(port)) { + modname = "DUAL DVB-C/T"; + port->class = DDB_PORT_TUNER; + port->type = DDB_TUNER_DVBCT_TR; + ddbwritel(I2C_SPEED_400, port->i2c->regs + I2C_TIMING); + } + printk("Port %d (TAB %d): %s\n", port->nr, port->nr+1, modname); +} + +static void ddb_input_init(struct ddb_port *port, int nr) +{ + struct ddb *dev = port->dev; + struct ddb_input *input = &dev->input[nr]; + + input->nr = nr; + input->port = port; + input->dma_buf_num = INPUT_DMA_BUFS; + input->dma_buf_size = INPUT_DMA_SIZE; + ddbwritel(0, TS_INPUT_CONTROL(nr)); + ddbwritel(2, TS_INPUT_CONTROL(nr)); + ddbwritel(0, TS_INPUT_CONTROL(nr)); + ddbwritel(0, DMA_BUFFER_ACK(nr)); + tasklet_init(&input->tasklet, input_tasklet, (unsigned long) input); + spin_lock_init(&input->lock); + init_waitqueue_head(&input->wq); +} + +static void ddb_output_init(struct ddb_port *port, int nr) +{ + struct ddb *dev = port->dev; + struct ddb_output *output = &dev->output[nr]; + output->nr = nr; + output->port = port; + output->dma_buf_num = OUTPUT_DMA_BUFS; + output->dma_buf_size = OUTPUT_DMA_SIZE; + + ddbwritel(0, TS_OUTPUT_CONTROL(nr)); + ddbwritel(2, TS_OUTPUT_CONTROL(nr)); + ddbwritel(0, TS_OUTPUT_CONTROL(nr)); + tasklet_init(&output->tasklet, output_tasklet, (unsigned long) output); + init_waitqueue_head(&output->wq); +} + +static void ddb_ports_init(struct ddb *dev) +{ + int i; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + port->dev = dev; + port->nr = i; + port->i2c = &dev->i2c[i]; + port->input[0] = &dev->input[2 * i]; + port->input[1] = &dev->input[2 * i + 1]; + port->output = &dev->output[i]; + + mutex_init(&port->i2c_gate_lock); + ddb_port_probe(port); + ddb_input_init(port, 2 * i); + ddb_input_init(port, 2 * i + 1); + ddb_output_init(port, i); + } +} + +static void ddb_ports_release(struct ddb *dev) +{ + int i; + struct ddb_port *port; + + for (i = 0; i < dev->info->port_num; i++) { + port = &dev->port[i]; + port->dev = dev; + tasklet_kill(&port->input[0]->tasklet); + tasklet_kill(&port->input[1]->tasklet); + tasklet_kill(&port->output->tasklet); + } +} + +/****************************************************************************/ +/****************************************************************************/ +/****************************************************************************/ + +static void irq_handle_i2c(struct ddb *dev, int n) +{ + struct ddb_i2c *i2c = &dev->i2c[n]; + + i2c->done = 1; + wake_up(&i2c->wq); +} + +static irqreturn_t irq_handler(int irq, void *dev_id) +{ + struct ddb *dev = (struct ddb *) dev_id; + u32 s = ddbreadl(INTERRUPT_STATUS); + + if (!s) + return IRQ_NONE; + + do { + ddbwritel(s, INTERRUPT_ACK); + + if (s & 0x00000001) irq_handle_i2c(dev, 0); + if (s & 0x00000002) irq_handle_i2c(dev, 1); + if (s & 0x00000004) irq_handle_i2c(dev, 2); + if (s & 0x00000008) irq_handle_i2c(dev, 3); + + if (s & 0x00000100) tasklet_schedule(&dev->input[0].tasklet); + if (s & 0x00000200) tasklet_schedule(&dev->input[1].tasklet); + if (s & 0x00000400) tasklet_schedule(&dev->input[2].tasklet); + if (s & 0x00000800) tasklet_schedule(&dev->input[3].tasklet); + if (s & 0x00001000) tasklet_schedule(&dev->input[4].tasklet); + if (s & 0x00002000) tasklet_schedule(&dev->input[5].tasklet); + if (s & 0x00004000) tasklet_schedule(&dev->input[6].tasklet); + if (s & 0x00008000) tasklet_schedule(&dev->input[7].tasklet); + + if (s & 0x00010000) tasklet_schedule(&dev->output[0].tasklet); + if (s & 0x00020000) tasklet_schedule(&dev->output[1].tasklet); + if (s & 0x00040000) tasklet_schedule(&dev->output[2].tasklet); + if (s & 0x00080000) tasklet_schedule(&dev->output[3].tasklet); + + /* if (s & 0x000f0000) printk("%08x\n", istat); */ + } while ((s = ddbreadl(INTERRUPT_STATUS))); + + return IRQ_HANDLED; +} + +/******************************************************************************/ +/******************************************************************************/ +/******************************************************************************/ + +static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) +{ + u32 data, shift; + + if (wlen > 4) + ddbwritel(1, SPI_CONTROL); + while (wlen > 4) { + /* FIXME: check for big-endian */ + data = swab32(*(u32 *)wbuf); + wbuf += 4; + wlen -= 4; + ddbwritel(data, SPI_DATA); + while (ddbreadl(SPI_CONTROL) & 0x0004); + } + + if (rlen) + ddbwritel(0x0001 | ((wlen << (8 + 3)) & 0x1f00), SPI_CONTROL); + else + ddbwritel(0x0003 | ((wlen << (8 + 3)) & 0x1f00), SPI_CONTROL); + + data=0; + shift = ((4 - wlen) * 8); + while (wlen) { + data <<= 8; + data |= *wbuf; + wlen--; + wbuf++; + } + if (shift) + data <<= shift; + ddbwritel(data, SPI_DATA); + while (ddbreadl(SPI_CONTROL) & 0x0004); + + if (!rlen) { + ddbwritel(0, SPI_CONTROL); + return 0; + } + if (rlen > 4) + ddbwritel(1, SPI_CONTROL); + + while (rlen > 4) { + ddbwritel(0xffffffff, SPI_DATA); + while (ddbreadl(SPI_CONTROL) & 0x0004); + data = ddbreadl(SPI_DATA); + *(u32 *) rbuf = swab32(data); + rbuf += 4; + rlen -= 4; + } + ddbwritel(0x0003 | ((rlen << (8 + 3)) & 0x1F00), SPI_CONTROL); + ddbwritel(0xffffffff, SPI_DATA); + while (ddbreadl(SPI_CONTROL) & 0x0004); + + data = ddbreadl(SPI_DATA); + ddbwritel(0, SPI_CONTROL); + + if (rlen < 4) + data <<= ((4 - rlen) * 8); + + while (rlen > 0) { + *rbuf = ((data >> 24) & 0xff); + data <<= 8; + rbuf++; + rlen--; + } + return 0; +} + +#define DDB_MAGIC 'd' + +struct ddb_flashio { + __u8 *write_buf; + __u32 write_len; + __u8 *read_buf; + __u32 read_len; +}; + +#define IOCTL_DDB_FLASHIO _IOWR(DDB_MAGIC, 0x00, struct ddb_flashio) + +#define DDB_NAME "ddbridge" + +static u32 ddb_num; +static struct ddb *ddbs[32]; +static struct class *ddb_class; +static int ddb_major; + +static int ddb_open(struct inode *inode, struct file *file) +{ + struct ddb *dev = ddbs[iminor(inode)]; + + file->private_data = dev; + return 0; +} + +static long ddb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct ddb *dev = file->private_data; + void *parg = (void *)arg; + int res = -EFAULT; + + switch (cmd) { + case IOCTL_DDB_FLASHIO: + { + struct ddb_flashio fio; + u8 *rbuf, *wbuf; + + if (copy_from_user(&fio, parg, sizeof(fio))) + break; + if (fio.write_len + fio.read_len > 1028) { + printk("IOBUF too small\n"); + return -ENOMEM; + } + wbuf = &dev->iobuf[0]; + if (!wbuf) + return -ENOMEM; + rbuf = wbuf + fio.write_len; + if (copy_from_user(wbuf, fio.write_buf, fio.write_len)) { + vfree(wbuf); + break; + } + res = flashio(dev, wbuf, fio.write_len, + rbuf, fio.read_len); + if (copy_to_user(fio.read_buf, rbuf, fio.read_len)) + res = -EFAULT; + break; + } + default: + break; + } + return res; +} + +static struct file_operations ddb_fops={ + .unlocked_ioctl = ddb_ioctl, + .open = ddb_open, +}; + +static char *ddb_devnode(struct device *device, mode_t *mode) +{ + struct ddb *dev = dev_get_drvdata(device); + + return kasprintf(GFP_KERNEL, "ddbridge/card%d", dev->nr); +} + +static int ddb_class_create(void) +{ + if ((ddb_major = register_chrdev(0, DDB_NAME, &ddb_fops))<0) + return ddb_major; + + ddb_class = class_create(THIS_MODULE, DDB_NAME); + if (IS_ERR(ddb_class)) { + unregister_chrdev(ddb_major, DDB_NAME); + return -1; + } + ddb_class->devnode = ddb_devnode; + return 0; +} + +static void ddb_class_destroy(void) +{ + class_destroy(ddb_class); + unregister_chrdev(ddb_major, DDB_NAME); +} + +static int ddb_device_create(struct ddb *dev) +{ + dev->nr = ddb_num++; + dev->ddb_dev = device_create(ddb_class, NULL, + MKDEV(ddb_major, dev->nr), + dev, "ddbridge%d", dev->nr); + ddbs[dev->nr] = dev; + if (IS_ERR(dev->ddb_dev)) + return -1; + return 0; +} + +static void ddb_device_destroy(struct ddb *dev) +{ + ddb_num--; + if (IS_ERR(dev->ddb_dev)) + return; + device_destroy(ddb_class, MKDEV(ddb_major, 0)); +} + + +/****************************************************************************/ +/****************************************************************************/ +/****************************************************************************/ + +static void ddb_unmap(struct ddb *dev) +{ + if (dev->regs) + iounmap(dev->regs); + vfree(dev); +} + + +static void __devexit ddb_remove(struct pci_dev *pdev) +{ + struct ddb *dev = (struct ddb *) pci_get_drvdata(pdev); + + ddb_ports_detach(dev); + ddb_i2c_release(dev); + + ddbwritel(0, INTERRUPT_ENABLE); + free_irq(dev->pdev->irq, dev); +#ifdef CONFIG_PCI_MSI + if (dev->msi) + pci_disable_msi(dev->pdev); +#endif + ddb_ports_release(dev); + ddb_buffers_free(dev); + ddb_device_destroy(dev); + + ddb_unmap(dev); + pci_set_drvdata(pdev, 0); + pci_disable_device(pdev); +} + + +static int __devinit ddb_probe(struct pci_dev *pdev, + const struct pci_device_id *id) +{ + struct ddb *dev; + int stat=0; + int irq_flag = IRQF_SHARED; + + if (pci_enable_device(pdev)<0) + return -ENODEV; + + dev = vmalloc(sizeof(struct ddb)); + if (dev == NULL) + return -ENOMEM; + memset(dev, 0, sizeof(struct ddb)); + + dev->pdev = pdev; + pci_set_drvdata(pdev, dev); + dev->info = (struct ddb_info *) id->driver_data; + printk("DDBridge driver detected: %s\n", dev->info->name); + + dev->regs = ioremap(pci_resource_start(dev->pdev,0), + pci_resource_len(dev->pdev,0)); + if (!dev->regs) { + stat = -ENOMEM; + goto fail; + } + printk("HW %08x FW %08x\n", ddbreadl(0), ddbreadl(4)); + +#ifdef CONFIG_PCI_MSI + if (pci_msi_enabled()) + stat = pci_enable_msi(dev->pdev); + if (stat) { + printk(KERN_INFO ": MSI not available.\n"); + } else { + irq_flag = 0; + dev->msi = 1; + } +#endif + if ((stat = request_irq(dev->pdev->irq, irq_handler, + irq_flag, "DDBridge", + (void *) dev))<0) + goto fail1; + ddbwritel(0, DMA_BASE_WRITE); + ddbwritel(0, DMA_BASE_READ); + ddbwritel(0xffffffff, INTERRUPT_ACK); + ddbwritel(0xfff0f, INTERRUPT_ENABLE); + ddbwritel(0, MSI1_ENABLE); + + if (ddb_i2c_init(dev) < 0) + goto fail1; + ddb_ports_init(dev); + if (ddb_buffers_alloc(dev) < 0) { + printk(KERN_INFO ": Could not allocate buffer memory\n"); + goto fail2; + } + if (ddb_ports_attach(dev) < 0) + goto fail3; + ddb_device_create(dev); + return 0; + +fail3: + ddb_ports_detach(dev); + printk("fail3\n"); + ddb_ports_release(dev); +fail2: + printk("fail2\n"); + ddb_buffers_free(dev); +fail1: + printk("fail1\n"); + if (dev->msi) + pci_disable_msi(dev->pdev); + free_irq(dev->pdev->irq, dev); +fail: + printk("fail\n"); + ddb_unmap(dev); + pci_set_drvdata(pdev, 0); + pci_disable_device(pdev); + return -1; +} + +/******************************************************************************/ +/******************************************************************************/ +/******************************************************************************/ + +static struct ddb_info ddb_none = { + .type = DDB_NONE, + .name = "Digital Devices PCIe bridge", +}; + +static struct ddb_info ddb_octopus = { + .type = DDB_OCTOPUS, + .name = "Digital Devices Octopus DVB adapter", + .port_num = 4, +}; + +static struct ddb_info ddb_octopus_le = { + .type = DDB_OCTOPUS, + .name = "Digital Devices Octopus LE DVB adapter", + .port_num = 2, +}; + +static struct ddb_info ddb_v6 = { + .type = DDB_OCTOPUS, + .name = "Digital Devices Cine S2 V6 DVB adapter", + .port_num = 3, +}; + +#define DDVID 0xdd01 /* Digital Devices Vendor ID */ + +#define DDB_ID(_vend, _dev, _subvend,_subdev,_driverdata) { \ + .vendor = _vend, .device = _dev, \ + .subvendor = _subvend, .subdevice = _subdev, \ + .driver_data = (unsigned long)&_driverdata } + +static const struct pci_device_id ddb_id_tbl[] __devinitdata = { + DDB_ID(DDVID, 0x0002, DDVID, 0x0001, ddb_octopus), + DDB_ID(DDVID, 0x0003, DDVID, 0x0001, ddb_octopus), + DDB_ID(DDVID, 0x0003, DDVID, 0x0002, ddb_octopus_le), + DDB_ID(DDVID, 0x0003, DDVID, 0x0010, ddb_octopus), + DDB_ID(DDVID, 0x0003, DDVID, 0x0020, ddb_v6), + /* in case sub-ids got deleted in flash */ + DDB_ID(DDVID, 0x0003, PCI_ANY_ID, PCI_ANY_ID, ddb_none), + {0} +}; +MODULE_DEVICE_TABLE(pci, ddb_id_tbl); + + +static struct pci_driver ddb_pci_driver = { + .name = "DDBridge", + .id_table = ddb_id_tbl, + .probe = ddb_probe, + .remove = ddb_remove, +}; + +static __init int module_init_ddbridge(void) +{ + printk("Digital Devices PCIE bridge driver, " + "Copyright (C) 2010-11 Digital Devices GmbH\n"); + if (ddb_class_create()) + return -1; + return pci_register_driver(&ddb_pci_driver); +} + +static __exit void module_exit_ddbridge(void) +{ + pci_unregister_driver(&ddb_pci_driver); + ddb_class_destroy(); +} + +module_init(module_init_ddbridge); +module_exit(module_exit_ddbridge); + +MODULE_DESCRIPTION("Digital Devices PCIe Bridge"); +MODULE_AUTHOR("Ralph Metzler"); +MODULE_LICENSE("GPL"); +MODULE_VERSION("0.5"); diff --git a/drivers/media/dvb/ddbridge/ddbridge-regs.h b/drivers/media/dvb/ddbridge/ddbridge-regs.h new file mode 100644 index 0000000..0130073 --- /dev/null +++ b/drivers/media/dvb/ddbridge/ddbridge-regs.h @@ -0,0 +1,151 @@ +/* + * ddbridge-regs.h: Digital Devices PCIe bridge driver + * + * Copyright (C) 2010-2011 Digital Devices GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 only, as published by the Free Software Foundation. + * + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA + * Or, point your browser to http://www.gnu.org/copyleft/gpl.html + */ + +// $Id: DD-DVBBridgeV1.h 273 2010-09-17 05:03:16Z manfred $ + +// Register Definitions + +#define CUR_REGISTERMAP_VERSION 0x10000 + +#define HARDWARE_VERSION 0x00 +#define REGISTERMAP_VERSION 0x04 + +// -------------------------------------------------------------------------- +// SPI Controller + +#define SPI_CONTROL 0x10 +#define SPI_DATA 0x14 + +// -------------------------------------------------------------------------- + +// Interrupt controller +// How many MSI's are available depends on HW (Min 2 max 8) +// How many are usable also depends on Host platform + +#define INTERRUPT_BASE (0x40) + +#define INTERRUPT_ENABLE (INTERRUPT_BASE + 0x00) +#define MSI0_ENABLE (INTERRUPT_BASE + 0x00) +#define MSI1_ENABLE (INTERRUPT_BASE + 0x04) +#define MSI2_ENABLE (INTERRUPT_BASE + 0x08) +#define MSI3_ENABLE (INTERRUPT_BASE + 0x0C) +#define MSI4_ENABLE (INTERRUPT_BASE + 0x10) +#define MSI5_ENABLE (INTERRUPT_BASE + 0x14) +#define MSI6_ENABLE (INTERRUPT_BASE + 0x18) +#define MSI7_ENABLE (INTERRUPT_BASE + 0x1C) + +#define INTERRUPT_STATUS (INTERRUPT_BASE + 0x20) +#define INTERRUPT_ACK (INTERRUPT_BASE + 0x20) + +#define INTMASK_I2C1 (0x00000001) +#define INTMASK_I2C2 (0x00000002) +#define INTMASK_I2C3 (0x00000004) +#define INTMASK_I2C4 (0x00000008) + +#define INTMASK_CIRQ1 (0x00000010) +#define INTMASK_CIRQ2 (0x00000020) +#define INTMASK_CIRQ3 (0x00000040) +#define INTMASK_CIRQ4 (0x00000080) + +#define INTMASK_TSINPUT1 (0x00000100) +#define INTMASK_TSINPUT2 (0x00000200) +#define INTMASK_TSINPUT3 (0x00000400) +#define INTMASK_TSINPUT4 (0x00000800) +#define INTMASK_TSINPUT5 (0x00001000) +#define INTMASK_TSINPUT6 (0x00002000) +#define INTMASK_TSINPUT7 (0x00004000) +#define INTMASK_TSINPUT8 (0x00008000) + +#define INTMASK_TSOUTPUT1 (0x00010000) +#define INTMASK_TSOUTPUT2 (0x00020000) +#define INTMASK_TSOUTPUT3 (0x00040000) +#define INTMASK_TSOUTPUT4 (0x00080000) + +// -------------------------------------------------------------------------- +// I2C Master Controller + +#define I2C_BASE (0x80) // Byte offset + +#define I2C_COMMAND (0x00) +#define I2C_TIMING (0x04) +#define I2C_TASKLENGTH (0x08) // High read, low write +#define I2C_TASKADDRESS (0x0C) // High read, low write + +#define I2C_MONITOR (0x1C) + +#define I2C_BASE_1 (I2C_BASE + 0x00) +#define I2C_BASE_2 (I2C_BASE + 0x20) +#define I2C_BASE_3 (I2C_BASE + 0x40) +#define I2C_BASE_4 (I2C_BASE + 0x60) + +#define I2C_BASE_N(i) (I2C_BASE + (i) * 0x20) + +#define I2C_TASKMEM_BASE (0x1000) // Byte offset +#define I2C_TASKMEM_SIZE (0x1000) + +#define I2C_SPEED_400 (0x04030404) +#define I2C_SPEED_200 (0x09080909) +#define I2C_SPEED_154 (0x0C0B0C0C) +#define I2C_SPEED_100 (0x13121313) +#define I2C_SPEED_77 (0x19181919) +#define I2C_SPEED_50 (0x27262727) + + +// -------------------------------------------------------------------------- +// DMA Controller + +#define DMA_BASE_WRITE (0x100) +#define DMA_BASE_READ (0x140) + +#define DMA_CONTROL (0x00) // 64 +#define DMA_ERROR (0x04) // 65 ( only read instance ) + +#define DMA_DIAG_CONTROL (0x1C) // 71 +#define DMA_DIAG_PACKETCOUNTER_LOW (0x20) // 72 +#define DMA_DIAG_PACKETCOUNTER_HIGH (0x24) // 73 +#define DMA_DIAG_TIMECOUNTER_LOW (0x28) // 74 +#define DMA_DIAG_TIMECOUNTER_HIGH (0x2C) // 75 +#define DMA_DIAG_RECHECKCOUNTER (0x30) // 76 ( Split completions on read ) +#define DMA_DIAG_WAITTIMEOUTINIT (0x34) // 77 +#define DMA_DIAG_WAITOVERFLOWCOUNTER (0x38) // 78 +#define DMA_DIAG_WAITCOUNTER (0x3C) // 79 + +// -------------------------------------------------------------------------- +// DMA Buffer + +#define TS_INPUT_BASE (0x200) +#define TS_INPUT_CONTROL(i) (TS_INPUT_BASE + (i) * 16 + 0x00) + +#define TS_OUTPUT_BASE (0x280) +#define TS_OUTPUT_CONTROL(i) (TS_OUTPUT_BASE + (i) * 16 + 0x00) + +#define DMA_BUFFER_BASE (0x300) + +#define DMA_BUFFER_CONTROL(i) (DMA_BUFFER_BASE + (i) * 16 + 0x00) +#define DMA_BUFFER_ACK(i) (DMA_BUFFER_BASE + (i) * 16 + 0x04) +#define DMA_BUFFER_CURRENT(i) (DMA_BUFFER_BASE + (i) * 16 + 0x08) +#define DMA_BUFFER_SIZE(i) (DMA_BUFFER_BASE + (i) * 16 + 0x0c) + +#define DMA_BASE_ADDRESS_TABLE (0x2000) +#define DMA_BASE_ADDRESS_TABLE_ENTRIES (512) + diff --git a/drivers/media/dvb/ddbridge/ddbridge.h b/drivers/media/dvb/ddbridge/ddbridge.h new file mode 100644 index 0000000..c836301 --- /dev/null +++ b/drivers/media/dvb/ddbridge/ddbridge.h @@ -0,0 +1,187 @@ +/* + * ddbridge.h: Digital Devices PCIe bridge driver + * + * Copyright (C) 2010-2011 Digital Devices GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 only, as published by the Free Software Foundation. + * + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA + * Or, point your browser to http://www.gnu.org/copyleft/gpl.html + */ + +#ifndef _DDBRIDGE_H_ +#define _DDBRIDGE_H_ + +#include <linux/types.h> +#include <linux/sched.h> +#include <linux/interrupt.h> +#include <linux/i2c.h> +#include <linux/mutex.h> +#include <asm/dma.h> +#include <linux/dvb/frontend.h> +#include <linux/dvb/ca.h> +#include <linux/dvb/video.h> +#include <linux/dvb/audio.h> +#include <linux/socket.h> + +#include "dmxdev.h" +#include "dvbdev.h" +#include "dvb_demux.h" +#include "dvb_frontend.h" +#include "dvb_ringbuffer.h" +#include "dvb_ca_en50221.h" +#include "dvb_net.h" +#include "cxd2099.h" + +#define DDB_MAX_I2C 4 +#define DDB_MAX_PORT 4 +#define DDB_MAX_INPUT 8 +#define DDB_MAX_OUTPUT 4 + +struct ddb_info { + int type; +#define DDB_NONE 0 +#define DDB_OCTOPUS 1 + char *name; + int port_num; + u32 port_type[DDB_MAX_PORT]; +}; + +/* DMA_SIZE MUST be divisible by 188 and 128 !!! */ + +#define INPUT_DMA_MAX_BUFS 32 /* hardware table limit */ +#define INPUT_DMA_BUFS 8 +#define INPUT_DMA_SIZE (128*47*21) + +#define OUTPUT_DMA_MAX_BUFS 32 +#define OUTPUT_DMA_BUFS 8 +#define OUTPUT_DMA_SIZE (128*47*21) + +struct ddb; +struct ddb_port; + +struct ddb_input { + struct ddb_port *port; + u32 nr; + int attached; + + dma_addr_t pbuf[INPUT_DMA_MAX_BUFS]; + u8 *vbuf[INPUT_DMA_MAX_BUFS]; + u32 dma_buf_num; + u32 dma_buf_size; + + struct tasklet_struct tasklet; + spinlock_t lock; + wait_queue_head_t wq; + int running; + u32 stat; + u32 cbuf; + u32 coff; + + struct dvb_adapter adap; + struct dvb_device *dev; + struct dvb_frontend *fe; + struct dvb_frontend *fe2; + struct dmxdev dmxdev; + struct dvb_demux demux; + struct dvb_net dvbnet; + struct dmx_frontend hw_frontend; + struct dmx_frontend mem_frontend; + int users; + int (*gate_ctrl)(struct dvb_frontend *, int); +}; + +struct ddb_output { + struct ddb_port *port; + u32 nr; + dma_addr_t pbuf[OUTPUT_DMA_MAX_BUFS]; + u8 *vbuf[OUTPUT_DMA_MAX_BUFS]; + u32 dma_buf_num; + u32 dma_buf_size; + struct tasklet_struct tasklet; + spinlock_t lock; + wait_queue_head_t wq; + int running; + u32 stat; + u32 cbuf; + u32 coff; + + struct dvb_adapter adap; + struct dvb_device *dev; +}; + +struct ddb_i2c { + struct ddb *dev; + u32 nr; + struct i2c_adapter adap; + struct i2c_adapter adap2; + u32 regs; + u32 rbuf; + u32 wbuf; + int done; + wait_queue_head_t wq; +}; + +struct ddb_port { + struct ddb *dev; + u32 nr; + struct ddb_i2c *i2c; + struct mutex i2c_gate_lock; + u32 class; +#define DDB_PORT_NONE 0 +#define DDB_PORT_CI 1 +#define DDB_PORT_TUNER 2 + u32 type; +#define DDB_TUNER_NONE 0 +#define DDB_TUNER_DVBS_ST 1 +#define DDB_TUNER_DVBS_ST_AA 2 +#define DDB_TUNER_DVBCT_TR 16 +#define DDB_TUNER_DVBCT_ST 17 + u32 adr; + + struct ddb_input *input[2]; + struct ddb_output *output; + struct dvb_ca_en50221 *en; +}; + +struct ddb { + struct pci_dev *pdev; + unsigned char *regs; + struct ddb_port port[DDB_MAX_PORT]; + struct ddb_i2c i2c[DDB_MAX_I2C]; + struct ddb_input input[DDB_MAX_INPUT]; + struct ddb_output output[DDB_MAX_OUTPUT]; + + struct device *ddb_dev; + int nr; + u8 iobuf[1028]; + + struct ddb_info *info; + int msi; +}; + +/****************************************************************************/ + +#define ddbwritel(_val, _adr) writel((_val), \ + (char *) (dev->regs+(_adr))) +#define ddbreadl(_adr) readl((char *) (dev->regs+(_adr))) +#define ddbcpyto(_adr,_src,_count) memcpy_toio((char *) \ + (dev->regs+(_adr)),(_src),(_count)) +#define ddbcpyfrom(_dst,_adr,_count) memcpy_fromio((_dst),(char *) \ + (dev->regs+(_adr)),(_count)) + +/****************************************************************************/ + +#endif -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 2/5] ddbridge: Codingstyle fixes 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss 2011-07-03 21:23 ` PATCH 1/5] ddbridge: Initial check-in Oliver Endriss @ 2011-07-03 21:24 ` Oliver Endriss 2011-07-03 21:25 ` [PATCH 3/5] ddbridge: Allow compiling of the driver Oliver Endriss ` (3 subsequent siblings) 5 siblings, 0 replies; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 21:24 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab Codingstyle fixes Signed-off-by: Oliver Endriss <o.endriss@gmx.de> --- drivers/media/dvb/ddbridge/ddbridge-core.c | 191 ++++++++++++++++------------ drivers/media/dvb/ddbridge/ddbridge-regs.h | 62 +++++----- drivers/media/dvb/ddbridge/ddbridge.h | 8 +- 3 files changed, 144 insertions(+), 117 deletions(-) diff --git a/drivers/media/dvb/ddbridge/ddbridge-core.c b/drivers/media/dvb/ddbridge/ddbridge-core.c index ba9974b..81634f1 100644 --- a/drivers/media/dvb/ddbridge/ddbridge-core.c +++ b/drivers/media/dvb/ddbridge/ddbridge-core.c @@ -55,7 +55,7 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr); static int i2c_read(struct i2c_adapter *adapter, u8 adr, u8 *val) { struct i2c_msg msgs[1] = {{.addr = adr, .flags = I2C_M_RD, - .buf = val, .len = 1 }}; + .buf = val, .len = 1 } }; return (i2c_transfer(adapter, msgs, 1) == 1) ? 0 : -1; } @@ -64,7 +64,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 adr, u8 reg, u8 *val) struct i2c_msg msgs[2] = {{.addr = adr, .flags = 0, .buf = ®, .len = 1 }, {.addr = adr, .flags = I2C_M_RD, - .buf = val, .len = 1 }}; + .buf = val, .len = 1 } }; return (i2c_transfer(adapter, msgs, 2) == 2) ? 0 : -1; } @@ -75,7 +75,7 @@ static int i2c_read_reg16(struct i2c_adapter *adapter, u8 adr, struct i2c_msg msgs[2] = {{.addr = adr, .flags = 0, .buf = msg, .len = 2}, {.addr = adr, .flags = I2C_M_RD, - .buf = val, .len = 1}}; + .buf = val, .len = 1} }; return (i2c_transfer(adapter, msgs, 2) == 2) ? 0 : -1; } @@ -89,15 +89,15 @@ static int ddb_i2c_cmd(struct ddb_i2c *i2c, u32 adr, u32 cmd) ddbwritel((adr << 9) | cmd, i2c->regs + I2C_COMMAND); stat = wait_event_timeout(i2c->wq, i2c->done == 1, HZ); if (stat <= 0) { - printk("I2C timeout\n"); + printk(KERN_ERR "I2C timeout\n"); { /* MSI debugging*/ u32 istat = ddbreadl(INTERRUPT_STATUS); - printk("IRS %08x\n", istat); + printk(KERN_ERR "IRS %08x\n", istat); ddbwritel(istat, INTERRUPT_ACK); } return -EIO; } - val=ddbreadl(i2c->regs+I2C_COMMAND); + val = ddbreadl(i2c->regs+I2C_COMMAND); if (val & 0x70000) return -EIO; return 0; @@ -108,7 +108,7 @@ static int ddb_i2c_master_xfer(struct i2c_adapter *adapter, { struct ddb_i2c *i2c = (struct ddb_i2c *)i2c_get_adapdata(adapter); struct ddb *dev = i2c->dev; - u8 addr=0; + u8 addr = 0; if (num) addr = msg[0].addr; @@ -116,7 +116,7 @@ static int ddb_i2c_master_xfer(struct i2c_adapter *adapter, if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD)) { memcpy_toio(dev->regs + I2C_TASKMEM_BASE + i2c->wbuf, - msg[0].buf,msg[0].len); + msg[0].buf, msg[0].len); ddbwritel(msg[0].len|(msg[1].len << 16), i2c->regs+I2C_TASKLENGTH); if (!ddb_i2c_cmd(i2c, addr, 1)) { @@ -128,7 +128,7 @@ static int ddb_i2c_master_xfer(struct i2c_adapter *adapter, } if (num == 1 && !(msg[0].flags & I2C_M_RD)) { - ddbcpyto(I2C_TASKMEM_BASE + i2c->wbuf,msg[0].buf, msg[0].len); + ddbcpyto(I2C_TASKMEM_BASE + i2c->wbuf, msg[0].buf, msg[0].len); ddbwritel(msg[0].len, i2c->regs + I2C_TASKLENGTH); if (!ddb_i2c_cmd(i2c, addr, 2)) return num; @@ -217,6 +217,7 @@ static int ddb_i2c_init(struct ddb *dev) /******************************************************************************/ /******************************************************************************/ +#if 0 static void set_table(struct ddb *dev, u32 off, dma_addr_t *pbuf, u32 num) { @@ -230,6 +231,7 @@ static void set_table(struct ddb *dev, u32 off, ddbwritel(mem >> 32, base + i * 8 + 4); } } +#endif static void ddb_address_table(struct ddb *dev) { @@ -401,7 +403,7 @@ static void ddb_output_start(struct ddb_output *output) ddbwritel(1, DMA_BASE_READ); ddbwritel(3, DMA_BUFFER_CONTROL(output->nr + 8)); - //ddbwritel(0xbd, TS_OUTPUT_CONTROL(output->nr)); + /* ddbwritel(0xbd, TS_OUTPUT_CONTROL(output->nr)); */ ddbwritel(0x1d, TS_OUTPUT_CONTROL(output->nr)); output->running = 1; spin_unlock_irq(&output->lock); @@ -438,7 +440,7 @@ static u32 ddb_output_free(struct ddb_output *output) return 0; } -static ssize_t ddb_output_write(struct ddb_output* output, +static ssize_t ddb_output_write(struct ddb_output *output, const u8 *buf, size_t count) { struct ddb *dev = output->port->dev; @@ -452,9 +454,9 @@ static ssize_t ddb_output_write(struct ddb_output* output, len = output->dma_buf_size - output->coff; if ((((output->cbuf + 1) % output->dma_buf_num) == idx) && (off == 0)) { - if (len<=188) + if (len <= 188) break; - len-=188; + len -= 188; } if (output->cbuf == idx) { if (off > output->coff) { @@ -496,7 +498,7 @@ static u32 ddb_input_avail(struct ddb_input *input) off = (stat & 0x7ff) << 7; if (ctrl & 4) { - printk("IA %d %d %08x\n", idx, off, ctrl); + printk(KERN_ERR "IA %d %d %08x\n", idx, off, ctrl); ddbwritel(input->stat, DMA_BUFFER_ACK(input->nr)); return 0; } @@ -545,7 +547,7 @@ static struct ddb_input *fe2input(struct ddb *dev, struct dvb_frontend *fe) int i; for (i = 0; i < dev->info->port_num * 2; i++) { - if (dev->input[i].fe==fe) + if (dev->input[i].fe == fe) return &dev->input[i]; } return NULL; @@ -573,11 +575,11 @@ static int demod_attach_drxk(struct ddb_input *input) struct i2c_adapter *i2c = &input->port->i2c->adap; struct dvb_frontend *fe; - fe=input->fe = dvb_attach(drxk_attach, - i2c, 0x29 + (input->nr&1), - &input->fe2); + fe = input->fe = dvb_attach(drxk_attach, + i2c, 0x29 + (input->nr&1), + &input->fe2); if (!input->fe) { - printk("No DRXK found!\n"); + printk(KERN_ERR "No DRXK found!\n"); return -ENODEV; } fe->sec_priv = input; @@ -595,7 +597,7 @@ static int tuner_attach_tda18271(struct ddb_input *input) input->fe->ops.i2c_gate_ctrl(input->fe, 1); fe = dvb_attach(tda18271c2dd_attach, input->fe, i2c, 0x60); if (!fe) { - printk("No TDA18271 found!\n"); + printk(KERN_ERR "No TDA18271 found!\n"); return -ENODEV; } if (input->fe->ops.i2c_gate_ctrl) @@ -662,17 +664,17 @@ static int demod_attach_stv0900(struct ddb_input *input, int type) struct i2c_adapter *i2c = &input->port->i2c->adap; struct stv090x_config *feconf = type ? &stv0900_aa : &stv0900; - input->fe=dvb_attach(stv090x_attach, feconf, i2c, - (input->nr & 1) ? STV090x_DEMODULATOR_1 - : STV090x_DEMODULATOR_0); + input->fe = dvb_attach(stv090x_attach, feconf, i2c, + (input->nr & 1) ? STV090x_DEMODULATOR_1 + : STV090x_DEMODULATOR_0); if (!input->fe) { - printk("No STV0900 found!\n"); + printk(KERN_ERR "No STV0900 found!\n"); return -ENODEV; } if (!dvb_attach(lnbh24_attach, input->fe, i2c, 0, 0, (input->nr & 1) ? (0x09 - type) : (0x0b - type))) { - printk("No LNBH24 found!\n"); + printk(KERN_ERR "No LNBH24 found!\n"); return -ENODEV; } return 0; @@ -688,10 +690,11 @@ static int tuner_attach_stv6110(struct ddb_input *input, int type) ctl = dvb_attach(stv6110x_attach, input->fe, tunerconf, i2c); if (!ctl) { - printk("No STV6110X found!\n"); + printk(KERN_ERR "No STV6110X found!\n"); return -ENODEV; } - printk("attach tuner input %d adr %02x\n", input->nr, tunerconf->addr); + printk(KERN_INFO "attach tuner input %d adr %02x\n", + input->nr, tunerconf->addr); feconf->tuner_init = ctl->tuner_init; feconf->tuner_sleep = ctl->tuner_sleep; @@ -813,11 +816,11 @@ static int dvb_input_attach(struct ddb_input *input) struct dvb_adapter *adap = &input->adap; struct dvb_demux *dvbdemux = &input->demux; - ret=dvb_register_adapter(adap, "DDBridge",THIS_MODULE, + ret = dvb_register_adapter(adap, "DDBridge", THIS_MODULE, &input->port->dev->pdev->dev, adapter_nr); if (ret < 0) { - printk("ddbridge: Could not register adapter." + printk(KERN_ERR "ddbridge: Could not register adapter." "Check if you enabled enough adapters in dvb-core!\n"); return ret; } @@ -876,7 +879,7 @@ static int dvb_input_attach(struct ddb_input *input) if (input->fe2) { if (dvb_register_frontend(adap, input->fe2) < 0) return -ENODEV; - input->fe2->tuner_priv=input->fe->tuner_priv; + input->fe2->tuner_priv = input->fe->tuner_priv; memcpy(&input->fe2->ops.tuner_ops, &input->fe->ops.tuner_ops, sizeof(struct dvb_tuner_ops)); @@ -942,9 +945,11 @@ static ssize_t ts_read(struct file *file, char *buf, static unsigned int ts_poll(struct file *file, poll_table *wait) { + /* struct dvb_device *dvbdev = file->private_data; struct ddb_output *output = dvbdev->priv; struct ddb_input *input = output->port->input[0]; + */ unsigned int mask = 0; #if 0 @@ -959,7 +964,7 @@ static unsigned int ts_poll(struct file *file, poll_table *wait) return mask; } -static struct file_operations ci_fops = { +static const struct file_operations ci_fops = { .owner = THIS_MODULE, .read = ts_read, .write = ts_write, @@ -995,7 +1000,7 @@ static void input_tasklet(unsigned long data) if (input->port->class == DDB_PORT_TUNER) { if (4&ddbreadl(DMA_BUFFER_CONTROL(input->nr))) - printk("Overflow input %d\n", input->nr); + printk(KERN_ERR "Overflow input %d\n", input->nr); while (input->cbuf != ((input->stat >> 11) & 0x1f) || (4&ddbreadl(DMA_BUFFER_CONTROL(input->nr)))) { dvb_dmx_swfilter_packets(&input->demux, @@ -1056,9 +1061,9 @@ static int ddb_ci_attach(struct ddb_port *port) ddb_output_start(port->output); dvb_ca_en50221_init(&port->output->adap, port->en, 0, 1); - ret=dvb_register_device(&port->output->adap, &port->output->dev, - &dvbdev_ci, (void *) port->output, - DVB_DEVICE_SEC); + ret = dvb_register_device(&port->output->adap, &port->output->dev, + &dvbdev_ci, (void *) port->output, + DVB_DEVICE_SEC); return ret; } @@ -1069,7 +1074,7 @@ static int ddb_port_attach(struct ddb_port *port) switch (port->class) { case DDB_PORT_TUNER: ret = dvb_input_attach(port->input[0]); - if (ret<0) + if (ret < 0) break; ret = dvb_input_attach(port->input[1]); break; @@ -1080,7 +1085,7 @@ static int ddb_port_attach(struct ddb_port *port) break; } if (ret < 0) - printk("port_attach on port %d failed\n", port->nr); + printk(KERN_ERR "port_attach on port %d failed\n", port->nr); return ret; } @@ -1132,7 +1137,7 @@ static void ddb_ports_detach(struct ddb *dev) static int port_has_ci(struct ddb_port *port) { u8 val; - return (i2c_read_reg(&port->i2c->adap, 0x40, 0, &val) ? 0 : 1); + return i2c_read_reg(&port->i2c->adap, 0x40, 0, &val) ? 0 : 1; } static int port_has_stv0900(struct ddb_port *port) @@ -1188,7 +1193,8 @@ static void ddb_port_probe(struct ddb_port *port) port->type = DDB_TUNER_DVBCT_TR; ddbwritel(I2C_SPEED_400, port->i2c->regs + I2C_TIMING); } - printk("Port %d (TAB %d): %s\n", port->nr, port->nr+1, modname); + printk(KERN_INFO "Port %d (TAB %d): %s\n", + port->nr, port->nr+1, modname); } static void ddb_input_init(struct ddb_port *port, int nr) @@ -1284,26 +1290,42 @@ static irqreturn_t irq_handler(int irq, void *dev_id) do { ddbwritel(s, INTERRUPT_ACK); - if (s & 0x00000001) irq_handle_i2c(dev, 0); - if (s & 0x00000002) irq_handle_i2c(dev, 1); - if (s & 0x00000004) irq_handle_i2c(dev, 2); - if (s & 0x00000008) irq_handle_i2c(dev, 3); - - if (s & 0x00000100) tasklet_schedule(&dev->input[0].tasklet); - if (s & 0x00000200) tasklet_schedule(&dev->input[1].tasklet); - if (s & 0x00000400) tasklet_schedule(&dev->input[2].tasklet); - if (s & 0x00000800) tasklet_schedule(&dev->input[3].tasklet); - if (s & 0x00001000) tasklet_schedule(&dev->input[4].tasklet); - if (s & 0x00002000) tasklet_schedule(&dev->input[5].tasklet); - if (s & 0x00004000) tasklet_schedule(&dev->input[6].tasklet); - if (s & 0x00008000) tasklet_schedule(&dev->input[7].tasklet); - - if (s & 0x00010000) tasklet_schedule(&dev->output[0].tasklet); - if (s & 0x00020000) tasklet_schedule(&dev->output[1].tasklet); - if (s & 0x00040000) tasklet_schedule(&dev->output[2].tasklet); - if (s & 0x00080000) tasklet_schedule(&dev->output[3].tasklet); - - /* if (s & 0x000f0000) printk("%08x\n", istat); */ + if (s & 0x00000001) + irq_handle_i2c(dev, 0); + if (s & 0x00000002) + irq_handle_i2c(dev, 1); + if (s & 0x00000004) + irq_handle_i2c(dev, 2); + if (s & 0x00000008) + irq_handle_i2c(dev, 3); + + if (s & 0x00000100) + tasklet_schedule(&dev->input[0].tasklet); + if (s & 0x00000200) + tasklet_schedule(&dev->input[1].tasklet); + if (s & 0x00000400) + tasklet_schedule(&dev->input[2].tasklet); + if (s & 0x00000800) + tasklet_schedule(&dev->input[3].tasklet); + if (s & 0x00001000) + tasklet_schedule(&dev->input[4].tasklet); + if (s & 0x00002000) + tasklet_schedule(&dev->input[5].tasklet); + if (s & 0x00004000) + tasklet_schedule(&dev->input[6].tasklet); + if (s & 0x00008000) + tasklet_schedule(&dev->input[7].tasklet); + + if (s & 0x00010000) + tasklet_schedule(&dev->output[0].tasklet); + if (s & 0x00020000) + tasklet_schedule(&dev->output[1].tasklet); + if (s & 0x00040000) + tasklet_schedule(&dev->output[2].tasklet); + if (s & 0x00080000) + tasklet_schedule(&dev->output[3].tasklet); + + /* if (s & 0x000f0000) printk(KERN_DEBUG "%08x\n", istat); */ } while ((s = ddbreadl(INTERRUPT_STATUS))); return IRQ_HANDLED; @@ -1325,7 +1347,8 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) wbuf += 4; wlen -= 4; ddbwritel(data, SPI_DATA); - while (ddbreadl(SPI_CONTROL) & 0x0004); + while (ddbreadl(SPI_CONTROL) & 0x0004) + ; } if (rlen) @@ -1333,7 +1356,7 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) else ddbwritel(0x0003 | ((wlen << (8 + 3)) & 0x1f00), SPI_CONTROL); - data=0; + data = 0; shift = ((4 - wlen) * 8); while (wlen) { data <<= 8; @@ -1344,7 +1367,8 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) if (shift) data <<= shift; ddbwritel(data, SPI_DATA); - while (ddbreadl(SPI_CONTROL) & 0x0004); + while (ddbreadl(SPI_CONTROL) & 0x0004) + ; if (!rlen) { ddbwritel(0, SPI_CONTROL); @@ -1355,7 +1379,8 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) while (rlen > 4) { ddbwritel(0xffffffff, SPI_DATA); - while (ddbreadl(SPI_CONTROL) & 0x0004); + while (ddbreadl(SPI_CONTROL) & 0x0004) + ; data = ddbreadl(SPI_DATA); *(u32 *) rbuf = swab32(data); rbuf += 4; @@ -1363,7 +1388,8 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) } ddbwritel(0x0003 | ((rlen << (8 + 3)) & 0x1F00), SPI_CONTROL); ddbwritel(0xffffffff, SPI_DATA); - while (ddbreadl(SPI_CONTROL) & 0x0004); + while (ddbreadl(SPI_CONTROL) & 0x0004) + ; data = ddbreadl(SPI_DATA); ddbwritel(0, SPI_CONTROL); @@ -1421,7 +1447,7 @@ static long ddb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) if (copy_from_user(&fio, parg, sizeof(fio))) break; if (fio.write_len + fio.read_len > 1028) { - printk("IOBUF too small\n"); + printk(KERN_ERR "IOBUF too small\n"); return -ENOMEM; } wbuf = &dev->iobuf[0]; @@ -1444,7 +1470,7 @@ static long ddb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return res; } -static struct file_operations ddb_fops={ +static const struct file_operations ddb_fops = { .unlocked_ioctl = ddb_ioctl, .open = ddb_open, }; @@ -1458,7 +1484,8 @@ static char *ddb_devnode(struct device *device, mode_t *mode) static int ddb_class_create(void) { - if ((ddb_major = register_chrdev(0, DDB_NAME, &ddb_fops))<0) + ddb_major = register_chrdev(0, DDB_NAME, &ddb_fops); + if (ddb_major < 0) return ddb_major; ddb_class = class_create(THIS_MODULE, DDB_NAME); @@ -1536,10 +1563,10 @@ static int __devinit ddb_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct ddb *dev; - int stat=0; + int stat = 0; int irq_flag = IRQF_SHARED; - if (pci_enable_device(pdev)<0) + if (pci_enable_device(pdev) < 0) return -ENODEV; dev = vmalloc(sizeof(struct ddb)); @@ -1550,15 +1577,15 @@ static int __devinit ddb_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev); dev->info = (struct ddb_info *) id->driver_data; - printk("DDBridge driver detected: %s\n", dev->info->name); + printk(KERN_INFO "DDBridge driver detected: %s\n", dev->info->name); - dev->regs = ioremap(pci_resource_start(dev->pdev,0), - pci_resource_len(dev->pdev,0)); + dev->regs = ioremap(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); if (!dev->regs) { stat = -ENOMEM; goto fail; } - printk("HW %08x FW %08x\n", ddbreadl(0), ddbreadl(4)); + printk(KERN_INFO "HW %08x FW %08x\n", ddbreadl(0), ddbreadl(4)); #ifdef CONFIG_PCI_MSI if (pci_msi_enabled()) @@ -1570,9 +1597,9 @@ static int __devinit ddb_probe(struct pci_dev *pdev, dev->msi = 1; } #endif - if ((stat = request_irq(dev->pdev->irq, irq_handler, - irq_flag, "DDBridge", - (void *) dev))<0) + stat = request_irq(dev->pdev->irq, irq_handler, + irq_flag, "DDBridge", (void *) dev); + if (stat < 0) goto fail1; ddbwritel(0, DMA_BASE_WRITE); ddbwritel(0, DMA_BASE_READ); @@ -1594,18 +1621,18 @@ static int __devinit ddb_probe(struct pci_dev *pdev, fail3: ddb_ports_detach(dev); - printk("fail3\n"); + printk(KERN_ERR "fail3\n"); ddb_ports_release(dev); fail2: - printk("fail2\n"); + printk(KERN_ERR "fail2\n"); ddb_buffers_free(dev); fail1: - printk("fail1\n"); + printk(KERN_ERR "fail1\n"); if (dev->msi) pci_disable_msi(dev->pdev); free_irq(dev->pdev->irq, dev); fail: - printk("fail\n"); + printk(KERN_ERR "fail\n"); ddb_unmap(dev); pci_set_drvdata(pdev, 0); pci_disable_device(pdev); @@ -1641,7 +1668,7 @@ static struct ddb_info ddb_v6 = { #define DDVID 0xdd01 /* Digital Devices Vendor ID */ -#define DDB_ID(_vend, _dev, _subvend,_subdev,_driverdata) { \ +#define DDB_ID(_vend, _dev, _subvend, _subdev, _driverdata) { \ .vendor = _vend, .device = _dev, \ .subvendor = _subvend, .subdevice = _subdev, \ .driver_data = (unsigned long)&_driverdata } @@ -1668,7 +1695,7 @@ static struct pci_driver ddb_pci_driver = { static __init int module_init_ddbridge(void) { - printk("Digital Devices PCIE bridge driver, " + printk(KERN_INFO "Digital Devices PCIE bridge driver, " "Copyright (C) 2010-11 Digital Devices GmbH\n"); if (ddb_class_create()) return -1; diff --git a/drivers/media/dvb/ddbridge/ddbridge-regs.h b/drivers/media/dvb/ddbridge/ddbridge-regs.h index 0130073..a3ccb31 100644 --- a/drivers/media/dvb/ddbridge/ddbridge-regs.h +++ b/drivers/media/dvb/ddbridge/ddbridge-regs.h @@ -21,26 +21,26 @@ * Or, point your browser to http://www.gnu.org/copyleft/gpl.html */ -// $Id: DD-DVBBridgeV1.h 273 2010-09-17 05:03:16Z manfred $ +/* DD-DVBBridgeV1.h 273 2010-09-17 05:03:16Z manfred */ -// Register Definitions +/* Register Definitions */ #define CUR_REGISTERMAP_VERSION 0x10000 #define HARDWARE_VERSION 0x00 #define REGISTERMAP_VERSION 0x04 -// -------------------------------------------------------------------------- -// SPI Controller +/* ------------------------------------------------------------------------- */ +/* SPI Controller */ #define SPI_CONTROL 0x10 #define SPI_DATA 0x14 -// -------------------------------------------------------------------------- +/* ------------------------------------------------------------------------- */ -// Interrupt controller -// How many MSI's are available depends on HW (Min 2 max 8) -// How many are usable also depends on Host platform +/* Interrupt controller */ +/* How many MSI's are available depends on HW (Min 2 max 8) */ +/* How many are usable also depends on Host platform */ #define INTERRUPT_BASE (0x40) @@ -81,15 +81,15 @@ #define INTMASK_TSOUTPUT3 (0x00040000) #define INTMASK_TSOUTPUT4 (0x00080000) -// -------------------------------------------------------------------------- -// I2C Master Controller +/* ------------------------------------------------------------------------- */ +/* I2C Master Controller */ -#define I2C_BASE (0x80) // Byte offset +#define I2C_BASE (0x80) /* Byte offset */ #define I2C_COMMAND (0x00) #define I2C_TIMING (0x04) -#define I2C_TASKLENGTH (0x08) // High read, low write -#define I2C_TASKADDRESS (0x0C) // High read, low write +#define I2C_TASKLENGTH (0x08) /* High read, low write */ +#define I2C_TASKADDRESS (0x0C) /* High read, low write */ #define I2C_MONITOR (0x1C) @@ -100,7 +100,7 @@ #define I2C_BASE_N(i) (I2C_BASE + (i) * 0x20) -#define I2C_TASKMEM_BASE (0x1000) // Byte offset +#define I2C_TASKMEM_BASE (0x1000) /* Byte offset */ #define I2C_TASKMEM_SIZE (0x1000) #define I2C_SPEED_400 (0x04030404) @@ -111,27 +111,27 @@ #define I2C_SPEED_50 (0x27262727) -// -------------------------------------------------------------------------- -// DMA Controller +/* ------------------------------------------------------------------------- */ +/* DMA Controller */ #define DMA_BASE_WRITE (0x100) #define DMA_BASE_READ (0x140) -#define DMA_CONTROL (0x00) // 64 -#define DMA_ERROR (0x04) // 65 ( only read instance ) - -#define DMA_DIAG_CONTROL (0x1C) // 71 -#define DMA_DIAG_PACKETCOUNTER_LOW (0x20) // 72 -#define DMA_DIAG_PACKETCOUNTER_HIGH (0x24) // 73 -#define DMA_DIAG_TIMECOUNTER_LOW (0x28) // 74 -#define DMA_DIAG_TIMECOUNTER_HIGH (0x2C) // 75 -#define DMA_DIAG_RECHECKCOUNTER (0x30) // 76 ( Split completions on read ) -#define DMA_DIAG_WAITTIMEOUTINIT (0x34) // 77 -#define DMA_DIAG_WAITOVERFLOWCOUNTER (0x38) // 78 -#define DMA_DIAG_WAITCOUNTER (0x3C) // 79 - -// -------------------------------------------------------------------------- -// DMA Buffer +#define DMA_CONTROL (0x00) /* 64 */ +#define DMA_ERROR (0x04) /* 65 ( only read instance ) */ + +#define DMA_DIAG_CONTROL (0x1C) /* 71 */ +#define DMA_DIAG_PACKETCOUNTER_LOW (0x20) /* 72 */ +#define DMA_DIAG_PACKETCOUNTER_HIGH (0x24) /* 73 */ +#define DMA_DIAG_TIMECOUNTER_LOW (0x28) /* 74 */ +#define DMA_DIAG_TIMECOUNTER_HIGH (0x2C) /* 75 */ +#define DMA_DIAG_RECHECKCOUNTER (0x30) /* 76 ( Split completions on read ) */ +#define DMA_DIAG_WAITTIMEOUTINIT (0x34) /* 77 */ +#define DMA_DIAG_WAITOVERFLOWCOUNTER (0x38) /* 78 */ +#define DMA_DIAG_WAITCOUNTER (0x3C) /* 79 */ + +/* ------------------------------------------------------------------------- */ +/* DMA Buffer */ #define TS_INPUT_BASE (0x200) #define TS_INPUT_CONTROL(i) (TS_INPUT_BASE + (i) * 16 + 0x00) diff --git a/drivers/media/dvb/ddbridge/ddbridge.h b/drivers/media/dvb/ddbridge/ddbridge.h index c836301..6d14893 100644 --- a/drivers/media/dvb/ddbridge/ddbridge.h +++ b/drivers/media/dvb/ddbridge/ddbridge.h @@ -177,10 +177,10 @@ struct ddb { #define ddbwritel(_val, _adr) writel((_val), \ (char *) (dev->regs+(_adr))) #define ddbreadl(_adr) readl((char *) (dev->regs+(_adr))) -#define ddbcpyto(_adr,_src,_count) memcpy_toio((char *) \ - (dev->regs+(_adr)),(_src),(_count)) -#define ddbcpyfrom(_dst,_adr,_count) memcpy_fromio((_dst),(char *) \ - (dev->regs+(_adr)),(_count)) +#define ddbcpyto(_adr, _src, _count) memcpy_toio((char *) \ + (dev->regs+(_adr)), (_src), (_count)) +#define ddbcpyfrom(_dst, _adr, _count) memcpy_fromio((_dst), (char *) \ + (dev->regs+(_adr)), (_count)) /****************************************************************************/ -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 3/5] ddbridge: Allow compiling of the driver 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss 2011-07-03 21:23 ` PATCH 1/5] ddbridge: Initial check-in Oliver Endriss 2011-07-03 21:24 ` [PATCH 2/5] ddbridge: Codingstyle fixes Oliver Endriss @ 2011-07-03 21:25 ` Oliver Endriss 2011-07-03 21:26 ` [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n Oliver Endriss ` (2 subsequent siblings) 5 siblings, 0 replies; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 21:25 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab Driver added to Makefile and Kconfig. Signed-off-by: Oliver Endriss <o.endriss@gmx.de> --- drivers/media/dvb/Kconfig | 4 ++++ drivers/media/dvb/Makefile | 3 ++- drivers/media/dvb/ddbridge/Kconfig | 18 ++++++++++++++++++ drivers/media/dvb/ddbridge/Makefile | 14 ++++++++++++++ 4 files changed, 38 insertions(+), 1 deletions(-) create mode 100644 drivers/media/dvb/ddbridge/Kconfig create mode 100644 drivers/media/dvb/ddbridge/Makefile diff --git a/drivers/media/dvb/Kconfig b/drivers/media/dvb/Kconfig index ee214c3..f6e40b3 100644 --- a/drivers/media/dvb/Kconfig +++ b/drivers/media/dvb/Kconfig @@ -80,6 +80,10 @@ comment "Supported nGene Adapters" depends on DVB_CORE && PCI && I2C source "drivers/media/dvb/ngene/Kconfig" +comment "Supported ddbridge ('Octopus') Adapters" + depends on DVB_CORE && PCI && I2C + source "drivers/media/dvb/ddbridge/Kconfig" + comment "Supported DVB Frontends" depends on DVB_CORE source "drivers/media/dvb/frontends/Kconfig" diff --git a/drivers/media/dvb/Makefile b/drivers/media/dvb/Makefile index a1a0875..b2cefe6 100644 --- a/drivers/media/dvb/Makefile +++ b/drivers/media/dvb/Makefile @@ -15,6 +15,7 @@ obj-y := dvb-core/ \ dm1105/ \ pt1/ \ mantis/ \ - ngene/ + ngene/ \ + ddbridge/ obj-$(CONFIG_DVB_FIREDTV) += firewire/ diff --git a/drivers/media/dvb/ddbridge/Kconfig b/drivers/media/dvb/ddbridge/Kconfig new file mode 100644 index 0000000..d099e1a --- /dev/null +++ b/drivers/media/dvb/ddbridge/Kconfig @@ -0,0 +1,18 @@ +config DVB_DDBRIDGE + tristate "Digital Devices bridge support" + depends on DVB_CORE && PCI && I2C + select DVB_LNBP21 if !DVB_FE_CUSTOMISE + select DVB_STV6110x if !DVB_FE_CUSTOMISE + select DVB_STV090x if !DVB_FE_CUSTOMISE + select DVB_DRXK if !DVB_FE_CUSTOMISE + select DVB_TDA18271C2DD if !DVB_FE_CUSTOMISE + ---help--- + Support for cards with the Digital Devices PCI express bridge: + - Octopus PCIe Bridge + - Octopus mini PCIe Bridge + - Octopus LE + - DuoFlex S2 Octopus + - DuoFlex CT Octopus + - cineS2(v6) + + Say Y if you own such a card and want to use it. diff --git a/drivers/media/dvb/ddbridge/Makefile b/drivers/media/dvb/ddbridge/Makefile new file mode 100644 index 0000000..de4fe19 --- /dev/null +++ b/drivers/media/dvb/ddbridge/Makefile @@ -0,0 +1,14 @@ +# +# Makefile for the ddbridge device driver +# + +ddbridge-objs := ddbridge-core.o + +obj-$(CONFIG_DVB_DDBRIDGE) += ddbridge.o + +EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ +EXTRA_CFLAGS += -Idrivers/media/dvb/frontends/ +EXTRA_CFLAGS += -Idrivers/media/common/tuners/ + +# For the staging CI driver cxd2099 +EXTRA_CFLAGS += -Idrivers/staging/cxd2099/ -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss ` (2 preceding siblings ...) 2011-07-03 21:25 ` [PATCH 3/5] ddbridge: Allow compiling of the driver Oliver Endriss @ 2011-07-03 21:26 ` Oliver Endriss 2011-07-04 10:14 ` Bjørn Mork 2011-07-03 21:27 ` [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) Oliver Endriss 2011-07-03 22:27 ` [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Mauro Carvalho Chehab 5 siblings, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 21:26 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab Fix compilation of ngene/ddbridge for DVB_CXD2099=n. Note: Bug was introduced by commit 'cxd2099: Update to latest version'. Signed-off-by: Oliver Endriss <o.endriss@gmx.de> --- drivers/staging/cxd2099/cxd2099.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/staging/cxd2099/cxd2099.h b/drivers/staging/cxd2099/cxd2099.h index 75459d4..19c588a 100644 --- a/drivers/staging/cxd2099/cxd2099.h +++ b/drivers/staging/cxd2099/cxd2099.h @@ -41,7 +41,7 @@ struct dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg, #else static inline struct dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg, - void *priv, struct i2c_adapter *i2c); + void *priv, struct i2c_adapter *i2c) { printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__); return NULL; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n 2011-07-03 21:26 ` [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n Oliver Endriss @ 2011-07-04 10:14 ` Bjørn Mork 0 siblings, 0 replies; 45+ messages in thread From: Bjørn Mork @ 2011-07-04 10:14 UTC (permalink / raw) To: linux-media Oliver Endriss <o.endriss@gmx.de> writes: > Fix compilation of ngene/ddbridge for DVB_CXD2099=n. > > Note: Bug was introduced by commit 'cxd2099: Update to latest version'. Shouldn't that patch instead be fixed and resubmitted? Bjørn ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss ` (3 preceding siblings ...) 2011-07-03 21:26 ` [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n Oliver Endriss @ 2011-07-03 21:27 ` Oliver Endriss 2011-07-04 0:06 ` Walter Van Eetvelt 2011-07-03 22:27 ` [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Mauro Carvalho Chehab 5 siblings, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 21:27 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab Update Kconfig description (ddbridge with cxd2099) Signed-off-by: Oliver Endriss <o.endriss@gmx.de> --- drivers/staging/cxd2099/Kconfig | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/staging/cxd2099/Kconfig b/drivers/staging/cxd2099/Kconfig index 9d638c3..b48aefd 100644 --- a/drivers/staging/cxd2099/Kconfig +++ b/drivers/staging/cxd2099/Kconfig @@ -1,9 +1,10 @@ config DVB_CXD2099 - tristate "CXD2099AR Common Interface driver" - depends on DVB_CORE && PCI && I2C && DVB_NGENE - ---help--- - Support for the CI module found on cineS2 DVB-S2, supported by - the Micronas PCIe device driver (ngene). + tristate "CXD2099AR Common Interface driver" + depends on DVB_CORE && PCI && I2C + ---help--- + Support for the CI module found on cards based on + - Micronas ngene PCIe bridge: cineS2 etc. + - Digital Devices PCIe bridge: Octopus series For now, data is passed through '/dev/dvb/adapterX/sec0': - Encrypted data must be written to 'sec0'. -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) 2011-07-03 21:27 ` [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) Oliver Endriss @ 2011-07-04 0:06 ` Walter Van Eetvelt 0 siblings, 0 replies; 45+ messages in thread From: Walter Van Eetvelt @ 2011-07-04 0:06 UTC (permalink / raw) To: Oliver Endriss; +Cc: linux-media, Mauro Carvalho Chehab Oliver Endriss schreef op Sun 03-07-2011 om 23:27 [+0200]: > Update Kconfig description (ddbridge with cxd2099) > > Signed-off-by: Oliver Endriss <o.endriss@gmx.de> > --- > drivers/staging/cxd2099/Kconfig | 11 ++++++----- > 1 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/cxd2099/Kconfig b/drivers/staging/cxd2099/Kconfig > index 9d638c3..b48aefd 100644 > --- a/drivers/staging/cxd2099/Kconfig > +++ b/drivers/staging/cxd2099/Kconfig > @@ -1,9 +1,10 @@ > config DVB_CXD2099 > - tristate "CXD2099AR Common Interface driver" > - depends on DVB_CORE && PCI && I2C && DVB_NGENE > - ---help--- > - Support for the CI module found on cineS2 DVB-S2, supported by > - the Micronas PCIe device driver (ngene). > + tristate "CXD2099AR Common Interface driver" > + depends on DVB_CORE && PCI && I2C > + ---help--- > + Support for the CI module found on cards based on > + - Micronas ngene PCIe bridge: cineS2 etc. > + - Digital Devices PCIe bridge: Octopus series > > For now, data is passed through '/dev/dvb/adapterX/sec0': > - Encrypted data must be written to 'sec0'. Hi Oliver, => can you explain a bit more on "data is passed through '/dev/dvb/adapterX/sec0':" How is the idea behind the setup? Is each adapter having its own sec0? Walter ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss ` (4 preceding siblings ...) 2011-07-03 21:27 ` [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) Oliver Endriss @ 2011-07-03 22:27 ` Mauro Carvalho Chehab 2011-07-03 23:24 ` Oliver Endriss 5 siblings, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-03 22:27 UTC (permalink / raw) To: Oliver Endriss; +Cc: linux-media Hi Oliver, Em 03-07-2011 18:21, Oliver Endriss escreveu: > [PATCH 1/5] ddbridge: Initial check-in > [PATCH 2/5] ddbridge: Codingstyle fixes > [PATCH 3/5] ddbridge: Allow compiling of the driver > [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n > [PATCH 5/5] cxd2099: Update Kconfig descrition (ddbridge support) > > Note: > This patch series depends on the previous one: > [PATCH 00/16] New drivers: DRX-K, TDA18271c2, Updates: CXD2099 and ngene I've applied both series today on an experimental tree that I use when merging some complex drivers. They are at: http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/ngene I didn't actually reviewed the patch series yet, but I noticed some troubles related to Coding Style, and 2 compilation breakages, when all drivers are selected, due to some duplicated symbols. So, I've applied some patches fixing the issues I noticed. It would be great if you could test if the changes didn't break anything. There's a problem that I've noticed already at the patch series: the usage of CHK_ERROR macro hided a trouble on some places, especially at drxd_hard.c. As you know, the macro was defined as: #define CHK_ERROR(s) if ((status = s)) break I've replaced it, on all places, using a small perl script, as the above is a CodingStyle violation, and may hide some troubles[1]. [1] http://git.linuxtv.org/mchehab/experimental.git?a=commit;h=792ecdd1cc494a1e10ed494052ed697ab4e1aa8a After the removal, I've noticed that this works fine on several places where the code have things like: do { status = foo() if (status < 0) break; } while (0); There are places, however, that there are two loops, like, for example, at: static int DRX_Start(struct drxd_state *state, s32 off) { ... do { ... switch (p->transmission_mode) { case TRANSMISSION_MODE_8K: transmissionParams |= SC_RA_RAM_OP_PARAM_MODE_8K; if (state->type_A) { status = Write16(state, EC_SB_REG_TR_MODE__A, EC_SB_REG_TR_MODE_8K, 0x0000); if (status < 0) break; } break; ... } ... } while (0); return status; On those cases, instead of returning the error status, the function will just ignore the error and proceed to the next switch(). In this specific routine, as there are no locks inside the code, the better fix would be to just replace: if (status < 0) break; by if (status < 0) return (status); But I suspect that the same trouble is also present on other parts of the code. Another issue that I've noticed alread is that, on some places, instead of doing "return -EINVAL" (or some other proper error code), the code is just doing: "return -1". Could you please take a look on those issues? Thanks! Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-03 22:27 ` [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Mauro Carvalho Chehab @ 2011-07-03 23:24 ` Oliver Endriss 2011-07-04 0:17 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-03 23:24 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab, Ralph Metzler Hi Mauro, On Monday 04 July 2011 00:27:54 Mauro Carvalho Chehab wrote: > Hi Oliver, > > Em 03-07-2011 18:21, Oliver Endriss escreveu: > > [PATCH 1/5] ddbridge: Initial check-in > > [PATCH 2/5] ddbridge: Codingstyle fixes > > [PATCH 3/5] ddbridge: Allow compiling of the driver > > [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n > > [PATCH 5/5] cxd2099: Update Kconfig descrition (ddbridge support) > > > > Note: > > This patch series depends on the previous one: > > [PATCH 00/16] New drivers: DRX-K, TDA18271c2, Updates: CXD2099 and ngene > > I've applied both series today on an experimental tree that I use when merging > some complex drivers. They are at: > http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/ngene > > I didn't actually reviewed the patch series yet, but I noticed some troubles > related to Coding Style, and 2 compilation breakages, when all drivers are selected, > due to some duplicated symbols. So, I've applied some patches fixing the issues > I noticed. It would be great if you could test if the changes didn't break anything. Apparently these duplicated symbols did not show up here, because I compiled the drivers as modules. :-( > There's a problem that I've noticed already at the patch series: the usage of > CHK_ERROR macro hided a trouble on some places, especially at drxd_hard.c. > > As you know, the macro was defined as: > #define CHK_ERROR(s) if ((status = s)) break > > I've replaced it, on all places, using a small perl script, as the above is a CodingStyle > violation, and may hide some troubles[1]. True. > [1] http://git.linuxtv.org/mchehab/experimental.git?a=commit;h=792ecdd1cc494a1e10ed494052ed697ab4e1aa8a > > After the removal, I've noticed that this works fine on several places > where the code have things like: > do { > status = foo() > if (status < 0) > break; > > } while (0); > > There are places, however, that there are two loops, like, for example, at: > > static int DRX_Start(struct drxd_state *state, s32 off) > { > ... > do { > ... > switch (p->transmission_mode) { > case TRANSMISSION_MODE_8K: > transmissionParams |= SC_RA_RAM_OP_PARAM_MODE_8K; > if (state->type_A) { > status = Write16(state, EC_SB_REG_TR_MODE__A, EC_SB_REG_TR_MODE_8K, 0x0000); > if (status < 0) > break; > } > break; > ... > } > > ... > } while (0); > > return status; > > On those cases, instead of returning the error status, the function > will just ignore the error and proceed to the next switch(). In this specific > routine, as there are no locks inside the code, the better fix would be to > just replace: > if (status < 0) > break; > by > if (status < 0) > return (status); > > But I suspect that the same trouble is also present on other parts of the code. > > Another issue that I've noticed alread is that, on some places, instead of doing > "return -EINVAL" (or some other proper error code), the code is just doing: "return -1". > > Could you please take a look on those issues? That CHK_ERROR stuff appears very dangerous to me. Btw, this is why I did not remove the curly braces { } in some cases: if (...) { CHK_ERROR(...) } It would have caused really nasty effects. Anyway, I spent the whole weekend to re-format the code carefully and create both patch series, trying not to break anything. I simply cannot go through the driver code and verify everything. Please note that I am not the driver author! I think Ralph should comment on this. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-03 23:24 ` Oliver Endriss @ 2011-07-04 0:17 ` Mauro Carvalho Chehab 2011-07-14 23:45 ` Oliver Endriss 0 siblings, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-04 0:17 UTC (permalink / raw) To: linux-media; +Cc: Oliver Endriss, Ralph Metzler Em 03-07-2011 20:24, Oliver Endriss escreveu: > Hi Mauro, > > On Monday 04 July 2011 00:27:54 Mauro Carvalho Chehab wrote: >> Hi Oliver, >> >> Em 03-07-2011 18:21, Oliver Endriss escreveu: >>> [PATCH 1/5] ddbridge: Initial check-in >>> [PATCH 2/5] ddbridge: Codingstyle fixes >>> [PATCH 3/5] ddbridge: Allow compiling of the driver >>> [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n >>> [PATCH 5/5] cxd2099: Update Kconfig descrition (ddbridge support) >>> >>> Note: >>> This patch series depends on the previous one: >>> [PATCH 00/16] New drivers: DRX-K, TDA18271c2, Updates: CXD2099 and ngene >> >> I've applied both series today on an experimental tree that I use when merging >> some complex drivers. They are at: >> http://git.linuxtv.org/mchehab/experimental.git?a=shortlog;h=refs/heads/ngene >> >> I didn't actually reviewed the patch series yet, but I noticed some troubles >> related to Coding Style, and 2 compilation breakages, when all drivers are selected, >> due to some duplicated symbols. So, I've applied some patches fixing the issues >> I noticed. It would be great if you could test if the changes didn't break anything. > > Apparently these duplicated symbols did not show up here, > because I compiled the drivers as modules. :-( Likely. When merging things, I compile here with allyesconfig, in order to catch duplicated symbols. >> There's a problem that I've noticed already at the patch series: the usage of >> CHK_ERROR macro hided a trouble on some places, especially at drxd_hard.c. >> >> As you know, the macro was defined as: >> #define CHK_ERROR(s) if ((status = s)) break >> >> I've replaced it, on all places, using a small perl script, as the above is a CodingStyle >> violation, and may hide some troubles[1]. > > True. > >> [1] http://git.linuxtv.org/mchehab/experimental.git?a=commit;h=792ecdd1cc494a1e10ed494052ed697ab4e1aa8a >> >> After the removal, I've noticed that this works fine on several places >> where the code have things like: >> do { >> status = foo() >> if (status < 0) >> break; >> >> } while (0); >> >> There are places, however, that there are two loops, like, for example, at: >> >> static int DRX_Start(struct drxd_state *state, s32 off) >> { >> ... >> do { >> ... >> switch (p->transmission_mode) { >> case TRANSMISSION_MODE_8K: >> transmissionParams |= SC_RA_RAM_OP_PARAM_MODE_8K; >> if (state->type_A) { >> status = Write16(state, EC_SB_REG_TR_MODE__A, EC_SB_REG_TR_MODE_8K, 0x0000); >> if (status < 0) >> break; >> } >> break; >> ... >> } >> >> ... >> } while (0); >> >> return status; >> >> On those cases, instead of returning the error status, the function >> will just ignore the error and proceed to the next switch(). In this specific >> routine, as there are no locks inside the code, the better fix would be to >> just replace: >> if (status < 0) >> break; >> by >> if (status < 0) >> return (status); >> >> But I suspect that the same trouble is also present on other parts of the code. >> >> Another issue that I've noticed alread is that, on some places, instead of doing >> "return -EINVAL" (or some other proper error code), the code is just doing: "return -1". >> >> Could you please take a look on those issues? > > That CHK_ERROR stuff appears very dangerous to me. > Btw, this is why I did not remove the curly braces { } in some cases: > if (...) { > CHK_ERROR(...) > } > It would have caused really nasty effects. True. > Anyway, I spent the whole weekend to re-format the code carefully > and create both patch series, trying not to break anything. > I simply cannot go through the driver code and verify everything. As the changes on CHK_ERROR were done via script, it is unlikely that it introduced any problems (well, except if some function is returning a positive value as an error code, but I think that this is not the case). I did the same replacement when I've cleanup the drx-d driver (well, the script were not the same, but it used a similar approach), and the changes didn't break anything, but it is safer to have a test, to be sure that no functional changes were introduced. A simple test with the code and some working board is probably enough to verify that nothing broke. The bad error check logic, checking for "break" inside two loops instead just one will require a careful code inspection, though. > Please note that I am not the driver author! > I think Ralph should comment on this. Ralph, Could you please take a look on it? Thanks! Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-04 0:17 ` Mauro Carvalho Chehab @ 2011-07-14 23:45 ` Oliver Endriss 2011-07-15 0:47 ` Mauro Carvalho Chehab 2011-07-15 4:18 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 45+ messages in thread From: Oliver Endriss @ 2011-07-14 23:45 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Ralph Metzler On Monday 04 July 2011 02:17:52 Mauro Carvalho Chehab wrote: > Em 03-07-2011 20:24, Oliver Endriss escreveu: ... > > Anyway, I spent the whole weekend to re-format the code carefully > > and create both patch series, trying not to break anything. > > I simply cannot go through the driver code and verify everything. > > As the changes on CHK_ERROR were done via script, it is unlikely that it > introduced any problems (well, except if some function is returning > a positive value as an error code, but I think that this is not the > case). > > I did the same replacement when I've cleanup the drx-d driver (well, the > script were not the same, but it used a similar approach), and the changes > didn't break anything, but it is safer to have a test, to be sure that no > functional changes were introduced. > > A simple test with the code and some working board is probably enough > to verify that nothing broke. Finally I found some time to do this 'simple' test. Congratulations! You completely broke the DRXK for ngene and ddbridge: - DVB-T tuning does not work anymore. - Module unloading fails as well. drxk is 'in use' due to bad reference count. (DVB-C not tested: I currently do not have access to a DVB-C signal.) Loading the driver: Jul 15 00:52:48 darkstar kernel: [ 184.487399] Digital Devices PCIE bridge driver, Copyright (C) 2010-11 Digital Devices GmbH Jul 15 00:52:48 darkstar kernel: [ 184.487460] DDBridge 0000:01:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 Jul 15 00:52:48 darkstar kernel: [ 184.487469] DDBridge driver detected: Digital Devices Octopus DVB adapter Jul 15 00:52:48 darkstar kernel: [ 184.487491] HW 00010001 FW 00010000 Jul 15 00:52:48 darkstar kernel: [ 184.488321] Port 0 (TAB 1): DUAL DVB-S2 Jul 15 00:52:48 darkstar kernel: [ 184.488837] Port 1 (TAB 2): NO MODULE Jul 15 00:52:48 darkstar kernel: [ 184.489654] Port 2 (TAB 3): DUAL DVB-C/T Jul 15 00:52:48 darkstar kernel: [ 184.490159] Port 3 (TAB 4): NO MODULE Jul 15 00:52:48 darkstar kernel: [ 184.491245] DVB: registering new adapter (DDBridge) Jul 15 00:52:48 darkstar kernel: [ 184.644296] LNBx2x attached on addr=b Jul 15 00:52:48 darkstar kernel: [ 184.644363] stv6110x_attach: Attaching STV6110x Jul 15 00:52:48 darkstar kernel: [ 184.644365] attach tuner input 0 adr 60 Jul 15 00:52:48 darkstar kernel: [ 184.644368] DVB: registering adapter 2 frontend 0 (STV090x Multistandard)... Jul 15 00:52:48 darkstar kernel: [ 184.644435] DVB: registering new adapter (DDBridge) Jul 15 00:52:48 darkstar kernel: [ 184.680305] LNBx2x attached on addr=9 Jul 15 00:52:48 darkstar kernel: [ 184.680373] stv6110x_attach: Attaching STV6110x Jul 15 00:52:48 darkstar kernel: [ 184.680375] attach tuner input 1 adr 63 Jul 15 00:52:48 darkstar kernel: [ 184.680378] DVB: registering adapter 3 frontend 0 (STV090x Multistandard)... Jul 15 00:52:48 darkstar kernel: [ 184.680445] DVB: registering new adapter (DDBridge) Jul 15 00:52:48 darkstar kernel: [ 184.688938] drxk: detected a drx-3913k, spin A3, xtal 27.000 MHz Jul 15 00:52:48 darkstar kernel: [ 185.108839] DRXK driver version 0.9.4300 Jul 15 00:52:50 darkstar kernel: [ 186.796361] DVB: registering adapter 4 frontend 0 (DRXK DVB-C)... Jul 15 00:52:50 darkstar kernel: [ 186.796429] DVB: registering adapter 4 frontend 0 (DRXK DVB-T)... Jul 15 00:52:50 darkstar kernel: [ 186.796471] DVB: registering new adapter (DDBridge) Jul 15 00:52:50 darkstar kernel: [ 186.804923] drxk: detected a drx-3913k, spin A3, xtal 27.000 MHz Jul 15 00:52:50 darkstar kernel: [ 187.224841] DRXK driver version 0.9.4300 Jul 15 00:52:52 darkstar kernel: [ 188.912354] DVB: registering adapter 5 frontend 0 (DRXK DVB-C)... Jul 15 00:52:52 darkstar kernel: [ 188.912424] DVB: registering adapter 5 frontend 0 (DRXK DVB-T)... When trying to tune, the log is flooded with: Jul 15 00:53:15 darkstar kernel: [ 211.537173] drxk: Error -22 on DVBTScCommand Jul 15 00:53:15 darkstar kernel: [ 211.538206] drxk: Error -22 on DVBTStart Jul 15 00:53:15 darkstar kernel: [ 211.539151] drxk: Error -22 on Start Jul 15 00:53:15 darkstar kernel: [ 211.940231] drxk: SCU not ready Jul 15 00:53:15 darkstar kernel: [ 211.941310] drxk: Error -5 on SetDVBT Jul 15 00:53:15 darkstar kernel: [ 211.942243] drxk: Error -5 on Start Jul 15 00:53:15 darkstar kernel: [ 212.340237] drxk: SCU not ready Jul 15 00:53:15 darkstar kernel: [ 212.341286] drxk: Error -5 on SetDVBT Jul 15 00:53:15 darkstar kernel: [ 212.342202] drxk: Error -5 on Start Jul 15 00:53:16 darkstar kernel: [ 212.740238] drxk: SCU not ready ... Unloading: ERROR: Module drxk is in use lsmod: Module Size Used by drxk 47332 2 Sorry, I currently do not have the time to dig through your changesets. With these bugs the driver is unusable and not ready for the kernel. I hereby NACK submission of the driver to the kernel! CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-14 23:45 ` Oliver Endriss @ 2011-07-15 0:47 ` Mauro Carvalho Chehab 2011-07-15 2:11 ` Oliver Endriss 2011-07-15 3:56 ` Mauro Carvalho Chehab 2011-07-15 4:18 ` Mauro Carvalho Chehab 1 sibling, 2 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 0:47 UTC (permalink / raw) To: linux-media; +Cc: Oliver Endriss, Ralph Metzler Em 14-07-2011 20:45, Oliver Endriss escreveu: > On Monday 04 July 2011 02:17:52 Mauro Carvalho Chehab wrote: >> Em 03-07-2011 20:24, Oliver Endriss escreveu: > ... >>> Anyway, I spent the whole weekend to re-format the code carefully >>> and create both patch series, trying not to break anything. >>> I simply cannot go through the driver code and verify everything. >> >> As the changes on CHK_ERROR were done via script, it is unlikely that it >> introduced any problems (well, except if some function is returning >> a positive value as an error code, but I think that this is not the >> case). >> >> I did the same replacement when I've cleanup the drx-d driver (well, the >> script were not the same, but it used a similar approach), and the changes >> didn't break anything, but it is safer to have a test, to be sure that no >> functional changes were introduced. >> >> A simple test with the code and some working board is probably enough >> to verify that nothing broke. > > Finally I found some time to do this 'simple' test. Thanks for testing it. Big changes on complex driver require testing. > Congratulations! You completely broke the DRXK for ngene and ddbridge: > - DVB-T tuning does not work anymore. I don't have any DVB-T signal here. I'll double check what changed there and see if I can identify a possible cause for it, but eventually I may not discover what's wrong. Before I start bisecting, I need to know if the starting point is working. So, had you test that DVB-T was working after your cleanup patches? > - Module unloading fails as well. drxk is 'in use' due to bad reference count. I was eventually expecting a feedback about that. The patch that changed the behavior is this one: http://git.linuxtv.org/media_tree.git?a=commitdiff;h=f087cdd6f4fa8bef0e7588b124f52649d3cebe67 What happens is that drxk_attach() allocates one state struct each time it is called, and it initializes both frontends with the state: state = kzalloc(sizeof(struct drxk_state), GFP_KERNEL); ... state->c_frontend.demodulator_priv = state; state->t_frontend.demodulator_priv = state; ... *fe_t = &state->t_frontend; ... return &state->c_frontend; If you call it twice, the driver will loose the references for the first struct, and you'll end by having a memory leak at driver removal, if both DVB-T and DVB-C are registered. On my code, I've made sure to call it only once, and then I ended by having a reference = (u32) -1 for device removal, preventing me to remove the driver, as . So, clearly, that function should be called just once. However, at DVB unregister, dvb_frontend_detach() will be called twice: static void unregister_dvb(struct em28xx_dvb *dvb) { dvb_net_release(&dvb->net); dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_mem); dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_hw); dvb_dmxdev_release(&dvb->dmxdev); dvb_dmx_release(&dvb->demux); if (dvb->fe[1]) dvb_unregister_frontend(dvb->fe[1]); dvb_unregister_frontend(dvb->fe[0]); if (dvb->fe[1]) dvb_frontend_detach(dvb->fe[1]); dvb_frontend_detach(dvb->fe[0]); dvb_unregister_adapter(&dvb->adapter); } Or we would need to write a special unregister function at the driver just to cover the cases where DRX-K is used, as, on all other drivers, the attach function is called twice, even when the frontend supports two different types. See cxd2820r_attach, for example: instead of initializing both frontends with just one call, it can be called twice. If called a second time, it will just use the previously allocated data. I think that the better is to revert my patch and apply a solution similar to cxd2820r_attach. It should work fine if called just once (like ngene/ddbridge) or twice (like em28xx). > (DVB-C not tested: I currently do not have access to a DVB-C signal.) Hmm... are you sure that DVB-C used to work? I found an error on DVB-C setup for the device I used for test, fixed on this patch: http://git.linuxtv.org/media_tree.git?a=commitdiff;h=21ff98772327ff182f54d2fcca69448e440e23d3 Basically, on the device I tested, scu command: SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM requires 2 parameters, instead of 4. I've preserved the old behavior there, assuming that your code was working, but I suspect that at least you need to do this: + setParamParameters[0] = QAM_TOP_ANNEX_A; + if (state->m_OperationMode == OM_QAM_ITU_C) + setEnvParameters[0] = QAM_TOP_ANNEX_C; /* Annex */ + else + setEnvParameters[0] = 0; + + status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult); Due to this logic there, at SetQAM: /* Env parameters */ setEnvParameters[2] = QAM_TOP_ANNEX_A; /* Annex */ if (state->m_OperationMode == OM_QAM_ITU_C) setEnvParameters[2] = QAM_TOP_ANNEX_C; /* Annex */ This var is filled, but there's no call to SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV. Also, iti initializes it as parameters[2], instead of parameters[0]. > Loading the driver: > Jul 15 00:52:48 darkstar kernel: [ 184.487399] Digital Devices PCIE bridge driver, Copyright (C) 2010-11 Digital Devices GmbH > Jul 15 00:52:48 darkstar kernel: [ 184.487460] DDBridge 0000:01:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 > Jul 15 00:52:48 darkstar kernel: [ 184.487469] DDBridge driver detected: Digital Devices Octopus DVB adapter > Jul 15 00:52:48 darkstar kernel: [ 184.487491] HW 00010001 FW 00010000 > Jul 15 00:52:48 darkstar kernel: [ 184.488321] Port 0 (TAB 1): DUAL DVB-S2 > Jul 15 00:52:48 darkstar kernel: [ 184.488837] Port 1 (TAB 2): NO MODULE > Jul 15 00:52:48 darkstar kernel: [ 184.489654] Port 2 (TAB 3): DUAL DVB-C/T > Jul 15 00:52:48 darkstar kernel: [ 184.490159] Port 3 (TAB 4): NO MODULE > Jul 15 00:52:48 darkstar kernel: [ 184.491245] DVB: registering new adapter (DDBridge) > Jul 15 00:52:48 darkstar kernel: [ 184.644296] LNBx2x attached on addr=b > Jul 15 00:52:48 darkstar kernel: [ 184.644363] stv6110x_attach: Attaching STV6110x > Jul 15 00:52:48 darkstar kernel: [ 184.644365] attach tuner input 0 adr 60 > Jul 15 00:52:48 darkstar kernel: [ 184.644368] DVB: registering adapter 2 frontend 0 (STV090x Multistandard)... > Jul 15 00:52:48 darkstar kernel: [ 184.644435] DVB: registering new adapter (DDBridge) > Jul 15 00:52:48 darkstar kernel: [ 184.680305] LNBx2x attached on addr=9 > Jul 15 00:52:48 darkstar kernel: [ 184.680373] stv6110x_attach: Attaching STV6110x > Jul 15 00:52:48 darkstar kernel: [ 184.680375] attach tuner input 1 adr 63 > Jul 15 00:52:48 darkstar kernel: [ 184.680378] DVB: registering adapter 3 frontend 0 (STV090x Multistandard)... > Jul 15 00:52:48 darkstar kernel: [ 184.680445] DVB: registering new adapter (DDBridge) > Jul 15 00:52:48 darkstar kernel: [ 184.688938] drxk: detected a drx-3913k, spin A3, xtal 27.000 MHz > Jul 15 00:52:48 darkstar kernel: [ 185.108839] DRXK driver version 0.9.4300 > Jul 15 00:52:50 darkstar kernel: [ 186.796361] DVB: registering adapter 4 frontend 0 (DRXK DVB-C)... > Jul 15 00:52:50 darkstar kernel: [ 186.796429] DVB: registering adapter 4 frontend 0 (DRXK DVB-T)... > Jul 15 00:52:50 darkstar kernel: [ 186.796471] DVB: registering new adapter (DDBridge) > Jul 15 00:52:50 darkstar kernel: [ 186.804923] drxk: detected a drx-3913k, spin A3, xtal 27.000 MHz > Jul 15 00:52:50 darkstar kernel: [ 187.224841] DRXK driver version 0.9.4300 > Jul 15 00:52:52 darkstar kernel: [ 188.912354] DVB: registering adapter 5 frontend 0 (DRXK DVB-C)... > Jul 15 00:52:52 darkstar kernel: [ 188.912424] DVB: registering adapter 5 frontend 0 (DRXK DVB-T)... > > When trying to tune, the log is flooded with: > Jul 15 00:53:15 darkstar kernel: [ 211.537173] drxk: Error -22 on DVBTScCommand > Jul 15 00:53:15 darkstar kernel: [ 211.538206] drxk: Error -22 on DVBTStart > Jul 15 00:53:15 darkstar kernel: [ 211.539151] drxk: Error -22 on Start > Jul 15 00:53:15 darkstar kernel: [ 211.940231] drxk: SCU not ready > Jul 15 00:53:15 darkstar kernel: [ 211.941310] drxk: Error -5 on SetDVBT > Jul 15 00:53:15 darkstar kernel: [ 211.942243] drxk: Error -5 on Start > Jul 15 00:53:15 darkstar kernel: [ 212.340237] drxk: SCU not ready > Jul 15 00:53:15 darkstar kernel: [ 212.341286] drxk: Error -5 on SetDVBT > Jul 15 00:53:15 darkstar kernel: [ 212.342202] drxk: Error -5 on Start > Jul 15 00:53:16 darkstar kernel: [ 212.740238] drxk: SCU not ready > ... > > Unloading: > ERROR: Module drxk is in use > > lsmod: > Module Size Used by > drxk 47332 2 > > > Sorry, I currently do not have the time to dig through your changesets. > > With these bugs the driver is unusable and not ready for the kernel. > > I hereby NACK submission of the driver to the kernel! > > CU > Oliver > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 0:47 ` Mauro Carvalho Chehab @ 2011-07-15 2:11 ` Oliver Endriss 2011-07-15 4:01 ` Mauro Carvalho Chehab 2011-07-15 3:56 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-15 2:11 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Ralph Metzler On Friday 15 July 2011 02:47:27 Mauro Carvalho Chehab wrote: > Em 14-07-2011 20:45, Oliver Endriss escreveu: > > On Monday 04 July 2011 02:17:52 Mauro Carvalho Chehab wrote: > >> Em 03-07-2011 20:24, Oliver Endriss escreveu: > > ... > >>> Anyway, I spent the whole weekend to re-format the code carefully > >>> and create both patch series, trying not to break anything. > >>> I simply cannot go through the driver code and verify everything. > >> > >> As the changes on CHK_ERROR were done via script, it is unlikely that it > >> introduced any problems (well, except if some function is returning > >> a positive value as an error code, but I think that this is not the > >> case). > >> > >> I did the same replacement when I've cleanup the drx-d driver (well, the > >> script were not the same, but it used a similar approach), and the changes > >> didn't break anything, but it is safer to have a test, to be sure that no > >> functional changes were introduced. > >> > >> A simple test with the code and some working board is probably enough > >> to verify that nothing broke. > > > > Finally I found some time to do this 'simple' test. > > Thanks for testing it. Big changes on complex driver require testing. > > > Congratulations! You completely broke the DRXK for ngene and ddbridge: > > - DVB-T tuning does not work anymore. > > I don't have any DVB-T signal here. I'll double check what changed there > and see if I can identify a possible cause for it, but eventually I may > not discover what's wrong. > > Before I start bisecting, I need to know if the starting point is working. > So, had you test that DVB-T was working after your cleanup patches? Yes, it worked. And now I double checked with media_build of July 3th + my patch series: It works as expected. Well, I did not test DVB-C, but people reported that DVB-C was working before I applied my cleanups. So I assume it worked. > > - Module unloading fails as well. drxk is 'in use' due to bad reference count. > > I was eventually expecting a feedback about that. The patch that changed the > behavior is this one: > > http://git.linuxtv.org/media_tree.git?a=commitdiff;h=f087cdd6f4fa8bef0e7588b124f52649d3cebe67 > > What happens is that drxk_attach() allocates one state struct each time it is called, > and it initializes both frontends with the state: drxk_attach() was meant to be called once. See dvb_input_attach() in ddbridge-core.c: case DDB_TUNER_DVBCT_TR: if (demod_attach_drxk(input) < 0) return -ENODEV; if (tuner_attach_tda18271(input) < 0) return -ENODEV; if (input->fe) { if (dvb_register_frontend(adap, input->fe) < 0) return -ENODEV; } if (input->fe2) { if (dvb_register_frontend(adap, input->fe2) < 0) return -ENODEV; input->fe2->tuner_priv=input->fe->tuner_priv; memcpy(&input->fe2->ops.tuner_ops, &input->fe->ops.tuner_ops, sizeof(struct dvb_tuner_ops)); } break; demod_attach_drxk() returns two frontends: fe and fe2. > state = kzalloc(sizeof(struct drxk_state), GFP_KERNEL); > ... > state->c_frontend.demodulator_priv = state; > state->t_frontend.demodulator_priv = state; > ... > *fe_t = &state->t_frontend; > ... > return &state->c_frontend; > > If you call it twice, the driver will loose the references for the first struct, and you'll > end by having a memory leak at driver removal, if both DVB-T and DVB-C are registered. See above. > On my code, I've made sure to call it only once, and then I ended by having a reference = (u32) -1 > for device removal, preventing me to remove the driver, as . > > So, clearly, that function should be called just once. However, at DVB unregister, > dvb_frontend_detach() will be called twice: > > static void unregister_dvb(struct em28xx_dvb *dvb) > { > dvb_net_release(&dvb->net); > dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_mem); > dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_hw); > dvb_dmxdev_release(&dvb->dmxdev); > dvb_dmx_release(&dvb->demux); > if (dvb->fe[1]) > dvb_unregister_frontend(dvb->fe[1]); > dvb_unregister_frontend(dvb->fe[0]); > if (dvb->fe[1]) > dvb_frontend_detach(dvb->fe[1]); > dvb_frontend_detach(dvb->fe[0]); > dvb_unregister_adapter(&dvb->adapter); > } >From ddbridge-core.c: if (input->fe2) dvb_unregister_frontend(input->fe2); if (input->fe) { dvb_unregister_frontend(input->fe); dvb_frontend_detach(input->fe); input->fe = NULL; } So dvb_frontend_detach() was called exactly once. Mauro, you must not change the logic of the frontend driver, unless you verified what the bridge driver does... > Or we would need to write a special unregister function at the driver just to > cover the cases where DRX-K is used, as, on all other drivers, the attach > function is called twice, even when the frontend supports two different types. > > See cxd2820r_attach, for example: instead of initializing both frontends > with just one call, it can be called twice. If called a second time, it will > just use the previously allocated data. > > I think that the better is to revert my patch and apply a solution similar > to cxd2820r_attach. It should work fine if called just once (like ngene/ddbridge) > or twice (like em28xx). > > > (DVB-C not tested: I currently do not have access to a DVB-C signal.) > > Hmm... are you sure that DVB-C used to work? I found an error on DVB-C setup for > the device I used for test, fixed on this patch: > > http://git.linuxtv.org/media_tree.git?a=commitdiff;h=21ff98772327ff182f54d2fcca69448e440e23d3 > > Basically, on the device I tested, scu command: > SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM > requires 2 parameters, instead of 4. > > I've preserved the old behavior there, assuming that your code was working, but I suspect that > at least you need to do this: > > + setParamParameters[0] = QAM_TOP_ANNEX_A; > + if (state->m_OperationMode == OM_QAM_ITU_C) > + setEnvParameters[0] = QAM_TOP_ANNEX_C; /* Annex */ > + else > + setEnvParameters[0] = 0; > + > + status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult); > > Due to this logic there, at SetQAM: > > /* Env parameters */ > setEnvParameters[2] = QAM_TOP_ANNEX_A; /* Annex */ > if (state->m_OperationMode == OM_QAM_ITU_C) > setEnvParameters[2] = QAM_TOP_ANNEX_C; /* Annex */ > > This var is filled, but there's no call to SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV. Also, > iti initializes it as parameters[2], instead of parameters[0]. Sorry, I can't test it. Maybe Ralph can comment on this. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 2:11 ` Oliver Endriss @ 2011-07-15 4:01 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 4:01 UTC (permalink / raw) To: Oliver Endriss; +Cc: linux-media, Ralph Metzler Em 14-07-2011 23:11, Oliver Endriss escreveu: > On Friday 15 July 2011 02:47:27 Mauro Carvalho Chehab wrote: >> Em 14-07-2011 20:45, Oliver Endriss escreveu: >>> On Monday 04 July 2011 02:17:52 Mauro Carvalho Chehab wrote: >>>> Em 03-07-2011 20:24, Oliver Endriss escreveu: >>> ... >>>>> Anyway, I spent the whole weekend to re-format the code carefully >>>>> and create both patch series, trying not to break anything. >>>>> I simply cannot go through the driver code and verify everything. >>>> >>>> As the changes on CHK_ERROR were done via script, it is unlikely that it >>>> introduced any problems (well, except if some function is returning >>>> a positive value as an error code, but I think that this is not the >>>> case). >>>> >>>> I did the same replacement when I've cleanup the drx-d driver (well, the >>>> script were not the same, but it used a similar approach), and the changes >>>> didn't break anything, but it is safer to have a test, to be sure that no >>>> functional changes were introduced. >>>> >>>> A simple test with the code and some working board is probably enough >>>> to verify that nothing broke. >>> >>> Finally I found some time to do this 'simple' test. >> >> Thanks for testing it. Big changes on complex driver require testing. >> >>> Congratulations! You completely broke the DRXK for ngene and ddbridge: >>> - DVB-T tuning does not work anymore. >> >> I don't have any DVB-T signal here. I'll double check what changed there >> and see if I can identify a possible cause for it, but eventually I may >> not discover what's wrong. >> >> Before I start bisecting, I need to know if the starting point is working. >> So, had you test that DVB-T was working after your cleanup patches? > > Yes, it worked. > > And now I double checked with media_build of July 3th + my patch series: > It works as expected. Ok, thanks for checking it. I'll see if I can discover what has changed. > Well, I did not test DVB-C, but people reported that DVB-C was working > before I applied my cleanups. So I assume it worked. >>> (DVB-C not tested: I currently do not have access to a DVB-C signal.) >> >> Hmm... are you sure that DVB-C used to work? I found an error on DVB-C setup for >> the device I used for test, fixed on this patch: >> >> http://git.linuxtv.org/media_tree.git?a=commitdiff;h=21ff98772327ff182f54d2fcca69448e440e23d3 >> >> Basically, on the device I tested, scu command: >> SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_PARAM >> requires 2 parameters, instead of 4. >> >> I've preserved the old behavior there, assuming that your code was working, but I suspect that >> at least you need to do this: >> >> + setParamParameters[0] = QAM_TOP_ANNEX_A; >> + if (state->m_OperationMode == OM_QAM_ITU_C) >> + setEnvParameters[0] = QAM_TOP_ANNEX_C; /* Annex */ >> + else >> + setEnvParameters[0] = 0; >> + >> + status = scu_command(state, SCU_RAM_COMMAND_STANDARD_QAM | SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV, 1, setEnvParameters, 1, &cmdResult); >> >> Due to this logic there, at SetQAM: >> >> /* Env parameters */ >> setEnvParameters[2] = QAM_TOP_ANNEX_A; /* Annex */ >> if (state->m_OperationMode == OM_QAM_ITU_C) >> setEnvParameters[2] = QAM_TOP_ANNEX_C; /* Annex */ >> >> This var is filled, but there's no call to SCU_RAM_COMMAND_CMD_DEMOD_SET_ENV. Also, >> iti initializes it as parameters[2], instead of parameters[0]. > > Sorry, I can't test it. Maybe Ralph can comment on this. Ralph, could you please double check the DEMOD_SET_ENV logic at the driver, before my fallback code to use a 2 parameters call for scu_command, instead of 4 (required by the firmware I have here for Terratec)? Thanks! Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 0:47 ` Mauro Carvalho Chehab 2011-07-15 2:11 ` Oliver Endriss @ 2011-07-15 3:56 ` Mauro Carvalho Chehab 2011-07-15 5:17 ` Oliver Endriss 1 sibling, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 3:56 UTC (permalink / raw) To: linux-media; +Cc: Oliver Endriss, Ralph Metzler Em 14-07-2011 21:47, Mauro Carvalho Chehab escreveu: > Em 14-07-2011 20:45, Oliver Endriss escreveu: >> - DVB-T tuning does not work anymore. > I think that the better is to revert my patch and apply a solution similar > to cxd2820r_attach. It should work fine if called just once (like ngene/ddbridge) > or twice (like em28xx). I ended by fixing it at the easiest way: Just add a hack at em28xx to work the same way as ngene/ddbridge. The code is not beautiful, but in order to fix, I would also need to touch at tda18271c2dd. Let's do it on another time. - [media] Remove the double symbol increment hack from drxk_hard Both ngene and ddbrige calls dvb_attach once for drxk_attach. The logic used there, and by tda18271c2dd driver is different from similar logic on other frontends. The right fix is to change them to use the same logic, but, while we don't do that, we need to patch em28xx-dvb in order to do cope with ngene/ddbridge magic. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c index d503558..a0e2ff5 100644 --- a/drivers/media/dvb/frontends/drxk_hard.c +++ b/drivers/media/dvb/frontends/drxk_hard.c @@ -6313,12 +6313,10 @@ static int drxk_c_get_tune_settings(struct dvb_frontend *fe, struct dvb_frontend static void drxk_t_release(struct dvb_frontend *fe) { -#if 0 - struct drxk_state *state = fe->demodulator_priv; - - dprintk(1, "\n"); - kfree(state); -#endif + /* + * There's nothing to release here, as the state struct + * is already freed by drxk_c_release. + */ } static int drxk_t_init(struct dvb_frontend *fe) @@ -6451,17 +6449,6 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config, goto error; *fe_t = &state->t_frontend; -#ifdef CONFIG_MEDIA_ATTACH - /* - * HACK: As this function initializes both DVB-T and DVB-C fe symbols, - * and calling it twice would create the state twice, leading into - * memory leaks, the right way is to call it only once. However, dvb - * release functions will call symbol_put twice. So, the solution is to - * artificially increment the usage count, in order to allow the - * driver to be released. - */ - symbol_get(drxk_attach); -#endif return &state->c_frontend; error: diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c index f8617d2..ab8a740 100644 --- a/drivers/media/video/em28xx/em28xx-dvb.c +++ b/drivers/media/video/em28xx/em28xx-dvb.c @@ -76,9 +76,10 @@ struct em28xx_dvb { struct dmx_frontend fe_mem; struct dvb_net net; - /* Due to DRX-D - probably need changes */ + /* Due to DRX-K - probably need changes */ int (*gate_ctrl)(struct dvb_frontend *, int); struct semaphore pll_mutex; + bool dont_attach_fe1; }; @@ -595,7 +596,7 @@ static void unregister_dvb(struct em28xx_dvb *dvb) if (dvb->fe[1]) dvb_unregister_frontend(dvb->fe[1]); dvb_unregister_frontend(dvb->fe[0]); - if (dvb->fe[1]) + if (dvb->fe[1] && !dvb->dont_attach_fe1) dvb_frontend_detach(dvb->fe[1]); dvb_frontend_detach(dvb->fe[0]); dvb_unregister_adapter(&dvb->adapter); @@ -771,21 +772,22 @@ static int dvb_init(struct em28xx *dev) case EM2884_BOARD_TERRATEC_H5: terratec_h5_init(dev); - /* dvb->fe[1] will be DVB-C, and dvb->fe[0] will be DVB-T */ + dvb->dont_attach_fe1 = 1; + dvb->fe[0] = dvb_attach(drxk_attach, &terratec_h5_drxk, &dev->i2c_adap, &dvb->fe[1]); - if (!dvb->fe[0] || !dvb->fe[1]) { + if (!dvb->fe[0]) { result = -EINVAL; goto out_free; } + /* FIXME: do we need a pll semaphore? */ dvb->fe[0]->sec_priv = dvb; sema_init(&dvb->pll_mutex, 1); dvb->gate_ctrl = dvb->fe[0]->ops.i2c_gate_ctrl; dvb->fe[0]->ops.i2c_gate_ctrl = drxk_gate_ctrl; - dvb->fe[1]->ops.i2c_gate_ctrl = drxk_gate_ctrl; dvb->fe[1]->id = 1; - /* Attach tda18271 */ + /* Attach tda18271 to DVB-C frontend */ if (dvb->fe[0]->ops.i2c_gate_ctrl) dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1); if (!dvb_attach(tda18271c2dd_attach, dvb->fe[0], &dev->i2c_adap, 0x60)) { @@ -794,8 +796,12 @@ static int dvb_init(struct em28xx *dev) } if (dvb->fe[0]->ops.i2c_gate_ctrl) dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 0); - if (dvb->fe[1]->ops.i2c_gate_ctrl) - dvb->fe[1]->ops.i2c_gate_ctrl(dvb->fe[1], 1); + + /* Hack - needed by drxk/tda18271c2dd */ + dvb->fe[1]->tuner_priv = dvb->fe[0]->tuner_priv; + memcpy(&dvb->fe[1]->ops.tuner_ops, + &dvb->fe[0]->ops.tuner_ops, + sizeof(dvb->fe[0]->ops.tuner_ops)); break; default: ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 3:56 ` Mauro Carvalho Chehab @ 2011-07-15 5:17 ` Oliver Endriss 2011-07-15 8:26 ` Ralph Metzler 0 siblings, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-15 5:17 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Ralph Metzler On Friday 15 July 2011 05:56:31 Mauro Carvalho Chehab wrote: > Em 14-07-2011 21:47, Mauro Carvalho Chehab escreveu: > > Em 14-07-2011 20:45, Oliver Endriss escreveu: > >> - DVB-T tuning does not work anymore. > > I think that the better is to revert my patch and apply a solution similar > > to cxd2820r_attach. It should work fine if called just once (like ngene/ddbridge) > > or twice (like em28xx). > > I ended by fixing it at the easiest way: Just add a hack at em28xx to work the same > way as ngene/ddbridge. > > The code is not beautiful, but in order to fix, I would also need to touch at > tda18271c2dd. Let's do it on another time. > > [media] Remove the double symbol increment hack from drxk_hard Thanks, module unloading works again. > Both ngene and ddbrige calls dvb_attach once for drxk_attach. > The logic used there, and by tda18271c2dd driver is different > from similar logic on other frontends. > > The right fix is to change them to use the same logic, but, > while we don't do that, we need to patch em28xx-dvb in order > to do cope with ngene/ddbridge magic. I disagree: The right fix is to extend the framework, and drop the secondary frondend completely. The current way of supporting multi-standard tuners is abusing the DVB API. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 5:17 ` Oliver Endriss @ 2011-07-15 8:26 ` Ralph Metzler 2011-07-15 13:25 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 45+ messages in thread From: Ralph Metzler @ 2011-07-15 8:26 UTC (permalink / raw) To: linux-media; +Cc: Mauro Carvalho Chehab Oliver Endriss writes: > > Both ngene and ddbrige calls dvb_attach once for drxk_attach. > > The logic used there, and by tda18271c2dd driver is different > > from similar logic on other frontends. > > > > The right fix is to change them to use the same logic, but, > > while we don't do that, we need to patch em28xx-dvb in order > > to do cope with ngene/ddbridge magic. > > I disagree: The right fix is to extend the framework, and drop the > secondary frondend completely. The current way of supporting > multi-standard tuners is abusing the DVB API. > Yes, exactly what I wanted to say. I am just working on yet another C/T combo. This time stv0367 and TDA18212. For both there are existing drivers and our own version again. I am trying to merge them so that there is not yet another discussion regarding new driver versions (but the very first version might still come out with separate drivers). At the same time I want to add delivery system properties to support everything in one frontend device. Adding a parameter to select C or T as default should help in most cases where the application does not support switching yet. Regards, Ralph ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 8:26 ` Ralph Metzler @ 2011-07-15 13:25 ` Mauro Carvalho Chehab 2011-07-15 17:01 ` Andreas Oberritter 0 siblings, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 13:25 UTC (permalink / raw) To: Ralph Metzler; +Cc: linux-media Em 15-07-2011 05:26, Ralph Metzler escreveu: > Oliver Endriss writes: > > > Both ngene and ddbrige calls dvb_attach once for drxk_attach. > > > The logic used there, and by tda18271c2dd driver is different > > > from similar logic on other frontends. > > > > > > The right fix is to change them to use the same logic, but, > > > while we don't do that, we need to patch em28xx-dvb in order > > > to do cope with ngene/ddbridge magic. > > > > I disagree: The right fix is to extend the framework, and drop the > > secondary frondend completely. The current way of supporting > > multi-standard tuners is abusing the DVB API. > > > > Yes, exactly what I wanted to say. Yeah, I see your point. The current approch, however, is in the mid-term of an unique multi-delivery-system frontend, and two frontend attachments. As such, it require some ugly glue at the bridge driver like: dvb->fe[1]->tuner_priv = dvb->fe[0]->tuner_priv; memcpy(&dvb->fe[1]->ops.tuner_ops, &dvb->fe[0]->ops.tuner_ops, sizeof(dvb->fe[0]->ops.tuner_ops)); My proposal is to keep it as-is for now, changing it after we extend the API to support multi-delivery-system frontend. > I am just working on yet another C/T combo. This time stv0367 and > TDA18212. For both there are existing drivers and our own version again. > I am trying to merge them so that there is not yet another > discussion regarding new driver versions Thanks! > (but the very first version > might still come out with separate drivers). > At the same time I want to add delivery system properties to > support everything in one frontend device. > Adding a parameter to select C or T as default should help in most > cases where the application does not support switching yet. If I understood well, creating a multi-delivery type of frontend for devices like DRX-K makes sense for me. We need to take some care about how to add support for them, to avoid breaking userspace, or to follow kernel deprecating rules, by adding some legacy compatibility glue for a few kernel versions. So, the sooner we add such support, the better, as less drivers will need to support a "fallback" mechanism. The current DVB version 5 API doesn't prevent some userspace application to change the delivery system[1] for a given frontend. This feature is actually used by DVB-T2 and DVB-S2 drivers. This actually improved the DVB API multi-fe support, by avoiding the need of create of a secondary frontend for T2/S2. Userspace applications can detect that feature by using FE_CAN_2G_MODULATION flag, but this mechanism doesn't allow other types of changes like from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such type of delivery system switch, using the same chip ended by needing to add two frontends. Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and add a way to query the type of delivery systems supported by a driver. [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM Cheers, Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 13:25 ` Mauro Carvalho Chehab @ 2011-07-15 17:01 ` Andreas Oberritter 2011-07-15 17:34 ` Mauro Carvalho Chehab 2011-07-15 23:41 ` Antti Palosaari 0 siblings, 2 replies; 45+ messages in thread From: Andreas Oberritter @ 2011-07-15 17:01 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Ralph Metzler, linux-media On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: > Em 15-07-2011 05:26, Ralph Metzler escreveu: >> At the same time I want to add delivery system properties to >> support everything in one frontend device. >> Adding a parameter to select C or T as default should help in most >> cases where the application does not support switching yet. > > If I understood well, creating a multi-delivery type of frontend for > devices like DRX-K makes sense for me. > > We need to take some care about how to add support for them, to avoid > breaking userspace, or to follow kernel deprecating rules, by adding > some legacy compatibility glue for a few kernel versions. So, the sooner > we add such support, the better, as less drivers will need to support > a "fallback" mechanism. > > The current DVB version 5 API doesn't prevent some userspace application > to change the delivery system[1] for a given frontend. This feature is > actually used by DVB-T2 and DVB-S2 drivers. This actually improved the > DVB API multi-fe support, by avoiding the need of create of a secondary > frontend for T2/S2. > > Userspace applications can detect that feature by using FE_CAN_2G_MODULATION > flag, but this mechanism doesn't allow other types of changes like > from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such > type of delivery system switch, using the same chip ended by needing to > add two frontends. > > Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and > add a way to query the type of delivery systems supported by a driver. > > [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM I don't think it's necessary to add a new flag. It should be sufficient to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be read-only and return an array of type fe_delivery_system_t. Querying this new property on present kernels hopefully fails with a non-zero return code. in which case FE_GET_INFO should be used to query the delivery system. In future kernels we can provide a default implementation, returning exactly one fe_delivery_system_t for unported drivers. Other drivers should be able to override this default implementation in their get_property callback. Regards, Andreas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 17:01 ` Andreas Oberritter @ 2011-07-15 17:34 ` Mauro Carvalho Chehab 2011-07-15 23:41 ` Antti Palosaari 1 sibling, 0 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 17:34 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Ralph Metzler, linux-media Em 15-07-2011 14:01, Andreas Oberritter escreveu: > On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>> At the same time I want to add delivery system properties to >>> support everything in one frontend device. >>> Adding a parameter to select C or T as default should help in most >>> cases where the application does not support switching yet. >> >> If I understood well, creating a multi-delivery type of frontend for >> devices like DRX-K makes sense for me. >> >> We need to take some care about how to add support for them, to avoid >> breaking userspace, or to follow kernel deprecating rules, by adding >> some legacy compatibility glue for a few kernel versions. So, the sooner >> we add such support, the better, as less drivers will need to support >> a "fallback" mechanism. >> >> The current DVB version 5 API doesn't prevent some userspace application >> to change the delivery system[1] for a given frontend. This feature is >> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >> DVB API multi-fe support, by avoiding the need of create of a secondary >> frontend for T2/S2. >> >> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >> flag, but this mechanism doesn't allow other types of changes like >> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >> type of delivery system switch, using the same chip ended by needing to >> add two frontends. >> >> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >> add a way to query the type of delivery systems supported by a driver. >> >> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM > > I don't think it's necessary to add a new flag. It should be sufficient > to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be > read-only and return an array of type fe_delivery_system_t. Yes, this would work properly. > > Querying this new property on present kernels hopefully fails with a > non-zero return code. in which case FE_GET_INFO should be used to query > the delivery system. Yes. it currently returns an specific error code for not supported. > > In future kernels we can provide a default implementation, returning > exactly one fe_delivery_system_t for unported drivers. Other drivers > should be able to override this default implementation in their > get_property callback. Seems fine for me. > > Regards, > Andreas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 17:01 ` Andreas Oberritter 2011-07-15 17:34 ` Mauro Carvalho Chehab @ 2011-07-15 23:41 ` Antti Palosaari 2011-07-16 12:25 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 45+ messages in thread From: Antti Palosaari @ 2011-07-15 23:41 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Mauro Carvalho Chehab, Ralph Metzler, linux-media On 07/15/2011 08:01 PM, Andreas Oberritter wrote: > On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>> At the same time I want to add delivery system properties to >>> support everything in one frontend device. >>> Adding a parameter to select C or T as default should help in most >>> cases where the application does not support switching yet. >> >> If I understood well, creating a multi-delivery type of frontend for >> devices like DRX-K makes sense for me. >> >> We need to take some care about how to add support for them, to avoid >> breaking userspace, or to follow kernel deprecating rules, by adding >> some legacy compatibility glue for a few kernel versions. So, the sooner >> we add such support, the better, as less drivers will need to support >> a "fallback" mechanism. >> >> The current DVB version 5 API doesn't prevent some userspace application >> to change the delivery system[1] for a given frontend. This feature is >> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >> DVB API multi-fe support, by avoiding the need of create of a secondary >> frontend for T2/S2. >> >> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >> flag, but this mechanism doesn't allow other types of changes like >> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >> type of delivery system switch, using the same chip ended by needing to >> add two frontends. >> >> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >> add a way to query the type of delivery systems supported by a driver. >> >> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM > > I don't think it's necessary to add a new flag. It should be sufficient > to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be > read-only and return an array of type fe_delivery_system_t. > > Querying this new property on present kernels hopefully fails with a > non-zero return code. in which case FE_GET_INFO should be used to query > the delivery system. > > In future kernels we can provide a default implementation, returning > exactly one fe_delivery_system_t for unported drivers. Other drivers > should be able to override this default implementation in their > get_property callback. One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 23:41 ` Antti Palosaari @ 2011-07-16 12:25 ` Mauro Carvalho Chehab 2011-07-16 14:16 ` Antti Palosaari 0 siblings, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-16 12:25 UTC (permalink / raw) To: Antti Palosaari; +Cc: Andreas Oberritter, Ralph Metzler, linux-media Em 15-07-2011 20:41, Antti Palosaari escreveu: > On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>> At the same time I want to add delivery system properties to >>>> support everything in one frontend device. >>>> Adding a parameter to select C or T as default should help in most >>>> cases where the application does not support switching yet. >>> >>> If I understood well, creating a multi-delivery type of frontend for >>> devices like DRX-K makes sense for me. >>> >>> We need to take some care about how to add support for them, to avoid >>> breaking userspace, or to follow kernel deprecating rules, by adding >>> some legacy compatibility glue for a few kernel versions. So, the sooner >>> we add such support, the better, as less drivers will need to support >>> a "fallback" mechanism. >>> >>> The current DVB version 5 API doesn't prevent some userspace application >>> to change the delivery system[1] for a given frontend. This feature is >>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >>> DVB API multi-fe support, by avoiding the need of create of a secondary >>> frontend for T2/S2. >>> >>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >>> flag, but this mechanism doesn't allow other types of changes like >>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >>> type of delivery system switch, using the same chip ended by needing to >>> add two frontends. >>> >>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >>> add a way to query the type of delivery systems supported by a driver. >>> >>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >> >> I don't think it's necessary to add a new flag. It should be sufficient >> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be >> read-only and return an array of type fe_delivery_system_t. >> >> Querying this new property on present kernels hopefully fails with a >> non-zero return code. in which case FE_GET_INFO should be used to query >> the delivery system. >> >> In future kernels we can provide a default implementation, returning >> exactly one fe_delivery_system_t for unported drivers. Other drivers >> should be able to override this default implementation in their >> get_property callback. > > One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. > > If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. In this case, the driver should just create two frontends, as currently. There's a difference when there are two physical FE's and just one FE: with 2 FE's, the userspace application can just keep both opened at the same time. Some applications (like vdr) assumes that all multi-fe are like that. When there's just a single FE, but the driver needs to "fork" it in two due to the API troubles, the driver needs to prevent the usage of both fe's, either at open or at the ioctl level. So, applications like vdr will only use the first frontend. Cheers, Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 12:25 ` Mauro Carvalho Chehab @ 2011-07-16 14:16 ` Antti Palosaari 2011-07-16 14:54 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 45+ messages in thread From: Antti Palosaari @ 2011-07-16 14:16 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Andreas Oberritter, Ralph Metzler, linux-media On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: > Em 15-07-2011 20:41, Antti Palosaari escreveu: >> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>> At the same time I want to add delivery system properties to >>>>> support everything in one frontend device. >>>>> Adding a parameter to select C or T as default should help in most >>>>> cases where the application does not support switching yet. >>>> >>>> If I understood well, creating a multi-delivery type of frontend for >>>> devices like DRX-K makes sense for me. >>>> >>>> We need to take some care about how to add support for them, to avoid >>>> breaking userspace, or to follow kernel deprecating rules, by adding >>>> some legacy compatibility glue for a few kernel versions. So, the sooner >>>> we add such support, the better, as less drivers will need to support >>>> a "fallback" mechanism. >>>> >>>> The current DVB version 5 API doesn't prevent some userspace application >>>> to change the delivery system[1] for a given frontend. This feature is >>>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >>>> DVB API multi-fe support, by avoiding the need of create of a secondary >>>> frontend for T2/S2. >>>> >>>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >>>> flag, but this mechanism doesn't allow other types of changes like >>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >>>> type of delivery system switch, using the same chip ended by needing to >>>> add two frontends. >>>> >>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >>>> add a way to query the type of delivery systems supported by a driver. >>>> >>>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>> >>> I don't think it's necessary to add a new flag. It should be sufficient >>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be >>> read-only and return an array of type fe_delivery_system_t. >>> >>> Querying this new property on present kernels hopefully fails with a >>> non-zero return code. in which case FE_GET_INFO should be used to query >>> the delivery system. >>> >>> In future kernels we can provide a default implementation, returning >>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>> should be able to override this default implementation in their >>> get_property callback. >> >> One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. >> >> If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. > > In this case, the driver should just create two frontends, as currently. > > There's a difference when there are two physical FE's and just one FE: > with 2 FE's, the userspace application can just keep both opened at > the same time. Some applications (like vdr) assumes that all multi-fe > are like that. Does this mean demod is not sleeping (.init() called)? > When there's just a single FE, but the driver needs to "fork" it in two > due to the API troubles, the driver needs to prevent the usage of both > fe's, either at open or at the ioctl level. So, applications like vdr > will only use the first frontend. Lets take example. There is shared MFE having DVB-S, DVB-T and DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have own. Currently it will shown as: * adapter0 ** frontend0 (DVB-S) ** frontend1 (DVB-T) ** frontend2 (DVB-C) Your new "ideal" solution will be: * adapter0 ** frontend0 (DVB-S/T/C) What really happens (mixed old and new): * adapter0 ** frontend0 (DVB-S) ** frontend1 (DVB-T/C) It does not look very good to offer this kind of mixed solution, since it is possible to offer only one solution for userspace, new or old, but not mixing. regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 14:16 ` Antti Palosaari @ 2011-07-16 14:54 ` Mauro Carvalho Chehab 2011-07-16 15:40 ` Andreas Oberritter 2011-07-16 15:40 ` Oliver Endriss 0 siblings, 2 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-16 14:54 UTC (permalink / raw) To: Antti Palosaari; +Cc: Andreas Oberritter, Ralph Metzler, linux-media Em 16-07-2011 11:16, Antti Palosaari escreveu: > On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>> At the same time I want to add delivery system properties to >>>>>> support everything in one frontend device. >>>>>> Adding a parameter to select C or T as default should help in most >>>>>> cases where the application does not support switching yet. >>>>> >>>>> If I understood well, creating a multi-delivery type of frontend for >>>>> devices like DRX-K makes sense for me. >>>>> >>>>> We need to take some care about how to add support for them, to avoid >>>>> breaking userspace, or to follow kernel deprecating rules, by adding >>>>> some legacy compatibility glue for a few kernel versions. So, the sooner >>>>> we add such support, the better, as less drivers will need to support >>>>> a "fallback" mechanism. >>>>> >>>>> The current DVB version 5 API doesn't prevent some userspace application >>>>> to change the delivery system[1] for a given frontend. This feature is >>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >>>>> DVB API multi-fe support, by avoiding the need of create of a secondary >>>>> frontend for T2/S2. >>>>> >>>>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >>>>> flag, but this mechanism doesn't allow other types of changes like >>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >>>>> type of delivery system switch, using the same chip ended by needing to >>>>> add two frontends. >>>>> >>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >>>>> add a way to query the type of delivery systems supported by a driver. >>>>> >>>>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>> >>>> I don't think it's necessary to add a new flag. It should be sufficient >>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be >>>> read-only and return an array of type fe_delivery_system_t. >>>> >>>> Querying this new property on present kernels hopefully fails with a >>>> non-zero return code. in which case FE_GET_INFO should be used to query >>>> the delivery system. >>>> >>>> In future kernels we can provide a default implementation, returning >>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>> should be able to override this default implementation in their >>>> get_property callback. >>> >>> One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. >>> >>> If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. >> >> In this case, the driver should just create two frontends, as currently. >> >> There's a difference when there are two physical FE's and just one FE: >> with 2 FE's, the userspace application can just keep both opened at >> the same time. Some applications (like vdr) assumes that all multi-fe >> are like that. > > Does this mean demod is not sleeping (.init() called)? > >> When there's just a single FE, but the driver needs to "fork" it in two >> due to the API troubles, the driver needs to prevent the usage of both >> fe's, either at open or at the ioctl level. So, applications like vdr >> will only use the first frontend. > > Lets take example. There is shared MFE having DVB-S, DVB-T and DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have own. > > Currently it will shown as: Let me name the approaches: Approach 1) > * adapter0 > ** frontend0 (DVB-S) > ** frontend1 (DVB-T) > ** frontend2 (DVB-C) Approach 2) > Your new "ideal" solution will be: > * adapter0 > ** frontend0 (DVB-S/T/C) Approach 3) > What really happens (mixed old and new): > * adapter0 > ** frontend0 (DVB-S) > ** frontend1 (DVB-T/C) What I've said before is that approach 3 is the "ideal" solution. > It does not look very good to offer this kind of mixed solution, since it is possible to offer only one solution for userspace, new or old, but not mixing. Good point. There's an additional aspect to handle: if a driver that uses approach 1, a conversion to either approach 2 or 3 would break existing applications that can't handle with the new approach. There's a 4th posibility: always offering fe0 with MFE capabilities, and creating additional fe's for old applications that can't cope with the new mode. For example, on a device that supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: Approach 4) fe0 is a frontend "superset" *adapter0 *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset *frontend1 (DVB-S/DVB-S2) *frontend2 (DVB-T/DVB-T2) *frontend3 (DVB-C) *frontend4 (ISDB-T) fe0 will need some special logic to allow redirecting a FE call to the right fe, if there are more than one physical frontend bound into the FE API. I'm starting to think that (4) is the better approach, as it won't break legacy applications, and it will provide an easier way for new applications to control the frontend with just one frontend. Cheers, Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 14:54 ` Mauro Carvalho Chehab @ 2011-07-16 15:40 ` Andreas Oberritter 2011-07-16 15:44 ` Antti Palosaari 2011-07-16 15:40 ` Oliver Endriss 1 sibling, 1 reply; 45+ messages in thread From: Andreas Oberritter @ 2011-07-16 15:40 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Antti Palosaari, Ralph Metzler, linux-media On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: > Em 16-07-2011 11:16, Antti Palosaari escreveu: >> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>> At the same time I want to add delivery system properties to >>>>>>> support everything in one frontend device. >>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>> cases where the application does not support switching yet. >>>>>> >>>>>> If I understood well, creating a multi-delivery type of frontend for >>>>>> devices like DRX-K makes sense for me. >>>>>> >>>>>> We need to take some care about how to add support for them, to avoid >>>>>> breaking userspace, or to follow kernel deprecating rules, by adding >>>>>> some legacy compatibility glue for a few kernel versions. So, the sooner >>>>>> we add such support, the better, as less drivers will need to support >>>>>> a "fallback" mechanism. >>>>>> >>>>>> The current DVB version 5 API doesn't prevent some userspace application >>>>>> to change the delivery system[1] for a given frontend. This feature is >>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >>>>>> DVB API multi-fe support, by avoiding the need of create of a secondary >>>>>> frontend for T2/S2. >>>>>> >>>>>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >>>>>> type of delivery system switch, using the same chip ended by needing to >>>>>> add two frontends. >>>>>> >>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >>>>>> add a way to query the type of delivery systems supported by a driver. >>>>>> >>>>>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>> >>>>> I don't think it's necessary to add a new flag. It should be sufficient >>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be >>>>> read-only and return an array of type fe_delivery_system_t. >>>>> >>>>> Querying this new property on present kernels hopefully fails with a >>>>> non-zero return code. in which case FE_GET_INFO should be used to query >>>>> the delivery system. >>>>> >>>>> In future kernels we can provide a default implementation, returning >>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>> should be able to override this default implementation in their >>>>> get_property callback. >>>> >>>> One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. >>>> >>>> If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. >>> >>> In this case, the driver should just create two frontends, as currently. >>> >>> There's a difference when there are two physical FE's and just one FE: >>> with 2 FE's, the userspace application can just keep both opened at >>> the same time. Some applications (like vdr) assumes that all multi-fe >>> are like that. >> >> Does this mean demod is not sleeping (.init() called)? >> >>> When there's just a single FE, but the driver needs to "fork" it in two >>> due to the API troubles, the driver needs to prevent the usage of both >>> fe's, either at open or at the ioctl level. So, applications like vdr >>> will only use the first frontend. >> >> Lets take example. There is shared MFE having DVB-S, DVB-T and DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have own. >> >> Currently it will shown as: > > Let me name the approaches: > > Approach 1) >> * adapter0 >> ** frontend0 (DVB-S) >> ** frontend1 (DVB-T) >> ** frontend2 (DVB-C) > > Approach 2) >> Your new "ideal" solution will be: >> * adapter0 >> ** frontend0 (DVB-S/T/C) > > Approach 3) >> What really happens (mixed old and new): >> * adapter0 >> ** frontend0 (DVB-S) >> ** frontend1 (DVB-T/C) > > What I've said before is that approach 3 is the "ideal" solution. > >> It does not look very good to offer this kind of mixed solution, since it is possible to offer only one solution for userspace, new or old, but not mixing. > > Good point. > > There's an additional aspect to handle: if a driver that uses approach 1, a conversion > to either approach 2 or 3 would break existing applications that can't handle with > the new approach. > > There's a 4th posibility: always offering fe0 with MFE capabilities, and creating additional fe's > for old applications that can't cope with the new mode. > For example, on a device that supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: > > Approach 4) fe0 is a frontend "superset" > > *adapter0 > *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset > *frontend1 (DVB-S/DVB-S2) > *frontend2 (DVB-T/DVB-T2) > *frontend3 (DVB-C) > *frontend4 (ISDB-T) > > fe0 will need some special logic to allow redirecting a FE call to the right fe, if > there are more than one physical frontend bound into the FE API. > > I'm starting to think that (4) is the better approach, as it won't break legacy > applications, and it will provide an easier way for new applications to control > the frontend with just one frontend. Approach 4 would break existing applications, because suddenly they'd have to cope with an additional device. It would be impossible for an existing application to tell whether frontend0 (from your example) was a real device or not. Approach 2 doesn't make any sense to me. The only sane approach is 3, because it creates one device node per demod chip. As Ralph already suggested, an easy way to not break applications is to add a module parameter which selects the default mode (DVB-C or DVB-T in Antti's example). One could also write a small command line application to switch modes independently from VDR et al. After all, you cannot connect both a DVB-C cable and a DVB-T antenna at the same time, so the vast majority of users won't ever want to switch modes at all. Regards, Andreas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:40 ` Andreas Oberritter @ 2011-07-16 15:44 ` Antti Palosaari 2011-07-16 15:53 ` Andreas Oberritter 2011-07-17 1:07 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 45+ messages in thread From: Antti Palosaari @ 2011-07-16 15:44 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Mauro Carvalho Chehab, Ralph Metzler, linux-media On 07/16/2011 06:40 PM, Andreas Oberritter wrote: > On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>> At the same time I want to add delivery system properties to >>>>>>>> support everything in one frontend device. >>>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>>> cases where the application does not support switching yet. >>>>>>> >>>>>>> If I understood well, creating a multi-delivery type of frontend for >>>>>>> devices like DRX-K makes sense for me. >>>>>>> >>>>>>> We need to take some care about how to add support for them, to avoid >>>>>>> breaking userspace, or to follow kernel deprecating rules, by adding >>>>>>> some legacy compatibility glue for a few kernel versions. So, the sooner >>>>>>> we add such support, the better, as less drivers will need to support >>>>>>> a "fallback" mechanism. >>>>>>> >>>>>>> The current DVB version 5 API doesn't prevent some userspace application >>>>>>> to change the delivery system[1] for a given frontend. This feature is >>>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the >>>>>>> DVB API multi-fe support, by avoiding the need of create of a secondary >>>>>>> frontend for T2/S2. >>>>>>> >>>>>>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION >>>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such >>>>>>> type of delivery system switch, using the same chip ended by needing to >>>>>>> add two frontends. >>>>>>> >>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and >>>>>>> add a way to query the type of delivery systems supported by a driver. >>>>>>> >>>>>>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>> >>>>>> I don't think it's necessary to add a new flag. It should be sufficient >>>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be >>>>>> read-only and return an array of type fe_delivery_system_t. >>>>>> >>>>>> Querying this new property on present kernels hopefully fails with a >>>>>> non-zero return code. in which case FE_GET_INFO should be used to query >>>>>> the delivery system. >>>>>> >>>>>> In future kernels we can provide a default implementation, returning >>>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>>> should be able to override this default implementation in their >>>>>> get_property callback. >>>>> >>>>> One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. >>>>> >>>>> If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. >>>> >>>> In this case, the driver should just create two frontends, as currently. >>>> >>>> There's a difference when there are two physical FE's and just one FE: >>>> with 2 FE's, the userspace application can just keep both opened at >>>> the same time. Some applications (like vdr) assumes that all multi-fe >>>> are like that. >>> >>> Does this mean demod is not sleeping (.init() called)? >>> >>>> When there's just a single FE, but the driver needs to "fork" it in two >>>> due to the API troubles, the driver needs to prevent the usage of both >>>> fe's, either at open or at the ioctl level. So, applications like vdr >>>> will only use the first frontend. >>> >>> Lets take example. There is shared MFE having DVB-S, DVB-T and DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have own. >>> >>> Currently it will shown as: >> >> Let me name the approaches: >> >> Approach 1) >>> * adapter0 >>> ** frontend0 (DVB-S) >>> ** frontend1 (DVB-T) >>> ** frontend2 (DVB-C) >> >> Approach 2) >>> Your new "ideal" solution will be: >>> * adapter0 >>> ** frontend0 (DVB-S/T/C) >> >> Approach 3) >>> What really happens (mixed old and new): >>> * adapter0 >>> ** frontend0 (DVB-S) >>> ** frontend1 (DVB-T/C) >> >> What I've said before is that approach 3 is the "ideal" solution. >> >>> It does not look very good to offer this kind of mixed solution, since it is possible to offer only one solution for userspace, new or old, but not mixing. >> >> Good point. >> >> There's an additional aspect to handle: if a driver that uses approach 1, a conversion >> to either approach 2 or 3 would break existing applications that can't handle with >> the new approach. >> >> There's a 4th posibility: always offering fe0 with MFE capabilities, and creating additional fe's >> for old applications that can't cope with the new mode. >> For example, on a device that supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: >> >> Approach 4) fe0 is a frontend "superset" >> >> *adapter0 >> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >> *frontend1 (DVB-S/DVB-S2) >> *frontend2 (DVB-T/DVB-T2) >> *frontend3 (DVB-C) >> *frontend4 (ISDB-T) >> >> fe0 will need some special logic to allow redirecting a FE call to the right fe, if >> there are more than one physical frontend bound into the FE API. >> >> I'm starting to think that (4) is the better approach, as it won't break legacy >> applications, and it will provide an easier way for new applications to control >> the frontend with just one frontend. > > Approach 4 would break existing applications, because suddenly they'd > have to cope with an additional device. It would be impossible for an > existing application to tell whether frontend0 (from your example) was a > real device or not. > > Approach 2 doesn't make any sense to me. I like approach 1 since it is very simple interface. Secondly I like approach 2. I think that more API issue than technical. > The only sane approach is 3, because it creates one device node per > demod chip. As Ralph already suggested, an easy way to not break > applications is to add a module parameter which selects the default mode > (DVB-C or DVB-T in Antti's example). One could also write a small > command line application to switch modes independently from VDR et al. > After all, you cannot connect both a DVB-C cable and a DVB-T antenna at > the same time, so the vast majority of users won't ever want to switch > modes at all. You are wrong, actually you can. At least here in Finland some cable networks offers DVB-T too. regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:44 ` Antti Palosaari @ 2011-07-16 15:53 ` Andreas Oberritter 2011-07-16 15:59 ` Antti Palosaari ` (2 more replies) 2011-07-17 1:07 ` Mauro Carvalho Chehab 1 sibling, 3 replies; 45+ messages in thread From: Andreas Oberritter @ 2011-07-16 15:53 UTC (permalink / raw) To: Antti Palosaari; +Cc: Mauro Carvalho Chehab, Ralph Metzler, linux-media On 16.07.2011 17:44, Antti Palosaari wrote: > On 07/16/2011 06:40 PM, Andreas Oberritter wrote: >> On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>>> At the same time I want to add delivery system properties to >>>>>>>>> support everything in one frontend device. >>>>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>>>> cases where the application does not support switching yet. >>>>>>>> >>>>>>>> If I understood well, creating a multi-delivery type of frontend >>>>>>>> for >>>>>>>> devices like DRX-K makes sense for me. >>>>>>>> >>>>>>>> We need to take some care about how to add support for them, to >>>>>>>> avoid >>>>>>>> breaking userspace, or to follow kernel deprecating rules, by >>>>>>>> adding >>>>>>>> some legacy compatibility glue for a few kernel versions. So, >>>>>>>> the sooner >>>>>>>> we add such support, the better, as less drivers will need to >>>>>>>> support >>>>>>>> a "fallback" mechanism. >>>>>>>> >>>>>>>> The current DVB version 5 API doesn't prevent some userspace >>>>>>>> application >>>>>>>> to change the delivery system[1] for a given frontend. This >>>>>>>> feature is >>>>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually >>>>>>>> improved the >>>>>>>> DVB API multi-fe support, by avoiding the need of create of a >>>>>>>> secondary >>>>>>>> frontend for T2/S2. >>>>>>>> >>>>>>>> Userspace applications can detect that feature by using >>>>>>>> FE_CAN_2G_MODULATION >>>>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that >>>>>>>> allow such >>>>>>>> type of delivery system switch, using the same chip ended by >>>>>>>> needing to >>>>>>>> add two frontends. >>>>>>>> >>>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to >>>>>>>> fe_caps_t, and >>>>>>>> add a way to query the type of delivery systems supported by a >>>>>>>> driver. >>>>>>>> >>>>>>>> [1] >>>>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>>>> >>>>>>> >>>>>>> I don't think it's necessary to add a new flag. It should be >>>>>>> sufficient >>>>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which >>>>>>> should be >>>>>>> read-only and return an array of type fe_delivery_system_t. >>>>>>> >>>>>>> Querying this new property on present kernels hopefully fails with a >>>>>>> non-zero return code. in which case FE_GET_INFO should be used to >>>>>>> query >>>>>>> the delivery system. >>>>>>> >>>>>>> In future kernels we can provide a default implementation, returning >>>>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>>>> should be able to override this default implementation in their >>>>>>> get_property callback. >>>>>> >>>>>> One thing I want to say is that consider about devices which does >>>>>> have MFE using two different *physical* demods, not integrated to >>>>>> same silicon. >>>>>> >>>>>> If you add such FE delsys switch mechanism it needs some more glue >>>>>> to bind two physical FEs to one virtual FE. I see much easier to >>>>>> keep all FEs as own - just register those under the same adapter >>>>>> if FEs are shared. >>>>> >>>>> In this case, the driver should just create two frontends, as >>>>> currently. >>>>> >>>>> There's a difference when there are two physical FE's and just one FE: >>>>> with 2 FE's, the userspace application can just keep both opened at >>>>> the same time. Some applications (like vdr) assumes that all multi-fe >>>>> are like that. >>>> >>>> Does this mean demod is not sleeping (.init() called)? >>>> >>>>> When there's just a single FE, but the driver needs to "fork" it in >>>>> two >>>>> due to the API troubles, the driver needs to prevent the usage of both >>>>> fe's, either at open or at the ioctl level. So, applications like vdr >>>>> will only use the first frontend. >>>> >>>> Lets take example. There is shared MFE having DVB-S, DVB-T and >>>> DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have >>>> own. One remark: In my previous mail I assumed that in your example DVB-S and either DVB-C or DVB-T can be tuned simultaneously, i.e. there are two antenna connectors and two tuners in addition to the two demod chips. If this assumtion was wrong, then of course approach 2 is the sane one, not approach 3. >>>> Currently it will shown as: >>> >>> Let me name the approaches: >>> >>> Approach 1) >>>> * adapter0 >>>> ** frontend0 (DVB-S) >>>> ** frontend1 (DVB-T) >>>> ** frontend2 (DVB-C) >>> >>> Approach 2) >>>> Your new "ideal" solution will be: >>>> * adapter0 >>>> ** frontend0 (DVB-S/T/C) >>> >>> Approach 3) >>>> What really happens (mixed old and new): >>>> * adapter0 >>>> ** frontend0 (DVB-S) >>>> ** frontend1 (DVB-T/C) >>> >>> What I've said before is that approach 3 is the "ideal" solution. >>> >>>> It does not look very good to offer this kind of mixed solution, >>>> since it is possible to offer only one solution for userspace, new >>>> or old, but not mixing. >>> >>> Good point. >>> >>> There's an additional aspect to handle: if a driver that uses >>> approach 1, a conversion >>> to either approach 2 or 3 would break existing applications that >>> can't handle with >>> the new approach. >>> >>> There's a 4th posibility: always offering fe0 with MFE capabilities, >>> and creating additional fe's >>> for old applications that can't cope with the new mode. >>> For example, on a device that supports >>> DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: >>> >>> Approach 4) fe0 is a frontend "superset" >>> >>> *adapter0 >>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >>> *frontend1 (DVB-S/DVB-S2) >>> *frontend2 (DVB-T/DVB-T2) >>> *frontend3 (DVB-C) >>> *frontend4 (ISDB-T) >>> >>> fe0 will need some special logic to allow redirecting a FE call to >>> the right fe, if >>> there are more than one physical frontend bound into the FE API. >>> >>> I'm starting to think that (4) is the better approach, as it won't >>> break legacy >>> applications, and it will provide an easier way for new applications >>> to control >>> the frontend with just one frontend. >> >> Approach 4 would break existing applications, because suddenly they'd >> have to cope with an additional device. It would be impossible for an >> existing application to tell whether frontend0 (from your example) was a >> real device or not. >> >> Approach 2 doesn't make any sense to me. > > I like approach 1 since it is very simple interface. Secondly I like > approach 2. I think that more API issue than technical. > > >> The only sane approach is 3, because it creates one device node per >> demod chip. As Ralph already suggested, an easy way to not break >> applications is to add a module parameter which selects the default mode >> (DVB-C or DVB-T in Antti's example). One could also write a small >> command line application to switch modes independently from VDR et al. >> After all, you cannot connect both a DVB-C cable and a DVB-T antenna at >> the same time, so the vast majority of users won't ever want to switch >> modes at all. > > You are wrong, actually you can. At least here in Finland some cable > networks offers DVB-T too. I know that there are cable operators which use DVB-T, but they don't use DVB-C simultaneously. This wouldn't make sense, unless they didn't want their customers to receive their signals. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:53 ` Andreas Oberritter @ 2011-07-16 15:59 ` Antti Palosaari 2011-07-16 16:37 ` Rémi Denis-Courmont 2011-07-17 0:56 ` Mauro Carvalho Chehab 2 siblings, 0 replies; 45+ messages in thread From: Antti Palosaari @ 2011-07-16 15:59 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Mauro Carvalho Chehab, Ralph Metzler, linux-media On 07/16/2011 06:53 PM, Andreas Oberritter wrote: > On 16.07.2011 17:44, Antti Palosaari wrote: >> On 07/16/2011 06:40 PM, Andreas Oberritter wrote: >>> On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >>>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>>>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>>>> At the same time I want to add delivery system properties to >>>>>>>>>> support everything in one frontend device. >>>>>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>>>>> cases where the application does not support switching yet. >>>>>>>>> >>>>>>>>> If I understood well, creating a multi-delivery type of frontend >>>>>>>>> for >>>>>>>>> devices like DRX-K makes sense for me. >>>>>>>>> >>>>>>>>> We need to take some care about how to add support for them, to >>>>>>>>> avoid >>>>>>>>> breaking userspace, or to follow kernel deprecating rules, by >>>>>>>>> adding >>>>>>>>> some legacy compatibility glue for a few kernel versions. So, >>>>>>>>> the sooner >>>>>>>>> we add such support, the better, as less drivers will need to >>>>>>>>> support >>>>>>>>> a "fallback" mechanism. >>>>>>>>> >>>>>>>>> The current DVB version 5 API doesn't prevent some userspace >>>>>>>>> application >>>>>>>>> to change the delivery system[1] for a given frontend. This >>>>>>>>> feature is >>>>>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually >>>>>>>>> improved the >>>>>>>>> DVB API multi-fe support, by avoiding the need of create of a >>>>>>>>> secondary >>>>>>>>> frontend for T2/S2. >>>>>>>>> >>>>>>>>> Userspace applications can detect that feature by using >>>>>>>>> FE_CAN_2G_MODULATION >>>>>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that >>>>>>>>> allow such >>>>>>>>> type of delivery system switch, using the same chip ended by >>>>>>>>> needing to >>>>>>>>> add two frontends. >>>>>>>>> >>>>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to >>>>>>>>> fe_caps_t, and >>>>>>>>> add a way to query the type of delivery systems supported by a >>>>>>>>> driver. >>>>>>>>> >>>>>>>>> [1] >>>>>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>>>>> >>>>>>>> >>>>>>>> I don't think it's necessary to add a new flag. It should be >>>>>>>> sufficient >>>>>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which >>>>>>>> should be >>>>>>>> read-only and return an array of type fe_delivery_system_t. >>>>>>>> >>>>>>>> Querying this new property on present kernels hopefully fails with a >>>>>>>> non-zero return code. in which case FE_GET_INFO should be used to >>>>>>>> query >>>>>>>> the delivery system. >>>>>>>> >>>>>>>> In future kernels we can provide a default implementation, returning >>>>>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>>>>> should be able to override this default implementation in their >>>>>>>> get_property callback. >>>>>>> >>>>>>> One thing I want to say is that consider about devices which does >>>>>>> have MFE using two different *physical* demods, not integrated to >>>>>>> same silicon. >>>>>>> >>>>>>> If you add such FE delsys switch mechanism it needs some more glue >>>>>>> to bind two physical FEs to one virtual FE. I see much easier to >>>>>>> keep all FEs as own - just register those under the same adapter >>>>>>> if FEs are shared. >>>>>> >>>>>> In this case, the driver should just create two frontends, as >>>>>> currently. >>>>>> >>>>>> There's a difference when there are two physical FE's and just one FE: >>>>>> with 2 FE's, the userspace application can just keep both opened at >>>>>> the same time. Some applications (like vdr) assumes that all multi-fe >>>>>> are like that. >>>>> >>>>> Does this mean demod is not sleeping (.init() called)? >>>>> >>>>>> When there's just a single FE, but the driver needs to "fork" it in >>>>>> two >>>>>> due to the API troubles, the driver needs to prevent the usage of both >>>>>> fe's, either at open or at the ioctl level. So, applications like vdr >>>>>> will only use the first frontend. >>>>> >>>>> Lets take example. There is shared MFE having DVB-S, DVB-T and >>>>> DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have >>>>> own. > > One remark: In my previous mail I assumed that in your example DVB-S and > either DVB-C or DVB-T can be tuned simultaneously, i.e. there are two > antenna connectors and two tuners in addition to the two demod chips. If > this assumtion was wrong, then of course approach 2 is the sane one, not > approach 3. My assumption was that frontends are using shared HW resources for reason or the other and thus only one FE can be used at the time. When there is no shared resources it should be implemented as multiple adapters. >>>>> Currently it will shown as: >>>> >>>> Let me name the approaches: >>>> >>>> Approach 1) >>>>> * adapter0 >>>>> ** frontend0 (DVB-S) >>>>> ** frontend1 (DVB-T) >>>>> ** frontend2 (DVB-C) >>>> >>>> Approach 2) >>>>> Your new "ideal" solution will be: >>>>> * adapter0 >>>>> ** frontend0 (DVB-S/T/C) >>>> >>>> Approach 3) >>>>> What really happens (mixed old and new): >>>>> * adapter0 >>>>> ** frontend0 (DVB-S) >>>>> ** frontend1 (DVB-T/C) >>>> >>>> What I've said before is that approach 3 is the "ideal" solution. >>>> >>>>> It does not look very good to offer this kind of mixed solution, >>>>> since it is possible to offer only one solution for userspace, new >>>>> or old, but not mixing. >>>> >>>> Good point. >>>> >>>> There's an additional aspect to handle: if a driver that uses >>>> approach 1, a conversion >>>> to either approach 2 or 3 would break existing applications that >>>> can't handle with >>>> the new approach. >>>> >>>> There's a 4th posibility: always offering fe0 with MFE capabilities, >>>> and creating additional fe's >>>> for old applications that can't cope with the new mode. >>>> For example, on a device that supports >>>> DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: >>>> >>>> Approach 4) fe0 is a frontend "superset" >>>> >>>> *adapter0 >>>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >>>> *frontend1 (DVB-S/DVB-S2) >>>> *frontend2 (DVB-T/DVB-T2) >>>> *frontend3 (DVB-C) >>>> *frontend4 (ISDB-T) >>>> >>>> fe0 will need some special logic to allow redirecting a FE call to >>>> the right fe, if >>>> there are more than one physical frontend bound into the FE API. >>>> >>>> I'm starting to think that (4) is the better approach, as it won't >>>> break legacy >>>> applications, and it will provide an easier way for new applications >>>> to control >>>> the frontend with just one frontend. >>> >>> Approach 4 would break existing applications, because suddenly they'd >>> have to cope with an additional device. It would be impossible for an >>> existing application to tell whether frontend0 (from your example) was a >>> real device or not. >>> >>> Approach 2 doesn't make any sense to me. >> >> I like approach 1 since it is very simple interface. Secondly I like >> approach 2. I think that more API issue than technical. >> >> >>> The only sane approach is 3, because it creates one device node per >>> demod chip. As Ralph already suggested, an easy way to not break >>> applications is to add a module parameter which selects the default mode >>> (DVB-C or DVB-T in Antti's example). One could also write a small >>> command line application to switch modes independently from VDR et al. >>> After all, you cannot connect both a DVB-C cable and a DVB-T antenna at >>> the same time, so the vast majority of users won't ever want to switch >>> modes at all. >> >> You are wrong, actually you can. At least here in Finland some cable >> networks offers DVB-T too. > > I know that there are cable operators which use DVB-T, but they don't > use DVB-C simultaneously. This wouldn't make sense, unless they didn't > want their customers to receive their signals. Hmmm, after all that's not big issue but they send both DVB-C and DVB-T muxes in same network. Most likely to offers some basic channels for customers who does not have DVB-C capable receiver. So setting desired FE to DVB-C mode blocks reception of DVB-T channels. regards Antti -- http://palosaari.fi/ ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:53 ` Andreas Oberritter 2011-07-16 15:59 ` Antti Palosaari @ 2011-07-16 16:37 ` Rémi Denis-Courmont 2011-07-17 2:51 ` Andreas Oberritter 2011-07-17 0:56 ` Mauro Carvalho Chehab 2 siblings, 1 reply; 45+ messages in thread From: Rémi Denis-Courmont @ 2011-07-16 16:37 UTC (permalink / raw) To: linux-media Le samedi 16 juillet 2011 18:53:16 Andreas Oberritter, vous avez écrit : > > You are wrong, actually you can. At least here in Finland some cable > > networks offers DVB-T too. > > I know that there are cable operators which use DVB-T, but they don't > use DVB-C simultaneously. This wouldn't make sense, unless they didn't > want their customers to receive their signals. They do offer both simultaneously. DNA (formerly Welho) in Helsinki provides both DVB-T and DVB-C on the same cable, obviously on different frequencies. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 16:37 ` Rémi Denis-Courmont @ 2011-07-17 2:51 ` Andreas Oberritter 2011-07-17 7:51 ` Rémi Denis-Courmont 0 siblings, 1 reply; 45+ messages in thread From: Andreas Oberritter @ 2011-07-17 2:51 UTC (permalink / raw) To: Rémi Denis-Courmont; +Cc: linux-media On 16.07.2011 18:37, Rémi Denis-Courmont wrote: > Le samedi 16 juillet 2011 18:53:16 Andreas Oberritter, vous avez écrit : >>> You are wrong, actually you can. At least here in Finland some cable >>> networks offers DVB-T too. >> >> I know that there are cable operators which use DVB-T, but they don't >> use DVB-C simultaneously. This wouldn't make sense, unless they didn't >> want their customers to receive their signals. > > They do offer both simultaneously. DNA (formerly Welho) in Helsinki provides > both DVB-T and DVB-C on the same cable, obviously on different frequencies. Is there any channel available on DVB-T which isn't available on DVB-C in this cable network? ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-17 2:51 ` Andreas Oberritter @ 2011-07-17 7:51 ` Rémi Denis-Courmont 0 siblings, 0 replies; 45+ messages in thread From: Rémi Denis-Courmont @ 2011-07-17 7:51 UTC (permalink / raw) To: Andreas Oberritter; +Cc: linux-media Le dimanche 17 juillet 2011 05:51:33 Andreas Oberritter, vous avez écrit : > On 16.07.2011 18:37, Rémi Denis-Courmont wrote: > > Le samedi 16 juillet 2011 18:53:16 Andreas Oberritter, vous avez écrit : > >>> You are wrong, actually you can. At least here in Finland some cable > >>> networks offers DVB-T too. > >> > >> I know that there are cable operators which use DVB-T, but they don't > >> use DVB-C simultaneously. This wouldn't make sense, unless they didn't > >> want their customers to receive their signals. > > > > They do offer both simultaneously. DNA (formerly Welho) in Helsinki > > provides both DVB-T and DVB-C on the same cable, obviously on different > > frequencies. > > Is there any channel available on DVB-T which isn't available on DVB-C > in this cable network? Probably, I wouldn't know. My S******g TV won't provision channels from both systems at the same time. And Linux does not support DVB-T on my TT-connect CT-3650 tuner. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:53 ` Andreas Oberritter 2011-07-16 15:59 ` Antti Palosaari 2011-07-16 16:37 ` Rémi Denis-Courmont @ 2011-07-17 0:56 ` Mauro Carvalho Chehab 2011-07-17 3:02 ` Andreas Oberritter 2011-07-17 7:39 ` Rémi Denis-Courmont 2 siblings, 2 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-17 0:56 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Antti Palosaari, Ralph Metzler, linux-media Em 16-07-2011 12:53, Andreas Oberritter escreveu: > On 16.07.2011 17:44, Antti Palosaari wrote: >> On 07/16/2011 06:40 PM, Andreas Oberritter wrote: >>> On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >>>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>>>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>>>> At the same time I want to add delivery system properties to >>>>>>>>>> support everything in one frontend device. >>>>>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>>>>> cases where the application does not support switching yet. >>>>>>>>> >>>>>>>>> If I understood well, creating a multi-delivery type of frontend >>>>>>>>> for >>>>>>>>> devices like DRX-K makes sense for me. >>>>>>>>> >>>>>>>>> We need to take some care about how to add support for them, to >>>>>>>>> avoid >>>>>>>>> breaking userspace, or to follow kernel deprecating rules, by >>>>>>>>> adding >>>>>>>>> some legacy compatibility glue for a few kernel versions. So, >>>>>>>>> the sooner >>>>>>>>> we add such support, the better, as less drivers will need to >>>>>>>>> support >>>>>>>>> a "fallback" mechanism. >>>>>>>>> >>>>>>>>> The current DVB version 5 API doesn't prevent some userspace >>>>>>>>> application >>>>>>>>> to change the delivery system[1] for a given frontend. This >>>>>>>>> feature is >>>>>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually >>>>>>>>> improved the >>>>>>>>> DVB API multi-fe support, by avoiding the need of create of a >>>>>>>>> secondary >>>>>>>>> frontend for T2/S2. >>>>>>>>> >>>>>>>>> Userspace applications can detect that feature by using >>>>>>>>> FE_CAN_2G_MODULATION >>>>>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that >>>>>>>>> allow such >>>>>>>>> type of delivery system switch, using the same chip ended by >>>>>>>>> needing to >>>>>>>>> add two frontends. >>>>>>>>> >>>>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to >>>>>>>>> fe_caps_t, and >>>>>>>>> add a way to query the type of delivery systems supported by a >>>>>>>>> driver. >>>>>>>>> >>>>>>>>> [1] >>>>>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>>>>> >>>>>>>> >>>>>>>> I don't think it's necessary to add a new flag. It should be >>>>>>>> sufficient >>>>>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which >>>>>>>> should be >>>>>>>> read-only and return an array of type fe_delivery_system_t. >>>>>>>> >>>>>>>> Querying this new property on present kernels hopefully fails with a >>>>>>>> non-zero return code. in which case FE_GET_INFO should be used to >>>>>>>> query >>>>>>>> the delivery system. >>>>>>>> >>>>>>>> In future kernels we can provide a default implementation, returning >>>>>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>>>>> should be able to override this default implementation in their >>>>>>>> get_property callback. >>>>>>> >>>>>>> One thing I want to say is that consider about devices which does >>>>>>> have MFE using two different *physical* demods, not integrated to >>>>>>> same silicon. >>>>>>> >>>>>>> If you add such FE delsys switch mechanism it needs some more glue >>>>>>> to bind two physical FEs to one virtual FE. I see much easier to >>>>>>> keep all FEs as own - just register those under the same adapter >>>>>>> if FEs are shared. >>>>>> >>>>>> In this case, the driver should just create two frontends, as >>>>>> currently. >>>>>> >>>>>> There's a difference when there are two physical FE's and just one FE: >>>>>> with 2 FE's, the userspace application can just keep both opened at >>>>>> the same time. Some applications (like vdr) assumes that all multi-fe >>>>>> are like that. >>>>> >>>>> Does this mean demod is not sleeping (.init() called)? >>>>> >>>>>> When there's just a single FE, but the driver needs to "fork" it in >>>>>> two >>>>>> due to the API troubles, the driver needs to prevent the usage of both >>>>>> fe's, either at open or at the ioctl level. So, applications like vdr >>>>>> will only use the first frontend. >>>>> >>>>> Lets take example. There is shared MFE having DVB-S, DVB-T and >>>>> DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have >>>>> own. > > One remark: In my previous mail I assumed that in your example DVB-S and > either DVB-C or DVB-T can be tuned simultaneously, i.e. there are two > antenna connectors and two tuners in addition to the two demod chips. If > this assumtion was wrong, then of course approach 2 is the sane one, not > approach 3. > >>>>> Currently it will shown as: >>>> >>>> Let me name the approaches: >>>> >>>> Approach 1) >>>>> * adapter0 >>>>> ** frontend0 (DVB-S) >>>>> ** frontend1 (DVB-T) >>>>> ** frontend2 (DVB-C) >>>> >>>> Approach 2) >>>>> Your new "ideal" solution will be: >>>>> * adapter0 >>>>> ** frontend0 (DVB-S/T/C) >>>> >>>> Approach 3) >>>>> What really happens (mixed old and new): >>>>> * adapter0 >>>>> ** frontend0 (DVB-S) >>>>> ** frontend1 (DVB-T/C) >>>> >>>> What I've said before is that approach 3 is the "ideal" solution. >>>> >>>>> It does not look very good to offer this kind of mixed solution, >>>>> since it is possible to offer only one solution for userspace, new >>>>> or old, but not mixing. >>>> >>>> Good point. >>>> >>>> There's an additional aspect to handle: if a driver that uses >>>> approach 1, a conversion >>>> to either approach 2 or 3 would break existing applications that >>>> can't handle with >>>> the new approach. >>>> >>>> There's a 4th posibility: always offering fe0 with MFE capabilities, >>>> and creating additional fe's >>>> for old applications that can't cope with the new mode. >>>> For example, on a device that supports >>>> DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: >>>> >>>> Approach 4) fe0 is a frontend "superset" >>>> >>>> *adapter0 >>>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >>>> *frontend1 (DVB-S/DVB-S2) >>>> *frontend2 (DVB-T/DVB-T2) >>>> *frontend3 (DVB-C) >>>> *frontend4 (ISDB-T) >>>> >>>> fe0 will need some special logic to allow redirecting a FE call to >>>> the right fe, if >>>> there are more than one physical frontend bound into the FE API. >>>> >>>> I'm starting to think that (4) is the better approach, as it won't >>>> break legacy >>>> applications, and it will provide an easier way for new applications >>>> to control >>>> the frontend with just one frontend. >>> >>> Approach 4 would break existing applications, because suddenly they'd >>> have to cope with an additional device. It would be impossible for an >>> existing application to tell whether frontend0 (from your example) was a >>> real device or not. >>> >>> Approach 2 doesn't make any sense to me. >> >> I like approach 1 since it is very simple interface. Yes: approach 1 has a simple interface. The only issue on that is that it is not possible to know if fe0 and fe1 can be used simultaneously, or if they're mutually exclusive. Several userspace applications currently don't work on that scenario, where the frontends are mutually exclusive. So, if we're going to use approach 1, we need to properly document that applications should be prepared for that scenario, and, maybe, adding some way for userspace to detect frontend "groups". >> Secondly I like >> approach 2. I think that more API issue than technical. Approach 2 limits the usage of two simultaneous fe, when they're not mutually exclusive. Not sure if this is actually a problem. >>> The only sane approach is 3, because it creates one device node per >>> demod chip. >> As Ralph already suggested, an easy way to not break >>> applications is to add a module parameter which selects the default mode >>> (DVB-C or DVB-T in Antti's example). One could also write a small >>> command line application to switch modes independently from VDR et al. There are two issues with this approach: - not breaking existing applications; - makes life harder for userspace, as they'll need to support multiple frontends and the delivery system switch. Ralph suggestion may help to solve the backport issue. Userspace applications that support DVB-T2/DVB-S2 already support delivery system switch. So, approach 3 seems feasible for me. >>> After all, you cannot connect both a DVB-C cable and a DVB-T antenna at >>> the same time, so the vast majority of users won't ever want to switch >>> modes at all. >> You are wrong, actually you can. At least here in Finland some cable >> networks offers DVB-T too. As Antti and Rémi pointed, there are issues with some cable operators. Not sure how critical is that, but an userspace application changing it via sysfs might work while the applications are not ported to support both ways. Antti/Rémi, how the current applications work with one physical frontend supporting both DVB-T and DVB-C? Do they allow to change channels from one to the other mode on a transparent way? Cheers, Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-17 0:56 ` Mauro Carvalho Chehab @ 2011-07-17 3:02 ` Andreas Oberritter 2011-07-17 3:59 ` Mauro Carvalho Chehab 2011-07-17 7:39 ` Rémi Denis-Courmont 1 sibling, 1 reply; 45+ messages in thread From: Andreas Oberritter @ 2011-07-17 3:02 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Antti Palosaari, Ralph Metzler, linux-media On 17.07.2011 02:56, Mauro Carvalho Chehab wrote: > Em 16-07-2011 12:53, Andreas Oberritter escreveu: >> On 16.07.2011 17:44, Antti Palosaari wrote: >>> On 07/16/2011 06:40 PM, Andreas Oberritter wrote: >>>> On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >>>>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>>>>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>>>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>>>>> At the same time I want to add delivery system properties to >>>>>>>>>>> support everything in one frontend device. >>>>>>>>>>> Adding a parameter to select C or T as default should help in most >>>>>>>>>>> cases where the application does not support switching yet. >>>>>>>>>> >>>>>>>>>> If I understood well, creating a multi-delivery type of frontend >>>>>>>>>> for >>>>>>>>>> devices like DRX-K makes sense for me. >>>>>>>>>> >>>>>>>>>> We need to take some care about how to add support for them, to >>>>>>>>>> avoid >>>>>>>>>> breaking userspace, or to follow kernel deprecating rules, by >>>>>>>>>> adding >>>>>>>>>> some legacy compatibility glue for a few kernel versions. So, >>>>>>>>>> the sooner >>>>>>>>>> we add such support, the better, as less drivers will need to >>>>>>>>>> support >>>>>>>>>> a "fallback" mechanism. >>>>>>>>>> >>>>>>>>>> The current DVB version 5 API doesn't prevent some userspace >>>>>>>>>> application >>>>>>>>>> to change the delivery system[1] for a given frontend. This >>>>>>>>>> feature is >>>>>>>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually >>>>>>>>>> improved the >>>>>>>>>> DVB API multi-fe support, by avoiding the need of create of a >>>>>>>>>> secondary >>>>>>>>>> frontend for T2/S2. >>>>>>>>>> >>>>>>>>>> Userspace applications can detect that feature by using >>>>>>>>>> FE_CAN_2G_MODULATION >>>>>>>>>> flag, but this mechanism doesn't allow other types of changes like >>>>>>>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that >>>>>>>>>> allow such >>>>>>>>>> type of delivery system switch, using the same chip ended by >>>>>>>>>> needing to >>>>>>>>>> add two frontends. >>>>>>>>>> >>>>>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to >>>>>>>>>> fe_caps_t, and >>>>>>>>>> add a way to query the type of delivery systems supported by a >>>>>>>>>> driver. >>>>>>>>>> >>>>>>>>>> [1] >>>>>>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>>>>>> >>>>>>>>> >>>>>>>>> I don't think it's necessary to add a new flag. It should be >>>>>>>>> sufficient >>>>>>>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which >>>>>>>>> should be >>>>>>>>> read-only and return an array of type fe_delivery_system_t. >>>>>>>>> >>>>>>>>> Querying this new property on present kernels hopefully fails with a >>>>>>>>> non-zero return code. in which case FE_GET_INFO should be used to >>>>>>>>> query >>>>>>>>> the delivery system. >>>>>>>>> >>>>>>>>> In future kernels we can provide a default implementation, returning >>>>>>>>> exactly one fe_delivery_system_t for unported drivers. Other drivers >>>>>>>>> should be able to override this default implementation in their >>>>>>>>> get_property callback. >>>>>>>> >>>>>>>> One thing I want to say is that consider about devices which does >>>>>>>> have MFE using two different *physical* demods, not integrated to >>>>>>>> same silicon. >>>>>>>> >>>>>>>> If you add such FE delsys switch mechanism it needs some more glue >>>>>>>> to bind two physical FEs to one virtual FE. I see much easier to >>>>>>>> keep all FEs as own - just register those under the same adapter >>>>>>>> if FEs are shared. >>>>>>> >>>>>>> In this case, the driver should just create two frontends, as >>>>>>> currently. >>>>>>> >>>>>>> There's a difference when there are two physical FE's and just one FE: >>>>>>> with 2 FE's, the userspace application can just keep both opened at >>>>>>> the same time. Some applications (like vdr) assumes that all multi-fe >>>>>>> are like that. >>>>>> >>>>>> Does this mean demod is not sleeping (.init() called)? >>>>>> >>>>>>> When there's just a single FE, but the driver needs to "fork" it in >>>>>>> two >>>>>>> due to the API troubles, the driver needs to prevent the usage of both >>>>>>> fe's, either at open or at the ioctl level. So, applications like vdr >>>>>>> will only use the first frontend. >>>>>> >>>>>> Lets take example. There is shared MFE having DVB-S, DVB-T and >>>>>> DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have >>>>>> own. >> >> One remark: In my previous mail I assumed that in your example DVB-S and >> either DVB-C or DVB-T can be tuned simultaneously, i.e. there are two >> antenna connectors and two tuners in addition to the two demod chips. If >> this assumtion was wrong, then of course approach 2 is the sane one, not >> approach 3. >> >>>>>> Currently it will shown as: >>>>> >>>>> Let me name the approaches: >>>>> >>>>> Approach 1) >>>>>> * adapter0 >>>>>> ** frontend0 (DVB-S) >>>>>> ** frontend1 (DVB-T) >>>>>> ** frontend2 (DVB-C) >>>>> >>>>> Approach 2) >>>>>> Your new "ideal" solution will be: >>>>>> * adapter0 >>>>>> ** frontend0 (DVB-S/T/C) >>>>> >>>>> Approach 3) >>>>>> What really happens (mixed old and new): >>>>>> * adapter0 >>>>>> ** frontend0 (DVB-S) >>>>>> ** frontend1 (DVB-T/C) >>>>> >>>>> What I've said before is that approach 3 is the "ideal" solution. >>>>> >>>>>> It does not look very good to offer this kind of mixed solution, >>>>>> since it is possible to offer only one solution for userspace, new >>>>>> or old, but not mixing. >>>>> >>>>> Good point. >>>>> >>>>> There's an additional aspect to handle: if a driver that uses >>>>> approach 1, a conversion >>>>> to either approach 2 or 3 would break existing applications that >>>>> can't handle with >>>>> the new approach. >>>>> >>>>> There's a 4th posibility: always offering fe0 with MFE capabilities, >>>>> and creating additional fe's >>>>> for old applications that can't cope with the new mode. >>>>> For example, on a device that supports >>>>> DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: >>>>> >>>>> Approach 4) fe0 is a frontend "superset" >>>>> >>>>> *adapter0 >>>>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >>>>> *frontend1 (DVB-S/DVB-S2) >>>>> *frontend2 (DVB-T/DVB-T2) >>>>> *frontend3 (DVB-C) >>>>> *frontend4 (ISDB-T) >>>>> >>>>> fe0 will need some special logic to allow redirecting a FE call to >>>>> the right fe, if >>>>> there are more than one physical frontend bound into the FE API. >>>>> >>>>> I'm starting to think that (4) is the better approach, as it won't >>>>> break legacy >>>>> applications, and it will provide an easier way for new applications >>>>> to control >>>>> the frontend with just one frontend. >>>> >>>> Approach 4 would break existing applications, because suddenly they'd >>>> have to cope with an additional device. It would be impossible for an >>>> existing application to tell whether frontend0 (from your example) was a >>>> real device or not. >>>> >>>> Approach 2 doesn't make any sense to me. >>> >>> I like approach 1 since it is very simple interface. > > Yes: approach 1 has a simple interface. The only issue on that is that it is > not possible to know if fe0 and fe1 can be used simultaneously, or if they're > mutually exclusive. Several userspace applications currently don't work on that > scenario, where the frontends are mutually exclusive. > > So, if we're going to use approach 1, we need to properly document that applications > should be prepared for that scenario, and, maybe, adding some way for userspace > to detect frontend "groups". > >>> Secondly I like >>> approach 2. I think that more API issue than technical. > > Approach 2 limits the usage of two simultaneous fe, when they're not > mutually exclusive. Not sure if this is actually a problem. This would be a problem, of course. If they're not mutually exclusive, then I'd expect the possibility to use them simultaneously. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-17 3:02 ` Andreas Oberritter @ 2011-07-17 3:59 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-17 3:59 UTC (permalink / raw) To: Andreas Oberritter; +Cc: Antti Palosaari, Ralph Metzler, linux-media Em 17-07-2011 00:02, Andreas Oberritter escreveu: >> Approach 2 limits the usage of two simultaneous fe, when they're not >> mutually exclusive. Not sure if this is actually a problem. > > This would be a problem, of course. If they're not mutually exclusive, > then I'd expect the possibility to use them simultaneously. >From userspace perspective, the possibility of using them simultaneously may not actually be useful, provided that they both share the same demux interface. However, I agree that adding an artificial limit there doesn't seem right. Cheers, Mauro. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-17 0:56 ` Mauro Carvalho Chehab 2011-07-17 3:02 ` Andreas Oberritter @ 2011-07-17 7:39 ` Rémi Denis-Courmont 2011-07-17 8:01 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 45+ messages in thread From: Rémi Denis-Courmont @ 2011-07-17 7:39 UTC (permalink / raw) To: linux-media Le dimanche 17 juillet 2011 03:56:36 Mauro Carvalho Chehab, vous avez écrit : > >>> After all, you cannot connect both a DVB-C cable and a DVB-T antenna at > >>> the same time, so the vast majority of users won't ever want to switch > >>> modes at all. > >> > >> You are wrong, actually you can. At least here in Finland some cable > >> networks offers DVB-T too. > > As Antti and Rémi pointed, there are issues with some cable operators. Not > sure how critical is that, but an userspace application changing it via > sysfs might work while the applications are not ported to support both > ways. Telling applications to use sysfs... I can see many ways that you might regret that in the future... Accessing sysfs directly from an application is against all the good practices I thought I had learnt regarding Linux. There is the theoretical possibility that udev gets "explicit" support for Linux DVB and exposes the properties nicely. But that would be rather inconvenient, and cannot be used to change properties. > Antti/Rémi, how the current applications work with one physical frontend > supporting both DVB-T and DVB-C? Do they allow to change channels from one > to the other mode on a transparent way? I don't know. VLC does not care if you switch from DVB-T to DVB-C, to the DVD drive or to YouTube. Each channel (or at least each multiplex) is a different playlist item. So it'll close the all device nodes and (re)open them. There are obviously other applications at stake. -- Rémi Denis-Courmont http://www.remlab.net/ http://fi.linkedin.com/in/remidenis ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-17 7:39 ` Rémi Denis-Courmont @ 2011-07-17 8:01 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-17 8:01 UTC (permalink / raw) To: Rémi Denis-Courmont; +Cc: linux-media Em 17-07-2011 04:39, Rémi Denis-Courmont escreveu: > Le dimanche 17 juillet 2011 03:56:36 Mauro Carvalho Chehab, vous avez écrit : >>>>> After all, you cannot connect both a DVB-C cable and a DVB-T antenna at >>>>> the same time, so the vast majority of users won't ever want to switch >>>>> modes at all. >>>> >>>> You are wrong, actually you can. At least here in Finland some cable >>>> networks offers DVB-T too. >> >> As Antti and Rémi pointed, there are issues with some cable operators. Not >> sure how critical is that, but an userspace application changing it via >> sysfs might work while the applications are not ported to support both >> ways. > > Telling applications to use sysfs... I can see many ways that you might regret > that in the future... I'm expressed it badly. What I meant to say is to have some sort of script or a specific application to allow users to change the delivery system, by changing the modprobe parameter, for the MFE drivers supported on <= 3.0 Kernel that won't fit in the agreed approach, while applications don't support the adopted approach directly. > Accessing sysfs directly from an application is against all the good practices > I thought I had learnt regarding Linux. There is the theoretical possibility > that udev gets "explicit" support for Linux DVB and exposes the properties > nicely. But that would be rather inconvenient, and cannot be used to change > properties. > >> Antti/Rémi, how the current applications work with one physical frontend >> supporting both DVB-T and DVB-C? Do they allow to change channels from one >> to the other mode on a transparent way? > > I don't know. VLC does not care if you switch from DVB-T to DVB-C, to the DVD > drive or to YouTube. Each channel (or at least each multiplex) is a different > playlist item. So it'll close the all device nodes and (re)open them. There > are obviously other applications at stake. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:44 ` Antti Palosaari 2011-07-16 15:53 ` Andreas Oberritter @ 2011-07-17 1:07 ` Mauro Carvalho Chehab 1 sibling, 0 replies; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-17 1:07 UTC (permalink / raw) To: Antti Palosaari; +Cc: Andreas Oberritter, Ralph Metzler, linux-media Em 16-07-2011 12:44, Antti Palosaari escreveu: > On 07/16/2011 06:40 PM, Andreas Oberritter wrote: >> On 16.07.2011 16:54, Mauro Carvalho Chehab wrote: >>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>> Approach 4) fe0 is a frontend "superset" >>> >>> *adapter0 >>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset >>> *frontend1 (DVB-S/DVB-S2) >>> *frontend2 (DVB-T/DVB-T2) >>> *frontend3 (DVB-C) >>> *frontend4 (ISDB-T) >>> >>> fe0 will need some special logic to allow redirecting a FE call to the right fe, if >>> there are more than one physical frontend bound into the FE API. >>> >>> I'm starting to think that (4) is the better approach, as it won't break legacy >>> applications, and it will provide an easier way for new applications to control >>> the frontend with just one frontend. >> >> Approach 4 would break existing applications, because suddenly they'd >> have to cope with an additional device. It would be impossible for an >> existing application to tell whether frontend0 (from your example) was a >> real device or not. (not sure who commented this... somehow, I didn't receive the original email - well, I'll just reply on Antti's answer) Yes, an existing application will not know how to handle such fe, but, as the other fe's are still provided, they can swill switch the delivery system by replacing the frontend they're using. There are some alternatives for this approach, like: Approach 5) fe0 is a frontend "superset", initialized to handle the first registered delivery system >>> *adapter0 >>> *frontend0 (DVB-S/DVB-S2), but allows changing to DVB-T/DVB-T2/DVB-C/ISDB-T >>> *frontend1 (DVB-T/DVB-T2) >>> *frontend2 (DVB-C) >>> *frontend3 (ISDB-T) (so, it is something between approach 1 and 4) Being frankly, I think that this would be messy. In any case, I think that, if we decide for something like approach 4 or 5, we should deprecate the support for the extra frontends, after kernel + 2 versions, so, falling back into approach 2 (e. g. just one frontend for all delivery systems). I also think that we should get a decision about that for 3.1, and port DRX-K to the agreed approach before the release of 3.1, as it will be one less driver that we'll need to concern about migrating. Cheers, Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 14:54 ` Mauro Carvalho Chehab 2011-07-16 15:40 ` Andreas Oberritter @ 2011-07-16 15:40 ` Oliver Endriss 2011-11-03 7:49 ` Steffen Barszus 1 sibling, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-16 15:40 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Antti Palosaari, Andreas Oberritter, Ralph Metzler, linux-media On Saturday 16 July 2011 16:54:50 Mauro Carvalho Chehab wrote: > Em 16-07-2011 11:16, Antti Palosaari escreveu: > > On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: > >> Em 15-07-2011 20:41, Antti Palosaari escreveu: > >>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: > >>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: > >>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: > >>>>>> At the same time I want to add delivery system properties to > >>>>>> support everything in one frontend device. > >>>>>> Adding a parameter to select C or T as default should help in most > >>>>>> cases where the application does not support switching yet. > >>>>> > >>>>> If I understood well, creating a multi-delivery type of frontend for > >>>>> devices like DRX-K makes sense for me. > >>>>> > >>>>> We need to take some care about how to add support for them, to avoid > >>>>> breaking userspace, or to follow kernel deprecating rules, by adding > >>>>> some legacy compatibility glue for a few kernel versions. So, the sooner > >>>>> we add such support, the better, as less drivers will need to support > >>>>> a "fallback" mechanism. > >>>>> > >>>>> The current DVB version 5 API doesn't prevent some userspace application > >>>>> to change the delivery system[1] for a given frontend. This feature is > >>>>> actually used by DVB-T2 and DVB-S2 drivers. This actually improved the > >>>>> DVB API multi-fe support, by avoiding the need of create of a secondary > >>>>> frontend for T2/S2. > >>>>> > >>>>> Userspace applications can detect that feature by using FE_CAN_2G_MODULATION > >>>>> flag, but this mechanism doesn't allow other types of changes like > >>>>> from/to DVB-T/DVB-C or from/to DVB-T/ISDB-T. So, drivers that allow such > >>>>> type of delivery system switch, using the same chip ended by needing to > >>>>> add two frontends. > >>>>> > >>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to fe_caps_t, and > >>>>> add a way to query the type of delivery systems supported by a driver. > >>>>> > >>>>> [1] http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM > >>>> > >>>> I don't think it's necessary to add a new flag. It should be sufficient > >>>> to add a property like "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be > >>>> read-only and return an array of type fe_delivery_system_t. > >>>> > >>>> Querying this new property on present kernels hopefully fails with a > >>>> non-zero return code. in which case FE_GET_INFO should be used to query > >>>> the delivery system. > >>>> > >>>> In future kernels we can provide a default implementation, returning > >>>> exactly one fe_delivery_system_t for unported drivers. Other drivers > >>>> should be able to override this default implementation in their > >>>> get_property callback. > >>> > >>> One thing I want to say is that consider about devices which does have MFE using two different *physical* demods, not integrated to same silicon. > >>> > >>> If you add such FE delsys switch mechanism it needs some more glue to bind two physical FEs to one virtual FE. I see much easier to keep all FEs as own - just register those under the same adapter if FEs are shared. > >> > >> In this case, the driver should just create two frontends, as currently. > >> > >> There's a difference when there are two physical FE's and just one FE: > >> with 2 FE's, the userspace application can just keep both opened at > >> the same time. Some applications (like vdr) assumes that all multi-fe > >> are like that. > > > > Does this mean demod is not sleeping (.init() called)? > > > >> When there's just a single FE, but the driver needs to "fork" it in two > >> due to the API troubles, the driver needs to prevent the usage of both > >> fe's, either at open or at the ioctl level. So, applications like vdr > >> will only use the first frontend. > > > > Lets take example. There is shared MFE having DVB-S, DVB-T and DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S have own. > > > > Currently it will shown as: > > Let me name the approaches: > > Approach 1) > > * adapter0 > > ** frontend0 (DVB-S) > > ** frontend1 (DVB-T) > > ** frontend2 (DVB-C) > > Approach 2) > > Your new "ideal" solution will be: > > * adapter0 > > ** frontend0 (DVB-S/T/C) > > Approach 3) > > What really happens (mixed old and new): Why does this happen? > > * adapter0 > > ** frontend0 (DVB-S) > > ** frontend1 (DVB-T/C) > > What I've said before is that approach 3 is the "ideal" solution. No, sorry. > > It does not look very good to offer this kind of mixed solution, since it is possible to offer only one solution for userspace, new or old, but not mixing. > > Good point. > > There's an additional aspect to handle: if a driver that uses approach 1, a conversion > to either approach 2 or 3 would break existing applications that can't handle with > the new approach. > > There's a 4th posibility: always offering fe0 with MFE capabilities, and creating additional fe's > for old applications that can't cope with the new mode. > For example, on a device that supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown as: > > Approach 4) fe0 is a frontend "superset" > > *adapter0 > *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE superset > *frontend1 (DVB-S/DVB-S2) > *frontend2 (DVB-T/DVB-T2) > *frontend3 (DVB-C) > *frontend4 (ISDB-T) > > fe0 will need some special logic to allow redirecting a FE call to the right fe, if > there are more than one physical frontend bound into the FE API. > > I'm starting to think that (4) is the better approach, as it won't break legacy > applications, and it will provide an easier way for new applications to control > the frontend with just one frontend. Nack. Do not make it more complicated than neccessary! Approach (2) is the way to go. I consider the current way as a clear abuse of the DVB API. It is a bug, not a feature! Originally it was intended to support multiple data paths per adapter. For example, A dual tuner DVB-S card should have been implemented as one adapter: adapterX +--- demux0/frontend0/ca0/dvr0/net0 | +--- demux1/frontend1/ca1/dvr1/net1 (Both tuners can be used concurrently without limitations.) My proposal is: If there is any kind of shared hardware, i.e. the application cannot use both adapters independently, these hardware must be folded into a single frontend. It is not so hard to implement, even for separate chips: The driver just has to "switch" from one set of frontend ops to another. Btw, which applications do really handle this fronten0/1 stuff correctly? VDR definitely does not. Access to the frontend1 fails. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-16 15:40 ` Oliver Endriss @ 2011-11-03 7:49 ` Steffen Barszus 2011-11-03 17:24 ` Lars Hanisch 0 siblings, 1 reply; 45+ messages in thread From: Steffen Barszus @ 2011-11-03 7:49 UTC (permalink / raw) To: linux-media Hi ! >From a users point of view i would like to have some clarification on this discussion. Lets take a (now real world) example. Having /dev/dvb/adapter0/demux0 /dev/dvb/adapter0/dvr0 /dev/dvb/adapter0/frontend0 /dev/dvb/adapter0/frontend1 /dev/dvb/adapter0/net0 /dev/dvb/adapter1/demux0 /dev/dvb/adapter1/dvr0 /dev/dvb/adapter1/frontend0 /dev/dvb/adapter1/frontend1 /dev/dvb/adapter1/net0 for a C/T card, where one fe is C and the other is T , connector is only one per adapter. How is the logic to handle this ? Two big issues i don't properly understand at the moment. 1.) VDR does not know that frontend1 is related to demux0. since no application changes magically on its own, can someone provide some information of what an application is expected to do, to handle this properly. With this information it could be discussed at i.e. vdr mailing list. 2.) How does an application/user/whatever know what can be used ? I mean there is only C connected OR T - how can the application know what fe can be used (assumed point 1. has been fixed). How would the user know, which is which and how one should specify what is connected ? 3.) ca/caio device handling - is this something an application should implement ... and how. Please help me to understand these points as this is something which pops up regular in discussion with our (yavdr.org) users. For 1 and 2 the only proper solution i see would be 1 frontend instead of 2 with a possibility to specifiy the transport in one way or another. (which -> to be discussed) Regards Steffen On Sat, 16 Jul 2011 17:40:53 +0200 Oliver Endriss <o.endriss@gmx.de> wrote: > On Saturday 16 July 2011 16:54:50 Mauro Carvalho Chehab wrote: > > Em 16-07-2011 11:16, Antti Palosaari escreveu: > > > On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: > > >> Em 15-07-2011 20:41, Antti Palosaari escreveu: > > >>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: > > >>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: > > >>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: > > >>>>>> At the same time I want to add delivery system properties to > > >>>>>> support everything in one frontend device. > > >>>>>> Adding a parameter to select C or T as default should help > > >>>>>> in most cases where the application does not support > > >>>>>> switching yet. > > >>>>> > > >>>>> If I understood well, creating a multi-delivery type of > > >>>>> frontend for devices like DRX-K makes sense for me. > > >>>>> > > >>>>> We need to take some care about how to add support for them, > > >>>>> to avoid breaking userspace, or to follow kernel deprecating > > >>>>> rules, by adding some legacy compatibility glue for a few > > >>>>> kernel versions. So, the sooner we add such support, the > > >>>>> better, as less drivers will need to support a "fallback" > > >>>>> mechanism. > > >>>>> > > >>>>> The current DVB version 5 API doesn't prevent some userspace > > >>>>> application to change the delivery system[1] for a given > > >>>>> frontend. This feature is actually used by DVB-T2 and DVB-S2 > > >>>>> drivers. This actually improved the DVB API multi-fe support, > > >>>>> by avoiding the need of create of a secondary frontend for > > >>>>> T2/S2. > > >>>>> > > >>>>> Userspace applications can detect that feature by using > > >>>>> FE_CAN_2G_MODULATION flag, but this mechanism doesn't allow > > >>>>> other types of changes like from/to DVB-T/DVB-C or from/to > > >>>>> DVB-T/ISDB-T. So, drivers that allow such type of delivery > > >>>>> system switch, using the same chip ended by needing to add > > >>>>> two frontends. > > >>>>> > > >>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to > > >>>>> fe_caps_t, and add a way to query the type of delivery > > >>>>> systems supported by a driver. > > >>>>> > > >>>>> [1] > > >>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM > > >>>> > > >>>> I don't think it's necessary to add a new flag. It should be > > >>>> sufficient to add a property like > > >>>> "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be read-only > > >>>> and return an array of type fe_delivery_system_t. > > >>>> > > >>>> Querying this new property on present kernels hopefully fails > > >>>> with a non-zero return code. in which case FE_GET_INFO should > > >>>> be used to query the delivery system. > > >>>> > > >>>> In future kernels we can provide a default implementation, > > >>>> returning exactly one fe_delivery_system_t for unported > > >>>> drivers. Other drivers should be able to override this default > > >>>> implementation in their get_property callback. > > >>> > > >>> One thing I want to say is that consider about devices which > > >>> does have MFE using two different *physical* demods, not > > >>> integrated to same silicon. > > >>> > > >>> If you add such FE delsys switch mechanism it needs some more > > >>> glue to bind two physical FEs to one virtual FE. I see much > > >>> easier to keep all FEs as own - just register those under the > > >>> same adapter if FEs are shared. > > >> > > >> In this case, the driver should just create two frontends, as > > >> currently. > > >> > > >> There's a difference when there are two physical FE's and just > > >> one FE: with 2 FE's, the userspace application can just keep > > >> both opened at the same time. Some applications (like vdr) > > >> assumes that all multi-fe are like that. > > > > > > Does this mean demod is not sleeping (.init() called)? > > > > > >> When there's just a single FE, but the driver needs to "fork" it > > >> in two due to the API troubles, the driver needs to prevent the > > >> usage of both fe's, either at open or at the ioctl level. So, > > >> applications like vdr will only use the first frontend. > > > > > > Lets take example. There is shared MFE having DVB-S, DVB-T and > > > DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S > > > have own. > > > > > > Currently it will shown as: > > > > Let me name the approaches: > > > > Approach 1) > > > * adapter0 > > > ** frontend0 (DVB-S) > > > ** frontend1 (DVB-T) > > > ** frontend2 (DVB-C) > > > > Approach 2) > > > Your new "ideal" solution will be: > > > * adapter0 > > > ** frontend0 (DVB-S/T/C) > > > > Approach 3) > > > What really happens (mixed old and new): > > Why does this happen? > > > > * adapter0 > > > ** frontend0 (DVB-S) > > > ** frontend1 (DVB-T/C) > > > > What I've said before is that approach 3 is the "ideal" solution. > > No, sorry. > > > > It does not look very good to offer this kind of mixed solution, > > > since it is possible to offer only one solution for userspace, > > > new or old, but not mixing. > > > > Good point. > > > > There's an additional aspect to handle: if a driver that uses > > approach 1, a conversion to either approach 2 or 3 would break > > existing applications that can't handle with the new approach. > > > > There's a 4th posibility: always offering fe0 with MFE > > capabilities, and creating additional fe's for old applications > > that can't cope with the new mode. For example, on a device that > > supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown > > as: > > > > Approach 4) fe0 is a frontend "superset" > > > > *adapter0 > > *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE > > superset *frontend1 (DVB-S/DVB-S2) > > *frontend2 (DVB-T/DVB-T2) > > *frontend3 (DVB-C) > > *frontend4 (ISDB-T) > > > > fe0 will need some special logic to allow redirecting a FE call to > > the right fe, if there are more than one physical frontend bound > > into the FE API. > > > > I'm starting to think that (4) is the better approach, as it won't > > break legacy applications, and it will provide an easier way for > > new applications to control the frontend with just one frontend. > > Nack. Do not make it more complicated than neccessary! > Approach (2) is the way to go. > > I consider the current way as a clear abuse of the DVB API. > It is a bug, not a feature! > > Originally it was intended to support multiple data paths per adapter. > For example, A dual tuner DVB-S card should have been implemented as > one adapter: > > adapterX +--- demux0/frontend0/ca0/dvr0/net0 > | > +--- demux1/frontend1/ca1/dvr1/net1 > > (Both tuners can be used concurrently without limitations.) > > My proposal is: > If there is any kind of shared hardware, i.e. the application cannot > use both adapters independently, these hardware must be folded into a > single frontend. > > It is not so hard to implement, even for separate chips: > The driver just has to "switch" from one set of frontend ops to > another. > > Btw, which applications do really handle this fronten0/1 stuff > correctly? VDR definitely does not. Access to the frontend1 fails. > > CU > Oliver > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-11-03 7:49 ` Steffen Barszus @ 2011-11-03 17:24 ` Lars Hanisch 0 siblings, 0 replies; 45+ messages in thread From: Lars Hanisch @ 2011-11-03 17:24 UTC (permalink / raw) To: linux-media Am 03.11.2011 08:49, schrieb Steffen Barszus: > Hi ! > >> From a users point of view i would like to have some clarification on > this discussion. > > Lets take a (now real world) example. > > Having > /dev/dvb/adapter0/demux0 > /dev/dvb/adapter0/dvr0 > /dev/dvb/adapter0/frontend0 > /dev/dvb/adapter0/frontend1 > /dev/dvb/adapter0/net0 > /dev/dvb/adapter1/demux0 > /dev/dvb/adapter1/dvr0 > /dev/dvb/adapter1/frontend0 > /dev/dvb/adapter1/frontend1 > /dev/dvb/adapter1/net0 > > for a C/T card, where one fe is C and the other is T , connector is > only one per adapter. > > How is the logic to handle this ? > > Two big issues i don't properly understand at the moment. > > 1.) VDR does not know that frontend1 is > related to demux0. since no application changes magically on its own, > can someone provide some information of what an application is expected > to do, to handle this properly. With this information it could be > discussed at i.e. vdr mailing list. > > 2.) How does an application/user/whatever know what can be used ? > I mean there is only C connected OR T - how can the application know > what fe can be used (assumed point 1. has been fixed). How would the > user know, which is which and how one should specify what is > connected ? > > 3.) ca/caio device handling - is this something an application should > implement ... and how. > > Please help me to understand these points as this is something which > pops up regular in discussion with our (yavdr.org) users. > > For 1 and 2 the only proper solution i see would be 1 frontend instead > of 2 with a possibility to specifiy the transport in one way or > another. (which -> to be discussed) For me as an application developer it wouldn't make sense if the frontend handles multiple delivery types as outlined in approach 2 and 3 from below. That would mean that *every* application has to ask the user and must provide some configuration possibility. And there has to be a new ioctl to set the delivery type (or is there one I don't know about?). Either I have DVB-C or DVB-T. So I would like to specify the delivery type at module load time with an option. Then there would be one frontend and every (existent) application would "just work". And it's configured at one place. Everyone who changes the connection frequently should learn to reload the module... :-) Regards, Lars. > > Regards > > Steffen > > > > On Sat, 16 Jul 2011 17:40:53 +0200 > Oliver Endriss<o.endriss@gmx.de> wrote: > >> On Saturday 16 July 2011 16:54:50 Mauro Carvalho Chehab wrote: >>> Em 16-07-2011 11:16, Antti Palosaari escreveu: >>>> On 07/16/2011 03:25 PM, Mauro Carvalho Chehab wrote: >>>>> Em 15-07-2011 20:41, Antti Palosaari escreveu: >>>>>> On 07/15/2011 08:01 PM, Andreas Oberritter wrote: >>>>>>> On 15.07.2011 15:25, Mauro Carvalho Chehab wrote: >>>>>>>> Em 15-07-2011 05:26, Ralph Metzler escreveu: >>>>>>>>> At the same time I want to add delivery system properties to >>>>>>>>> support everything in one frontend device. >>>>>>>>> Adding a parameter to select C or T as default should help >>>>>>>>> in most cases where the application does not support >>>>>>>>> switching yet. >>>>>>>> >>>>>>>> If I understood well, creating a multi-delivery type of >>>>>>>> frontend for devices like DRX-K makes sense for me. >>>>>>>> >>>>>>>> We need to take some care about how to add support for them, >>>>>>>> to avoid breaking userspace, or to follow kernel deprecating >>>>>>>> rules, by adding some legacy compatibility glue for a few >>>>>>>> kernel versions. So, the sooner we add such support, the >>>>>>>> better, as less drivers will need to support a "fallback" >>>>>>>> mechanism. >>>>>>>> >>>>>>>> The current DVB version 5 API doesn't prevent some userspace >>>>>>>> application to change the delivery system[1] for a given >>>>>>>> frontend. This feature is actually used by DVB-T2 and DVB-S2 >>>>>>>> drivers. This actually improved the DVB API multi-fe support, >>>>>>>> by avoiding the need of create of a secondary frontend for >>>>>>>> T2/S2. >>>>>>>> >>>>>>>> Userspace applications can detect that feature by using >>>>>>>> FE_CAN_2G_MODULATION flag, but this mechanism doesn't allow >>>>>>>> other types of changes like from/to DVB-T/DVB-C or from/to >>>>>>>> DVB-T/ISDB-T. So, drivers that allow such type of delivery >>>>>>>> system switch, using the same chip ended by needing to add >>>>>>>> two frontends. >>>>>>>> >>>>>>>> Maybe we can add a generic FE_CAN_MULTI_DELIVERY flag to >>>>>>>> fe_caps_t, and add a way to query the type of delivery >>>>>>>> systems supported by a driver. >>>>>>>> >>>>>>>> [1] >>>>>>>> http://linuxtv.org/downloads/v4l-dvb-apis/FE_GET_SET_PROPERTY.html#DTV-DELIVERY-SYSTEM >>>>>>> >>>>>>> I don't think it's necessary to add a new flag. It should be >>>>>>> sufficient to add a property like >>>>>>> "DTV_SUPPORTED_DELIVERY_SYSTEMS", which should be read-only >>>>>>> and return an array of type fe_delivery_system_t. >>>>>>> >>>>>>> Querying this new property on present kernels hopefully fails >>>>>>> with a non-zero return code. in which case FE_GET_INFO should >>>>>>> be used to query the delivery system. >>>>>>> >>>>>>> In future kernels we can provide a default implementation, >>>>>>> returning exactly one fe_delivery_system_t for unported >>>>>>> drivers. Other drivers should be able to override this default >>>>>>> implementation in their get_property callback. >>>>>> >>>>>> One thing I want to say is that consider about devices which >>>>>> does have MFE using two different *physical* demods, not >>>>>> integrated to same silicon. >>>>>> >>>>>> If you add such FE delsys switch mechanism it needs some more >>>>>> glue to bind two physical FEs to one virtual FE. I see much >>>>>> easier to keep all FEs as own - just register those under the >>>>>> same adapter if FEs are shared. >>>>> >>>>> In this case, the driver should just create two frontends, as >>>>> currently. >>>>> >>>>> There's a difference when there are two physical FE's and just >>>>> one FE: with 2 FE's, the userspace application can just keep >>>>> both opened at the same time. Some applications (like vdr) >>>>> assumes that all multi-fe are like that. >>>> >>>> Does this mean demod is not sleeping (.init() called)? >>>> >>>>> When there's just a single FE, but the driver needs to "fork" it >>>>> in two due to the API troubles, the driver needs to prevent the >>>>> usage of both fe's, either at open or at the ioctl level. So, >>>>> applications like vdr will only use the first frontend. >>>> >>>> Lets take example. There is shared MFE having DVB-S, DVB-T and >>>> DVB-C. DVB-T and DVB-C are integrated to one chip whilst DVB-S >>>> have own. >>>> >>>> Currently it will shown as: >>> >>> Let me name the approaches: >>> >>> Approach 1) >>>> * adapter0 >>>> ** frontend0 (DVB-S) >>>> ** frontend1 (DVB-T) >>>> ** frontend2 (DVB-C) >>> >>> Approach 2) >>>> Your new "ideal" solution will be: >>>> * adapter0 >>>> ** frontend0 (DVB-S/T/C) >>> >>> Approach 3) >>>> What really happens (mixed old and new): >> >> Why does this happen? >> >>>> * adapter0 >>>> ** frontend0 (DVB-S) >>>> ** frontend1 (DVB-T/C) >>> >>> What I've said before is that approach 3 is the "ideal" solution. >> >> No, sorry. >> >>>> It does not look very good to offer this kind of mixed solution, >>>> since it is possible to offer only one solution for userspace, >>>> new or old, but not mixing. >>> >>> Good point. >>> >>> There's an additional aspect to handle: if a driver that uses >>> approach 1, a conversion to either approach 2 or 3 would break >>> existing applications that can't handle with the new approach. >>> >>> There's a 4th posibility: always offering fe0 with MFE >>> capabilities, and creating additional fe's for old applications >>> that can't cope with the new mode. For example, on a device that >>> supports DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T, it will be shown >>> as: >>> >>> Approach 4) fe0 is a frontend "superset" >>> >>> *adapter0 >>> *frontend0 (DVB-S/DVB-S2/DVB-T/DVB-T2/DVB-C/ISDB-T) - aka: FE >>> superset *frontend1 (DVB-S/DVB-S2) >>> *frontend2 (DVB-T/DVB-T2) >>> *frontend3 (DVB-C) >>> *frontend4 (ISDB-T) >>> >>> fe0 will need some special logic to allow redirecting a FE call to >>> the right fe, if there are more than one physical frontend bound >>> into the FE API. >>> >>> I'm starting to think that (4) is the better approach, as it won't >>> break legacy applications, and it will provide an easier way for >>> new applications to control the frontend with just one frontend. >> >> Nack. Do not make it more complicated than neccessary! >> Approach (2) is the way to go. >> >> I consider the current way as a clear abuse of the DVB API. >> It is a bug, not a feature! >> >> Originally it was intended to support multiple data paths per adapter. >> For example, A dual tuner DVB-S card should have been implemented as >> one adapter: >> >> adapterX +--- demux0/frontend0/ca0/dvr0/net0 >> | >> +--- demux1/frontend1/ca1/dvr1/net1 >> >> (Both tuners can be used concurrently without limitations.) >> >> My proposal is: >> If there is any kind of shared hardware, i.e. the application cannot >> use both adapters independently, these hardware must be folded into a >> single frontend. >> >> It is not so hard to implement, even for separate chips: >> The driver just has to "switch" from one set of frontend ops to >> another. >> >> Btw, which applications do really handle this fronten0/1 stuff >> correctly? VDR definitely does not. Access to the frontend1 fails. >> >> CU >> Oliver >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-14 23:45 ` Oliver Endriss 2011-07-15 0:47 ` Mauro Carvalho Chehab @ 2011-07-15 4:18 ` Mauro Carvalho Chehab 2011-07-15 5:21 ` Oliver Endriss 1 sibling, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 4:18 UTC (permalink / raw) To: linux-media; +Cc: Oliver Endriss, Ralph Metzler Em 14-07-2011 20:45, Oliver Endriss escreveu: > - DVB-T tuning does not work anymore. The enclosed patch should fix the issue. It were due to a wrong goto error replacements that happened at the changeset that were fixing the error propagation logic. Sorry for that. Please test. Cheers, Mauro. [media] drxk: Fix a bug at some switches that broke DVB-T The error propagation changeset c23bf4402 broke the DVB-T code, as it wrongly replaced some break with goto error. Fix the broken logic. Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c index a0e2ff5..217796d 100644 --- a/drivers/media/dvb/frontends/drxk_hard.c +++ b/drivers/media/dvb/frontends/drxk_hard.c @@ -3451,13 +3451,13 @@ static int DVBTCtrlSetEchoThreshold(struct drxk_state *state, data |= ((echoThres->threshold << OFDM_SC_RA_RAM_ECHO_THRES_2K__B) & (OFDM_SC_RA_RAM_ECHO_THRES_2K__M)); - goto error; + break; case DRX_FFTMODE_8K: data &= ~OFDM_SC_RA_RAM_ECHO_THRES_8K__M; data |= ((echoThres->threshold << OFDM_SC_RA_RAM_ECHO_THRES_8K__B) & (OFDM_SC_RA_RAM_ECHO_THRES_8K__M)); - goto error; + break; default: return -EINVAL; goto error; @@ -3825,10 +3825,10 @@ static int SetDVBT(struct drxk_state *state, u16 IntermediateFreqkHz, /* fall through , try first guess DRX_FFTMODE_8K */ case TRANSMISSION_MODE_8K: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_MODE_8K; - goto error; + break; case TRANSMISSION_MODE_2K: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_MODE_2K; - goto error; + break; } /* guard */ @@ -3839,16 +3839,16 @@ static int SetDVBT(struct drxk_state *state, u16 IntermediateFreqkHz, /* fall through , try first guess DRX_GUARD_1DIV4 */ case GUARD_INTERVAL_1_4: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_4; - goto error; + break; case GUARD_INTERVAL_1_32: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_32; - goto error; + break; case GUARD_INTERVAL_1_16: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_16; - goto error; + break; case GUARD_INTERVAL_1_8: transmissionParams |= OFDM_SC_RA_RAM_OP_PARAM_GUARD_8; - goto error; + break; } /* hierarchy */ ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 4:18 ` Mauro Carvalho Chehab @ 2011-07-15 5:21 ` Oliver Endriss 2011-07-15 12:40 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 45+ messages in thread From: Oliver Endriss @ 2011-07-15 5:21 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Ralph Metzler On Friday 15 July 2011 06:18:27 Mauro Carvalho Chehab wrote: > Em 14-07-2011 20:45, Oliver Endriss escreveu: > > - DVB-T tuning does not work anymore. > > The enclosed patch should fix the issue. It were due to a wrong goto error > replacements that happened at the changeset that were fixing the error > propagation logic. Sorry for that. > > Please test. Done. DVB-T works again. Thanks. @all Could someone please test DVB-C? > [media] drxk: Fix a bug at some switches that broke DVB-T > > The error propagation changeset c23bf4402 broke the DVB-T > code, as it wrongly replaced some break with goto error. > Fix the broken logic. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c > index a0e2ff5..217796d 100644 > --- a/drivers/media/dvb/frontends/drxk_hard.c > +++ b/drivers/media/dvb/frontends/drxk_hard.c > @@ -3451,13 +3451,13 @@ static int DVBTCtrlSetEchoThreshold(struct drxk_state *state, > data |= ((echoThres->threshold << > OFDM_SC_RA_RAM_ECHO_THRES_2K__B) > & (OFDM_SC_RA_RAM_ECHO_THRES_2K__M)); > - goto error; > + break; > case DRX_FFTMODE_8K: > data &= ~OFDM_SC_RA_RAM_ECHO_THRES_8K__M; > data |= ((echoThres->threshold << > OFDM_SC_RA_RAM_ECHO_THRES_8K__B) > & (OFDM_SC_RA_RAM_ECHO_THRES_8K__M)); > - goto error; > + break; > default: > return -EINVAL; > goto error; ^^^^^^^^^^^ Hm, this 'goto' should be removed. CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 5:21 ` Oliver Endriss @ 2011-07-15 12:40 ` Mauro Carvalho Chehab 2011-07-17 11:44 ` Oliver Endriss 0 siblings, 1 reply; 45+ messages in thread From: Mauro Carvalho Chehab @ 2011-07-15 12:40 UTC (permalink / raw) To: linux-media; +Cc: Oliver Endriss, Ralph Metzler Em 15-07-2011 02:21, Oliver Endriss escreveu: > On Friday 15 July 2011 06:18:27 Mauro Carvalho Chehab wrote: >> Em 14-07-2011 20:45, Oliver Endriss escreveu: >>> - DVB-T tuning does not work anymore. >> >> The enclosed patch should fix the issue. It were due to a wrong goto error >> replacements that happened at the changeset that were fixing the error >> propagation logic. Sorry for that. >> >> Please test. > > Done. DVB-T works again. Thanks. Thanks for reporting the issue and testing the fix! > @all > Could someone please test DVB-C? > >> [media] drxk: Fix a bug at some switches that broke DVB-T >> >> The error propagation changeset c23bf4402 broke the DVB-T >> code, as it wrongly replaced some break with goto error. >> Fix the broken logic. >> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> >> >> diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c >> index a0e2ff5..217796d 100644 >> --- a/drivers/media/dvb/frontends/drxk_hard.c >> +++ b/drivers/media/dvb/frontends/drxk_hard.c >> @@ -3451,13 +3451,13 @@ static int DVBTCtrlSetEchoThreshold(struct drxk_state *state, >> data |= ((echoThres->threshold << >> OFDM_SC_RA_RAM_ECHO_THRES_2K__B) >> & (OFDM_SC_RA_RAM_ECHO_THRES_2K__M)); >> - goto error; >> + break; >> case DRX_FFTMODE_8K: >> data &= ~OFDM_SC_RA_RAM_ECHO_THRES_8K__M; >> data |= ((echoThres->threshold << >> OFDM_SC_RA_RAM_ECHO_THRES_8K__B) >> & (OFDM_SC_RA_RAM_ECHO_THRES_8K__M)); >> - goto error; >> + break; >> default: >> return -EINVAL; >> goto error; > ^^^^^^^^^^^ > Hm, this 'goto' should be removed. True. I've just added a small trivial patch removing this goto, and a break after a return. Thanks! Mauro ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) 2011-07-15 12:40 ` Mauro Carvalho Chehab @ 2011-07-17 11:44 ` Oliver Endriss 0 siblings, 0 replies; 45+ messages in thread From: Oliver Endriss @ 2011-07-17 11:44 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media Mauro, could you please apply the two patches. I get complaints that drx is broken in media_build... CU Oliver -- ---------------------------------------------------------------- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ ---------------------------------------------------------------- ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2011-11-03 17:30 UTC | newest] Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-03 21:21 [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Oliver Endriss 2011-07-03 21:23 ` PATCH 1/5] ddbridge: Initial check-in Oliver Endriss 2011-07-03 21:24 ` [PATCH 2/5] ddbridge: Codingstyle fixes Oliver Endriss 2011-07-03 21:25 ` [PATCH 3/5] ddbridge: Allow compiling of the driver Oliver Endriss 2011-07-03 21:26 ` [PATCH 4/5] cxd2099: Fix compilation of ngene/ddbridge for DVB_CXD2099=n Oliver Endriss 2011-07-04 10:14 ` Bjørn Mork 2011-07-03 21:27 ` [PATCH 5/5] cxd2099: Update Kconfig description (ddbridge support) Oliver Endriss 2011-07-04 0:06 ` Walter Van Eetvelt 2011-07-03 22:27 ` [PATCH 0/5] Driver support for cards based on Digital Devices bridge (ddbridge) Mauro Carvalho Chehab 2011-07-03 23:24 ` Oliver Endriss 2011-07-04 0:17 ` Mauro Carvalho Chehab 2011-07-14 23:45 ` Oliver Endriss 2011-07-15 0:47 ` Mauro Carvalho Chehab 2011-07-15 2:11 ` Oliver Endriss 2011-07-15 4:01 ` Mauro Carvalho Chehab 2011-07-15 3:56 ` Mauro Carvalho Chehab 2011-07-15 5:17 ` Oliver Endriss 2011-07-15 8:26 ` Ralph Metzler 2011-07-15 13:25 ` Mauro Carvalho Chehab 2011-07-15 17:01 ` Andreas Oberritter 2011-07-15 17:34 ` Mauro Carvalho Chehab 2011-07-15 23:41 ` Antti Palosaari 2011-07-16 12:25 ` Mauro Carvalho Chehab 2011-07-16 14:16 ` Antti Palosaari 2011-07-16 14:54 ` Mauro Carvalho Chehab 2011-07-16 15:40 ` Andreas Oberritter 2011-07-16 15:44 ` Antti Palosaari 2011-07-16 15:53 ` Andreas Oberritter 2011-07-16 15:59 ` Antti Palosaari 2011-07-16 16:37 ` Rémi Denis-Courmont 2011-07-17 2:51 ` Andreas Oberritter 2011-07-17 7:51 ` Rémi Denis-Courmont 2011-07-17 0:56 ` Mauro Carvalho Chehab 2011-07-17 3:02 ` Andreas Oberritter 2011-07-17 3:59 ` Mauro Carvalho Chehab 2011-07-17 7:39 ` Rémi Denis-Courmont 2011-07-17 8:01 ` Mauro Carvalho Chehab 2011-07-17 1:07 ` Mauro Carvalho Chehab 2011-07-16 15:40 ` Oliver Endriss 2011-11-03 7:49 ` Steffen Barszus 2011-11-03 17:24 ` Lars Hanisch 2011-07-15 4:18 ` Mauro Carvalho Chehab 2011-07-15 5:21 ` Oliver Endriss 2011-07-15 12:40 ` Mauro Carvalho Chehab 2011-07-17 11:44 ` Oliver Endriss
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).