Hi Moritz,
2016-04-14 Moritz Kühner <kuehner.moritz at gmail.com>:
> if atomctrl_get_voltage_evv_on_sclk returns non zero (fail) in the expansion
> of the PP_ASSERT_WITH_CODE macro the continue will actually do nothing
> (The macro uses a do ... while(0) as scope, which eats the continue).
> Based on the code I don't think this was the intent.
> Unfortunately fixing this requires rewriting the control flow and
> removing the macros.
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 54
> +++++++++++++----------
> 1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> index 50afb02..9a5d10a 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> @@ -429,19 +429,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr)
> }
> }
> }
> - PP_ASSERT_WITH_CODE(0 ==
> atomctrl_get_voltage_evv_on_sclk
> + if (0 == atomctrl_get_voltage_evv_on_sclk
> (hwmgr, VOLTAGE_TYPE_VDDGFX,
> sclk,
> - virtual_voltage_id, &vddgfx),
> - "Error retrieving EVV voltage
> value!", continue);
> -
> - /* need to make sure vddgfx is less than 2v or
> else, it could burn the ASIC. */
> - PP_ASSERT_WITH_CODE((vddgfx < 2000 && vddgfx !=
> 0), "Invalid VDDGFX value!", return -1);
> -
> - /* the voltage should not be zero nor equal to
> leakage ID */
> - if (vddgfx != 0 && vddgfx !=
> virtual_voltage_id) {
> -
> data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx;
> -
> data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] =
> virtual_voltage_id;
> - data->vddcgfx_leakage.count++;
> + virtual_voltage_id, &vddgfx)) {
> + /* need to make sure vddgfx is less
> than 2v or else, it could burn the ASIC. */
> + PP_ASSERT_WITH_CODE((vddgfx < 2000 &&
> vddgfx != 0), "Invalid VDDGFX value!", return -1);
> +
> + /* the voltage should not be zero nor
> equal to leakage ID */
> + if (vddgfx != 0 && vddgfx !=
> virtual_voltage_id) {
> +
> data->vddcgfx_leakage.actual_voltage[data->vddcgfx_leakage.count] = vddgfx;
> +
> data->vddcgfx_leakage.leakage_id[data->vddcgfx_leakage.count] =
> virtual_voltage_id;
> + data->vddcgfx_leakage.count++;
> + }
> + }
> + else
> + {
> + printk("%s\n", "Error retrieving EVV
> voltage value!");
Use DRM_ERROR here. And no need to for %s on this format string. I think
you want something like this:
DRM_ERROR("Error retrieving EVV voltage value!\n");
> }
> }
> } else {
> @@ -449,19 +452,22 @@ int tonga_get_evv_voltage(struct pp_hwmgr *hwmgr)
> if (0 == tonga_get_sclk_for_voltage_evv(hwmgr,
> pptable_info->vddc_lookup_table,
> virtual_voltage_id, &sclk)) {
> - PP_ASSERT_WITH_CODE(0 ==
> atomctrl_get_voltage_evv_on_sclk
> + if (0 == atomctrl_get_voltage_evv_on_sclk
> (hwmgr, VOLTAGE_TYPE_VDDC, sclk,
> - virtual_voltage_id, &vddc),
> - "Error retrieving EVV voltage
> value!", continue);
> -
> - /* need to make sure vddc is less than 2v or
> else, it could burn the ASIC. */
> - PP_ASSERT_WITH_CODE(vddc < 2000, "Invalid VDDC
> value!", return -1);
> -
> - /* the voltage should not be zero nor equal to
> leakage ID */
> - if (vddc != 0 && vddc != virtual_voltage_id) {
> -
> data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc;
> -
> data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id;
> - data->vddc_leakage.count++;
> + virtual_voltage_id, &vddc)) {
> + /* need to make sure vddc is less than
> 2v or else, it could burn the ASIC. */
> + PP_ASSERT_WITH_CODE(vddc < 2000,
> "Invalid VDDC value!", return -1);
> +
> + /* the voltage should not be zero nor
> equal to leakage ID */
> + if (vddc != 0 && vddc !=
> virtual_voltage_id) {
> +
> data->vddc_leakage.actual_voltage[data->vddc_leakage.count] = vddc;
> +
> data->vddc_leakage.leakage_id[data->vddc_leakage.count] = virtual_voltage_id;
> + data->vddc_leakage.count++;
> + }
> + }
> + else
> + {
> + printk("%s\n", "Error retrieving EVV
> voltage value!");
same here.
Gustavo
