| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHCN7xJBv0jqiPWj3gOJBrK67BgSNsiX6cDg-Tau4A8D16HPUA@mail.gmail.com>
Date: Fri, 13 Dec 2024 08:57:12 -0600
From: Adam Ford <aford173@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
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
On Fri, Dec 13, 2024 at 8:13 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> 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....
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?
adam
>
> >
> > /*
> > * 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