Re: [PATCH iwl-next v1] ice: add support for unmanaged dpll on E830 NIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
+}




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux