All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
@ 2025-03-05 12:17 Yemike Abhilash Chandra
  2025-03-06  9:35 ` kernel test robot
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Yemike Abhilash Chandra @ 2025-03-05 12:17 UTC (permalink / raw)
  To: tomi.valkeinen, mchehab
  Cc: linux-media, linux-kernel, vaishnav.a, u-kumar1, r-donadkar

The DS90UB960-Q1 includes a second I2C interface for independent control
of the deserializer and remote devices. However, the current driver does
not utilize it, thus restricting users to either CSI TX0 or CSI TX1 on
the primary I2C interface. Enable the second I2C interface, allowing
flexible routing where CSI TX0 can be used on the primary and CSI TX1 on
the secondary, or vice versa by enabling appropriate ports in DT. To
achieve the same only modify the bits relevant to the enabled RX and TX
ports of that interface and during probe and enable_streams call, few
registers were being reset to HW reset state, these operations are not
necessary for functionality and resets the state when secondary I2C
interface is probed, thus drop them.

DS90UB960 data sheet: https://www.ti.com/lit/ds/symlink/ds90ub960-q1.pdf
Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
---
 drivers/media/i2c/ds90ub960.c | 64 +++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 5dde8452739b..4c0052e05071 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -73,6 +73,9 @@
 
 #define UB960_NUM_BC_GPIOS		4
 
+#define UB960_ALL_RX_PORTS_MASK	GENMASK(3, 0)
+#define UB960_CSI_TX0			BIT(4)
+
 /*
  * Register map
  *
@@ -111,6 +114,7 @@
 #define UB960_SR_SCL_HIGH_TIME			0x0a
 #define UB960_SR_SCL_LOW_TIME			0x0b
 #define UB960_SR_RX_PORT_CTL			0x0c
+#define UB960_SR_RX_PORT_CTL_BCC_MAP		GENMASK(7, 4)
 #define UB960_SR_IO_CTL				0x0d
 #define UB960_SR_GPIO_PIN_STS			0x0e
 #define UB960_SR_GPIO_INPUT_CTL			0x0f
@@ -524,6 +528,8 @@ struct ub960_data {
 
 	u32 tx_data_rate;		/* Nominal data rate (Gb/s) */
 	s64 tx_link_freq[1];
+	u8 rx_mask;
+	u8 tx_mask;
 
 	struct i2c_atr *atr;
 
@@ -2168,6 +2174,17 @@ static void ub960_init_rx_port_ub9702(struct ub960_data *priv,
 static int ub960_init_rx_ports(struct ub960_data *priv)
 {
 	unsigned int nport;
+	u8 enabled_rxports_mask;
+	u8 enabled_rxports;
+	int ret;
+
+	/* Configure i2c interface for RX ports */
+	enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
+	enabled_rxports = (priv->tx_mask & UB960_CSI_TX0)  ? 0x00 : enabled_rxports_mask;
+
+	ret = ub960_update_bits(priv, UB960_SR_RX_PORT_CTL, enabled_rxports_mask, enabled_rxports);
+	if (ret)
+		return ret;
 
 	for (nport = 0; nport < priv->hw_data->num_rxports; nport++) {
 		struct ub960_rxport *rxport = priv->rxports[nport];
@@ -2509,14 +2526,6 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 		}
 	}
 
-	/* Configure RX ports */
-
-	/*
-	 * Keep all port forwardings disabled by default. Forwarding will be
-	 * enabled in ub960_enable_rx_port.
-	 */
-	fwd_ctl = GENMASK(7, 4);
-
 	for (unsigned int nport = 0; nport < priv->hw_data->num_rxports;
 	     nport++) {
 		struct ub960_rxport *rxport = priv->rxports[nport];
@@ -2570,9 +2579,9 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 			fwd_ctl &= ~BIT(nport); /* forward to TX0 */
 	}
 
-	ub960_write(priv, UB960_SR_FWD_CTL1, fwd_ctl);
+	ret = ub960_update_bits(priv, UB960_SR_FWD_CTL1, priv->rx_mask, fwd_ctl);
 
-	return 0;
+	return ret;
 }
 
 static void ub960_update_streaming_status(struct ub960_data *priv)
@@ -3574,6 +3583,32 @@ static int ub960_parse_dt_txports(struct ub960_data *priv)
 	return 0;
 }
 
+static int ub960_parse_active_ports(struct ub960_data *priv)
+{
+	struct device *dev = &priv->client->dev;
+	int nport;
+
+	priv->rx_mask = 0;
+	priv->tx_mask = 0;
+
+	for (nport = 0; nport < priv->hw_data->num_rxports + priv->hw_data->num_txports; nport++) {
+		struct fwnode_handle *ep_fwnode;
+
+		ep_fwnode = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), nport, 0, 0);
+		if (!ep_fwnode)
+			continue;
+
+		if (nport < priv->hw_data->num_rxports)
+			priv->rx_mask |= BIT(nport);
+		else
+			priv->tx_mask |= BIT(nport);
+
+		fwnode_handle_put(ep_fwnode);
+	}
+
+	return 0;
+}
+
 static int ub960_parse_dt(struct ub960_data *priv)
 {
 	int ret;
@@ -3900,11 +3935,6 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 		!!(dev_sts & BIT(4)), refclk_freq,
 		clk_get_rate(priv->refclk) / HZ_PER_MHZ);
 
-	/* Disable all RX ports by default */
-	ret = ub960_write(priv, UB960_SR_RX_PORT_CTL, 0);
-	if (ret)
-		goto err_pd_gpio;
-
 	/* release GPIO lock */
 	if (priv->hw_data->is_ub9702) {
 		ret = ub960_update_bits(priv, UB960_SR_RESET,
@@ -3969,6 +3999,10 @@ static int ub960_probe(struct i2c_client *client)
 	if (ret)
 		goto err_mutex_destroy;
 
+	ret = ub960_parse_active_ports(priv);
+	if (ret)
+		goto err_disable_core_hw;
+
 	ret = ub960_parse_dt(priv);
 	if (ret)
 		goto err_disable_core_hw;
-- 
2.34.1


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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-05 12:17 [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface Yemike Abhilash Chandra
@ 2025-03-06  9:35 ` kernel test robot
  2025-03-06 11:47 ` kernel test robot
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-03-06  9:35 UTC (permalink / raw)
  To: Yemike Abhilash Chandra; +Cc: llvm, oe-kbuild-all

Hi Yemike,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on linuxtv-media-pending/master]
[also build test WARNING on linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yemike-Abhilash-Chandra/media-i2c-ds90ub960-Enable-second-i2c-interface/20250305-201858
base:   https://git.linuxtv.org/media-ci/media-pending.git master
patch link:    https://lore.kernel.org/r/20250305121705.2143540-1-y-abhilashchandra%40ti.com
patch subject: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
config: arm-randconfig-004-20250306 (https://download.01.org/0day-ci/archive/20250306/202503061704.adraY1Jm-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503061704.adraY1Jm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503061704.adraY1Jm-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/ds90ub960.c:2577:4: warning: variable 'fwd_ctl' is uninitialized when used here [-Wuninitialized]
    2577 |                         fwd_ctl |= BIT(nport); /* forward to TX1 */
         |                         ^~~~~~~
   drivers/media/i2c/ds90ub960.c:2458:12: note: initialize the variable 'fwd_ctl' to silence this warning
    2458 |         u8 fwd_ctl;
         |                   ^
         |                    = '\0'
   1 warning generated.


vim +/fwd_ctl +2577 drivers/media/i2c/ds90ub960.c

afe267f2d368f5 Tomi Valkeinen          2023-06-19  2454  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2455  static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2456  					       struct v4l2_subdev_state *state)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2457  {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2458  	u8 fwd_ctl;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2459  	struct {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2460  		u32 num_streams;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2461  		u8 pixel_dt;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2462  		u8 meta_dt;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2463  		u32 meta_lines;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2464  		u32 tx_port;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2465  	} rx_data[UB960_MAX_RX_NPORTS] = {};
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2466  	u8 vc_map[UB960_MAX_RX_NPORTS] = {};
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2467  	struct v4l2_subdev_route *route;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2468  	int ret;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2469  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2470  	ret = ub960_validate_stream_vcs(priv);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2471  	if (ret)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2472  		return ret;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2473  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2474  	ub960_get_vc_maps(priv, state, vc_map);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2475  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2476  	for_each_active_route(&state->routing, route) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2477  		struct ub960_rxport *rxport;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2478  		struct ub960_txport *txport;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2479  		struct v4l2_mbus_framefmt *fmt;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2480  		const struct ub960_format_info *ub960_fmt;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2481  		unsigned int nport;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2482  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2483  		nport = ub960_pad_to_port(priv, route->sink_pad);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2484  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2485  		rxport = priv->rxports[nport];
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2486  		if (!rxport)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2487  			return -EINVAL;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2488  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2489  		txport = priv->txports[ub960_pad_to_port(priv, route->source_pad)];
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2490  		if (!txport)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2491  			return -EINVAL;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2492  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2493  		rx_data[nport].tx_port = ub960_pad_to_port(priv, route->source_pad);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2494  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2495  		rx_data[nport].num_streams++;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2496  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2497  		/* For the rest, we are only interested in parallel busses */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2498  		if (rxport->rx_mode == RXPORT_MODE_CSI2_SYNC ||
093d69ad556df2 Tomi Valkeinen          2023-07-31  2499  		    rxport->rx_mode == RXPORT_MODE_CSI2_NONSYNC)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2500  			continue;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2501  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2502  		if (rx_data[nport].num_streams > 2)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2503  			return -EPIPE;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2504  
d0fde6aae2bacd Sakari Ailus            2023-10-13  2505  		fmt = v4l2_subdev_state_get_format(state, route->sink_pad,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2506  						   route->sink_stream);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2507  		if (!fmt)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2508  			return -EPIPE;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2509  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2510  		ub960_fmt = ub960_find_format(fmt->code);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2511  		if (!ub960_fmt)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2512  			return -EPIPE;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2513  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2514  		if (ub960_fmt->meta) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2515  			if (fmt->height > 3) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2516  				dev_err(&priv->client->dev,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2517  					"rx%u: unsupported metadata height %u\n",
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2518  					nport, fmt->height);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2519  				return -EPIPE;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2520  			}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2521  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2522  			rx_data[nport].meta_dt = ub960_fmt->datatype;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2523  			rx_data[nport].meta_lines = fmt->height;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2524  		} else {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2525  			rx_data[nport].pixel_dt = ub960_fmt->datatype;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2526  		}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2527  	}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2528  
cff7e9e5aee4ba Tomi Valkeinen          2024-12-06  2529  	for (unsigned int nport = 0; nport < priv->hw_data->num_rxports;
cff7e9e5aee4ba Tomi Valkeinen          2024-12-06  2530  	     nport++) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2531  		struct ub960_rxport *rxport = priv->rxports[nport];
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2532  		u8 vc = vc_map[nport];
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2533  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2534  		if (rx_data[nport].num_streams == 0)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2535  			continue;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2536  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2537  		switch (rxport->rx_mode) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2538  		case RXPORT_MODE_RAW10:
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2539  			ub960_rxport_write(priv, nport, UB960_RR_RAW10_ID,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2540  				rx_data[nport].pixel_dt | (vc << UB960_RR_RAW10_ID_VC_SHIFT));
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2541  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2542  			ub960_rxport_write(priv, rxport->nport,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2543  				UB960_RR_RAW_EMBED_DTYPE,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2544  				(rx_data[nport].meta_lines << UB960_RR_RAW_EMBED_DTYPE_LINES_SHIFT) |
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2545  					rx_data[nport].meta_dt);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2546  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2547  			break;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2548  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2549  		case RXPORT_MODE_RAW12_HF:
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2550  		case RXPORT_MODE_RAW12_LF:
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2551  			/* Not implemented */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2552  			break;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2553  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2554  		case RXPORT_MODE_CSI2_SYNC:
093d69ad556df2 Tomi Valkeinen          2023-07-31  2555  		case RXPORT_MODE_CSI2_NONSYNC:
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2556  			if (!priv->hw_data->is_ub9702) {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2557  				/* Map all VCs from this port to the same VC */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2558  				ub960_rxport_write(priv, nport, UB960_RR_CSI_VC_MAP,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2559  						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(3)) |
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2560  						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(2)) |
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2561  						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(1)) |
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2562  						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(0)));
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2563  			} else {
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2564  				unsigned int i;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2565  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2566  				/* Map all VCs from this port to VC(nport) */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2567  				for (i = 0; i < 8; i++)
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2568  					ub960_rxport_write(priv, nport,
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2569  							   UB960_RR_VC_ID_MAP(i),
5dbbd0609b83f6 Tomi Valkeinen          2024-12-06  2570  							   (nport << 4) | nport);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2571  			}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2572  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2573  			break;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2574  		}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2575  
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2576  		if (rx_data[nport].tx_port == 1)
afe267f2d368f5 Tomi Valkeinen          2023-06-19 @2577  			fwd_ctl |= BIT(nport); /* forward to TX1 */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2578  		else
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2579  			fwd_ctl &= ~BIT(nport); /* forward to TX0 */
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2580  	}
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2581  
03fb91f512dd2e Yemike Abhilash Chandra 2025-03-05  2582  	ret = ub960_update_bits(priv, UB960_SR_FWD_CTL1, priv->rx_mask, fwd_ctl);
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2583  
03fb91f512dd2e Yemike Abhilash Chandra 2025-03-05  2584  	return ret;
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2585  }
afe267f2d368f5 Tomi Valkeinen          2023-06-19  2586  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-05 12:17 [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface Yemike Abhilash Chandra
  2025-03-06  9:35 ` kernel test robot
@ 2025-03-06 11:47 ` kernel test robot
  2025-03-06 14:58 ` kernel test robot
  2025-03-18 14:46 ` Tomi Valkeinen
  3 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-03-06 11:47 UTC (permalink / raw)
  To: Yemike Abhilash Chandra; +Cc: oe-kbuild-all

Hi Yemike,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on linuxtv-media-pending/master]
[also build test ERROR on linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yemike-Abhilash-Chandra/media-i2c-ds90ub960-Enable-second-i2c-interface/20250305-201858
base:   https://git.linuxtv.org/media-ci/media-pending.git master
patch link:    https://lore.kernel.org/r/20250305121705.2143540-1-y-abhilashchandra%40ti.com
patch subject: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
config: sparc64-randconfig-002-20250306 (https://download.01.org/0day-ci/archive/20250306/202503061904.IJrDnytx-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503061904.IJrDnytx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503061904.IJrDnytx-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/media/i2c/ds90ub960.c: In function 'ub960_init_rx_ports':
>> drivers/media/i2c/ds90ub960.c:2182:32: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
    2182 |         enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
         |                                ^~~~~~~~~~


vim +/FIELD_PREP +2182 drivers/media/i2c/ds90ub960.c

  2173	
  2174	static int ub960_init_rx_ports(struct ub960_data *priv)
  2175	{
  2176		unsigned int nport;
  2177		u8 enabled_rxports_mask;
  2178		u8 enabled_rxports;
  2179		int ret;
  2180	
  2181		/* Configure i2c interface for RX ports */
> 2182		enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
  2183		enabled_rxports = (priv->tx_mask & UB960_CSI_TX0)  ? 0x00 : enabled_rxports_mask;
  2184	
  2185		ret = ub960_update_bits(priv, UB960_SR_RX_PORT_CTL, enabled_rxports_mask, enabled_rxports);
  2186		if (ret)
  2187			return ret;
  2188	
  2189		for (nport = 0; nport < priv->hw_data->num_rxports; nport++) {
  2190			struct ub960_rxport *rxport = priv->rxports[nport];
  2191	
  2192			if (!rxport)
  2193				continue;
  2194	
  2195			if (priv->hw_data->is_ub9702)
  2196				ub960_init_rx_port_ub9702(priv, rxport);
  2197			else
  2198				ub960_init_rx_port_ub960(priv, rxport);
  2199		}
  2200	
  2201		return 0;
  2202	}
  2203	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-05 12:17 [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface Yemike Abhilash Chandra
  2025-03-06  9:35 ` kernel test robot
  2025-03-06 11:47 ` kernel test robot
@ 2025-03-06 14:58 ` kernel test robot
  2025-03-18 14:46 ` Tomi Valkeinen
  3 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-03-06 14:58 UTC (permalink / raw)
  To: Yemike Abhilash Chandra; +Cc: llvm, oe-kbuild-all

Hi Yemike,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on linuxtv-media-pending/master]
[also build test ERROR on linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yemike-Abhilash-Chandra/media-i2c-ds90ub960-Enable-second-i2c-interface/20250305-201858
base:   https://git.linuxtv.org/media-ci/media-pending.git master
patch link:    https://lore.kernel.org/r/20250305121705.2143540-1-y-abhilashchandra%40ti.com
patch subject: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
config: i386-buildonly-randconfig-001-20250306 (https://download.01.org/0day-ci/archive/20250306/202503062235.DNAQREOf-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503062235.DNAQREOf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503062235.DNAQREOf-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/media/i2c/ds90ub960.c:2182:25: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2182 |         enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
         |                                ^
   1 error generated.


vim +/FIELD_PREP +2182 drivers/media/i2c/ds90ub960.c

  2173	
  2174	static int ub960_init_rx_ports(struct ub960_data *priv)
  2175	{
  2176		unsigned int nport;
  2177		u8 enabled_rxports_mask;
  2178		u8 enabled_rxports;
  2179		int ret;
  2180	
  2181		/* Configure i2c interface for RX ports */
> 2182		enabled_rxports_mask = FIELD_PREP(UB960_SR_RX_PORT_CTL_BCC_MAP, priv->rx_mask);
  2183		enabled_rxports = (priv->tx_mask & UB960_CSI_TX0)  ? 0x00 : enabled_rxports_mask;
  2184	
  2185		ret = ub960_update_bits(priv, UB960_SR_RX_PORT_CTL, enabled_rxports_mask, enabled_rxports);
  2186		if (ret)
  2187			return ret;
  2188	
  2189		for (nport = 0; nport < priv->hw_data->num_rxports; nport++) {
  2190			struct ub960_rxport *rxport = priv->rxports[nport];
  2191	
  2192			if (!rxport)
  2193				continue;
  2194	
  2195			if (priv->hw_data->is_ub9702)
  2196				ub960_init_rx_port_ub9702(priv, rxport);
  2197			else
  2198				ub960_init_rx_port_ub960(priv, rxport);
  2199		}
  2200	
  2201		return 0;
  2202	}
  2203	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-05 12:17 [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface Yemike Abhilash Chandra
                   ` (2 preceding siblings ...)
  2025-03-06 14:58 ` kernel test robot
@ 2025-03-18 14:46 ` Tomi Valkeinen
  2025-03-20 10:36   ` Yemike Abhilash Chandra
  3 siblings, 1 reply; 7+ messages in thread
From: Tomi Valkeinen @ 2025-03-18 14:46 UTC (permalink / raw)
  To: Yemike Abhilash Chandra, mchehab
  Cc: linux-media, linux-kernel, vaishnav.a, u-kumar1, r-donadkar

Hi,

On 05/03/2025 14:17, Yemike Abhilash Chandra wrote:
> The DS90UB960-Q1 includes a second I2C interface for independent control
> of the deserializer and remote devices. However, the current driver does
> not utilize it, thus restricting users to either CSI TX0 or CSI TX1 on
> the primary I2C interface. Enable the second I2C interface, allowing
> flexible routing where CSI TX0 can be used on the primary and CSI TX1 on
> the secondary, or vice versa by enabling appropriate ports in DT. To
> achieve the same only modify the bits relevant to the enabled RX and TX
> ports of that interface and during probe and enable_streams call, few
> registers were being reset to HW reset state, these operations are not
> necessary for functionality and resets the state when secondary I2C
> interface is probed, thus drop them.

I'm a bit confused about the description. My recollection is that both 
CSI TX0 and TX1 can be programmed just fine from the first I2C 
interface. Is that not so?

Also, even if the driver supports both CSI TXes, at the moment v4l2 
framework doesn't work with it, at least in many cases. E.g. if you 
connect one TX to a CSIRX, the other TX to another CSIRX, and those 
CSIRXes are independent, have their own media graphs, it's not going to 
work at all.

So I guess my question is, what's the target here, how did you test 
this, etc?

  Tomi


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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-18 14:46 ` Tomi Valkeinen
@ 2025-03-20 10:36   ` Yemike Abhilash Chandra
  2025-04-02 11:27     ` Tomi Valkeinen
  0 siblings, 1 reply; 7+ messages in thread
From: Yemike Abhilash Chandra @ 2025-03-20 10:36 UTC (permalink / raw)
  To: Tomi Valkeinen, mchehab
  Cc: linux-media, linux-kernel, vaishnav.a, u-kumar1, r-donadkar

Hi Tomi

On 18/03/25 20:16, Tomi Valkeinen wrote:
> Hi,
> 
> On 05/03/2025 14:17, Yemike Abhilash Chandra wrote:
>> The DS90UB960-Q1 includes a second I2C interface for independent control
>> of the deserializer and remote devices. However, the current driver does
>> not utilize it, thus restricting users to either CSI TX0 or CSI TX1 on
>> the primary I2C interface. Enable the second I2C interface, allowing
>> flexible routing where CSI TX0 can be used on the primary and CSI TX1 on
>> the secondary, or vice versa by enabling appropriate ports in DT. To
>> achieve the same only modify the bits relevant to the enabled RX and TX
>> ports of that interface and during probe and enable_streams call, few
>> registers were being reset to HW reset state, these operations are not
>> necessary for functionality and resets the state when secondary I2C
>> interface is probed, thus drop them.
> 
> I'm a bit confused about the description. My recollection is that both 
> CSI TX0 and TX1 can be programmed just fine from the first I2C 
> interface. Is that not so?
> 

I apologize for not giving the entire context while sending the RFC.
The purpose of this patch is not only to enable secondary I2C interface
but also to overcome the v4l2 framework limitation by doing that.

> Also, even if the driver supports both CSI TXes, at the moment v4l2 
> framework doesn't work with it, at least in many cases. E.g. if you 
> connect one TX to a CSIRX, the other TX to another CSIRX, and those 
> CSIRXes are independent, have their own media graphs, it's not going to 
> work at all.
> 

Lets say that the overlay applied is as shown in [1]

[1]: 
https://gist.github.com/Yemike-Abhilash-Chandra/5c53a5f3a77954b28c5bd4c27cd336a5

On the physical connection, if we have one V3Link Fusion, where:
CSITX0 is connected to CSIRX0 and CSITX1 is connected to CSIRX1
and the following overlays are applied:

ti/k3-am68-sk-v3link-fusion-dual-csitx.dtbo \ (same overlay as in [1])
ti/k3-v3link-imx219-0-2.dtbo ti/k3-v3link-imx219-0-3.dtbo \
ti/k3-v3link-imx219-1-0.dtbo ti/k3-v3link-imx219-1-1.dtbo

and now each media graph will contain two IMX219 sensors and a
UB960 , with the same UB960 being emulated in both media graphs.
This configuration assigns CSITX0 to the first media
graph and CSITX1 to the second media graph and the
sensors in the first media graph are programmed using the
primary I2C bus, while the sensors in the second media graph
are programmed using the secondary I2C bus.

and this will not break the existing usage, as it will
dynamically check the overlay applied and use primary and
secondary I2C interface accordingly ( primary for Port4
CSITX0 and secondary for port5 CSITX1 )

> So I guess my question is, what's the target here, how did you test 
> this, etc?
> 

for the details I discussed above, I have attached detailed logs
including applying the v3link overlay [1] and sensor overlays
and setting up routes. I ran a free running capture after that.

[2]: 
https://gist.github.com/Yemike-Abhilash-Chandra/1afc731e098fd23cad32dd5438852219

>   Tomi
> 

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

* Re: [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface
  2025-03-20 10:36   ` Yemike Abhilash Chandra
@ 2025-04-02 11:27     ` Tomi Valkeinen
  0 siblings, 0 replies; 7+ messages in thread
From: Tomi Valkeinen @ 2025-04-02 11:27 UTC (permalink / raw)
  To: Yemike Abhilash Chandra
  Cc: linux-media, linux-kernel, vaishnav.a, u-kumar1, r-donadkar,
	mchehab

Hi,

On 20/03/2025 12:36, Yemike Abhilash Chandra wrote:
> Hi Tomi
> 
> On 18/03/25 20:16, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 05/03/2025 14:17, Yemike Abhilash Chandra wrote:
>>> The DS90UB960-Q1 includes a second I2C interface for independent control
>>> of the deserializer and remote devices. However, the current driver does
>>> not utilize it, thus restricting users to either CSI TX0 or CSI TX1 on
>>> the primary I2C interface. Enable the second I2C interface, allowing
>>> flexible routing where CSI TX0 can be used on the primary and CSI TX1 on
>>> the secondary, or vice versa by enabling appropriate ports in DT. To
>>> achieve the same only modify the bits relevant to the enabled RX and TX
>>> ports of that interface and during probe and enable_streams call, few
>>> registers were being reset to HW reset state, these operations are not
>>> necessary for functionality and resets the state when secondary I2C
>>> interface is probed, thus drop them.
>>
>> I'm a bit confused about the description. My recollection is that both 
>> CSI TX0 and TX1 can be programmed just fine from the first I2C 
>> interface. Is that not so?
>>
> 
> I apologize for not giving the entire context while sending the RFC.
> The purpose of this patch is not only to enable secondary I2C interface
> but also to overcome the v4l2 framework limitation by doing that.
> 
>> Also, even if the driver supports both CSI TXes, at the moment v4l2 
>> framework doesn't work with it, at least in many cases. E.g. if you 
>> connect one TX to a CSIRX, the other TX to another CSIRX, and those 
>> CSIRXes are independent, have their own media graphs, it's not going 
>> to work at all.
>>
> 
> Lets say that the overlay applied is as shown in [1]
> 
> [1]: https://gist.github.com/Yemike-Abhilash- 
> Chandra/5c53a5f3a77954b28c5bd4c27cd336a5

Ok. No, we can't merge anything like that. You're creating two linux 
devices for a single HW device, without any specific support for such. 
If it works for you, it's just by luck.

  Tomi


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

end of thread, other threads:[~2025-04-02 11:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 12:17 [PATCH RFC] media: i2c: ds90ub960: Enable second i2c interface Yemike Abhilash Chandra
2025-03-06  9:35 ` kernel test robot
2025-03-06 11:47 ` kernel test robot
2025-03-06 14:58 ` kernel test robot
2025-03-18 14:46 ` Tomi Valkeinen
2025-03-20 10:36   ` Yemike Abhilash Chandra
2025-04-02 11:27     ` Tomi Valkeinen

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.