Thread (1 message) 1 message, 1 author, 2016-11-14

RE: [PATCH 5/5] media: platform: rcar_drif: Add DRIF support

From: Ramesh Shanmugasundaram <hidden>
Date: 2016-11-14 16:11:31
Also in: linux-media, linux-renesas-soc 

Hi Hans,

Thanks for the review comments.

Subject: Re: [PATCH 5/5] media: platform: rcar_drif: Add DRIF support

On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
quoted
This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3
SoCs.
quoted
The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF
device represents a channel and each channel can have one or two
sub-channels respectively depending on the target board.

DRIF supports only Rx functionality. It receives samples from a RF
frontend tuner chip it is interfaced with. The combination of DRIF and
the tuner device, which is registered as a sub-device, determines the
receive sample rate and format.

In order to be compliant as a V4L2 SDR device, DRIF needs to bind with
the tuner device, which can be provided by a third party vendor. DRIF
acts as a slave device and the tuner device acts as a master
transmitting the samples. The driver allows asynchronous binding of a
tuner device that is registered as a v4l2 sub-device. The driver can
learn about the tuner it is interfaced with based on port endpoint
properties of the device in device tree. The V4L2 SDR device inherits
the controls exposed by the tuner device.

The device can also be configured to use either one or both of the
data pins at runtime based on the master (tuner) configuration.

Signed-off-by: Ramesh Shanmugasundaram
[off-list ref]
---
 .../devicetree/bindings/media/renesas,drif.txt     |  136 ++
 drivers/media/platform/Kconfig                     |   25 +
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/rcar_drif.c                 | 1574
++++++++++++++++++++
quoted
 4 files changed, 1736 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/media/renesas,drif.txt
 create mode 100644 drivers/media/platform/rcar_drif.c

diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
b/Documentation/devicetree/bindings/media/renesas,drif.txt
new file mode 100644
index 0000000..d65368a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
@@ -0,0 +1,136 @@
+Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
+------------------------------------------------------------
+
+R-Car Gen3 DRIF is a serial slave device. It interfaces with a master
+device as shown below
+
++---------------------+                +---------------------+
+|                     |-----SCK------->|CLK                  |
+|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
+|                     |-----SD0------->|D0                   |
+|                     |-----SD1------->|D1                   |
++---------------------+                +---------------------+
+
+Each DRIF channel (drifn) consists of two sub-channels (drifn0 &
drifn1).
quoted
+The sub-channels are like two individual channels in itself that
+share the common CLK & SYNC. Each sub-channel has it's own dedicated
+resources like irq, dma channels, address space & clock.
+
+The device tree model represents the channel and each of it's
+sub-channel as a separate node. The parent channel ties the
+sub-channels together with their phandles.
+
+Required properties of a sub-channel:
+-------------------------------------
+- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
R8A7795 SoC.
quoted
+	      "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
device.
quoted
+	      When compatible with the generic version, nodes must list the
+	      SoC-specific version corresponding to the platform first
+	      followed by the generic version.
+- reg: offset and length of that sub-channel.
+- interrupts: associated with that sub-channel.
+- clocks: phandle and clock specifier of that sub-channel.
+- clock-names: clock input name string: "fck".
+- dmas: phandles to the DMA channel of that sub-channel.
+- dma-names: names of the DMA channel: "rx".
+
+Optional properties of a sub-channel:
+-------------------------------------
+- power-domains: phandle to the respective power domain.
+
+Required properties of a channel:
+---------------------------------
+- pinctrl-0: pin control group to be used for this channel.
+- pinctrl-names: must be "default".
+- sub-channels : phandles to the two sub-channels.
+
+Optional properties of a channel:
+---------------------------------
+- port: child port node of a channel that defines the local and remote
+        endpoints. The remote endpoint is assumed to be a tuner
subdevice
quoted
+	endpoint.
+- renesas,syncmd       : sync mode
+			 0 (Frame start sync pulse mode. 1-bit width pulse
+			    indicates start of a frame)
+			 1 (L/R sync or I2S mode) (default)
+- renesas,lsb-first    : empty property indicates lsb bit is received
first.
quoted
+			 When not defined msb bit is received first (default)
+- renesas,syncac-pol-high  : empty property indicates sync signal
polarity.
quoted
+			 When defined, active high or high->low sync signal.
+			 When not defined, active low or low->high sync signal
+			 (default)
+- renesas,dtdl         : delay between sync signal and start of
reception.
quoted
+			 Must contain one of the following values:
+			 0   (no bit delay)
+			 50  (0.5-clock-cycle delay)
+			 100 (1-clock-cycle delay) (default)
+			 150 (1.5-clock-cycle delay)
+			 200 (2-clock-cycle delay)
+- renesas,syncdl       : delay between end of reception and sync signal
edge.
quoted
+			 Must contain one of the following values:
+			 0   (no bit delay) (default)
+			 50  (0.5-clock-cycle delay)
+			 100 (1-clock-cycle delay)
+			 150 (1.5-clock-cycle delay)
+			 200 (2-clock-cycle delay)
+			 300 (3-clock-cycle delay)
+
+Example
+--------
+
+SoC common dtsi file
+
+		drif00: rif@e6f40000 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			reg = <0 0xe6f40000 0 0x64>;
+			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 515>;
+			clock-names = "fck";
+			dmas = <&dmac1 0x20>, <&dmac2 0x20>;
+			dma-names = "rx", "rx";
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
+		drif01: rif@e6f50000 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			reg = <0 0xe6f50000 0 0x64>;
+			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 514>;
+			clock-names = "fck";
+			dmas = <&dmac1 0x22>, <&dmac2 0x22>;
+			dma-names = "rx", "rx";
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
+		drif0: rif@0 {
+			compatible = "renesas,r8a7795-drif",
+				     "renesas,rcar-gen3-drif";
+			sub-channels = <&drif00>, <&drif01>;
+			status = "disabled";
+		};
+
+Board specific dts file
+
+&drif00 {
+	status = "okay";
+};
+
+&drif01 {
+	status = "okay";
+};
+
+&drif0 {
+	pinctrl-0 = <&drif0_pins>;
+	pinctrl-names = "default";
+	renesas,syncac-pol-high;
+	status = "okay";
+	port {
+		drif0_ep: endpoint {
+		     remote-endpoint = <&tuner_subdev_ep>;
+		};
+	};
+};
diff --git a/drivers/media/platform/Kconfig
b/drivers/media/platform/Kconfig index 754edbf1..0ae83a8 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -393,3 +393,28 @@ menuconfig DVB_PLATFORM_DRIVERS  if
DVB_PLATFORM_DRIVERS  source
"drivers/media/platform/sti/c8sectpfe/Kconfig"
 endif #DVB_PLATFORM_DRIVERS
+
+menuconfig SDR_PLATFORM_DRIVERS
+	bool "SDR platform devices"
+	depends on MEDIA_SDR_SUPPORT
+	default n
+	---help---
+	  Say Y here to enable support for platform-specific SDR Drivers.
+
+if SDR_PLATFORM_DRIVERS
+
+config VIDEO_RCAR_DRIF
+	tristate "Renesas Digitial Radio Interface (DRIF)"
+	depends on VIDEO_V4L2 && HAS_DMA
+	depends on ARCH_RENESAS
+	select VIDEOBUF2_VMALLOC
+	---help---
+	  Say Y if you want to enable R-Car Gen3 DRIF support. DRIF is
Digital
quoted
+	  Radio Interface that interfaces with an RF front end chip. It is a
+	  receiver of digital data which uses DMA to transfer received data
to
quoted
+	  a configured location for an application to use.
+
+	  To compile this driver as a module, choose M here; the module
+	  will be called rcar_drif.
+
+endif # SDR_PLATFORM_DRIVERS
diff --git a/drivers/media/platform/Makefile
b/drivers/media/platform/Makefile index f842933..49ce238 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_SOC_CAMERA)		+= soc_camera/

 obj-$(CONFIG_VIDEO_RENESAS_FCP) 	+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU) 	+= rcar_jpu.o
+obj-$(CONFIG_VIDEO_RCAR_DRIF)		+= rcar_drif.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1/

 obj-y	+= omap/
diff --git a/drivers/media/platform/rcar_drif.c
b/drivers/media/platform/rcar_drif.c
new file mode 100644
index 0000000..34dc282
--- /dev/null
+++ b/drivers/media/platform/rcar_drif.c
@@ -0,0 +1,1574 @@
<snip>

+#define for_each_rcar_drif_subdev(sd, tmp, ch)				\
+	list_for_each_entry_safe(sd, tmp, &ch->v4l2_dev.subdevs, list)
+

Please don't use this. media/v4l2-device.h has a bunch of similar
functions for this. Use those instead.
Thanks. Agreed.

<snip>
quoted
+static int rcar_drif_querycap(struct file *file, void *fh,
+			      struct v4l2_capability *cap) {
+	struct rcar_drif_chan *ch = video_drvdata(file);
+
+	strlcpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
+	strlcpy(cap->card, ch->vdev.name, sizeof(cap->card));
+	cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
+				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
Set device_caps in struct video_device and drop it here.

The core will fill in cap->device_caps and cap->capabilities for you.
Agreed.

quoted
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+		 ch->vdev.name);
+	return 0;
+}
+
+static int rcar_drif_set_default_format(struct rcar_drif_chan *ch) {
+	unsigned int i;
+
+	for (i = 0; i < NUM_FORMATS; i++) {
+		/* Find any matching fmt and set it as default */
+		if (ch->num_hw_schans == formats[i].num_schans) {
+			ch->fmt_idx = i;
+			ch->cur_schans_mask = ch->hw_schans_mask;
+			ch->num_cur_schans = ch->num_hw_schans;
+			dev_dbg(ch->dev, "default fmt[%u]: mask %lu num %u\n",
+				i, ch->cur_schans_mask, ch->num_cur_schans);
+			return 0;
+		}
+	}
+	dev_err(ch->dev, "no matching sdr fmt found\n");
+	return -EINVAL;
+}
+
+static int rcar_drif_enum_fmt_sdr_cap(struct file *file, void *priv,
+				      struct v4l2_fmtdesc *f)
+{
+	if (f->index >= NUM_FORMATS)
+		return -EINVAL;
+
+	strlcpy(f->description, formats[f->index].name,
+sizeof(f->description));
Drop this. The core fills that in for you.
Agreed.

quoted
+	f->pixelformat = formats[f->index].pixelformat;
+	return 0;
+}
+
+static int rcar_drif_g_fmt_sdr_cap(struct file *file, void *priv,
+				   struct v4l2_format *f)
+{
+	struct rcar_drif_chan *ch = video_drvdata(file);
+
+	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
+	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
+	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
+	return 0;
+}
+
[snip]

quoted
+/* Parse sub-devs (tuner) to find a matching device */ static int
+rcar_drif_parse_subdevs(struct device *dev,
+				   struct v4l2_async_notifier *notifier) {
+	struct device_node *node = NULL;
+
+	notifier->subdevs = devm_kzalloc(dev, sizeof(*notifier->subdevs),
+					 GFP_KERNEL);
+	if (!notifier->subdevs)
+		return -ENOMEM;
+
+	node = of_graph_get_next_endpoint(dev->of_node, node);
Do:

	if (!node)
		return 0;

And the remainder can be shifted one tab to the left.
Agreed.

quoted
+	if (node) {
+		struct rcar_drif_async_subdev *rsd;
+
+		rsd = devm_kzalloc(dev, sizeof(*rsd), GFP_KERNEL);
+		if (!rsd) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		notifier->subdevs[notifier->num_subdevs] = &rsd->asd;
+		rsd->asd.match.of.node =
of_graph_get_remote_port_parent(node);
quoted
+		of_node_put(node);
+		if (!rsd->asd.match.of.node) {
+			dev_warn(dev, "bad remote port parent\n");
+			return -EINVAL;
+		}
+
+		rsd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+		notifier->num_subdevs++;
+	}
+	return 0;
+}
+
+/* SIRMDR1 configuration */
+static int rcar_drif_validate_syncmd(struct rcar_drif_chan *ch, u32
+val) {
+	if (val > 1) {
+		dev_err(ch->dev, "invalid syncmd %u using L/R mode\n", val);
+		return -EINVAL;
+	}
+
+	ch->mdr1 &= ~(3 << 28);	/* Clear current settings */
+	if (val == 0)
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_FRAME;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_LR;
+	return 0;
+}
+
+/* Get the dtdl or syncdl bits as in MSIOF */ static u32
+rcar_drif_get_dtdl_or_syncdl_bits(u32 dtdl_or_syncdl) {
+	/*
+	 * DTDL/SYNCDL bit	: dtdl/syncdl
+	 * b'000		: 0
+	 * b'001		: 100
+	 * b'010		: 200
+	 * b'011 (SYNCDL only)	: 300
+	 * b'101		: 50
+	 * b'110		: 150
+	 */
+	if (dtdl_or_syncdl % 100)
+		return dtdl_or_syncdl / 100 + 5;
+	else
Line can be dropped.
Agreed.

quoted
+		return dtdl_or_syncdl / 100;
+}
+
+static int rcar_drif_validate_dtdl_syncdl(struct rcar_drif_chan *ch)
+{
+	struct device_node *np = ch->dev->of_node;
+	u32 dtdl = 100, syncdl = 0;
+
+	ch->mdr1 |= RCAR_DRIF_SIRMDR1_DTDL_1 | RCAR_DRIF_SIRMDR1_SYNCDL_0;
+	of_property_read_u32(np, "renesas,dtdl", &dtdl);
+	of_property_read_u32(np, "renesas,syncdl", &syncdl);
+
+	/* Sanity checks */
+	if (dtdl > 200 || syncdl > 300) {
+		dev_err(ch->dev, "invalid dtdl %u/syncdl %u\n", dtdl, syncdl);
+		return -EINVAL;
+	}
+	if ((dtdl + syncdl) % 100) {
+		dev_err(ch->dev, "sum of dtdl %u & syncdl %u not OK\n",
+			dtdl, syncdl);
+		return -EINVAL;
+	}
+	ch->mdr1 &= ~(7 << 20) & ~(7 << 16);	/* Clear current settings
*/
quoted
+	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(dtdl) << 20;
+	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(syncdl) << 16;
+	return 0;
+}
+
+static int rcar_drif_parse_properties(struct rcar_drif_chan *ch) {
+	struct device_node *np = ch->dev->of_node;
+	u32 syncmd;
+	int ret;
+
+	/* Set the defaults and check for overrides */
+	ch->mdr1 = RCAR_DRIF_SIRMDR1_SYNCMD_LR;
+	if (!of_property_read_u32(np, "renesas,syncmd", &syncmd)) {
+		ret = rcar_drif_validate_syncmd(ch, syncmd);
+		if (ret)
+			return ret;
+	}
+
+	if (of_find_property(np, "renesas,lsb-first", NULL))
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_LSB_FIRST;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_MSB_FIRST;
+
+	if (of_find_property(np, "renesas,syncac-pol-high", NULL))
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_HIGH;
+	else
+		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_LOW;
+
+	return rcar_drif_validate_dtdl_syncdl(ch);
+}
+
+static u32 rcar_drif_enum_sub_channels(struct platform_device *pdev,
+					struct platform_device *s_pdev[]) {
+	struct device_node *s_np;
+	u32 hw_schans_mask = 0;
+	unsigned int i;
+
+	for (i = 0; i < RCAR_DRIF_MAX_SUBCHANS; i++) {
+		s_np = of_parse_phandle(pdev->dev.of_node, "sub-channels", i);
+		if (s_np && of_device_is_available(s_np)) {
+			s_pdev[i] = of_find_device_by_node(s_np);
+			if (s_pdev[i]) {
+				hw_schans_mask |= BIT(i);
+				dev_dbg(&s_pdev[i]->dev, "schan%u ok\n", i);
+			}
+		}
+	}
+	return hw_schans_mask;
+}
+
+static int rcar_drif_probe(struct platform_device *pdev) {
+	struct platform_device *s_pdev[RCAR_DRIF_MAX_SUBCHANS];
+	unsigned long hw_schans_mask;
+	struct rcar_drif_chan *ch;
+	unsigned int i;
+	int ret;
+
+	/*
+	 * Sub-channel resources are managed by the parent channel instance.
+	 * The sub-channel instance helps only in registering with power
domain
quoted
+	 * to aid in run-time pm support
+	 */
+	if (!of_find_property(pdev->dev.of_node, "sub-channels", NULL))
+		return 0;
+
+	/* Parent channel instance */
+	hw_schans_mask = rcar_drif_enum_sub_channels(pdev, s_pdev);
+	if (!hw_schans_mask) {
+		dev_err(&pdev->dev, "no sub-channels enabled\n");
+		return -ENODEV;
+	}
+
+
+	/* Reserve memory for driver structure */
+	ch = devm_kzalloc(&pdev->dev, sizeof(*ch), GFP_KERNEL);
+	if (!ch) {
+		ret = PTR_ERR(ch);
+		dev_err(&pdev->dev, "failed alloc drif context\n");
+		return ret;
+	}
+	ch->dev = &pdev->dev;
+
+	/* Parse device tree optional properties */
+	ret = rcar_drif_parse_properties(ch);
+	if (ret)
+		return ret;
+
+	dev_dbg(ch->dev, "parsed mdr1 0x%08x\n", ch->mdr1);
+
+	/* Setup enabled sub-channels */
+	for_each_rcar_drif_subchannel(i, &hw_schans_mask) {
+		struct clk *clkp;
+		struct resource	*res;
+		void __iomem *base;
+
+		/* Peripheral clock */
+		clkp = devm_clk_get(&s_pdev[i]->dev, "fck");
+		if (IS_ERR(clkp)) {
+			ret = PTR_ERR(clkp);
+			dev_err(&s_pdev[i]->dev, "clk get failed (%d)\n", ret);
+			return ret;
+		}
+
+		/* Register map */
+		res = platform_get_resource(s_pdev[i], IORESOURCE_MEM, 0);
+		base = devm_ioremap_resource(&s_pdev[i]->dev, res);
+		if (IS_ERR(base)) {
+			ret = PTR_ERR(base);
+			dev_err(&s_pdev[i]->dev, "ioremap failed (%d)\n", ret);
+			return ret;
+		}
+
+		/* Reserve memory for enabled sub-channel */
+		ch->sch[i] = devm_kzalloc(&pdev->dev, sizeof(*ch->sch[i]),
+					  GFP_KERNEL);
+		if (!ch->sch[i]) {
+			ret = PTR_ERR(ch);
+			dev_err(&s_pdev[i]->dev, "failed alloc sub-channel\n");
+			return ret;
+		}
+		ch->sch[i]->pdev = s_pdev[i];
+		ch->sch[i]->clkp = clkp;
+		ch->sch[i]->base = base;
+		ch->sch[i]->num = i;
+		ch->sch[i]->start = res->start;
+		ch->sch[i]->parent = ch;
+		ch->num_hw_schans++;
+	}
+	ch->hw_schans_mask = hw_schans_mask;
+
+	/* Validate any supported format for enabled sub-channels */
+	ret = rcar_drif_set_default_format(ch);
+	if (ret)
+		return ret;
+
+	/* Set defaults */
+	ch->num_hwbufs = RCAR_DRIF_DEFAULT_NUM_HWBUFS;
+	ch->hwbuf_size = RCAR_DRIF_DEFAULT_HWBUF_SIZE;
+
+	mutex_init(&ch->v4l2_mutex);
+	mutex_init(&ch->vb_queue_mutex);
+	spin_lock_init(&ch->queued_bufs_lock);
+	INIT_LIST_HEAD(&ch->queued_bufs);
+
+	/* Init videobuf2 queue structure */
+	ch->vb_queue.type = V4L2_BUF_TYPE_SDR_CAPTURE;
+	ch->vb_queue.io_modes = VB2_READ | VB2_MMAP | VB2_DMABUF;
+	ch->vb_queue.drv_priv = ch;
+	ch->vb_queue.buf_struct_size = sizeof(struct rcar_drif_frame_buf);
+	ch->vb_queue.ops = &rcar_drif_vb2_ops;
+	ch->vb_queue.mem_ops = &vb2_vmalloc_memops;
+	ch->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+
+	/* Init videobuf2 queue */
+	ret = vb2_queue_init(&ch->vb_queue);
+	if (ret) {
+		dev_err(ch->dev, "could not initialize vb2 queue\n");
+		return ret;
+	}
+
+	/* Init video_device structure */
+	ch->vdev = rcar_drif_vdev;
Don't embed video_device, use video_device_alloc instead. A lot of drivers
embed this, but it turns out not to be a good idea. So new drivers should
use video_device_alloc.
Agreed. 

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  




  

    Related discussions
    
  



    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help