On 8/26/25 17:31, Arkadiusz Kubalewski wrote:
Hardware variants of E830 may support an unmanaged DPLL where the configuration is hardcoded within the hardware and firmware, meaning users cannot modify settings. However, users are able to check the DPLL lock status and obtain configuration information through the Linux DPLL subsystem. Availability of 'loss of lock' health status code determines if such support is available, if true, register single dpll device with 1 input and 1 output and provide hardcoded/read only properties of a pin and dpll device. User is only allowed to check dpll device status and receive notifications on dpll lock status change. When present, the DPLL device locks to an external signal provided through the PCIe/OCP pin. The expected input signal is 1PPS (1 Pulse Per Second) embedded on a 10MHz reference clock. The DPLL produces output: - for MAC (Media Access Control) & PHY (Physical Layer) clocks, - 1PPS for synchronization of onboard PHC (Precision Hardware Clock) timer. Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@xxxxxxxxx> --- .../device_drivers/ethernet/intel/ice.rst | 83 +++++ .../net/ethernet/intel/ice/ice_adminq_cmd.h | 4 + drivers/net/ethernet/intel/ice/ice_common.c | 110 +++++++ drivers/net/ethernet/intel/ice/ice_common.h | 7 + drivers/net/ethernet/intel/ice/ice_dpll.c | 306 ++++++++++++++++-- drivers/net/ethernet/intel/ice/ice_dpll.h | 11 + drivers/net/ethernet/intel/ice/ice_main.c | 14 +- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 46 +++ drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 1 + 9 files changed, 560 insertions(+), 22 deletions(-)
+int ice_is_health_status_code_supported(struct ice_hw *hw, u16 code, + bool *supported) +{ + struct ice_aqc_health_status_elem *buff __free(kfree) = NULL;
observation: netdev maintainers don't like __free
+ const int BUFF_SIZE = ICE_AQC_HEALTH_STATUS_CODE_NUM; + int ret; + + *supported = false;
it's best to don't change output pointers on error
+ + buff = kcalloc(BUFF_SIZE, sizeof(*buff), GFP_KERNEL); + if (!buff) + return -ENOMEM; + ret = ice_aq_get_health_status_supported(hw, buff, BUFF_SIZE); + if (ret) + return ret; + for (int i = 0; i < BUFF_SIZE && buff[i].health_status_code; i++) + if (le16_to_cpu(buff[i].health_status_code) == code) { + *supported = true;
so, this line should be enough
+ break; + } + + return 0; +} + +/** + * ice_get_last_health_status_code - get last health status for given code + * @hw: pointer to the hardware structure + * @out: pointer to the health status struct to be filled + * @code: health status code to check + * + * Return: 0 on success, negative error code otherwise + */ +int ice_get_last_health_status_code(struct ice_hw *hw, + struct ice_aqc_health_status_elem *out, + u16 code) +{ + struct ice_aqc_health_status_elem *buff __free(kfree) = NULL; + const int BUFF_SIZE = ICE_AQC_HEALTH_STATUS_CODE_NUM; + int last_status = -1, ret;
nit: variables with initial value set should be on the end of given declaration line (here @ret should be first)
+ + buff = kcalloc(BUFF_SIZE, sizeof(*buff), GFP_KERNEL); + if (!buff) + return -ENOMEM; + ret = ice_aq_get_health_status(hw, buff, BUFF_SIZE); + if (ret) + return ret; + for (int i = 0; i < BUFF_SIZE && buff[i].health_status_code; i++) + if (le16_to_cpu(buff[i].health_status_code) == code) + last_status = i; + + if (last_status >= 0 && last_status < BUFF_SIZE)
second part of the contidion is always true
+ memcpy(out, &buff[last_status], sizeof(*out)); + else + memset(out, 0, sizeof(*out));
weird, but let it be
+ + return ret; +}