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

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.

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

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.

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

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.

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

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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (3)
-
Jaehoon Chung
-
Joonyoung Shim
-
Simon Glass