diff mbox series

[net-next,2/3] eth: fbnic: hwmon: Add support for reading temperature and voltage sensors

Message ID 20250114000705.2081288-3-sanman.p211993@gmail.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series eth: fbnic: Add hardware monitoring support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 15 this patch: 15
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 255 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Sanman Pradhan Jan. 14, 2025, 12:07 a.m. UTC
Add support for reading temperature and voltage sensor data from firmware
by implementing a new TSENE message type and response parsing. This adds
message handler infrastructure to transmit sensor read requests and parse
responses. The sensor data will be exposed through the driver's hwmon interface.

Signed-off-by: Sanman Pradhan <sanman.p211993@gmail.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c  | 89 ++++++++++++++++++++-
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h  | 15 ++++
 drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 72 +++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_mac.h |  7 ++
 4 files changed, 179 insertions(+), 4 deletions(-)

--
2.43.5

Comments

Andrew Lunn Jan. 14, 2025, 1:19 a.m. UTC | #1
> @@ -50,6 +50,10 @@ struct fbnic_fw_completion {
>  	struct kref ref_count;
>  	int result;
>  	union {
> +		struct {
> +			s32 millivolts;
> +			s32 millidegrees;
> +		} tsene;
>  	} u;
>  };

Why have a union which only has one member?

	Andrew
Jakub Kicinski Jan. 14, 2025, 4 a.m. UTC | #2
On Tue, 14 Jan 2025 02:19:08 +0100 Andrew Lunn wrote:
> > @@ -50,6 +50,10 @@ struct fbnic_fw_completion {
> >  	struct kref ref_count;
> >  	int result;
> >  	union {
> > +		struct {
> > +			s32 millivolts;
> > +			s32 millidegrees;
> > +		} tsene;
> >  	} u;
> >  };  
> 
> Why have a union which only has one member?

One member per command, the commit msg on patch 1 mentions:

  The data from the various response types will be added to 
  the "union u" by subsequent commits.

More commands are on the way. It's a coin toss whether it's better to
add the union later or have to re-indent already added structs.
Michal Swiatkowski Jan. 14, 2025, 8:31 a.m. UTC | #3
On Mon, Jan 13, 2025 at 04:07:04PM -0800, Sanman Pradhan wrote:
> Add support for reading temperature and voltage sensor data from firmware
> by implementing a new TSENE message type and response parsing. This adds
> message handler infrastructure to transmit sensor read requests and parse
> responses. The sensor data will be exposed through the driver's hwmon interface.
> 
> Signed-off-by: Sanman Pradhan <sanman.p211993@gmail.com>
> ---
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.c  | 89 ++++++++++++++++++++-
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.h  | 15 ++++
>  drivers/net/ethernet/meta/fbnic/fbnic_mac.c | 72 +++++++++++++++++
>  drivers/net/ethernet/meta/fbnic/fbnic_mac.h |  7 ++
>  4 files changed, 179 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> index 320615a122e4..bbc7c1c0c37e 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> @@ -228,9 +228,9 @@ static void fbnic_mbx_process_tx_msgs(struct fbnic_dev *fbd)
>  	tx_mbx->head = head;
>  }
> 

[...]

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> --
> 2.43.5
diff mbox series

Patch

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index 320615a122e4..bbc7c1c0c37e 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -228,9 +228,9 @@  static void fbnic_mbx_process_tx_msgs(struct fbnic_dev *fbd)
 	tx_mbx->head = head;
 }

-static __maybe_unused int fbnic_mbx_map_req_w_cmpl(struct fbnic_dev *fbd,
-						   struct fbnic_tlv_msg *msg,
-						   struct fbnic_fw_completion *cmpl_data)
+static int fbnic_mbx_map_req_w_cmpl(struct fbnic_dev *fbd,
+				    struct fbnic_tlv_msg *msg,
+				    struct fbnic_fw_completion *cmpl_data)
 {
 	unsigned long flags;
 	int err;
@@ -269,7 +269,7 @@  static void fbnic_fw_release_cmpl_data(struct kref *kref)
 	kfree(cmpl_data);
 }

-static __maybe_unused struct fbnic_fw_completion *
+static struct fbnic_fw_completion *
 fbnic_fw_get_cmpl_by_type(struct fbnic_dev *fbd, u32 msg_type)
 {
 	struct fbnic_fw_completion *cmpl_data = NULL;
@@ -708,6 +708,84 @@  void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd)
 		dev_warn(fbd->dev, "Failed to send heartbeat message\n");
 }

+/**
+ * fbnic_fw_xmit_tsene_read_msg - Create and transmit a sensor read request
+ * @fbd: FBNIC device structure
+ * @cmpl_data: Completion data structure to store sensor response
+ *
+ * Asks the firmware to provide an update with the latest sensor data.
+ * The response will contain temperature and voltage readings.
+ *
+ * Return: 0 on success, negative error value on failure
+ */
+int fbnic_fw_xmit_tsene_read_msg(struct fbnic_dev *fbd,
+				 struct fbnic_fw_completion *cmpl_data)
+{
+	struct fbnic_tlv_msg *msg;
+	int err;
+
+	if (!fbnic_fw_present(fbd))
+		return -ENODEV;
+
+	msg = fbnic_tlv_msg_alloc(FBNIC_TLV_MSG_ID_TSENE_READ_REQ);
+	if (!msg)
+		return -ENOMEM;
+
+	err = fbnic_mbx_map_req_w_cmpl(fbd, msg, cmpl_data);
+	if (err)
+		goto free_message;
+
+	return 0;
+
+free_message:
+	free_page((unsigned long)msg);
+	return err;
+}
+
+static const struct fbnic_tlv_index fbnic_tsene_read_resp_index[] = {
+	FBNIC_TLV_ATTR_S32(FBNIC_TSENE_THERM),
+	FBNIC_TLV_ATTR_S32(FBNIC_TSENE_VOLT),
+	FBNIC_TLV_ATTR_S32(FBNIC_TSENE_ERROR),
+	FBNIC_TLV_ATTR_LAST
+};
+
+static int fbnic_fw_parse_tsene_read_resp(void *opaque,
+					  struct fbnic_tlv_msg **results)
+{
+	struct fbnic_fw_completion *cmpl_data;
+	struct fbnic_dev *fbd = opaque;
+	int err = 0;
+
+	/* Verify we have a completion pointer to provide with data */
+	cmpl_data = fbnic_fw_get_cmpl_by_type(fbd,
+					      FBNIC_TLV_MSG_ID_TSENE_READ_RESP);
+	if (!cmpl_data)
+		return -EINVAL;
+
+	if (results[FBNIC_TSENE_ERROR]) {
+		err = fbnic_tlv_attr_get_unsigned(results[FBNIC_TSENE_ERROR]);
+		if (err)
+			goto exit_complete;
+	}
+
+	if (!results[FBNIC_TSENE_THERM] || !results[FBNIC_TSENE_VOLT]) {
+		err = -EINVAL;
+		goto exit_complete;
+	}
+
+	cmpl_data->u.tsene.millidegrees =
+		fbnic_tlv_attr_get_signed(results[FBNIC_TSENE_THERM]);
+	cmpl_data->u.tsene.millivolts =
+		fbnic_tlv_attr_get_signed(results[FBNIC_TSENE_VOLT]);
+
+exit_complete:
+	cmpl_data->result = err;
+	complete(&cmpl_data->done);
+	fbnic_fw_put_cmpl(cmpl_data);
+
+	return err;
+}
+
 static const struct fbnic_tlv_parser fbnic_fw_tlv_parser[] = {
 	FBNIC_TLV_PARSER(FW_CAP_RESP, fbnic_fw_cap_resp_index,
 			 fbnic_fw_parse_cap_resp),
@@ -715,6 +793,9 @@  static const struct fbnic_tlv_parser fbnic_fw_tlv_parser[] = {
 			 fbnic_fw_parse_ownership_resp),
 	FBNIC_TLV_PARSER(HEARTBEAT_RESP, fbnic_heartbeat_resp_index,
 			 fbnic_fw_parse_heartbeat_resp),
+	FBNIC_TLV_PARSER(TSENE_READ_RESP,
+			 fbnic_tsene_read_resp_index,
+			 fbnic_fw_parse_tsene_read_resp),
 	FBNIC_TLV_MSG_ERROR
 };

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
index ff304baade91..fe68333d51b1 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
@@ -50,6 +50,10 @@  struct fbnic_fw_completion {
 	struct kref ref_count;
 	int result;
 	union {
+		struct {
+			s32 millivolts;
+			s32 millidegrees;
+		} tsene;
 	} u;
 };

@@ -61,6 +65,8 @@  void fbnic_mbx_flush_tx(struct fbnic_dev *fbd);
 int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
 int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
 void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
+int fbnic_fw_xmit_tsene_read_msg(struct fbnic_dev *fbd,
+				 struct fbnic_fw_completion *cmpl_data);
 void fbnic_fw_init_cmpl(struct fbnic_fw_completion *cmpl_data,
 			u32 msg_type);
 void fbnic_fw_clear_compl(struct fbnic_dev *fbd);
@@ -89,6 +95,8 @@  enum {
 	FBNIC_TLV_MSG_ID_OWNERSHIP_RESP			= 0x13,
 	FBNIC_TLV_MSG_ID_HEARTBEAT_REQ			= 0x14,
 	FBNIC_TLV_MSG_ID_HEARTBEAT_RESP			= 0x15,
+	FBNIC_TLV_MSG_ID_TSENE_READ_REQ			= 0x3C,
+	FBNIC_TLV_MSG_ID_TSENE_READ_RESP		= 0x3D,
 };

 #define FBNIC_FW_CAP_RESP_VERSION_MAJOR		CSR_GENMASK(31, 24)
@@ -130,6 +138,13 @@  enum {
 	FBNIC_FW_LINK_FEC_BASER			= 3,
 };

+enum {
+	FBNIC_TSENE_THERM			= 0x0,
+	FBNIC_TSENE_VOLT			= 0x1,
+	FBNIC_TSENE_ERROR			= 0x2,
+	FBNIC_TSENE_MSG_MAX
+};
+
 enum {
 	FBNIC_FW_OWNERSHIP_FLAG			= 0x0,
 	FBNIC_FW_OWNERSHIP_MSG_MAX
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
index 7b654d0a6dac..14291401f463 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.c
@@ -686,6 +686,77 @@  fbnic_mac_get_eth_mac_stats(struct fbnic_dev *fbd, bool reset,
 			    MAC_STAT_TX_BROADCAST);
 }

+static int fbnic_mac_get_sensor_asic(struct fbnic_dev *fbd, int id,
+				     long *val)
+{
+	struct fbnic_fw_completion *fw_cmpl;
+	int err = 0, retries = 5;
+	s32 *sensor;
+
+	fw_cmpl = kzalloc(sizeof(*fw_cmpl), GFP_KERNEL);
+	if (!fw_cmpl)
+		return -ENOMEM;
+
+	/* Initialize completion and queue it for FW to process */
+	fbnic_fw_init_cmpl(fw_cmpl, FBNIC_TLV_MSG_ID_TSENE_READ_RESP);
+
+	switch (id) {
+	case FBNIC_SENSOR_TEMP:
+		sensor = &fw_cmpl->u.tsene.millidegrees;
+		break;
+	case FBNIC_SENSOR_VOLTAGE:
+		sensor = &fw_cmpl->u.tsene.millivolts;
+		break;
+	default:
+		err = -EINVAL;
+		goto exit_free;
+	}
+
+	err = fbnic_fw_xmit_tsene_read_msg(fbd, fw_cmpl);
+	if (err) {
+		dev_err(fbd->dev,
+			"Failed to transmit TSENE read msg, err %d\n",
+			err);
+		goto exit_free;
+	}
+
+	/* Allow 2 seconds for reply, resend and try up to 5 times */
+	while (!wait_for_completion_timeout(&fw_cmpl->done, 2 * HZ)) {
+		retries--;
+
+		if (retries == 0) {
+			dev_err(fbd->dev,
+				"Timed out waiting for TSENE read\n");
+			err = -ETIMEDOUT;
+			goto exit_cleanup;
+		}
+
+		err = fbnic_fw_xmit_tsene_read_msg(fbd, NULL);
+		if (err) {
+			dev_err(fbd->dev,
+				"Failed to transmit TSENE read msg, err %d\n",
+				err);
+			goto exit_cleanup;
+		}
+	}
+
+	/* Handle error returned by firmware */
+	if (fw_cmpl->result) {
+		err = fw_cmpl->result;
+		dev_err(fbd->dev, "%s: Firmware returned error %d\n",
+			__func__, err);
+		goto exit_cleanup;
+	}
+
+	*val = *sensor;
+exit_cleanup:
+	fbnic_fw_clear_compl(fbd);
+exit_free:
+	fbnic_fw_put_cmpl(fw_cmpl);
+
+	return err;
+}
+
 static const struct fbnic_mac fbnic_mac_asic = {
 	.init_regs = fbnic_mac_init_regs,
 	.pcs_enable = fbnic_pcs_enable_asic,
@@ -695,6 +766,7 @@  static const struct fbnic_mac fbnic_mac_asic = {
 	.get_eth_mac_stats = fbnic_mac_get_eth_mac_stats,
 	.link_down = fbnic_mac_link_down_asic,
 	.link_up = fbnic_mac_link_up_asic,
+	.get_sensor = fbnic_mac_get_sensor_asic,
 };

 /**
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
index 476239a9d381..05a591653e09 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
@@ -47,6 +47,11 @@  enum {
 #define FBNIC_LINK_MODE_PAM4	(FBNIC_LINK_50R1)
 #define FBNIC_LINK_MODE_MASK	(FBNIC_LINK_AUTO - 1)

+enum fbnic_sensor_id {
+	FBNIC_SENSOR_TEMP,		/* Temp in millidegrees Centigrade */
+	FBNIC_SENSOR_VOLTAGE,		/* Voltage in millivolts */
+};
+
 /* This structure defines the interface hooks for the MAC. The MAC hooks
  * will be configured as a const struct provided with a set of function
  * pointers.
@@ -83,6 +88,8 @@  struct fbnic_mac {

 	void (*link_down)(struct fbnic_dev *fbd);
 	void (*link_up)(struct fbnic_dev *fbd, bool tx_pause, bool rx_pause);
+
+	int (*get_sensor)(struct fbnic_dev *fbd, int id, long *val);
 };

 int fbnic_mac_init(struct fbnic_dev *fbd);