Message ID | 1448897699-20475-1-git-send-email-maxime.ripard@free-electrons.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 11/30, Maxime Ripard wrote: > Contrary to what the datasheet says, the pre divider doesn't seem to be > incremented by one in the PLL2, but just uses the value from the register, > with 0 being a bypass. > > This fixes the audio playing too fast. > > Since we now have the same pre-divider flags, and the only difference with > the A10 is the post-divider offset, also remove the structure to just pass > the offset as an argument. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > > Hi Stephen, Mike, > > Could you apply this patch for 4.4? > I take it this should have a Fixes: eb662f854710 ("clk: sunxi: pll2: Add A13 support") attached to it?
On 11/30, Maxime Ripard wrote: > @@ -191,25 +186,17 @@ err_unmap: > iounmap(reg); > } > > -static struct sun4i_pll2_data sun4i_a10_pll2_data = { > - .pre_div_flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, > -}; > - > static void __init sun4i_a10_pll2_setup(struct device_node *node) > { > - sun4i_pll2_setup(node, &sun4i_a10_pll2_data); > + sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); And it doesn't compile, because we just deleted the data that this is taking an address of. Hmph.
Hi Maxime, [auto build test ERROR on: clk/clk-next] [also build test ERROR on: v4.4-rc3 next-20151127] url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/clk-sunxi-pll2-Fix-clock-running-too-fast/20151130-233840 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next config: arm-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/clk/sunxi/clk-a10-pll2.c: In function 'sun4i_a10_pll2_setup': >> drivers/clk/sunxi/clk-a10-pll2.c:191:26: error: 'sun4i_a10_pll2_data' undeclared (first use in this function) sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); ^ drivers/clk/sunxi/clk-a10-pll2.c:191:26: note: each undeclared identifier is reported only once for each function it appears in >> drivers/clk/sunxi/clk-a10-pll2.c:191:2: error: too many arguments to function 'sun4i_pll2_setup' sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); ^ drivers/clk/sunxi/clk-a10-pll2.c:46:20: note: declared here static void __init sun4i_pll2_setup(struct device_node *node, ^ drivers/clk/sunxi/clk-a10-pll2.c: In function 'sun5i_a13_pll2_setup': >> drivers/clk/sunxi/clk-a10-pll2.c:199:26: error: 'sun5i_a13_pll2_data' undeclared (first use in this function) sun4i_pll2_setup(node, &sun5i_a13_pll2_data, 1); ^ drivers/clk/sunxi/clk-a10-pll2.c:199:2: error: too many arguments to function 'sun4i_pll2_setup' sun4i_pll2_setup(node, &sun5i_a13_pll2_data, 1); ^ drivers/clk/sunxi/clk-a10-pll2.c:46:20: note: declared here static void __init sun4i_pll2_setup(struct device_node *node, ^ vim +/sun4i_a10_pll2_data +191 drivers/clk/sunxi/clk-a10-pll2.c 185 err_unmap: 186 iounmap(reg); 187 } 188 189 static void __init sun4i_a10_pll2_setup(struct device_node *node) 190 { > 191 sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); 192 } 193 194 CLK_OF_DECLARE(sun4i_a10_pll2, "allwinner,sun4i-a10-pll2-clk", 195 sun4i_a10_pll2_setup); 196 197 static void __init sun5i_a13_pll2_setup(struct device_node *node) 198 { > 199 sun4i_pll2_setup(node, &sun5i_a13_pll2_data, 1); 200 } 201 202 CLK_OF_DECLARE(sun5i_a13_pll2, "allwinner,sun5i-a13-pll2-clk", --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Nov 30, 2015 at 11:32:16AM -0800, Stephen Boyd wrote: > On 11/30, Maxime Ripard wrote: > > @@ -191,25 +186,17 @@ err_unmap: > > iounmap(reg); > > } > > > > -static struct sun4i_pll2_data sun4i_a10_pll2_data = { > > - .pre_div_flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, > > -}; > > - > > static void __init sun4i_a10_pll2_setup(struct device_node *node) > > { > > - sun4i_pll2_setup(node, &sun4i_a10_pll2_data); > > + sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); > > And it doesn't compile, because we just deleted the data that > this is taking an address of. Hmph. Sorry for the screw up, I'll resend a new version tomorrow... Maxime
diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c index 5484c31ec568..30d4bfb4cd3d 100644 --- a/drivers/clk/sunxi/clk-a10-pll2.c +++ b/drivers/clk/sunxi/clk-a10-pll2.c @@ -41,15 +41,10 @@ #define SUN4I_PLL2_OUTPUTS 4 -struct sun4i_pll2_data { - u32 post_div_offset; - u32 pre_div_flags; -}; - static DEFINE_SPINLOCK(sun4i_a10_pll2_lock); static void __init sun4i_pll2_setup(struct device_node *node, - struct sun4i_pll2_data *data) + int post_div_offset) { const char *clk_name = node->name, *parent; struct clk **clks, *base_clk, *prediv_clk; @@ -76,7 +71,7 @@ static void __init sun4i_pll2_setup(struct device_node *node, parent, 0, reg, SUN4I_PLL2_PRE_DIV_SHIFT, SUN4I_PLL2_PRE_DIV_WIDTH, - data->pre_div_flags, + CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, &sun4i_a10_pll2_lock); if (!prediv_clk) { pr_err("Couldn't register the prediv clock\n"); @@ -127,7 +122,7 @@ static void __init sun4i_pll2_setup(struct device_node *node, */ val = readl(reg); val &= ~(SUN4I_PLL2_POST_DIV_MASK << SUN4I_PLL2_POST_DIV_SHIFT); - val |= (SUN4I_PLL2_POST_DIV_VALUE - data->post_div_offset) << SUN4I_PLL2_POST_DIV_SHIFT; + val |= (SUN4I_PLL2_POST_DIV_VALUE - post_div_offset) << SUN4I_PLL2_POST_DIV_SHIFT; writel(val, reg); of_property_read_string_index(node, "clock-output-names", @@ -191,25 +186,17 @@ err_unmap: iounmap(reg); } -static struct sun4i_pll2_data sun4i_a10_pll2_data = { - .pre_div_flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO, -}; - static void __init sun4i_a10_pll2_setup(struct device_node *node) { - sun4i_pll2_setup(node, &sun4i_a10_pll2_data); + sun4i_pll2_setup(node, &sun4i_a10_pll2_data, 0); } CLK_OF_DECLARE(sun4i_a10_pll2, "allwinner,sun4i-a10-pll2-clk", sun4i_a10_pll2_setup); -static struct sun4i_pll2_data sun5i_a13_pll2_data = { - .post_div_offset = 1, -}; - static void __init sun5i_a13_pll2_setup(struct device_node *node) { - sun4i_pll2_setup(node, &sun5i_a13_pll2_data); + sun4i_pll2_setup(node, &sun5i_a13_pll2_data, 1); } CLK_OF_DECLARE(sun5i_a13_pll2, "allwinner,sun5i-a13-pll2-clk",
Contrary to what the datasheet says, the pre divider doesn't seem to be incremented by one in the PLL2, but just uses the value from the register, with 0 being a bypass. This fixes the audio playing too fast. Since we now have the same pre-divider flags, and the only difference with the A10 is the post-divider offset, also remove the structure to just pass the offset as an argument. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- Hi Stephen, Mike, Could you apply this patch for 4.4? Thanks, Maxime drivers/clk/sunxi/clk-a10-pll2.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-)