Hi Sakari,

On Mon, 2021-07-05 at 10:13 +0300, Sakari Ailus wrote:

Hi Enrico,

On Fri, Jul 02, 2021 at 11:59:17AM +0200, Stefan Riedmueller wrote:

quoted

From: Enrico Scholz <redacted>

The real and requested clock can differ and because it is used to
calculate PLL values, the real clock rate should be read.

Do you have a system where this happens? That suggests there's a wrong
value in DT.

The use case here is when the clock is supplied by one of the clock outputs of
a SOC which might not hit the requested frequency exactly due to internal PLL
configuration. So to get a better pixel clock the actual clock rate is read to
calculate the PLL parameters on the sensor. At least that's the idea.

Regards,
Stefan

The preference nowadays is to rely on assigned-clock-rates, even though
it's inherently somewhat unreliable, just as clk_set_rate(). This is an
existing driver though. The old ones could be kept for compatibility with
older DT binaries.

quoted

Signed-off-by: Enrico Scholz <redacted>
Signed-off-by: Stefan Riedmueller <redacted>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/mt9p031.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 77567341ec98..3eaaa8d44523 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c

@@ -255,6 +255,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031)
 
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9p031->subdev);
 	struct mt9p031_platform_data *pdata = mt9p031->pdata;
+	unsigned long ext_freq;
 	int ret;
 
 	mt9p031->clk = devm_clk_get(&client->dev, NULL);

@@ -265,13 +266,15 @@ static int mt9p031_clk_setup(struct mt9p031

*mt9p031)
 	if (ret < 0)
 		return ret;
 
+	ext_freq = clk_get_rate(mt9p031->clk);
+
 	/* If the external clock frequency is out of bounds for the PLL use
the
 	 * pixel clock divider only and disable the PLL.
 	 */
-	if (pdata->ext_freq > limits.ext_clock_max) {
+	if (ext_freq > limits.ext_clock_max) {
 		unsigned int div;
 
-		div = DIV_ROUND_UP(pdata->ext_freq, pdata->target_freq);
+		div = DIV_ROUND_UP(ext_freq, pdata->target_freq);
 		div = roundup_pow_of_two(div) / 2;
 
 		mt9p031->clk_div = min_t(unsigned int, div, 64);

@@ -280,7 +283,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031)
 		return 0;
 	}
 
-	mt9p031->pll.ext_clock = pdata->ext_freq;
+	mt9p031->pll.ext_clock = ext_freq;
 	mt9p031->pll.pix_clock = pdata->target_freq;
 	mt9p031->use_pll = true;