Re: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate
From: Peng Fan
Date: Mon May 22 2023 - 22:33:26 EST
On 5/7/2023 3:53 AM, Adam Ford wrote:
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
Currently, certain clocks are derrived as a divider from their
parent clock. For some clocks, even when CLK_SET_RATE_PARENT
is set, the parent clock is not properly set which can lead
to some relatively inaccurate clock values.
Unlike imx/clk-composite-93 and imx/clk-divider-gate, it
cannot rely on calling a standard determine_rate function,
because the 8m composite clocks have a pre-divider and
post-divider. Because of this, a custom determine_rate
function is necessary to determine the maximum clock
division which is equivalent to pre-divider * the
post-divider.
With this added, the system can attempt to adjust the parent rate
when the proper flags are set which can lead to a more precise clock
value.
On the imx8mplus, no clock changes are present.
On the Mini and Nano, this can help achieve more accurate
lcdif clocks. When trying to get a pixel clock of 31.500MHz
on an imx8m Nano, the clocks divided the 594MHz down, but
left the parent rate untouched which caused a calulation error.
Not all clocks has pre/post div both.
If CLK_SET_RATE_PARENT not set, would there be any issues for
other clocks?
Regards,
Peng.
Before:
video_pll 594000000
video_pll_bypass 594000000
video_pll_out 594000000
disp_pixel 31263158
disp_pixel_clk 31263158
Variance = -236842 Hz
After this patch:
video_pll 31500000
video_pll_bypass 31500000
video_pll_out 31500000
disp_pixel 31500000
disp_pixel_clk 31500000
Variance = 0 Hz
All other clocks rates and parent were the same.
Similar results on imx8mm were found.
Fixes: 690dccc4a0bf ("Revert "clk: imx: composite-8m: Add support to determine_rate"")
Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
---
V2: Fix build warning found by build bot and fix prediv_value
and div_value because the values stored are the divisor - 1,
so we need to add 1 to the values to be correct.
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index cbf0d7955a00..7a6e3ce97133 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -119,10 +119,41 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
return ret;
}
+static int imx8m_divider_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_divider *divider = to_clk_divider(hw);
+ int prediv_value;
+ int div_value;
+
+ /* if read only, just return current value */
+ if (divider->flags & CLK_DIVIDER_READ_ONLY) {
+ u32 val;
+
+ val = readl(divider->reg);
+ prediv_value = val >> divider->shift;
+ prediv_value &= clk_div_mask(divider->width);
+ prediv_value++;
+
+ div_value = val >> PCG_DIV_SHIFT;
+ div_value &= clk_div_mask(PCG_DIV_WIDTH);
+ div_value++;
+
+ return divider_ro_determine_rate(hw, req, divider->table,
+ PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
+ divider->flags, prediv_value * div_value);
+ }
+
+ return divider_determine_rate(hw, req, divider->table,
+ PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
+ divider->flags);
+}
+
static const struct clk_ops imx8m_clk_composite_divider_ops = {
.recalc_rate = imx8m_clk_composite_divider_recalc_rate,
.round_rate = imx8m_clk_composite_divider_round_rate,
.set_rate = imx8m_clk_composite_divider_set_rate,
+ .determine_rate = imx8m_divider_determine_rate,
};
static u8 imx8m_clk_composite_mux_get_parent(struct clk_hw *hw)
--
2.39.2