Question about compile warnings of exynos clock

static unsigned long exynos5_get_periph_rate(int peripheral)

{
        struct clk_bit_info *bit_info = &clk_bit_info[peripheral];

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.

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 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.

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 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.

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.

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 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.

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 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?

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