All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v14 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2
@ 2018-04-26 20:21 Niklas Söderlund
  2018-04-26 20:21 ` [PATCH v14 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation Niklas Söderlund
  2018-04-26 20:21 ` [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver Niklas Söderlund
  0 siblings, 2 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-26 20:21 UTC (permalink / raw)
  To: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven,
	Niklas Söderlund

Hi,

This is the latest incarnation of R-Car MIPI CSI-2 receiver driver. It's
based on top of the media-tree and are tested on Renesas Salvator-X
together with adv7482 and the now in tree rcar-vin driver :-)

I hope this is the last incarnation of this patch-set, I do think it is
ready for upstream consumption :-)

* Changes since v13
- Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
- Add define for PHCLM_STOPSTATECKL.
- Update spelling in comments.
- Update calculation in rcar_csi2_calc_phypll() according to 
  https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one 
  before v14 did not take into account that 2 bits per sample is 
  transmitted.
- Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch 
  statement to set correct number of lanes to enable.
- Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match 
  style of rest of file.
- Switch to %u instead of 0x%x when printing bus type.
- Switch to %u instead of %d for priv->lanes which is unsigned.
- Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in 
  rcar_csi2_formats[].
- Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
- Set INTSTATE after PL-11 is confirmed to match flow chart in 
  datasheet.
- Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
- Add Maxime's and laurent's tags.

* Changes since v12
- Fixed spelling mistakes in commit messages and documentation patch,
  thanks Laurent.
- Mark endpoints in port 1 as optional as it is allowed to not connect a
  VIN to the CSI-2 and instead have it only use its parallel input
  source (for those VIN that have one).
- Added Ack from Sakari, thanks!
- Media bus codes are u32 not unsigned int.
- Ignore error check for v4l2_subdev_call(sd, video, s_stream, 0)
- Do not set subdev host data as it's not used,
  v4l2_set_subdev_hostdata().
- Fixed spelling errors in commit message.
- Add SPDX header
- Rename badly named variable tmp to vcdt_part.
- Cache subdevice in bound callback instead of looking it up in the
  media graph. By doing this rcar_csi2_sd_info() can be removed.
- Print unsigned using %u not %d.
- Call pm_runtime_enable() before calling v4l2_async_register_subdev().
- Dropped of_match_ptr() as OF is required.

* Changes since v11
- Added missing call to v4l2_async_notifier_unregister().
- Fixed missing reg popery in bindings documentation.
- Add Rob's ack to 01/02.
- Dropped 'media:' prefix from patch subjects as it seems they are added
  first when a patch is picked up by the maintainer.
- Fixed typo in comment enpoint -> endpoint, thanks Hans.
- Added Hans Reviewed-by to driver.

* Changes since v10
- Renamed Documentation/devicetree/bindings/media/rcar-csi2.txt to
  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
- Add extra newline in rcar_csi2_code_to_fmt()
- Use locally stored format information instead of reading it from the
  remote subdevice, Sakari pointed out that the pipeline is validated
  before .s_stream() is called so this is safe.
- Do not check return from media_entity_to_v4l2_subdev() in
  rcar_csi2_start(), Sakari pointed out it can't fail. Also move logic
  to find the remote subdevice is moved to the only user of it,
  rcar_csi2_calc_phypll().
- Move pm_runtime_get_sync() and pm_runtime_put() to
  rcar_csi2_s_stream() and remove rcar_csi2_s_power().
- Add validation of pixel code to rcar_csi2_set_pad_format().
- Remove static rcar_csi2_notify_unbind() as it only printed a debug
  message.

* Changes since v9
- Add reset property to device tree example
- Use BIT(x) instead of (1 << x)
- Use u16 in struct phypll_hsfreqrange to pack struct better.
- Use unsigned int type for loop variable in rcar_csi2_code_to_fmt
- Move fields inside struct struct rcar_csi2_info and struct rcar_csi2
  to pack struct's tighter.
- Use %u instead of %d when printing __u32.
- Don't check return of platform_get_resource(), let
  devm_ioremap_resource() handle it.
- Store quirk workaround for r8a7795 ES1.0 in the data field of struct
  soc_device_attribute.

Changes since v8:
- Updated bindings documentation, thanks Rob!
- Make use of the now in media-tree sub-notifier V4L2 API
- Add delay when resetting the IP to allow for a proper reset
- Fix bug in s_stream error path where the usage count was off if an
  error was hit.
- Add support for H3 ES2.0

Changes since v7:
- Rebase on top of the latest incremental async patches.
- Fix comments on DT documentation.
- Use v4l2_ctrl_g_ctrl_int64() instead of v4l2_g_ext_ctrls().
- Handle try formats in .set_fmt() and .get_fmt().
- Don't call v4l2_device_register_subdev_nodes() as this is not needed
  with the complete() callbacks synchronized.
- Fix line over 80 chars.
- Fix varies spelling mistakes.

Changes since v6:
- Rebased on top of Sakaris fwnode patches.
- Changed of RCAR_CSI2_PAD_MAX to NR_OF_RCAR_CSI2_PAD.
- Remove assumption about unknown media bus type, thanks Sakari for
  pointing this out.
- Created table for supported format information instead of scattering
  this information around the driver, thanks Sakari!
- Small newline fixes and reduce some indentation levels

Changes since v5:
- Make use of the incremental async subnotifer and helper to map DT
  endpoint to media pad number. This moves functionality which
  previously in the Gen3 patches for R-Car VIN driver to this R-Car
  CSI-2 driver. This is done in preparation to support the ADV7482
  driver in development by Kieran which will register more then one
  subdevice and the CSI-2 driver needs to cope wit this. Further more it
  prepares the driver for another use-case where more then one subdevice
  is present upstream for the CSI-2.
- Small cleanups.
- Add explicit include for linux/io.h, thanks Kieran.

Changes since v4:
- Match SoC part numbers and drop trailing space in documentation,
  thanks Geert for pointing this out.
- Clarify that the driver is a CSI-2 receiver by supervised
  s/interface/receiver/, thanks Laurent.
- Add entries in Kconfig and Makefile alphabetically instead of append.
- Rename struct rcar_csi2 member swap to lane_swap.
- Remove macros to wrap calls to dev_{dbg,info,warn,err}.
- Add wrappers for ioread32 and iowrite32.
- Remove unused interrupt handler, but keep checking in probe that there
  are a interrupt define in DT.
- Rework how to wait for LP-11 state, thanks Laurent for the great idea!
- Remove unneeded delay in rcar_csi2_reset()
- Remove check for duplicated lane id:s from DT parsing. Broken out to a
  separate patch adding this check directly to v4l2_of_parse_endpoint().
- Fixed rcar_csi2_start() to ask it's source subdevice for information
  about pixel rate and frame format. With this change having
  {set,get}_fmt operations became redundant, it was only used for
  figuring out this out so dropped them.
- Tabulated frequency settings map.
- Dropped V4L2_SUBDEV_FL_HAS_DEVNODE it should never have been set.
- Switched from MEDIA_ENT_F_ATV_DECODER to
  MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER as entity function. I can't
  find a more suitable function, and what the hardware do is to fetch
  video from an external chip and passes it on to a another SoC internal
  IP it's sort of a formatter.
- Break out DT documentation and code in two patches.

Changes since v3:
- Update DT binding documentation with input from Geert Uytterhoeven,
  thanks

Changes since v2:
- Added media control pads as this is needed by the new rcar-vin driver.
- Update DT bindings after review comments and to add r8a7796 support.
- Add get_fmt handler.
- Fix media bus format error s/YUYV8/UYVY8/

Changes since v1:
- Drop dependency on a pad aware s_stream operation.
- Use the DT bindings format "renesas,<soctype>-<device>", thanks Geert
  for pointing this out.

Niklas Söderlund (2):
  rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
  rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

 .../bindings/media/renesas,rcar-csi2.txt      |  99 ++
 MAINTAINERS                                   |   1 +
 drivers/media/platform/rcar-vin/Kconfig       |  12 +
 drivers/media/platform/rcar-vin/Makefile      |   1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c   | 883 ++++++++++++++++++
 5 files changed, 996 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
 create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c

-- 
2.17.0

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

* [PATCH v14 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
  2018-04-26 20:21 [PATCH v14 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 Niklas Söderlund
@ 2018-04-26 20:21 ` Niklas Söderlund
  2018-04-26 20:21 ` [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver Niklas Söderlund
  1 sibling, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-26 20:21 UTC (permalink / raw)
  To: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven,
	Niklas Söderlund

Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
are located between the video sources (CSI-2 transmitters) and the video
grabbers (VIN) on Gen3 of Renesas R-Car SoC.

Each CSI-2 device is connected to more than one VIN device which
simultaneously can receive video from the same CSI-2 device. Each VIN
device can also be connected to more than one CSI-2 device. The routing
of which links are used is controlled by the VIN devices. There are only
a few possible routes which are set by hardware limitations, which are
different for each SoC in the Gen3 family.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---

* Changes since v13
- Add Laurent's tag.
---
 .../bindings/media/renesas,rcar-csi2.txt      | 99 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt

diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
new file mode 100644
index 0000000000000000..6f71f997dc48eee9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -0,0 +1,99 @@
+Renesas R-Car MIPI CSI-2
+------------------------
+
+The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the
+Renesas R-Car family of devices. It is used in conjunction with the
+R-Car VIN module, which provides the video capture capabilities.
+
+Mandatory properties
+--------------------
+ - compatible: Must be one or more of the following
+   - "renesas,r8a7795-csi2" for the R8A7795 device.
+   - "renesas,r8a7796-csi2" for the R8A7796 device.
+
+ - reg: the register base and size for the device registers
+ - interrupts: the interrupt for the device
+ - clocks: reference to the parent clock
+
+The device node shall contain two 'port' child nodes according to the
+bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt. Port 0 shall connect to the CSI-2 source. Port 1
+shall connect to all the R-Car VIN modules that have a hardware
+connection to the CSI-2 receiver.
+
+- Port 0 - Video source (mandatory)
+	- Endpoint 0 - sub-node describing the endpoint that is the video source
+
+- Port 1 - VIN instances (optional)
+	- One endpoint sub-node for every R-Car VIN instance which is connected
+	  to the R-Car CSI-2 receiver.
+
+Example:
+
+	csi20: csi2@fea80000 {
+		compatible = "renesas,r8a7796-csi2";
+		reg = <0 0xfea80000 0 0x10000>;
+		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 714>;
+		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		resets = <&cpg 714>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				reg = <0>;
+
+				csi20_in: endpoint@0 {
+					reg = <0>;
+					clock-lanes = <0>;
+					data-lanes = <1>;
+					remote-endpoint = <&adv7482_txb>;
+				};
+			};
+
+			port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				reg = <1>;
+
+				csi20vin0: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vin0csi20>;
+				};
+				csi20vin1: endpoint@1 {
+					reg = <1>;
+					remote-endpoint = <&vin1csi20>;
+				};
+				csi20vin2: endpoint@2 {
+					reg = <2>;
+					remote-endpoint = <&vin2csi20>;
+				};
+				csi20vin3: endpoint@3 {
+					reg = <3>;
+					remote-endpoint = <&vin3csi20>;
+				};
+				csi20vin4: endpoint@4 {
+					reg = <4>;
+					remote-endpoint = <&vin4csi20>;
+				};
+				csi20vin5: endpoint@5 {
+					reg = <5>;
+					remote-endpoint = <&vin5csi20>;
+				};
+				csi20vin6: endpoint@6 {
+					reg = <6>;
+					remote-endpoint = <&vin6csi20>;
+				};
+				csi20vin7: endpoint@7 {
+					reg = <7>;
+					remote-endpoint = <&vin7csi20>;
+				};
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0a1410d5a621835d..45371a323e790c97 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8814,6 +8814,7 @@ L:	linux-media@vger.kernel.org
 L:	linux-renesas-soc@vger.kernel.org
 T:	git git://linuxtv.org/media_tree.git
 S:	Supported
+F:	Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
 F:	Documentation/devicetree/bindings/media/rcar_vin.txt
 F:	drivers/media/platform/rcar-vin/
 
-- 
2.17.0

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

* [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-26 20:21 [PATCH v14 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 Niklas Söderlund
  2018-04-26 20:21 ` [PATCH v14 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation Niklas Söderlund
@ 2018-04-26 20:21 ` Niklas Söderlund
  2018-04-26 21:30   ` Laurent Pinchart
  2018-04-28 11:28   ` jacopo mondi
  1 sibling, 2 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-26 20:21 UTC (permalink / raw)
  To: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven,
	Niklas Söderlund

A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
connected between the video sources and the video grabbers (VIN).

Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---

* Changes since v13
- Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
- Add define for PHCLM_STOPSTATECKL.
- Update spelling in comments.
- Update calculation in rcar_csi2_calc_phypll() according to
  https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
  before v14 did not take into account that 2 bits per sample is
  transmitted.
- Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
  statement to set correct number of lanes to enable.
- Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
  style of rest of file.
- Switch to %u instead of 0x%x when printing bus type.
- Switch to %u instead of %d for priv->lanes which is unsigned.
- Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
  rcar_csi2_formats[].
- Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
- Set INTSTATE after PL-11 is confirmed to match flow chart in
  datasheet.
- Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
- Add Maxime's and laurent's tags.
---
 drivers/media/platform/rcar-vin/Kconfig     |  12 +
 drivers/media/platform/rcar-vin/Makefile    |   1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
 3 files changed, 896 insertions(+)
 create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c

diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
index 8fa7ee468c63afb9..3dfeb91f8f186528 100644
--- a/drivers/media/platform/rcar-vin/Kconfig
+++ b/drivers/media/platform/rcar-vin/Kconfig
@@ -1,3 +1,15 @@
+config VIDEO_RCAR_CSI2
+	tristate "R-Car MIPI CSI-2 Receiver"
+	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select V4L2_FWNODE
+	---help---
+	  Support for Renesas R-Car MIPI CSI-2 receiver.
+	  Supports R-Car Gen3 SoCs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rcar-csi2.
+
 config VIDEO_RCAR_VIN
 	tristate "R-Car Video Input (VIN) Driver"
 	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
diff --git a/drivers/media/platform/rcar-vin/Makefile b/drivers/media/platform/rcar-vin/Makefile
index 48c5632c21dc060b..5ab803d3e7c1aa57 100644
--- a/drivers/media/platform/rcar-vin/Makefile
+++ b/drivers/media/platform/rcar-vin/Makefile
@@ -1,3 +1,4 @@
 rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o
 
+obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
 obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
new file mode 100644
index 0000000000000000..49b29d5680f9d80b
--- /dev/null
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -0,0 +1,883 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Renesas R-Car MIPI CSI-2 Receiver
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/sys_soc.h>
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+/* Register offsets and bits */
+
+/* Control Timing Select */
+#define TREF_REG			0x00
+#define TREF_TREF			BIT(0)
+
+/* Software Reset */
+#define SRST_REG			0x04
+#define SRST_SRST			BIT(0)
+
+/* PHY Operation Control */
+#define PHYCNT_REG			0x08
+#define PHYCNT_SHUTDOWNZ		BIT(17)
+#define PHYCNT_RSTZ			BIT(16)
+#define PHYCNT_ENABLECLK		BIT(4)
+#define PHYCNT_ENABLE_3			BIT(3)
+#define PHYCNT_ENABLE_2			BIT(2)
+#define PHYCNT_ENABLE_1			BIT(1)
+#define PHYCNT_ENABLE_0			BIT(0)
+
+/* Checksum Control */
+#define CHKSUM_REG			0x0c
+#define CHKSUM_ECC_EN			BIT(1)
+#define CHKSUM_CRC_EN			BIT(0)
+
+/*
+ * Channel Data Type Select
+ * VCDT[0-15]:  Channel 1 VCDT[16-31]:  Channel 2
+ * VCDT2[0-15]: Channel 3 VCDT2[16-31]: Channel 4
+ */
+#define VCDT_REG			0x10
+#define VCDT2_REG			0x14
+#define VCDT_VCDTN_EN			BIT(15)
+#define VCDT_SEL_VC(n)			(((n) & 0x3) << 8)
+#define VCDT_SEL_DTN_ON			BIT(6)
+#define VCDT_SEL_DT(n)			(((n) & 0x3f) << 0)
+
+/* Frame Data Type Select */
+#define FRDT_REG			0x18
+
+/* Field Detection Control */
+#define FLD_REG				0x1c
+#define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
+#define FLD_FLD_EN4			BIT(3)
+#define FLD_FLD_EN3			BIT(2)
+#define FLD_FLD_EN2			BIT(1)
+#define FLD_FLD_EN			BIT(0)
+
+/* Automatic Standby Control */
+#define ASTBY_REG			0x20
+
+/* Long Data Type Setting 0 */
+#define LNGDT0_REG			0x28
+
+/* Long Data Type Setting 1 */
+#define LNGDT1_REG			0x2c
+
+/* Interrupt Enable */
+#define INTEN_REG			0x30
+
+/* Interrupt Source Mask */
+#define INTCLOSE_REG			0x34
+
+/* Interrupt Status Monitor */
+#define INTSTATE_REG			0x38
+#define INTSTATE_INT_ULPS_START		BIT(7)
+#define INTSTATE_INT_ULPS_END		BIT(6)
+
+/* Interrupt Error Status Monitor */
+#define INTERRSTATE_REG			0x3c
+
+/* Short Packet Data */
+#define SHPDAT_REG			0x40
+
+/* Short Packet Count */
+#define SHPCNT_REG			0x44
+
+/* LINK Operation Control */
+#define LINKCNT_REG			0x48
+#define LINKCNT_MONITOR_EN		BIT(31)
+#define LINKCNT_REG_MONI_PACT_EN	BIT(25)
+#define LINKCNT_ICLK_NONSTOP		BIT(24)
+
+/* Lane Swap */
+#define LSWAP_REG			0x4c
+#define LSWAP_L3SEL(n)			(((n) & 0x3) << 6)
+#define LSWAP_L2SEL(n)			(((n) & 0x3) << 4)
+#define LSWAP_L1SEL(n)			(((n) & 0x3) << 2)
+#define LSWAP_L0SEL(n)			(((n) & 0x3) << 0)
+
+/* PHY Test Interface Write Register */
+#define PHTW_REG			0x50
+
+/* PHY Test Interface Clear */
+#define PHTC_REG			0x58
+#define PHTC_TESTCLR			BIT(0)
+
+/* PHY Frequency Control */
+#define PHYPLL_REG			0x68
+#define PHYPLL_HSFREQRANGE(n)		((n) << 16)
+
+struct phypll_hsfreqrange {
+	u16 mbps;
+	u16 reg;
+};
+
+static const struct phypll_hsfreqrange hsfreqrange_h3_v3h_m3n[] = {
+	{ .mbps =   80, .reg = 0x00 },
+	{ .mbps =   90, .reg = 0x10 },
+	{ .mbps =  100, .reg = 0x20 },
+	{ .mbps =  110, .reg = 0x30 },
+	{ .mbps =  120, .reg = 0x01 },
+	{ .mbps =  130, .reg = 0x11 },
+	{ .mbps =  140, .reg = 0x21 },
+	{ .mbps =  150, .reg = 0x31 },
+	{ .mbps =  160, .reg = 0x02 },
+	{ .mbps =  170, .reg = 0x12 },
+	{ .mbps =  180, .reg = 0x22 },
+	{ .mbps =  190, .reg = 0x32 },
+	{ .mbps =  205, .reg = 0x03 },
+	{ .mbps =  220, .reg = 0x13 },
+	{ .mbps =  235, .reg = 0x23 },
+	{ .mbps =  250, .reg = 0x33 },
+	{ .mbps =  275, .reg = 0x04 },
+	{ .mbps =  300, .reg = 0x14 },
+	{ .mbps =  325, .reg = 0x25 },
+	{ .mbps =  350, .reg = 0x35 },
+	{ .mbps =  400, .reg = 0x05 },
+	{ .mbps =  450, .reg = 0x26 },
+	{ .mbps =  500, .reg = 0x36 },
+	{ .mbps =  550, .reg = 0x37 },
+	{ .mbps =  600, .reg = 0x07 },
+	{ .mbps =  650, .reg = 0x18 },
+	{ .mbps =  700, .reg = 0x28 },
+	{ .mbps =  750, .reg = 0x39 },
+	{ .mbps =  800, .reg = 0x09 },
+	{ .mbps =  850, .reg = 0x19 },
+	{ .mbps =  900, .reg = 0x29 },
+	{ .mbps =  950, .reg = 0x3a },
+	{ .mbps = 1000, .reg = 0x0a },
+	{ .mbps = 1050, .reg = 0x1a },
+	{ .mbps = 1100, .reg = 0x2a },
+	{ .mbps = 1150, .reg = 0x3b },
+	{ .mbps = 1200, .reg = 0x0b },
+	{ .mbps = 1250, .reg = 0x1b },
+	{ .mbps = 1300, .reg = 0x2b },
+	{ .mbps = 1350, .reg = 0x3c },
+	{ .mbps = 1400, .reg = 0x0c },
+	{ .mbps = 1450, .reg = 0x1c },
+	{ .mbps = 1500, .reg = 0x2c },
+	/* guard */
+	{ .mbps =   0,	.reg = 0x00 },
+};
+
+static const struct phypll_hsfreqrange hsfreqrange_m3w_h3es1[] = {
+	{ .mbps =   80,	.reg = 0x00 },
+	{ .mbps =   90,	.reg = 0x10 },
+	{ .mbps =  100,	.reg = 0x20 },
+	{ .mbps =  110,	.reg = 0x30 },
+	{ .mbps =  120,	.reg = 0x01 },
+	{ .mbps =  130,	.reg = 0x11 },
+	{ .mbps =  140,	.reg = 0x21 },
+	{ .mbps =  150,	.reg = 0x31 },
+	{ .mbps =  160,	.reg = 0x02 },
+	{ .mbps =  170,	.reg = 0x12 },
+	{ .mbps =  180,	.reg = 0x22 },
+	{ .mbps =  190,	.reg = 0x32 },
+	{ .mbps =  205,	.reg = 0x03 },
+	{ .mbps =  220,	.reg = 0x13 },
+	{ .mbps =  235,	.reg = 0x23 },
+	{ .mbps =  250,	.reg = 0x33 },
+	{ .mbps =  275,	.reg = 0x04 },
+	{ .mbps =  300,	.reg = 0x14 },
+	{ .mbps =  325,	.reg = 0x05 },
+	{ .mbps =  350,	.reg = 0x15 },
+	{ .mbps =  400,	.reg = 0x25 },
+	{ .mbps =  450,	.reg = 0x06 },
+	{ .mbps =  500,	.reg = 0x16 },
+	{ .mbps =  550,	.reg = 0x07 },
+	{ .mbps =  600,	.reg = 0x17 },
+	{ .mbps =  650,	.reg = 0x08 },
+	{ .mbps =  700,	.reg = 0x18 },
+	{ .mbps =  750,	.reg = 0x09 },
+	{ .mbps =  800,	.reg = 0x19 },
+	{ .mbps =  850,	.reg = 0x29 },
+	{ .mbps =  900,	.reg = 0x39 },
+	{ .mbps =  950,	.reg = 0x0a },
+	{ .mbps = 1000,	.reg = 0x1a },
+	{ .mbps = 1050,	.reg = 0x2a },
+	{ .mbps = 1100,	.reg = 0x3a },
+	{ .mbps = 1150,	.reg = 0x0b },
+	{ .mbps = 1200,	.reg = 0x1b },
+	{ .mbps = 1250,	.reg = 0x2b },
+	{ .mbps = 1300,	.reg = 0x3b },
+	{ .mbps = 1350,	.reg = 0x0c },
+	{ .mbps = 1400,	.reg = 0x1c },
+	{ .mbps = 1450,	.reg = 0x2c },
+	{ .mbps = 1500,	.reg = 0x3c },
+	/* guard */
+	{ .mbps =   0,	.reg = 0x00 },
+};
+
+/* PHY ESC Error Monitor */
+#define PHEERM_REG			0x74
+
+/* PHY Clock Lane Monitor */
+#define PHCLM_REG			0x78
+#define PHCLM_STOPSTATECKL		BIT(0)
+
+/* PHY Data Lane Monitor */
+#define PHDLM_REG			0x7c
+
+/* CSI0CLK Frequency Configuration Preset Register */
+#define CSI0CLKFCPR_REG			0x260
+#define CSI0CLKFREQRANGE(n)		((n & 0x3f) << 16)
+
+struct rcar_csi2_format {
+	u32 code;
+	unsigned int datatype;
+	unsigned int bpp;
+};
+
+static const struct rcar_csi2_format rcar_csi2_formats[] = {
+	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
+	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
+	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
+	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
+	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
+};
+
+static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++)
+		if (rcar_csi2_formats[i].code == code)
+			return &rcar_csi2_formats[i];
+
+	return NULL;
+}
+
+enum rcar_csi2_pads {
+	RCAR_CSI2_SINK,
+	RCAR_CSI2_SOURCE_VC0,
+	RCAR_CSI2_SOURCE_VC1,
+	RCAR_CSI2_SOURCE_VC2,
+	RCAR_CSI2_SOURCE_VC3,
+	NR_OF_RCAR_CSI2_PAD,
+};
+
+struct rcar_csi2_info {
+	const struct phypll_hsfreqrange *hsfreqrange;
+	unsigned int csi0clkfreqrange;
+	bool clear_ulps;
+	bool init_phtw;
+};
+
+struct rcar_csi2 {
+	struct device *dev;
+	void __iomem *base;
+	const struct rcar_csi2_info *info;
+
+	struct v4l2_subdev subdev;
+	struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
+
+	struct v4l2_async_notifier notifier;
+	struct v4l2_async_subdev asd;
+	struct v4l2_subdev *remote;
+
+	struct v4l2_mbus_framefmt mf;
+
+	struct mutex lock;
+	int stream_count;
+
+	unsigned short lanes;
+	unsigned char lane_swap[4];
+};
+
+static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct rcar_csi2, subdev);
+}
+
+static inline struct rcar_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct rcar_csi2, notifier);
+}
+
+static u32 rcar_csi2_read(struct rcar_csi2 *priv, unsigned int reg)
+{
+	return ioread32(priv->base + reg);
+}
+
+static void rcar_csi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
+{
+	iowrite32(data, priv->base + reg);
+}
+
+static void rcar_csi2_reset(struct rcar_csi2 *priv)
+{
+	rcar_csi2_write(priv, SRST_REG, SRST_SRST);
+	usleep_range(100, 150);
+	rcar_csi2_write(priv, SRST_REG, 0);
+}
+
+static int rcar_csi2_wait_phy_start(struct rcar_csi2 *priv)
+{
+	int timeout;
+
+	/* Wait for the clock and data lanes to enter LP-11 state. */
+	for (timeout = 100; timeout > 0; timeout--) {
+		const u32 lane_mask = (1 << priv->lanes) - 1;
+
+		if ((rcar_csi2_read(priv, PHCLM_REG) & PHCLM_STOPSTATECKL)  &&
+		    (rcar_csi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
+			return 0;
+
+		msleep(20);
+	}
+
+	dev_err(priv->dev, "Timeout waiting for LP-11 state\n");
+
+	return -ETIMEDOUT;
+}
+
+static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
+				 u32 *phypll)
+{
+	const struct phypll_hsfreqrange *hsfreq;
+	struct v4l2_subdev *source;
+	struct v4l2_ctrl *ctrl;
+	u64 mbps;
+
+	if (!priv->remote)
+		return -ENODEV;
+
+	source = priv->remote;
+
+	/* Read the pixel rate control from remote */
+	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
+	if (!ctrl) {
+		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
+			source->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * Calculate the phypll in mbps (from v4l2 documentation)
+	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
+	 */
+	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
+	do_div(mbps, priv->lanes * 2000000);
+
+	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
+		if (hsfreq->mbps >= mbps)
+			break;
+
+	if (!hsfreq->mbps) {
+		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
+		return -ERANGE;
+	}
+
+	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
+		hsfreq->mbps);
+
+	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
+
+	return 0;
+}
+
+static int rcar_csi2_start(struct rcar_csi2 *priv)
+{
+	const struct rcar_csi2_format *format;
+	u32 phycnt, phypll, vcdt = 0, vcdt2 = 0;
+	unsigned int i;
+	int ret;
+
+	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
+		priv->mf.width, priv->mf.height,
+		priv->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
+
+	/* Code is validated in set_fmt */
+	format = rcar_csi2_code_to_fmt(priv->mf.code);
+
+	/*
+	 * Enable all Virtual Channels
+	 *
+	 * NOTE: It's not possible to get individual datatype for each
+	 *       source virtual channel. Once this is possible in V4L2
+	 *       it should be used here.
+	 */
+	for (i = 0; i < 4; i++) {
+		u32 vcdt_part;
+
+		vcdt_part = VCDT_SEL_VC(i) | VCDT_VCDTN_EN | VCDT_SEL_DTN_ON |
+			VCDT_SEL_DT(format->datatype);
+
+		/* Store in correct reg and offset */
+		if (i < 2)
+			vcdt |= vcdt_part << ((i % 2) * 16);
+		else
+			vcdt2 |= vcdt_part << ((i % 2) * 16);
+	}
+
+	phycnt = PHYCNT_ENABLECLK;
+	phycnt |= (1 << priv->lanes) - 1;
+
+	ret = rcar_csi2_calc_phypll(priv, format->bpp, &phypll);
+	if (ret)
+		return ret;
+
+	/* Init */
+	rcar_csi2_write(priv, TREF_REG, TREF_TREF);
+	rcar_csi2_reset(priv);
+	rcar_csi2_write(priv, PHTC_REG, 0);
+
+	/* Configure */
+	rcar_csi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
+			FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
+	rcar_csi2_write(priv, VCDT_REG, vcdt);
+	rcar_csi2_write(priv, VCDT2_REG, vcdt2);
+	/* Lanes are zero indexed */
+	rcar_csi2_write(priv, LSWAP_REG,
+			LSWAP_L0SEL(priv->lane_swap[0] - 1) |
+			LSWAP_L1SEL(priv->lane_swap[1] - 1) |
+			LSWAP_L2SEL(priv->lane_swap[2] - 1) |
+			LSWAP_L3SEL(priv->lane_swap[3] - 1));
+
+	if (priv->info->init_phtw) {
+		/*
+		 * This is for H3 ES2.0
+		 *
+		 * NOTE: Additional logic is needed here when
+		 * support for V3H and/or M3-N is added
+		 */
+		rcar_csi2_write(priv, PHTW_REG, 0x01cc01e2);
+		rcar_csi2_write(priv, PHTW_REG, 0x010101e3);
+		rcar_csi2_write(priv, PHTW_REG, 0x010101e4);
+		rcar_csi2_write(priv, PHTW_REG, 0x01100104);
+		rcar_csi2_write(priv, PHTW_REG, 0x01030100);
+		rcar_csi2_write(priv, PHTW_REG, 0x01800100);
+	}
+
+	/* Start */
+	rcar_csi2_write(priv, PHYPLL_REG, phypll);
+
+	/* Set frequency range if we have it */
+	if (priv->info->csi0clkfreqrange)
+		rcar_csi2_write(priv, CSI0CLKFCPR_REG,
+				CSI0CLKFREQRANGE(priv->info->csi0clkfreqrange));
+
+	rcar_csi2_write(priv, PHYCNT_REG, phycnt);
+	rcar_csi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
+			LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
+	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
+	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ |
+			PHYCNT_RSTZ);
+
+	ret = rcar_csi2_wait_phy_start(priv);
+	if (ret)
+		return ret;
+
+	/* Clear Ultra Low Power interrupt */
+	if (priv->info->clear_ulps)
+		rcar_csi2_write(priv, INTSTATE_REG,
+				INTSTATE_INT_ULPS_START |
+				INTSTATE_INT_ULPS_END);
+	return 0;
+}
+
+static void rcar_csi2_stop(struct rcar_csi2 *priv)
+{
+	rcar_csi2_write(priv, PHYCNT_REG, 0);
+
+	rcar_csi2_reset(priv);
+}
+
+static int rcar_csi2_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+	struct v4l2_subdev *nextsd;
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	if (!priv->remote) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	nextsd = priv->remote;
+
+	if (enable && priv->stream_count == 0) {
+		pm_runtime_get_sync(priv->dev);
+
+		ret = rcar_csi2_start(priv);
+		if (ret) {
+			pm_runtime_put(priv->dev);
+			goto out;
+		}
+
+		ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
+		if (ret) {
+			rcar_csi2_stop(priv);
+			pm_runtime_put(priv->dev);
+			goto out;
+		}
+	} else if (!enable && priv->stream_count == 1) {
+		rcar_csi2_stop(priv);
+		v4l2_subdev_call(nextsd, video, s_stream, 0);
+		pm_runtime_put(priv->dev);
+	}
+
+	priv->stream_count += enable ? 1 : -1;
+out:
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int rcar_csi2_set_pad_format(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_pad_config *cfg,
+				    struct v4l2_subdev_format *format)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+	struct v4l2_mbus_framefmt *framefmt;
+
+	if (!rcar_csi2_code_to_fmt(format->format.code))
+		return -EINVAL;
+
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+		priv->mf = format->format;
+	} else {
+		framefmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+		*framefmt = format->format;
+	}
+
+	return 0;
+}
+
+static int rcar_csi2_get_pad_format(struct v4l2_subdev *sd,
+				    struct v4l2_subdev_pad_config *cfg,
+				    struct v4l2_subdev_format *format)
+{
+	struct rcar_csi2 *priv = sd_to_csi2(sd);
+
+	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		format->format = priv->mf;
+	else
+		format->format = *v4l2_subdev_get_try_format(sd, cfg, 0);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
+	.s_stream = rcar_csi2_s_stream,
+};
+
+static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
+	.set_fmt = rcar_csi2_set_pad_format,
+	.get_fmt = rcar_csi2_get_pad_format,
+};
+
+static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
+	.video	= &rcar_csi2_video_ops,
+	.pad	= &rcar_csi2_pad_ops,
+};
+
+/* -----------------------------------------------------------------------------
+ * Async handling and registration of subdevices and links
+ */
+
+static int rcar_csi2_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
+	int pad;
+
+	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
+					  MEDIA_PAD_FL_SOURCE);
+	if (pad < 0) {
+		dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
+		return pad;
+	}
+
+	priv->remote = subdev;
+
+	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
+
+	return media_create_pad_link(&subdev->entity, pad,
+				     &priv->subdev.entity, 0,
+				     MEDIA_LNK_FL_ENABLED |
+				     MEDIA_LNK_FL_IMMUTABLE);
+}
+
+static void rcar_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
+				       struct v4l2_subdev *subdev,
+				       struct v4l2_async_subdev *asd)
+{
+	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
+
+	priv->remote = NULL;
+
+	dev_dbg(priv->dev, "Unbind %s\n", subdev->name);
+}
+
+static const struct v4l2_async_notifier_operations rcar_csi2_notify_ops = {
+	.bound = rcar_csi2_notify_bound,
+	.unbind = rcar_csi2_notify_unbind,
+};
+
+static int rcar_csi2_parse_v4l2(struct rcar_csi2 *priv,
+				struct v4l2_fwnode_endpoint *vep)
+{
+	unsigned int i;
+
+	/* Only port 0 endpoint 0 is valid */
+	if (vep->base.port || vep->base.id)
+		return -ENOTCONN;
+
+	if (vep->bus_type != V4L2_MBUS_CSI2) {
+		dev_err(priv->dev, "Unsupported bus: %u\n", vep->bus_type);
+		return -EINVAL;
+	}
+
+	priv->lanes = vep->bus.mipi_csi2.num_data_lanes;
+	if (priv->lanes != 1 && priv->lanes != 2 && priv->lanes != 4) {
+		dev_err(priv->dev, "Unsupported number of data-lanes: %u\n",
+			priv->lanes);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
+		priv->lane_swap[i] = i < priv->lanes ?
+			vep->bus.mipi_csi2.data_lanes[i] : i;
+
+		/* Check for valid lane number */
+		if (priv->lane_swap[i] < 1 || priv->lane_swap[i] > 4) {
+			dev_err(priv->dev, "data-lanes must be in 1-4 range\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
+{
+	struct device_node *ep;
+	struct v4l2_fwnode_endpoint v4l2_ep;
+	int ret;
+
+	ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
+	if (!ep) {
+		dev_err(priv->dev, "Not connected to subdevice\n");
+		return -EINVAL;
+	}
+
+	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
+	if (ret) {
+		dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
+		of_node_put(ep);
+		return -EINVAL;
+	}
+
+	ret = rcar_csi2_parse_v4l2(priv, &v4l2_ep);
+	if (ret) {
+		of_node_put(ep);
+		return ret;
+	}
+
+	priv->asd.match.fwnode =
+		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
+	priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
+
+	of_node_put(ep);
+
+	priv->notifier.subdevs = devm_kzalloc(priv->dev,
+					      sizeof(*priv->notifier.subdevs),
+					      GFP_KERNEL);
+	if (!priv->notifier.subdevs)
+		return -ENOMEM;
+
+	priv->notifier.num_subdevs = 1;
+	priv->notifier.subdevs[0] = &priv->asd;
+	priv->notifier.ops = &rcar_csi2_notify_ops;
+
+	dev_dbg(priv->dev, "Found '%pOF'\n",
+		to_of_node(priv->asd.match.fwnode));
+
+	return v4l2_async_subdev_notifier_register(&priv->subdev,
+						   &priv->notifier);
+}
+
+/* -----------------------------------------------------------------------------
+ * Platform Device Driver
+ */
+
+static const struct media_entity_operations rcar_csi2_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static int rcar_csi2_probe_resources(struct rcar_csi2 *priv,
+				     struct platform_device *pdev)
+{
+	struct resource *res;
+	int irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	return 0;
+}
+
+static const struct rcar_csi2_info rcar_csi2_info_r8a7795 = {
+	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
+	.clear_ulps = true,
+	.init_phtw = true,
+	.csi0clkfreqrange = 0x20,
+};
+
+static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
+	.hsfreqrange = hsfreqrange_m3w_h3es1,
+};
+
+static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
+	.hsfreqrange = hsfreqrange_m3w_h3es1,
+};
+
+static const struct of_device_id rcar_csi2_of_table[] = {
+	{
+		.compatible = "renesas,r8a7795-csi2",
+		.data = &rcar_csi2_info_r8a7795,
+	},
+	{
+		.compatible = "renesas,r8a7796-csi2",
+		.data = &rcar_csi2_info_r8a7796,
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
+
+static const struct soc_device_attribute r8a7795es1[] = {
+	{
+		.soc_id = "r8a7795", .revision = "ES1.*",
+		.data = &rcar_csi2_info_r8a7795es1,
+	},
+	{ /* sentinel */}
+};
+
+static int rcar_csi2_probe(struct platform_device *pdev)
+{
+	const struct soc_device_attribute *attr;
+	struct rcar_csi2 *priv;
+	unsigned int i;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->info = of_device_get_match_data(&pdev->dev);
+
+	/*
+	 * r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
+	 * have it's own compatible string.
+	 */
+	attr = soc_device_match(r8a7795es1);
+	if (attr)
+		priv->info = attr->data;
+
+	priv->dev = &pdev->dev;
+
+	mutex_init(&priv->lock);
+	priv->stream_count = 0;
+
+	ret = rcar_csi2_probe_resources(priv, pdev);
+	if (ret) {
+		dev_err(priv->dev, "Failed to get resources\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	ret = rcar_csi2_parse_dt(priv);
+	if (ret)
+		return ret;
+
+	priv->subdev.owner = THIS_MODULE;
+	priv->subdev.dev = &pdev->dev;
+	v4l2_subdev_init(&priv->subdev, &rcar_csi2_subdev_ops);
+	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
+	snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
+		 KBUILD_MODNAME, dev_name(&pdev->dev));
+	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
+	priv->subdev.entity.ops = &rcar_csi2_entity_ops;
+
+	priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
+	for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
+		priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
+
+	ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
+				     priv->pads);
+	if (ret)
+		goto error;
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = v4l2_async_register_subdev(&priv->subdev);
+	if (ret < 0)
+		goto error;
+
+	dev_info(priv->dev, "%d lanes found\n", priv->lanes);
+
+	return 0;
+
+error:
+	v4l2_async_notifier_unregister(&priv->notifier);
+	v4l2_async_notifier_cleanup(&priv->notifier);
+
+	return ret;
+}
+
+static int rcar_csi2_remove(struct platform_device *pdev)
+{
+	struct rcar_csi2 *priv = platform_get_drvdata(pdev);
+
+	v4l2_async_notifier_unregister(&priv->notifier);
+	v4l2_async_notifier_cleanup(&priv->notifier);
+	v4l2_async_unregister_subdev(&priv->subdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static struct platform_driver __refdata rcar_csi2_pdrv = {
+	.remove	= rcar_csi2_remove,
+	.probe	= rcar_csi2_probe,
+	.driver	= {
+		.name	= "rcar-csi2",
+		.of_match_table	= rcar_csi2_of_table,
+	},
+};
+
+module_platform_driver(rcar_csi2_pdrv);
+
+MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund@ragnatech.se>");
+MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
+MODULE_LICENSE("GPL");
-- 
2.17.0

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-26 20:21 ` [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver Niklas Söderlund
@ 2018-04-26 21:30   ` Laurent Pinchart
  2018-04-26 23:28       ` Niklas Söderlund
  2018-04-28 11:28   ` jacopo mondi
  1 sibling, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2018-04-26 21:30 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Niklas,

Thank you for the patch.

On Thursday, 26 April 2018 23:21:21 EEST Niklas Söderlund wrote:
> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> connected between the video sources and the video grabbers (VIN).
> 
> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> 
> * Changes since v13
> - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> - Add define for PHCLM_STOPSTATECKL.
> - Update spelling in comments.
> - Update calculation in rcar_csi2_calc_phypll() according to
>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
>   before v14 did not take into account that 2 bits per sample is
>   transmitted.

Just one small comment about this, please see below.

> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
>   statement to set correct number of lanes to enable.
> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
>   style of rest of file.
> - Switch to %u instead of 0x%x when printing bus type.
> - Switch to %u instead of %d for priv->lanes which is unsigned.
> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
>   rcar_csi2_formats[].
> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> - Set INTSTATE after PL-11 is confirmed to match flow chart in
>   datasheet.
> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> - Add Maxime's and laurent's tags.
> ---
>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
>  drivers/media/platform/rcar-vin/Makefile    |   1 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
>  3 files changed, 896 insertions(+)
>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c

[snip]


> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> index 0000000000000000..49b29d5680f9d80b
> --- /dev/null
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c

[snip]

> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
> +				 u32 *phypll)
> +{
> +	const struct phypll_hsfreqrange *hsfreq;
> +	struct v4l2_subdev *source;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	if (!priv->remote)
> +		return -ENODEV;
> +
> +	source = priv->remote;
> +
> +	/* Read the pixel rate control from remote */
> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> +	if (!ctrl) {
> +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> +			source->name);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Calculate the phypll in mbps (from v4l2 documentation)
> +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> +	 */
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> +	do_div(mbps, priv->lanes * 2000000);

pixel rate * bits per sample will give you the overall bit rate, which you 
then divide by the number of lanes to get the bitrate per lane, and then by 2 
as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then end up 
with the link frequency, which is thus expressed in MHz, not in Mbps. I would 
thus name the mbps variable freq, and rename the phypll_hsfreqrange mbps field 
to freq (maybe with a small comment right after the field to tell the value is 
expressed in MHz).

> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> +		if (hsfreq->mbps >= mbps)
> +			break;
> +
> +	if (!hsfreq->mbps) {
> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> +		hsfreq->mbps);
> +
> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> +
> +	return 0;
> +}

[snip]

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-26 21:30   ` Laurent Pinchart
@ 2018-04-26 23:28       ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-26 23:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Laurent,

Thanks for your feedback.

On 2018-04-27 00:30:25 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 23:21:21 EEST Niklas Söderlund wrote:
> > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> > connected between the video sources and the video grabbers (VIN).
> > 
> > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> > Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > ---
> > 
> > * Changes since v13
> > - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> > - Add define for PHCLM_STOPSTATECKL.
> > - Update spelling in comments.
> > - Update calculation in rcar_csi2_calc_phypll() according to
> >   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> >   before v14 did not take into account that 2 bits per sample is
> >   transmitted.
> 
> Just one small comment about this, please see below.
> 
> > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> >   statement to set correct number of lanes to enable.
> > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> >   style of rest of file.
> > - Switch to %u instead of 0x%x when printing bus type.
> > - Switch to %u instead of %d for priv->lanes which is unsigned.
> > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> >   rcar_csi2_formats[].
> > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> > - Set INTSTATE after PL-11 is confirmed to match flow chart in
> >   datasheet.
> > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> > - Add Maxime's and laurent's tags.
> > ---
> >  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> >  drivers/media/platform/rcar-vin/Makefile    |   1 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
> >  3 files changed, 896 insertions(+)
> >  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> 
> [snip]
> 
> 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> > index 0000000000000000..49b29d5680f9d80b
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> 
> [snip]
> 
> > +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
> > +				 u32 *phypll)
> > +{
> > +	const struct phypll_hsfreqrange *hsfreq;
> > +	struct v4l2_subdev *source;
> > +	struct v4l2_ctrl *ctrl;
> > +	u64 mbps;
> > +
> > +	if (!priv->remote)
> > +		return -ENODEV;
> > +
> > +	source = priv->remote;
> > +
> > +	/* Read the pixel rate control from remote */
> > +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > +	if (!ctrl) {
> > +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> > +			source->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * Calculate the phypll in mbps (from v4l2 documentation)
> > +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> > +	 */
> > +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> > +	do_div(mbps, priv->lanes * 2000000);
> 
> pixel rate * bits per sample will give you the overall bit rate, which you 
> then divide by the number of lanes to get the bitrate per lane, and then by 2 
> as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then end up 
> with the link frequency, which is thus expressed in MHz, not in Mbps. I would 
> thus name the mbps variable freq, and rename the phypll_hsfreqrange mbps field 
> to freq (maybe with a small comment right after the field to tell the value is 
> expressed in MHz).

I agree that freq would be a better name for what it represents. Never 
the less I prefer to stick with mbps as that is whats used in the 
datasheet. See Table '25.9 HSFREQRANGE Bit Set Values'.

With this in mind if you still feel strongly about renaming it I could 
do so. But at lest for me it feels more useful to easily be able to map 
the variable to the datasheet tables :-)

> 
> > +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > +		if (hsfreq->mbps >= mbps)
> > +			break;
> > +
> > +	if (!hsfreq->mbps) {
> > +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > +		return -ERANGE;
> > +	}
> > +
> > +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> > +		hsfreq->mbps);
> > +
> > +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > +
> > +	return 0;
> > +}
> 
> [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
@ 2018-04-26 23:28       ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-26 23:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Laurent,

Thanks for your feedback.

On 2018-04-27 00:30:25 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 23:21:21 EEST Niklas S�derlund wrote:
> > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> > connected between the video sources and the video grabbers (VIN).
> > 
> > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > 
> > Signed-off-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> > Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > ---
> > 
> > * Changes since v13
> > - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> > - Add define for PHCLM_STOPSTATECKL.
> > - Update spelling in comments.
> > - Update calculation in rcar_csi2_calc_phypll() according to
> >   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> >   before v14 did not take into account that 2 bits per sample is
> >   transmitted.
> 
> Just one small comment about this, please see below.
> 
> > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> >   statement to set correct number of lanes to enable.
> > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> >   style of rest of file.
> > - Switch to %u instead of 0x%x when printing bus type.
> > - Switch to %u instead of %d for priv->lanes which is unsigned.
> > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> >   rcar_csi2_formats[].
> > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> > - Set INTSTATE after PL-11 is confirmed to match flow chart in
> >   datasheet.
> > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> > - Add Maxime's and laurent's tags.
> > ---
> >  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> >  drivers/media/platform/rcar-vin/Makefile    |   1 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
> >  3 files changed, 896 insertions(+)
> >  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> 
> [snip]
> 
> 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> > index 0000000000000000..49b29d5680f9d80b
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> 
> [snip]
> 
> > +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
> > +				 u32 *phypll)
> > +{
> > +	const struct phypll_hsfreqrange *hsfreq;
> > +	struct v4l2_subdev *source;
> > +	struct v4l2_ctrl *ctrl;
> > +	u64 mbps;
> > +
> > +	if (!priv->remote)
> > +		return -ENODEV;
> > +
> > +	source = priv->remote;
> > +
> > +	/* Read the pixel rate control from remote */
> > +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > +	if (!ctrl) {
> > +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> > +			source->name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/*
> > +	 * Calculate the phypll in mbps (from v4l2 documentation)
> > +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> > +	 */
> > +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> > +	do_div(mbps, priv->lanes * 2000000);
> 
> pixel rate * bits per sample will give you the overall bit rate, which you 
> then divide by the number of lanes to get the bitrate per lane, and then by 2 
> as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then end up 
> with the link frequency, which is thus expressed in MHz, not in Mbps. I would 
> thus name the mbps variable freq, and rename the phypll_hsfreqrange mbps field 
> to freq (maybe with a small comment right after the field to tell the value is 
> expressed in MHz).

I agree that freq would be a better name for what it represents. Never 
the less I prefer to stick with mbps as that is whats used in the 
datasheet. See Table '25.9 HSFREQRANGE Bit Set Values'.

With this in mind if you still feel strongly about renaming it I could 
do so. But at lest for me it feels more useful to easily be able to map 
the variable to the datasheet tables :-)

> 
> > +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > +		if (hsfreq->mbps >= mbps)
> > +			break;
> > +
> > +	if (!hsfreq->mbps) {
> > +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > +		return -ERANGE;
> > +	}
> > +
> > +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> > +		hsfreq->mbps);
> > +
> > +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > +
> > +	return 0;
> > +}
> 
> [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas S�derlund

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-26 20:21 ` [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver Niklas Söderlund
  2018-04-26 21:30   ` Laurent Pinchart
@ 2018-04-28 11:28   ` jacopo mondi
  2018-04-28 13:31       ` Niklas Söderlund
  1 sibling, 1 reply; 13+ messages in thread
From: jacopo mondi @ 2018-04-28 11:28 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

[-- Attachment #1: Type: text/plain, Size: 29534 bytes --]

Hi Niklas,
   apart from a small comment, as my comments on v13 have been
   clarified

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

On Thu, Apr 26, 2018 at 10:21:21PM +0200, Niklas Söderlund wrote:
> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> connected between the video sources and the video grabbers (VIN).
>
> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> ---
>
> * Changes since v13
> - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> - Add define for PHCLM_STOPSTATECKL.
> - Update spelling in comments.
> - Update calculation in rcar_csi2_calc_phypll() according to
>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
>   before v14 did not take into account that 2 bits per sample is
>   transmitted.
> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
>   statement to set correct number of lanes to enable.
> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
>   style of rest of file.
> - Switch to %u instead of 0x%x when printing bus type.
> - Switch to %u instead of %d for priv->lanes which is unsigned.
> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
>   rcar_csi2_formats[].
> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> - Set INTSTATE after PL-11 is confirmed to match flow chart in
>   datasheet.
> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> - Add Maxime's and laurent's tags.
> ---
>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
>  drivers/media/platform/rcar-vin/Makefile    |   1 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
>  3 files changed, 896 insertions(+)
>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
>
> diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
> index 8fa7ee468c63afb9..3dfeb91f8f186528 100644
> --- a/drivers/media/platform/rcar-vin/Kconfig
> +++ b/drivers/media/platform/rcar-vin/Kconfig
> @@ -1,3 +1,15 @@
> +config VIDEO_RCAR_CSI2
> +	tristate "R-Car MIPI CSI-2 Receiver"
> +	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	select V4L2_FWNODE
> +	---help---
> +	  Support for Renesas R-Car MIPI CSI-2 receiver.
> +	  Supports R-Car Gen3 SoCs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rcar-csi2.
> +
>  config VIDEO_RCAR_VIN
>  	tristate "R-Car Video Input (VIN) Driver"
>  	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER
> diff --git a/drivers/media/platform/rcar-vin/Makefile b/drivers/media/platform/rcar-vin/Makefile
> index 48c5632c21dc060b..5ab803d3e7c1aa57 100644
> --- a/drivers/media/platform/rcar-vin/Makefile
> +++ b/drivers/media/platform/rcar-vin/Makefile
> @@ -1,3 +1,4 @@
>  rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o
>
> +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
>  obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> new file mode 100644
> index 0000000000000000..49b29d5680f9d80b
> --- /dev/null
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -0,0 +1,883 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> + *
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/sys_soc.h>
> +
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-mc.h>
> +#include <media/v4l2-subdev.h>
> +
> +/* Register offsets and bits */
> +
> +/* Control Timing Select */
> +#define TREF_REG			0x00
> +#define TREF_TREF			BIT(0)
> +
> +/* Software Reset */
> +#define SRST_REG			0x04
> +#define SRST_SRST			BIT(0)
> +
> +/* PHY Operation Control */
> +#define PHYCNT_REG			0x08
> +#define PHYCNT_SHUTDOWNZ		BIT(17)
> +#define PHYCNT_RSTZ			BIT(16)
> +#define PHYCNT_ENABLECLK		BIT(4)
> +#define PHYCNT_ENABLE_3			BIT(3)
> +#define PHYCNT_ENABLE_2			BIT(2)
> +#define PHYCNT_ENABLE_1			BIT(1)
> +#define PHYCNT_ENABLE_0			BIT(0)
> +
> +/* Checksum Control */
> +#define CHKSUM_REG			0x0c
> +#define CHKSUM_ECC_EN			BIT(1)
> +#define CHKSUM_CRC_EN			BIT(0)
> +
> +/*
> + * Channel Data Type Select
> + * VCDT[0-15]:  Channel 1 VCDT[16-31]:  Channel 2
> + * VCDT2[0-15]: Channel 3 VCDT2[16-31]: Channel 4
> + */
> +#define VCDT_REG			0x10
> +#define VCDT2_REG			0x14
> +#define VCDT_VCDTN_EN			BIT(15)
> +#define VCDT_SEL_VC(n)			(((n) & 0x3) << 8)
> +#define VCDT_SEL_DTN_ON			BIT(6)
> +#define VCDT_SEL_DT(n)			(((n) & 0x3f) << 0)
> +
> +/* Frame Data Type Select */
> +#define FRDT_REG			0x18
> +
> +/* Field Detection Control */
> +#define FLD_REG				0x1c
> +#define FLD_FLD_NUM(n)			(((n) & 0xff) << 16)
> +#define FLD_FLD_EN4			BIT(3)
> +#define FLD_FLD_EN3			BIT(2)
> +#define FLD_FLD_EN2			BIT(1)
> +#define FLD_FLD_EN			BIT(0)
> +
> +/* Automatic Standby Control */
> +#define ASTBY_REG			0x20
> +
> +/* Long Data Type Setting 0 */
> +#define LNGDT0_REG			0x28
> +
> +/* Long Data Type Setting 1 */
> +#define LNGDT1_REG			0x2c
> +
> +/* Interrupt Enable */
> +#define INTEN_REG			0x30
> +
> +/* Interrupt Source Mask */
> +#define INTCLOSE_REG			0x34
> +
> +/* Interrupt Status Monitor */
> +#define INTSTATE_REG			0x38
> +#define INTSTATE_INT_ULPS_START		BIT(7)
> +#define INTSTATE_INT_ULPS_END		BIT(6)
> +
> +/* Interrupt Error Status Monitor */
> +#define INTERRSTATE_REG			0x3c
> +
> +/* Short Packet Data */
> +#define SHPDAT_REG			0x40
> +
> +/* Short Packet Count */
> +#define SHPCNT_REG			0x44
> +
> +/* LINK Operation Control */
> +#define LINKCNT_REG			0x48
> +#define LINKCNT_MONITOR_EN		BIT(31)
> +#define LINKCNT_REG_MONI_PACT_EN	BIT(25)
> +#define LINKCNT_ICLK_NONSTOP		BIT(24)
> +
> +/* Lane Swap */
> +#define LSWAP_REG			0x4c
> +#define LSWAP_L3SEL(n)			(((n) & 0x3) << 6)
> +#define LSWAP_L2SEL(n)			(((n) & 0x3) << 4)
> +#define LSWAP_L1SEL(n)			(((n) & 0x3) << 2)
> +#define LSWAP_L0SEL(n)			(((n) & 0x3) << 0)
> +
> +/* PHY Test Interface Write Register */
> +#define PHTW_REG			0x50
> +
> +/* PHY Test Interface Clear */
> +#define PHTC_REG			0x58
> +#define PHTC_TESTCLR			BIT(0)
> +
> +/* PHY Frequency Control */
> +#define PHYPLL_REG			0x68
> +#define PHYPLL_HSFREQRANGE(n)		((n) << 16)
> +
> +struct phypll_hsfreqrange {
> +	u16 mbps;
> +	u16 reg;
> +};
> +
> +static const struct phypll_hsfreqrange hsfreqrange_h3_v3h_m3n[] = {
> +	{ .mbps =   80, .reg = 0x00 },
> +	{ .mbps =   90, .reg = 0x10 },
> +	{ .mbps =  100, .reg = 0x20 },
> +	{ .mbps =  110, .reg = 0x30 },
> +	{ .mbps =  120, .reg = 0x01 },
> +	{ .mbps =  130, .reg = 0x11 },
> +	{ .mbps =  140, .reg = 0x21 },
> +	{ .mbps =  150, .reg = 0x31 },
> +	{ .mbps =  160, .reg = 0x02 },
> +	{ .mbps =  170, .reg = 0x12 },
> +	{ .mbps =  180, .reg = 0x22 },
> +	{ .mbps =  190, .reg = 0x32 },
> +	{ .mbps =  205, .reg = 0x03 },
> +	{ .mbps =  220, .reg = 0x13 },
> +	{ .mbps =  235, .reg = 0x23 },
> +	{ .mbps =  250, .reg = 0x33 },
> +	{ .mbps =  275, .reg = 0x04 },
> +	{ .mbps =  300, .reg = 0x14 },
> +	{ .mbps =  325, .reg = 0x25 },
> +	{ .mbps =  350, .reg = 0x35 },
> +	{ .mbps =  400, .reg = 0x05 },
> +	{ .mbps =  450, .reg = 0x26 },
> +	{ .mbps =  500, .reg = 0x36 },
> +	{ .mbps =  550, .reg = 0x37 },
> +	{ .mbps =  600, .reg = 0x07 },
> +	{ .mbps =  650, .reg = 0x18 },
> +	{ .mbps =  700, .reg = 0x28 },
> +	{ .mbps =  750, .reg = 0x39 },
> +	{ .mbps =  800, .reg = 0x09 },
> +	{ .mbps =  850, .reg = 0x19 },
> +	{ .mbps =  900, .reg = 0x29 },
> +	{ .mbps =  950, .reg = 0x3a },
> +	{ .mbps = 1000, .reg = 0x0a },
> +	{ .mbps = 1050, .reg = 0x1a },
> +	{ .mbps = 1100, .reg = 0x2a },
> +	{ .mbps = 1150, .reg = 0x3b },
> +	{ .mbps = 1200, .reg = 0x0b },
> +	{ .mbps = 1250, .reg = 0x1b },
> +	{ .mbps = 1300, .reg = 0x2b },
> +	{ .mbps = 1350, .reg = 0x3c },
> +	{ .mbps = 1400, .reg = 0x0c },
> +	{ .mbps = 1450, .reg = 0x1c },
> +	{ .mbps = 1500, .reg = 0x2c },
> +	/* guard */
> +	{ .mbps =   0,	.reg = 0x00 },
> +};
> +
> +static const struct phypll_hsfreqrange hsfreqrange_m3w_h3es1[] = {
> +	{ .mbps =   80,	.reg = 0x00 },
> +	{ .mbps =   90,	.reg = 0x10 },
> +	{ .mbps =  100,	.reg = 0x20 },
> +	{ .mbps =  110,	.reg = 0x30 },
> +	{ .mbps =  120,	.reg = 0x01 },
> +	{ .mbps =  130,	.reg = 0x11 },
> +	{ .mbps =  140,	.reg = 0x21 },
> +	{ .mbps =  150,	.reg = 0x31 },
> +	{ .mbps =  160,	.reg = 0x02 },
> +	{ .mbps =  170,	.reg = 0x12 },
> +	{ .mbps =  180,	.reg = 0x22 },
> +	{ .mbps =  190,	.reg = 0x32 },
> +	{ .mbps =  205,	.reg = 0x03 },
> +	{ .mbps =  220,	.reg = 0x13 },
> +	{ .mbps =  235,	.reg = 0x23 },
> +	{ .mbps =  250,	.reg = 0x33 },
> +	{ .mbps =  275,	.reg = 0x04 },
> +	{ .mbps =  300,	.reg = 0x14 },
> +	{ .mbps =  325,	.reg = 0x05 },
> +	{ .mbps =  350,	.reg = 0x15 },
> +	{ .mbps =  400,	.reg = 0x25 },
> +	{ .mbps =  450,	.reg = 0x06 },
> +	{ .mbps =  500,	.reg = 0x16 },
> +	{ .mbps =  550,	.reg = 0x07 },
> +	{ .mbps =  600,	.reg = 0x17 },
> +	{ .mbps =  650,	.reg = 0x08 },
> +	{ .mbps =  700,	.reg = 0x18 },
> +	{ .mbps =  750,	.reg = 0x09 },
> +	{ .mbps =  800,	.reg = 0x19 },
> +	{ .mbps =  850,	.reg = 0x29 },
> +	{ .mbps =  900,	.reg = 0x39 },
> +	{ .mbps =  950,	.reg = 0x0a },
> +	{ .mbps = 1000,	.reg = 0x1a },
> +	{ .mbps = 1050,	.reg = 0x2a },
> +	{ .mbps = 1100,	.reg = 0x3a },
> +	{ .mbps = 1150,	.reg = 0x0b },
> +	{ .mbps = 1200,	.reg = 0x1b },
> +	{ .mbps = 1250,	.reg = 0x2b },
> +	{ .mbps = 1300,	.reg = 0x3b },
> +	{ .mbps = 1350,	.reg = 0x0c },
> +	{ .mbps = 1400,	.reg = 0x1c },
> +	{ .mbps = 1450,	.reg = 0x2c },
> +	{ .mbps = 1500,	.reg = 0x3c },
> +	/* guard */
> +	{ .mbps =   0,	.reg = 0x00 },
> +};
> +
> +/* PHY ESC Error Monitor */
> +#define PHEERM_REG			0x74
> +
> +/* PHY Clock Lane Monitor */
> +#define PHCLM_REG			0x78
> +#define PHCLM_STOPSTATECKL		BIT(0)
> +
> +/* PHY Data Lane Monitor */
> +#define PHDLM_REG			0x7c
> +
> +/* CSI0CLK Frequency Configuration Preset Register */
> +#define CSI0CLKFCPR_REG			0x260
> +#define CSI0CLKFREQRANGE(n)		((n & 0x3f) << 16)
> +
> +struct rcar_csi2_format {
> +	u32 code;
> +	unsigned int datatype;
> +	unsigned int bpp;
> +};
> +
> +static const struct rcar_csi2_format rcar_csi2_formats[] = {
> +	{ .code = MEDIA_BUS_FMT_RGB888_1X24,	.datatype = 0x24, .bpp = 24 },
> +	{ .code = MEDIA_BUS_FMT_UYVY8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
> +	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
> +	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
> +};
> +
> +static const struct rcar_csi2_format *rcar_csi2_code_to_fmt(unsigned int code)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rcar_csi2_formats); i++)
> +		if (rcar_csi2_formats[i].code == code)
> +			return &rcar_csi2_formats[i];
> +
> +	return NULL;
> +}
> +
> +enum rcar_csi2_pads {
> +	RCAR_CSI2_SINK,
> +	RCAR_CSI2_SOURCE_VC0,
> +	RCAR_CSI2_SOURCE_VC1,
> +	RCAR_CSI2_SOURCE_VC2,
> +	RCAR_CSI2_SOURCE_VC3,
> +	NR_OF_RCAR_CSI2_PAD,
> +};
> +
> +struct rcar_csi2_info {
> +	const struct phypll_hsfreqrange *hsfreqrange;
> +	unsigned int csi0clkfreqrange;
> +	bool clear_ulps;
> +	bool init_phtw;
> +};
> +
> +struct rcar_csi2 {
> +	struct device *dev;
> +	void __iomem *base;
> +	const struct rcar_csi2_info *info;
> +
> +	struct v4l2_subdev subdev;
> +	struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
> +
> +	struct v4l2_async_notifier notifier;
> +	struct v4l2_async_subdev asd;
> +	struct v4l2_subdev *remote;
> +
> +	struct v4l2_mbus_framefmt mf;
> +
> +	struct mutex lock;
> +	int stream_count;
> +
> +	unsigned short lanes;
> +	unsigned char lane_swap[4];
> +};
> +
> +static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct rcar_csi2, subdev);
> +}
> +
> +static inline struct rcar_csi2 *notifier_to_csi2(struct v4l2_async_notifier *n)
> +{
> +	return container_of(n, struct rcar_csi2, notifier);
> +}
> +
> +static u32 rcar_csi2_read(struct rcar_csi2 *priv, unsigned int reg)
> +{
> +	return ioread32(priv->base + reg);
> +}
> +
> +static void rcar_csi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
> +{
> +	iowrite32(data, priv->base + reg);
> +}
> +
> +static void rcar_csi2_reset(struct rcar_csi2 *priv)
> +{
> +	rcar_csi2_write(priv, SRST_REG, SRST_SRST);
> +	usleep_range(100, 150);
> +	rcar_csi2_write(priv, SRST_REG, 0);
> +}
> +
> +static int rcar_csi2_wait_phy_start(struct rcar_csi2 *priv)
> +{
> +	int timeout;
> +
> +	/* Wait for the clock and data lanes to enter LP-11 state. */
> +	for (timeout = 100; timeout > 0; timeout--) {
> +		const u32 lane_mask = (1 << priv->lanes) - 1;
> +
> +		if ((rcar_csi2_read(priv, PHCLM_REG) & PHCLM_STOPSTATECKL)  &&
> +		    (rcar_csi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
> +			return 0;
> +
> +		msleep(20);
> +	}
> +
> +	dev_err(priv->dev, "Timeout waiting for LP-11 state\n");
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int bpp,
> +				 u32 *phypll)
> +{
> +	const struct phypll_hsfreqrange *hsfreq;
> +	struct v4l2_subdev *source;
> +	struct v4l2_ctrl *ctrl;
> +	u64 mbps;
> +
> +	if (!priv->remote)
> +		return -ENODEV;
> +
> +	source = priv->remote;
> +
> +	/* Read the pixel rate control from remote */
> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> +	if (!ctrl) {
> +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> +			source->name);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Calculate the phypll in mbps (from v4l2 documentation)
> +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> +	 */
> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> +	do_div(mbps, priv->lanes * 2000000);
> +
> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> +		if (hsfreq->mbps >= mbps)
> +			break;
> +
> +	if (!hsfreq->mbps) {
> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> +		return -ERANGE;
> +	}
> +
> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n", mbps,
> +		hsfreq->mbps);
> +
> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_start(struct rcar_csi2 *priv)
> +{
> +	const struct rcar_csi2_format *format;
> +	u32 phycnt, phypll, vcdt = 0, vcdt2 = 0;
> +	unsigned int i;
> +	int ret;
> +
> +	dev_dbg(priv->dev, "Input size (%ux%u%c)\n",
> +		priv->mf.width, priv->mf.height,
> +		priv->mf.field == V4L2_FIELD_NONE ? 'p' : 'i');
> +
> +	/* Code is validated in set_fmt */
> +	format = rcar_csi2_code_to_fmt(priv->mf.code);
> +
> +	/*
> +	 * Enable all Virtual Channels
> +	 *
> +	 * NOTE: It's not possible to get individual datatype for each
> +	 *       source virtual channel. Once this is possible in V4L2
> +	 *       it should be used here.
> +	 */
> +	for (i = 0; i < 4; i++) {
> +		u32 vcdt_part;
> +
> +		vcdt_part = VCDT_SEL_VC(i) | VCDT_VCDTN_EN | VCDT_SEL_DTN_ON |
> +			VCDT_SEL_DT(format->datatype);
> +
> +		/* Store in correct reg and offset */
> +		if (i < 2)
> +			vcdt |= vcdt_part << ((i % 2) * 16);
> +		else
> +			vcdt2 |= vcdt_part << ((i % 2) * 16);
> +	}
> +
> +	phycnt = PHYCNT_ENABLECLK;
> +	phycnt |= (1 << priv->lanes) - 1;
> +
> +	ret = rcar_csi2_calc_phypll(priv, format->bpp, &phypll);
> +	if (ret)
> +		return ret;
> +
> +	/* Init */
> +	rcar_csi2_write(priv, TREF_REG, TREF_TREF);
> +	rcar_csi2_reset(priv);
> +	rcar_csi2_write(priv, PHTC_REG, 0);
> +
> +	/* Configure */
> +	rcar_csi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> +			FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> +	rcar_csi2_write(priv, VCDT_REG, vcdt);
> +	rcar_csi2_write(priv, VCDT2_REG, vcdt2);
> +	/* Lanes are zero indexed */
> +	rcar_csi2_write(priv, LSWAP_REG,
> +			LSWAP_L0SEL(priv->lane_swap[0] - 1) |
> +			LSWAP_L1SEL(priv->lane_swap[1] - 1) |
> +			LSWAP_L2SEL(priv->lane_swap[2] - 1) |
> +			LSWAP_L3SEL(priv->lane_swap[3] - 1));
> +
> +	if (priv->info->init_phtw) {
> +		/*
> +		 * This is for H3 ES2.0
> +		 *
> +		 * NOTE: Additional logic is needed here when
> +		 * support for V3H and/or M3-N is added
> +		 */
> +		rcar_csi2_write(priv, PHTW_REG, 0x01cc01e2);
> +		rcar_csi2_write(priv, PHTW_REG, 0x010101e3);
> +		rcar_csi2_write(priv, PHTW_REG, 0x010101e4);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01100104);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01030100);
> +		rcar_csi2_write(priv, PHTW_REG, 0x01800100);
> +	}
> +
> +	/* Start */
> +	rcar_csi2_write(priv, PHYPLL_REG, phypll);
> +
> +	/* Set frequency range if we have it */
> +	if (priv->info->csi0clkfreqrange)
> +		rcar_csi2_write(priv, CSI0CLKFCPR_REG,
> +				CSI0CLKFREQRANGE(priv->info->csi0clkfreqrange));
> +
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt);
> +	rcar_csi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> +			LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> +	rcar_csi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ |
> +			PHYCNT_RSTZ);
> +
> +	ret = rcar_csi2_wait_phy_start(priv);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear Ultra Low Power interrupt */
> +	if (priv->info->clear_ulps)
> +		rcar_csi2_write(priv, INTSTATE_REG,
> +				INTSTATE_INT_ULPS_START |
> +				INTSTATE_INT_ULPS_END);
> +	return 0;
> +}
> +
> +static void rcar_csi2_stop(struct rcar_csi2 *priv)
> +{
> +	rcar_csi2_write(priv, PHYCNT_REG, 0);
> +
> +	rcar_csi2_reset(priv);
> +}
> +
> +static int rcar_csi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	struct v4l2_subdev *nextsd;
> +	int ret = 0;
> +
> +	mutex_lock(&priv->lock);
> +
> +	if (!priv->remote) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	nextsd = priv->remote;
> +
> +	if (enable && priv->stream_count == 0) {
> +		pm_runtime_get_sync(priv->dev);
> +
> +		ret = rcar_csi2_start(priv);
> +		if (ret) {
> +			pm_runtime_put(priv->dev);
> +			goto out;
> +		}
> +
> +		ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> +		if (ret) {
> +			rcar_csi2_stop(priv);
> +			pm_runtime_put(priv->dev);
> +			goto out;
> +		}
> +	} else if (!enable && priv->stream_count == 1) {
> +		rcar_csi2_stop(priv);
> +		v4l2_subdev_call(nextsd, video, s_stream, 0);
> +		pm_runtime_put(priv->dev);
> +	}
> +
> +	priv->stream_count += enable ? 1 : -1;
> +out:
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int rcar_csi2_set_pad_format(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *format)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +	struct v4l2_mbus_framefmt *framefmt;
> +
> +	if (!rcar_csi2_code_to_fmt(format->format.code))
> +		return -EINVAL;
> +
> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +		priv->mf = format->format;
> +	} else {
> +		framefmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +		*framefmt = format->format;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_get_pad_format(struct v4l2_subdev *sd,
> +				    struct v4l2_subdev_pad_config *cfg,
> +				    struct v4l2_subdev_format *format)
> +{
> +	struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> +	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +		format->format = priv->mf;
> +	else
> +		format->format = *v4l2_subdev_get_try_format(sd, cfg, 0);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
> +	.s_stream = rcar_csi2_s_stream,
> +};
> +
> +static const struct v4l2_subdev_pad_ops rcar_csi2_pad_ops = {
> +	.set_fmt = rcar_csi2_set_pad_format,
> +	.get_fmt = rcar_csi2_get_pad_format,
> +};
> +
> +static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> +	.video	= &rcar_csi2_video_ops,
> +	.pad	= &rcar_csi2_pad_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * Async handling and registration of subdevices and links
> + */
> +
> +static int rcar_csi2_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> +	int pad;
> +
> +	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
> +					  MEDIA_PAD_FL_SOURCE);
> +	if (pad < 0) {
> +		dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
> +		return pad;
> +	}
> +
> +	priv->remote = subdev;
> +
> +	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
> +
> +	return media_create_pad_link(&subdev->entity, pad,
> +				     &priv->subdev.entity, 0,
> +				     MEDIA_LNK_FL_ENABLED |
> +				     MEDIA_LNK_FL_IMMUTABLE);
> +}
> +
> +static void rcar_csi2_notify_unbind(struct v4l2_async_notifier *notifier,
> +				       struct v4l2_subdev *subdev,
> +				       struct v4l2_async_subdev *asd)
> +{
> +	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
> +
> +	priv->remote = NULL;
> +
> +	dev_dbg(priv->dev, "Unbind %s\n", subdev->name);
> +}
> +
> +static const struct v4l2_async_notifier_operations rcar_csi2_notify_ops = {
> +	.bound = rcar_csi2_notify_bound,
> +	.unbind = rcar_csi2_notify_unbind,
> +};
> +
> +static int rcar_csi2_parse_v4l2(struct rcar_csi2 *priv,
> +				struct v4l2_fwnode_endpoint *vep)
> +{
> +	unsigned int i;
> +
> +	/* Only port 0 endpoint 0 is valid */
> +	if (vep->base.port || vep->base.id)
> +		return -ENOTCONN;
> +
> +	if (vep->bus_type != V4L2_MBUS_CSI2) {
> +		dev_err(priv->dev, "Unsupported bus: %u\n", vep->bus_type);
> +		return -EINVAL;
> +	}
> +
> +	priv->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +	if (priv->lanes != 1 && priv->lanes != 2 && priv->lanes != 4) {
> +		dev_err(priv->dev, "Unsupported number of data-lanes: %u\n",
> +			priv->lanes);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
> +		priv->lane_swap[i] = i < priv->lanes ?
> +			vep->bus.mipi_csi2.data_lanes[i] : i;
> +
> +		/* Check for valid lane number */
> +		if (priv->lane_swap[i] < 1 || priv->lane_swap[i] > 4) {
> +			dev_err(priv->dev, "data-lanes must be in 1-4 range\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> +{
> +	struct device_node *ep;
> +	struct v4l2_fwnode_endpoint v4l2_ep;
> +	int ret;
> +
> +	ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> +	if (!ep) {
> +		dev_err(priv->dev, "Not connected to subdevice\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &v4l2_ep);
> +	if (ret) {
> +		dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
> +		of_node_put(ep);
> +		return -EINVAL;
> +	}
> +
> +	ret = rcar_csi2_parse_v4l2(priv, &v4l2_ep);
> +	if (ret) {
> +		of_node_put(ep);
> +		return ret;
> +	}
> +
> +	priv->asd.match.fwnode =
> +		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> +	priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> +
> +	of_node_put(ep);
> +
> +	priv->notifier.subdevs = devm_kzalloc(priv->dev,
> +					      sizeof(*priv->notifier.subdevs),
> +					      GFP_KERNEL);
> +	if (!priv->notifier.subdevs)
> +		return -ENOMEM;
> +
> +	priv->notifier.num_subdevs = 1;
> +	priv->notifier.subdevs[0] = &priv->asd;
> +	priv->notifier.ops = &rcar_csi2_notify_ops;
> +
> +	dev_dbg(priv->dev, "Found '%pOF'\n",
> +		to_of_node(priv->asd.match.fwnode));
> +
> +	return v4l2_async_subdev_notifier_register(&priv->subdev,
> +						   &priv->notifier);
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Platform Device Driver
> + */
> +
> +static const struct media_entity_operations rcar_csi2_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static int rcar_csi2_probe_resources(struct rcar_csi2 *priv,
> +				     struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int irq;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	return 0;
> +}
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7795 = {
> +	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
> +	.clear_ulps = true,
> +	.init_phtw = true,
> +	.csi0clkfreqrange = 0x20,
> +};
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
> +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> +};
> +
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
> +	.hsfreqrange = hsfreqrange_m3w_h3es1,
> +};
> +
> +static const struct of_device_id rcar_csi2_of_table[] = {
> +	{
> +		.compatible = "renesas,r8a7795-csi2",
> +		.data = &rcar_csi2_info_r8a7795,
> +	},
> +	{
> +		.compatible = "renesas,r8a7796-csi2",
> +		.data = &rcar_csi2_info_r8a7796,
> +	},
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> +
> +static const struct soc_device_attribute r8a7795es1[] = {
> +	{
> +		.soc_id = "r8a7795", .revision = "ES1.*",
> +		.data = &rcar_csi2_info_r8a7795es1,
> +	},
> +	{ /* sentinel */}
> +};
> +
> +static int rcar_csi2_probe(struct platform_device *pdev)
> +{
> +	const struct soc_device_attribute *attr;
> +	struct rcar_csi2 *priv;
> +	unsigned int i;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->info = of_device_get_match_data(&pdev->dev);
> +
> +	/*
> +	 * r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
> +	 * have it's own compatible string.
> +	 */
> +	attr = soc_device_match(r8a7795es1);
> +	if (attr)
> +		priv->info = attr->data;
> +
> +	priv->dev = &pdev->dev;
> +
> +	mutex_init(&priv->lock);
> +	priv->stream_count = 0;
> +
> +	ret = rcar_csi2_probe_resources(priv, pdev);
> +	if (ret) {
> +		dev_err(priv->dev, "Failed to get resources\n");
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	ret = rcar_csi2_parse_dt(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->subdev.owner = THIS_MODULE;
> +	priv->subdev.dev = &pdev->dev;
> +	v4l2_subdev_init(&priv->subdev, &rcar_csi2_subdev_ops);
> +	v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
> +	snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> +		 KBUILD_MODNAME, dev_name(&pdev->dev));
> +	priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> +	priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> +
> +	priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> +	for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> +		priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> +	ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
> +				     priv->pads);
> +	if (ret)
> +		goto error;
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = v4l2_async_register_subdev(&priv->subdev);
> +	if (ret < 0)
> +		goto error;
> +
> +	dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> +
> +	return 0;
> +
> +error:
> +	v4l2_async_notifier_unregister(&priv->notifier);
> +	v4l2_async_notifier_cleanup(&priv->notifier);
> +
> +	return ret;
> +}
> +
> +static int rcar_csi2_remove(struct platform_device *pdev)
> +{
> +	struct rcar_csi2 *priv = platform_get_drvdata(pdev);
> +
> +	v4l2_async_notifier_unregister(&priv->notifier);
> +	v4l2_async_notifier_cleanup(&priv->notifier);
> +	v4l2_async_unregister_subdev(&priv->subdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver __refdata rcar_csi2_pdrv = {
> +	.remove	= rcar_csi2_remove,
> +	.probe	= rcar_csi2_probe,
> +	.driver	= {
> +		.name	= "rcar-csi2",
> +		.of_match_table	= rcar_csi2_of_table,
> +	},
> +};
> +
> +module_platform_driver(rcar_csi2_pdrv);
> +
> +MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund@ragnatech.se>");
> +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> +MODULE_LICENSE("GPL");

This doesn't match the SPDX header that reports GPL-2.0

Thank you!
    j

> --
> 2.17.0
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-28 11:28   ` jacopo mondi
@ 2018-04-28 13:31       ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-28 13:31 UTC (permalink / raw)
  To: jacopo mondi
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Jacopo,

Thanks for your feedback.

On 2018-04-28 13:28:27 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>    apart from a small comment, as my comments on v13 have been
>    clarified
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks!

[snip]

> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > new file mode 100644
> > index 0000000000000000..49b29d5680f9d80b
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -0,0 +1,883 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Corp.
> > + */

[snip]

> > +MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund@ragnatech.se>");
> > +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> > +MODULE_LICENSE("GPL");
> 
> This doesn't match the SPDX header that reports GPL-2.0

I'm now officially more confused then normal :-) I really tried to get 
this right and the combination I use here

    // SPDX-License-Identifier: GPL-2.0
    MODULE_LICENSE("GPL");

Seems to be used all over the kernel, did some digging on the master 
branch of the media tree from a few days ago and found 265 files with 
this combination using this script:

    count=0
    for f in $(git grep -l "SPDX-License-Identifier: GPL-2.0$"); do
	    if grep -q 'MODULE_LICENSE("GPL")' $f; then
                echo $f
                grep SPDX-License-Identifier $f
                grep MODULE_LICENSE $f;
                count=$(($count + 1))
	    fi
    done
    echo "Count: $count"

I'm happy to post a new version of this series to make this right but 
I'm afraid that I at this point know what right is. My intention is to 
replace a licence text found in an old Renesas BSP which this work is 
loosely based on:

    * This program is free software; you can redistribute it and/or modify
    * it under the terms of the GNU General Public License version 2 as
    * published by the Free Software Foundation.

So it's quiet clear it's GPL-2.0 and not GPL-2.0+ and AFIK what I have done 
here is correct, please tell me why I'm wrong and how I can correct it :-)

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
@ 2018-04-28 13:31       ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-04-28 13:31 UTC (permalink / raw)
  To: jacopo mondi
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Jacopo,

Thanks for your feedback.

On 2018-04-28 13:28:27 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>    apart from a small comment, as my comments on v13 have been
>    clarified
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks!

[snip]

> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > new file mode 100644
> > index 0000000000000000..49b29d5680f9d80b
> > --- /dev/null
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -0,0 +1,883 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Corp.
> > + */

[snip]

> > +MODULE_AUTHOR("Niklas S�derlund <niklas.soderlund@ragnatech.se>");
> > +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> > +MODULE_LICENSE("GPL");
> 
> This doesn't match the SPDX header that reports GPL-2.0

I'm now officially more confused then normal :-) I really tried to get 
this right and the combination I use here

    // SPDX-License-Identifier: GPL-2.0
    MODULE_LICENSE("GPL");

Seems to be used all over the kernel, did some digging on the master 
branch of the media tree from a few days ago and found 265 files with 
this combination using this script:

    count=0
    for f in $(git grep -l "SPDX-License-Identifier: GPL-2.0$"); do
	    if grep -q 'MODULE_LICENSE("GPL")' $f; then
                echo $f
                grep SPDX-License-Identifier $f
                grep MODULE_LICENSE $f;
                count=$(($count + 1))
	    fi
    done
    echo "Count: $count"

I'm happy to post a new version of this series to make this right but 
I'm afraid that I at this point know what right is. My intention is to 
replace a licence text found in an old Renesas BSP which this work is 
loosely based on:

    * This program is free software; you can redistribute it and/or modify
    * it under the terms of the GNU General Public License version 2 as
    * published by the Free Software Foundation.

So it's quiet clear it's GPL-2.0 and not GPL-2.0+ and AFIK what I have done 
here is correct, please tell me why I'm wrong and how I can correct it :-)

-- 
Regards,
Niklas S�derlund

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-28 13:31       ` Niklas Söderlund
  (?)
@ 2018-04-28 13:54       ` jacopo mondi
  -1 siblings, 0 replies; 13+ messages in thread
From: jacopo mondi @ 2018-04-28 13:54 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, Sakari Ailus,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

Hi Niklas,

On Sat, Apr 28, 2018 at 03:31:14PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your feedback.
>
> On 2018-04-28 13:28:27 +0200, Jacopo Mondi wrote:
> > Hi Niklas,
> >    apart from a small comment, as my comments on v13 have been
> >    clarified
> >
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks!
>
> [snip]
>
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > new file mode 100644
> > > index 0000000000000000..49b29d5680f9d80b
> > > --- /dev/null
> > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > @@ -0,0 +1,883 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for Renesas R-Car MIPI CSI-2 Receiver
> > > + *
> > > + * Copyright (C) 2018 Renesas Electronics Corp.
> > > + */
>
> [snip]
>
> > > +MODULE_AUTHOR("Niklas Söderlund <niklas.soderlund@ragnatech.se>");
> > > +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> > > +MODULE_LICENSE("GPL");
> >
> > This doesn't match the SPDX header that reports GPL-2.0
>
> I'm now officially more confused then normal :-) I really tried to get
> this right and the combination I use here
>
>     // SPDX-License-Identifier: GPL-2.0
>     MODULE_LICENSE("GPL");
>
> Seems to be used all over the kernel, did some digging on the master
> branch of the media tree from a few days ago and found 265 files with
> this combination using this script:
>
>     count=0
>     for f in $(git grep -l "SPDX-License-Identifier: GPL-2.0$"); do
> 	    if grep -q 'MODULE_LICENSE("GPL")' $f; then
>                 echo $f
>                 grep SPDX-License-Identifier $f
>                 grep MODULE_LICENSE $f;
>                 count=$(($count + 1))
> 	    fi
>     done
>     echo "Count: $count"
>
> I'm happy to post a new version of this series to make this right but
> I'm afraid that I at this point know what right is. My intention is to
> replace a licence text found in an old Renesas BSP which this work is
> loosely based on:
>
>     * This program is free software; you can redistribute it and/or modify
>     * it under the terms of the GNU General Public License version 2 as
>     * published by the Free Software Foundation.
>
> So it's quiet clear it's GPL-2.0 and not GPL-2.0+ and AFIK what I have done
> here is correct, please tell me why I'm wrong and how I can correct it :-)
>

I was just expecting to see "GPL v2" if the SPDX identifier reports
GPL-2.0

Maybe that's not even a thing, anyway, do not waste any time on this,
it a very minor nit.

Thank you
  j

> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-26 23:28       ` Niklas Söderlund
  (?)
@ 2018-04-28 16:01       ` Laurent Pinchart
  2018-05-13 19:08           ` Niklas Söderlund
  -1 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2018-04-28 16:01 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Niklas,

On Friday, 27 April 2018 02:28:32 EEST Niklas Söderlund wrote:
> On 2018-04-27 00:30:25 +0300, Laurent Pinchart wrote:
> > On Thursday, 26 April 2018 23:21:21 EEST Niklas Söderlund wrote:
> >> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> >> supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> >> connected between the video sources and the video grabbers (VIN).
> >> 
> >> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> >> 
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> 
> >> ---
> >> 
> >> * Changes since v13
> >> - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> >> - Add define for PHCLM_STOPSTATECKL.
> >> - Update spelling in comments.
> >> - Update calculation in rcar_csi2_calc_phypll() according to
> >>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> >>   before v14 did not take into account that 2 bits per sample is
> >>   transmitted.
> > 
> > Just one small comment about this, please see below.
> > 
> >> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> >>   statement to set correct number of lanes to enable.
> >> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> >>   style of rest of file.
> >> - Switch to %u instead of 0x%x when printing bus type.
> >> - Switch to %u instead of %d for priv->lanes which is unsigned.
> >> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> >>   rcar_csi2_formats[].
> >> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> >> - Set INTSTATE after PL-11 is confirmed to match flow chart in
> >>   datasheet.
> >> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> >> - Add Maxime's and laurent's tags.
> >> ---
> >> 
> >>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> >>  drivers/media/platform/rcar-vin/Makefile    |   1 +
> >>  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
> >>  3 files changed, 896 insertions(+)
> >>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> > 
> > [snip]
> > 
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> >> index 0000000000000000..49b29d5680f9d80b
> >> --- /dev/null
> > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > 
> > [snip]
> > 
> >> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int
> >> bpp,
> >> +				 u32 *phypll)
> >> +{
> >> +	const struct phypll_hsfreqrange *hsfreq;
> >> +	struct v4l2_subdev *source;
> >> +	struct v4l2_ctrl *ctrl;
> >> +	u64 mbps;
> >> +
> >> +	if (!priv->remote)
> >> +		return -ENODEV;
> >> +
> >> +	source = priv->remote;
> >> +
> >> +	/* Read the pixel rate control from remote */
> >> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> >> +	if (!ctrl) {
> >> +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> >> +			source->name);
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Calculate the phypll in mbps (from v4l2 documentation)
> >> +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> >> +	 */
> >> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> >> +	do_div(mbps, priv->lanes * 2000000);
> > 
> > pixel rate * bits per sample will give you the overall bit rate, which you
> > then divide by the number of lanes to get the bitrate per lane, and then
> > by 2 as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then
> > end up with the link frequency, which is thus expressed in MHz, not in
> > Mbps. I would thus name the mbps variable freq, and rename the
> > phypll_hsfreqrange mbps field to freq (maybe with a small comment right
> > after the field to tell the value is expressed in MHz).
> 
> I agree that freq would be a better name for what it represents. Never
> the less I prefer to stick with mbps as that is whats used in the
> datasheet. See Table '25.9 HSFREQRANGE Bit Set Values'.
> 
> With this in mind if you still feel strongly about renaming it I could
> do so. But at lest for me it feels more useful to easily be able to map
> the variable to the datasheet tables :-)

After reading the datasheet I don't care too strongly about a rename, but I 
now care about getting it right :-) The datasheet clearly specifies the 
maximum data rate to be 1.5 Gbps / lane (section 25.1.1 Features), and figure 
25.9 corroborates that and shows a corresponding frequency of 750 MHz. I thus 
believe that the values in table 25.9 and 25.10 are indeed bitrates (in Mbps / 
lane), so I think you shouldn't divide by 2 in the above formula.

> >> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> >> +		if (hsfreq->mbps >= mbps)
> >> +			break;
> >> +
> >> +	if (!hsfreq->mbps) {
> >> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> >> +		return -ERANGE;
> >> +	}
> >> +
> >> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n",
> >> mbps,
> >> +		hsfreq->mbps);
> >> +
> >> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> >> +
> >> +	return 0;
> >> +}
> > 
> > [snip]

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
  2018-04-28 16:01       ` Laurent Pinchart
@ 2018-05-13 19:08           ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-05-13 19:08 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Laurent,

On 2018-04-28 19:01:37 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Friday, 27 April 2018 02:28:32 EEST Niklas Söderlund wrote:
> > On 2018-04-27 00:30:25 +0300, Laurent Pinchart wrote:
> > > On Thursday, 26 April 2018 23:21:21 EEST Niklas Söderlund wrote:
> > >> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > >> supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> > >> connected between the video sources and the video grabbers (VIN).
> > >> 
> > >> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > >> 
> > >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> > >> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >> 
> > >> ---
> > >> 
> > >> * Changes since v13
> > >> - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> > >> - Add define for PHCLM_STOPSTATECKL.
> > >> - Update spelling in comments.
> > >> - Update calculation in rcar_csi2_calc_phypll() according to
> > >>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> > >>   before v14 did not take into account that 2 bits per sample is
> > >>   transmitted.
> > > 
> > > Just one small comment about this, please see below.
> > > 
> > >> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> > >>   statement to set correct number of lanes to enable.
> > >> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> > >>   style of rest of file.
> > >> - Switch to %u instead of 0x%x when printing bus type.
> > >> - Switch to %u instead of %d for priv->lanes which is unsigned.
> > >> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> > >>   rcar_csi2_formats[].
> > >> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> > >> - Set INTSTATE after PL-11 is confirmed to match flow chart in
> > >>   datasheet.
> > >> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> > >> - Add Maxime's and laurent's tags.
> > >> ---
> > >> 
> > >>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> > >>  drivers/media/platform/rcar-vin/Makefile    |   1 +
> > >>  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
> > >>  3 files changed, 896 insertions(+)
> > >>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> > > 
> > > [snip]
> > > 
> > >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > >> b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> > >> index 0000000000000000..49b29d5680f9d80b
> > >> --- /dev/null
> > > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > 
> > > [snip]
> > > 
> > >> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int
> > >> bpp,
> > >> +				 u32 *phypll)
> > >> +{
> > >> +	const struct phypll_hsfreqrange *hsfreq;
> > >> +	struct v4l2_subdev *source;
> > >> +	struct v4l2_ctrl *ctrl;
> > >> +	u64 mbps;
> > >> +
> > >> +	if (!priv->remote)
> > >> +		return -ENODEV;
> > >> +
> > >> +	source = priv->remote;
> > >> +
> > >> +	/* Read the pixel rate control from remote */
> > >> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > >> +	if (!ctrl) {
> > >> +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> > >> +			source->name);
> > >> +		return -EINVAL;
> > >> +	}
> > >> +
> > >> +	/*
> > >> +	 * Calculate the phypll in mbps (from v4l2 documentation)
> > >> +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> > >> +	 */
> > >> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> > >> +	do_div(mbps, priv->lanes * 2000000);
> > > 
> > > pixel rate * bits per sample will give you the overall bit rate, which you
> > > then divide by the number of lanes to get the bitrate per lane, and then
> > > by 2 as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then
> > > end up with the link frequency, which is thus expressed in MHz, not in
> > > Mbps. I would thus name the mbps variable freq, and rename the
> > > phypll_hsfreqrange mbps field to freq (maybe with a small comment right
> > > after the field to tell the value is expressed in MHz).
> > 
> > I agree that freq would be a better name for what it represents. Never
> > the less I prefer to stick with mbps as that is whats used in the
> > datasheet. See Table '25.9 HSFREQRANGE Bit Set Values'.
> > 
> > With this in mind if you still feel strongly about renaming it I could
> > do so. But at lest for me it feels more useful to easily be able to map
> > the variable to the datasheet tables :-)
> 
> After reading the datasheet I don't care too strongly about a rename, but I 
> now care about getting it right :-) The datasheet clearly specifies the 
> maximum data rate to be 1.5 Gbps / lane (section 25.1.1 Features), and figure 
> 25.9 corroborates that and shows a corresponding frequency of 750 MHz. I thus 
> believe that the values in table 25.9 and 25.10 are indeed bitrates (in Mbps / 
> lane), so I think you shouldn't divide by 2 in the above formula.

As this now appears to have been fully sorted out on the adv748x end I'm 
now in full agreement with you and I will not divide by 2 to get the 
bitrate here for the CSI-2 driver.

> 
> > >> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > >> +		if (hsfreq->mbps >= mbps)
> > >> +			break;
> > >> +
> > >> +	if (!hsfreq->mbps) {
> > >> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > >> +		return -ERANGE;
> > >> +	}
> > >> +
> > >> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n",
> > >> mbps,
> > >> +		hsfreq->mbps);
> > >> +
> > >> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > >> +
> > >> +	return 0;
> > >> +}
> > > 
> > > [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
@ 2018-05-13 19:08           ` Niklas Söderlund
  0 siblings, 0 replies; 13+ messages in thread
From: Niklas Söderlund @ 2018-05-13 19:08 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Sakari Ailus, jacopo mondi,
	linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven

Hi Laurent,

On 2018-04-28 19:01:37 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Friday, 27 April 2018 02:28:32 EEST Niklas S�derlund wrote:
> > On 2018-04-27 00:30:25 +0300, Laurent Pinchart wrote:
> > > On Thursday, 26 April 2018 23:21:21 EEST Niklas S�derlund wrote:
> > >> A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > >> supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are
> > >> connected between the video sources and the video grabbers (VIN).
> > >> 
> > >> Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > >> 
> > >> Signed-off-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > >> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> > >> Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >> 
> > >> ---
> > >> 
> > >> * Changes since v13
> > >> - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i].
> > >> - Add define for PHCLM_STOPSTATECKL.
> > >> - Update spelling in comments.
> > >> - Update calculation in rcar_csi2_calc_phypll() according to
> > >>   https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one
> > >>   before v14 did not take into account that 2 bits per sample is
> > >>   transmitted.
> > > 
> > > Just one small comment about this, please see below.
> > > 
> > >> - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch
> > >>   statement to set correct number of lanes to enable.
> > >> - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match
> > >>   style of rest of file.
> > >> - Switch to %u instead of 0x%x when printing bus type.
> > >> - Switch to %u instead of %d for priv->lanes which is unsigned.
> > >> - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in
> > >>   rcar_csi2_formats[].
> > >> - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16.
> > >> - Set INTSTATE after PL-11 is confirmed to match flow chart in
> > >>   datasheet.
> > >> - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs.
> > >> - Add Maxime's and laurent's tags.
> > >> ---
> > >> 
> > >>  drivers/media/platform/rcar-vin/Kconfig     |  12 +
> > >>  drivers/media/platform/rcar-vin/Makefile    |   1 +
> > >>  drivers/media/platform/rcar-vin/rcar-csi2.c | 883 ++++++++++++++++++++
> > >>  3 files changed, 896 insertions(+)
> > >>  create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> > > 
> > > [snip]
> > > 
> > >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > >> b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644
> > >> index 0000000000000000..49b29d5680f9d80b
> > >> --- /dev/null
> > > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > > 
> > > [snip]
> > > 
> > >> +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv, unsigned int
> > >> bpp,
> > >> +				 u32 *phypll)
> > >> +{
> > >> +	const struct phypll_hsfreqrange *hsfreq;
> > >> +	struct v4l2_subdev *source;
> > >> +	struct v4l2_ctrl *ctrl;
> > >> +	u64 mbps;
> > >> +
> > >> +	if (!priv->remote)
> > >> +		return -ENODEV;
> > >> +
> > >> +	source = priv->remote;
> > >> +
> > >> +	/* Read the pixel rate control from remote */
> > >> +	ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > >> +	if (!ctrl) {
> > >> +		dev_err(priv->dev, "no pixel rate control in subdev %s\n",
> > >> +			source->name);
> > >> +		return -EINVAL;
> > >> +	}
> > >> +
> > >> +	/*
> > >> +	 * Calculate the phypll in mbps (from v4l2 documentation)
> > >> +	 * link_freq = (pixel_rate * bits_per_sample) / (2 * nr_of_lanes)
> > >> +	 */
> > >> +	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> > >> +	do_div(mbps, priv->lanes * 2000000);
> > > 
> > > pixel rate * bits per sample will give you the overall bit rate, which you
> > > then divide by the number of lanes to get the bitrate per lane, and then
> > > by 2 as D-PHY is a DDR PHY and transmits 2 bits per clock cycle. You then
> > > end up with the link frequency, which is thus expressed in MHz, not in
> > > Mbps. I would thus name the mbps variable freq, and rename the
> > > phypll_hsfreqrange mbps field to freq (maybe with a small comment right
> > > after the field to tell the value is expressed in MHz).
> > 
> > I agree that freq would be a better name for what it represents. Never
> > the less I prefer to stick with mbps as that is whats used in the
> > datasheet. See Table '25.9 HSFREQRANGE Bit Set Values'.
> > 
> > With this in mind if you still feel strongly about renaming it I could
> > do so. But at lest for me it feels more useful to easily be able to map
> > the variable to the datasheet tables :-)
> 
> After reading the datasheet I don't care too strongly about a rename, but I 
> now care about getting it right :-) The datasheet clearly specifies the 
> maximum data rate to be 1.5 Gbps / lane (section 25.1.1 Features), and figure 
> 25.9 corroborates that and shows a corresponding frequency of 750 MHz. I thus 
> believe that the values in table 25.9 and 25.10 are indeed bitrates (in Mbps / 
> lane), so I think you shouldn't divide by 2 in the above formula.

As this now appears to have been fully sorted out on the adv748x end I'm 
now in full agreement with you and I will not divide by 2 to get the 
bitrate here for the CSI-2 driver.

> 
> > >> +	for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> > >> +		if (hsfreq->mbps >= mbps)
> > >> +			break;
> > >> +
> > >> +	if (!hsfreq->mbps) {
> > >> +		dev_err(priv->dev, "Unsupported PHY speed (%llu Mbps)", mbps);
> > >> +		return -ERANGE;
> > >> +	}
> > >> +
> > >> +	dev_dbg(priv->dev, "PHY HSFREQRANGE requested %llu got %u Mbps\n",
> > >> mbps,
> > >> +		hsfreq->mbps);
> > >> +
> > >> +	*phypll = PHYPLL_HSFREQRANGE(hsfreq->reg);
> > >> +
> > >> +	return 0;
> > >> +}
> > > 
> > > [snip]
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas S�derlund

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

end of thread, other threads:[~2018-05-13 19:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 20:21 [PATCH v14 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 Niklas Söderlund
2018-04-26 20:21 ` [PATCH v14 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation Niklas Söderlund
2018-04-26 20:21 ` [PATCH v14 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver Niklas Söderlund
2018-04-26 21:30   ` Laurent Pinchart
2018-04-26 23:28     ` Niklas Söderlund
2018-04-26 23:28       ` Niklas Söderlund
2018-04-28 16:01       ` Laurent Pinchart
2018-05-13 19:08         ` Niklas Söderlund
2018-05-13 19:08           ` Niklas Söderlund
2018-04-28 11:28   ` jacopo mondi
2018-04-28 13:31     ` Niklas Söderlund
2018-04-28 13:31       ` Niklas Söderlund
2018-04-28 13:54       ` jacopo mondi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.