Hi Keke
sorry for the late feedback, hope you're still interested in
upstreaming this driver
On Wed, Sep 18, 2024 at 02:07:13PM +0800, Keke Li via B4 Relay wrote:
From: Keke Li <keke.li@xxxxxxxxxxx>
This driver is used to receive mipi data from image sensor.
Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
Signed-off-by: Keke Li <keke.li@xxxxxxxxxxx>
---
MAINTAINERS | 7 +
drivers/media/platform/amlogic/Kconfig | 1 +
drivers/media/platform/amlogic/Makefile | 2 +
.../media/platform/amlogic/c3-mipi-csi2/Kconfig | 16 +
.../media/platform/amlogic/c3-mipi-csi2/Makefile | 3 +
.../platform/amlogic/c3-mipi-csi2/c3-mipi-csi2.c | 910 +++++++++++++++++++++
6 files changed, 939 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2cdd7cacec86..9e75874a6e69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1209,6 +1209,13 @@ F: Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
F: drivers/perf/amlogic/
F: include/soc/amlogic/
+AMLOGIC MIPI CSI2 DRIVER
+M: Keke Li <keke.li@xxxxxxxxxxx>
+L: linux-media@xxxxxxxxxxxxxxx
+S: Maintained
+F: Documentation/devicetree/bindings/media/amlogic,c3-mipi-csi2.yaml
+F: drivers/media/platform/amlogic/c3-mipi-csi2/
+
AMPHENOL CHIPCAP 2 HUMIDITY-TEMPERATURE IIO DRIVER
M: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
L: linux-hwmon@xxxxxxxxxxxxxxx
diff --git a/drivers/media/platform/amlogic/Kconfig b/drivers/media/platform/amlogic/Kconfig
index 5014957404e9..b7c2de14848b 100644
--- a/drivers/media/platform/amlogic/Kconfig
+++ b/drivers/media/platform/amlogic/Kconfig
@@ -2,4 +2,5 @@
comment "Amlogic media platform drivers"
+source "drivers/media/platform/amlogic/c3-mipi-csi2/Kconfig"
source "drivers/media/platform/amlogic/meson-ge2d/Kconfig"
diff --git a/drivers/media/platform/amlogic/Makefile b/drivers/media/platform/amlogic/Makefile
index d3cdb8fa4ddb..4f571ce5d13e 100644
--- a/drivers/media/platform/amlogic/Makefile
+++ b/drivers/media/platform/amlogic/Makefile
@@ -1,2 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
+
+obj-y += c3-mipi-csi2/
obj-y += meson-ge2d/
diff --git a/drivers/media/platform/amlogic/c3-mipi-csi2/Kconfig b/drivers/media/platform/amlogic/c3-mipi-csi2/Kconfig
new file mode 100644
index 000000000000..0d7b2e203273
--- /dev/null
+++ b/drivers/media/platform/amlogic/c3-mipi-csi2/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config VIDEO_C3_MIPI_CSI2
+ tristate "Amlogic C3 MIPI CSI-2 receiver"
+ depends on ARCH_MESON || COMPILE_TEST
+ depends on VIDEO_DEV
+ depends on OF
+ select MEDIA_CONTROLLER
+ select V4L2_FWNODE
+ select VIDEO_V4L2_SUBDEV_API
+ help
+ Video4Linux2 driver for Amlogic C3 MIPI CSI-2 receiver.
+ C3 MIPI CSI-2 receiver is used to receive MIPI data from
+ image sensor.
+
+ To compile this driver as a module choose m here.
diff --git a/drivers/media/platform/amlogic/c3-mipi-csi2/Makefile b/drivers/media/platform/amlogic/c3-mipi-csi2/Makefile
new file mode 100644
index 000000000000..cc08fc722bfd
--- /dev/null
+++ b/drivers/media/platform/amlogic/c3-mipi-csi2/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_VIDEO_C3_MIPI_CSI2) += c3-mipi-csi2.o
diff --git a/drivers/media/platform/amlogic/c3-mipi-csi2/c3-mipi-csi2.c b/drivers/media/platform/amlogic/c3-mipi-csi2/c3-mipi-csi2.c
new file mode 100644
index 000000000000..6ac60d5b26a8
--- /dev/null
+++ b/drivers/media/platform/amlogic/c3-mipi-csi2/c3-mipi-csi2.c
@@ -0,0 +1,910 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Copyright (C) 2024 Amlogic, Inc. All rights reserved
+ */
+
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+/* C3 CSI-2 submodule definition */
+enum {
+ SUBMD_APHY,
+ SUBMD_DPHY,
+ SUBMD_HOST,
+};
+
+#define CSI2_SUBMD_MASK GENMASK(17, 16)
+#define CSI2_SUBMD_SHIFT 16
+#define CSI2_SUBMD(x) (((x) & (CSI2_SUBMD_MASK)) >> (CSI2_SUBMD_SHIFT))
+#define CSI2_REG_ADDR_MASK GENMASK(15, 0)
+#define CSI2_REG_ADDR(x) ((x) & (CSI2_REG_ADDR_MASK))
+#define CSI2_REG_A(x) ((SUBMD_APHY << CSI2_SUBMD_SHIFT) | (x))
+#define CSI2_REG_D(x) ((SUBMD_DPHY << CSI2_SUBMD_SHIFT) | (x))
+#define CSI2_REG_H(x) ((SUBMD_HOST << CSI2_SUBMD_SHIFT) | (x))
+
+#define MIPI_CSI2_CLOCK_NUM_MAX 3
+#define MIPI_CSI2_SUBDEV_NAME "mipi-csi2"
Isn't the name too generic ? Should it report at least "c3-mipi-csi2"
?
+
+/* C3 CSI-2 APHY register */
+#define MIPI_CSI_2M_PHY2_CNTL1 CSI2_REG_A(0x44)
+#define MIPI_APHY_NORMAL_CNTL1 0x3f425C00
All other hex addresses use small capitals for letters
+
+#define MIPI_CSI_2M_PHY2_CNTL2 CSI2_REG_A(0x48)
+#define MIPI_APHY_4LANES_CNTL2 0x033a0000
+#define MIPI_APHY_NORMAL_CNTL2 0x333a0000
+
+#define MIPI_CSI_2M_PHY2_CNTL3 CSI2_REG_A(0x4c)
+#define MIPI_APHY_2LANES_CNTL3 0x03800000
+
+/* C3 CSI-2 DPHY register */
+#define MIPI_PHY_CTRL CSI2_REG_D(0x00)
+#define MIPI_DPHY_LANES_ENABLE 0x0
+
+#define MIPI_PHY_CLK_LANE_CTRL CSI2_REG_D(0x04)
+#define MIPI_DPHY_CLK_CONTINUE_MODE 0x3d8
+
+#define MIPI_PHY_DATA_LANE_CTRL CSI2_REG_D(0x08)
+#define MIPI_DPHY_LANE_CTRL_DISABLE 0x0
+
+#define MIPI_PHY_DATA_LANE_CTRL1 CSI2_REG_D(0x0c)
+#define MIPI_DPHY_INSERT_ERRESC BIT(0)
+#define MIPI_DPHY_HS_SYNC_CHECK BIT(1)
+#define MIPI_DPHY_FIVE_HS_PIPE GENMASK(6, 2)
+#define MIPI_DPHY_FIVE_HS_PIPE_SHIFT 2
+#define MIPI_DPHY_DATA_PIPE_SELECT GENMASK(9, 7)
+#define MIPI_DPHY_DATA_PIPE_SELECT_SHIFT 7
+
+#define MIPI_PHY_TCLK_MISS CSI2_REG_D(0x10)
+#define MIPI_DPHY_CLK_MISS 0x9
+
+#define MIPI_PHY_TCLK_SETTLE CSI2_REG_D(0x14)
+#define MIPI_DPHY_CLK_SETTLE 0x1F
+
+#define MIPI_PHY_THS_EXIT CSI2_REG_D(0x18)
+#define MIPI_DPHY_HS_EXIT 0x8
+
+#define MIPI_PHY_THS_SKIP CSI2_REG_D(0x1c)
+#define MIPI_DPHY_HS_SKIP 0xa
+
+#define MIPI_PHY_THS_SETTLE CSI2_REG_D(0x20)
+#define MIPI_PHY_TINIT CSI2_REG_D(0x24)
+#define MIPI_DPHY_INIT_CYCLES 0x4e20
+
+#define MIPI_PHY_TULPS_C CSI2_REG_D(0x28)
+#define MIPI_DPHY_ULPS_CHECK_CYCLES 0x1000
+
+#define MIPI_PHY_TULPS_S CSI2_REG_D(0x2c)
+#define MIPI_DPHY_ULPS_START_CYCLES 0x100
+
+#define MIPI_PHY_TMBIAS CSI2_REG_D(0x30)
+#define MIPI_DPHY_MBIAS_CYCLES 0x100
+
+#define MIPI_PHY_TLP_EN_W CSI2_REG_D(0x34)
+#define MIPI_DPHY_ULPS_STOP_CYCLES 0xC
+
+#define MIPI_PHY_TLPOK CSI2_REG_D(0x38)
+#define MIPI_DPHY_POWER_UP_CYCLES 0x100
+
+#define MIPI_PHY_TWD_INIT CSI2_REG_D(0x3c)
+#define MIPI_DPHY_INIT_WATCH_DOG 0x400000
+
+#define MIPI_PHY_TWD_HS CSI2_REG_D(0x40)
+#define MIPI_DPHY_HS_WATCH_DOG 0x400000
+
+#define MIPI_PHY_MUX_CTRL0 CSI2_REG_D(0x284)
+#define MIPI_DPHY_LANE3_SELECT GENMASK(3, 0)
+#define MIPI_DPHY_LANE2_SELECT GENMASK(7, 4)
+#define MIPI_DPHY_LANE2_SELECT_SHIFT 4
+#define MIPI_DPHY_LANE1_SELECT GENMASK(11, 8)
+#define MIPI_DPHY_LANE1_SELECT_SHIFT 8
+#define MIPI_DPHY_LANE0_SELECT GENMASK(14, 12)
+
+#define MIPI_PHY_MUX_CTRL1 CSI2_REG_D(0x288)
+#define MIPI_DPHY_LANE3_CTRL_SIGNAL GENMASK(3, 0)
+#define MIPI_DPHY_LANE2_CTRL_SIGNAL GENMASK(7, 4)
+#define MIPI_DPHY_LANE2_CTRL_SIGNAL_SHIFT 4
+#define MIPI_DPHY_LANE1_CTRL_SIGNAL GENMASK(11, 8)
+#define MIPI_DPHY_LANE1_CTRL_SIGNAL_SHIFT 8
+#define MIPI_DPHY_LANE0_CTRL_SIGNAL GENMASK(14, 12)
+#define MIPI_DPHY_CLK_SELECT BIT(17)
+
+/* C3 CSI-2 HOST register */
+#define CSI2_HOST_N_LANES CSI2_REG_H(0x04)
+#define CSI2_HOST_CSI2_RESETN CSI2_REG_H(0x10)
+#define CSI2_HOST_RESETN_DEFAULT 0x0
+#define CSI2_HOST_RESETN_RST_VALUE BIT(0)
+
+#define CSI2_HOST_MASK1 CSI2_REG_H(0x28)
+#define CSI2_HOST_ERROR_MASK1 GENMASK(28, 0)
+
+#define MIPI_CSI2_MAX_WIDTH 2888
+#define MIPI_CSI2_MIN_WIDTH 160
+#define MIPI_CSI2_MAX_HEIGHT 2240
+#define MIPI_CSI2_MIN_HEIGHT 120
+#define MIPI_CSI2_DEFAULT_WIDTH 1920
+#define MIPI_CSI2_DEFAULT_HEIGHT 1080
+#define MIPI_CSI2_DEFAULT_FMT MEDIA_BUS_FMT_SRGGB10_1X10
+
+/* C3 CSI-2 pad list */
+enum {
+ MIPI_CSI2_PAD_SINK,
+ MIPI_CSI2_PAD_SRC,
+ MIPI_CSI2_PAD_MAX
+};
+
+/**
You don't need to kernel-doc in-driver types and functions.
Documentation is always good, but this won't be parsed by kernel-doc
(afaiu) so you should drop one * from /**
+ * struct csi_info - MIPI CSI2 information
+ *
+ * @clocks: array of MIPI CSI2 clock names
+ * @clock_rates: array of MIPI CSI2 clock rate
+ * @clock_num: actual clock number
+ */
+struct csi_info {
+ char *clocks[MIPI_CSI2_CLOCK_NUM_MAX];
+ u32 clock_rates[MIPI_CSI2_CLOCK_NUM_MAX];
+ u32 clock_num;
+};
+
+/**
+ * struct csi_device - MIPI CSI2 platform device
+ *
+ * @dev: pointer to the struct device
+ * @aphy: MIPI CSI2 aphy register address
+ * @dphy: MIPI CSI2 dphy register address
+ * @host: MIPI CSI2 host register address
+ * @clks: array of MIPI CSI2 clocks
+ * @sd: MIPI CSI2 sub-device
+ * @pads: MIPI CSI2 sub-device pads
+ * @notifier: notifier to register on the v4l2-async API
+ * @src_sd: source sub-device
+ * @bus: MIPI CSI2 bus information
+ * @src_sd_pad: source sub-device pad
+ * @lock: protect MIPI CSI2 device
+ * @info: version-specific MIPI CSI2 information
+ */
+struct csi_device {
+ struct device *dev;
+ void __iomem *aphy;
+ void __iomem *dphy;
+ void __iomem *host;
+ struct clk_bulk_data clks[MIPI_CSI2_CLOCK_NUM_MAX];
+
+ struct v4l2_subdev sd;
+ struct media_pad pads[MIPI_CSI2_PAD_MAX];
+ struct v4l2_async_notifier notifier;
+ struct v4l2_subdev *src_sd;
+ struct v4l2_mbus_config_mipi_csi2 bus;
+
+ u16 src_sd_pad;
+ struct mutex lock; /* Protect csi device */
All the operations which receive a subdev_state are guaranteed to be locked
so you can avoid manually locking in enable/disable streams
(and drop #include cleanup.h if you don't use guards in any other
place)
+ const struct csi_info *info;
+};
+
+static const u32 c3_mipi_csi_formats[] = {
+ MEDIA_BUS_FMT_SBGGR10_1X10,
+ MEDIA_BUS_FMT_SGBRG10_1X10,
+ MEDIA_BUS_FMT_SGRBG10_1X10,
+ MEDIA_BUS_FMT_SRGGB10_1X10,
+ MEDIA_BUS_FMT_SBGGR12_1X12,
+ MEDIA_BUS_FMT_SGBRG12_1X12,
+ MEDIA_BUS_FMT_SGRBG12_1X12,
+ MEDIA_BUS_FMT_SRGGB12_1X12,
+};
+
+/* Hardware configuration */
+
+static void c3_mipi_csi_write(struct csi_device *csi, u32 reg, u32 val)
+{
+ void __iomem *addr;
+
+ switch (CSI2_SUBMD(reg)) {
+ case SUBMD_APHY:
+ addr = csi->aphy + CSI2_REG_ADDR(reg);
+ break;
+ case SUBMD_DPHY:
+ addr = csi->dphy + CSI2_REG_ADDR(reg);
+ break;
+ case SUBMD_HOST:
+ addr = csi->host + CSI2_REG_ADDR(reg);
+ break;
+ default:
+ dev_err(csi->dev, "Invalid sub-module: %lu\n", CSI2_SUBMD(reg));
+ return;
+ }
+
+ writel(val, addr);
+}
+
+static void c3_mipi_csi_update_bits(struct csi_device *csi, u32 reg,
+ u32 mask, u32 val)
+{
+ void __iomem *addr;
+ u32 orig, tmp;
+
+ switch (CSI2_SUBMD(reg)) {
+ case SUBMD_APHY:
+ addr = csi->aphy + CSI2_REG_ADDR(reg);
+ break;
+ case SUBMD_DPHY:
+ addr = csi->dphy + CSI2_REG_ADDR(reg);
+ break;
+ case SUBMD_HOST:
+ addr = csi->host + CSI2_REG_ADDR(reg);
+ break;
+ default:
+ dev_err(csi->dev, "Invalid sub-module: %lu\n", CSI2_SUBMD(reg));
+ return;
+ }
This is repeated in two functions and could be grouped to a common
place. Up to you
+
+ orig = readl(addr);
+ tmp = orig & ~mask;
+ tmp |= val & mask;
+
+ if (tmp != orig)
+ writel(tmp, addr);
+}
+
+static void c3_mipi_csi_cfg_aphy(struct csi_device *csi, u32 lanes)
+{
+ c3_mipi_csi_write(csi, MIPI_CSI_2M_PHY2_CNTL1, MIPI_APHY_NORMAL_CNTL1);
+
+ if (lanes == 4)
+ c3_mipi_csi_write(csi, MIPI_CSI_2M_PHY2_CNTL2, MIPI_APHY_4LANES_CNTL2);
+ else
+ c3_mipi_csi_write(csi, MIPI_CSI_2M_PHY2_CNTL2, MIPI_APHY_NORMAL_CNTL2);
+
+ if (lanes == 2)
+ c3_mipi_csi_write(csi, MIPI_CSI_2M_PHY2_CNTL3, MIPI_APHY_2LANES_CNTL3);
The driver seems to only accept 2 or 4 lanes. What is
MIPI_APHY_NORMAL_CNTL2 for ?
+}
+
+static void c3_mipi_csi_2lanes_setting(struct csi_device *csi)
+{
+ /* Disable lane 2 and lane 3 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE3_SELECT, 0xf);
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE2_SELECT,
+ 0xf << MIPI_DPHY_LANE2_SELECT_SHIFT);
+ /* Select analog data lane 1 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE1_SELECT,
+ 0x1 << MIPI_DPHY_LANE1_SELECT_SHIFT);
+ /* Select analog data lane 0 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE0_SELECT, 0x0);
+
+ /* Disable lane 2 and lane 3 control signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE3_CTRL_SIGNAL, 0xf);
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE2_CTRL_SIGNAL,
+ 0xf << MIPI_DPHY_LANE2_CTRL_SIGNAL_SHIFT);
+ /* Select lane 1 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE1_CTRL_SIGNAL,
+ 0x1 << MIPI_DPHY_LANE1_CTRL_SIGNAL_SHIFT);
+ /* Select lane 0 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE0_CTRL_SIGNAL, 0x0);
+ /* Select input 0 as clock */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_CLK_SELECT,
+ MIPI_DPHY_CLK_SELECT);
+}
+
+static void c3_mipi_csi_4lanes_setting(struct csi_device *csi)
+{
+ /* Select analog data lane 3 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE3_SELECT, 0x3);
+ /* Select analog data lane 2 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE2_SELECT,
+ 0x2 << MIPI_DPHY_LANE2_SELECT_SHIFT);
+ /* Select analog data lane 1 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE1_SELECT,
+ 0x1 << MIPI_DPHY_LANE1_SELECT_SHIFT);
+ /* Select analog data lane 0 */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL0, MIPI_DPHY_LANE0_SELECT, 0x0);
+
+ /* Select lane 3 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE3_CTRL_SIGNAL, 0x3);
+ /* Select lane 2 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE2_CTRL_SIGNAL,
+ 0x2 << MIPI_DPHY_LANE2_CTRL_SIGNAL_SHIFT);
+ /* Select lane 1 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE1_CTRL_SIGNAL,
+ 0x1 << MIPI_DPHY_LANE1_CTRL_SIGNAL_SHIFT);
+ /* Select lane 0 signal */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_LANE0_CTRL_SIGNAL, 0x0);
+ /* Select input 0 as clock */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_MUX_CTRL1, MIPI_DPHY_CLK_SELECT,
+ MIPI_DPHY_CLK_SELECT);
+}
+
+static void c3_mipi_csi_cfg_dphy(struct csi_device *csi, u32 lanes, s64 rate)
+{
+ u32 val;
+ u32 settle;
+
+ /* Calculate the high speed settle */
+ val = DIV_ROUND_UP(1000000000, rate);
+ settle = (16 * val + 230) / 10;
+
+ c3_mipi_csi_write(csi, MIPI_PHY_CLK_LANE_CTRL, MIPI_DPHY_CLK_CONTINUE_MODE);
+ c3_mipi_csi_write(csi, MIPI_PHY_TCLK_MISS, MIPI_DPHY_CLK_MISS);
+ c3_mipi_csi_write(csi, MIPI_PHY_TCLK_SETTLE, MIPI_DPHY_CLK_SETTLE);
+ c3_mipi_csi_write(csi, MIPI_PHY_THS_EXIT, MIPI_DPHY_HS_EXIT);
+ c3_mipi_csi_write(csi, MIPI_PHY_THS_SKIP, MIPI_DPHY_HS_SKIP);
+ c3_mipi_csi_write(csi, MIPI_PHY_THS_SETTLE, settle);
+ c3_mipi_csi_write(csi, MIPI_PHY_TINIT, MIPI_DPHY_INIT_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TMBIAS, MIPI_DPHY_MBIAS_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TULPS_C, MIPI_DPHY_ULPS_CHECK_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TULPS_S, MIPI_DPHY_ULPS_START_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TLP_EN_W, MIPI_DPHY_ULPS_STOP_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TLPOK, MIPI_DPHY_POWER_UP_CYCLES);
+ c3_mipi_csi_write(csi, MIPI_PHY_TWD_INIT, MIPI_DPHY_INIT_WATCH_DOG);
+ c3_mipi_csi_write(csi, MIPI_PHY_TWD_HS, MIPI_DPHY_HS_WATCH_DOG);
+ c3_mipi_csi_write(csi, MIPI_PHY_DATA_LANE_CTRL, MIPI_DPHY_LANE_CTRL_DISABLE);
+
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_DATA_LANE_CTRL1, MIPI_DPHY_INSERT_ERRESC,
+ MIPI_DPHY_INSERT_ERRESC);
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_DATA_LANE_CTRL1, MIPI_DPHY_HS_SYNC_CHECK,
+ MIPI_DPHY_HS_SYNC_CHECK);
+ /*
+ * Set 5 pipe lines to the same high speed.
+ * Each bit for one pipe line.
+ */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_DATA_LANE_CTRL1, MIPI_DPHY_FIVE_HS_PIPE,
+ 0x1f << MIPI_DPHY_FIVE_HS_PIPE_SHIFT);
+
+ /* Output data with pipe line data. */
+ c3_mipi_csi_update_bits(csi, MIPI_PHY_DATA_LANE_CTRL1, MIPI_DPHY_DATA_PIPE_SELECT,
+ 0x3 << MIPI_DPHY_DATA_PIPE_SELECT_SHIFT);
Would it be possible to provide a definition for these 0x1f and 0x3
values ?
+ if (lanes == 2)
+ c3_mipi_csi_2lanes_setting(csi);
+ else
+ c3_mipi_csi_4lanes_setting(csi);
+
+ /* Enable digital data and clock lanes */
+ c3_mipi_csi_write(csi, MIPI_PHY_CTRL, MIPI_DPHY_LANES_ENABLE);
+}
+
+static void c3_mipi_csi_cfg_host(struct csi_device *csi, u32 lanes)
+{
+ /* Reset CSI-2 controller output */
+ c3_mipi_csi_write(csi, CSI2_HOST_CSI2_RESETN, CSI2_HOST_RESETN_DEFAULT);
+ c3_mipi_csi_write(csi, CSI2_HOST_CSI2_RESETN, CSI2_HOST_RESETN_RST_VALUE);
+
+ /* Set data lane number */
+ c3_mipi_csi_write(csi, CSI2_HOST_N_LANES, lanes - 1);
+
+ /* Enable error mask */
+ c3_mipi_csi_write(csi, CSI2_HOST_MASK1, CSI2_HOST_ERROR_MASK1);
+}
+
+static int c3_mipi_csi_start_stream(struct csi_device *csi)
+{
+ s64 link_freq;
+ s64 lane_rate;
+
+ link_freq = v4l2_get_link_freq(csi->src_sd->ctrl_handler, 0, 0);
+ if (link_freq < 0) {
+ dev_err(csi->dev, "Unable to obtain link frequency: %lld\n", link_freq);
+ return link_freq;
+ }
+
+ lane_rate = link_freq * 2;
+ if (lane_rate > 1500000000)
I would dev_err here too
+ return -EINVAL;
+
+ c3_mipi_csi_cfg_aphy(csi, csi->bus.num_data_lanes);
+ c3_mipi_csi_cfg_dphy(csi, csi->bus.num_data_lanes, lane_rate);
+ c3_mipi_csi_cfg_host(csi, csi->bus.num_data_lanes);
+
+ return 0;
+}
+
+static int c3_mipi_csi_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct csi_device *csi = v4l2_get_subdevdata(sd);
+ u64 sink_streams;
+ int ret;
+
+ guard(mutex)(&csi->lock);
+
+ pm_runtime_resume_and_get(csi->dev);
+
+ c3_mipi_csi_start_stream(csi);
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state, pad,
+ MIPI_CSI2_PAD_SINK,
+ &streams_mask);
+ ret = v4l2_subdev_enable_streams(csi->src_sd,
+ csi->src_sd_pad,
+ sink_streams);
+ if (ret) {
+ pm_runtime_put(csi->dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int c3_mipi_csi_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct csi_device *csi = v4l2_get_subdevdata(sd);
+ u64 sink_streams;
+ int ret;
+
+ guard(mutex)(&csi->lock);
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state, pad,
+ MIPI_CSI2_PAD_SINK,
+ &streams_mask);
+ ret = v4l2_subdev_disable_streams(csi->src_sd,
+ csi->src_sd_pad,
+ sink_streams);
+ if (ret)
+ dev_err(csi->dev, "Failed to disable %s\n", csi->src_sd->name);
+
+ pm_runtime_put(csi->dev);
+
+ return ret;
+}
+
+static int c3_mipi_csi_cfg_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = MIPI_CSI2_DEFAULT_WIDTH,
+ .height = MIPI_CSI2_DEFAULT_HEIGHT,
+ .code = MIPI_CSI2_DEFAULT_FMT,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_COLORSPACE_RAW,
+ .ycbcr_enc = V4L2_YCBCR_ENC_601,
+ .quantization = V4L2_QUANTIZATION_LIM_RANGE,
I presume for Raw Bayer data the quantization range is full ?
+ .xfer_func = V4L2_XFER_FUNC_NONE,
+ };
+ int ret;
+
+ ret = v4l2_subdev_routing_validate(sd, routing,
+ V4L2_SUBDEV_ROUTING_ONLY_1_TO_1);
+ if (ret)
+ return ret;
You should validate that the provided routing table matches what the
driver supports, so only [0/0]->[1/0]
Now that I've said so, if the routing table is not modifiable I wonder
if you should support set_routing() at all, or it could be left out
until you don't add support for more streams to the driver.
After all this driver implements support for routing but doesn't set
the V4L2_SUBDEV_FL_STREAMS flag, so the operation is disallowed from
userspace for now.
+
+ ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int c3_mipi_csi_init_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route routes;
+ struct v4l2_subdev_krouting routing;
+
+ routes.sink_pad = MIPI_CSI2_PAD_SINK;
+ routes.sink_stream = 0;
+ routes.source_pad = MIPI_CSI2_PAD_SRC;
+ routes.source_stream = 0;
+ routes.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
+
+ routing.num_routes = 1;
+ routing.routes = &routes;
+
+ return c3_mipi_csi_cfg_routing(sd, state, &routing);
+}
+
+static int c3_mipi_csi_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ bool is_streaming = v4l2_subdev_is_streaming(sd);
+
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE && is_streaming)
+ return -EBUSY;
+
+ return c3_mipi_csi_cfg_routing(sd, state, routing);
+}
+
+static int c3_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ switch (code->pad) {
+ case MIPI_CSI2_PAD_SINK:
+ if (code->index >= ARRAY_SIZE(c3_mipi_csi_formats))
+ return -EINVAL;
+
+ code->code = c3_mipi_csi_formats[code->index];
+ break;
+ case MIPI_CSI2_PAD_SRC:
+ struct v4l2_mbus_framefmt *fmt;
+
+ if (code->index > 0)
+ return -EINVAL;
+
+ fmt = v4l2_subdev_state_get_format(state, code->pad);
+ code->code = fmt->code;
+ break;
I'm not sure if the V4L2 API specify that the formats on a pad should
be enumerated in full, regardless of the configuration, or like you're
doing here reflect the subdev configuration. I like what you have here
more, so unless someone screams I think it's fine.
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int c3_mipi_csi_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_format *format)
+{
+ struct v4l2_mbus_framefmt *fmt;
+ unsigned int i;
+
+ if (format->pad != MIPI_CSI2_PAD_SINK)
+ return v4l2_subdev_get_fmt(sd, state, format);
+
+ fmt = v4l2_subdev_state_get_format(state, format->pad);
Could you clarify what other streams you plan to support ? As you
support routing I presume you are preparing to capture
multiple streams of data like image + embedded data, or to support
sensors which sends data on different virtual channels ?
How do you see this driver evolve ? Will it be augmented with an
additional source pad directed to a video device where to capture
embedded data from ?
I'm wondering because if PAD_SINK become multiplexed, you won't be
allowed to set a format there. It only works now because you have a
single stream.
