[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdVx9zYw7ZpyH=d_rs==a+_jzi--Fax5cVe-8UW+RvRx+g@mail.gmail.com>
Date: Fri, 13 Dec 2024 15:13:15 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Adam Ford <aford173@...il.com>
Cc: linux-phy@...ts.infradead.org, aford@...conembedded.com, sandor.yu@....com,
Frieder Schrempf <frieder.schrempf@...tron.de>, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Dominique Martinet <dominique.martinet@...ark-techno.com>,
Marco Felsch <m.felsch@...gutronix.de>,
Uwe Kleine-König <u.kleine-koenig@...libre.com>,
Lucas Stach <l.stach@...gutronix.de>, linux-kernel@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH V2 3/3] phy: freescale: fsl-samsung-hdmi: Clean up
fld_tg_code calculation
Hi Adam,
On Sat, Oct 26, 2024 at 3:21 PM Adam Ford <aford173@...il.com> wrote:
> Currently, the calcuation for fld_tg_code is based on a lookup table,
calculation (everywhere)
> but there are gaps in the lookup table, and frequencies in these
> gaps may not properly use the correct divider. Based on the description
> of FLD_CK_DIV, the internal PLL frequency should be less than 50 MHz,
> so directly calcuate the value of FLD_CK_DIV from pixclk.
> This allow for proper calcuation of any pixel clock and eliminates a
> few gaps in the LUT.
>
> Since the value of the int_pllclk is in Hz, do the fixed-point
> math in Hz to achieve a more accurate value and reduces the complexity
> of the caluation to 24MHz * (256 / int_pllclk).
>
> Fixes: 6ad082bee902 ("phy: freescale: add Samsung HDMI PHY")
> Signed-off-by: Adam Ford <aford173@...il.com>
> Reviewed-by: Frieder Schrempf <frieder.schrempf@...tron.de>
Thanks for your patch, which is now commit d567679f2b6a8bce ("phy:
freescale: fsl-samsung-hdmi: Clean up fld_tg_code calculation") in
next-20241209 and later.
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -331,25 +331,17 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
> {
> u32 pclk = cfg->pixclk;
> u32 fld_tg_code;
> - u32 pclk_khz;
> - u8 div = 1;
> -
> - switch (cfg->pixclk) {
> - case 22250000 ... 47500000:
> - div = 1;
> - break;
> - case 50349650 ... 99000000:
> - div = 2;
> - break;
> - case 100699300 ... 198000000:
> - div = 4;
> - break;
> - case 205000000 ... 297000000:
> - div = 8;
> - break;
> + u32 int_pllclk;
> + u8 div;
> +
> + /* Find int_pllclk speed */
> + for (div = 0; div < 4; div++) {
> + int_pllclk = pclk / (1 << div);
> + if (int_pllclk < (50 * MHZ))
> + break;
> }
>
> - writeb(FIELD_PREP(REG12_CK_DIV_MASK, ilog2(div)), phy->regs + PHY_REG(12));
> + writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
This causes a build failure on m68k with gcc version 9.5.0 (Ubuntu
9.5.0-1ubuntu1~22.04):
CC [M] drivers/phy/freescale/phy-fsl-samsung-hdmi.o
In file included from ./arch/m68k/include/asm/io_mm.h:25,
from ./arch/m68k/include/asm/io.h:8,
from ./include/linux/io.h:14,
from ./include/linux/iopoll.h:14,
from drivers/phy/freescale/phy-fsl-samsung-hdmi.c:12:
In function ‘fsl_samsung_hdmi_phy_configure_pll_lock_det’,
inlined from ‘fsl_samsung_hdmi_phy_configure’ at
drivers/phy/freescale/phy-fsl-samsung-hdmi.c:470:2:
././include/linux/compiler_types.h:542:38: error: call to
‘__compiletime_assert_147’ declared with attribute error: FIELD_PREP:
value too large for the field
542 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
./arch/m68k/include/asm/raw_io.h:30:82: note: in definition of macro ‘out_8’
30 | #define out_8(addr,b) (void)((*(__force volatile u8 *)
(unsigned long)(addr)) = (b))
|
^
drivers/phy/freescale/phy-fsl-samsung-hdmi.c:344:2: note: in expansion
of macro ‘writeb’
344 | writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
| ^~~~~~
././include/linux/compiler_types.h:530:2: note: in expansion of macro
‘__compiletime_assert’
530 | __compiletime_assert(condition, msg, prefix, suffix)
| ^~~~~~~~~~~~~~~~~~~~
././include/linux/compiler_types.h:542:2: note: in expansion of macro
‘_compiletime_assert’
542 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro
‘compiletime_assert’
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
./include/linux/bitfield.h:68:3: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
| ^~~~~~~~~~~~~~~~
./include/linux/bitfield.h:115:3: note: in expansion of macro ‘__BF_FIELD_CHECK’
115 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~
drivers/phy/freescale/phy-fsl-samsung-hdmi.c:344:9: note: in expansion
of macro ‘FIELD_PREP’
344 | writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
| ^~~~~~~~~~
As it builds fine on i386, I looked at the preprocessed source files,
but didn't see any differences that could explain this.
I changed cross-compiler to gcc version 10.5.0 (Ubuntu 10.5.0-1ubuntu1~22.04),
and that fixed the issue on m68k.
I changed the native compiler to gcc-9, and the build started failing
on i386 and x86_64, too....
>
> /*
> * Calculation for the frequency lock detector target code (fld_tg_code)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists