linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] KT091 FM/AM driver
@ 2020-07-17  0:44 Santiago Hormazabal
  2020-07-17  0:44 ` [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-07-17  0:44 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, 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
is 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.
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) master of this repo for testing.
I've also compiled the v4l-compliance from sources and it passed all the
tests. The output of that is at the end of this note.

v4l2-compliance SHA: e50041186be9f69dd94b64fb924115201726e72a, 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/radio-kt0913.c            | 1181 +++++++++++++++++
 3 files changed, 1239 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-07-17  0:44 [PATCH 0/3] KT091 FM/AM driver Santiago Hormazabal
@ 2020-07-17  0:44 ` Santiago Hormazabal
  2020-07-23 20:51   ` Rob Herring
  2020-07-17  0:44 ` [PATCH 2/3] media: kt0913: device tree binding Santiago Hormazabal
  2020-07-17  0:44 ` [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro Santiago Hormazabal
  2 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-07-17  0:44 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, 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 9aeab66be85f..35985934a672 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -545,6 +545,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: kt0913: device tree binding
  2020-07-17  0:44 [PATCH 0/3] KT091 FM/AM driver Santiago Hormazabal
  2020-07-17  0:44 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
@ 2020-07-17  0:44 ` Santiago Hormazabal
  2020-07-23 20:51   ` Rob Herring
  2020-07-17  0:44 ` [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro Santiago Hormazabal
  2 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-07-17  0:44 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, 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

* [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17  0:44 [PATCH 0/3] KT091 FM/AM driver Santiago Hormazabal
  2020-07-17  0:44 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
  2020-07-17  0:44 ` [PATCH 2/3] media: kt0913: device tree binding Santiago Hormazabal
@ 2020-07-17  0:44 ` Santiago Hormazabal
  2020-07-17  9:29   ` Hans Verkuil
  2 siblings, 1 reply; 11+ messages in thread
From: Santiago Hormazabal @ 2020-07-17  0:44 UTC (permalink / raw)
  To: linux-media, devicetree, Rob Herring, Ezequiel Garcia,
	Hans Verkuil, 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) and a H2+ AllWinner SoC running the master
(at this time) of the media_tree.

Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
---
 drivers/media/radio/radio-kt0913.c | 1181 ++++++++++++++++++++++++++++
 1 file changed, 1181 insertions(+)
 create mode 100644 drivers/media/radio/radio-kt0913.c

diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
new file mode 100644
index 000000000000..9350d1764d44
--- /dev/null
+++ b/drivers/media/radio/radio-kt0913.c
@@ -0,0 +1,1181 @@
+// 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.
+ */
+
+#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-fm-am"
+
+/* ************************************************************************* */
+
+/* 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_deemphasis;  /* 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_CAMUS, 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_CAMUS */
+		.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() failed for 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() failed for 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() failed for 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 = BAND_FM;
+	int ret;
+
+	if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
+		return -EINVAL;
+
+	if (freq == 0)
+		return -EINVAL;
+
+	/* check if the requested frequency is contained on the AM band */
+	if (freq <= kt0913_bands[BAND_AM].rangehigh)
+		new_band = BAND_AM;
+	else {
+		/* check if the requested frequency is contained on the FM band */
+		if (freq >= kt0913_bands[BAND_FM].rangelow)
+			new_band = BAND_FM;
+		/* check if the requested frequency is contained on the campus
+		 * FM band only if that feature was enabled
+		 */
+		else if (kt0913_use_campus_band &&
+			(freq >= kt0913_bands[BAND_FM_CAMUS].rangelow))
+			new_band = BAND_FM_CAMUS;
+		else {
+			v4l2_warn(radio->client,
+				"frequency out of allowed RF bands (%u kHz)",
+				v4l2_freq_to_khz(freq));
+			return -EINVAL;
+		}
+	}
+
+	/* is the requested band different than the one currently set? */
+	if (radio->band != new_band) {
+		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_CAMUS];
+		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 */
+	if (!i2c_check_functionality(client->adapter,
+		I2C_FUNC_SMBUS_READ_WORD_DATA | I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
+		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_deemphasis = 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 },
+	{ }
+};
+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.1");
+
+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

* Re: [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17  0:44 ` [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro Santiago Hormazabal
@ 2020-07-17  9:29   ` Hans Verkuil
  2020-07-17  9:51     ` Joe Perches
  2020-07-17 15:10     ` Santiago Hormazabal
  0 siblings, 2 replies; 11+ messages in thread
From: Hans Verkuil @ 2020-07-17  9:29 UTC (permalink / raw)
  To: Santiago Hormazabal, linux-media, devicetree, Rob Herring,
	Ezequiel Garcia, linux-kernel

Hi Santiago,

Nice, it's been a long time since I last received a new radio driver :-)

I have a bunch of comments below, nothing major.

Main thing you need to do is to run 'checkpatch --strict' over the patch
to fix codingstyle issues. And I realized that one corner of the API wasn't
checked by v4l2-compliance, so you need to get the latest version of
v4l2-compliance where I added new checks.

On 17/07/2020 02:44, 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,

I can't find that site, is the URL correct?

If you know of a cheap shield that has this chip then let me know, I might
just get one myself.

> part number ZJ-801B) and a H2+ AllWinner SoC running the master
> (at this time) of the media_tree.
> 
> Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
> ---
>  drivers/media/radio/radio-kt0913.c | 1181 ++++++++++++++++++++++++++++
>  1 file changed, 1181 insertions(+)
>  create mode 100644 drivers/media/radio/radio-kt0913.c
> 
> diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
> new file mode 100644
> index 000000000000..9350d1764d44
> --- /dev/null
> +++ b/drivers/media/radio/radio-kt0913.c
> @@ -0,0 +1,1181 @@
> +// 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.
> + */
> +
> +#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

It's standard linux codingstyle to use lowercase for hex numbers.
Can you change that throughout the source for the next version?

> +
> +/* 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-fm-am"
> +
> +/* ************************************************************************* */
> +
> +/* 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 */

These comments do not seem to be aligned. Can you fix that? It looks ugly.

> +
> +	/* 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_deemphasis;  /* 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_CAMUS, BAND_AM };

BAND_FM_CAMUS -> BAND_FM_CAMPUS

> +
> +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_CAMUS */

CAMUS -> 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)

Put the newline just before the function name, so:

static inline struct kt0913_device *
v4l2_device_to_device(struct v4l2_device *v4l2_dev)

It's easier to read that way.

> +{
> +	return container_of(v4l2_dev,
> +		struct kt0913_device, v4l2_dev);
> +}
> +
> +static inline struct kt0913_device *v4l2_ctrl_to_device(
> +	struct v4l2_ctrl *ctrl_handler)

Same here and perhaps elsewhere as well if this happens in other places.

> +{
> +	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)

Align this properly:

static int __kt0913_get_fm_frequency(struct kt0913_device *radio,
				     unsigned int *frequency)

I suspect you didn't run 'scripts/checkpatch.pl --strict' over these patches.
Please do, it helps avoid such issues.

> +{
> +	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:

This can just be an 'if'. A switch is overkill here.

> +		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() failed for 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() failed for 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() failed for 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 = BAND_FM;
> +	int ret;
> +
> +	if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
> +		return -EINVAL;
> +
> +	if (freq == 0)
> +		return -EINVAL;
> +
> +	/* check if the requested frequency is contained on the AM band */
> +	if (freq <= kt0913_bands[BAND_AM].rangehigh)
> +		new_band = BAND_AM;
> +	else {
> +		/* check if the requested frequency is contained on the FM band */
> +		if (freq >= kt0913_bands[BAND_FM].rangelow)
> +			new_band = BAND_FM;
> +		/* check if the requested frequency is contained on the campus
> +		 * FM band only if that feature was enabled
> +		 */
> +		else if (kt0913_use_campus_band &&
> +			(freq >= kt0913_bands[BAND_FM_CAMUS].rangelow))
> +			new_band = BAND_FM_CAMUS;

This seems a bit overly complex. Just do:

		new_band = kt0913_use_campus_band ? BAND_FM_CAMUS : BAND_FM;
		if (freq < kt0913_bands[new_band].rangelow)
			return -EINVAL;

> +		else {
> +			v4l2_warn(radio->client,
> +				"frequency out of allowed RF bands (%u kHz)",
> +				v4l2_freq_to_khz(freq));

This is wrong. It should clamp the frequency to the closest valid frequency
range and just continue.

Hmm, v4l2-compliance should have failed on that, but it doesn't do this check
when iterating over the frequency bands. That's a bug. I've fixed that in
v4l2-compliance, so if you do a git pull for the v4l-utils repo then the
v4l2-compliance utility should now fail on this.

> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* is the requested band different than the one currently set? */
> +	if (radio->band != new_band) {
> +		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_CAMUS];
> +		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 */
> +	if (!i2c_check_functionality(client->adapter,
> +		I2C_FUNC_SMBUS_READ_WORD_DATA | I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
> +		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) {

Don't check after each control,

> +		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_deemphasis = 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) {

It's sufficient to just do the check at the end. See e.g.
drivers/media/radio/si4713/si4713.c

> +		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 },
> +	{ }
> +};
> +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.1");
> +
> +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)");>

Regards,

	Hans

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

* Re: [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17  9:29   ` Hans Verkuil
@ 2020-07-17  9:51     ` Joe Perches
  2020-07-17 10:04       ` Hans Verkuil
  2020-07-17 15:10     ` Santiago Hormazabal
  1 sibling, 1 reply; 11+ messages in thread
From: Joe Perches @ 2020-07-17  9:51 UTC (permalink / raw)
  To: Hans Verkuil, Santiago Hormazabal, linux-media, devicetree,
	Rob Herring, Ezequiel Garcia, linux-kernel

On Fri, 2020-07-17 at 11:29 +0200, Hans Verkuil wrote:
> It's standard linux codingstyle to use lowercase for hex numbers.
> Can you change that throughout the source for the next version?

Is there a standard?  It's not in coding-style.rst.

While I prefer lowercase too, it seems the kernel has
only ~2:1 preference for lowercase to uppercase hex.

$ git grep -ohP '\b0[xX][0-9a-f]+\b' | grep [a-f] | wc -l
1149833
$ git grep -ohP '\b0[xX][0-9A-F]+\b' | grep [A-F] | wc -l
575781



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

* Re: [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17  9:51     ` Joe Perches
@ 2020-07-17 10:04       ` Hans Verkuil
  2020-07-17 17:58         ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2020-07-17 10:04 UTC (permalink / raw)
  To: Joe Perches, Santiago Hormazabal, linux-media, devicetree,
	Rob Herring, Ezequiel Garcia, linux-kernel

On 17/07/2020 11:51, Joe Perches wrote:
> On Fri, 2020-07-17 at 11:29 +0200, Hans Verkuil wrote:
>> It's standard linux codingstyle to use lowercase for hex numbers.
>> Can you change that throughout the source for the next version?
> 
> Is there a standard?  It's not in coding-style.rst.
> 
> While I prefer lowercase too, it seems the kernel has
> only ~2:1 preference for lowercase to uppercase hex.
> 
> $ git grep -ohP '\b0[xX][0-9a-f]+\b' | grep [a-f] | wc -l
> 1149833
> $ git grep -ohP '\b0[xX][0-9A-F]+\b' | grep [A-F] | wc -l
> 575781
> 
> 

Well, it's indeed not a standard for the kernel as a whole, but certainly
for drivers/media:

$ git grep -ohP '\b0[xX][0-9a-f]+\b' drivers/media/ | grep [a-f] | wc -l
109272
$ git grep -ohP '\b0[xX][0-9A-F]+\b' drivers/media/ | grep [A-F] | wc -l
22392

The media subsystem has a 5:1 preference for lowercase. And uppercase is
mostly found in older drivers.

Still, I really prefer lowercase over uppercase, especially in new drivers.

Regards,

	Hans

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

* Re: [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17  9:29   ` Hans Verkuil
  2020-07-17  9:51     ` Joe Perches
@ 2020-07-17 15:10     ` Santiago Hormazabal
  1 sibling, 0 replies; 11+ messages in thread
From: Santiago Hormazabal @ 2020-07-17 15:10 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, devicetree, Rob Herring, Ezequiel Garcia, linux-kernel

On Fri, 17 Jul 2020 at 06:29, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Santiago,
>
> Nice, it's been a long time since I last received a new radio driver :-)
>
> I have a bunch of comments below, nothing major.
>
> Main thing you need to do is to run 'checkpatch --strict' over the patch
> to fix codingstyle issues. And I realized that one corner of the API wasn't
> checked by v4l2-compliance, so you need to get the latest version of
> v4l2-compliance where I added new checks.
>
Sure, I'll get the newest v4l2-compliance and run it again, and
(probably) fix the issues on the code.

> On 17/07/2020 02:44, 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,
>
> I can't find that site, is the URL correct?

Right, it seems like the manufacturer is gone, and the specs of this
module didn't get
saved by the web.archive.org; but I found these chips readily available over
AliExpress/Alibaba and so on. I scavenged this module from an
not-so-old home theater.
I found a Japanese provider of these chips over
https://www.aitendo.com/product/7448;
and they also provide a breakout board for the SSOP16 package, but I
assume it'll be
easier to just buy the chip over AliExpress and use a standard SSOP16L breakout.
I also saw a cool project that used this chip with an arduino, where
the minimal components
needed for the chip to run are described over
https://github.com/xiaolaba/KT0913_RADIO.

> If you know of a cheap shield that has this chip then let me know, I might
> just get one myself.
>
> > part number ZJ-801B) and a H2+ AllWinner SoC running the master
> > (at this time) of the media_tree.
> >
> > Signed-off-by: Santiago Hormazabal <santiagohssl@gmail.com>
> > ---
> >  drivers/media/radio/radio-kt0913.c | 1181 ++++++++++++++++++++++++++++
> >  1 file changed, 1181 insertions(+)
> >  create mode 100644 drivers/media/radio/radio-kt0913.c
> >
> > diff --git a/drivers/media/radio/radio-kt0913.c b/drivers/media/radio/radio-kt0913.c
> > new file mode 100644
> > index 000000000000..9350d1764d44
> > --- /dev/null
> > +++ b/drivers/media/radio/radio-kt0913.c
> > @@ -0,0 +1,1181 @@
> > +// 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.
> > + */
> > +
> > +#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
>
> It's standard linux codingstyle to use lowercase for hex numbers.
> Can you change that throughout the source for the next version?
>
Yes, of course.

> > +
> > +/* 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-fm-am"
> > +
> > +/* ************************************************************************* */
> > +
> > +/* 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 */
>
> These comments do not seem to be aligned. Can you fix that? It looks ugly.
>
Yes, sorry, I forgot about that. The same applies to the other
comments on the following lines.

> > +
> > +     /* 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_deemphasis;  /* 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_CAMUS, BAND_AM };
>
> BAND_FM_CAMUS -> BAND_FM_CAMPUS
>
Woops! I don't know why I screwed up the naming of "CAMPUS". I'll fix those.

> > +
> > +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_CAMUS */
>
> CAMUS -> 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)
>
> Put the newline just before the function name, so:
>
> static inline struct kt0913_device *
> v4l2_device_to_device(struct v4l2_device *v4l2_dev)
>
> It's easier to read that way.
>
Right, I'll apply these changes to the functions whose arguments don't
fit on the single
line.

> > +{
> > +     return container_of(v4l2_dev,
> > +             struct kt0913_device, v4l2_dev);
> > +}
> > +
> > +static inline struct kt0913_device *v4l2_ctrl_to_device(
> > +     struct v4l2_ctrl *ctrl_handler)
>
> Same here and perhaps elsewhere as well if this happens in other places.
>
> > +{
> > +     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)
>
> Align this properly:
>
> static int __kt0913_get_fm_frequency(struct kt0913_device *radio,
>                                      unsigned int *frequency)
>
> I suspect you didn't run 'scripts/checkpatch.pl --strict' over these patches.
> Please do, it helps avoid such issues.
>
I ran the script, but without the argument --script. I'll run those
again after I fix all the items
you've mentioned.

> > +{
> > +     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:
>
> This can just be an 'if'. A switch is overkill here.
>
Good catch. Originally it was a switch between 75us, 50us, and return
-EINVAL for disabled
deemphasis since the chip doesn't support that.

> > +             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() failed for 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() failed for 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() failed for 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 = BAND_FM;
> > +     int ret;
> > +
> > +     if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
> > +             return -EINVAL;
> > +
> > +     if (freq == 0)
> > +             return -EINVAL;
> > +
> > +     /* check if the requested frequency is contained on the AM band */
> > +     if (freq <= kt0913_bands[BAND_AM].rangehigh)
> > +             new_band = BAND_AM;
> > +     else {
> > +             /* check if the requested frequency is contained on the FM band */
> > +             if (freq >= kt0913_bands[BAND_FM].rangelow)
> > +                     new_band = BAND_FM;
> > +             /* check if the requested frequency is contained on the campus
> > +              * FM band only if that feature was enabled
> > +              */
> > +             else if (kt0913_use_campus_band &&
> > +                     (freq >= kt0913_bands[BAND_FM_CAMUS].rangelow))
> > +                     new_band = BAND_FM_CAMUS;
>
> This seems a bit overly complex. Just do:
>
>                 new_band = kt0913_use_campus_band ? BAND_FM_CAMUS : BAND_FM;
>                 if (freq < kt0913_bands[new_band].rangelow)
>                         return -EINVAL;
>
I like that way better, thanks.

> > +             else {
> > +                     v4l2_warn(radio->client,
> > +                             "frequency out of allowed RF bands (%u kHz)",
> > +                             v4l2_freq_to_khz(freq));
>
> This is wrong. It should clamp the frequency to the closest valid frequency
> range and just continue.
>
> Hmm, v4l2-compliance should have failed on that, but it doesn't do this check
> when iterating over the frequency bands. That's a bug. I've fixed that in
> v4l2-compliance, so if you do a git pull for the v4l-utils repo then the
> v4l2-compliance utility should now fail on this.
>
I was in doubt about this originally, since I didn't know I should
clamp to the closest valid
frequency. I asked on the IRC about this, and I changed my code
properly at that time since
it was failing the compliance test. I'll fix this by not returning
-EINVAL and let the clamp() do
its work.

> > +                     return -EINVAL;
> > +             }
> > +     }
> > +
> > +     /* is the requested band different than the one currently set? */
> > +     if (radio->band != new_band) {
> > +             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_CAMUS];
> > +             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 */
> > +     if (!i2c_check_functionality(client->adapter,
> > +             I2C_FUNC_SMBUS_READ_WORD_DATA | I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
> > +             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) {
>
> Don't check after each control,
>
> > +             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_deemphasis = 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) {
>
> It's sufficient to just do the check at the end. See e.g.
> drivers/media/radio/si4713/si4713.c
>
Gotcha.

> > +             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 },
> > +     { }
> > +};
> > +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.1");
> > +
> > +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)");>
>
> Regards,
>
>         Hans
Thanks for your feedback!
As you might have noticed, this is my first attempt to create a kernel driver.
And sorry for the previous HTML email, I forgot to turn that off.
I'll work on the fixes and send another set of patches as soon as I
find some time.

Regards,
- Santiago H.

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

* Re: [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro.
  2020-07-17 10:04       ` Hans Verkuil
@ 2020-07-17 17:58         ` Joe Perches
  0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2020-07-17 17:58 UTC (permalink / raw)
  To: Hans Verkuil, Santiago Hormazabal, linux-media, devicetree,
	Rob Herring, Ezequiel Garcia, linux-kernel

On Fri, 2020-07-17 at 12:04 +0200, Hans Verkuil wrote:
> On 17/07/2020 11:51, Joe Perches wrote:
> > On Fri, 2020-07-17 at 11:29 +0200, Hans Verkuil wrote:
> > > It's standard linux codingstyle to use lowercase for hex numbers.
> > > Can you change that throughout the source for the next version?
> > 
> > Is there a standard?  It's not in coding-style.rst.
> > 
> > While I prefer lowercase too, it seems the kernel has
> > only ~2:1 preference for lowercase to uppercase hex.
> > 
> > $ git grep -ohP '\b0[xX][0-9a-f]+\b' | grep [a-f] | wc -l
> > 1149833
> > $ git grep -ohP '\b0[xX][0-9A-F]+\b' | grep [A-F] | wc -l
> > 575781
> > 
> Well, it's indeed not a standard for the kernel as a whole, but certainly
> for drivers/media:
> 
> $ git grep -ohP '\b0[xX][0-9a-f]+\b' drivers/media/ | grep [a-f] | wc -l
> 109272
> $ git grep -ohP '\b0[xX][0-9A-F]+\b' drivers/media/ | grep [A-F] | wc -l
> 22392
> 
> The media subsystem has a 5:1 preference for lowercase. And uppercase is
> mostly found in older drivers.
> Still, I really prefer lowercase over uppercase, especially in new drivers.

That's certainly any maintainer's preference right.

Slightly unrelated:

The last 100k commits have only a ~2.5:1 use of
lowercase to uppercase hex constants.

While my cut-off for declaring something a generic
kernel style standard is also ~5:1, this isn't a
style check I would put into checkpatch as the
per-subsystem variability is quite high.

cheers, Joe


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

* Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro
  2020-07-17  0:44 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
@ 2020-07-23 20:51   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-07-23 20:51 UTC (permalink / raw)
  To: Santiago Hormazabal
  Cc: linux-media, Hans Verkuil, devicetree, Ezequiel Garcia,
	Rob Herring, linux-kernel

On Thu, 16 Jul 2020 21:44:39 -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 2/3] media: kt0913: device tree binding
  2020-07-17  0:44 ` [PATCH 2/3] media: kt0913: device tree binding Santiago Hormazabal
@ 2020-07-23 20:51   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-07-23 20:51 UTC (permalink / raw)
  To: Santiago Hormazabal
  Cc: linux-kernel, Rob Herring, linux-media, Ezequiel Garcia,
	devicetree, Hans Verkuil

On Thu, 16 Jul 2020 21:44:40 -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
> 

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

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

end of thread, other threads:[~2020-07-23 20:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17  0:44 [PATCH 0/3] KT091 FM/AM driver Santiago Hormazabal
2020-07-17  0:44 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add KT Micro Santiago Hormazabal
2020-07-23 20:51   ` Rob Herring
2020-07-17  0:44 ` [PATCH 2/3] media: kt0913: device tree binding Santiago Hormazabal
2020-07-23 20:51   ` Rob Herring
2020-07-17  0:44 ` [PATCH 3/3] media: Add support for the AM/FM radio chip KT0913 from KT Micro Santiago Hormazabal
2020-07-17  9:29   ` Hans Verkuil
2020-07-17  9:51     ` Joe Perches
2020-07-17 10:04       ` Hans Verkuil
2020-07-17 17:58         ` Joe Perches
2020-07-17 15:10     ` Santiago Hormazabal

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).