* [U-Boot] Question about compile warnings of exynos clock
@ 2015-01-13 2:24 Joonyoung Shim
2015-01-13 2:27 ` Simon Glass
0 siblings, 1 reply; 11+ messages in thread
From: Joonyoung Shim @ 2015-01-13 2:24 UTC (permalink / raw)
To: u-boot
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.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:24 [U-Boot] Question about compile warnings of exynos clock Joonyoung Shim
@ 2015-01-13 2:27 ` Simon Glass
2015-01-13 2:36 ` Joonyoung Shim
0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2015-01-13 2:27 UTC (permalink / raw)
To: u-boot
Hi,
On 12 January 2015 at 18:24, Joonyoung Shim <jy0922.shim@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.
Regards,
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:27 ` Simon Glass
@ 2015-01-13 2:36 ` Joonyoung Shim
2015-01-13 2:47 ` Simon Glass
0 siblings, 1 reply; 11+ messages in thread
From: Joonyoung Shim @ 2015-01-13 2:36 UTC (permalink / raw)
To: u-boot
Hi,
On 01/13/2015 11:27 AM, Simon Glass wrote:
> Hi,
>
> On 12 January 2015 at 18:24, Joonyoung Shim <jy0922.shim@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.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:36 ` Joonyoung Shim
@ 2015-01-13 2:47 ` Simon Glass
2015-01-13 2:51 ` Joonyoung Shim
0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2015-01-13 2:47 UTC (permalink / raw)
To: u-boot
Hi,
On 12 January 2015 at 18:36, Joonyoung Shim <jy0922.shim@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@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.
Regards,
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:47 ` Simon Glass
@ 2015-01-13 2:51 ` Joonyoung Shim
2015-01-13 2:56 ` Simon Glass
0 siblings, 1 reply; 11+ messages in thread
From: Joonyoung Shim @ 2015-01-13 2:51 UTC (permalink / raw)
To: u-boot
Hi,
On 01/13/2015 11:47 AM, Simon Glass wrote:
> Hi,
>
> On 12 January 2015 at 18:36, Joonyoung Shim <jy0922.shim@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@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.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:51 ` Joonyoung Shim
@ 2015-01-13 2:56 ` Simon Glass
2015-01-13 4:10 ` Jaehoon Chung
0 siblings, 1 reply; 11+ messages in thread
From: Simon Glass @ 2015-01-13 2:56 UTC (permalink / raw)
To: u-boot
Hi,
On 12 January 2015 at 18:51, Joonyoung Shim <jy0922.shim@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@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@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?
Regards,
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 2:56 ` Simon Glass
@ 2015-01-13 4:10 ` Jaehoon Chung
0 siblings, 0 replies; 11+ messages in thread
From: Jaehoon Chung @ 2015-01-13 4:10 UTC (permalink / raw)
To: u-boot
Hi,
On 01/13/2015 11:56 AM, Simon Glass wrote:
> Hi,
>
> On 12 January 2015 at 18:51, Joonyoung Shim <jy0922.shim@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@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@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/
Best Regards,
Jaehoon Chung
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] Question about compile warnings of exynos clock
@ 2015-01-13 5:40 Akshay Saraswat
2015-01-13 5:56 ` Joonyoung Shim
2015-01-13 5:58 ` Jaehoon Chung
0 siblings, 2 replies; 11+ messages in thread
From: Akshay Saraswat @ 2015-01-13 5:40 UTC (permalink / raw)
To: u-boot
Hi,
>Hi,
>
>
>On 01/13/2015 11:56 AM, Simon Glass wrote:
>> Hi,
>>
>> On 12 January 2015 at 18:51, Joonyoung Shim <jy0922.shim@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@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@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?
>
>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
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 5:40 Akshay Saraswat
@ 2015-01-13 5:56 ` Joonyoung Shim
2015-01-13 5:58 ` Jaehoon Chung
1 sibling, 0 replies; 11+ messages in thread
From: Joonyoung Shim @ 2015-01-13 5:56 UTC (permalink / raw)
To: u-boot
Hi Akshay,
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@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@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@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.
>
Then, do you have any plan to fix it and convert to use generic clock get api?
> 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?
>
I used toolchain of linaro
http://releases.linaro.org/14.09/components/toolchain/binaries/gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_linux.tar.xz
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 5:40 Akshay Saraswat
2015-01-13 5:56 ` Joonyoung Shim
@ 2015-01-13 5:58 ` Jaehoon Chung
2015-01-14 4:42 ` Simon Glass
1 sibling, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2015-01-13 5:58 UTC (permalink / raw)
To: u-boot
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@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@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@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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* [U-Boot] Question about compile warnings of exynos clock
2015-01-13 5:58 ` Jaehoon Chung
@ 2015-01-14 4:42 ` Simon Glass
0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2015-01-14 4:42 UTC (permalink / raw)
To: u-boot
Hi,
On 12 January 2015 at 21:58, Jaehoon Chung <jh80.chung@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@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@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@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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-01-14 4:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-13 2:24 [U-Boot] Question about compile warnings of exynos clock Joonyoung Shim
2015-01-13 2:27 ` Simon Glass
2015-01-13 2:36 ` Joonyoung Shim
2015-01-13 2:47 ` Simon Glass
2015-01-13 2:51 ` Joonyoung Shim
2015-01-13 2:56 ` Simon Glass
2015-01-13 4:10 ` Jaehoon Chung
-- strict thread matches above, loose matches on Subject: below --
2015-01-13 5:40 Akshay Saraswat
2015-01-13 5:56 ` Joonyoung Shim
2015-01-13 5:58 ` Jaehoon Chung
2015-01-14 4:42 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox