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

[Jaehoon Chung]

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

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>>
>>> Regards,
>>> Simon
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
> 
> Regards,
> Akshay Saraswat 
>