diff mbox series

[1/2] media: ov5675: Derive delay cycles from the clock rate reported

Message ID 20240711-linux-next-ov5675-v1-1-69e9b6c62c16@linaro.org (mailing list archive)
State New
Headers show
Series media: ov5675: Fixup ov5675 reset failures | expand

Commit Message

Bryan O'Donoghue July 11, 2024, 10:20 a.m. UTC 
The ov5675 driver expresses its reset delays in terms of XVCLK cycles as
per the ov5675 specification. XVCLK can be anything in the range of 6 MHz
to 24 MHz inclusive.

Upstream we use 19.2 MHz however, since the delays are calculated in terms
of clock cycles as opposed to fixed intervals it makes sense to facilitate
any potential clock we might support.

Do so by reading the XVCLK rate and using the returned rate instead of
operating from a static definition.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/i2c/ov5675.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Quentin Schulz July 11, 2024, 10:32 a.m. UTC | #1 
Hi Bryan,

On 7/11/24 12:20 PM, Bryan O'Donoghue wrote:
> The ov5675 driver expresses its reset delays in terms of XVCLK cycles as
> per the ov5675 specification. XVCLK can be anything in the range of 6 MHz
> to 24 MHz inclusive.
> 
> Upstream we use 19.2 MHz however, since the delays are calculated in terms
> of clock cycles as opposed to fixed intervals it makes sense to facilitate
> any potential clock we might support.
> 
> Do so by reading the XVCLK rate and using the returned rate instead of
> operating from a static definition.
> 

We're actually running this sensor at **almost** 19.2MHz but are having 
intermittent issues, so this could probably help us too. In the end (for 
my employer), we should just add support for 24MHz as that's a frequency 
our products support but I'm lacking time right now. One day hopefully.

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

The change isn't really useful for upstream yet but makes sense to me, so:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index 3641911bc73f..92bd35133a5d 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -497,6 +497,7 @@  struct ov5675 {
 	struct media_pad pad;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct clk *xvclk;
+	u32 xvclk_rate;
 	struct gpio_desc *reset_gpio;
 	struct regulator_bulk_data supplies[OV5675_NUM_SUPPLIES];
 
@@ -973,10 +974,11 @@  static int ov5675_set_stream(struct v4l2_subdev *sd, int enable)
 static int ov5675_power_off(struct device *dev)
 {
 	/* 512 xvclk cycles after the last SCCB transation or MIPI frame end */
-	u32 delay_us = DIV_ROUND_UP(512, OV5675_XVCLK_19_2 / 1000 / 1000);
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov5675 *ov5675 = to_ov5675(sd);
+	u32 delay_us;
 
+	delay_us = DIV_ROUND_UP(512, ov5675->xvclk_rate / 1000 / 1000);
 	usleep_range(delay_us, delay_us * 2);
 
 	clk_disable_unprepare(ov5675->xvclk);
@@ -988,11 +990,13 @@  static int ov5675_power_off(struct device *dev)
 
 static int ov5675_power_on(struct device *dev)
 {
-	u32 delay_us = DIV_ROUND_UP(8192, OV5675_XVCLK_19_2 / 1000 / 1000);
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov5675 *ov5675 = to_ov5675(sd);
+	u32 delay_us;
 	int ret;
 
+	delay_us = DIV_ROUND_UP(8192, ov5675->xvclk_rate / 1000 / 1000);
+
 	ret = clk_prepare_enable(ov5675->xvclk);
 	if (ret < 0) {
 		dev_err(dev, "failed to enable xvclk: %d\n", ret);
@@ -1178,7 +1182,6 @@  static int ov5675_get_hwcfg(struct ov5675 *ov5675, struct device *dev)
 	struct v4l2_fwnode_endpoint bus_cfg = {
 		.bus_type = V4L2_MBUS_CSI2_DPHY
 	};
-	u32 xvclk_rate;
 	int ret;
 	unsigned int i, j;
 
@@ -1192,10 +1195,10 @@  static int ov5675_get_hwcfg(struct ov5675 *ov5675, struct device *dev)
 				     PTR_ERR(ov5675->xvclk));
 
 	if (ov5675->xvclk) {
-		xvclk_rate = clk_get_rate(ov5675->xvclk);
+		ov5675->xvclk_rate = clk_get_rate(ov5675->xvclk);
 	} else {
 		ret = fwnode_property_read_u32(fwnode, "clock-frequency",
-					       &xvclk_rate);
+					       &ov5675->xvclk_rate);
 
 		if (ret) {
 			dev_err(dev, "can't get clock frequency");
@@ -1203,9 +1206,9 @@  static int ov5675_get_hwcfg(struct ov5675 *ov5675, struct device *dev)
 		}
 	}
 
-	if (xvclk_rate != OV5675_XVCLK_19_2) {
+	if (ov5675->xvclk_rate != OV5675_XVCLK_19_2) {
 		dev_err(dev, "external clock rate %u is unsupported",
-			xvclk_rate);
+			ov5675->xvclk_rate);
 		return -EINVAL;
 	}