From 55b2a312c29695520f813e61d668318169895ba7 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 8 Apr 2013 06:31:30 -0300 Subject: [PATCH] [media] radio-si4713: use V4L2 core lock Simplify locking by using the V4L2 core lock mechanism. This allows us to remove all locking from the i2c module. This will also simplify the upcoming conversion to the control framework. Signed-off-by: Hans Verkuil Acked-by: Eduardo Valentin Tested-by: Eduardo Valentin Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/radio-si4713.c | 4 + drivers/media/radio/si4713-i2c.c | 156 +++++++---------------------- drivers/media/radio/si4713-i2c.h | 1 - 3 files changed, 40 insertions(+), 121 deletions(-) diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c index f0f0a90c69ed..633c545438f7 100644 --- a/drivers/media/radio/radio-si4713.c +++ b/drivers/media/radio/radio-si4713.c @@ -49,6 +49,7 @@ MODULE_ALIAS("platform:radio-si4713"); struct radio_si4713_device { struct v4l2_device v4l2_dev; struct video_device radio_dev; + struct mutex lock; }; /* radio_si4713_fops - file operations interface */ @@ -247,6 +248,7 @@ static int radio_si4713_pdriver_probe(struct platform_device *pdev) rval = -ENOMEM; goto exit; } + mutex_init(&rsdev->lock); rval = v4l2_device_register(&pdev->dev, &rsdev->v4l2_dev); if (rval) { @@ -272,6 +274,8 @@ static int radio_si4713_pdriver_probe(struct platform_device *pdev) rsdev->radio_dev = radio_si4713_vdev_template; rsdev->radio_dev.v4l2_dev = &rsdev->v4l2_dev; + /* Serialize all access to the si4713 */ + rsdev->radio_dev.lock = &rsdev->lock; video_set_drvdata(&rsdev->radio_dev, rsdev); if (video_register_device(&rsdev->radio_dev, VFL_TYPE_RADIO, radio_nr)) { dev_err(&pdev->dev, "Could not register video device.\n"); diff --git a/drivers/media/radio/si4713-i2c.c b/drivers/media/radio/si4713-i2c.c index e305c14272b5..1cb9a2e7f683 100644 --- a/drivers/media/radio/si4713-i2c.c +++ b/drivers/media/radio/si4713-i2c.c @@ -21,7 +21,6 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include #include #include #include @@ -458,15 +457,13 @@ static int si4713_checkrev(struct si4713_device *sdev) int rval; u8 resp[SI4713_GETREV_NRESP]; - mutex_lock(&sdev->mutex); - rval = si4713_send_command(sdev, SI4713_CMD_GET_REV, NULL, 0, resp, ARRAY_SIZE(resp), DEFAULT_TIMEOUT); if (rval < 0) - goto unlock; + return rval; if (resp[1] == SI4713_PRODUCT_NUMBER) { v4l2_info(&sdev->sd, "chip found @ 0x%02x (%s)\n", @@ -475,9 +472,6 @@ static int si4713_checkrev(struct si4713_device *sdev) v4l2_err(&sdev->sd, "Invalid product number\n"); rval = -EINVAL; } - -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -778,17 +772,9 @@ static int si4713_tx_rds_ps(struct si4713_device *sdev, u8 psid, static int si4713_set_power_state(struct si4713_device *sdev, u8 value) { - int rval; - - mutex_lock(&sdev->mutex); - if (value) - rval = si4713_powerup(sdev); - else - rval = si4713_powerdown(sdev); - - mutex_unlock(&sdev->mutex); - return rval; + return si4713_powerup(sdev); + return si4713_powerdown(sdev); } static int si4713_set_mute(struct si4713_device *sdev, u16 mute) @@ -797,8 +783,6 @@ static int si4713_set_mute(struct si4713_device *sdev, u16 mute) mute = set_mute(mute); - mutex_lock(&sdev->mutex); - if (sdev->power_state) rval = si4713_write_property(sdev, SI4713_TX_LINE_INPUT_MUTE, mute); @@ -806,8 +790,6 @@ static int si4713_set_mute(struct si4713_device *sdev, u16 mute) if (rval >= 0) sdev->mute = get_mute(mute); - mutex_unlock(&sdev->mutex); - return rval; } @@ -820,15 +802,13 @@ static int si4713_set_rds_ps_name(struct si4713_device *sdev, char *ps_name) if (!strlen(ps_name)) memset(ps_name, 0, MAX_RDS_PS_NAME + 1); - mutex_lock(&sdev->mutex); - if (sdev->power_state) { /* Write the new ps name and clear the padding */ for (i = 0; i < MAX_RDS_PS_NAME; i += (RDS_BLOCK / 2)) { rval = si4713_tx_rds_ps(sdev, (i / (RDS_BLOCK / 2)), ps_name + i); if (rval < 0) - goto unlock; + return rval; } /* Setup the size to be sent */ @@ -841,19 +821,16 @@ static int si4713_set_rds_ps_name(struct si4713_device *sdev, char *ps_name) SI4713_TX_RDS_PS_MESSAGE_COUNT, rds_ps_nblocks(len)); if (rval < 0) - goto unlock; + return rval; rval = si4713_write_property(sdev, SI4713_TX_RDS_PS_REPEAT_COUNT, DEFAULT_RDS_PS_REPEAT_COUNT * 2); if (rval < 0) - goto unlock; + return rval; } strncpy(sdev->rds_info.ps_name, ps_name, MAX_RDS_PS_NAME); - -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -864,14 +841,12 @@ static int si4713_set_rds_radio_text(struct si4713_device *sdev, char *rt) u8 b_index = 0, cr_inserted = 0; s8 left; - mutex_lock(&sdev->mutex); - if (!sdev->power_state) goto copy; rval = si4713_tx_rds_buff(sdev, RDS_BLOCK_CLEAR, 0, 0, 0, &left); if (rval < 0) - goto unlock; + return rval; if (!strlen(rt)) goto copy; @@ -898,7 +873,7 @@ static int si4713_set_rds_radio_text(struct si4713_device *sdev, char *rt) compose_u16(rt[t_index + 2], rt[t_index + 3]), &left); if (rval < 0) - goto unlock; + return rval; t_index += RDS_RADIOTEXT_BLK_SIZE; @@ -908,9 +883,6 @@ static int si4713_set_rds_radio_text(struct si4713_device *sdev, char *rt) copy: strncpy(sdev->rds_info.radio_text, rt, MAX_RDS_RADIO_TEXT); - -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1114,9 +1086,7 @@ static int si4713_write_econtrol_tune(struct si4713_device *sdev, rval = validate_range(&sdev->sd, control); if (rval < 0) - goto exit; - - mutex_lock(&sdev->mutex); + return rval; switch (control->id) { case V4L2_CID_TUNE_POWER_LEVEL: @@ -1128,8 +1098,7 @@ static int si4713_write_econtrol_tune(struct si4713_device *sdev, antcap = control->value; break; default: - rval = -EINVAL; - goto unlock; + return -EINVAL; } if (sdev->power_state) @@ -1140,9 +1109,6 @@ static int si4713_write_econtrol_tune(struct si4713_device *sdev, sdev->antenna_capacitor = antcap; } -unlock: - mutex_unlock(&sdev->mutex); -exit: return rval; } @@ -1159,12 +1125,12 @@ static int si4713_write_econtrol_integers(struct si4713_device *sdev, rval = validate_range(&sdev->sd, control); if (rval < 0) - goto exit; + return rval; rval = si4713_choose_econtrol_action(sdev, control->id, &shadow, &bit, &mask, &property, &mul, &table, &size); if (rval < 0) - goto exit; + return rval; val = control->value; if (mul) { @@ -1172,24 +1138,22 @@ static int si4713_write_econtrol_integers(struct si4713_device *sdev, } else if (table) { rval = usecs_to_dev(control->value, table, size); if (rval < 0) - goto exit; + return rval; val = rval; rval = 0; } - mutex_lock(&sdev->mutex); - if (sdev->power_state) { if (mask) { rval = si4713_read_property(sdev, property, &val); if (rval < 0) - goto unlock; + return rval; val = set_bits(val, control->value, bit, mask); } rval = si4713_write_property(sdev, property, val); if (rval < 0) - goto unlock; + return rval; if (mask) val = control->value; } @@ -1199,16 +1163,13 @@ static int si4713_write_econtrol_integers(struct si4713_device *sdev, } else if (table) { rval = dev_to_usecs(val, table, size); if (rval < 0) - goto unlock; + return rval; *shadow = rval; rval = 0; } else { *shadow = val; } -unlock: - mutex_unlock(&sdev->mutex); -exit: return rval; } @@ -1231,9 +1192,7 @@ static int si4713_setup(struct si4713_device *sdev) return -ENOMEM; /* Get a local copy to avoid race */ - mutex_lock(&sdev->mutex); memcpy(tmp, sdev, sizeof(*sdev)); - mutex_unlock(&sdev->mutex); ctrl.id = V4L2_CID_RDS_TX_PI; ctrl.value = tmp->rds_info.pi; @@ -1338,17 +1297,15 @@ static int si4713_initialize(struct si4713_device *sdev) rval = si4713_set_power_state(sdev, POWER_ON); if (rval < 0) - goto exit; + return rval; rval = si4713_checkrev(sdev); if (rval < 0) - goto exit; + return rval; rval = si4713_set_power_state(sdev, POWER_OFF); if (rval < 0) - goto exit; - - mutex_lock(&sdev->mutex); + return rval; sdev->rds_info.pi = DEFAULT_RDS_PI; sdev->rds_info.pty = DEFAULT_RDS_PTY; @@ -1380,9 +1337,6 @@ static int si4713_initialize(struct si4713_device *sdev) sdev->stereo = 1; sdev->tune_rnl = DEFAULT_TUNE_RNL; - mutex_unlock(&sdev->mutex); - -exit: return rval; } @@ -1428,7 +1382,7 @@ exit: /* * si4713_update_tune_status - update properties from tx_tune_status - * command. Must be called with sdev->mutex held. + * command. * @sdev: si4713_device structure for the device we are communicating */ static int si4713_update_tune_status(struct si4713_device *sdev) @@ -1456,12 +1410,10 @@ static int si4713_read_econtrol_tune(struct si4713_device *sdev, { s32 rval = 0; - mutex_lock(&sdev->mutex); - if (sdev->power_state) { rval = si4713_update_tune_status(sdev); if (rval < 0) - goto unlock; + return rval; } switch (control->id) { @@ -1472,11 +1424,9 @@ static int si4713_read_econtrol_tune(struct si4713_device *sdev, control->value = sdev->antenna_capacitor; break; default: - rval = -EINVAL; + return -EINVAL; } -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1494,14 +1444,12 @@ static int si4713_read_econtrol_integers(struct si4713_device *sdev, rval = si4713_choose_econtrol_action(sdev, control->id, &shadow, &bit, &mask, &property, &mul, &table, &size); if (rval < 0) - goto exit; - - mutex_lock(&sdev->mutex); + return rval; if (sdev->power_state) { rval = si4713_read_property(sdev, property, &val); if (rval < 0) - goto unlock; + return rval; /* Keep negative values for threshold */ if (control->id == V4L2_CID_AUDIO_COMPRESSION_THRESHOLD) @@ -1516,9 +1464,6 @@ static int si4713_read_econtrol_integers(struct si4713_device *sdev, control->value = *shadow; -unlock: - mutex_unlock(&sdev->mutex); -exit: return rval; } @@ -1712,14 +1657,12 @@ static int si4713_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl) if (!sdev) return -ENODEV; - mutex_lock(&sdev->mutex); - if (sdev->power_state) { rval = si4713_read_property(sdev, SI4713_TX_LINE_INPUT_MUTE, &sdev->mute); if (rval < 0) - goto unlock; + return rval; } switch (ctrl->id) { @@ -1728,8 +1671,6 @@ static int si4713_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl) break; } -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1779,7 +1720,6 @@ static long si4713_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) if (!arg) return -EINVAL; - mutex_lock(&sdev->mutex); switch (cmd) { case SI4713_IOC_MEASURE_RNL: frequency = v4l2_to_si4713(rnl->frequency); @@ -1788,11 +1728,11 @@ static long si4713_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) /* Set desired measurement frequency */ rval = si4713_tx_tune_measure(sdev, frequency, 0); if (rval < 0) - goto unlock; + return rval; /* get results from tune status */ rval = si4713_update_tune_status(sdev); if (rval < 0) - goto unlock; + return rval; } rnl->rnl = sdev->tune_rnl; break; @@ -1802,8 +1742,6 @@ static long si4713_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) rval = -ENOIOCTLCMD; } -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1822,15 +1760,11 @@ static int si4713_g_modulator(struct v4l2_subdev *sd, struct v4l2_modulator *vm) struct si4713_device *sdev = to_si4713_device(sd); int rval = 0; - if (!sdev) { - rval = -ENODEV; - goto exit; - } + if (!sdev) + return -ENODEV; - if (vm->index > 0) { - rval = -EINVAL; - goto exit; - } + if (vm->index > 0) + return -EINVAL; strncpy(vm->name, "FM Modulator", 32); vm->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_LOW | @@ -1840,15 +1774,13 @@ static int si4713_g_modulator(struct v4l2_subdev *sd, struct v4l2_modulator *vm) vm->rangelow = si4713_to_v4l2(FREQ_RANGE_LOW); vm->rangehigh = si4713_to_v4l2(FREQ_RANGE_HIGH); - mutex_lock(&sdev->mutex); - if (sdev->power_state) { u32 comp_en = 0; rval = si4713_read_property(sdev, SI4713_TX_COMPONENT_ENABLE, &comp_en); if (rval < 0) - goto unlock; + return rval; sdev->stereo = get_status_bit(comp_en, 1, 1 << 1); sdev->rds_info.enabled = get_status_bit(comp_en, 2, 1 << 2); @@ -1866,9 +1798,6 @@ static int si4713_g_modulator(struct v4l2_subdev *sd, struct v4l2_modulator *vm) else vm->txsubchans &= ~V4L2_TUNER_SUB_RDS; -unlock: - mutex_unlock(&sdev->mutex); -exit: return rval; } @@ -1896,13 +1825,11 @@ static int si4713_s_modulator(struct v4l2_subdev *sd, const struct v4l2_modulato rds = !!(vm->txsubchans & V4L2_TUNER_SUB_RDS); - mutex_lock(&sdev->mutex); - if (sdev->power_state) { rval = si4713_read_property(sdev, SI4713_TX_COMPONENT_ENABLE, &p); if (rval < 0) - goto unlock; + return rval; p = set_bits(p, stereo, 1, 1 << 1); p = set_bits(p, rds, 2, 1 << 2); @@ -1910,14 +1837,12 @@ static int si4713_s_modulator(struct v4l2_subdev *sd, const struct v4l2_modulato rval = si4713_write_property(sdev, SI4713_TX_COMPONENT_ENABLE, p); if (rval < 0) - goto unlock; + return rval; } sdev->stereo = stereo; sdev->rds_info.enabled = rds; -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1929,23 +1854,19 @@ static int si4713_g_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *f) f->type = V4L2_TUNER_RADIO; - mutex_lock(&sdev->mutex); - if (sdev->power_state) { u16 freq; u8 p, a, n; rval = si4713_tx_tune_status(sdev, 0x00, &freq, &p, &a, &n); if (rval < 0) - goto unlock; + return rval; sdev->frequency = freq; } f->frequency = si4713_to_v4l2(sdev->frequency); -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -1960,19 +1881,15 @@ static int si4713_s_frequency(struct v4l2_subdev *sd, const struct v4l2_frequenc if (frequency < FREQ_RANGE_LOW || frequency > FREQ_RANGE_HIGH) return -EDOM; - mutex_lock(&sdev->mutex); - if (sdev->power_state) { rval = si4713_tx_tune_freq(sdev, frequency); if (rval < 0) - goto unlock; + return rval; frequency = rval; rval = 0; } sdev->frequency = frequency; -unlock: - mutex_unlock(&sdev->mutex); return rval; } @@ -2030,7 +1947,6 @@ static int si4713_probe(struct i2c_client *client, v4l2_i2c_subdev_init(&sdev->sd, client, &si4713_subdev_ops); - mutex_init(&sdev->mutex); init_completion(&sdev->work); if (client->irq) { diff --git a/drivers/media/radio/si4713-i2c.h b/drivers/media/radio/si4713-i2c.h index c6dfa7fb101c..979828da9d27 100644 --- a/drivers/media/radio/si4713-i2c.h +++ b/drivers/media/radio/si4713-i2c.h @@ -220,7 +220,6 @@ struct si4713_device { /* v4l2_subdev and i2c reference (v4l2_subdev priv data) */ struct v4l2_subdev sd; /* private data structures */ - struct mutex mutex; struct completion work; struct rds_info rds_info; struct limiter_info limiter_info; -- 2.39.5