* [PATCH 1/5] [media] dvb-frontends/stv0910: release lock on gate_ctrl() failure
2017-08-23 16:09 [PATCH 0/5] last-minute misc ddbridge related changed Daniel Scheller
@ 2017-08-23 16:09 ` Daniel Scheller
2017-08-23 16:09 ` [PATCH 2/5] [media] ddbridge: fix teardown/deregistration order in ddb_input_detach() Daniel Scheller
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Scheller @ 2017-08-23 16:09 UTC (permalink / raw)
To: linux-media, mchehab, mchehab; +Cc: jasmin, Julia Lawall
From: Daniel Scheller <d.scheller@gmx.net>
Whenever write_reg() fails to open/close the demod's I2C gate, release the
lock to avoid deadlocking situations. If I2c gate open failed, there's no
need to hold a lock, and if close fails, the mutex_unlock() at the end of
the function is never reached, leaving the mutex_lock in locked state,
which in turn will cause potential for deadlocks. Thus, release the lock
on failure.
While we're touching gate_ctrl(), add some explanation about the need for
locking and the shared I2C bus/gate.
Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
drivers/media/dvb-frontends/stv0910.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c
index d1ae9553f74c..0d4a6a115159 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1221,17 +1221,32 @@ static int gate_ctrl(struct dvb_frontend *fe, int enable)
struct stv *state = fe->demodulator_priv;
u8 i2crpt = state->i2crpt & ~0x86;
- if (enable)
- mutex_lock(&state->base->i2c_lock);
+ /*
+ * mutex_lock note: Concurrent I2C gate bus accesses must be
+ * prevented (STV0910 = dual demod on a single IC with a single I2C
+ * gate/bus, and two tuners attached), similar to most (if not all)
+ * other I2C host interfaces/busses.
+ *
+ * enable=1 (open I2C gate) will grab the lock
+ * enable=0 (close I2C gate) releases the lock
+ */
- if (enable)
+ if (enable) {
+ mutex_lock(&state->base->i2c_lock);
i2crpt |= 0x80;
- else
+ } else {
i2crpt |= 0x02;
+ }
if (write_reg(state, state->nr ? RSTV0910_P2_I2CRPT :
- RSTV0910_P1_I2CRPT, i2crpt) < 0)
+ RSTV0910_P1_I2CRPT, i2crpt) < 0) {
+ /* don't hold the I2C bus lock on failure */
+ mutex_unlock(&state->base->i2c_lock);
+ dev_err(&state->base->i2c->dev,
+ "%s() write_reg failure (enable=%d)\n",
+ __func__, enable);
return -EIO;
+ }
state->i2crpt = i2crpt;
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] [media] ddbridge: fix teardown/deregistration order in ddb_input_detach()
2017-08-23 16:09 [PATCH 0/5] last-minute misc ddbridge related changed Daniel Scheller
2017-08-23 16:09 ` [PATCH 1/5] [media] dvb-frontends/stv0910: release lock on gate_ctrl() failure Daniel Scheller
@ 2017-08-23 16:09 ` Daniel Scheller
2017-08-23 16:10 ` [PATCH 3/5] [media] ddbridge: fix sparse warnings Daniel Scheller
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Scheller @ 2017-08-23 16:09 UTC (permalink / raw)
To: linux-media, mchehab, mchehab; +Cc: jasmin, Matthias Schwarzott
From: Daniel Scheller <d.scheller@gmx.net>
Brought to attention by Matthias Schwarzott <zzam@gentoo.org> by fixing
possible use-after-free faults in some demod drivers:
In ddb_input_detach(), the i2c_client is unregistered and removed before
dvb frontends are unregistered and detached. While no use-after-free issue
was observed so far, there is another issue with this:
dvb->attached keeps track of the state of the input/output registration,
and the i2c_client unregistration takes place only if everything was
successful (dvb->attached == 0x31). If for some reason an error occurred
during the frontend setup, that value stays at 0x20. In the following
error handling and cleanup, ddb_input_detach() will skip down to that
state, leaving the i2c_client registered, causing refcount issues.
Fix this by moving the i2c_client deregistration down to case 0x20.
Cc: Matthias Schwarzott <zzam@gentoo.org>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
drivers/media/pci/ddbridge/ddbridge-core.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 2464bde1c432..281b6739b0c1 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1255,11 +1255,6 @@ static void dvb_input_detach(struct ddb_input *input)
switch (dvb->attached) {
case 0x31:
- client = dvb->i2c_client[0];
- if (client) {
- module_put(client->dev.driver->owner);
- i2c_unregister_device(client);
- }
if (dvb->fe2)
dvb_unregister_frontend(dvb->fe2);
if (dvb->fe)
@@ -1273,6 +1268,12 @@ static void dvb_input_detach(struct ddb_input *input)
dvb->fe = dvb->fe2 = NULL;
/* fallthrough */
case 0x20:
+ client = dvb->i2c_client[0];
+ if (client) {
+ module_put(client->dev.driver->owner);
+ i2c_unregister_device(client);
+ }
+
dvb_net_release(&dvb->dvbnet);
/* fallthrough */
case 0x12:
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] [media] ddbridge: fix sparse warnings
2017-08-23 16:09 [PATCH 0/5] last-minute misc ddbridge related changed Daniel Scheller
2017-08-23 16:09 ` [PATCH 1/5] [media] dvb-frontends/stv0910: release lock on gate_ctrl() failure Daniel Scheller
2017-08-23 16:09 ` [PATCH 2/5] [media] ddbridge: fix teardown/deregistration order in ddb_input_detach() Daniel Scheller
@ 2017-08-23 16:10 ` Daniel Scheller
2017-08-23 16:10 ` [PATCH 4/5] [media] staging/cxd2099: Add module parameter for buffer mode Daniel Scheller
2017-08-23 16:10 ` [PATCH 5/5] [media] dvb-frontends/stv0910: change minsymrate to 100Ksyms/s Daniel Scheller
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Scheller @ 2017-08-23 16:10 UTC (permalink / raw)
To: linux-media, mchehab, mchehab; +Cc: jasmin, Ralph Metzler
From: Daniel Scheller <d.scheller@gmx.net>
Fix several
drivers/media/pci/ddbridge/ddbridge-core.c: warning: symbol ... was not declared. Should it be static?
drivers/media/pci/ddbridge/ddbridge-core.c: warning: Using plain integer as NULL pointer
drivers/media/pci/ddbridge/ddbridge-io.h: warning: cast removes address space of expression
drivers/media/pci/ddbridge/ddbridge-io.h: warning: incorrect type in argument 1 (different address spaces)
at multiple places.
Cc: Ralph Metzler <rjkm@metzlerbros.de>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
drivers/media/pci/ddbridge/ddbridge-core.c | 26 +++++++++++++-------------
drivers/media/pci/ddbridge/ddbridge-io.h | 12 ++++++------
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 281b6739b0c1..f4bd4908acdd 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -69,7 +69,7 @@ MODULE_PARM_DESC(adapter_alloc,
/****************************************************************************/
-DEFINE_MUTEX(redirect_lock);
+static DEFINE_MUTEX(redirect_lock);
struct workqueue_struct *ddb_wq;
@@ -135,8 +135,8 @@ static void ddb_redirect_dma(struct ddb *dev,
static int ddb_unredirect(struct ddb_port *port)
{
- struct ddb_input *oredi, *iredi = 0;
- struct ddb_output *iredo = 0;
+ struct ddb_input *oredi, *iredi = NULL;
+ struct ddb_output *iredo = NULL;
/* dev_info(port->dev->dev,
* "unredirect %d.%d\n", port->dev->nr, port->nr);
@@ -160,14 +160,14 @@ static int ddb_unredirect(struct ddb_port *port)
ddb_redirect_dma(oredi->port->dev,
oredi->dma, iredo->dma);
}
- port->input[0]->redo = 0;
+ port->input[0]->redo = NULL;
ddb_set_dma_table(port->input[0]);
}
oredi->redi = iredi;
- port->input[0]->redi = 0;
+ port->input[0]->redi = NULL;
}
- oredi->redo = 0;
- port->output->redi = 0;
+ oredi->redo = NULL;
+ port->output->redi = NULL;
ddb_set_dma_table(oredi);
done:
@@ -209,7 +209,7 @@ static int ddb_redirect(u32 i, u32 p)
if (input2) {
if (input->redi) {
input2->redi = input->redi;
- input->redi = 0;
+ input->redi = NULL;
} else
input2->redi = input;
}
@@ -811,11 +811,11 @@ static const struct file_operations ci_fops = {
.open = ts_open,
.release = ts_release,
.poll = ts_poll,
- .mmap = 0,
+ .mmap = NULL,
};
static struct dvb_device dvbdev_ci = {
- .priv = 0,
+ .priv = NULL,
.readers = 1,
.writers = 1,
.users = 2,
@@ -2053,7 +2053,7 @@ static struct dvb_ca_en50221 en_templ = {
static void ci_attach(struct ddb_port *port)
{
- struct ddb_ci *ci = 0;
+ struct ddb_ci *ci = NULL;
ci = kzalloc(sizeof(*ci), GFP_KERNEL);
if (!ci)
@@ -2206,7 +2206,7 @@ static void ci_xo2_attach(struct ddb_port *port)
/****************************************************************************/
/****************************************************************************/
-struct cxd2099_cfg cxd_cfg = {
+static struct cxd2099_cfg cxd_cfg = {
.bitrate = 72000,
.adr = 0x40,
.polarity = 1,
@@ -3445,7 +3445,7 @@ int ddb_device_create(struct ddb *dev)
if (res) {
ddb_device_attrs_del(dev);
device_destroy(&ddb_class, MKDEV(ddb_major, dev->nr));
- ddbs[dev->nr] = 0;
+ ddbs[dev->nr] = NULL;
dev->ddb_dev = ERR_PTR(-ENODEV);
} else
ddb_num++;
diff --git a/drivers/media/pci/ddbridge/ddbridge-io.h b/drivers/media/pci/ddbridge/ddbridge-io.h
index ce92e9484075..a4c6bbe09168 100644
--- a/drivers/media/pci/ddbridge/ddbridge-io.h
+++ b/drivers/media/pci/ddbridge/ddbridge-io.h
@@ -27,32 +27,32 @@
static inline u32 ddblreadl(struct ddb_link *link, u32 adr)
{
- return readl((char *) (link->dev->regs + (adr)));
+ return readl(link->dev->regs + adr);
}
static inline void ddblwritel(struct ddb_link *link, u32 val, u32 adr)
{
- writel(val, (char *) (link->dev->regs + (adr)));
+ writel(val, link->dev->regs + adr);
}
static inline u32 ddbreadl(struct ddb *dev, u32 adr)
{
- return readl((char *) (dev->regs + (adr)));
+ return readl(dev->regs + adr);
}
static inline void ddbwritel(struct ddb *dev, u32 val, u32 adr)
{
- writel(val, (char *) (dev->regs + (adr)));
+ writel(val, dev->regs + adr);
}
static inline void ddbcpyto(struct ddb *dev, u32 adr, void *src, long count)
{
- return memcpy_toio((char *) (dev->regs + adr), src, count);
+ return memcpy_toio(dev->regs + adr, src, count);
}
static inline void ddbcpyfrom(struct ddb *dev, void *dst, u32 adr, long count)
{
- return memcpy_fromio(dst, (char *) (dev->regs + adr), count);
+ return memcpy_fromio(dst, dev->regs + adr, count);
}
static inline u32 safe_ddbreadl(struct ddb *dev, u32 adr)
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] [media] staging/cxd2099: Add module parameter for buffer mode
2017-08-23 16:09 [PATCH 0/5] last-minute misc ddbridge related changed Daniel Scheller
` (2 preceding siblings ...)
2017-08-23 16:10 ` [PATCH 3/5] [media] ddbridge: fix sparse warnings Daniel Scheller
@ 2017-08-23 16:10 ` Daniel Scheller
2017-08-23 16:10 ` [PATCH 5/5] [media] dvb-frontends/stv0910: change minsymrate to 100Ksyms/s Daniel Scheller
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Scheller @ 2017-08-23 16:10 UTC (permalink / raw)
To: linux-media, mchehab, mchehab; +Cc: jasmin
From: Daniel Scheller <d.scheller@gmx.net>
The buffer mode of the cxd2099 driver requires more work regarding error
handling and thus can cause issues in some cases, so disable it by default
and make that mode of operation controllable by users via a module
parameter (ie. 'modprobe cxd2099 buffermode=1' enables current behaviour).
The upstream codebase also has the buffer mode disabled by default, so
we should match this (but users still can test things out using the
modparm).
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
Tested-by: Jasmin Jessich <jasmin@anw.at>
Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
drivers/staging/media/cxd2099/cxd2099.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/cxd2099/cxd2099.c b/drivers/staging/media/cxd2099/cxd2099.c
index f28916ea69f1..3e30f4864e2b 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -33,8 +33,9 @@
#include "cxd2099.h"
-/* comment this line to deactivate the cxd2099ar buffer mode */
-#define BUFFER_MODE 1
+static int buffermode;
+module_param(buffermode, int, 0444);
+MODULE_PARM_DESC(buffermode, "Enable use of the CXD2099AR buffer mode (default: disabled)");
static int read_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int ecount);
@@ -221,7 +222,6 @@ static int write_reg(struct cxd *ci, u8 reg, u8 val)
return write_regm(ci, reg, val, 0xff);
}
-#ifdef BUFFER_MODE
static int write_block(struct cxd *ci, u8 adr, u8 *data, u16 n)
{
int status = 0;
@@ -248,7 +248,6 @@ static int write_block(struct cxd *ci, u8 adr, u8 *data, u16 n)
}
return status;
}
-#endif
static void set_mode(struct cxd *ci, int mode)
{
@@ -642,8 +641,6 @@ static int read_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int ecount)
return len;
}
-#ifdef BUFFER_MODE
-
static int write_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int ecount)
{
struct cxd *ci = ca->data;
@@ -658,7 +655,6 @@ static int write_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int ecount)
mutex_unlock(&ci->lock);
return ecount;
}
-#endif
static struct dvb_ca_en50221 en_templ = {
.read_attribute_mem = read_attribute_mem,
@@ -669,11 +665,8 @@ static struct dvb_ca_en50221 en_templ = {
.slot_shutdown = slot_shutdown,
.slot_ts_enable = slot_ts_enable,
.poll_slot_status = poll_slot_status,
-#ifdef BUFFER_MODE
.read_data = read_data,
.write_data = write_data,
-#endif
-
};
struct dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg,
@@ -703,6 +696,14 @@ struct dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg,
ci->en.data = ci;
init(ci);
dev_info(&i2c->dev, "Attached CXD2099AR at %02x\n", ci->cfg.adr);
+
+ if (!buffermode) {
+ ci->en.read_data = NULL;
+ ci->en.write_data = NULL;
+ } else {
+ dev_info(&i2c->dev, "Using CXD2099AR buffer mode");
+ }
+
return &ci->en;
}
EXPORT_SYMBOL(cxd2099_attach);
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] [media] dvb-frontends/stv0910: change minsymrate to 100Ksyms/s
2017-08-23 16:09 [PATCH 0/5] last-minute misc ddbridge related changed Daniel Scheller
` (3 preceding siblings ...)
2017-08-23 16:10 ` [PATCH 4/5] [media] staging/cxd2099: Add module parameter for buffer mode Daniel Scheller
@ 2017-08-23 16:10 ` Daniel Scheller
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Scheller @ 2017-08-23 16:10 UTC (permalink / raw)
To: linux-media, mchehab, mchehab; +Cc: jasmin, Ralph Metzler, Richard Scobie
From: Daniel Scheller <d.scheller@gmx.net>
The demodulator supports symbol rates as low as 100Ksyms/s - the demod
setup in start() already handles such low symbol rates and reviewers
of stv0910 equipped cards even found and tested transponders with
SRs in that range. So, announce this in the fe_ops.
Cc: Ralph Metzler <rjkm@metzlerbros.de>
Cc: Richard Scobie <r.scobie@clear.net.nz>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
drivers/media/dvb-frontends/stv0910.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/stv0910.c b/drivers/media/dvb-frontends/stv0910.c
index 0d4a6a115159..8bf855c301f5 100644
--- a/drivers/media/dvb-frontends/stv0910.c
+++ b/drivers/media/dvb-frontends/stv0910.c
@@ -1698,7 +1698,7 @@ static const struct dvb_frontend_ops stv0910_ops = {
.frequency_max = 2150000,
.frequency_stepsize = 0,
.frequency_tolerance = 0,
- .symbol_rate_min = 1000000,
+ .symbol_rate_min = 100000,
.symbol_rate_max = 70000000,
.caps = FE_CAN_INVERSION_AUTO |
FE_CAN_FEC_AUTO |
--
2.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread