On 12/04/2011 06:59 AM, Anatolij Gustschin wrote:
> On Sun, 4 Dec 2011 12:30:40 +0100
> Marek Vasut <marek.va...@gmail.com> wrote:
>
>>> Fix:
>>> clocks.c: In function 'setup_post_dividers':
>>> clocks.c:175: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:177: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:179: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:181: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:183: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:185: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:187: warning: comparison is always true due to limited range of
>>> data type
>>> clocks.c:189: warning: comparison is always true due to limited range of
>>> data type
>>>
>>> Signed-off-by: Anatolij Gustschin <ag...@denx.de>
>>> Cc: sricharan <r.sricha...@ti.com>
>>> Cc: Tom Rini <tr...@ti.com>
>>> ---
>>> Some notes:
>>>
>>> - GCC v4.5.1 didn't warn here
>>> - GCC v4.6.1 seems to have a bug and can't compile this code:
>>> clocks.c: In function 'enable_non_essential_clocks':
>>> clocks.c:349:13: internal compiler error: in decode_addr_const, at
>>> varasm.c:2632
>>>
>>> arch/arm/include/asm/arch-omap5/clocks.h | 16 ++++++++--------
>>> 1 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-omap5/clocks.h
>>> b/arch/arm/include/asm/arch-omap5/clocks.h index fa99f65..d0e6dd6 100644
>>> --- a/arch/arm/include/asm/arch-omap5/clocks.h
>>> +++ b/arch/arm/include/asm/arch-omap5/clocks.h
>>> @@ -686,14 +686,14 @@ struct dpll_regs {
>>> struct dpll_params {
>>> u32 m;
>>> u32 n;
>>> - u8 m2;
>>> - u8 m3;
>>> - u8 h11;
>>> - u8 h12;
>>> - u8 h13;
>>> - u8 h14;
>>> - u8 h22;
>>> - u8 h23;
>>> + s8 m2;
>>> + s8 m3;
>>> + s8 h11;
>>> + s8 h12;
>>> + s8 h13;
>>> + s8 h14;
>>> + s8 h22;
>>> + s8 h23;
>>> };
>>>
>>> extern struct omap5_prcm_regs *const prcm;
>>
>> Make clock registers a signed type? whoa
>
> No, we don't make registers a signed type. This is parameters structure
> for some parameter tables containing -1 as an indicator that the
> parameter shouldn't be written to the register. Using unsigned type
> for structure field results in parameter value 255:
>
> static const struct dpll_params per_dpll_params_768mhz[NUM_SYS_CLKS] = {
> {32, 0, 4, 3, 6, 4, -1, 2, -1, -1}, /* 12 MHz */
> {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1}, /* 13 MHz */
> {160, 6, 4, 3, 6, 4, -1, 2, -1, -1}, /* 16.8 MHz */
> {20, 0, 4, 3, 6, 4, -1, 2, -1, -1}, /* 19.2 MHz */
> {192, 12, 4, 3, 6, 4, -1, 2, -1, -1}, /* 26 MHz */
> {-1, -1, -1, -1, -1, -1, -1, -1, -1, -1}, /* 27 MHz */
> {10, 0, 4, 3, 6, 4, -1, 2, -1, -1} /* 38.4 MHz */
> };
>
> The code then checks:
>
> void setup_post_dividers(u32 *const base, const struct dpll_params *params)
> {
> struct dpll_regs *const dpll_regs = (struct dpll_regs *)base;
>
> /* Setup post-dividers */
> if (params->m2 >= 0)
> writel(params->m2, &dpll_regs->cm_div_m2_dpll);
> if (params->m3 >= 0)
> writel(params->m3, &dpll_regs->cm_div_m3_dpll);
> if (params->h11 >= 0)
> writel(params->h11, &dpll_regs->cm_div_h11_dpll);
> if (params->h12 >= 0)
> writel(params->h12, &dpll_regs->cm_div_h12_dpll);
> if (params->h13 >= 0)
> writel(params->h13, &dpll_regs->cm_div_h13_dpll);
> if (params->h14 >= 0)
> writel(params->h14, &dpll_regs->cm_div_h14_dpll);
> if (params->h22 >= 0)
> writel(params->h22, &dpll_regs->cm_div_h22_dpll);
> if (params->h23 >= 0)
> writel(params->h23, &dpll_regs->cm_div_h23_dpll);
> }
>
> The result is that the registers will always be written to, since
> the comparison is always true. This is apparently not intended in
> the code.
>
> The actual registers structure 'struct dpll_regs' uses unsigned type.
>
> This sneaked in in the commit 2e5ba489 adding omap5 clock support.
> The similar parameter structure for omap4 used signed type for the
> fields in question.
>
> Newer gcc doesn't warn here unless -Wextra option is used.
Sricharan, my examination, this analysis is correct, can you confirm
that omap5 is supposed to work like omap4 in this case? Thanks.
--
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
