public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [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