>>- if (!std_info) >>+ if (!std_info->stdid) >> return -1; >> > This is a NACK. We shouldn't check for stdid since the function is supposed > to update std_info. So just remove > > if (!std_info) > return -1; I don't see how std_info could get updated. consider the loop in case std_info->stdid equals 0: for (index = 0; index < ARRAY_SIZE(ch_params); index++) { config = &ch_params[index]; (config is a local variable) if (config->stdid & std_info->stdid) { This fails for every index if std_info->stdid equals 0. memcpy(std_info, config, sizeof(*config)); break; } } So we always reach this with index == ARRAY_SIZE(ch_params) if (index == ARRAY_SIZE(ch_params)) return -1; So we could have returned -1 earlier. > I am okay with the below change. So please re-submit the patch with the > above change and my ACK. > > Thanks > > Murali > >>+ if (k == VPIF_DISPLAY_MAX_DEVICES) >>+ k = VPIF_DISPLAY_MAX_DEVICES - 1; actually I think this is still not right. shouldn't it be be k = VPIF_DISPLAY_MAX_DEVICES - 1; > are you using this driver in your project? No, I just found this in the code. Thanks, Roel -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html