Hi Manjunatha, Thanks for all the work, it looks much better now! I found a few little things here, but otherwise you can stick a Reviewed-by: Hans Verkuil <hverkuil@xxxxxxxxx> tag in the whole driver series. On Monday, January 10, 2011 14:04:56 manjunatha_halli@xxxxxx wrote: > From: Manjunatha Halli <manjunatha_halli@xxxxxx> > > These are the sources for the common interfaces required by the > FM V4L2 driver for TI WL127x and WL128x chips. > > These implement the FM channel-8 protocol communication with the > chip. This makes use of the Shared Transport as its transport. > > Signed-off-by: Manjunatha Halli <manjunatha_halli@xxxxxx> > --- > drivers/media/radio/wl128x/fmdrv_common.c | 1693 +++++++++++++++++++++++++++++ > drivers/media/radio/wl128x/fmdrv_common.h | 402 +++++++ > 2 files changed, 2095 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/radio/wl128x/fmdrv_common.c > create mode 100644 drivers/media/radio/wl128x/fmdrv_common.h > > diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c > new file mode 100644 > index 0000000..46f5fe4 > --- /dev/null > +++ b/drivers/media/radio/wl128x/fmdrv_common.c <snip> > +u32 fmc_set_freq(struct fmdev *fmdev, u32 freq_to_set) > +{ > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + return fm_rx_set_freq(fmdev, freq_to_set); > + break; 'break' after a 'return' can be removed. This is repeated several times. > + > + case FM_MODE_TX: > + return fm_tx_set_freq(fmdev, freq_to_set); > + break; > + > + default: > + return -EINVAL; > + } > +} > + > +u32 fmc_get_freq(struct fmdev *fmdev, u32 *cur_tuned_frq) > +{ > + if (fmdev->rx.freq == FM_UNDEFINED_FREQ) { > + fmerr("RX frequency is not set\n"); > + return -EPERM; > + } > + if (cur_tuned_frq == NULL) { > + fmerr("Invalid memory\n"); > + return -ENOMEM; > + } > + > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + *cur_tuned_frq = fmdev->rx.freq; > + break; > + > + case FM_MODE_TX: > + *cur_tuned_frq = 0; /* TODO : Change this later */ > + break; > + > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +u32 fmc_set_region(struct fmdev *fmdev, u8 region_to_set) > +{ > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + return fm_rx_set_region(fmdev, region_to_set); > + break; > + > + case FM_MODE_TX: > + return fm_tx_set_region(fmdev, region_to_set); > + break; > + > + default: > + return -EINVAL; > + } > +} > + > +u32 fmc_set_mute_mode(struct fmdev *fmdev, u8 mute_mode_toset) > +{ > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + return fm_rx_set_mute_mode(fmdev, mute_mode_toset); > + break; > + > + case FM_MODE_TX: > + return fm_tx_set_mute_mode(fmdev, mute_mode_toset); > + break; > + > + default: > + return -EINVAL; > + } > +} > + > +u32 fmc_set_stereo_mono(struct fmdev *fmdev, u16 mode) > +{ > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + return fm_rx_set_stereo_mono(fmdev, mode); > + break; > + > + case FM_MODE_TX: > + return fm_tx_set_stereo_mono(fmdev, mode); > + break; > + > + default: > + return -EINVAL; > + } > +} > + > +u32 fmc_set_rds_mode(struct fmdev *fmdev, u8 rds_en_dis) > +{ > + switch (fmdev->curr_fmmode) { > + case FM_MODE_RX: > + return fm_rx_set_rds_mode(fmdev, rds_en_dis); > + break; > + > + case FM_MODE_TX: > + return fm_tx_set_rds_mode(fmdev, rds_en_dis); > + break; > + > + default: > + return -EINVAL; > + } > +} > + > +/* Sends power off command to the chip */ > +static u32 fm_power_down(struct fmdev *fmdev) > +{ > + u16 payload; > + u32 ret = 0; > + > + if (!test_bit(FM_CORE_READY, &fmdev->flag)) { > + fmerr("FM core is not ready\n"); > + return -EPERM; > + } > + if (fmdev->curr_fmmode == FM_MODE_OFF) { > + fmdbg("FM chip is already in OFF state\n"); > + return ret; > + } > + > + payload = 0x0; > + ret = fmc_send_cmd(fmdev, FM_POWER_MODE, REG_WR, &payload, > + sizeof(payload), NULL, NULL); > + if (ret < 0) > + return ret; > + > + ret = fmc_release(fmdev); Just do 'return fmc_release(fmdev);' here. > + > + return ret; > +} > + > +/* Reads init command from FM firmware file and loads to the chip */ > +static u32 fm_download_firmware(struct fmdev *fmdev, const u8 *fw_name) > +{ > + const struct firmware *fw_entry; > + struct bts_header *fw_header; > + struct bts_action *action; > + struct bts_action_delay *delay; > + u8 *fw_data; > + int ret, fw_len, cmd_cnt; > + > + cmd_cnt = 0; > + set_bit(FM_FW_DW_INPROGRESS, &fmdev->flag); > + > + ret = request_firmware(&fw_entry, fw_name, > + &fmdev->radio_dev->dev); > + if (ret < 0) { > + fmerr("Unable to read firmware(%s) content\n", fw_name); > + return ret; > + } > + fmdbg("Firmware(%s) length : %d bytes\n", fw_name, fw_entry->size); > + > + fw_data = (void *)fw_entry->data; > + fw_len = fw_entry->size; > + > + fw_header = (struct bts_header *)fw_data; > + if (fw_header->magic != FM_FW_FILE_HEADER_MAGIC) { > + fmerr("%s not a legal TI firmware file\n", fw_name); > + ret = -EINVAL; > + goto rel_fw; > + } > + fmdbg("FW(%s) magic number : 0x%x\n", fw_name, fw_header->magic); > + > + /* Skip file header info , we already verified it */ > + fw_data += sizeof(struct bts_header); > + fw_len -= sizeof(struct bts_header); > + > + while (fw_data && fw_len > 0) { > + action = (struct bts_action *)fw_data; > + > + switch (action->type) { > + case ACTION_SEND_COMMAND: /* Send */ > + if (fmc_send_cmd(fmdev, 0, 0, action->data, > + action->size, NULL, NULL)) > + goto rel_fw; > + > + cmd_cnt++; > + break; > + > + case ACTION_DELAY: /* Delay */ > + delay = (struct bts_action_delay *)action->data; > + mdelay(delay->msec); > + break; > + } > + > + fw_data += (sizeof(struct bts_action) + (action->size)); > + fw_len -= (sizeof(struct bts_action) + (action->size)); > + } > + fmdbg("Firmare commands(%d) loaded to chip\n", cmd_cnt); Firmare -> Firmware > +rel_fw: > + release_firmware(fw_entry); > + clear_bit(FM_FW_DW_INPROGRESS, &fmdev->flag); > + > + return ret; > +} <snip> Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html