[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHCN7x+xM13-DQ-miA7NbvY1wsr0197LQYCbLFbpur7oDf9TnQ@mail.gmail.com>
Date: Tue, 31 Dec 2024 11:02:38 -0600
From: Adam Ford <aford173@...il.com>
To: Pei Xiao <xiaopei01@...inos.cn>
Cc: geert@...ux-m68k.org, aford@...conembedded.com, arnd@...db.de,
dominique.martinet@...ark-techno.com, frieder.schrempf@...tron.de,
kishon@...nel.org, l.stach@...gutronix.de, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, m.felsch@...gutronix.de, sandor.yu@....com,
u.kleine-koenig@...libre.com, vkoul@...nel.org
Subject: Re: [PATCH] phy: freescale: fsl-samsung-hdmi: fix build error in fsl_samsung_hdmi_phy_configure_pll_lock_det
On Mon, Dec 30, 2024 at 8:19 PM Pei Xiao <xiaopei01@...inos.cn> wrote:
>
>
> 在 2024/12/31 10:11, Adam Ford 写道:
> > On Sun, Dec 29, 2024 at 8:11 PM Pei Xiao <xiaopei01@...inos.cn> wrote:
> >> FIELD_PREP() checks that a constant fits into the available bitfield,
> >> but the index div equals to 4,is out of range, which gcc
> >> correctly complains about:
> >>
> >> 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_538’
> >> declared with attribute error: FIELD_PREP: value too large for the field
> >> 542 | _compiletime_assert(condition, msg, __compiletime_assert_,
> >> __COUNTER__)
> >> | ^
> >> ././include/linux/compiler_types.h:523:4: note: in definition of
> >> macro ‘__compiletime_assert’ 523 | prefix ## suffix();
> >> | ^~~~~~
> >> ././include/linux/compiler_types.h:542:2: note: in expansion of macro
> >> ‘_compiletime_assert’
> >> 542 | _compiletime_assert(condition, msg, __compiletime_assert_,
> >> __COUNTER__)
> >>
> >> Rework this so the field value which is restricted to the range of 0 to 3,
> >> so build error will fix.
> >>
> >> Fixes: d567679f2b6a ("phy: freescale: fsl-samsung-hdmi: Clean up fld_tg_code calculation")
> >> Signed-off-by: Pei Xiao <xiaopei01@...inos.cn>
> >> ---
> >> drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> >> index 5eac70a1e858..3e4d1a5160ea 100644
> >> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> >> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> >> @@ -341,7 +341,7 @@ fsl_samsung_hdmi_phy_configure_pll_lock_det(struct fsl_samsung_hdmi_phy *phy,
> >> break;
> >> }
> >>
> >> - writeb(FIELD_PREP(REG12_CK_DIV_MASK, div), phy->regs + PHY_REG(12));
> >> + writeb(FIELD_PREP(REG12_CK_DIV_MASK, div == 4 ? div - 1 : div), phy->regs + PHY_REG(12));
> > The for-loop above this line states: for (div = 0; div < 4; div++)
> > How could this ever reach 4? If it did reach 4, the calculation for
> > int_pllclk would need to be recalculated since int_pllclk = pclk / (1
> > << div);
>
> yes, you are right,may be never reach 4,so div - 1 wille fix FIELD_PREP
>
> build error.
>
> #define REG12_CK_DIV_MASK GENMASK(5, 4)
>
> only two bit ,max value is 3.
Would doing a logical AND of (div & 0x03) make the compiler error go
away? I feel like the evaluation of checking if div==4 is unnecessary
since the for-loop won't let it get to 4 and it might add an
additional instruction.
adam
>
> thanks!
>
> Pei.
>
> >
> > adam
> >> /*
> >> * Calculation for the frequency lock detector target code (fld_tg_code)
> >> --
> >> 2.25.1
> >>
Powered by blists - more mailing lists