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

Jiri Pirko
2023-07-20T14:09:25Z

Thu, Jul 20, 2023 at 11:19:01AM CEST, vadim.fedorenko@linux.dev wrote:
>From: Arkadiusz Kubalewski <arkadiusz.kubalewski@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! …