* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.