[U-Boot] Question about compile warnings of exynos clock

[Simon Glass]

Hi,

On 12 January 2015 at 21:58, Jaehoon Chung <jh80.chung at samsung.com> wrote:
> Hi.
>
> On 01/13/2015 02:40 PM, Akshay Saraswat wrote:
>> Hi,
>>
>>> Hi,
>>>
>>>
>>> On 01/13/2015 11:56 AM, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On 12 January 2015 at 18:51, Joonyoung Shim <jy0922.shim at samsung.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 01/13/2015 11:47 AM, Simon Glass wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 12 January 2015 at 18:36, Joonyoung Shim <jy0922.shim at samsung.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 01/13/2015 11:27 AM, Simon Glass wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 12 January 2015 at 18:24, Joonyoung Shim <jy0922.shim at samsung.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I found below compile warnings,
>>>>>>>>>
>>>>>>>>>   CC      arch/arm/cpu/armv7/exynos/clock.o
>>>>>>>>> arch/arm/cpu/armv7/exynos/clock.c: In function .clock_get_periph_rate.:
>>>>>>>>> arch/arm/cpu/armv7/exynos/clock.c:265:47: warning: array subscript is above array bounds [-Warray-bounds]
>>>>>>>>>   struct clk_bit_info *bit_info = &clk_bit_info[peripheral];
>>>>>>>>>                                                ^
>>>>>>>>> arch/arm/cpu/armv7/exynos/clock.c:265:47: warning: array subscript is above array bounds [-Warray-bounds]
>>>>>>>>>
>>>>>>>>> ...
>>>>>>>>>> static unsigned long exynos5_get_periph_rate(int peripheral)
>>>>>>>>>> {
>>>>>>>>>>         struct clk_bit_info *bit_info = &clk_bit_info[peripheral];
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This can access out of bounds of clk_bit_info[] array from
>>>>>>>>> exynos5_get_periph_rate(). The peripheral value comes from
>>>>>>>>> enum periph_id but it gets out of count clk_bit_info[] array.
>>>>>>>>>
>>>>>>>>> So, i don't think exynos5_get_periph_rate is working correctly.
>>>>>>>>> Currently, exynos5_get_periph_rate is used by clock_get_periph_rate only
>>>>>>>>> from get_pwm_clk.
>>>>>>>>>
>>>>>>>>> Is it ongoing to work for generic api to get the clk freq? If not,
>>>>>>>>> let's remove exynos5_get_periph_rate and clock_get_periph_rate.
>>>>>>>>
>>>>>>>> That's going in the wrong direction - these functions make the code
>>>>>>>> much easier to follow and refactor. We should remove get_pwm_clk(),
>>>>>>>> get_mmc_clk() etc. and use generic functions instead.
>>>>>>>>
>>>>>>>
>>>>>>> I know, but current codes are wrong, so first i want to correct it even
>>>>>>> if it is old way because it's really easy. And then we can go to generic
>>>>>>> functions.
>>>>>>
>>>>>> So not remove the generic functions?
>>>>>>
>>>>>> Is it this line which is broken?
>>>>>>
>>>>>> clock_get_periph_rate(PERIPH_ID_PWM0);
>>>>>>
>>>>>> If so, it seems like everything else uses its own function. It should
>>>>>> all move easily to the generic function I think.
>>>>>>
>>>>>
>>>>> The basic problem is the generic function is wrong.
>>>>
>>>> Oh dear, does it not work for anything?
>>>
>>> I have posted the patch relevant to this. I removed the generic peripheral clock API.
>>> It's not used anywhere. Just get clock for pwm0.
>>>
>>> how about?
>>>
>>> https://patchwork.ozlabs.org/patch/426535/
>>>
>>
>> I dont think it's a good idea to remove this function and expect to put in same
>> effort again in future. Instead, fixing this warning should be easier.
>>
>> By the way, I don't see the mentioned warning when I compile ToT u-boot-samsung
>> for smdk5250 and smdk5420 with arm-2011.09 compiler. Do we need any other patch
>> or config to encounter this?
>
> It is critical problem. I don't understand how it's working fine.
> PERIPH_ID_PWM0 is 132, that's out of boundary for clk_bit_info. isn't?
> Then what's clk_bit_info[132]'s value and how access it?
> If you fix this problem, we need not to remove clock_get_periph_rate()..
> But if we need to wait for long time, i want to remove this api.
>
> Best Regards,
> Jaehoon Chung

It looks like Akshay has sent a patch super-fast  Does that resolve the issue?

Regards,
Simon