iio: adc: adrv902x: add init cal and status attrs - #3526
Conversation
Expose the remaining init calibrations and the calibration status readbacks through sysfs so a run can be configured and inspected from userspace without a debugger. New writable init-cal enables: calibrate_orx_qec_en ADI_ADRV9025_ORX_QEC_INIT calibrate_orx_lo_delay_en ADI_ADRV9025_ORX_LO_DELAY calibrate_adc_en ADI_ADRV9025_ADC_TUNER calibrate_orx_adc_en ADI_ADRV9025_ORX_TIA New read-only status attributes: init_cals_complete_check adi_adrv9025_InitCalsCheckCompleteGet() init_cals_all_status adi_adrv9025_InitCalsDetailedStatusGet() tracking_cals_all_status adi_adrv9025_TrackingCalAllStateGet() All seven are registered for both adrv9026 and adrv9029. Signed-off-by: Georgian Raul <Raul.Georgian@analog.com>
d8550db to
3656416
Compare
stefpopa
left a comment
There was a problem hiding this comment.
A few minor comments from my side.
| break; | ||
|
|
||
| case ADRV9025_INIT_CALS_COMPLETE_CHECK: | ||
| uint8_t status; |
There was a problem hiding this comment.
Variables declared mid-block violate kernel style. Move all declarations to the top of the switch case
|
|
||
| case ADRV9025_INIT_CALS_COMPLETE_CHECK: | ||
| uint8_t status; | ||
| uint8_t ARMflag; |
|
|
||
| break; | ||
| case ADRV9025_INIT_STATUS_ALL: | ||
| adi_adrv9025_InitCalStatus_t initStatus; |
There was a problem hiding this comment.
same comment, move declaration to the top of the switch case.
|
|
||
| static IIO_DEVICE_ATTR(tracking_cals_all_status, 0644, adrv9025_phy_show, | ||
| NULL, ADRV9025_TRACKING_STATUS_ALL); | ||
|
|
There was a problem hiding this comment.
This looks like wrong permissions. init_cals_all_status and tracking_cals_all_status are declared with mode 0644 but pass NULL for the store function. Probably it should be 0444
| case ADRV9025_INIT_CALS_COMPLETE_CHECK: | ||
| uint8_t status; | ||
| uint8_t ARMflag; | ||
| ret = adi_adrv9025_InitCalsCheckCompleteGet(phy->madDevice, &status, &ARMflag); |
There was a problem hiding this comment.
The naming of the ARMflag is inconsistent, as it uses CamelCase. The kernel preferred style is arm_flag.
| break; | ||
|
|
||
| case ADRV9025_TRACKING_STATUS_ALL: | ||
| adi_adrv9025_TrackingCalState_t st; |
There was a problem hiding this comment.
another mid-block declaration. Move declaration to the top or wrap each case body in braces to create a new scope.
|
|
||
| ret = adi_adrv9025_TrackingCalAllStateGet(phy->madDevice, &st); | ||
| if (ret) | ||
| adrv9025_dev_err(phy); |
There was a problem hiding this comment.
Hmm... Should you return ret or return adrv9025_dev_err(phy)?
if (ret) {
adrv9025_dev_err(phy);
return ret;
}
or
if (ret)
return adrv9025_dev_err(phy);
please check this everywhere in the code.
This PR implements the initial calibration IIO attributes and the
status functions for both initial and tracking calibrations.
adi_adrv9025_InitCalsDetailedStatusGet() gives the error code, how
many microseconds the last calibration took, what ran last time,
and what calibrations ran since power up. The last two are
per-channel bitmasks, decoded against
adi_adrv9025_InitCalibrations_e, which can be found in
adi_adrv9025_cals_types.h.
adi_adrv9025_TrackingCalAllStateGet() gives a 32-bit mask with one
bit per calibration instance, flagging which ones are in the error
state, plus the state of each of the 32 tracking calibrations
(8 types across 4 channels).
adi_adrv9025_InitCalsCheckCompleteGet() can be used to verify if
the init calibrations are completed. It returns a running flag,
which is 0 when the ARM has completed the calibrations or never
ran them and 1 while they are currently running, and separately a
3-bit ARM error code.
When the calibration is successful, the error code should always
be 0.
PR Type
PR Checklist