devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] MT9M114 driver bugfix and improvements
@ 2025-03-05 10:14 Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding Mathis Foerst
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

Hi,

this patch series contains the following bugfix and improvements
for the MT9M114 camera driver:

Changelog:

v2 -> v3:
- Dropped PATCH 2 ("media: mt9m114: Add get_mbus_config").
  Based on the comments, this issure won't be fixed in the MT9M114
  driver but in "imx-media-csi.c" in a separate patch.
- Renumbered patches accordingly.
- Fix the incomplete renaming of the DT property from 'pad-slew-rate'
  to 'onnn,slew-rate' in PATCH 1 and 6.
- Fix checkpatch formatting suggestions in PATCH 2 and 6.

v1 -> v2:
- Fix the subjects of the patches
- Dropped PATCH 1 ("Add bypass-pll DT-binding") as it can be automatically
  detected if the PLL should be bypassed.
- Renumbered patches accordingly
- Switch to uint32, add default value and clarify documentation in PATCH 1
- Add 'Fixes' and 'Cc' tags as suggested in PATCH 6

Link to v1 discussion:
https://lore.kernel.org/linux-media/20250226153929.274562-1-mathis.foerst@mt.com/
Link to v2 discussion:
https://lore.kernel.org/linux-media/20250304103647.34235-1-mathis.foerst@mt.com/


Bugfixes:
- Fix a deadlock when using the V4L2 pad-ops get/set_frame_interval

New Features:
- Bypass the internal PLL if EXTCLK matches the configured link_frequency
- Make the slew-rate of the output pads configurable via DT
- Allow to change the cropping configuration and the horizontal/vertical
  flipping while the sensor is in streaming state

Thanks,
Mathis

Mathis Foerst (6):
  media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding
  media: mt9m114: Bypass PLL if required
  media: mt9m114: Factor out mt9m114_configure_pa
  media: mt9m114: Allow set_selection while streaming
  media: mt9m114: Fix deadlock in get_frame_interval/set_frame_interval
  media: mt9m114: Set pad-slew-rate

 .../bindings/media/i2c/onnn,mt9m114.yaml      |   9 +
 drivers/media/i2c/mt9m114.c                   | 172 ++++++++++++------
 2 files changed, 130 insertions(+), 51 deletions(-)

-- 
2.34.1


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

* [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  2025-03-05 13:08   ` Rob Herring
  2025-03-05 10:14 ` [PATCH v3 2/6] media: mt9m114: Bypass PLL if required Mathis Foerst
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

The MT9M114 supports the different slew rates (0 to 7) on the output pads.
At the moment, this is hardcoded to 7 (the fastest rate).
The user might want to change this values due to EMC requirements.

Add the 'onnn,slew-rate' property to the MT9M114 DT-bindings for selecting
the desired slew rate.

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 .../devicetree/bindings/media/i2c/onnn,mt9m114.yaml      | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
index f6b87892068a..c184bc04b743 100644
--- a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
@@ -70,6 +70,15 @@ properties:
           - bus-type
           - link-frequencies
 
+  onnn,slew-rate:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Slew rate ot the output pads DOUT[7:0], LINE_VALID, FRAME_VALID and
+      PIXCLK. Higher values imply steeper voltage-flanks on the pads.
+    minimum: 0
+    maximum: 7
+    default: 7
+
 required:
   - compatible
   - reg
-- 
2.34.1


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

* [PATCH v3 2/6] media: mt9m114: Bypass PLL if required
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 3/6] media: mt9m114: Factor out mt9m114_configure_pa Mathis Foerst
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

The MT9M114 sensor has an internal PLL that generates the required SYSCLK
from EXTCLK. It also has the option to bypass the PLL and use EXTCLK
directly as SYSCLK.
The current driver implementation uses a hardcoded PLL configuration that
requires a specific EXTCLK frequency. Depending on the available clocks,
it can be desirable to use a different PLL configuration or to bypass it.

The link-frequency of the output bus (Parallel or MIPI-CSI) is configured
in the device tree.

Check if EXTCLK can be used as SYSCLK to achieve this link-frequency. If
yes, bypass the PLL.
Otherwise, (as before) check if EXTCLK and the default PLL configuration
provide the required SYSCLK to achieve the link-frequency. If yes, use the
PLL. If no, throw an error.

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 drivers/media/i2c/mt9m114.c | 62 ++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
index 5f0b0ad8f885..b06003b69f6f 100644
--- a/drivers/media/i2c/mt9m114.c
+++ b/drivers/media/i2c/mt9m114.c
@@ -261,6 +261,7 @@
 #define MT9M114_CAM_PGA_PGA_CONTROL			CCI_REG16(0xc95e)
 #define MT9M114_CAM_SYSCTL_PLL_ENABLE			CCI_REG8(0xc97e)
 #define MT9M114_CAM_SYSCTL_PLL_ENABLE_VALUE			BIT(0)
+#define MT9M114_CAM_SYSCTL_PLL_DISABLE_VALUE			0x00
 #define MT9M114_CAM_SYSCTL_PLL_DIVIDER_M_N		CCI_REG16(0xc980)
 #define MT9M114_CAM_SYSCTL_PLL_DIVIDER_VALUE(m, n)		(((n) << 8) | (m))
 #define MT9M114_CAM_SYSCTL_PLL_DIVIDER_P		CCI_REG16(0xc982)
@@ -377,6 +378,7 @@ struct mt9m114 {
 	struct gpio_desc *reset;
 	struct regulator_bulk_data supplies[3];
 	struct v4l2_fwnode_endpoint bus_cfg;
+	bool bypass_pll;
 
 	struct {
 		unsigned int m;
@@ -743,14 +745,20 @@ static int mt9m114_initialize(struct mt9m114 *sensor)
 	}
 
 	/* Configure the PLL. */
-	cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_ENABLE,
-		  MT9M114_CAM_SYSCTL_PLL_ENABLE_VALUE, &ret);
-	cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_DIVIDER_M_N,
-		  MT9M114_CAM_SYSCTL_PLL_DIVIDER_VALUE(sensor->pll.m,
-						       sensor->pll.n),
-		  &ret);
-	cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_DIVIDER_P,
-		  MT9M114_CAM_SYSCTL_PLL_DIVIDER_P_VALUE(sensor->pll.p), &ret);
+	if (sensor->bypass_pll) {
+		cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_ENABLE,
+			  MT9M114_CAM_SYSCTL_PLL_DISABLE_VALUE, &ret);
+	} else {
+		cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_ENABLE,
+			  MT9M114_CAM_SYSCTL_PLL_ENABLE_VALUE, &ret);
+		cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_DIVIDER_M_N,
+			  MT9M114_CAM_SYSCTL_PLL_DIVIDER_VALUE(sensor->pll.m,
+							       sensor->pll.n),
+			  &ret);
+		cci_write(sensor->regmap, MT9M114_CAM_SYSCTL_PLL_DIVIDER_P,
+			  MT9M114_CAM_SYSCTL_PLL_DIVIDER_P_VALUE(sensor->pll.p),
+			  &ret);
+	}
 	cci_write(sensor->regmap, MT9M114_CAM_SENSOR_CFG_PIXCLK,
 		  sensor->pixrate, &ret);
 
@@ -2235,9 +2243,19 @@ static const struct dev_pm_ops mt9m114_pm_ops = {
  * Probe & Remove
  */
 
+static int mt9m114_verify_link_frequency(struct mt9m114 *sensor)
+{
+	unsigned int link_freq = sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY
+				? sensor->pixrate * 8 : sensor->pixrate * 2;
+	if (sensor->bus_cfg.nr_of_link_frequencies != 1 ||
+	    sensor->bus_cfg.link_frequencies[0] != link_freq)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int mt9m114_clk_init(struct mt9m114 *sensor)
 {
-	unsigned int link_freq;
 
 	/* Hardcode the PLL multiplier and dividers to default settings. */
 	sensor->pll.m = 32;
@@ -2249,19 +2267,27 @@ static int mt9m114_clk_init(struct mt9m114 *sensor)
 	 * for 16-bit per pixel, transmitted in DDR over a single lane. For
 	 * parallel mode, the sensor ouputs one pixel in two PIXCLK cycles.
 	 */
-	sensor->pixrate = clk_get_rate(sensor->clk) * sensor->pll.m
-			/ ((sensor->pll.n + 1) * (sensor->pll.p + 1));
 
-	link_freq = sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY
-		  ? sensor->pixrate * 8 : sensor->pixrate * 2;
+	/*
+	 * Check if EXTCLK fits the configured link frequency. Bypass the PLL
+	 * in this case.
+	 */
+	sensor->pixrate = clk_get_rate(sensor->clk) / 2;
+	if (mt9m114_verify_link_frequency(sensor) == 0) {
+		sensor->bypass_pll = true;
+		return 0;
+	}
 
-	if (sensor->bus_cfg.nr_of_link_frequencies != 1 ||
-	    sensor->bus_cfg.link_frequencies[0] != link_freq) {
-		dev_err(&sensor->client->dev, "Unsupported DT link-frequencies\n");
-		return -EINVAL;
+	/* Check if the PLL configuration fits the configured link frequency */
+	sensor->pixrate = clk_get_rate(sensor->clk) * sensor->pll.m
+			/ ((sensor->pll.n + 1) * (sensor->pll.p + 1));
+	if (mt9m114_verify_link_frequency(sensor) == 0) {
+		sensor->bypass_pll = false;
+		return 0;
 	}
 
-	return 0;
+	dev_err(&sensor->client->dev, "Unsupported DT link-frequencies\n");
+	return -EINVAL;
 }
 
 static int mt9m114_identify(struct mt9m114 *sensor)
-- 
2.34.1


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

* [PATCH v3 3/6] media: mt9m114: Factor out mt9m114_configure_pa
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 2/6] media: mt9m114: Bypass PLL if required Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 4/6] media: mt9m114: Allow set_selection while streaming Mathis Foerst
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

The function mt9m114_configure writes the configuration registers of both,
the pixel array (pa) and the image flow processor (ifp).
This is undesirable if only the config of the pa should be changed without
affecting the ifp.

Factor out the function mt9m114_configure_pa() that just writes the
pa-configuration.

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 drivers/media/i2c/mt9m114.c | 49 +++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
index b06003b69f6f..9a49dab65180 100644
--- a/drivers/media/i2c/mt9m114.c
+++ b/drivers/media/i2c/mt9m114.c
@@ -789,39 +789,22 @@ static int mt9m114_initialize(struct mt9m114 *sensor)
 	return 0;
 }
 
-static int mt9m114_configure(struct mt9m114 *sensor,
-			     struct v4l2_subdev_state *pa_state,
-			     struct v4l2_subdev_state *ifp_state)
+static int mt9m114_configure_pa(struct mt9m114 *sensor, struct v4l2_subdev_state *pa_state)
 {
 	const struct v4l2_mbus_framefmt *pa_format;
 	const struct v4l2_rect *pa_crop;
-	const struct mt9m114_format_info *ifp_info;
-	const struct v4l2_mbus_framefmt *ifp_format;
-	const struct v4l2_rect *ifp_crop;
-	const struct v4l2_rect *ifp_compose;
-	unsigned int hratio, vratio;
-	u64 output_format;
 	u64 read_mode;
+	unsigned int hratio, vratio;
 	int ret = 0;
 
 	pa_format = v4l2_subdev_state_get_format(pa_state, 0);
 	pa_crop = v4l2_subdev_state_get_crop(pa_state, 0);
 
-	ifp_format = v4l2_subdev_state_get_format(ifp_state, 1);
-	ifp_info = mt9m114_format_info(sensor, 1, ifp_format->code);
-	ifp_crop = v4l2_subdev_state_get_crop(ifp_state, 0);
-	ifp_compose = v4l2_subdev_state_get_compose(ifp_state, 0);
-
 	ret = cci_read(sensor->regmap, MT9M114_CAM_SENSOR_CONTROL_READ_MODE,
 		       &read_mode, NULL);
 	if (ret < 0)
 		return ret;
 
-	ret = cci_read(sensor->regmap, MT9M114_CAM_OUTPUT_FORMAT,
-		       &output_format, NULL);
-	if (ret < 0)
-		return ret;
-
 	hratio = pa_crop->width / pa_format->width;
 	vratio = pa_crop->height / pa_format->height;
 
@@ -853,6 +836,34 @@ static int mt9m114_configure(struct mt9m114 *sensor,
 	cci_write(sensor->regmap, MT9M114_CAM_SENSOR_CONTROL_READ_MODE,
 		  read_mode, &ret);
 
+	return ret;
+}
+
+static int mt9m114_configure(struct mt9m114 *sensor,
+			     struct v4l2_subdev_state *pa_state,
+			     struct v4l2_subdev_state *ifp_state)
+{
+	const struct mt9m114_format_info *ifp_info;
+	const struct v4l2_mbus_framefmt *ifp_format;
+	const struct v4l2_rect *ifp_crop;
+	const struct v4l2_rect *ifp_compose;
+	u64 output_format;
+	int ret = 0;
+
+	ifp_format = v4l2_subdev_state_get_format(ifp_state, 1);
+	ifp_info = mt9m114_format_info(sensor, 1, ifp_format->code);
+	ifp_crop = v4l2_subdev_state_get_crop(ifp_state, 0);
+	ifp_compose = v4l2_subdev_state_get_compose(ifp_state, 0);
+
+	ret = cci_read(sensor->regmap, MT9M114_CAM_OUTPUT_FORMAT,
+		       &output_format, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = mt9m114_configure_pa(sensor, pa_state);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Color pipeline (IFP) cropping and scaling. Subtract 4 from the left
 	 * and top coordinates to compensate for the lines and columns removed
-- 
2.34.1


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

* [PATCH v3 4/6] media: mt9m114: Allow set_selection while streaming
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
                   ` (2 preceding siblings ...)
  2025-03-05 10:14 ` [PATCH v3 3/6] media: mt9m114: Factor out mt9m114_configure_pa Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 5/6] media: mt9m114: Fix deadlock in get_frame_interval/set_frame_interval Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 6/6] media: mt9m114: Set pad-slew-rate Mathis Foerst
  5 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

The current implementation does not apply changes to the crop-
configuration of the sensor immediately if the sensor is in
streaming state. The user has to stop and restart the stream for
the changes to be applied.
The same holds for changes to the V4L2 controls HFLIP & VFLIP.
This can be undesirable e.g. in a calibration usecase where the user
wants to see the impact of his changes in a live video stream.

Call mt9m114_configure_pa() in mt9m114_pa_set_selection() if the sensor is
in streaming state and issue a CONFIG_CHANGE to apply the changes
immediately.
Issue a CONFIG_CHANGE when the V4L2 controls HFLIP or VFLIP are set if the
sensor is in streaming state to apply the change immediately.

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 drivers/media/i2c/mt9m114.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
index 9a49dab65180..65b9124e464f 100644
--- a/drivers/media/i2c/mt9m114.c
+++ b/drivers/media/i2c/mt9m114.c
@@ -1098,6 +1098,13 @@ static int mt9m114_pa_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = cci_update_bits(sensor->regmap,
 				      MT9M114_CAM_SENSOR_CONTROL_READ_MODE,
 				      mask, ctrl->val ? mask : 0, NULL);
+		if (ret)
+			return ret;
+		if (sensor->streaming) {
+			// Changing the flip config while streaming requires a CONFIG_CHANGE
+			ret = mt9m114_set_state(sensor,
+						MT9M114_SYS_STATE_ENTER_CONFIG_CHANGE);
+		}
 		break;
 
 	case V4L2_CID_VFLIP:
@@ -1105,6 +1112,13 @@ static int mt9m114_pa_s_ctrl(struct v4l2_ctrl *ctrl)
 		ret = cci_update_bits(sensor->regmap,
 				      MT9M114_CAM_SENSOR_CONTROL_READ_MODE,
 				      mask, ctrl->val ? mask : 0, NULL);
+		if (ret)
+			return ret;
+		if (sensor->streaming) {
+			// Changing the flip config while streaming requires a CONFIG_CHANGE
+			ret = mt9m114_set_state(sensor,
+						MT9M114_SYS_STATE_ENTER_CONFIG_CHANGE);
+		}
 		break;
 
 	default:
@@ -1286,6 +1300,7 @@ static int mt9m114_pa_set_selection(struct v4l2_subdev *sd,
 	struct mt9m114 *sensor = pa_to_mt9m114(sd);
 	struct v4l2_mbus_framefmt *format;
 	struct v4l2_rect *crop;
+	int ret = 0;
 
 	if (sel->target != V4L2_SEL_TGT_CROP)
 		return -EINVAL;
@@ -1316,10 +1331,21 @@ static int mt9m114_pa_set_selection(struct v4l2_subdev *sd,
 	format->width = crop->width;
 	format->height = crop->height;
 
-	if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-		mt9m114_pa_ctrl_update_blanking(sensor, format);
+	if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+		return ret;
 
-	return 0;
+	mt9m114_pa_ctrl_update_blanking(sensor, format);
+
+	/* Apply values immediately if streaming */
+	if (sensor->streaming) {
+		ret = mt9m114_configure_pa(sensor, state);
+		if (ret)
+			return ret;
+		// Changing the cropping config requires a CONFIG_CHANGE
+		ret = mt9m114_set_state(sensor,	MT9M114_SYS_STATE_ENTER_CONFIG_CHANGE);
+	}
+
+	return ret;
 }
 
 static const struct v4l2_subdev_pad_ops mt9m114_pa_pad_ops = {
-- 
2.34.1


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

* [PATCH v3 5/6] media: mt9m114: Fix deadlock in get_frame_interval/set_frame_interval
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
                   ` (3 preceding siblings ...)
  2025-03-05 10:14 ` [PATCH v3 4/6] media: mt9m114: Allow set_selection while streaming Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  2025-03-05 10:14 ` [PATCH v3 6/6] media: mt9m114: Set pad-slew-rate Mathis Foerst
  5 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst, stable

Getting / Setting the frame interval using the V4L2 subdev pad ops
get_frame_interval/set_frame_interval causes a deadlock, as the
subdev state is locked in the [1] but also in the driver itself.

In [2] it's described that the caller is responsible to acquire and
release the lock in this case. Therefore, acquiring the lock in the
driver is wrong.

Remove the lock acquisitions/releases from mt9m114_ifp_get_frame_interval()
and mt9m114_ifp_set_frame_interval().

[1] drivers/media/v4l2-core/v4l2-subdev.c - line 1129
[2] Documentation/driver-api/media/v4l2-subdev.rst

Fixes: 24d756e914fc ("media: i2c: Add driver for onsemi MT9M114 camera sensor")
Cc: stable@vger.kernel.org

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 drivers/media/i2c/mt9m114.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
index 65b9124e464f..79c97ab19be9 100644
--- a/drivers/media/i2c/mt9m114.c
+++ b/drivers/media/i2c/mt9m114.c
@@ -1644,13 +1644,9 @@ static int mt9m114_ifp_get_frame_interval(struct v4l2_subdev *sd,
 	if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE)
 		return -EINVAL;
 
-	mutex_lock(sensor->ifp.hdl.lock);
-
 	ival->numerator = 1;
 	ival->denominator = sensor->ifp.frame_rate;
 
-	mutex_unlock(sensor->ifp.hdl.lock);
-
 	return 0;
 }
 
@@ -1669,8 +1665,6 @@ static int mt9m114_ifp_set_frame_interval(struct v4l2_subdev *sd,
 	if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE)
 		return -EINVAL;
 
-	mutex_lock(sensor->ifp.hdl.lock);
-
 	if (ival->numerator != 0 && ival->denominator != 0)
 		sensor->ifp.frame_rate = min_t(unsigned int,
 					       ival->denominator / ival->numerator,
@@ -1684,8 +1678,6 @@ static int mt9m114_ifp_set_frame_interval(struct v4l2_subdev *sd,
 	if (sensor->streaming)
 		ret = mt9m114_set_frame_rate(sensor);
 
-	mutex_unlock(sensor->ifp.hdl.lock);
-
 	return ret;
 }
 
-- 
2.34.1


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

* [PATCH v3 6/6] media: mt9m114: Set pad-slew-rate
  2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
                   ` (4 preceding siblings ...)
  2025-03-05 10:14 ` [PATCH v3 5/6] media: mt9m114: Fix deadlock in get_frame_interval/set_frame_interval Mathis Foerst
@ 2025-03-05 10:14 ` Mathis Foerst
  5 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-05 10:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathis Foerst, Laurent Pinchart, Mauro Carvalho Chehab,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
	linux-media, devicetree, manuel.traut, mathis.foerst

The MT9M114 supports the different slew rates (0 to 7) on the output pads.
At the moment, this is hardcoded to 7 (the fastest rate).
The user might want to change this values due to EMC requirements.

Read the 'onnn,slew-rate' from the DT and configure the pad slew rates of
the output pads accordingly in mt9m114_initialize().
Remove the hardcoded slew rate setting from the mt9m114_init table.

Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
---
 drivers/media/i2c/mt9m114.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
index 79c97ab19be9..df694a3e14d5 100644
--- a/drivers/media/i2c/mt9m114.c
+++ b/drivers/media/i2c/mt9m114.c
@@ -42,6 +42,9 @@
 #define MT9M114_RESET_AND_MISC_CONTROL			CCI_REG16(0x001a)
 #define MT9M114_RESET_SOC					BIT(0)
 #define MT9M114_PAD_SLEW				CCI_REG16(0x001e)
+#define MT9M114_PAD_SLEW_MIN					0x00
+#define MT9M114_PAD_SLEW_MAX					0x07
+#define MT9M114_PAD_SLEW_DEFAULT				0x07
 #define MT9M114_PAD_CONTROL				CCI_REG16(0x0032)
 
 /* XDMA registers */
@@ -388,6 +391,7 @@ struct mt9m114 {
 
 	unsigned int pixrate;
 	bool streaming;
+	u32 pad_slew_rate;
 
 	/* Pixel Array */
 	struct {
@@ -645,9 +649,6 @@ static const struct cci_reg_sequence mt9m114_init[] = {
 	{ MT9M114_CAM_SENSOR_CFG_FINE_INTEG_TIME_MAX,	1459 },
 	{ MT9M114_CAM_SENSOR_CFG_FINE_CORRECTION,	96 },
 	{ MT9M114_CAM_SENSOR_CFG_REG_0_DATA,		32 },
-
-	/* Miscellaneous settings */
-	{ MT9M114_PAD_SLEW,				0x0777 },
 };
 
 /* -----------------------------------------------------------------------------
@@ -778,6 +779,13 @@ static int mt9m114_initialize(struct mt9m114 *sensor)
 	if (ret < 0)
 		return ret;
 
+	value = (sensor->pad_slew_rate & 0xF)
+	      | (sensor->pad_slew_rate & 0xF) << 4
+	      |	(sensor->pad_slew_rate & 0xF) << 8;
+	cci_write(sensor->regmap, MT9M114_PAD_SLEW, value, &ret);
+	if (ret < 0)
+		return ret;
+
 	ret = mt9m114_set_state(sensor, MT9M114_SYS_STATE_ENTER_CONFIG_CHANGE);
 	if (ret < 0)
 		return ret;
@@ -2357,6 +2365,8 @@ static int mt9m114_parse_dt(struct mt9m114 *sensor)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(&sensor->client->dev);
 	struct fwnode_handle *ep;
+	struct device_node *dev_node = sensor->client->dev.of_node;
+	u32 slew_rate;
 	int ret;
 
 	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
@@ -2385,6 +2395,11 @@ static int mt9m114_parse_dt(struct mt9m114 *sensor)
 		goto error;
 	}
 
+	ret = of_property_read_u32(dev_node, "onnn,slew-rate", &slew_rate);
+	if (ret || slew_rate < MT9M114_PAD_SLEW_MIN || slew_rate > MT9M114_PAD_SLEW_MAX)
+		slew_rate = MT9M114_PAD_SLEW_DEFAULT;
+	sensor->pad_slew_rate = slew_rate;
+
 	return 0;
 
 error:
-- 
2.34.1


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

* Re: [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding
  2025-03-05 10:14 ` [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding Mathis Foerst
@ 2025-03-05 13:08   ` Rob Herring
  2025-03-07  9:41     ` Mathis Foerst
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2025-03-05 13:08 UTC (permalink / raw)
  To: Mathis Foerst
  Cc: linux-kernel, Laurent Pinchart, Mauro Carvalho Chehab,
	Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, linux-media,
	devicetree, manuel.traut, mathis.foerst

On Wed, Mar 05, 2025 at 11:14:48AM +0100, Mathis Foerst wrote:
> The MT9M114 supports the different slew rates (0 to 7) on the output pads.
> At the moment, this is hardcoded to 7 (the fastest rate).
> The user might want to change this values due to EMC requirements.
> 
> Add the 'onnn,slew-rate' property to the MT9M114 DT-bindings for selecting
> the desired slew rate.
> 
> Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
> ---
>  .../devicetree/bindings/media/i2c/onnn,mt9m114.yaml      | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> index f6b87892068a..c184bc04b743 100644
> --- a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> @@ -70,6 +70,15 @@ properties:
>            - bus-type
>            - link-frequencies
>  
> +  onnn,slew-rate:

Just 'slew-rate' which is already defined in the pinctrl binding.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Slew rate ot the output pads DOUT[7:0], LINE_VALID, FRAME_VALID and
> +      PIXCLK. Higher values imply steeper voltage-flanks on the pads.
> +    minimum: 0
> +    maximum: 7
> +    default: 7
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

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

* Re: [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding
  2025-03-05 13:08   ` Rob Herring
@ 2025-03-07  9:41     ` Mathis Foerst
  0 siblings, 0 replies; 9+ messages in thread
From: Mathis Foerst @ 2025-03-07  9:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, Laurent Pinchart, Mauro Carvalho Chehab,
	Krzysztof Kozlowski, Conor Dooley, Sakari Ailus, linux-media,
	devicetree, manuel.traut, mathis.foerst

Hi Rob,

thank you for your input.

On Wed, Mar 05, 2025 at 07:08:21AM -0600, Rob Herring wrote:
> On Wed, Mar 05, 2025 at 11:14:48AM +0100, Mathis Foerst wrote:
> > The MT9M114 supports the different slew rates (0 to 7) on the output pads.
> > At the moment, this is hardcoded to 7 (the fastest rate).
> > The user might want to change this values due to EMC requirements.
> > 
> > Add the 'onnn,slew-rate' property to the MT9M114 DT-bindings for selecting
> > the desired slew rate.
> > 
> > Signed-off-by: Mathis Foerst <mathis.foerst@mt.com>
> > ---
> >  .../devicetree/bindings/media/i2c/onnn,mt9m114.yaml      | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> > index f6b87892068a..c184bc04b743 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> > @@ -70,6 +70,15 @@ properties:
> >            - bus-type
> >            - link-frequencies
> >  
> > +  onnn,slew-rate:
> 
> Just 'slew-rate' which is already defined in the pinctrl binding.

Okay, I fixed this in v4.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Slew rate ot the output pads DOUT[7:0], LINE_VALID, FRAME_VALID and
> > +      PIXCLK. Higher values imply steeper voltage-flanks on the pads.
> > +    minimum: 0
> > +    maximum: 7
> > +    default: 7
> > +
> >  required:
> >    - compatible
> >    - reg
> > -- 
> > 2.34.1
> > 

Best regards,
Mathis Foerst

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

end of thread, other threads:[~2025-03-07  9:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 10:14 [PATCH v3 0/6] MT9M114 driver bugfix and improvements Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 1/6] media: dt-bindings: mt9m114: Add onnn,slew-rate DT-binding Mathis Foerst
2025-03-05 13:08   ` Rob Herring
2025-03-07  9:41     ` Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 2/6] media: mt9m114: Bypass PLL if required Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 3/6] media: mt9m114: Factor out mt9m114_configure_pa Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 4/6] media: mt9m114: Allow set_selection while streaming Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 5/6] media: mt9m114: Fix deadlock in get_frame_interval/set_frame_interval Mathis Foerst
2025-03-05 10:14 ` [PATCH v3 6/6] media: mt9m114: Set pad-slew-rate Mathis Foerst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).