diff mbox series

[RFC,net-next,4/7] net/ism: Add kernel-doc comments for ism functions

Message ID 20250115195527.2094320-5-wintera@linux.ibm.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Provide an ism layer | 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: 2 this patch: 2
netdev/build_tools success Errors and warnings before: 0 (+1) this patch: 0 (+1)
netdev/cc_maintainers success CCed 2 of 2 maintainers
netdev/build_clang success Errors and warnings before: 22 this patch: 22
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: 2 this patch: 2
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 176 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

Alexandra Winter Jan. 15, 2025, 7:55 p.m. UTC
Note that in this RFC this patch is not complete, future versions
of this patch need to contain comments for all ism_ops.
Especially signal_event() and handle_event() need a good generic
description.

Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
---
 include/linux/ism.h | 115 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 105 insertions(+), 10 deletions(-)

Comments

Halil Pasic Jan. 15, 2025, 10:06 p.m. UTC | #1
On Wed, 15 Jan 2025 20:55:24 +0100
Alexandra Winter <wintera@linux.ibm.com> wrote:

> Note that in this RFC this patch is not complete, future versions
> of this patch need to contain comments for all ism_ops.
> Especially signal_event() and handle_event() need a good generic
> description.

Such notes don't belong to the commit message, but to the inline cover
letter IMHO.

> 
> Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
> ---
diff mbox series

Patch

diff --git a/include/linux/ism.h b/include/linux/ism.h
index 50975847248f..bc165d077071 100644
--- a/include/linux/ism.h
+++ b/include/linux/ism.h
@@ -13,11 +13,26 @@ 
 #include <linux/workqueue.h>
 #include <linux/uuid.h>
 
-/* The remote peer rgid can use dmb_tok to write into this buffer. */
+/*
+ * DMB - Direct Memory Buffer
+ * ==========================
+ * An ism client provides an DMB as input buffer for a local receiving
+ * ism device for exactly one (remote) sending ism device. Only this
+ * sending device can send data into this DMB using move_data(). Sender
+ * and receiver can be the same device.
+ * TODO: Alignment and length rules (CPU and DMA). Device specific?
+ */
 struct ism_dmb {
+	/* dmb_tok - Token for this dmb
+	 * Used by remote sender to address this dmb.
+	 * Provided by ism fabric in register_dmb().
+	 * Unique per ism fabric.
+	 */
 	u64 dmb_tok;
+	/* rgid - GID of designated remote sending device */
 	u64 rgid;
 	u32 dmb_len;
+	/* sba_idx - Index of this DMB on this receiving device */
 	u32 sba_idx;
 	u32 vlan_valid;
 	u32 vlan_id;
@@ -25,6 +40,8 @@  struct ism_dmb {
 	dma_addr_t dma_addr;
 };
 
+/* ISM event structure (currently device type specific) */
+// TODO: Define and describe generic event properties
 struct ism_event {
 	u32 type;
 	u32 code;
@@ -33,38 +50,89 @@  struct ism_event {
 	u64 info;
 };
 
+//TODO: use enum typedef
 #define ISM_EVENT_DMB	0
 #define ISM_EVENT_GID	1
 #define ISM_EVENT_SWR	2
 
 struct ism_dev;
 
+/*
+ * ISM clients
+ * ===========
+ * All ism clients have access to all ism devices
+ * and must provide the following functions to be called by
+ * ism device drivers:
+ */
 struct ism_client {
+	/* client name for logging and debugging purposes */
 	const char *name;
+	/**
+	 *  add() - add an ism device
+	 *  @dev: device that was added
+	 *
+	 * Will be called during ism_register_client() for all existing
+	 * ism devices and whenever a new ism device is registered.
+	 * *dev is valid until ism_client->remove() is called.
+	 */
 	void (*add)(struct ism_dev *dev);
+	/**
+	 * remove() - remove an ism device
+	 * @dev: device to be removed
+	 *
+	 * Will be called whenever an ism device is unregistered.
+	 * Before this call the device is already inactive: It will
+	 * no longer call client handlers.
+	 * The client must not access *dev after this call.
+	 */
 	void (*remove)(struct ism_dev *dev);
+	/**
+	 * handle_event() - Handle control information sent by device
+	 * @dev: device reporting the event
+	 * @event: ism event structure
+	 */
 	void (*handle_event)(struct ism_dev *dev, struct ism_event *event);
-	/* Parameter dmbemask contains a bit vector with updated DMBEs, if sent
-	 * via ism_move_data(). Callback function must handle all active bits
-	 * indicated by dmbemask.
+	/**
+	 * handle_irq() - Handle signalling of a DMB
+	 * @dev: device owns the dmb
+	 * @bit: sba_idx=idx of the ism_dmb that got signalled
+	 *	TODO: Pass a priv pointer to ism_dmb instead of 'bit'(?)
+	 * @dmbemask: ism signalling mask of the dmb
+	 *
+	 * Handle signalling of a dmb that was registered by this client
+	 * for this device.
+	 * The ism device can coalesce multiple signalling triggers into a
+	 * single call of handle_irq(). dmbemask can be used to indicate
+	 * different kinds of triggers.
 	 */
 	void (*handle_irq)(struct ism_dev *dev, unsigned int bit, u16 dmbemask);
-	/* Private area - don't touch! */
+	/* client index - provided by ism layer */
 	u8 id;
 };
 
 int ism_register_client(struct ism_client *client);
 int  ism_unregister_client(struct ism_client *client);
 
+//TODO: Pair descriptions with functions
+/*
+ * ISM devices
+ * ===========
+ */
 /* Mandatory operations for all ism devices:
  * int (*query_remote_gid)(struct ism_dev *dev, uuid_t *rgid,
  *	                   u32 vid_valid, u32 vid);
  *	Query whether remote GID rgid is reachable via this device and this
  *	vlan id. Vlan id is only checked if vid_valid != 0.
+ *	Returns 0 if remote gid is reachable.
  *
  * int (*register_dmb)(struct ism_dev *dev, struct ism_dmb *dmb,
  *			    void *client);
- *	Register an ism_dmb buffer for this device and this client.
+ *	Allocate and register an ism_dmb buffer for this device and this client.
+ *	The following fields of ism_dmb must be valid:
+ *	rgid, dmb_len, vlan_*; Optionally:requested sba_idx (non-zero)
+ *	Upon return the following fields will be valid: dmb_tok, sba_idx
+ *		cpu_addr, dma_addr (if applicable)
+ *	Returns zero on success
  *
  * int (*unregister_dmb)(struct ism_dev *dev, struct ism_dmb *dmb);
  *	Unregister an ism_dmb buffer
@@ -81,10 +149,15 @@  int  ism_unregister_client(struct ism_client *client);
  * u16 (*get_chid)(struct ism_dev *dev);
  *	Returns ism fabric identifier (channel id) of this device.
  *	Only devices on the same ism fabric can communicate.
- *	chid is unique per HW system, except for 0xFFFF, which denotes
- *	an ism_loopback device that can only communicate with itself.
- *	Use chid for fast negative checks, but only query_remote_gid()
- *	can give a reliable positive answer.
+ *	chid is unique per HW system. Use chid for fast negative checks,
+ *	but only query_remote_gid() can give a reliable positive answer:
+ *	Different chid: ism is not possible
+ *	Same chid: ism traffic may be possible or not
+ *		   (e.g. different HW systems)
+ *	EXCEPTION: A value of 0xFFFF denotes an ism_loopback device
+ *		that can only communicate with itself. Use GID or
+ *		query_remote_gid()to determine whether sender and
+ *		receiver use the same ism_loopback device.
  *
  * struct device* (*get_dev)(struct ism_dev *dev);
  *
@@ -109,6 +182,28 @@  struct ism_ops {
 	int (*register_dmb)(struct ism_dev *dev, struct ism_dmb *dmb,
 			    struct ism_client *client);
 	int (*unregister_dmb)(struct ism_dev *dev, struct ism_dmb *dmb);
+	/**
+	 * move_data() - write into a remote dmb
+	 * @dev: Local sending ism device
+	 * @dmb_tok: Token of the remote dmb
+	 * @idx: signalling index
+	 * @sf: signalling flag;
+	 *      if true, idx will be turned on at target ism interrupt mask
+	 *      and target device will be signalled, if required.
+	 * @offset: offset within target dmb
+	 * @data: pointer to data to be sent
+	 * @size: length of data to be sent
+	 *
+	 * Use dev to write data of size at offset into a remote dmb
+	 * identified by dmb_tok. Data is moved synchronously, *data can
+	 * be freed when this function returns.
+	 *
+	 * If signalling flag (sf) is true, bit number idx bit will be
+	 * turned on in the ism signalling mask, that belongs to the
+	 * target dmb, and handle_irq() of the ism client that owns this
+	 * dmb will be called, if required. The target device may chose to
+	 * coalesce multiple signalling triggers.
+	 */
 	int (*move_data)(struct ism_dev *dev, u64 dmb_tok, unsigned int idx,
 			 bool sf, unsigned int offset, void *data,
 			 unsigned int size);