[PATCH V2 3/3] phy: freescale: fsl-samsung-hdmi: Clean up fld_tg_code calculation
Geert Uytterhoeven
geert at linux-m68k.org
Fri Dec 13 07:16:21 PST 2024
Hi Adam,
On Fri, Dec 13, 2024 at 3:57 PM Adam Ford <aford173 at gmail.com> wrote:
> On Fri, Dec 13, 2024 at 8:13 AM Geert Uytterhoeven <geert at linux-m68k.org> wrote:
> > On Sat, Oct 26, 2024 at 3:21 PM Adam Ford <aford173 at gmail.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 at gmail.com>
> > > Reviewed-by: Frieder Schrempf <frieder.schrempf at kontron.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....
>
> I use the default compiler on Ubuntu 24.04.
> Do you think it's best that I just replace the FIELD_PREP macro with
> manual code for that line?
I think let's wait and see...
Perhaps this (compiler bug?) can be worked around in the
FIELD_PREP() macro itself.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-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
More information about the linux-phy
mailing list