[bug report] soc: mediatek: SVS: add mt8192 SVS GPU driver

[Dan Carpenter]

Hello Roger Lu,

The patch 0bbb09b2af9d: "soc: mediatek: SVS: add mt8192 SVS GPU
driver" from May 16, 2022, leads to the following (unpublished) Smatch
static checker warning:

	drivers/soc/mediatek/mtk-svs.c:532 svs_adjust_pm_opp_volts()
	warn: loop overwrites return value 'ret'

drivers/soc/mediatek/mtk-svs.c
    487 static int svs_adjust_pm_opp_volts(struct svs_bank *svsb)
    488 {
    489         int ret = -EPERM, tzone_temp = 0;
                    ^^^^^^^^^^^^^
What is this -EPERM for?

    490         u32 i, svsb_volt, opp_volt, temp_voffset = 0, opp_start, opp_stop;
    491 
    492         mutex_lock(&svsb->lock);
    493 
    494         /*
    495          * 2-line bank updates its corresponding opp volts.
    496          * 1-line bank updates all opp volts.
    497          */
    498         if (svsb->type == SVSB_HIGH) {
    499                 opp_start = 0;
    500                 opp_stop = svsb->turn_pt;
    501         } else if (svsb->type == SVSB_LOW) {
    502                 opp_start = svsb->turn_pt;
    503                 opp_stop = svsb->opp_count;
    504         } else {
    505                 opp_start = 0;
    506                 opp_stop = svsb->opp_count;
    507         }
    508 
    509         /* Get thermal effect */
    510         if (svsb->phase == SVSB_PHASE_MON) {
    511                 ret = thermal_zone_get_temp(svsb->tzd, &tzone_temp);
    512                 if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND &&
    513                             svsb->temp < SVSB_TEMP_LOWER_BOUND)) {
    514                         dev_err(svsb->dev, "%s: %d (0x%x), run default volts\n",
    515                                 svsb->tzone_name, ret, svsb->temp);
    516                         svsb->phase = SVSB_PHASE_ERROR;

ret is set here but there is no goto unlock_mutex;

    517                 }
    518 
    519                 if (tzone_temp >= svsb->tzone_htemp)
    520                         temp_voffset += svsb->tzone_htemp_voffset;
    521                 else if (tzone_temp <= svsb->tzone_ltemp)
    522                         temp_voffset += svsb->tzone_ltemp_voffset;
    523 
    524                 /* 2-line bank update all opp volts when running mon mode */
    525                 if (svsb->type == SVSB_HIGH || svsb->type == SVSB_LOW) {
    526                         opp_start = 0;
    527                         opp_stop = svsb->opp_count;
    528                 }
    529         }
    530 
    531         /* vmin <= svsb_volt (opp_volt) <= default opp voltage */
--> 532         for (i = opp_start; i < opp_stop; i++) {

I guess the bug is that there was supposed to be an explicit check?

	if (opp_start == opp_stop) {
		ret = -EPERM;
		goto unlock_mutex;
	}

Otherwise, we are possibly returning the ret from the /* Get thermal
effect */ block.

    533                 switch (svsb->phase) {
    534                 case SVSB_PHASE_ERROR:
    535                         opp_volt = svsb->opp_dvolt[i];
    536                         break;
    537                 case SVSB_PHASE_INIT01:
    538                         /* do nothing */
    539                         goto unlock_mutex;
    540                 case SVSB_PHASE_INIT02:
    541                         svsb_volt = max(svsb->volt[i], svsb->vmin);
    542                         opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
    543                                                              svsb->volt_step,
    544                                                              svsb->volt_base);
    545                         break;
    546                 case SVSB_PHASE_MON:
    547                         svsb_volt = max(svsb->volt[i] + temp_voffset, svsb->vmin);
    548                         opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
    549                                                              svsb->volt_step,
    550                                                              svsb->volt_base);
    551                         break;
    552                 default:
    553                         dev_err(svsb->dev, "unknown phase: %u\n", svsb->phase);
    554                         ret = -EINVAL;
    555                         goto unlock_mutex;
    556                 }
    557 
    558                 opp_volt = min(opp_volt, svsb->opp_dvolt[i]);
    559                 ret = dev_pm_opp_adjust_voltage(svsb->opp_dev,
    560                                                 svsb->opp_dfreq[i],
    561                                                 opp_volt, opp_volt,
    562                                                 svsb->opp_dvolt[i]);
    563                 if (ret) {
    564                         dev_err(svsb->dev, "set %uuV fail: %d\n",
    565                                 opp_volt, ret);
    566                         goto unlock_mutex;
    567                 }
    568         }
    569 
    570 unlock_mutex:
    571         mutex_unlock(&svsb->lock);
    572 
    573         return ret;
    574 }

regards,
dan carpenter