* [PATCH 2/2] [media] dib9000: implement error handling for DibAcquireLock
@ 2012-03-06 21:08 Alexey Khoroshilov
2012-03-19 17:35 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 3+ messages in thread
From: Alexey Khoroshilov @ 2012-03-06 21:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Patrick Boettcher
Cc: Alexey Khoroshilov, Olivier Grenie, linux-media, linux-kernel,
ldv-project
DibAcquireLock() is implemented as mutex_lock_interruptible()
but the driver does not handle unsuccessful locking.
As a result it may lead to unlock of an unheld mutex.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/media/dvb/frontends/dib9000.c | 115 ++++++++++++++++++++++++++-------
1 files changed, 91 insertions(+), 24 deletions(-)
diff --git a/drivers/media/dvb/frontends/dib9000.c b/drivers/media/dvb/frontends/dib9000.c
index d96b6a1..80848b4 100644
--- a/drivers/media/dvb/frontends/dib9000.c
+++ b/drivers/media/dvb/frontends/dib9000.c
@@ -33,7 +33,7 @@ struct i2c_device {
/* lock */
#define DIB_LOCK struct mutex
-#define DibAcquireLock(lock) do { if (mutex_lock_interruptible(lock) < 0) dprintk("could not get the lock"); } while (0)
+#define DibAcquireLock(lock) mutex_lock_interruptible(lock)
#define DibReleaseLock(lock) mutex_unlock(lock)
#define DibInitLock(lock) mutex_init(lock)
#define DibFreeLock(lock)
@@ -446,7 +446,10 @@ static int dib9000_risc_mem_read(struct dib9000_state *state, u8 cmd, u8 * b, u1
if (!state->platform.risc.fw_is_running)
return -EIO;
- DibAcquireLock(&state->platform.risc.mem_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
dib9000_risc_mem_setup(state, cmd | 0x80);
dib9000_risc_mem_read_chunks(state, b, len);
DibReleaseLock(&state->platform.risc.mem_lock);
@@ -459,7 +462,10 @@ static int dib9000_risc_mem_write(struct dib9000_state *state, u8 cmd, const u8
if (!state->platform.risc.fw_is_running)
return -EIO;
- DibAcquireLock(&state->platform.risc.mem_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
dib9000_risc_mem_setup(state, cmd);
dib9000_risc_mem_write_chunks(state, b, m->size);
DibReleaseLock(&state->platform.risc.mem_lock);
@@ -531,7 +537,10 @@ static int dib9000_mbx_send_attr(struct dib9000_state *state, u8 id, u16 * data,
if (!state->platform.risc.fw_is_running)
return -EINVAL;
- DibAcquireLock(&state->platform.risc.mbx_if_lock);
+ if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
tmp = MAX_MAILBOX_TRY;
do {
size = dib9000_read_word_attr(state, 1043, attr) & 0xff;
@@ -593,7 +602,10 @@ static u8 dib9000_mbx_read(struct dib9000_state *state, u16 * data, u8 risc_id,
if (!state->platform.risc.fw_is_running)
return 0;
- DibAcquireLock(&state->platform.risc.mbx_if_lock);
+ if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
+ dprintk("could not get the lock");
+ return 0;
+ }
if (risc_id == 1)
mc_base = 16;
else
@@ -701,7 +713,10 @@ static int dib9000_mbx_process(struct dib9000_state *state, u16 attr)
if (!state->platform.risc.fw_is_running)
return -1;
- DibAcquireLock(&state->platform.risc.mbx_lock);
+ if (DibAcquireLock(&state->platform.risc.mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ return -1;
+ }
if (dib9000_mbx_count(state, 1, attr)) /* 1=RiscB */
ret = dib9000_mbx_fetch_to_cache(state, attr);
@@ -1178,7 +1193,10 @@ static int dib9000_fw_get_channel(struct dvb_frontend *fe)
struct dibDVBTChannel *ch;
int ret = 0;
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
ret = -EIO;
goto error;
@@ -1660,7 +1678,10 @@ static int dib9000_fw_component_bus_xfer(struct i2c_adapter *i2c_adap, struct i2
p[12] = 0;
}
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ return 0;
+ }
dib9000_risc_mem_write(state, FE_MM_W_COMPONENT_ACCESS, p);
@@ -1768,7 +1789,10 @@ int dib9000_fw_pid_filter_ctrl(struct dvb_frontend *fe, u8 onoff)
return 0;
}
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
val = dib9000_read_word(state, 294 + 1) & 0xffef;
val |= (onoff & 0x1) << 4;
@@ -1800,7 +1824,10 @@ int dib9000_fw_pid_filter(struct dvb_frontend *fe, u8 id, u16 pid, u8 onoff)
return 0;
}
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
dprintk("Index %x, PID %d, OnOff %d", id, pid, onoff);
ret = dib9000_write_word(state, 300 + 1 + id,
onoff ? (1 << 13) | pid : 0);
@@ -1848,7 +1875,10 @@ static int dib9000_sleep(struct dvb_frontend *fe)
u8 index_frontend;
int ret = 0;
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
ret = state->fe[index_frontend]->ops.sleep(state->fe[index_frontend]);
if (ret < 0)
@@ -1874,8 +1904,12 @@ static int dib9000_get_frontend(struct dvb_frontend *fe)
fe_status_t stat;
int ret = 0;
- if (state->get_frontend_internal == 0)
- DibAcquireLock(&state->demod_lock);
+ if (state->get_frontend_internal == 0) {
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
+ }
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
state->fe[index_frontend]->ops.read_status(state->fe[index_frontend], &stat);
@@ -1978,7 +2012,10 @@ static int dib9000_set_frontend(struct dvb_frontend *fe)
}
state->pid_ctrl_index = -1; /* postpone the pid filtering cmd */
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return 0;
+ }
fe->dtv_property_cache.delivery_system = SYS_DVBT;
@@ -2138,7 +2175,10 @@ static int dib9000_read_status(struct dvb_frontend *fe, fe_status_t * stat)
u8 index_frontend;
u16 lock = 0, lock_slave = 0;
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++)
lock_slave |= dib9000_read_lock(state->fe[index_frontend]);
@@ -2168,8 +2208,15 @@ static int dib9000_read_ber(struct dvb_frontend *fe, u32 * ber)
u16 *c;
int ret = 0;
- DibAcquireLock(&state->demod_lock);
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ ret = -EINTR;
+ goto error;
+ }
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
DibReleaseLock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
@@ -2196,7 +2243,10 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
u16 val;
int ret = 0;
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
*strength = 0;
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
state->fe[index_frontend]->ops.read_signal_strength(state->fe[index_frontend], &val);
@@ -2206,7 +2256,11 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
*strength += val;
}
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ ret = -EINTR;
+ goto error;
+ }
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
DibReleaseLock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
@@ -2233,10 +2287,13 @@ static u32 dib9000_get_snr(struct dvb_frontend *fe)
u32 n, s, exp;
u16 val;
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ return 0;
+ }
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
DibReleaseLock(&state->platform.risc.mem_mbx_lock);
- return -EIO;
+ return 0;
}
dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR, (u8 *) c, 16 * 2);
DibReleaseLock(&state->platform.risc.mem_mbx_lock);
@@ -2269,7 +2326,10 @@ static int dib9000_read_snr(struct dvb_frontend *fe, u16 * snr)
u8 index_frontend;
u32 snr_master;
- DibAcquireLock(&state->demod_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
snr_master = dib9000_get_snr(fe);
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++)
snr_master += dib9000_get_snr(state->fe[index_frontend]);
@@ -2291,8 +2351,15 @@ static int dib9000_read_unc_blocks(struct dvb_frontend *fe, u32 * unc)
u16 *c = (u16 *)state->i2c_read_buffer;
int ret = 0;
- DibAcquireLock(&state->demod_lock);
- DibAcquireLock(&state->platform.risc.mem_mbx_lock);
+ if (DibAcquireLock(&state->demod_lock) < 0) {
+ dprintk("could not get the lock");
+ return -EINTR;
+ }
+ if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ dprintk("could not get the lock");
+ ret = -EINTR;
+ goto error;
+ }
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
DibReleaseLock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] [media] dib9000: implement error handling for DibAcquireLock
2012-03-06 21:08 [PATCH 2/2] [media] dib9000: implement error handling for DibAcquireLock Alexey Khoroshilov
@ 2012-03-19 17:35 ` Mauro Carvalho Chehab
2012-03-19 20:48 ` [PATCH] [media] dib9000: get rid of Dib*Lock macros Alexey Khoroshilov
0 siblings, 1 reply; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-19 17:35 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Mauro Carvalho Chehab, Patrick Boettcher, Olivier Grenie,
linux-media, linux-kernel, ldv-project
Em 06-03-2012 18:08, Alexey Khoroshilov escreveu:
> DibAcquireLock() is implemented as mutex_lock_interruptible()
> but the driver does not handle unsuccessful locking.
> As a result it may lead to unlock of an unheld mutex.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
> drivers/media/dvb/frontends/dib9000.c | 115 ++++++++++++++++++++++++++-------
> 1 files changed, 91 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/dvb/frontends/dib9000.c b/drivers/media/dvb/frontends/dib9000.c
> index d96b6a1..80848b4 100644
> --- a/drivers/media/dvb/frontends/dib9000.c
> +++ b/drivers/media/dvb/frontends/dib9000.c
> @@ -33,7 +33,7 @@ struct i2c_device {
>
> /* lock */
> #define DIB_LOCK struct mutex
> -#define DibAcquireLock(lock) do { if (mutex_lock_interruptible(lock) < 0) dprintk("could not get the lock"); } while (0)
> +#define DibAcquireLock(lock) mutex_lock_interruptible(lock)
> #define DibReleaseLock(lock) mutex_unlock(lock)
> #define DibInitLock(lock) mutex_init(lock)
> #define DibFreeLock(lock)
It will likely be better to remove all those macros, as they just make the
driver code harder to review.
I'll apply this patch, as it fixes a bug, but it would be great if you
could send us a patch to get rid of those macros.
Thanks,
Mauro
> @@ -446,7 +446,10 @@ static int dib9000_risc_mem_read(struct dib9000_state *state, u8 cmd, u8 * b, u1
> if (!state->platform.risc.fw_is_running)
> return -EIO;
>
> - DibAcquireLock(&state->platform.risc.mem_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> dib9000_risc_mem_setup(state, cmd | 0x80);
> dib9000_risc_mem_read_chunks(state, b, len);
> DibReleaseLock(&state->platform.risc.mem_lock);
> @@ -459,7 +462,10 @@ static int dib9000_risc_mem_write(struct dib9000_state *state, u8 cmd, const u8
> if (!state->platform.risc.fw_is_running)
> return -EIO;
>
> - DibAcquireLock(&state->platform.risc.mem_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> dib9000_risc_mem_setup(state, cmd);
> dib9000_risc_mem_write_chunks(state, b, m->size);
> DibReleaseLock(&state->platform.risc.mem_lock);
> @@ -531,7 +537,10 @@ static int dib9000_mbx_send_attr(struct dib9000_state *state, u8 id, u16 * data,
> if (!state->platform.risc.fw_is_running)
> return -EINVAL;
>
> - DibAcquireLock(&state->platform.risc.mbx_if_lock);
> + if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> tmp = MAX_MAILBOX_TRY;
> do {
> size = dib9000_read_word_attr(state, 1043, attr) & 0xff;
> @@ -593,7 +602,10 @@ static u8 dib9000_mbx_read(struct dib9000_state *state, u16 * data, u8 risc_id,
> if (!state->platform.risc.fw_is_running)
> return 0;
>
> - DibAcquireLock(&state->platform.risc.mbx_if_lock);
> + if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
> + dprintk("could not get the lock");
> + return 0;
> + }
> if (risc_id == 1)
> mc_base = 16;
> else
> @@ -701,7 +713,10 @@ static int dib9000_mbx_process(struct dib9000_state *state, u16 attr)
> if (!state->platform.risc.fw_is_running)
> return -1;
>
> - DibAcquireLock(&state->platform.risc.mbx_lock);
> + if (DibAcquireLock(&state->platform.risc.mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + return -1;
> + }
>
> if (dib9000_mbx_count(state, 1, attr)) /* 1=RiscB */
> ret = dib9000_mbx_fetch_to_cache(state, attr);
> @@ -1178,7 +1193,10 @@ static int dib9000_fw_get_channel(struct dvb_frontend *fe)
> struct dibDVBTChannel *ch;
> int ret = 0;
>
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
> ret = -EIO;
> goto error;
> @@ -1660,7 +1678,10 @@ static int dib9000_fw_component_bus_xfer(struct i2c_adapter *i2c_adap, struct i2
> p[12] = 0;
> }
>
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + return 0;
> + }
>
> dib9000_risc_mem_write(state, FE_MM_W_COMPONENT_ACCESS, p);
>
> @@ -1768,7 +1789,10 @@ int dib9000_fw_pid_filter_ctrl(struct dvb_frontend *fe, u8 onoff)
> return 0;
> }
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
>
> val = dib9000_read_word(state, 294 + 1) & 0xffef;
> val |= (onoff & 0x1) << 4;
> @@ -1800,7 +1824,10 @@ int dib9000_fw_pid_filter(struct dvb_frontend *fe, u8 id, u16 pid, u8 onoff)
> return 0;
> }
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> dprintk("Index %x, PID %d, OnOff %d", id, pid, onoff);
> ret = dib9000_write_word(state, 300 + 1 + id,
> onoff ? (1 << 13) | pid : 0);
> @@ -1848,7 +1875,10 @@ static int dib9000_sleep(struct dvb_frontend *fe)
> u8 index_frontend;
> int ret = 0;
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
> ret = state->fe[index_frontend]->ops.sleep(state->fe[index_frontend]);
> if (ret < 0)
> @@ -1874,8 +1904,12 @@ static int dib9000_get_frontend(struct dvb_frontend *fe)
> fe_status_t stat;
> int ret = 0;
>
> - if (state->get_frontend_internal == 0)
> - DibAcquireLock(&state->demod_lock);
> + if (state->get_frontend_internal == 0) {
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> + }
>
> for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
> state->fe[index_frontend]->ops.read_status(state->fe[index_frontend], &stat);
> @@ -1978,7 +2012,10 @@ static int dib9000_set_frontend(struct dvb_frontend *fe)
> }
>
> state->pid_ctrl_index = -1; /* postpone the pid filtering cmd */
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return 0;
> + }
>
> fe->dtv_property_cache.delivery_system = SYS_DVBT;
>
> @@ -2138,7 +2175,10 @@ static int dib9000_read_status(struct dvb_frontend *fe, fe_status_t * stat)
> u8 index_frontend;
> u16 lock = 0, lock_slave = 0;
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++)
> lock_slave |= dib9000_read_lock(state->fe[index_frontend]);
>
> @@ -2168,8 +2208,15 @@ static int dib9000_read_ber(struct dvb_frontend *fe, u32 * ber)
> u16 *c;
> int ret = 0;
>
> - DibAcquireLock(&state->demod_lock);
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + ret = -EINTR;
> + goto error;
> + }
> if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
> DibReleaseLock(&state->platform.risc.mem_mbx_lock);
> ret = -EIO;
> @@ -2196,7 +2243,10 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
> u16 val;
> int ret = 0;
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> *strength = 0;
> for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++) {
> state->fe[index_frontend]->ops.read_signal_strength(state->fe[index_frontend], &val);
> @@ -2206,7 +2256,11 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
> *strength += val;
> }
>
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + ret = -EINTR;
> + goto error;
> + }
> if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
> DibReleaseLock(&state->platform.risc.mem_mbx_lock);
> ret = -EIO;
> @@ -2233,10 +2287,13 @@ static u32 dib9000_get_snr(struct dvb_frontend *fe)
> u32 n, s, exp;
> u16 val;
>
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + return 0;
> + }
> if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
> DibReleaseLock(&state->platform.risc.mem_mbx_lock);
> - return -EIO;
> + return 0;
> }
> dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR, (u8 *) c, 16 * 2);
> DibReleaseLock(&state->platform.risc.mem_mbx_lock);
> @@ -2269,7 +2326,10 @@ static int dib9000_read_snr(struct dvb_frontend *fe, u16 * snr)
> u8 index_frontend;
> u32 snr_master;
>
> - DibAcquireLock(&state->demod_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> snr_master = dib9000_get_snr(fe);
> for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (state->fe[index_frontend] != NULL); index_frontend++)
> snr_master += dib9000_get_snr(state->fe[index_frontend]);
> @@ -2291,8 +2351,15 @@ static int dib9000_read_unc_blocks(struct dvb_frontend *fe, u32 * unc)
> u16 *c = (u16 *)state->i2c_read_buffer;
> int ret = 0;
>
> - DibAcquireLock(&state->demod_lock);
> - DibAcquireLock(&state->platform.risc.mem_mbx_lock);
> + if (DibAcquireLock(&state->demod_lock) < 0) {
> + dprintk("could not get the lock");
> + return -EINTR;
> + }
> + if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
> + dprintk("could not get the lock");
> + ret = -EINTR;
> + goto error;
> + }
> if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
> DibReleaseLock(&state->platform.risc.mem_mbx_lock);
> ret = -EIO;
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] [media] dib9000: get rid of Dib*Lock macros
2012-03-19 17:35 ` Mauro Carvalho Chehab
@ 2012-03-19 20:48 ` Alexey Khoroshilov
0 siblings, 0 replies; 3+ messages in thread
From: Alexey Khoroshilov @ 2012-03-19 20:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Alexey Khoroshilov, Patrick Boettcher, Olivier Grenie,
linux-media, linux-kernel, ldv-project
The patch replaces Dib*Lock macros with direct calls to mutex functions
as soon as they just make the driver code harder to review
(per request of Mauro).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/media/dvb/frontends/dib9000.c | 128 +++++++++++++++------------------
1 files changed, 58 insertions(+), 70 deletions(-)
diff --git a/drivers/media/dvb/frontends/dib9000.c b/drivers/media/dvb/frontends/dib9000.c
index 80848b4..953102f 100644
--- a/drivers/media/dvb/frontends/dib9000.c
+++ b/drivers/media/dvb/frontends/dib9000.c
@@ -31,13 +31,6 @@ struct i2c_device {
u8 *i2c_write_buffer;
};
-/* lock */
-#define DIB_LOCK struct mutex
-#define DibAcquireLock(lock) mutex_lock_interruptible(lock)
-#define DibReleaseLock(lock) mutex_unlock(lock)
-#define DibInitLock(lock) mutex_init(lock)
-#define DibFreeLock(lock)
-
struct dib9000_pid_ctrl {
#define DIB9000_PID_FILTER_CTRL 0
#define DIB9000_PID_FILTER 1
@@ -82,11 +75,11 @@ struct dib9000_state {
} fe_mm[18];
u8 memcmd;
- DIB_LOCK mbx_if_lock; /* to protect read/write operations */
- DIB_LOCK mbx_lock; /* to protect the whole mailbox handling */
+ struct mutex mbx_if_lock; /* to protect read/write operations */
+ struct mutex mbx_lock; /* to protect the whole mailbox handling */
- DIB_LOCK mem_lock; /* to protect the memory accesses */
- DIB_LOCK mem_mbx_lock; /* to protect the memory-based mailbox */
+ struct mutex mem_lock; /* to protect the memory accesses */
+ struct mutex mem_mbx_lock; /* to protect the memory-based mailbox */
#define MBX_MAX_WORDS (256 - 200 - 2)
#define DIB9000_MSG_CACHE_SIZE 2
@@ -108,7 +101,7 @@ struct dib9000_state {
struct i2c_msg msg[2];
u8 i2c_write_buffer[255];
u8 i2c_read_buffer[255];
- DIB_LOCK demod_lock;
+ struct mutex demod_lock;
u8 get_frontend_internal;
struct dib9000_pid_ctrl pid_ctrl[10];
s8 pid_ctrl_index; /* -1: empty list; -2: do not use the list */
@@ -446,13 +439,13 @@ static int dib9000_risc_mem_read(struct dib9000_state *state, u8 cmd, u8 * b, u1
if (!state->platform.risc.fw_is_running)
return -EIO;
- if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
dib9000_risc_mem_setup(state, cmd | 0x80);
dib9000_risc_mem_read_chunks(state, b, len);
- DibReleaseLock(&state->platform.risc.mem_lock);
+ mutex_unlock(&state->platform.risc.mem_lock);
return 0;
}
@@ -462,13 +455,13 @@ static int dib9000_risc_mem_write(struct dib9000_state *state, u8 cmd, const u8
if (!state->platform.risc.fw_is_running)
return -EIO;
- if (DibAcquireLock(&state->platform.risc.mem_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
dib9000_risc_mem_setup(state, cmd);
dib9000_risc_mem_write_chunks(state, b, m->size);
- DibReleaseLock(&state->platform.risc.mem_lock);
+ mutex_unlock(&state->platform.risc.mem_lock);
return 0;
}
@@ -537,7 +530,7 @@ static int dib9000_mbx_send_attr(struct dib9000_state *state, u8 id, u16 * data,
if (!state->platform.risc.fw_is_running)
return -EINVAL;
- if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mbx_if_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -584,7 +577,7 @@ static int dib9000_mbx_send_attr(struct dib9000_state *state, u8 id, u16 * data,
ret = (u8) dib9000_write_word_attr(state, 1043, 1 << 14, attr);
out:
- DibReleaseLock(&state->platform.risc.mbx_if_lock);
+ mutex_unlock(&state->platform.risc.mbx_if_lock);
return ret;
}
@@ -602,7 +595,7 @@ static u8 dib9000_mbx_read(struct dib9000_state *state, u16 * data, u8 risc_id,
if (!state->platform.risc.fw_is_running)
return 0;
- if (DibAcquireLock(&state->platform.risc.mbx_if_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mbx_if_lock) < 0) {
dprintk("could not get the lock");
return 0;
}
@@ -643,7 +636,7 @@ static u8 dib9000_mbx_read(struct dib9000_state *state, u16 * data, u8 risc_id,
/* Update register nb_mes_in_TX */
dib9000_write_word_attr(state, 1028 + mc_base, 1 << 14, attr);
- DibReleaseLock(&state->platform.risc.mbx_if_lock);
+ mutex_unlock(&state->platform.risc.mbx_if_lock);
return size + 1;
}
@@ -713,7 +706,7 @@ static int dib9000_mbx_process(struct dib9000_state *state, u16 attr)
if (!state->platform.risc.fw_is_running)
return -1;
- if (DibAcquireLock(&state->platform.risc.mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mbx_lock) < 0) {
dprintk("could not get the lock");
return -1;
}
@@ -724,7 +717,7 @@ static int dib9000_mbx_process(struct dib9000_state *state, u16 attr)
tmp = dib9000_read_word_attr(state, 1229, attr); /* Clear the IRQ */
/* if (tmp) */
/* dprintk( "cleared IRQ: %x", tmp); */
- DibReleaseLock(&state->platform.risc.mbx_lock);
+ mutex_unlock(&state->platform.risc.mbx_lock);
return ret;
}
@@ -1193,7 +1186,7 @@ static int dib9000_fw_get_channel(struct dvb_frontend *fe)
struct dibDVBTChannel *ch;
int ret = 0;
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -1323,7 +1316,7 @@ static int dib9000_fw_get_channel(struct dvb_frontend *fe)
}
error:
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
return ret;
}
@@ -1678,7 +1671,7 @@ static int dib9000_fw_component_bus_xfer(struct i2c_adapter *i2c_adap, struct i2
p[12] = 0;
}
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
return 0;
}
@@ -1692,7 +1685,7 @@ static int dib9000_fw_component_bus_xfer(struct i2c_adapter *i2c_adap, struct i2
/* do the transaction */
if (dib9000_fw_memmbx_sync(state, FE_SYNC_COMPONENT_ACCESS) < 0) {
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
return 0;
}
@@ -1700,7 +1693,7 @@ static int dib9000_fw_component_bus_xfer(struct i2c_adapter *i2c_adap, struct i2
if ((num > 1) && (msg[1].flags & I2C_M_RD))
dib9000_risc_mem_read(state, FE_MM_RW_COMPONENT_ACCESS_BUFFER, msg[1].buf, msg[1].len);
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
return num;
}
@@ -1789,7 +1782,7 @@ int dib9000_fw_pid_filter_ctrl(struct dvb_frontend *fe, u8 onoff)
return 0;
}
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -1799,7 +1792,7 @@ int dib9000_fw_pid_filter_ctrl(struct dvb_frontend *fe, u8 onoff)
dprintk("PID filter enabled %d", onoff);
ret = dib9000_write_word(state, 294 + 1, val);
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -1824,14 +1817,14 @@ int dib9000_fw_pid_filter(struct dvb_frontend *fe, u8 id, u16 pid, u8 onoff)
return 0;
}
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
dprintk("Index %x, PID %d, OnOff %d", id, pid, onoff);
ret = dib9000_write_word(state, 300 + 1 + id,
onoff ? (1 << 13) | pid : 0);
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
EXPORT_SYMBOL(dib9000_fw_pid_filter);
@@ -1851,11 +1844,6 @@ static void dib9000_release(struct dvb_frontend *demod)
for (index_frontend = 1; (index_frontend < MAX_NUMBER_OF_FRONTENDS) && (st->fe[index_frontend] != NULL); index_frontend++)
dvb_frontend_detach(st->fe[index_frontend]);
- DibFreeLock(&state->platform.risc.mbx_if_lock);
- DibFreeLock(&state->platform.risc.mbx_lock);
- DibFreeLock(&state->platform.risc.mem_lock);
- DibFreeLock(&state->platform.risc.mem_mbx_lock);
- DibFreeLock(&state->demod_lock);
dibx000_exit_i2c_master(&st->i2c_master);
i2c_del_adapter(&st->tuner_adap);
@@ -1875,7 +1863,7 @@ static int dib9000_sleep(struct dvb_frontend *fe)
u8 index_frontend;
int ret = 0;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -1887,7 +1875,7 @@ static int dib9000_sleep(struct dvb_frontend *fe)
ret = dib9000_mbx_send(state, OUT_MSG_FE_SLEEP, NULL, 0);
error:
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -1905,7 +1893,7 @@ static int dib9000_get_frontend(struct dvb_frontend *fe)
int ret = 0;
if (state->get_frontend_internal == 0) {
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -1964,7 +1952,7 @@ static int dib9000_get_frontend(struct dvb_frontend *fe)
return_value:
if (state->get_frontend_internal == 0)
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -2012,7 +2000,7 @@ static int dib9000_set_frontend(struct dvb_frontend *fe)
}
state->pid_ctrl_index = -1; /* postpone the pid filtering cmd */
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return 0;
}
@@ -2081,7 +2069,7 @@ static int dib9000_set_frontend(struct dvb_frontend *fe)
/* check the tune result */
if (exit_condition == 1) { /* tune failed */
dprintk("tune failed");
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
/* tune failed; put all the pid filtering cmd to junk */
state->pid_ctrl_index = -1;
return 0;
@@ -2137,7 +2125,7 @@ static int dib9000_set_frontend(struct dvb_frontend *fe)
/* turn off the diversity for the last frontend */
dib9000_fw_set_diversity_in(state->fe[index_frontend - 1], 0);
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
if (state->pid_ctrl_index >= 0) {
u8 index_pid_filter_cmd;
u8 pid_ctrl_index = state->pid_ctrl_index;
@@ -2175,7 +2163,7 @@ static int dib9000_read_status(struct dvb_frontend *fe, fe_status_t * stat)
u8 index_frontend;
u16 lock = 0, lock_slave = 0;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -2197,7 +2185,7 @@ static int dib9000_read_status(struct dvb_frontend *fe, fe_status_t * stat)
if ((lock & 0x0008) || (lock_slave & 0x0008))
*stat |= FE_HAS_LOCK;
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return 0;
}
@@ -2208,30 +2196,30 @@ static int dib9000_read_ber(struct dvb_frontend *fe, u32 * ber)
u16 *c;
int ret = 0;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
ret = -EINTR;
goto error;
}
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
goto error;
}
dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR,
state->i2c_read_buffer, 16 * 2);
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
c = (u16 *)state->i2c_read_buffer;
*ber = c[10] << 16 | c[11];
error:
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -2243,7 +2231,7 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
u16 val;
int ret = 0;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -2256,18 +2244,18 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
*strength += val;
}
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
ret = -EINTR;
goto error;
}
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
goto error;
}
dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR, (u8 *) c, 16 * 2);
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
val = 65535 - c[4];
if (val > 65535 - *strength)
@@ -2276,7 +2264,7 @@ static int dib9000_read_signal_strength(struct dvb_frontend *fe, u16 * strength)
*strength += val;
error:
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -2287,16 +2275,16 @@ static u32 dib9000_get_snr(struct dvb_frontend *fe)
u32 n, s, exp;
u16 val;
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
return 0;
}
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
return 0;
}
dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR, (u8 *) c, 16 * 2);
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
val = c[7];
n = (val >> 4) & 0xff;
@@ -2326,7 +2314,7 @@ static int dib9000_read_snr(struct dvb_frontend *fe, u16 * snr)
u8 index_frontend;
u32 snr_master;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
@@ -2340,7 +2328,7 @@ static int dib9000_read_snr(struct dvb_frontend *fe, u16 * snr)
} else
*snr = 0;
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return 0;
}
@@ -2351,27 +2339,27 @@ static int dib9000_read_unc_blocks(struct dvb_frontend *fe, u32 * unc)
u16 *c = (u16 *)state->i2c_read_buffer;
int ret = 0;
- if (DibAcquireLock(&state->demod_lock) < 0) {
+ if (mutex_lock_interruptible(&state->demod_lock) < 0) {
dprintk("could not get the lock");
return -EINTR;
}
- if (DibAcquireLock(&state->platform.risc.mem_mbx_lock) < 0) {
+ if (mutex_lock_interruptible(&state->platform.risc.mem_mbx_lock) < 0) {
dprintk("could not get the lock");
ret = -EINTR;
goto error;
}
if (dib9000_fw_memmbx_sync(state, FE_SYNC_CHANNEL) < 0) {
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
ret = -EIO;
goto error;
}
dib9000_risc_mem_read(state, FE_MM_R_FE_MONITOR, (u8 *) c, 16 * 2);
- DibReleaseLock(&state->platform.risc.mem_mbx_lock);
+ mutex_unlock(&state->platform.risc.mem_mbx_lock);
*unc = c[12];
error:
- DibReleaseLock(&state->demod_lock);
+ mutex_unlock(&state->demod_lock);
return ret;
}
@@ -2514,11 +2502,11 @@ struct dvb_frontend *dib9000_attach(struct i2c_adapter *i2c_adap, u8 i2c_addr, c
st->gpio_val = DIB9000_GPIO_DEFAULT_VALUES;
st->gpio_pwm_pos = DIB9000_GPIO_DEFAULT_PWM_POS;
- DibInitLock(&st->platform.risc.mbx_if_lock);
- DibInitLock(&st->platform.risc.mbx_lock);
- DibInitLock(&st->platform.risc.mem_lock);
- DibInitLock(&st->platform.risc.mem_mbx_lock);
- DibInitLock(&st->demod_lock);
+ mutex_init(&st->platform.risc.mbx_if_lock);
+ mutex_init(&st->platform.risc.mbx_lock);
+ mutex_init(&st->platform.risc.mem_lock);
+ mutex_init(&st->platform.risc.mem_mbx_lock);
+ mutex_init(&st->demod_lock);
st->get_frontend_internal = 0;
st->pid_ctrl_index = -2;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-19 20:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-06 21:08 [PATCH 2/2] [media] dib9000: implement error handling for DibAcquireLock Alexey Khoroshilov
2012-03-19 17:35 ` Mauro Carvalho Chehab
2012-03-19 20:48 ` [PATCH] [media] dib9000: get rid of Dib*Lock macros Alexey Khoroshilov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox