On 19/8/2025 16:30, Christoph Hellwig wrote:

On Mon, Aug 18, 2025 at 04:43:00PM +0800, Nick Chan wrote:

quoted

 };
 
+struct apple_nvme_hw {
+	bool has_lsq_nvmmu;
+	u32 max_queue_depth;
+	void (*submit_cmd)(struct apple_nvme_queue *q, struct nvme_command *cmd);

Please stick to 80 character lines for the NVMe code.

Also I don't think an indirect call here is a good idea.  This is right
in the command submission fast path.  A simple branch will be a lot
faster.

Ack for both of these points will check the other codes that got more
indented as well.

quoted

+
+	if (q->is_adminq)
+		memcpy(&q->sqes[tag], cmd, sizeof(*cmd));
+	else
+		memcpy((void *)q->sqes + (tag << APPLE_NVME_IOSQES), cmd, sizeof(*cmd));

This could use a helper and / or comment to make the calculation
more obvious.

This part of the code could be further simplified and after that
it is similar to nvme_sq_copy_cmd() in pci.c:

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index f3999b8ef7ab..ff4c2f87770c 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c

@@ -295,20 +295,17 @@ static void apple_nvme_submit_cmd_t8015(struct apple_nvme_queue *q,
 				  struct nvme_command *cmd)
 {
 	struct apple_nvme *anv = queue_to_apple_nvme(q);
-	u32 tag;
 
 	spin_lock_irq(&anv->lock);
 
-	tag = q->sq_tail;
-	q->sq_tail += 1;
-
-	if (q->sq_tail == anv->hw->max_queue_depth)
-		q->sq_tail = 0;
-
 	if (q->is_adminq)
-		memcpy(&q->sqes[tag], cmd, sizeof(*cmd));
+		memcpy(&q->sqes[q->sq_tail], cmd, sizeof(*cmd));
 	else
-		memcpy((void *)q->sqes + (tag << APPLE_NVME_IOSQES), cmd, sizeof(*cmd));
+		memcpy((void *)q->sqes + (q->sq_tail << APPLE_NVME_IOSQES),
+			cmd, sizeof(*cmd));
+
+	if (++q->sq_tail == anv->hw->max_queue_depth)
+		q->sq_tail = 0;
 
 	writel(q->sq_tail, q->sq_db);
 	spin_unlock_irq(&anv->lock);

And this seems obvious enough on its own, and should not need comments.

quoted

+	anv->hw = (const struct apple_nvme_hw *)of_device_get_match_data(&pdev->dev);

Do we even need this cast?

Don't think so, will remove.

Best regards,
Nick Chan