Skip to contentSkip to conversation

Question about compile warnings of exynos clock

U-Boot Joonyoung Shim 3 participants 7 messages Jan 13, 2015

lore
Share & tools
lore
#0 lore raw

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]

...

Quoted 4 lines

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.

#1 lore raw

Hi,

Quoted 21 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 6 lines

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

#2 lore raw

Hi,

Quoted 28 lines · Simon Glass

Hi,

Quoted 25 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 7 lines
Quoted 2 lines

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.

#3 lore raw

Hi,

Quoted 34 lines · Joonyoung Shim

Hi,

Quoted 31 lines · Simon Glass

Hi,

Quoted 27 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 8 lines
Quoted 4 lines

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

#4 lore raw

Hi,

Quoted 46 lines · Simon Glass

Hi,

Quoted 34 lines · Joonyoung Shim

Hi,

Quoted 31 lines · Simon Glass

Hi,

Quoted 27 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 8 lines
Quoted 4 lines

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.

Quoted 9 lines

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.

#5 lore raw

Hi,

Quoted 53 lines · Joonyoung Shim

Hi,

Quoted 50 lines · Simon Glass

Hi,

Quoted 34 lines · Joonyoung Shim

Hi,

Quoted 31 lines · Simon Glass

Hi,

Quoted 27 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 8 lines
Quoted 4 lines

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?

Quoted 7 lines
Quoted 5 lines

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

#6 lore raw

Hi,

Quoted 58 lines · Simon Glass

Hi,

Quoted 55 lines · Joonyoung Shim

Hi,

Quoted 52 lines · Simon Glass

Hi,

Quoted 34 lines · Joonyoung Shim

Hi,

Quoted 31 lines · Simon Glass

Hi,

Quoted 27 lines · Joonyoung Shim

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]

...

Quoted 4 lines

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.

Quoted 8 lines
Quoted 4 lines

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);

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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

Quoted 8 lines

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot