linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] KT0913 FM/AM driver
@ 2020-08-31 22:05 Santiago Hormazabal
  2020-08-31 22:05 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Santiago Hormazabal @ 2020-08-31 22:05 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel
  Cc: Santiago Hormazabal

media: adds support for kt0913 FM/AM tuner chip

Adds a driver for the KT0913 FM/AM tuner chip from KT Micro. This chip
can be found on many low cost FM/AM radios and DVD/Home Theaters.
The chip provides two ways of usage, a manual mode (requiring only a
few buttons) or complete control via I2C. This driver uses the latter.
It exposes the minimum functionality of this chip, which includes tuning
an AM or FM station given its frequency, reading the signal strength,
setting Stereo (only on FM) or Mono (available on AM/FM), Mute, Volume
and Audio Gain.
I left some TODOs on the code, like supporting the chip's hardware seek
feature, using a RW/RO regmaps rather than a single volatile regmap,
show the FM SNR as a RO control and the FM/AM AFC deviation as another
RO control, and supporting the KT0915 chip.
The module I've used comes from SZZSJDZ.com, a now defunct company.
However, it's possible to buy this chip directly from Aliexpress or
similar sites.
I tested this on two systems, the first one being a Raspberry Pi 4 with
the unstable 5.x kernel, but later I moved to a Banana Pi 2 Zero where
I used the (current at this time, 07d999f) master of this repo for testing.
I've also compiled the v4l-compliance from current sources (79918a59) and it
passed all the tests. The output of that is at the end of this note.

Note: This is the third set of patches for the driver, where I (tried to)
address the comments that the reviewers added on the previous sets.

Differences between v1 and v2: Fixed the logic for band selection when
running the S_FREQUENCY ioctl. Before this, it would return a -EINVAL error
when an improper frequency was selected. Now it tries to select the nearest
available band frequency.
Fixed some text alignment issues.
Difference between v2 and v3: Fixed typos (CAMUS -> CAMPUS), text alignment
fixes, converted all hex values into lowercase, renamed driver name to just
kt0913 rather than kt0913-am-fm.

v4l2-compliance SHA: 79918a591a9ad362f107795ee4046d39e6dfcb67, 32 bits, 32-bit time_t

Compliance test for kt0913 device /dev/radio0:

Driver Info:
	Driver name      : kt0913
	Card type        : kt0913
	Bus info         : I2C:radio0
	Driver version   : 5.9.0
	Capabilities     : 0x80250000
		Tuner
		Radio
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x00250000
		Tuner
		Radio
		Extended Pix Format

Required ioctls:
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/radio0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

	test invalid ioctls: OK
Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK
	test VIDIOC_LOG_STATUS: OK

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK
	test VIDIOC_G/S_FREQUENCY: OK
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 0 Audio Inputs: 0 Tuners: 1

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 8 Private Controls: 0

Format ioctls:
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK (Not Supported)
	test VIDIOC_TRY_FMT: OK (Not Supported)
	test VIDIOC_S_FMT: OK (Not Supported)
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK (Not Supported)

Codec ioctls:
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
	test VIDIOC_EXPBUF: OK (Not Supported)
	test Requests: OK (Not Supported)

Total for kt0913 device /dev/radio0: 45, Succeeded: 45, Failed: 0, Warnings: 0

Santiago Hormazabal (3):
  dt-bindings: vendor-prefixes: Add KT Micro
  media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  media: kt0913: device tree binding

 .../bindings/media/i2c/ktm,kt0913.yaml        |   56 +
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 drivers/media/radio/Kconfig                   |   10 +
 drivers/media/radio/Makefile                  |    1 +
 drivers/media/radio/radio-kt0913.c            | 1196 +++++++++++++++++
 5 files changed, 1265 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
 create mode 100644 drivers/media/radio/radio-kt0913.c

-- 
2.24.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro
  2020-08-31 22:05 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
@ 2020-08-31 22:05 ` Santiago Hormazabal
  2020-09-14 18:27   ` Rob Herring
  2020-08-31 22:06 ` [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from " Santiago Hormazabal
  2020-08-31 22:06 ` [PATCH 3/3] media: kt0913: device tree binding Santiago Hormazabal
  2 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-08-31 22:05 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel
  Cc: Santiago Hormazabal

Adds ktm as the prefix of KT Micro, Inc.

Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2baee2c817c1..42fcbe256113 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -551,6 +551,8 @@ patternProperties:
     description: Kontron S&T AG
   "^kosagi,.*":
     description: Sutajio Ko-Usagi PTE Ltd.
+  "^ktm,.*":
+    description: KT Micro, Inc.
   "^kyo,.*":
     description: Kyocera Corporation
   "^lacie,.*":
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-08-31 22:05 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
  2020-08-31 22:05 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
@ 2020-08-31 22:06 ` Santiago Hormazabal
  2020-09-03 16:45   ` Joe Perches
  2020-09-09 14:06   ` Hans Verkuil
  2020-08-31 22:06 ` [PATCH 3/3] media: kt0913: device tree binding Santiago Hormazabal
  2 siblings, 2 replies; 11+ messages in thread
From: Santiago Hormazabal @ 2020-08-31 22:06 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel
  Cc: Santiago Hormazabal

This chip requires almost no support components and can used over I2C.
The driver uses the I2C bus and exposes the controls as a V4L2 radio.
Tested with a module that contains this chip (from SZZSJDZ.com,
part number ZJ-801B, even tho the company seems defunct now), and an H2+
AllWinner SoC running a kernel built off 07d999f of the media_tree.

Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
---
 drivers/media/radio/Kconfig        |   10 +
 drivers/media/radio/Makefile       |    1 +
 drivers/media/radio/radio-kt0913.c | 1196 ++++++++++++++++++++++++++++
 3 files changed, 1207 insertions(+)
 create mode 100644 drivers/media/radio/radio-kt0913.c

diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig
index d29e29645e04..ac9053a95f3a 100644
--- a/drivers/media/radio/Kconfig
+++ b/drivers/media/radio/Kconfig
@@ -226,6 +226,16 @@ config RADIO_WL1273
 # TI's ST based wl128x FM radio
 source "drivers/media/radio/wl128x/Kconfig"
 
+config RADIO_KT0913
+	tristate "KT0913 I2C FM/AM radio support"
+	depends on I2C && VIDEO_V4L2
+	help
+	  Say Y here if you want to use the KT0913 FM/AM chip.
+	  This is a low cost chip that uses the I2C bus.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called radio-kt0913.
+
 #
 # ISA drivers configuration
 #
diff --git a/drivers/media/radio/Makefile b/drivers/media/radio/Makefile
index 53c7ae135460..a314121f7771 100644
--- a/drivers/media/radio/Makefile
+++ b/drivers/media/radio/Makefile
@@ -34,5 +34,6 @@ obj-$(CONFIG_RADIO_WL1273) += radio-wl1273.o
 obj-$(CONFIG_RADIO_WL128X) += wl128x/
 obj-$(CONFIG_RADIO_TEA575X) += tea575x.o
 obj-$(CONFIG_USB_RAREMONO) += radio-raremono.o
+obj-$(CONFIG_RADIO_KT0913) += radio-kt0913.o
 
 shark2-objs := radio-shark2.o radio-tea5777.o
diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
new file mode 100644
index 000000000000..067d7ceac079
--- /dev/null
+++ b/drivers/media/radio/radio-kt0913.c
@@ -0,0 +1,1196 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * drivers/media/radio/radio-kt0913.c
+ *
+ * Driver for the KT0913 radio chip from KTMicro.
+ * This driver provides a v4l2 interface to the tuner, using the I2C
+ * protocol to communicate with the chip.
+ * It exposes two bands, one for AM and another for FM. If the "campus
+ * band" feature needs to be enabled, set the corresponding module parameter
+ * to 1.
+ * Reference Clock and Audio DAC anti-pop configurations should be
+ * set via a device tree node. Defaults will be used otherwise.
+ *
+ * Audio output should be routed to a speaker or an audio capture
+ * device.
+ *
+ * Based on radio-tea5764 by Fabio Belavenuto <belavenuto@gmail.com>
+ *
+ *  Copyright (c) 2020 Santiago Hormazabal <santiagohssl@gmail.com>
+ *
+ * TODO:
+ *  use rd and wr support for the regmap instead of volatile regs.
+ *  add support for the hardware-assisted frequency seek.
+ *  export FM SNR and AM/FM AFC deviation values as RO controls.
+ *  support the kt0915 chip.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/math64.h>
+#include <linux/regmap.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
+
+ /* ************************************************************************* */
+
+ /* registers of the kt0913 */
+#define KT0913_REG_CHIP_ID      0x01
+#define KT0913_REG_SEEK         0x02
+#define KT0913_REG_TUNE         0x03
+#define KT0913_REG_VOLUME       0x04
+#define KT0913_REG_DSPCFGA      0x05
+#define KT0913_REG_LOCFGA       0x0a
+#define KT0913_REG_LOCFGC       0x0c
+#define KT0913_REG_RXCFG        0x0f
+#define KT0913_REG_STATUSA      0x12
+#define KT0913_REG_STATUSB      0x13
+#define KT0913_REG_STATUSC      0x14
+#define KT0913_REG_AMSYSCFG     0x16
+#define KT0913_REG_AMCHAN       0x17
+#define KT0913_REG_AMCALI       0x18
+#define KT0913_REG_GPIOCFG      0x1d
+#define KT0913_REG_AMDSP        0x22
+#define KT0913_REG_AMSTATUSA    0x24
+#define KT0913_REG_AMSTATUSB    0x25
+#define KT0913_REG_SOFTMUTE     0x2e
+#define KT0913_REG_AMCFG        0x33
+#define KT0913_REG_AMCFG2       0x34
+#define KT0913_REG_AFC          0x3c
+
+/* register symbols masks, values and shift count */
+#define KT0913_TUNE_FMTUNE_MASK 0x8000 /* FM Tune enable */
+#define KT0913_TUNE_FMTUNE_ON 0x8000 /* FM Tune enabled */
+#define KT0913_TUNE_FMTUNE_OFF 0x0000 /* FM Tune disabled */
+#define KT0913_TUNE_FMCHAN_MASK 0x0fff /* frequency in kHz / 50kHz */
+
+#define KT0913_VOLUME_DMUTE_MASK 0x2000
+#define KT0913_VOLUME_DMUTE_ON 0x0000
+#define KT0913_VOLUME_DMUTE_OFF 0x2000
+#define KT0913_VOLUME_DE_MASK 0x0800 /* de-emphasis time constant */
+#define KT0913_VOLUME_DE_75US 0x0000 /* 75us */
+#define KT0913_VOLUME_DE_50US 0x0800 /* 50us */
+#define KT0913_VOLUME_POP_MASK 0x30 /* audio dac anti-pop config */
+#define KT0913_VOLUME_POP_SHIFT 4
+
+#define KT0913_DSPCFGA_MONO_MASK 0x8000 /* mono select (0=stereo, 1=mono) */
+#define KT0913_DSPCFGA_MONO_ON 0x8000 /* mono */
+#define KT0913_DSPCFGA_MONO_OFF 0x0000 /* stereo */
+
+#define KT0913_LOCFG_CAMPUSBAND_EN_MASK 0x0008 /* campus band fm enable */
+#define KT0913_LOCFG_CAMPUSBAND_EN_ON 0x0008 /* FM range 64-110MHz */
+#define KT0913_LOCFG_CAMPUSBAND_EN_OFF 0x0000 /* FM range 32-110MHz */
+
+#define KT0913_RXCFGA_STDBY_MASK 0x1000 /* standby mode enable */
+#define KT0913_RXCFGA_STDBY_ON 0x1000 /* standby mode enabled */
+#define KT0913_RXCFGA_STDBY_OFF 0x0000 /* standby mode disabled */
+#define KT0913_RXCFGA_VOLUME_MASK 0x001f /* volume control */
+
+#define KT0913_STATUSA_XTAL_OK 0x8000 /* crystal ready indicator */
+#define KT0913_STATUSA_STC 0x4000 /* seek/tune complete */
+
+#define KT0913_STATUSA_PLL_LOCK_MASK 0x800 /* system pll ready indicator */
+#define KT0913_STATUSA_PLL_LOCK_LOCKED 0x800 /* system pll ready */
+#define KT0913_STATUSA_PLL_LOCK_UNLOCKED 0x000 /* not ready */
+#define KT0913_STATUSA_LO_LOCK 0x400 /* LO synthesizer ready indicator */
+#define KT0913_STATUSA_ST_MASK 0x300 /* stereo indicator (0x300=stereo, otherwise mono) */
+#define KT0913_STATUSA_ST_STEREO 0x300 /* stereo */
+#define KT0913_STATUSA_FMRSSI_MASK 0xf8 /* FM RSSI (-100dBm + FMRSSI*3dBm) */
+#define KT0913_STATUSA_FMRSSI_SHIFT 3
+
+#define KT0913_STATUSC_PWSTATUS 0x8000 /* power status indicator */
+#define KT0913_STATUSC_CHIPRDY 0x2000 /* chip ready indicator */
+#define KT0913_STATUSC_FMSNR 0x1fc0 /* FM SNR (unknown units) */
+
+#define KT0913_AMCHAN_AMTUNE_MASK 0x8000 /* AM tune enable */
+#define KT0913_AMCHAN_AMTUNE_ON 0x8000 /* AM tune enabled */
+#define KT0913_AMCHAN_AMTUNE_OFF 0x0000 /* AM tune disabled */
+#define KT0913_AMCHAN_AMCHAN_MASK 0x7ff /* am channel in kHz */
+
+#define KT0913_AMSYSCFG_AM_FM_MASK 0x8000 /* am/fm mode control */
+#define KT0913_AMSYSCFG_AM_FM_AM 0x8000 /* am mode */
+#define KT0913_AMSYSCFG_AM_FM_FM 0x0000 /* fm mode (default) */
+#define KT0913_AMSYSCFG_REFCLK_MASK 0x0f00 /* reference clock selection */
+#define KT0913_AMSYSCFG_REFCLK_SHIFT 8
+#define KT0913_AMSYSCFG_AU_GAIN_MASK 0x00c0 /* audio gain selection */
+#define KT0913_AMSYSCFG_AU_GAIN_6DB 0x0040 /* 6dB audio gain */
+#define KT0913_AMSYSCFG_AU_GAIN_3DB 0x0000 /* 3dB audio gain (default) */
+#define KT0913_AMSYSCFG_AU_GAIN_0DB 0x00c0 /* 0dB audio gain */
+#define KT0913_AMSYSCFG_AU_GAIN_MIN_3DB 0x0080 /* -3dB audio gain */
+
+#define KT0913_AMSTATUSA_AMRSSI_MASK 0x1f00 /* am channel rssi */
+#define KT0913_AMSTATUSA_AMRSSI_SHIFT 8
+
+/* constants */
+#define KT0913_CHIP_ID  0x544B /* ASCII of 'KT' */
+
+#define V4L2_KHZ_FREQ_MUL 16U /* v4l2 uses 16x the kHz value as their freq */
+#define KT0913_FMCHAN_MUL 50U /* kt0913 uses freqs with a 50kHz multiplier */
+#define KT0913_FM_RANGE_LOW_NO_CAMPUS 64000U /* 64MHz lower bound for FM */
+#define KT0913_FM_RANGE_LOW_CAMPUS 32000U /* 32MHz lower bound for campus FM */
+#define KT0913_FM_RANGE_HIGH 110000U /* 110MHz upper bound for FM */
+#define KT0913_AM_RANGE_LOW  500U /* 500kHz lower bound for AM */
+#define KT0913_AM_RANGE_HIGH 1710U /* 1710kHz upper bound for AM */
+
+#define KT0913_FM_AM_DRIVER_NAME "kt0913"
+
+/* ************************************************************************* */
+
+/* v4l2 device number to use. -1 will assign the next free one */
+static int kt0913_v4l2_radio_nr = -1;
+/* use the extended range of FM down to 32MHz. disabled by default */
+static int kt0913_use_campus_band;
+
+/* ************************************************************************* */
+
+/* kt0913 status struct */
+struct kt0913_device {
+	struct v4l2_device v4l2_dev;		/* main v4l2 struct */
+	struct i2c_client *client;		/* I2C client */
+	struct video_device vdev;		/* vide_device struct */
+	struct v4l2_ctrl_handler ctrl_handler;	/* ctrl_handler struct */
+
+	/* V4L2 Controls */
+	struct v4l2_ctrl *ctrl_pll_lock;	/* PLL lock */
+	struct v4l2_ctrl *ctrl_volume;		/* Overall volume */
+	struct v4l2_ctrl *ctrl_au_gain;		/* Audio Gain */
+	struct v4l2_ctrl *ctrl_mute;		/* Master mute */
+	struct v4l2_ctrl *ctrl_deemp;		/* Deemphasis */
+
+	/* current operation band (fm, fm_campus, am) */
+	unsigned int band;
+
+	/* audio dac anti-pop setting:
+	 *  0 -> 100uF (default)
+	 *  1 -> 60uF
+	 *  2 -> 20uF
+	 *  3 -> 10uF
+	 */
+	unsigned int audio_anti_pop;
+
+	/* reference clock selection:
+	 *  0 -> 32.768kHz (default)
+	 *  1 -> 6.5MHz
+	 *  2 -> 7.6MHz
+	 *  3 -> 12MHz
+	 *  4 -> 13MHz
+	 *  5 -> 15.2MHz
+	 *  6 -> 19.2MHz
+	 *  7 -> 24MHz
+	 *  8 -> 26MHz
+	 *  9 -> 38kHz
+	 */
+	unsigned int refclock_val;
+
+	/* Regmap */
+	struct regmap *regmap;
+
+	/* For core assisted locking */
+	struct mutex mutex;
+};
+
+/* ************************************************************************* */
+
+/* Regmap settings */
+static const struct regmap_range kt0913_regmap_all_registers_range[] = {
+	regmap_reg_range(0x01, 0x05),
+	regmap_reg_range(0x0a, 0x0a),
+	regmap_reg_range(0x0c, 0x0c),
+	regmap_reg_range(0x0f, 0x0f),
+	regmap_reg_range(0x12, 0x14),
+	regmap_reg_range(0x16, 0x18),
+	regmap_reg_range(0x1d, 0x1d),
+	regmap_reg_range(0x22, 0x22),
+	regmap_reg_range(0x24, 0x25),
+	regmap_reg_range(0x2e, 0x2f),
+	regmap_reg_range(0x30, 0x34),
+	regmap_reg_range(0x3a, 0x3a),
+	regmap_reg_range(0x3c, 0x3c),
+};
+
+static const struct regmap_access_table kt0913_all_registers_access_table = {
+	.yes_ranges = kt0913_regmap_all_registers_range,
+	.n_yes_ranges = ARRAY_SIZE(kt0913_regmap_all_registers_range),
+};
+
+static const struct reg_sequence kt0913_init_regs_to_defaults[] = {
+	/* Standby disabled, volume 0dB */
+	{ KT0913_REG_RXCFG, 0x881f },
+	/* FM Channel spacing = 50kHz, Right & Left unmuted */
+	{ KT0913_REG_SEEK, 0x000b },
+	/* Stereo, High Stereo/Mono blend level, blend disabled */
+	{ KT0913_REG_DSPCFGA, 0x1000 },
+	/* FM AFC Enabled */
+	{ KT0913_REG_LOCFGA, 0x0100 },
+	/* Campus band disabled by default */
+	{ KT0913_REG_LOCFGC, 0x0024 },
+	/*
+	 * FM mode, internal defined bands, clock from XT, 32.768kHz
+	 * 3dB audio gain, AM AFC Enabled
+	 */
+	{ KT0913_REG_AMSYSCFG, 0x0002 },
+	/* Default AM freq = 504kHz */
+	{ KT0913_REG_AMCHAN, 0x01f8},
+	/* VOL and CH GPIOs set to HiZ */
+	{ KT0913_REG_GPIOCFG, 0x0000 },
+	/* AM Channel bandwidth = 6kHz, non-differential output */
+	{ KT0913_REG_AMDSP, 0xafc4 },
+	/*
+	 * softmute is disabled on AM and FM, but set the defaults:
+	 * strong softmute attn., slow softmute attack/recover,
+	 * lowest AM softumte start level, almost the minimum
+	 * softmute target volume, RSSI mode for softmute, lowest
+	 * FM softmute start level
+	 */
+	{ KT0913_REG_SOFTMUTE, 0x0010 },
+	/* 1kHz for AM channel space, working mode A for the keys */
+	{ KT0913_REG_AMCFG, 0x1401 },
+	/* TIME1 = shortest, TIME2 = fastest */
+	{ KT0913_REG_AMCFG, 0x4050 },
+	/* set 86MHz as the default frequency, and tune it */
+	{ KT0913_REG_TUNE, 0x86b8 },
+	/*
+	 * FM&AM Softmute disabled, Mute disabled, 75us deemp.,
+	 * no bass boost, 100uF anti pop cap
+	 */
+	{ KT0913_REG_VOLUME, 0xe080 },
+};
+
+static const struct regmap_config kt0913_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = KT0913_REG_AFC,
+	.volatile_table = &kt0913_all_registers_access_table,
+	.cache_type = REGCACHE_RBTREE,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
+/* ************************************************************************* */
+
+/* bands where the kt0913 operates */
+enum { BAND_FM, BAND_FM_CAMPUS, BAND_AM };
+
+static const struct v4l2_frequency_band kt0913_bands[] = {
+	{
+		/* BAND_FM */
+		.type = V4L2_TUNER_RADIO,
+		.index = 0, /* index provided to v4l2 */
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+				V4L2_TUNER_CAP_FREQ_BANDS,
+		.rangelow = KT0913_FM_RANGE_LOW_NO_CAMPUS * V4L2_KHZ_FREQ_MUL,
+		.rangehigh = KT0913_FM_RANGE_HIGH * V4L2_KHZ_FREQ_MUL,
+		.modulation = V4L2_BAND_MODULATION_FM,
+	},
+	{
+		/* BAND_FM_CAMPUS */
+		.type = V4L2_TUNER_RADIO,
+		.index = 0, /* index provided to v4l2 */
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+				V4L2_TUNER_CAP_FREQ_BANDS,
+		.rangelow = KT0913_FM_RANGE_LOW_CAMPUS * V4L2_KHZ_FREQ_MUL,
+		.rangehigh = KT0913_FM_RANGE_HIGH * V4L2_KHZ_FREQ_MUL,
+		.modulation = V4L2_BAND_MODULATION_FM,
+	},
+	{
+		/* BAND_AM */
+		.type = V4L2_TUNER_RADIO,
+		.index = 1, /* index provided to v4l2 */
+		.capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_FREQ_BANDS,
+		.rangelow = KT0913_AM_RANGE_LOW * V4L2_KHZ_FREQ_MUL,
+		.rangehigh = KT0913_AM_RANGE_HIGH * V4L2_KHZ_FREQ_MUL,
+		.modulation = V4L2_BAND_MODULATION_AM,
+	},
+};
+
+/* ************************************************************************* */
+
+static inline struct kt0913_device *
+v4l2_device_to_device(struct v4l2_device *v4l2_dev)
+{
+	return container_of(v4l2_dev,
+		struct kt0913_device, v4l2_dev);
+}
+
+static inline struct kt0913_device *
+v4l2_ctrl_to_device(struct v4l2_ctrl *ctrl_handler)
+{
+	return container_of(ctrl_handler->handler,
+		struct kt0913_device, ctrl_handler);
+}
+
+/* ************************************************************************* */
+
+static inline u32 khz_to_v4l2_freq(unsigned int freq)
+{
+	return freq * V4L2_KHZ_FREQ_MUL;
+}
+
+static inline unsigned int v4l2_freq_to_khz(u32 v4l2_freq)
+{
+	return v4l2_freq / V4L2_KHZ_FREQ_MUL;
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_fm_frequency(struct kt0913_device *radio,
+				     unsigned int *frequency)
+{
+	unsigned int tune_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_TUNE, &tune_reg);
+
+	if (ret)
+		return ret;
+
+	*frequency = (tune_reg & KT0913_TUNE_FMCHAN_MASK) * KT0913_FMCHAN_MUL;
+
+	return 0;
+}
+
+static int __kt0913_set_fm_frequency(struct kt0913_device *radio,
+				     unsigned int frequency)
+{
+	return regmap_write(radio->regmap, KT0913_REG_TUNE,
+		KT0913_TUNE_FMTUNE_ON | (frequency / KT0913_FMCHAN_MUL));
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_mute(struct kt0913_device *radio, int on)
+{
+	return regmap_update_bits(radio->regmap,
+		KT0913_REG_VOLUME, KT0913_VOLUME_DMUTE_MASK,
+		on ? KT0913_VOLUME_DMUTE_ON : KT0913_VOLUME_DMUTE_OFF);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_deemphasis(struct kt0913_device *radio, s32 deemp)
+{
+	switch (deemp) {
+	case V4L2_DEEMPHASIS_75_uS:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_VOLUME, KT0913_VOLUME_DE_MASK,
+			KT0913_VOLUME_DE_50US);
+
+		/* 50us is used for the disabled option (which is not supported
+		 * on the chip) and the 50uS value
+		 */
+	default:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_VOLUME, KT0913_VOLUME_DE_MASK,
+			KT0913_VOLUME_DE_75US);
+	}
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_volume(struct kt0913_device *radio, s32 volume)
+{
+	/* map [-60, 0] to [1, 31] which is what the kt0913 expects */
+	volume = (volume / 2) + 31;
+	return regmap_update_bits(radio->regmap,
+		KT0913_REG_RXCFG, KT0913_RXCFGA_VOLUME_MASK,
+		volume);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_standby(struct kt0913_device *radio, int standby)
+{
+	return regmap_update_bits(radio->regmap,
+		KT0913_REG_RXCFG, KT0913_RXCFGA_STDBY_MASK,
+		standby ? KT0913_RXCFGA_STDBY_ON : KT0913_RXCFGA_STDBY_OFF);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_pll_status(struct kt0913_device *radio, int *locked)
+{
+	unsigned int statusa_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_STATUSA, &statusa_reg);
+
+	if (ret)
+		return ret;
+
+	*locked = (statusa_reg & KT0913_STATUSA_PLL_LOCK_MASK) ==
+		KT0913_STATUSA_PLL_LOCK_LOCKED ? 1 : 0;
+
+	return 0;
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_rx_stereo_or_mono(struct kt0913_device *radio,
+					  int *stereo)
+{
+	unsigned int statusa_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_STATUSA, &statusa_reg);
+
+	if (ret)
+		return ret;
+
+	*stereo = (statusa_reg & KT0913_STATUSA_ST_MASK) ==
+		KT0913_STATUSA_ST_STEREO ? 1 : 0;
+
+	return 0;
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_fm_rssi(struct kt0913_device *radio, s32 *rssi)
+{
+	unsigned int statusa_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_STATUSA, &statusa_reg);
+
+	if (ret)
+		return ret;
+
+	/* RSSI(dBm) = -100 + FMRSSI<4:0> * 3dBm
+	 * even tho we can get the value in dBm, we want a %
+	 */
+	*rssi = (statusa_reg & KT0913_STATUSA_FMRSSI_MASK) >>
+		KT0913_STATUSA_FMRSSI_SHIFT;
+	/* map range 0-31 to 0-65535 */
+	*rssi *= 65535;
+	*rssi /= KT0913_STATUSA_FMRSSI_MASK >> KT0913_STATUSA_FMRSSI_SHIFT;
+
+	return 0;
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_cfg_stereo_enabled(struct kt0913_device *radio,
+					   int *stereo)
+{
+	unsigned int dspcfga_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_DSPCFGA, &dspcfga_reg);
+
+	if (ret)
+		return ret;
+
+	*stereo = (dspcfga_reg & KT0913_DSPCFGA_MONO_MASK) ==
+		KT0913_DSPCFGA_MONO_OFF ? 1 : 0;
+
+	return ret;
+}
+
+static int __kt0913_set_cfg_stereo_enabled(struct kt0913_device *radio,
+					   int stereo)
+{
+	return regmap_update_bits(radio->regmap,
+		KT0913_REG_DSPCFGA, KT0913_DSPCFGA_MONO_MASK,
+		stereo ? KT0913_DSPCFGA_MONO_OFF : KT0913_DSPCFGA_MONO_ON);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_au_gain(struct kt0913_device *radio, s32 gain)
+{
+	switch (gain) {
+	case 6:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
+			KT0913_AMSYSCFG_AU_GAIN_6DB);
+	case 3:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
+			KT0913_AMSYSCFG_AU_GAIN_3DB);
+	case 0:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
+			KT0913_AMSYSCFG_AU_GAIN_0DB);
+	case -3:
+		return regmap_update_bits(radio->regmap,
+			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
+			KT0913_AMSYSCFG_AU_GAIN_MIN_3DB);
+	default:
+		return -EINVAL;
+	}
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_set_am_fm_band(struct kt0913_device *radio,
+				   unsigned int band)
+{
+	return regmap_update_bits(radio->regmap,
+		KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AM_FM_MASK,
+		band == BAND_AM ?
+		KT0913_AMSYSCFG_AM_FM_AM : KT0913_AMSYSCFG_AM_FM_FM);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_am_frequency(struct kt0913_device *radio,
+				     unsigned int *frequency)
+{
+	unsigned int amchan_reg;
+	int ret = regmap_read(radio->regmap, KT0913_REG_AMCHAN, &amchan_reg);
+
+	if (ret)
+		return ret;
+
+	*frequency = (amchan_reg & KT0913_AMCHAN_AMCHAN_MASK);
+
+	return 0;
+}
+
+static int __kt0913_set_am_frequency(struct kt0913_device *radio,
+				     unsigned int frequency)
+{
+	return regmap_write(radio->regmap, KT0913_REG_AMCHAN,
+		KT0913_AMCHAN_AMTUNE_ON | frequency);
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_get_am_rssi(struct kt0913_device *radio, s32 *rssi)
+{
+	unsigned int amstatusa_reg;
+	int ret = regmap_read(radio->regmap,
+		KT0913_REG_AMSTATUSA, &amstatusa_reg);
+
+	if (ret)
+		return ret;
+
+	/* AMRSSI(dBm) = -90 + AMRSSI<4:0> * 3dBm
+	 * even tho we can get the value in dBm, we want a %
+	 */
+	*rssi = (amstatusa_reg & KT0913_AMSTATUSA_AMRSSI_MASK) >>
+		 KT0913_AMSTATUSA_AMRSSI_SHIFT;
+	/* map range 0-31 to 0-65535 */
+	*rssi *= 65535;
+	*rssi /= KT0913_AMSTATUSA_AMRSSI_MASK >> KT0913_AMSTATUSA_AMRSSI_SHIFT;
+
+	return 0;
+}
+
+/* ************************************************************************* */
+
+static int __kt0913_init(struct kt0913_device *radio)
+{
+	int ret = 0;
+
+	/* write the defaults */
+	ret = regmap_multi_reg_write(radio->regmap,
+				     kt0913_init_regs_to_defaults,
+				     ARRAY_SIZE(kt0913_init_regs_to_defaults));
+	if (ret) {
+		v4l2_err(radio->client,
+			 "regmap_multi_reg_write() failed! %d", ret);
+		return ret;
+	}
+
+	/* set the audio dac anti-pop config */
+	ret = regmap_update_bits(radio->regmap,
+				 KT0913_REG_VOLUME, KT0913_VOLUME_POP_MASK,
+				 radio->audio_anti_pop <<
+				 KT0913_VOLUME_POP_SHIFT);
+	if (ret) {
+		v4l2_err(radio->client,
+			 "regmap_update_bits() err on anti-pop cfg! %d", ret);
+		return ret;
+	}
+
+	/* set the reference clock config */
+	ret = regmap_update_bits(radio->regmap,
+				 KT0913_REG_AMSYSCFG,
+				 KT0913_AMSYSCFG_REFCLK_MASK,
+				 radio->refclock_val <<
+				 KT0913_AMSYSCFG_REFCLK_SHIFT);
+	if (ret) {
+		v4l2_err(radio->client,
+			 "regmap_update_bits() err on refclk cfg! %d", ret);
+		return ret;
+	}
+
+	if (kt0913_use_campus_band) {
+		v4l2_info(radio->client,
+			  "campus band is enabled!");
+		/* set the campus band bit */
+		ret = regmap_update_bits(radio->regmap,
+					 KT0913_REG_LOCFGC,
+					 KT0913_LOCFG_CAMPUSBAND_EN_MASK,
+					 KT0913_LOCFG_CAMPUSBAND_EN_ON);
+		if (ret) {
+			v4l2_err(radio->client,
+				 "regmap_update_bits() err on campus band! %d",
+				 ret);
+			return ret;
+		}
+	}
+
+	return __kt0913_set_mute(radio, true);
+}
+
+/* ************************************************************************* */
+
+static int kt0913_ioctl_vidioc_g_frequency(struct file *file, void *priv,
+					   struct v4l2_frequency *f)
+{
+	struct kt0913_device *radio = video_drvdata(file);
+	int ret;
+
+	if (f->tuner != 0)
+		return -EINVAL;
+
+	f->type = V4L2_TUNER_RADIO;
+
+	if (radio->band == BAND_AM)
+		ret = __kt0913_get_am_frequency(radio, &f->frequency);
+	else
+		ret = __kt0913_get_fm_frequency(radio, &f->frequency);
+
+	if (ret)
+		return ret;
+
+	/* convert kHz freq into v4l2 freq */
+	f->frequency = khz_to_v4l2_freq(f->frequency);
+
+	return 0;
+}
+
+static int kt0913_ioctl_vidioc_s_frequency(struct file *file, void *priv,
+					   const struct v4l2_frequency *f)
+{
+	struct kt0913_device *radio = video_drvdata(file);
+	unsigned int freq = f->frequency;
+	unsigned int new_band;
+	unsigned int half_gap_freq;
+	int ret;
+
+	if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
+		return -EINVAL;
+
+	if (freq == 0)
+		return -EINVAL;
+
+	/* calculate the middle frequency on the gap of the bands */
+	half_gap_freq = kt0913_bands[BAND_AM].rangehigh;
+
+	if (kt0913_use_campus_band)
+		/* using the AM band and the campus FM band if enabled */
+		half_gap_freq += kt0913_bands[BAND_FM_CAMPUS].rangelow;
+	else
+		/* or using the AM band and the standard FM band */
+		half_gap_freq += kt0913_bands[BAND_FM].rangelow;
+
+	half_gap_freq /= 2;
+
+	if (freq <= half_gap_freq)
+		new_band = BAND_AM;
+	else if (freq <= kt0913_bands[BAND_FM].rangelow &&
+		 kt0913_use_campus_band)
+		new_band = BAND_FM_CAMPUS;
+	else
+		new_band = BAND_FM;
+
+	/* is the requested band different than the one currently set? */
+	if (radio->band != new_band) {
+		/* update the band on the device */
+		ret = __kt0913_set_am_fm_band(radio, new_band);
+		if (ret)
+			return ret;
+		radio->band = new_band;
+	}
+
+	/* clamp the frequency to the band boundaries */
+	freq = clamp(freq, kt0913_bands[new_band].rangelow,
+		     kt0913_bands[new_band].rangehigh);
+
+	/* convert v4l2 freq to kHz */
+	freq = v4l2_freq_to_khz(freq);
+
+	if (radio->band == BAND_AM)
+		return __kt0913_set_am_frequency(radio, freq);
+	else
+		return __kt0913_set_fm_frequency(radio, freq);
+}
+
+static int kt0913_ioctl_vidioc_enum_freq_bands(struct file *file, void *priv,
+					       struct v4l2_frequency_band *band)
+{
+	if (band->tuner != 0)
+		return -EINVAL;
+
+	switch (band->index) {
+	case 0:
+		if (kt0913_use_campus_band)
+			*band = kt0913_bands[BAND_FM_CAMPUS];
+		else
+			*band = kt0913_bands[BAND_FM];
+		return 0;
+	case 1:
+		*band = kt0913_bands[BAND_AM];
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* ************************************************************************* */
+
+/* V4L2 vidioc */
+static int kt0913_ioctl_vidioc_querycap(struct file *file, void *priv,
+					struct v4l2_capability *capability)
+{
+	struct kt0913_device *radio = video_drvdata(file);
+	struct video_device *dev;
+
+	if (!radio)
+		return -ENODEV;
+
+	dev = &radio->vdev;
+
+	if (!dev)
+		return -ENODEV;
+
+	strscpy(capability->driver, KT0913_FM_AM_DRIVER_NAME,
+		sizeof(capability->driver));
+	strscpy(capability->card, dev->name, sizeof(capability->card));
+	snprintf(capability->bus_info, sizeof(capability->bus_info),
+		 "I2C:%s", dev_name(&dev->dev));
+	return 0;
+}
+
+static int kt0913_ioctl_vidioc_g_tuner(struct file *file, void *priv,
+				       struct v4l2_tuner *v)
+{
+	struct kt0913_device *radio = video_drvdata(file);
+	int ret;
+	int stereo_enabled;
+	int is_stereo;
+
+	if (v->index != 0)
+		return -EINVAL;
+
+	strscpy(v->name, "FM/AM", sizeof(v->name));
+	v->type = V4L2_TUNER_RADIO;
+
+	v->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO |
+		V4L2_TUNER_CAP_FREQ_BANDS;
+
+	v->rangelow = kt0913_bands[BAND_AM].rangelow;
+	v->rangehigh = kt0913_bands[BAND_FM].rangehigh;
+
+	if (radio->band == BAND_AM) {
+		v->rxsubchans = V4L2_TUNER_SUB_MONO;
+		v->audmode = V4L2_TUNER_MODE_MONO;
+
+		ret = __kt0913_get_am_rssi(radio, &v->signal);
+		if (ret)
+			return ret;
+	} else {
+		ret = __kt0913_get_cfg_stereo_enabled(radio, &stereo_enabled);
+		if (ret)
+			return ret;
+
+		v->rxsubchans = stereo_enabled ?
+			V4L2_TUNER_SUB_STEREO : V4L2_TUNER_SUB_MONO;
+
+		ret = __kt0913_get_rx_stereo_or_mono(radio, &is_stereo);
+		if (ret)
+			return ret;
+
+		v->audmode = is_stereo ?
+			V4L2_TUNER_MODE_STEREO : V4L2_TUNER_MODE_MONO;
+
+		ret = __kt0913_get_fm_rssi(radio, &v->signal);
+		if (ret)
+			return ret;
+	}
+
+	/* AFC is enabled and active by default */
+	v->afc = 1;
+
+	return 0;
+}
+
+static int kt0913_ioctl_vidioc_s_tuner(struct file *file, void *priv,
+				       const struct v4l2_tuner *v)
+{
+	struct kt0913_device *radio = video_drvdata(file);
+
+	if (v->index != 0)
+		return -EINVAL;
+
+	/* only mono and stereo are supported */
+	if (v->audmode != V4L2_TUNER_MODE_MONO &&
+	    v->audmode != V4L2_TUNER_MODE_STEREO)
+		return 0;
+
+	/* AM is mono only, so don't try to set it to stereo */
+	if (radio->band == BAND_AM && v->audmode != V4L2_TUNER_MODE_MONO)
+		return 0;
+
+	/* set to stereo if specified, otherwise set to mono */
+	return __kt0913_set_cfg_stereo_enabled(radio,
+		v->audmode == V4L2_TUNER_MODE_STEREO);
+}
+
+static int kt0913_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct kt0913_device *radio = v4l2_ctrl_to_device(ctrl);
+
+	switch (ctrl->id) {
+	case V4L2_CID_AUDIO_MUTE:
+		return __kt0913_set_mute(radio, ctrl->val);
+	case V4L2_CID_AUDIO_VOLUME:
+		return __kt0913_set_volume(radio, ctrl->val);
+	case V4L2_CID_GAIN:
+		return __kt0913_set_au_gain(radio, ctrl->val);
+	case V4L2_CID_TUNE_DEEMPHASIS:
+		return __kt0913_set_deemphasis(radio, ctrl->val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int kt0913_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct kt0913_device *radio = v4l2_ctrl_to_device(ctrl);
+
+	switch (ctrl->id) {
+	case V4L2_CID_RF_TUNER_PLL_LOCK:
+		return __kt0913_get_pll_status(radio, &ctrl->val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct v4l2_ctrl_ops kt0913_ctrl_ops = {
+	.s_ctrl = kt0913_s_ctrl,
+	.g_volatile_ctrl = kt0913_g_volatile_ctrl,
+};
+
+/* ************************************************************************* */
+
+/* File system interface (use the ancillary fops for v4l2) */
+static const struct v4l2_file_operations kt0913_radio_fops = {
+	.owner = THIS_MODULE,
+	.open = v4l2_fh_open,
+	.release = v4l2_fh_release,
+	.poll = v4l2_ctrl_poll,
+	.unlocked_ioctl = video_ioctl2,
+};
+
+/* ioctl ops */
+static const struct v4l2_ioctl_ops kt0913_ioctl_ops = {
+	.vidioc_querycap = kt0913_ioctl_vidioc_querycap,
+	.vidioc_g_tuner = kt0913_ioctl_vidioc_g_tuner,
+	.vidioc_s_tuner = kt0913_ioctl_vidioc_s_tuner,
+	.vidioc_g_frequency = kt0913_ioctl_vidioc_g_frequency,
+	.vidioc_s_frequency = kt0913_ioctl_vidioc_s_frequency,
+	.vidioc_enum_freq_bands = kt0913_ioctl_vidioc_enum_freq_bands,
+	/* use ancillary functions for these: */
+	.vidioc_log_status = v4l2_ctrl_log_status,
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+};
+
+/* V4L2 RADIO device structure */
+static struct video_device kt0913_radio_template = {
+	.name = KT0913_FM_AM_DRIVER_NAME,
+	.fops = &kt0913_radio_fops,
+	.ioctl_ops = &kt0913_ioctl_ops,
+	.release = video_device_release_empty,
+	.vfl_dir = VFL_DIR_RX,
+	.device_caps = V4L2_CAP_TUNER | V4L2_CAP_RADIO,
+};
+
+/* ************************************************************************* */
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id kt0913_of_match[] = {
+	{ .compatible = "ktm,kt0913" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, kt0913_of_match);
+#endif /* IS_ENABLED(CONFIG_OF) */
+
+static void __kt0913_parse_dt(struct kt0913_device *radio)
+{
+	const void *ptr_anti_pop = of_get_property(radio->client->dev.of_node,
+		"ktm,anti-pop", NULL);
+	const void *ptr_refclk = of_get_property(radio->client->dev.of_node,
+		"ktm,refclk", NULL);
+
+	if (ptr_anti_pop) {
+		radio->audio_anti_pop =
+			clamp(be32_to_cpup(ptr_anti_pop), 0U, 3U);
+	} else {
+		radio->audio_anti_pop = 0;
+		v4l2_warn(radio->client,
+			  "No ktm,anti-pop on dt node, using default");
+	}
+
+	if (ptr_refclk) {
+		radio->refclock_val =
+			clamp(be32_to_cpup(ptr_refclk), 0U, 9U);
+	} else {
+		radio->refclock_val = 0;
+		v4l2_warn(radio->client,
+			  "No ktm,refclk on dt node, using default");
+	}
+}
+
+/* ************************************************************************* */
+
+static int kt0913_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct kt0913_device *radio;
+	struct v4l2_device *v4l2_dev;
+	struct v4l2_ctrl_handler *hdl;
+	struct regmap *regmap;
+	int ret;
+
+	pr_debug("%s\n", __func__);
+
+	/* this driver uses word R/W i2c operations, check if it's supported */
+	ret = i2c_check_functionality(client->adapter,
+				      I2C_FUNC_SMBUS_READ_WORD_DATA |
+				      I2C_FUNC_SMBUS_WRITE_WORD_DATA);
+	if (!ret) {
+		v4l2_err(client,
+			 "I2C adapter doesn't support word operations");
+		return -EIO;
+	}
+
+	/* check if the device exist on the bus before initializing it */
+	ret = i2c_smbus_read_word_data(client, KT0913_REG_CHIP_ID);
+	if (ret < 0) {
+		v4l2_err(client,
+			 "Error reading CHIP ID of the kt0913 (%d)", ret);
+		return ret;
+	}
+
+	/* check if the CHIP ID register value matches the expected value */
+	if (ret != KT0913_CHIP_ID) {
+		v4l2_err(radio->client,
+			 "Invalid CHIP ID: 0x%x, expected 0x%x",
+			 ret, KT0913_CHIP_ID);
+		return -ENODEV;
+	}
+
+	v4l2_info(client,
+		  "kt0913 found @ 0x%x (%s)\n",
+		  client->addr, client->adapter->name);
+
+	/* alloc context for the kt0913 radio struct */
+	radio = devm_kzalloc(&client->dev, sizeof(*radio), GFP_KERNEL);
+	if (!radio)
+		return -ENOMEM;
+
+	v4l2_dev = &radio->v4l2_dev;
+	ret = v4l2_device_register(&client->dev, v4l2_dev);
+	if (ret < 0) {
+		v4l2_err(client,
+			 "could not register v4l2_dev\n");
+		goto errfr;
+	}
+
+	mutex_init(&radio->mutex);
+
+	/* register the control handler from the context struct */
+	hdl = &radio->ctrl_handler;
+	v4l2_ctrl_handler_init(hdl, 5);
+
+	/* add the control: Mute */
+	radio->ctrl_mute = v4l2_ctrl_new_std(hdl, &kt0913_ctrl_ops,
+					     V4L2_CID_AUDIO_MUTE,
+					     0, 1, 1, 0);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register control: mute\n");
+		goto errunreg;
+	}
+
+	/* add the control: Volume */
+	radio->ctrl_volume = v4l2_ctrl_new_std(hdl, &kt0913_ctrl_ops,
+					       V4L2_CID_AUDIO_VOLUME,
+					       -60, 0, 2, 0);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register control: Volume\n");
+		goto errunreg;
+	}
+
+	/* add the control: audio gain */
+	radio->ctrl_au_gain = v4l2_ctrl_new_std(hdl, &kt0913_ctrl_ops,
+						V4L2_CID_GAIN,
+						-3, 6, 3, 3);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register control: audio gain\n");
+		goto errunreg;
+	}
+	radio->ctrl_au_gain->flags |= V4L2_CTRL_FLAG_SLIDER;
+
+	/* add the control: PLL Lock */
+	radio->ctrl_pll_lock = v4l2_ctrl_new_std(hdl, &kt0913_ctrl_ops,
+						 V4L2_CID_RF_TUNER_PLL_LOCK,
+						 0, 1, 1, 0);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register control: pll lock\n");
+		goto errunreg;
+	}
+	radio->ctrl_pll_lock->flags |= (V4L2_CTRL_FLAG_VOLATILE |
+		V4L2_CTRL_FLAG_READ_ONLY);
+
+	/* add the control: deemphasis */
+	radio->ctrl_deemp = v4l2_ctrl_new_std_menu(hdl, &kt0913_ctrl_ops,
+						   V4L2_CID_TUNE_DEEMPHASIS,
+						   V4L2_DEEMPHASIS_75_uS,
+						   0,
+						   V4L2_DEEMPHASIS_75_uS);
+	if (hdl->error) {
+		ret = hdl->error;
+		v4l2_err(v4l2_dev, "Could not register control: deemphasis\n");
+		goto errunreg;
+	}
+	/* the control handler is ready to be used */
+	v4l2_dev->ctrl_handler = hdl;
+
+	radio->vdev = kt0913_radio_template;
+	radio->vdev.lock = &radio->mutex;
+	radio->vdev.v4l2_dev = v4l2_dev;
+	video_set_drvdata(&radio->vdev, radio);
+
+	radio->client = client;
+	i2c_set_clientdata(client, radio);
+
+	/* init the regmap of the kt0913 */
+	regmap = devm_regmap_init_i2c(client, &kt0913_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		v4l2_err(client,
+			 "devm_regmap_init_i2c() failed! %d", ret);
+		goto errunreg;
+	}
+	radio->regmap = regmap;
+
+	__kt0913_parse_dt(radio);
+
+	/* init the kt0913 into a known state */
+	ret = __kt0913_init(radio);
+	if (ret) {
+		v4l2_err(client,
+			 "__kt0913_init() failed! %d", ret);
+		goto errunreg;
+	}
+
+	pm_runtime_get_noresume(&client->dev);
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_dont_use_autosuspend(&client->dev);
+
+	ret = video_register_device(&radio->vdev,
+				    VFL_TYPE_RADIO, kt0913_v4l2_radio_nr);
+	if (ret < 0) {
+		v4l2_err(client,
+			 "Could not register video device!");
+		goto error_pm_disable;
+	}
+
+	v4l2_info(client, "registered.");
+	return 0;
+error_pm_disable:
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+errunreg:
+	v4l2_ctrl_handler_free(hdl);
+	v4l2_device_unregister(v4l2_dev);
+errfr:
+	__kt0913_set_standby(radio, true);
+	kfree(radio);
+	return ret;
+}
+
+static int kt0913_remove(struct i2c_client *client)
+{
+	struct kt0913_device *radio = i2c_get_clientdata(client);
+
+	pr_debug("%s\n", __func__);
+	if (!radio)
+		return -EINVAL;
+
+	__kt0913_set_standby(radio, true);
+
+	pm_runtime_get_sync(&client->dev);
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+	pm_runtime_put_noidle(&client->dev);
+
+	video_unregister_device(&radio->vdev);
+	v4l2_ctrl_handler_free(&radio->ctrl_handler);
+	v4l2_device_unregister(&radio->v4l2_dev);
+
+	v4l2_info(client, "removed.");
+	return 0;
+}
+
+/* ************************************************************************* */
+
+#ifdef CONFIG_PM
+static int kt0913_i2c_pm_runtime_suspend(struct device *dev)
+{
+	struct kt0913_device *radio = i2c_get_clientdata(to_i2c_client(dev));
+
+	pr_debug("%s\n", __func__);
+	if (!radio)
+		return 0;
+
+	return __kt0913_set_standby(radio, true);
+}
+
+static int kt0913_i2c_pm_runtime_resume(struct device *dev)
+{
+	struct kt0913_device *radio = i2c_get_clientdata(to_i2c_client(dev));
+
+	pr_debug("%s\n", __func__);
+	if (!radio)
+		return 0;
+
+	return __kt0913_set_standby(radio, false);
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops kt0913_i2c_pm_ops = {
+	SET_RUNTIME_PM_OPS(kt0913_i2c_pm_runtime_suspend,
+			   kt0913_i2c_pm_runtime_resume, NULL)
+};
+
+static const struct i2c_device_id kt0913_idtable[] = {
+	{ "kt0913", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, kt0913_idtable);
+
+static struct i2c_driver kt0913_driver = {
+	.driver = {
+		.name = "kt0913",
+		.of_match_table = of_match_ptr(kt0913_of_match),
+		.pm = &kt0913_i2c_pm_ops,
+	},
+	.probe = kt0913_probe,
+	.remove = kt0913_remove,
+	.id_table = kt0913_idtable,
+};
+module_i2c_driver(kt0913_driver);
+
+MODULE_AUTHOR("Santiago Hormazabal <santiagohssl@gmail.com>");
+MODULE_DESCRIPTION("KTMicro KT0913 AM/FM receiver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("0.0.3");
+
+module_param(kt0913_use_campus_band, int, 0);
+MODULE_PARM_DESC(kt0913_use_campus_band, "Use the Campus Band feature (FM range 32MHz-110MHz)");
+module_param(kt0913_v4l2_radio_nr, int, 0);
+MODULE_PARM_DESC(kt0913_v4l2_radio_nr, "v4l2 device number to use (i.e. /dev/radioX)");
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] media: kt0913: device tree binding
  2020-08-31 22:05 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
  2020-08-31 22:05 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
  2020-08-31 22:06 ` [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from " Santiago Hormazabal
@ 2020-08-31 22:06 ` Santiago Hormazabal
  2020-09-03 16:11   ` Rob Herring
  2 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-08-31 22:06 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel
  Cc: Santiago Hormazabal

Document bindings for the kt0913 AM/FM radio tuner.

Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
---
 .../bindings/media/i2c/ktm,kt0913.yaml        | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml b/Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
new file mode 100644
index 000000000000..2c3d1795da43
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020 Santiago Hormazabal <santiagohssl@gmail.com>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ktm,kt0913.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device-Tree bindings for the KTMicro KT0913 FM/AM radio tuner.
+
+maintainers:
+  - Santiago Hormazabal <santiagohssl@gmail.com>
+
+description: |-
+  The KT0913 is a low cost, low components FM/AM radio chip.
+  It uses the I2C protocol for operation.
+
+properties:
+  compatible:
+    const: ktm,kt0913
+
+  reg:
+    description: I2C device address
+    const: 0x35
+
+  ktm,anti-pop:
+    description:  |
+      Selects the DAC Anti-Pop capacitor. Possible values are
+      0 thru 3, which corresponds to 100uF (default), 60uF, 20uF or 10uF.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    enum: [0, 1, 2, 3]
+
+  ktm,refclk:
+    description:  |
+      Selects the reference clock used on the KT0913. Possible
+      values are 0 thru 9, which corresponds to 32.768kHz (default),
+      6.5MHz, 7.6MHz, 12MHz, 13MHz, 15.2MHz, 19.2MHz, 24MHz, 26MHz, 38kHz.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        kt0913: fm-am-tuner@35 {
+            compatible = "ktm,kt0913";
+            reg = <0x35>;
+            ktm,anti-pop = <0x01>;
+            ktm,refclk = <0x00>;
+        };
+    };
+...
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] media: kt0913: device tree binding
  2020-08-31 22:06 ` [PATCH 3/3] media: kt0913: device tree binding Santiago Hormazabal
@ 2020-09-03 16:11   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-09-03 16:11 UTC (permalink / raw)
  To: Santiago Hormazabal
  Cc: Rob Herring, linux-media, Ezequiel Garcia, Hans Verkuil,
	Mauro Carvalho Chehab, devicetree, linux-kernel

On Mon, 31 Aug 2020 19:06:01 -0300, Santiago Hormazabal wrote:
> Document bindings for the kt0913 AM/FM radio tuner.
> 
> Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
> ---
>  .../bindings/media/i2c/ktm,kt0913.yaml        | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

./Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/media/i2c/ktm,kt0913.yaml#
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dts:22.17-30: Warning (reg_format): /example-0/i2c/fm-am-tuner@35:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dts:19.13-26.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dts:19.13-26.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dts:20.36-25.15: Warning (avoid_default_addr_size): /example-0/i2c/fm-am-tuner@35: Relying on default #address-cells value
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dts:20.36-25.15: Warning (avoid_default_addr_size): /example-0/i2c/fm-am-tuner@35: Relying on default #size-cells value
Documentation/devicetree/bindings/media/i2c/ktm,kt0913.example.dt.yaml: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size'


See https://patchwork.ozlabs.org/patch/1354644

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-08-31 22:06 ` [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from " Santiago Hormazabal
@ 2020-09-03 16:45   ` Joe Perches
  2020-09-03 19:51     ` Santiago Hormazabal
  2020-09-09 14:06   ` Hans Verkuil
  1 sibling, 1 reply; 11+ messages in thread
From: Joe Perches @ 2020-09-03 16:45 UTC (permalink / raw)
  To: Santiago Hormazabal, linux-media, devicetree, Rob Herring,
	Ezequiel Garcia, Hans Verkuil, Mauro Carvalho Chehab,
	linux-kernel

On Mon, 2020-08-31 at 19:06 -0300, Santiago Hormazabal wrote:
> This chip requires almost no support components and can used over I2C.
> The driver uses the I2C bus and exposes the controls as a V4L2 radio.
> Tested with a module that contains this chip (from SZZSJDZ.com,
> part number ZJ-801B, even tho the company seems defunct now), and an H2+
> AllWinner SoC running a kernel built off 07d999f of the media_tree.

Thanks.

style trivia:

[]
> diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
[]
> +static const struct reg_sequence kt0913_init_regs_to_defaults[] = {
> +	/* Standby disabled, volume 0dB */
> +	{ KT0913_REG_RXCFG, 0x881f },

These might be more legible on single lines,
ignoring the 80 column limits.

> +	/* FM Channel spacing = 50kHz, Right & Left unmuted */
> +	{ KT0913_REG_SEEK, 0x000b },

etc...

[]

> +static int __kt0913_set_fm_frequency(struct kt0913_device *radio,
> +				     unsigned int frequency)
> +{
> +	return regmap_write(radio->regmap, KT0913_REG_TUNE,
> +		KT0913_TUNE_FMTUNE_ON | (frequency / KT0913_FMCHAN_MUL));

It might be nicer to align multi-line statements to the
open parenthesis.

[]

> +static int __kt0913_set_au_gain(struct kt0913_device *radio, s32 gain)
> +{
> +	switch (gain) {
> +	case 6:
> +		return regmap_update_bits(radio->regmap,
> +			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> +			KT0913_AMSYSCFG_AU_GAIN_6DB);
> +	case 3:
> +		return regmap_update_bits(radio->regmap,
> +			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> +			KT0913_AMSYSCFG_AU_GAIN_3DB);
> +	case 0:
> +		return regmap_update_bits(radio->regmap,
> +			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> +			KT0913_AMSYSCFG_AU_GAIN_0DB);
> +	case -3:
> +		return regmap_update_bits(radio->regmap,
> +			KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> +			KT0913_AMSYSCFG_AU_GAIN_MIN_3DB);
> +	default:
> +		return -EINVAL;
> +	}
> +}

It's generally more legible to write this with an intermediate
variable holding the changed value.  It's also most commonly
smaller object code.

static int __kt0913_set_au_gain(struct kt0913_device *radio, s32 gain)
{
	int val;

	switch (gain) {
	case 6:
		val = KT0913_AMSYSCFG_AU_GAIN_6DB;
		break;
	case 3:
		val = KT0913_AMSYSCFG_AU_GAIN_3DB;
		break;
	case 0:
		val = KT0913_AMSYSCFG_AU_GAIN_0DB;
		break;
	case -3:
		val = KT0913_AMSYSCFG_AU_GAIN_MIN_3DB;
		break;
	default:
		return -EINVAL;
	}

	return regmap_update_bits(radio->regmap, KT0913_REG_AMSYSCFG,
				  KT0913_AMSYSCFG_AU_GAIN_MASK, val);
}



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-09-03 16:45   ` Joe Perches
@ 2020-09-03 19:51     ` Santiago Hormazabal
  0 siblings, 0 replies; 11+ messages in thread
From: Santiago Hormazabal @ 2020-09-03 19:51 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel

Hi Joe,
Thanks for the feedback.

On Thu, 3 Sep 2020 at 13:45, Joe Perches <joe@perches.com> wrote:
>
> On Mon, 2020-08-31 at 19:06 -0300, Santiago Hormazabal wrote:
> > This chip requires almost no support components and can used over I2C.
> > The driver uses the I2C bus and exposes the controls as a V4L2 radio.
> > Tested with a module that contains this chip (from SZZSJDZ.com,
> > part number ZJ-801B, even tho the company seems defunct now), and an H2+
> > AllWinner SoC running a kernel built off 07d999f of the media_tree.
>
> Thanks.
>
> style trivia:
>
> []
> > diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
> []
> > +static const struct reg_sequence kt0913_init_regs_to_defaults[] = {
> > +     /* Standby disabled, volume 0dB */
> > +     { KT0913_REG_RXCFG, 0x881f },
>
> These might be more legible on single lines,
> ignoring the 80 column limits.
>
> > +     /* FM Channel spacing = 50kHz, Right & Left unmuted */
> > +     { KT0913_REG_SEEK, 0x000b },
>
> etc...
>
I agree, didn't know I could exceed the limit in these situations.

> []
>
> > +static int __kt0913_set_fm_frequency(struct kt0913_device *radio,
> > +                                  unsigned int frequency)
> > +{
> > +     return regmap_write(radio->regmap, KT0913_REG_TUNE,
> > +             KT0913_TUNE_FMTUNE_ON | (frequency / KT0913_FMCHAN_MUL));
>
> It might be nicer to align multi-line statements to the
> open parenthesis.
>
Yes, that's totally true. What about these cases where the other part
of the lines would exceed 80 chars? For instance, if I align the
second line to the open parenthesis of the first line, I'll be past
the 80 chars limit. Splitting it back again to fit that would make the
code not so much readable.

> []
>
> > +static int __kt0913_set_au_gain(struct kt0913_device *radio, s32 gain)
> > +{
> > +     switch (gain) {
> > +     case 6:
> > +             return regmap_update_bits(radio->regmap,
> > +                     KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> > +                     KT0913_AMSYSCFG_AU_GAIN_6DB);
> > +     case 3:
> > +             return regmap_update_bits(radio->regmap,
> > +                     KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> > +                     KT0913_AMSYSCFG_AU_GAIN_3DB);
> > +     case 0:
> > +             return regmap_update_bits(radio->regmap,
> > +                     KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> > +                     KT0913_AMSYSCFG_AU_GAIN_0DB);
> > +     case -3:
> > +             return regmap_update_bits(radio->regmap,
> > +                     KT0913_REG_AMSYSCFG, KT0913_AMSYSCFG_AU_GAIN_MASK,
> > +                     KT0913_AMSYSCFG_AU_GAIN_MIN_3DB);
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +}
>
> It's generally more legible to write this with an intermediate
> variable holding the changed value.  It's also most commonly
> smaller object code.
>
> static int __kt0913_set_au_gain(struct kt0913_device *radio, s32 gain)
> {
>         int val;
>
>         switch (gain) {
>         case 6:
>                 val = KT0913_AMSYSCFG_AU_GAIN_6DB;
>                 break;
>         case 3:
>                 val = KT0913_AMSYSCFG_AU_GAIN_3DB;
>                 break;
>         case 0:
>                 val = KT0913_AMSYSCFG_AU_GAIN_0DB;
>                 break;
>         case -3:
>                 val = KT0913_AMSYSCFG_AU_GAIN_MIN_3DB;
>                 break;
>         default:
>                 return -EINVAL;
>         }
>
>         return regmap_update_bits(radio->regmap, KT0913_REG_AMSYSCFG,
>                                   KT0913_AMSYSCFG_AU_GAIN_MASK, val);
> }
>
>
I agree, thanks for your feedback.

I'll wait for your reply to fix the other issue you've mentioned, and
I'll fix the others in the meantime.

Thanks!

- Santiago H.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-08-31 22:06 ` [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from " Santiago Hormazabal
  2020-09-03 16:45   ` Joe Perches
@ 2020-09-09 14:06   ` Hans Verkuil
  1 sibling, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2020-09-09 14:06 UTC (permalink / raw)
  To: Santiago Hormazabal, linux-media, devicetree, Rob Herring,
	Ezequiel Garcia, Mauro Carvalho Chehab, linux-kernel

Hi Santiago,

On 01/09/2020 00:06, Santiago Hormazabal wrote:
> This chip requires almost no support components and can used over I2C.
> The driver uses the I2C bus and exposes the controls as a V4L2 radio.
> Tested with a module that contains this chip (from SZZSJDZ.com,
> part number ZJ-801B, even tho the company seems defunct now), and an H2+
> AllWinner SoC running a kernel built off 07d999f of the media_tree.
> 
> Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
> ---
>  drivers/media/radio/Kconfig        |   10 +
>  drivers/media/radio/Makefile       |    1 +
>  drivers/media/radio/radio-kt0913.c | 1196 ++++++++++++++++++++++++++++
>  3 files changed, 1207 insertions(+)
>  create mode 100644 drivers/media/radio/radio-kt0913.c

One more thing: you need to add an entry to the MAINTAINERS file for this
driver. It can either be part of this patch, or added in a separate patch.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro
  2020-08-31 22:05 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
@ 2020-09-14 18:27   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-09-14 18:27 UTC (permalink / raw)
  To: Santiago Hormazabal
  Cc: Hans Verkuil, Rob Herring, devicetree, Ezequiel Garcia,
	linux-media, linux-kernel, Mauro Carvalho Chehab

On Mon, 31 Aug 2020 19:05:59 -0300, Santiago Hormazabal wrote:
> Adds ktm as the prefix of KT Micro, Inc.
> 
> Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] KT0913 FM/AM driver
  2020-08-03  2:09 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
@ 2020-08-03 12:17 ` Ezequiel Garcia
  0 siblings, 0 replies; 11+ messages in thread
From: Ezequiel Garcia @ 2020-08-03 12:17 UTC (permalink / raw)
  To: Santiago Hormazabal, linux-media, devicetree, Rob Herring,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel

Hello Santiago,

Nice work and welcome to the kernel community.

On Sun, 2020-08-02 at 23:09 -0300, Santiago Hormazabal wrote:
> media: adds support for kt0913 FM/AM tuner chip
> 

I don't think this line above should be there.

Also, this seems to be v2. You are missing a "PATCH v2" prefix
on the patches subject, and a v2 changelog on the cover letter.

Some examples for you to look at:

https://patchwork.linuxtv.org/project/linux-media/cover/BN6PR04MB06603B2CD7F2C56B322AF882A3710@BN6PR04MB0660.namprd04.prod.outlook.com/

https://patchwork.linuxtv.org/project/linux-media/cover/20200717145324.292820-1-jacopo+renesas@jmondi.org/

The vN+1 subject makes it easier for reviewers to keep
track of submissions, and the changelog makes it easier
to track the changes.

Thanks!
Ezequiel
 
> Adds a driver for the KT0913 FM/AM tuner chip from KT Micro. This chip
> can be found on many low cost FM/AM radios and DVD/Home Theaters.
> The chip provides two ways of usage, a manual mode (requiring only a
> few buttons) or complete control via I2C. This driver uses the latter.
> It exposes the minimum functionality of this chip, which includes tuning
> an AM or FM station given its frequency, reading the signal strength,
> setting Stereo (only on FM) or Mono (available on AM/FM), Mute, Volume
> and Audio Gain.
> I left some TODOs on the code, like supporting the chip's hardware seek
> feature, using a RW/RO regmaps rather than a single volatile regmap,
> show the FM SNR as a RO control and the FM/AM AFC deviation as another
> RO control.
> The module I've used comes from SZZSJDZ.com, a now defunct company.
> However, it's possible to buy this chip directly from Aliexpress or
> similar sites.
> I tested this on two systems, the first one being a Raspberry Pi 4 with
> the unstable 5.x kernel, but later I moved to a Banana Pi 2 Zero where
> I used the (current at this time, 8f2a4a9) master of this repo for testing.
> I've also compiled the v4l-compliance from sources (c7f0328) and it passed
> all the tests. The output of that is at the end of this note.
> 
> Note: This is the second set of patches for the driver, where I (tried to)
> address the comments that the reviewers added on the previous set.
> 
> v4l2-compliance SHA: c7f03287bbd64c168975e7ff3192e6fd3b507686, 32 bits, 32-bit time_t
> 
> Compliance test for kt0913-fm-am device /dev/radio0:
> 
> Driver Info:
> 	Driver name      : kt0913-fm-am
> 	Card type        : kt0913-fm-am
> 	Bus info         : I2C:radio0
> 	Driver version   : 5.8.0
> 	Capabilities     : 0x80250000
> 		Tuner
> 		Radio
> 		Extended Pix Format
> 		Device Capabilities
> 	Device Caps      : 0x00250000
> 		Tuner
> 		Radio
> 		Extended Pix Format
> 
> Required ioctls:
> 	test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
> 	test second /dev/radio0 open: OK
> 	test VIDIOC_QUERYCAP: OK
> 	test VIDIOC_G/S_PRIORITY: OK
> 	test for unlimited opens: OK
> 
> 	test invalid ioctls: OK
> Debug ioctls:
> 	test VIDIOC_DBG_G/S_REGISTER: OK
> 	test VIDIOC_LOG_STATUS: OK
> 
> Input ioctls:
> 	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK
> 	test VIDIOC_G/S_FREQUENCY: OK
> 	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> 	test VIDIOC_ENUMAUDIO: OK (Not Supported)
> 	test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> 	test VIDIOC_G/S_AUDIO: OK (Not Supported)
> 	Inputs: 0 Audio Inputs: 0 Tuners: 1
> 
> Output ioctls:
> 	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> 	test VIDIOC_G/S_FREQUENCY: OK
> 	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> 	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> 	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> 	Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
> 	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> 	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> 	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> 	test VIDIOC_G/S_EDID: OK (Not Supported)
> 
> Control ioctls:
> 	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> 	test VIDIOC_QUERYCTRL: OK
> 	test VIDIOC_G/S_CTRL: OK
> 	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> 	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> 	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> 	Standard Controls: 8 Private Controls: 0
> 
> Format ioctls:
> 	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
> 	test VIDIOC_G/S_PARM: OK (Not Supported)
> 	test VIDIOC_G_FBUF: OK (Not Supported)
> 	test VIDIOC_G_FMT: OK (Not Supported)
> 	test VIDIOC_TRY_FMT: OK (Not Supported)
> 	test VIDIOC_S_FMT: OK (Not Supported)
> 	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> 	test Cropping: OK (Not Supported)
> 	test Composing: OK (Not Supported)
> 	test Scaling: OK (Not Supported)
> 
> Codec ioctls:
> 	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> 	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> 	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls:
> 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
> 	test VIDIOC_EXPBUF: OK (Not Supported)
> 	test Requests: OK (Not Supported)
> 
> Total for kt0913-fm-am device /dev/radio0: 45, Succeeded: 45, Failed: 0, Warnings: 0
> 
> Santiago Hormazabal (3):
>   dt-bindings: vendor-prefixes: Add KT Micro
>   media: kt0913: device tree binding
>   media: Add support for the AM/FM radio chip KT0913 from KT Micro.
> 
>  .../bindings/media/i2c/ktm,kt0913.yaml        |   56 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
>  drivers/media/radio/Kconfig                   |   10 +
>  drivers/media/radio/Makefile                  |    1 +
>  drivers/media/radio/radio-kt0913.c            | 1196 +++++++++++++++++
>  5 files changed, 1265 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
>  create mode 100644 drivers/media/radio/radio-kt0913.c
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 0/3] KT0913 FM/AM driver
@ 2020-08-03  2:09 Santiago Hormazabal
  2020-08-03 12:17 ` Ezequiel Garcia
  0 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-08-03  2:09 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, Mauro Carvalho Chehab, linux-kernel
  Cc: Santiago Hormazabal

media: adds support for kt0913 FM/AM tuner chip

Adds a driver for the KT0913 FM/AM tuner chip from KT Micro. This chip
can be found on many low cost FM/AM radios and DVD/Home Theaters.
The chip provides two ways of usage, a manual mode (requiring only a
few buttons) or complete control via I2C. This driver uses the latter.
It exposes the minimum functionality of this chip, which includes tuning
an AM or FM station given its frequency, reading the signal strength,
setting Stereo (only on FM) or Mono (available on AM/FM), Mute, Volume
and Audio Gain.
I left some TODOs on the code, like supporting the chip's hardware seek
feature, using a RW/RO regmaps rather than a single volatile regmap,
show the FM SNR as a RO control and the FM/AM AFC deviation as another
RO control.
The module I've used comes from SZZSJDZ.com, a now defunct company.
However, it's possible to buy this chip directly from Aliexpress or
similar sites.
I tested this on two systems, the first one being a Raspberry Pi 4 with
the unstable 5.x kernel, but later I moved to a Banana Pi 2 Zero where
I used the (current at this time, 8f2a4a9) master of this repo for testing.
I've also compiled the v4l-compliance from sources (c7f0328) and it passed
all the tests. The output of that is at the end of this note.

Note: This is the second set of patches for the driver, where I (tried to)
address the comments that the reviewers added on the previous set.

v4l2-compliance SHA: c7f03287bbd64c168975e7ff3192e6fd3b507686, 32 bits, 32-bit time_t

Compliance test for kt0913-fm-am device /dev/radio0:

Driver Info:
	Driver name      : kt0913-fm-am
	Card type        : kt0913-fm-am
	Bus info         : I2C:radio0
	Driver version   : 5.8.0
	Capabilities     : 0x80250000
		Tuner
		Radio
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x00250000
		Tuner
		Radio
		Extended Pix Format

Required ioctls:
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/radio0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

	test invalid ioctls: OK
Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK
	test VIDIOC_LOG_STATUS: OK

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK
	test VIDIOC_G/S_FREQUENCY: OK
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 0 Audio Inputs: 0 Tuners: 1

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
	test VIDIOC_QUERYCTRL: OK
	test VIDIOC_G/S_CTRL: OK
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 8 Private Controls: 0

Format ioctls:
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
	test VIDIOC_G_FMT: OK (Not Supported)
	test VIDIOC_TRY_FMT: OK (Not Supported)
	test VIDIOC_S_FMT: OK (Not Supported)
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK (Not Supported)

Codec ioctls:
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
	test VIDIOC_EXPBUF: OK (Not Supported)
	test Requests: OK (Not Supported)

Total for kt0913-fm-am device /dev/radio0: 45, Succeeded: 45, Failed: 0, Warnings: 0

Santiago Hormazabal (3):
  dt-bindings: vendor-prefixes: Add KT Micro
  media: kt0913: device tree binding
  media: Add support for the AM/FM radio chip KT0913 from KT Micro.

 .../bindings/media/i2c/ktm,kt0913.yaml        |   56 +
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 drivers/media/radio/Kconfig                   |   10 +
 drivers/media/radio/Makefile                  |    1 +
 drivers/media/radio/radio-kt0913.c            | 1196 +++++++++++++++++
 5 files changed, 1265 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ktm,kt0913.yaml
 create mode 100644 drivers/media/radio/radio-kt0913.c

-- 
2.24.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-09-14 18:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31 22:05 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
2020-08-31 22:05 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
2020-09-14 18:27   ` Rob Herring
2020-08-31 22:06 ` [PATCH 2/3] media: Add support for the AM/FM radio chip KT0913 from " Santiago Hormazabal
2020-09-03 16:45   ` Joe Perches
2020-09-03 19:51     ` Santiago Hormazabal
2020-09-09 14:06   ` Hans Verkuil
2020-08-31 22:06 ` [PATCH 3/3] media: kt0913: device tree binding Santiago Hormazabal
2020-09-03 16:11   ` Rob Herring
  -- strict thread matches above, loose matches on Subject: below --
2020-08-03  2:09 [PATCH 0/3] KT0913 FM/AM driver Santiago Hormazabal
2020-08-03 12:17 ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).