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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZLk/9zwbBHgs+rlb@nanopsycho>
Date: Thu, 20 Jul 2023 16:08:55 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Jonathan Lemon <jonathan.lemon@...il.com>,
	Paolo Abeni <pabeni@...hat.com>,
	Milena Olech <milena.olech@...el.com>,
	Michal Michalik <michal.michalik@...el.com>,
	linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
	mschmidt@...hat.com, netdev@...r.kernel.org,
	linux-clk@...r.kernel.org, Bart Van Assche <bvanassche@....org>
Subject: Re: [PATCH 09/11] ice: implement dpll interface to control cgu

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


[...]