2023-05-06 17:32:12

by Adam Ford

[permalink] [raw]
Subject: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate

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 which can lead to some relatively
inaccurate clock values for child clocks.

Unlike imx/clk-composite-93 and imx/clk-divider-gate, it
cannot rely on calling a standard clock 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 in most cases.

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.

For example. 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.

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.

Unlike the previous attempt, when tested with imx8mp, no clock
rate or parent changes were found.

Fixes: 690dccc4a0bf ("Revert "clk: imx: composite-8m: Add support to determine_rate"")
Signed-off-by: Adam Ford <[email protected]>

diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index cbf0d7955a00..fd49385bd2ae 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -119,10 +119,39 @@ static int imx8m_clk_composite_divider_set_rate(struct clk_hw *hw,
return ret;
}

+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);
+
+ div_value = val >> PCG_DIV_SHIFT;
+ div_value &= clk_div_mask(PCG_DIV_WIDTH);
+
+ 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


2023-05-06 18:30:20

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate

Hi Adam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on v6.3 next-20230505]
[cannot apply to abelvesa/clk/imx linus/master]
[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/Adam-Ford/clk-imx-composite-8m-Add-imx8m_divider_determine_rate/20230507-002453
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20230506162403.375212-1-aford173%40gmail.com
patch subject: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230507/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/d92df3b38fc20beb735cb3b75f118b45d1ae304b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Adam-Ford/clk-imx-composite-8m-Add-imx8m_divider_determine_rate/20230507-002453
git checkout d92df3b38fc20beb735cb3b75f118b45d1ae304b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/clk/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/clk/imx/clk-composite-8m.c:122:5: warning: no previous prototype for 'imx8m_divider_determine_rate' [-Wmissing-prototypes]
122 | int imx8m_divider_determine_rate(struct clk_hw *hw,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/imx8m_divider_determine_rate +122 drivers/clk/imx/clk-composite-8m.c

121
> 122 int imx8m_divider_determine_rate(struct clk_hw *hw,
123 struct clk_rate_request *req)
124 {
125 struct clk_divider *divider = to_clk_divider(hw);
126 int prediv_value;
127 int div_value;
128
129 /* if read only, just return current value */
130 if (divider->flags & CLK_DIVIDER_READ_ONLY) {
131 u32 val;
132
133 val = readl(divider->reg);
134 prediv_value = val >> divider->shift;
135 prediv_value &= clk_div_mask(divider->width);
136
137 div_value = val >> PCG_DIV_SHIFT;
138 div_value &= clk_div_mask(PCG_DIV_WIDTH);
139
140 return divider_ro_determine_rate(hw, req, divider->table,
141 PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
142 divider->flags, prediv_value * div_value);
143 }
144
145 return divider_determine_rate(hw, req, divider->table,
146 PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
147 divider->flags);
148 }
149

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

2023-05-06 18:49:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate

Hi Adam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on v6.3 next-20230505]
[cannot apply to abelvesa/clk/imx linus/master]
[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/Adam-Ford/clk-imx-composite-8m-Add-imx8m_divider_determine_rate/20230507-002453
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20230506162403.375212-1-aford173%40gmail.com
patch subject: [PATCH] clk: imx: composite-8m: Add imx8m_divider_determine_rate
config: powerpc-randconfig-r012-20230507 (https://download.01.org/0day-ci/archive/20230507/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/d92df3b38fc20beb735cb3b75f118b45d1ae304b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Adam-Ford/clk-imx-composite-8m-Add-imx8m_divider_determine_rate/20230507-002453
git checkout d92df3b38fc20beb735cb3b75f118b45d1ae304b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/clk/imx/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/clk/imx/clk-composite-8m.c:122:5: warning: no previous prototype for function 'imx8m_divider_determine_rate' [-Wmissing-prototypes]
int imx8m_divider_determine_rate(struct clk_hw *hw,
^
drivers/clk/imx/clk-composite-8m.c:122:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int imx8m_divider_determine_rate(struct clk_hw *hw,
^
static
1 warning generated.


vim +/imx8m_divider_determine_rate +122 drivers/clk/imx/clk-composite-8m.c

121
> 122 int imx8m_divider_determine_rate(struct clk_hw *hw,
123 struct clk_rate_request *req)
124 {
125 struct clk_divider *divider = to_clk_divider(hw);
126 int prediv_value;
127 int div_value;
128
129 /* if read only, just return current value */
130 if (divider->flags & CLK_DIVIDER_READ_ONLY) {
131 u32 val;
132
133 val = readl(divider->reg);
134 prediv_value = val >> divider->shift;
135 prediv_value &= clk_div_mask(divider->width);
136
137 div_value = val >> PCG_DIV_SHIFT;
138 div_value &= clk_div_mask(PCG_DIV_WIDTH);
139
140 return divider_ro_determine_rate(hw, req, divider->table,
141 PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
142 divider->flags, prediv_value * div_value);
143 }
144
145 return divider_determine_rate(hw, req, divider->table,
146 PCG_PREDIV_WIDTH + PCG_DIV_WIDTH,
147 divider->flags);
148 }
149

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