Hmm... well... I'm still somewhat confused. Your patch changed behavior now. Previously you were reading the current value and subtracting 6, now you're just overwriting the value. I thought you were doing that because you didn't know what the existing calibration value was (e.g. it might be different per board and initialized from fuses or something), so you could only adjust the existing one.

But if you are sure that setting it directly is okay then this approach is fine.

View Change

3 comments:

• File src/soc/mediatek/mt8183/mt6358.c:

  • Patch Set #14, Line 736: unsigned int vsim2_mv, unsigned int cali_mv

    If these voltages really just get added like that, does it make sense to have two separate parameters? Why not just have one millivolts parameter and then do something like

     assert(millivolts % 10 == 0);
     int cali_mv = millivolts % 100;
     switch (millivolts - cali_mv) {
       ...
     }

  • Patch Set #14, Line 762: return;

    This should die() or assert()

  • Patch Set #14, Line 768: if (cali_mv > 100)

    You should assert() that the value is in range instead

To view, visit change 32057. To unsubscribe, or for help writing mail filters, visit settings.