[PATCH 09/11] ice: implement dpll interface to control cgu

[Jiri Pirko]

Thu, Jul 20, 2023 at 11:19:01AM CEST, vadim.fedorenko at linux.dev wrote:
>From: Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>

[...]


>+/**
>+ * ice_dpll_pin_enable - enable a pin on dplls
>+ * @hw: board private hw structure
>+ * @pin: pointer to a pin
>+ * @pin_type: type of pin being enabled
>+ * @extack: error reporting
>+ *
>+ * Enable a pin on both dplls. Store current state in pin->flags.
>+ *
>+ * Context: Called under pf->dplls.lock
>+ * Return:
>+ * * 0 - OK
>+ * * negative - error
>+ */
>+static int
>+ice_dpll_pin_enable(struct ice_hw *hw, struct ice_dpll_pin *pin,
>+		    enum ice_dpll_pin_type pin_type,
>+		    struct netlink_ext_ack *extack)
>+{
>+	u8 flags = 0;
>+	int ret;
>+



I don't follow. Howcome you don't check if the mode is freerun here or
not? Is it valid to enable a pin when freerun mode? What happens?

Also, I am probably slow, but I still don't see anywhere in this
patchset any description about why we need the freerun mode. What is
diffrerent between:
1) freerun mode
2) automatic mode & all pins disabled?

Isn't the behaviour of 1) and 2) exactly the same? If no, why? This
needs to be documented, please.



Another question, I asked the last time as well, but was not heard:
Consider example where you have 2 netdevices, eth0 and eth1, each
connected with a single DPLL pin:
eth0 - DPLL pin 10 (DPLL device id 2)
eth1 - DPLL pin 11 (DPLL device id 2)

You have a SyncE daemon running on top eth0 and eth1.

Could you please describe following 2 flows?

1) SyncE daemon selects eth0 as a source of clock
2) SyncE daemon selects eth1 as a source of clock


For mlx5 it goes like:

DPLL device mode is MANUAL.
1)
 SynceE daemon uses RTNetlink to obtain DPLL pin number of eth0
    -> pin_id: 10
 SenceE daemon will use PIN_GET with pin_id 10 to get DPLL device id
    -> device_id: 2
 SynceE daemon does PIN_SET cmd on pin_id 10, device_id 2 -> state = CONNECTED

2)
 SynceE daemon uses RTNetlink to obtain DPLL pin number of eth1
    -> pin_id: 11
 SenceE daemon will use PIN_GET with pin_id 11 to get DPLL device id
    -> device_id: 2
 SynceE daemon does PIN_SET cmd on pin_id 10, device_id 2 -> state = CONNECTED
 (that will in HW disconnect previously connected pin 10, there will be
  notification of pin_id 10, device_id -> state DISCONNECT)


Thanks!


[...]