On Mon, Apr 9, 2012 at 1:46 AM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote: >> -----Original Message----- >> From: Olof Johansson [mailto:olof@xxxxxxxxx] >> Sent: Monday, April 09, 2012 1:16 PM >> >> I'm not 100% sure if the fix for 'adjust_request' is correct, since >> it's uncertain what the original intent was. But it's so clearly an >> uninitialized pointer dereference that my resolution seems to make sense. >> >> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train': >> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in >> this function [-Wuninitialized] >> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here >> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function >> [-Wuninitialized] >> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here >> >> Signed-off-by: Olof Johansson <olof@xxxxxxxxx> >> --- >> drivers/video/exynos/exynos_dp_core.c | 23 +++++++++-------------- >> 1 file changed, 9 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c >> index 2a4481c..8973e18 100644 >> --- a/drivers/video/exynos/exynos_dp_core.c >> +++ b/drivers/video/exynos/exynos_dp_core.c >> @@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training( >> struct exynos_dp_device *dp, >> int lane) >> { >> - u32 reg; >> - >> switch (lane) { >> case 0: >> - reg = exynos_dp_get_lane0_link_training(dp); >> - break; >> + return exynos_dp_get_lane0_link_training(dp); >> case 1: >> - reg = exynos_dp_get_lane1_link_training(dp); >> - break; >> + return exynos_dp_get_lane1_link_training(dp); >> case 2: >> - reg = exynos_dp_get_lane2_link_training(dp); >> - break; >> + return exynos_dp_get_lane2_link_training(dp); >> case 3: >> - reg = exynos_dp_get_lane3_link_training(dp); >> - break; >> + return exynos_dp_get_lane3_link_training(dp); >> } > > I don't like multi return. In a small helper function like this there's nothing wrong with it. Larger functions? Sure. Adding a default in the switch didn't seem like an improvement to me. But I'll leave it up to you. >> >> - return reg; >> + WARN_ON(1); >> + return 0; >> } >> >> static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp) >> @@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp) >> 6, link_status); >> lane_count = dp->link_train.lane_count; >> >> + adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1 >> + - DPCD_ADDR_LANE0_1_STATUS); >> + > > It makes the problem. adjust_request will be different. > > OK, I understand what you want to do. > I will send the version 2 patch which is simpler. > > Thank you for sending the patch. Ok, thanks. As I said, I'm not sure what your intent with the second (else) code path was. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html