On Tue, 13 May 2025 22:40:00 +0000 Marek Pazdan wrote: > Common Management Interface Specification defines > Management Signaling Layer (MSL) control and status signals. This change > provides API for following signals status reading: > - signal allowing the host to request module reset (Reset) > - signal allowing the host to detect module presence (Presence) > - signal allowing the host to detect module interrupt (Int) > Additionally API allows for Reset signal assertion with > following constraints: > - reset cannot be asserted if firmware update is in progress > - if reset is asserted, firmware update cannot be started > - if reset is asserted, power mode cannot be get/set > In all above constraint cases -EBUSY error is returned. > > After reset, module will set all registers to default > values. Default value for Page0 byte 93 register is 0x00 what implies that > module power mode after reset depends on LPMode HW pin state. > If software power mode control is required, bit 0 of Page0 byte93 needs > to be enabled. > Module reset assertion implies failure of every module's related > SMBus transactions. Device driver developers should take this into > consideration if driver provides API for querying module's related data. > One example can be HWMON providing module temperature report. > In such case driver should monitor module status and in time of reset > assertion it should return HWMON report which informs that temperature > data is not available due to module's reset state. > The same applies to power mode set/get. Ethtool API has already > checking for module reset state but similar checking needs to be > implemented in the driver if it requests power mode for other > functionality. > Additionally module reset is link hitful operation. Link is brought down > when reset is asserted. If device driver doesn't provide functionality > for monitoring transceiver state, it needs to be implemented in parallel > to get/set_module_mgmt_signal API. When module reset gets deasserted, > transceiver process reinitialization. The end of reinitialization > process is signalled via Page 00h Byte 6 bit 0 "Initialization complete > flags". If there is no implementation for monitoring this bit in place, > it needs to be added to bring up the link after transceiver > initialization is complete. > > Signed-off-by: Marek Pazdan <[email protected]>
A few drive by comments, I leave the real review to Andrew. Instead of posting in-reply-to please add lore links to previous versions, eg. v2: https://lore.kernel.org/all/[email protected]/ under the --- separator. I almost missed this posting. When you post v3 please make sure to CC Ido and Danielle who implemented the FW flashing for modules. > diff --git a/Documentation/netlink/specs/ethtool.yaml > b/Documentation/netlink/specs/ethtool.yaml > index c650cd3dcb80..38eebbe18f55 100644 > --- a/Documentation/netlink/specs/ethtool.yaml > +++ b/Documentation/netlink/specs/ethtool.yaml > @@ -1528,6 +1528,24 @@ attribute-sets: > name: hwtstamp-flags > type: nest > nested-attributes: bitset > + - > + name: module-mgmt > + attr-cnt-name: __ethtool-a-module-mgmt-cnt > + attributes: > + - > + name: unspec > + type: unused > + value: 0 no need, just skip the unspec attr and YNL will number the first one from 1 keeping 0 as rejected type. > + - > + name: header > + type: nest > + nested-attributes: header > + - > + name: type > + type: u8 u32 will give us more flexibility later. And attr sizes in netlink are aligned to 4B so a u8 consumes 4 bytes anyway. > + - > + name: value > + type: u8 Do you think we'll never need to set / clear / change multiple bits at once? We could wrap the type / value into a nest and support repeating that nest (multi-attr: true) > +/** > + * enum ethtool_module_mgmt_signal_type - plug-in module discrete > + * status hardware signals for management as per CMIS spec. > + * @ETHTOOL_MODULE_MGMT_RESET: Signal allowing the host to request > + * a module reset. > + * @ETHTOOL_MODULE_MGMT_INT: Signal allowing the module to assert > + * an interrupt request to the host. > + * @ETHTOOL_MODULE_MGMT_PRESENT: Signal allowing the module to signal > + * its presence status to the host. Not sure what the use case would be for setting INT and PRESENT. So the combined API (driver facing) to treat RESET and read-only bits as equivalent may not be the best fit. Just a feeling tho. > + */ > +enum ethtool_module_mgmt_signal_type { > + ETHTOOL_MODULE_MGMT_RESET = 1, > + ETHTOOL_MODULE_MGMT_INT, > + ETHTOOL_MODULE_MGMT_PRESENT, Please define the enums in the YNL spec, see https://lore.kernel.org/all/[email protected]/
