[PATCH v2 0/9] Add support ANS2 NVMe on Apple A11

[PATCH v2 0/9] Add support ANS2 NVMe on Apple A11

[Nick Chan]

Apple A11 SoC comes with an NVMe controller that is similar to the
NVMe controller in M1, but less customized. Notably, it does not
have NVMMU or Linear SQ.

The controller is wired to an older variant of the SART address filter,
and uses an older variant of ASC mailbox, so add support for them too.

This variant of mailbox is also present on Apple T2 SoC.

This series has everything required to support NVMe on Apple A11 SoC.
However, due to RTKit protocol version support limitation, it will
only work when booted with at least iOS 14 iBoot.

Tested on A11 as well as M1 Pro (to make sure nothing broke).

Patch 1-2 adds support for the older mailbox
Patch 3-5 adds support for the older SART
Patch 6-7 adds support for the NVMe controller found in Apple A11 SoC
Patch 8-9 adds the required device tree nodes.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
Changes in v2:
- Remove bogus command_id decrement followed by increment in apple_nvme_handle_cqe()
- Split apple_nvme_submit_cmd() into two functions for t8015 and t8103 since these
  two code paths were not sharing much code.
- Link to v1: https://lore.kernel.org/r/20250811-t8015-nvme-v1-0-ef9c200e74a7@gmail.com

---
Nick Chan (9):
      dt-bindings: mailbox: apple,mailbox: Add ASC mailboxes on Apple A11 and T2
      soc: apple: mailbox: Add Apple A11 and T2 mailbox support
      dt-bindings: iommu: apple,sart: Add Apple A11
      soc: apple: sart: Make allow flags SART version dependent
      soc: apple: sart: Add SARTv0 support
      dt-bindings: nvme: apple,nvme-ans: Add Apple A11
      nvme: apple: Add Apple A11 support
      arm64: dts: apple: t8015: Fix PCIE power domains dependencies
      arm64: dts: apple: t8015: Add NVMe nodes

 .../devicetree/bindings/iommu/apple,sart.yaml      |   1 +
 .../devicetree/bindings/mailbox/apple,mailbox.yaml |   7 +
 .../devicetree/bindings/nvme/apple,nvme-ans.yaml   |  15 +-
 arch/arm64/boot/dts/apple/t8015-pmgr.dtsi          |   1 +
 arch/arm64/boot/dts/apple/t8015.dtsi               |  34 ++++
 drivers/nvme/host/apple.c                          | 198 +++++++++++++++------
 drivers/soc/apple/mailbox.c                        |  19 ++
 drivers/soc/apple/sart.c                           |  60 ++++++-
 8 files changed, 267 insertions(+), 68 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250808-t8015-nvme-73a4fcda25dd

Best regards,
-- 
Nick Chan <towinchenmi@gmail.com>

[PATCH v2 1/9] dt-bindings: mailbox: apple,mailbox: Add ASC mailboxes on Apple A11 and T2

[Nick Chan]

Add bindings for ASC mailboxes as found on Apple A11 and T2 SoCs. These
mailboxes are used for coprocessors including Secure Enclave Processor
(SEP), the NVMe coprocessor and the system management controller.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
index 474c1a0f99f34777e1bed7fc0a34f89320a93b7c..bdf58f03b84833ecd93a34b91f6262d1706a002d 100644
--- a/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
@@ -33,6 +33,13 @@ properties:
               - apple,t6000-asc-mailbox
           - const: apple,asc-mailbox-v4
 
+      - description:
+          An older ASC mailbox interface found on T2 and A11 that is also
+          used for the NVMe coprocessor and the system management
+          controller.
+        items:
+          - const: apple,t8015-asc-mailbox
+
       - description:
           M3 mailboxes are an older variant with a slightly different MMIO
           interface still found on the M1. It is used for the Thunderbolt

-- 
2.50.1

[PATCH v2 2/9] soc: apple: mailbox: Add Apple A11 and T2 mailbox support

[Nick Chan]

Add ASC mailbox support for Apple A11 and T2 SoCs, which is used for
coprocessors in the system.

Reviewed-by: Sven Peter <sven@kernel.org>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/soc/apple/mailbox.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/soc/apple/mailbox.c b/drivers/soc/apple/mailbox.c
index 49a0955e82d6cf5eef83e5f63ba8d31194c65324..8f29108dc69ac964236121d439e86c923a441265 100644
--- a/drivers/soc/apple/mailbox.c
+++ b/drivers/soc/apple/mailbox.c
@@ -47,6 +47,9 @@
 #define APPLE_ASC_MBOX_I2A_RECV0 0x830
 #define APPLE_ASC_MBOX_I2A_RECV1 0x838
 
+#define APPLE_T8015_MBOX_A2I_CONTROL	0x108
+#define APPLE_T8015_MBOX_I2A_CONTROL	0x10c
+
 #define APPLE_M3_MBOX_CONTROL_FULL BIT(16)
 #define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
 
@@ -382,6 +385,21 @@ static int apple_mbox_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct apple_mbox_hw apple_mbox_t8015_hw = {
+	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
+	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
+
+	.a2i_control = APPLE_T8015_MBOX_A2I_CONTROL,
+	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
+	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
+
+	.i2a_control = APPLE_T8015_MBOX_I2A_CONTROL,
+	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
+	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
+
+	.has_irq_controls = false,
+};
+
 static const struct apple_mbox_hw apple_mbox_asc_hw = {
 	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
 	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
@@ -418,6 +436,7 @@ static const struct apple_mbox_hw apple_mbox_m3_hw = {
 
 static const struct of_device_id apple_mbox_of_match[] = {
 	{ .compatible = "apple,asc-mailbox-v4", .data = &apple_mbox_asc_hw },
+	{ .compatible = "apple,t8015-asc-mailbox", .data = &apple_mbox_t8015_hw },
 	{ .compatible = "apple,m3-mailbox-v2", .data = &apple_mbox_m3_hw },
 	{}
 };

-- 
2.50.1

[PATCH v2 3/9] dt-bindings: iommu: apple,sart: Add Apple A11

[Nick Chan]

Add bindings for SARTv0 as found on Apple A11 SoC.

Reviewed-by: Sven Peter <sven@kernel.org>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 Documentation/devicetree/bindings/iommu/apple,sart.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iommu/apple,sart.yaml b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
index e87c1520fea60a2de549aa8a469fcded52e3b6e3..95d55ec688779bd52b8aef316a4c1ba7b7449574 100644
--- a/Documentation/devicetree/bindings/iommu/apple,sart.yaml
+++ b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
@@ -29,6 +29,7 @@ description:
 properties:
   compatible:
     oneOf:
+      - const: apple,t8015-sart
       - items:
           - const: apple,t8112-sart
           - const: apple,t6000-sart

-- 
2.50.1

[PATCH v2 4/9] soc: apple: sart: Make allow flags SART version dependent

[Nick Chan]

SART versions that uses different allow flags will be added.

Reviewed-by: Sven Peter <sven@kernel.org>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/soc/apple/sart.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/apple/sart.c b/drivers/soc/apple/sart.c
index afa11173689979f100a63221af03214f4a928a7c..318f6e518a54f6edb8b97e320519df8f77c7f143 100644
--- a/drivers/soc/apple/sart.c
+++ b/drivers/soc/apple/sart.c
@@ -25,9 +25,6 @@
 
 #define APPLE_SART_MAX_ENTRIES 16
 
-/* This is probably a bitfield but the exact meaning of each bit is unknown. */
-#define APPLE_SART_FLAGS_ALLOW 0xff
-
 /* SARTv2 registers */
 #define APPLE_SART2_CONFIG(idx)	      (0x00 + 4 * (idx))
 #define APPLE_SART2_CONFIG_FLAGS      GENMASK(31, 24)
@@ -38,6 +35,8 @@
 #define APPLE_SART2_PADDR(idx)	(0x40 + 4 * (idx))
 #define APPLE_SART2_PADDR_SHIFT 12
 
+#define APPLE_SART2_FLAGS_ALLOW 0xff
+
 /* SARTv3 registers */
 #define APPLE_SART3_CONFIG(idx) (0x00 + 4 * (idx))
 
@@ -48,11 +47,15 @@
 #define APPLE_SART3_SIZE_SHIFT 12
 #define APPLE_SART3_SIZE_MAX   GENMASK(29, 0)
 
+#define APPLE_SART3_FLAGS_ALLOW 0xff
+
 struct apple_sart_ops {
 	void (*get_entry)(struct apple_sart *sart, int index, u8 *flags,
 			  phys_addr_t *paddr, size_t *size);
 	void (*set_entry)(struct apple_sart *sart, int index, u8 flags,
 			  phys_addr_t paddr_shifted, size_t size_shifted);
+	/* This is probably a bitfield but the exact meaning of each bit is unknown. */
+	unsigned int flags_allow;
 	unsigned int size_shift;
 	unsigned int paddr_shift;
 	size_t size_max;
@@ -95,6 +98,7 @@ static void sart2_set_entry(struct apple_sart *sart, int index, u8 flags,
 static struct apple_sart_ops sart_ops_v2 = {
 	.get_entry = sart2_get_entry,
 	.set_entry = sart2_set_entry,
+	.flags_allow = APPLE_SART2_FLAGS_ALLOW,
 	.size_shift = APPLE_SART2_CONFIG_SIZE_SHIFT,
 	.paddr_shift = APPLE_SART2_PADDR_SHIFT,
 	.size_max = APPLE_SART2_CONFIG_SIZE_MAX,
@@ -122,6 +126,7 @@ static void sart3_set_entry(struct apple_sart *sart, int index, u8 flags,
 static struct apple_sart_ops sart_ops_v3 = {
 	.get_entry = sart3_get_entry,
 	.set_entry = sart3_set_entry,
+	.flags_allow = APPLE_SART3_FLAGS_ALLOW,
 	.size_shift = APPLE_SART3_SIZE_SHIFT,
 	.paddr_shift = APPLE_SART3_PADDR_SHIFT,
 	.size_max = APPLE_SART3_SIZE_MAX,
@@ -233,7 +238,7 @@ int apple_sart_add_allowed_region(struct apple_sart *sart, phys_addr_t paddr,
 		if (test_and_set_bit(i, &sart->used_entries))
 			continue;
 
-		ret = sart_set_entry(sart, i, APPLE_SART_FLAGS_ALLOW, paddr,
+		ret = sart_set_entry(sart, i, sart->ops->flags_allow, paddr,
 				     size);
 		if (ret) {
 			dev_dbg(sart->dev,

-- 
2.50.1

[PATCH v2 5/9] soc: apple: sart: Add SARTv0 support

[Nick Chan]

Add support for SARTv0 as found on Apple A11 SoC.

Reviewed-by: Sven Peter <sven@kernel.org>
Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/soc/apple/sart.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/soc/apple/sart.c b/drivers/soc/apple/sart.c
index 318f6e518a54f6edb8b97e320519df8f77c7f143..4ff1942b82a74cabeda99540ba63c6e92386986d 100644
--- a/drivers/soc/apple/sart.c
+++ b/drivers/soc/apple/sart.c
@@ -25,6 +25,18 @@
 
 #define APPLE_SART_MAX_ENTRIES 16
 
+/* SARTv0 registers */
+#define APPLE_SART0_CONFIG(idx)       (0x00 + 4 * (idx))
+#define APPLE_SART0_CONFIG_FLAGS      GENMASK(28, 24)
+#define APPLE_SART0_CONFIG_SIZE       GENMASK(18, 0)
+#define APPLE_SART0_CONFIG_SIZE_SHIFT 12
+#define APPLE_SART0_CONFIG_SIZE_MAX   GENMASK(18, 0)
+
+#define APPLE_SART0_PADDR(idx)  (0x40 + 4 * (idx))
+#define APPLE_SART0_PADDR_SHIFT 12
+
+#define APPLE_SART0_FLAGS_ALLOW 0xf
+
 /* SARTv2 registers */
 #define APPLE_SART2_CONFIG(idx)	      (0x00 + 4 * (idx))
 #define APPLE_SART2_CONFIG_FLAGS      GENMASK(31, 24)
@@ -71,6 +83,39 @@ struct apple_sart {
 	unsigned long used_entries;
 };
 
+static void sart0_get_entry(struct apple_sart *sart, int index, u8 *flags,
+	phys_addr_t *paddr, size_t *size)
+{
+	u32 cfg = readl(sart->regs + APPLE_SART0_CONFIG(index));
+	phys_addr_t paddr_ = readl(sart->regs + APPLE_SART0_PADDR(index));
+	size_t size_ = FIELD_GET(APPLE_SART0_CONFIG_SIZE, cfg);
+
+	*flags = FIELD_GET(APPLE_SART0_CONFIG_FLAGS, cfg);
+	*size = size_ << APPLE_SART0_CONFIG_SIZE_SHIFT;
+	*paddr = paddr_ << APPLE_SART0_PADDR_SHIFT;
+}
+
+static void sart0_set_entry(struct apple_sart *sart, int index, u8 flags,
+	phys_addr_t paddr_shifted, size_t size_shifted)
+{
+	u32 cfg;
+
+	cfg = FIELD_PREP(APPLE_SART0_CONFIG_FLAGS, flags);
+	cfg |= FIELD_PREP(APPLE_SART0_CONFIG_SIZE, size_shifted);
+
+	writel(paddr_shifted, sart->regs + APPLE_SART0_PADDR(index));
+	writel(cfg, sart->regs + APPLE_SART0_CONFIG(index));
+}
+
+static struct apple_sart_ops sart_ops_v0 = {
+	.get_entry = sart0_get_entry,
+	.set_entry = sart0_set_entry,
+	.flags_allow = APPLE_SART0_FLAGS_ALLOW,
+	.size_shift = APPLE_SART0_CONFIG_SIZE_SHIFT,
+	.paddr_shift = APPLE_SART0_PADDR_SHIFT,
+	.size_max = APPLE_SART0_CONFIG_SIZE_MAX,
+};
+
 static void sart2_get_entry(struct apple_sart *sart, int index, u8 *flags,
 			    phys_addr_t *paddr, size_t *size)
 {
@@ -319,6 +364,10 @@ static const struct of_device_id apple_sart_of_match[] = {
 		.compatible = "apple,t8103-sart",
 		.data = &sart_ops_v2,
 	},
+	{
+		.compatible = "apple,t8015-sart",
+		.data = &sart_ops_v0,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, apple_sart_of_match);

-- 
2.50.1

[PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Nick Chan]

Add ANS2 NVMe bindings for Apple A11 SoC.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 .../devicetree/bindings/nvme/apple,nvme-ans.yaml          | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
index fc6555724e1858e8a16f6750302ff0ad9c4e5b88..4127d7b0a0f066fd0e144b32d1b676e3406b9d5a 100644
--- a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
+++ b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
@@ -11,12 +11,14 @@ maintainers:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - apple,t8103-nvme-ans2
-          - apple,t8112-nvme-ans2
-          - apple,t6000-nvme-ans2
-      - const: apple,nvme-ans2
+    oneOf:
+      - const: apple,t8015-nvme-ans2
+      - items:
+          - enum:
+              - apple,t8103-nvme-ans2
+              - apple,t8112-nvme-ans2
+              - apple,t6000-nvme-ans2
+          - const: apple,nvme-ans2
 
   reg:
     items:
@@ -67,6 +69,7 @@ if:
     compatible:
       contains:
         enum:
+          - apple,t8015-nvme-ans2
           - apple,t8103-nvme-ans2
           - apple,t8112-nvme-ans2
 then:

-- 
2.50.1

[PATCH v2 7/9] nvme: apple: Add Apple A11 support

[Nick Chan]

Add support for ANS2 NVMe on Apple A11 SoC.

This version of ANS2 is less quirky than the one in M1, and does not have
NVMMU or Linear SQ. However, it still requires a non-standard 128-byte
SQE.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 drivers/nvme/host/apple.c | 198 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 139 insertions(+), 59 deletions(-)

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 1286c31320e630cb012009d6b962526e0553869f..f3999b8ef7ab759fe7b6d14bfc03671c51f21e82 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -35,7 +35,6 @@
 #include "nvme.h"
 
 #define APPLE_ANS_BOOT_TIMEOUT	  USEC_PER_SEC
-#define APPLE_ANS_MAX_QUEUE_DEPTH 64
 
 #define APPLE_ANS_COPROC_CPU_CONTROL	 0x44
 #define APPLE_ANS_COPROC_CPU_CONTROL_RUN BIT(4)
@@ -75,6 +74,8 @@
 #define APPLE_NVME_AQ_DEPTH	   2
 #define APPLE_NVME_AQ_MQ_TAG_DEPTH (APPLE_NVME_AQ_DEPTH - 1)
 
+#define APPLE_NVME_IOSQES	7
+
 /*
  * These can be higher, but we need to ensure that any command doesn't
  * require an sg allocation that needs more than a page of data.
@@ -142,6 +143,7 @@ struct apple_nvme_queue {
 	u32 __iomem *sq_db;
 	u32 __iomem *cq_db;
 
+	u16 sq_tail;
 	u16 cq_head;
 	u8 cq_phase;
 
@@ -166,11 +168,18 @@ struct apple_nvme_iod {
 	struct scatterlist *sg;
 };
 
+struct apple_nvme_hw {
+	bool has_lsq_nvmmu;
+	u32 max_queue_depth;
+	void (*submit_cmd)(struct apple_nvme_queue *q, struct nvme_command *cmd);
+};
+
 struct apple_nvme {
 	struct device *dev;
 
 	void __iomem *mmio_coproc;
 	void __iomem *mmio_nvme;
+	const struct apple_nvme_hw *hw;
 
 	struct device **pd_dev;
 	struct device_link **pd_link;
@@ -215,10 +224,12 @@ static inline struct apple_nvme *queue_to_apple_nvme(struct apple_nvme_queue *q)
 
 static unsigned int apple_nvme_queue_depth(struct apple_nvme_queue *q)
 {
-	if (q->is_adminq)
+	struct apple_nvme *anv = queue_to_apple_nvme(q);
+
+	if (q->is_adminq && anv->hw->has_lsq_nvmmu)
 		return APPLE_NVME_AQ_DEPTH;
 
-	return APPLE_ANS_MAX_QUEUE_DEPTH;
+	return anv->hw->max_queue_depth;
 }
 
 static void apple_nvme_rtkit_crashed(void *cookie, const void *crashlog, size_t crashlog_size)
@@ -280,7 +291,31 @@ static void apple_nvmmu_inval(struct apple_nvme_queue *q, unsigned int tag)
 				     "NVMMU TCB invalidation failed\n");
 }
 
-static void apple_nvme_submit_cmd(struct apple_nvme_queue *q,
+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));
+	else
+		memcpy((void *)q->sqes + (tag << APPLE_NVME_IOSQES), cmd, sizeof(*cmd));
+
+	writel(q->sq_tail, q->sq_db);
+	spin_unlock_irq(&anv->lock);
+}
+
+
+static void apple_nvme_submit_cmd_t8103(struct apple_nvme_queue *q,
 				  struct nvme_command *cmd)
 {
 	struct apple_nvme *anv = queue_to_apple_nvme(q);
@@ -590,7 +625,8 @@ static inline void apple_nvme_handle_cqe(struct apple_nvme_queue *q,
 	__u16 command_id = READ_ONCE(cqe->command_id);
 	struct request *req;
 
-	apple_nvmmu_inval(q, command_id);
+	if (anv->hw->has_lsq_nvmmu)
+		apple_nvmmu_inval(q, command_id);
 
 	req = nvme_find_rq(apple_nvme_queue_tagset(anv, q), command_id);
 	if (unlikely(!req)) {
@@ -685,7 +721,7 @@ static int apple_nvme_create_cq(struct apple_nvme *anv)
 	c.create_cq.opcode = nvme_admin_create_cq;
 	c.create_cq.prp1 = cpu_to_le64(anv->ioq.cq_dma_addr);
 	c.create_cq.cqid = cpu_to_le16(1);
-	c.create_cq.qsize = cpu_to_le16(APPLE_ANS_MAX_QUEUE_DEPTH - 1);
+	c.create_cq.qsize = cpu_to_le16(anv->hw->max_queue_depth - 1);
 	c.create_cq.cq_flags = cpu_to_le16(NVME_QUEUE_PHYS_CONTIG | NVME_CQ_IRQ_ENABLED);
 	c.create_cq.irq_vector = cpu_to_le16(0);
 
@@ -713,7 +749,7 @@ static int apple_nvme_create_sq(struct apple_nvme *anv)
 	c.create_sq.opcode = nvme_admin_create_sq;
 	c.create_sq.prp1 = cpu_to_le64(anv->ioq.sq_dma_addr);
 	c.create_sq.sqid = cpu_to_le16(1);
-	c.create_sq.qsize = cpu_to_le16(APPLE_ANS_MAX_QUEUE_DEPTH - 1);
+	c.create_sq.qsize = cpu_to_le16(anv->hw->max_queue_depth - 1);
 	c.create_sq.sq_flags = cpu_to_le16(NVME_QUEUE_PHYS_CONTIG);
 	c.create_sq.cqid = cpu_to_le16(1);
 
@@ -765,7 +801,7 @@ static blk_status_t apple_nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
 	}
 
 	nvme_start_request(req);
-	apple_nvme_submit_cmd(q, cmnd);
+	anv->hw->submit_cmd(q, cmnd);
 	return BLK_STS_OK;
 
 out_free_cmd:
@@ -970,11 +1006,13 @@ static const struct blk_mq_ops apple_nvme_mq_ops = {
 static void apple_nvme_init_queue(struct apple_nvme_queue *q)
 {
 	unsigned int depth = apple_nvme_queue_depth(q);
+	struct apple_nvme *anv = queue_to_apple_nvme(q);
 
 	q->cq_head = 0;
 	q->cq_phase = 1;
-	memset(q->tcbs, 0,
-	       APPLE_ANS_MAX_QUEUE_DEPTH * sizeof(struct apple_nvmmu_tcb));
+	if (anv->hw->has_lsq_nvmmu)
+		memset(q->tcbs, 0,
+			anv->hw->max_queue_depth * sizeof(struct apple_nvmmu_tcb));
 	memset(q->cqes, 0, depth * sizeof(struct nvme_completion));
 	WRITE_ONCE(q->enabled, true);
 	wmb(); /* ensure the first interrupt sees the initialization */
@@ -1069,49 +1107,56 @@ static void apple_nvme_reset_work(struct work_struct *work)
 
 	dma_set_max_seg_size(anv->dev, 0xffffffff);
 
-	/*
-	 * Enable NVMMU and linear submission queues.
-	 * While we could keep those disabled and pretend this is slightly
-	 * more common NVMe controller we'd still need some quirks (e.g.
-	 * sq entries will be 128 bytes) and Apple might drop support for
-	 * that mode in the future.
-	 */
-	writel(APPLE_ANS_LINEAR_SQ_EN,
-	       anv->mmio_nvme + APPLE_ANS_LINEAR_SQ_CTRL);
+	if (anv->hw->has_lsq_nvmmu) {
+		/*
+		 * Enable NVMMU and linear submission queues.
+		 * While we could keep those disabled and pretend this is slightly
+		 * more common NVMe controller we'd still need some quirks (e.g.
+		 * sq entries will be 128 bytes) and Apple might drop support for
+		 * that mode in the future.
+		 */
+		writel(APPLE_ANS_LINEAR_SQ_EN,
+			anv->mmio_nvme + APPLE_ANS_LINEAR_SQ_CTRL);
 
-	/* Allow as many pending command as possible for both queues */
-	writel(APPLE_ANS_MAX_QUEUE_DEPTH | (APPLE_ANS_MAX_QUEUE_DEPTH << 16),
-	       anv->mmio_nvme + APPLE_ANS_MAX_PEND_CMDS_CTRL);
+		/* Allow as many pending command as possible for both queues */
+		writel(anv->hw->max_queue_depth | (anv->hw->max_queue_depth << 16),
+			anv->mmio_nvme + APPLE_ANS_MAX_PEND_CMDS_CTRL);
 
-	/* Setup the NVMMU for the maximum admin and IO queue depth */
-	writel(APPLE_ANS_MAX_QUEUE_DEPTH - 1,
-	       anv->mmio_nvme + APPLE_NVMMU_NUM_TCBS);
+		/* Setup the NVMMU for the maximum admin and IO queue depth */
+		writel(anv->hw->max_queue_depth - 1,
+			anv->mmio_nvme + APPLE_NVMMU_NUM_TCBS);
 
-	/*
-	 * This is probably a chicken bit: without it all commands where any PRP
-	 * is set to zero (including those that don't use that field) fail and
-	 * the co-processor complains about "completed with err BAD_CMD-" or
-	 * a "NULL_PRP_PTR_ERR" in the syslog
-	 */
-	writel(readl(anv->mmio_nvme + APPLE_ANS_UNKNOWN_CTRL) &
-		       ~APPLE_ANS_PRP_NULL_CHECK,
-	       anv->mmio_nvme + APPLE_ANS_UNKNOWN_CTRL);
+		/*
+		 * This is probably a chicken bit: without it all commands where any PRP
+		 * is set to zero (including those that don't use that field) fail and
+		 * the co-processor complains about "completed with err BAD_CMD-" or
+		 * a "NULL_PRP_PTR_ERR" in the syslog
+		 */
+		writel(readl(anv->mmio_nvme + APPLE_ANS_UNKNOWN_CTRL) &
+			~APPLE_ANS_PRP_NULL_CHECK,
+			anv->mmio_nvme + APPLE_ANS_UNKNOWN_CTRL);
+	}
 
 	/* Setup the admin queue */
-	aqa = APPLE_NVME_AQ_DEPTH - 1;
+	if (anv->hw->has_lsq_nvmmu)
+		aqa = APPLE_NVME_AQ_DEPTH - 1;
+	else
+		aqa = anv->hw->max_queue_depth - 1;
 	aqa |= aqa << 16;
 	writel(aqa, anv->mmio_nvme + NVME_REG_AQA);
 	writeq(anv->adminq.sq_dma_addr, anv->mmio_nvme + NVME_REG_ASQ);
 	writeq(anv->adminq.cq_dma_addr, anv->mmio_nvme + NVME_REG_ACQ);
 
-	/* Setup NVMMU for both queues */
-	writeq(anv->adminq.tcb_dma_addr,
-	       anv->mmio_nvme + APPLE_NVMMU_ASQ_TCB_BASE);
-	writeq(anv->ioq.tcb_dma_addr,
-	       anv->mmio_nvme + APPLE_NVMMU_IOSQ_TCB_BASE);
+	if (anv->hw->has_lsq_nvmmu) {
+		/* Setup NVMMU for both queues */
+		writeq(anv->adminq.tcb_dma_addr,
+			anv->mmio_nvme + APPLE_NVMMU_ASQ_TCB_BASE);
+		writeq(anv->ioq.tcb_dma_addr,
+			anv->mmio_nvme + APPLE_NVMMU_IOSQ_TCB_BASE);
+	}
 
 	anv->ctrl.sqsize =
-		APPLE_ANS_MAX_QUEUE_DEPTH - 1; /* 0's based queue depth */
+		anv->hw->max_queue_depth - 1; /* 0's based queue depth */
 	anv->ctrl.cap = readq(anv->mmio_nvme + NVME_REG_CAP);
 
 	dev_dbg(anv->dev, "Enabling controller now");
@@ -1282,8 +1327,9 @@ static int apple_nvme_alloc_tagsets(struct apple_nvme *anv)
 	 * both queues. The admin queue gets the first APPLE_NVME_AQ_DEPTH which
 	 * must be marked as reserved in the IO queue.
 	 */
-	anv->tagset.reserved_tags = APPLE_NVME_AQ_DEPTH;
-	anv->tagset.queue_depth = APPLE_ANS_MAX_QUEUE_DEPTH - 1;
+	if (anv->hw->has_lsq_nvmmu)
+		anv->tagset.reserved_tags = APPLE_NVME_AQ_DEPTH;
+	anv->tagset.queue_depth = anv->hw->max_queue_depth - 1;
 	anv->tagset.timeout = NVME_IO_TIMEOUT;
 	anv->tagset.numa_node = NUMA_NO_NODE;
 	anv->tagset.cmd_size = sizeof(struct apple_nvme_iod);
@@ -1307,6 +1353,7 @@ static int apple_nvme_queue_alloc(struct apple_nvme *anv,
 				  struct apple_nvme_queue *q)
 {
 	unsigned int depth = apple_nvme_queue_depth(q);
+	size_t iosq_size;
 
 	q->cqes = dmam_alloc_coherent(anv->dev,
 				      depth * sizeof(struct nvme_completion),
@@ -1314,22 +1361,29 @@ static int apple_nvme_queue_alloc(struct apple_nvme *anv,
 	if (!q->cqes)
 		return -ENOMEM;
 
+	if (anv->hw->has_lsq_nvmmu)
+		iosq_size = depth * sizeof(struct nvme_command);
+	else
+		iosq_size = depth << APPLE_NVME_IOSQES;
+
 	q->sqes = dmam_alloc_coherent(anv->dev,
-				      depth * sizeof(struct nvme_command),
+				      iosq_size,
 				      &q->sq_dma_addr, GFP_KERNEL);
 	if (!q->sqes)
 		return -ENOMEM;
 
-	/*
-	 * We need the maximum queue depth here because the NVMMU only has a
-	 * single depth configuration shared between both queues.
-	 */
-	q->tcbs = dmam_alloc_coherent(anv->dev,
-				      APPLE_ANS_MAX_QUEUE_DEPTH *
-					      sizeof(struct apple_nvmmu_tcb),
-				      &q->tcb_dma_addr, GFP_KERNEL);
-	if (!q->tcbs)
-		return -ENOMEM;
+	if (anv->hw->has_lsq_nvmmu) {
+		/*
+		 * We need the maximum queue depth here because the NVMMU only has a
+		 * single depth configuration shared between both queues.
+		 */
+		q->tcbs = dmam_alloc_coherent(anv->dev,
+			anv->hw->max_queue_depth *
+				sizeof(struct apple_nvmmu_tcb),
+			&q->tcb_dma_addr, GFP_KERNEL);
+		if (!q->tcbs)
+			return -ENOMEM;
+	}
 
 	/*
 	 * initialize phase to make sure the allocated and empty memory
@@ -1413,6 +1467,12 @@ static struct apple_nvme *apple_nvme_alloc(struct platform_device *pdev)
 	anv->adminq.is_adminq = true;
 	platform_set_drvdata(pdev, anv);
 
+	anv->hw = (const struct apple_nvme_hw *)of_device_get_match_data(&pdev->dev);
+	if (!anv->hw) {
+		ret = -ENODEV;
+		goto put_dev;
+	}
+
 	ret = apple_nvme_attach_genpd(anv);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "Failed to attach power domains");
@@ -1444,10 +1504,17 @@ static struct apple_nvme *apple_nvme_alloc(struct platform_device *pdev)
 		goto put_dev;
 	}
 
-	anv->adminq.sq_db = anv->mmio_nvme + APPLE_ANS_LINEAR_ASQ_DB;
-	anv->adminq.cq_db = anv->mmio_nvme + APPLE_ANS_ACQ_DB;
-	anv->ioq.sq_db = anv->mmio_nvme + APPLE_ANS_LINEAR_IOSQ_DB;
-	anv->ioq.cq_db = anv->mmio_nvme + APPLE_ANS_IOCQ_DB;
+	if (anv->hw->has_lsq_nvmmu) {
+		anv->adminq.sq_db = anv->mmio_nvme + APPLE_ANS_LINEAR_ASQ_DB;
+		anv->adminq.cq_db = anv->mmio_nvme + APPLE_ANS_ACQ_DB;
+		anv->ioq.sq_db = anv->mmio_nvme + APPLE_ANS_LINEAR_IOSQ_DB;
+		anv->ioq.cq_db = anv->mmio_nvme + APPLE_ANS_IOCQ_DB;
+	} else {
+		anv->adminq.sq_db = anv->mmio_nvme + NVME_REG_DBS;
+		anv->adminq.cq_db = anv->mmio_nvme + APPLE_ANS_ACQ_DB;
+		anv->ioq.sq_db = anv->mmio_nvme + NVME_REG_DBS + 8;
+		anv->ioq.cq_db = anv->mmio_nvme + APPLE_ANS_IOCQ_DB;
+	}
 
 	anv->sart = devm_apple_sart_get(dev);
 	if (IS_ERR(anv->sart)) {
@@ -1625,8 +1692,21 @@ static int apple_nvme_suspend(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(apple_nvme_pm_ops, apple_nvme_suspend,
 				apple_nvme_resume);
 
+static const struct apple_nvme_hw apple_nvme_t8015_hw = {
+	.has_lsq_nvmmu = false,
+	.max_queue_depth = 16,
+	.submit_cmd = &apple_nvme_submit_cmd_t8015,
+};
+
+static const struct apple_nvme_hw apple_nvme_t8103_hw = {
+	.has_lsq_nvmmu = true,
+	.max_queue_depth = 64,
+	.submit_cmd = &apple_nvme_submit_cmd_t8103,
+};
+
 static const struct of_device_id apple_nvme_of_match[] = {
-	{ .compatible = "apple,nvme-ans2" },
+	{ .compatible = "apple,t8015-nvme-ans2", .data = &apple_nvme_t8015_hw },
+	{ .compatible = "apple,nvme-ans2", .data = &apple_nvme_t8103_hw },
 	{},
 };
 MODULE_DEVICE_TABLE(of, apple_nvme_of_match);

-- 
2.50.1

[PATCH v2 8/9] arm64: dts: apple: t8015: Fix PCIE power domains dependencies

[Nick Chan]

Fix the dependency topology of PCIE power domain nodes, as required by ANS2
NVME controller.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 arch/arm64/boot/dts/apple/t8015-pmgr.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/apple/t8015-pmgr.dtsi b/arch/arm64/boot/dts/apple/t8015-pmgr.dtsi
index e238c2d2732f7944dc1f984a4eb647ba212b9ea5..1d8da9c7863e5b7a732888342de9d481f309edd8 100644
--- a/arch/arm64/boot/dts/apple/t8015-pmgr.dtsi
+++ b/arch/arm64/boot/dts/apple/t8015-pmgr.dtsi
@@ -658,6 +658,7 @@ ps_pcie: power-controller@80318 {
 		#power-domain-cells = <0>;
 		#reset-cells = <0>;
 		label = "pcie";
+		power-domains = <&ps_pcie_aux>, <&ps_pcie_direct>, <&ps_pcie_ref>;
 	};
 
 	ps_pcie_aux: power-controller@80320 {

-- 
2.50.1

[PATCH v2 9/9] arm64: dts: apple: t8015: Add NVMe nodes

[Nick Chan]

Add nodes for NVMe and associated mailbox and sart for Apple A11 SoC.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
---
 arch/arm64/boot/dts/apple/t8015.dtsi | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t8015.dtsi b/arch/arm64/boot/dts/apple/t8015.dtsi
index 12acf8fc8bc6bcde6b11773cadd97e9ee115f510..84acf2839fb6279dcc956e1f4cee1afa909d2f27 100644
--- a/arch/arm64/boot/dts/apple/t8015.dtsi
+++ b/arch/arm64/boot/dts/apple/t8015.dtsi
@@ -402,6 +402,40 @@ pinctrl_smc: pinctrl@236024000 {
 			 */
 			status = "disabled";
 		};
+
+		ans_mbox: mbox@257008000 {
+			compatible = "apple,t8015-asc-mailbox";
+			reg = <0x2 0x57008000 0x0 0x4000>;
+			interrupt-parent = <&aic>;
+			interrupts = <AIC_IRQ 265 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_IRQ 266 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_IRQ 267 IRQ_TYPE_LEVEL_HIGH>,
+				<AIC_IRQ 268 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "send-empty", "send-not-empty",
+				"recv-empty", "recv-not-empty";
+			#mbox-cells = <0>;
+			power-domains = <&ps_ans2>;
+		};
+
+		sart: iommu@259c50000 {
+			compatible = "apple,t8015-sart";
+			reg = <0x2 0x59c50000 0x0 0x10000>;
+			power-domains = <&ps_ans2>;
+		};
+
+		nvme@259cc0000 {
+			compatible = "apple,t8015-nvme-ans2";
+			reg = <0x2 0x59cc0000 0x0 0x40000>,
+				<0x2 0x59d20000 0x0 0x2000>;
+			reg-names = "nvme", "ans";
+			interrupt-parent = <&aic>;
+			interrupts = <AIC_IRQ 270 IRQ_TYPE_LEVEL_HIGH>;
+			mboxes = <&ans_mbox>;
+			apple,sart = <&sart>;
+			power-domains = <&ps_ans2>, <&ps_pcie>;
+			power-domain-names = "ans", "apcie0";
+			resets = <&ps_ans2>;
+		};
 	};
 
 	timer {

-- 
2.50.1

Re: [PATCH v2 7/9] nvme: apple: Add Apple A11 support

[Christoph Hellwig]

On Mon, Aug 18, 2025 at 04:43:00PM +0800, Nick Chan wrote:
>  };
>  
> +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.

> +
> +	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.

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

Do we even need this cast?

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Krzysztof Kozlowski]

On Mon, Aug 18, 2025 at 04:42:59PM +0800, Nick Chan wrote:
> Add ANS2 NVMe bindings for Apple A11 SoC.
> 
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  .../devicetree/bindings/nvme/apple,nvme-ans.yaml          | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
> index fc6555724e1858e8a16f6750302ff0ad9c4e5b88..4127d7b0a0f066fd0e144b32d1b676e3406b9d5a 100644
> --- a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
> +++ b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
> @@ -11,12 +11,14 @@ maintainers:
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - apple,t8103-nvme-ans2
> -          - apple,t8112-nvme-ans2
> -          - apple,t6000-nvme-ans2
> -      - const: apple,nvme-ans2
> +    oneOf:
> +      - const: apple,t8015-nvme-ans2
> +      - items:
> +          - enum:
> +              - apple,t8103-nvme-ans2
> +              - apple,t8112-nvme-ans2
> +              - apple,t6000-nvme-ans2
> +          - const: apple,nvme-ans2

When some months ago this pattern of generic fallback appeared, I
believe I commented it is bad idea. So now months later we have a proof
- generic fallback is useless and you should have been using SoC
specific compatibles from the start.

Now it is just confusing and this broken pattern will be spreading more
and more, because you folks put generic compatibles everywhere.

Best regards,
Krzysztof

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Sven Peter]

On 19.08.25 11:18, Krzysztof Kozlowski wrote:
> On Mon, Aug 18, 2025 at 04:42:59PM +0800, Nick Chan wrote:
>> Add ANS2 NVMe bindings for Apple A11 SoC.
>>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>   .../devicetree/bindings/nvme/apple,nvme-ans.yaml          | 15 +++++++++------
>>   1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>> index fc6555724e1858e8a16f6750302ff0ad9c4e5b88..4127d7b0a0f066fd0e144b32d1b676e3406b9d5a 100644
>> --- a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>> +++ b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>> @@ -11,12 +11,14 @@ maintainers:
>>   
>>   properties:
>>     compatible:
>> -    items:
>> -      - enum:
>> -          - apple,t8103-nvme-ans2
>> -          - apple,t8112-nvme-ans2
>> -          - apple,t6000-nvme-ans2
>> -      - const: apple,nvme-ans2
>> +    oneOf:
>> +      - const: apple,t8015-nvme-ans2
>> +      - items:
>> +          - enum:
>> +              - apple,t8103-nvme-ans2
>> +              - apple,t8112-nvme-ans2
>> +              - apple,t6000-nvme-ans2
>> +          - const: apple,nvme-ans2
> 
> When some months ago this pattern of generic fallback appeared, I
> believe I commented it is bad idea. So now months later we have a proof
> - generic fallback is useless and you should have been using SoC
> specific compatibles from the start.
> 
> Now it is just confusing and this broken pattern will be spreading more
> and more, because you folks put generic compatibles everywhere.

I haven't commented on the dt-bindings yet because I suspect this patch 
is wrong but haven't had time to test this yet.

I believe we want "apple,t8015-nvme-ans2", "apple,nvme-ans2" here and
then use the code Nick added for "apple,nvme-ans2" by default and only
enable additional features (NVMMU, linear submission queue) when we see
the SoC-specific compatibles for t8103, t8112, and t6000. IIRC these
newer SoCs still support the old way of submitting commands just fine
and the new way was added at some point to add support for this weird
integrated IOMMU.

I've already seen some strings about ANS3 somewhere which I suspect
will be the controller in some future SoC (or maybe M3/M4 which we 
haven't reverse engineered yet) that actually breaks compatibility.

It's too late to drop them here but if you're strongly opposed to these
generic fallbacks we can just switch to only use tXXXX-nvme-ans3 at that
point without making anything confusing. Same for any other new hardware
blocks we reverse engineer and upstream.



Sven

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Krzysztof Kozlowski]

On 19/08/2025 12:01, Sven Peter wrote:
> On 19.08.25 11:18, Krzysztof Kozlowski wrote:
>> On Mon, Aug 18, 2025 at 04:42:59PM +0800, Nick Chan wrote:
>>> Add ANS2 NVMe bindings for Apple A11 SoC.
>>>
>>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>>> ---
>>>   .../devicetree/bindings/nvme/apple,nvme-ans.yaml          | 15 +++++++++------
>>>   1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>> index fc6555724e1858e8a16f6750302ff0ad9c4e5b88..4127d7b0a0f066fd0e144b32d1b676e3406b9d5a 100644
>>> --- a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>> +++ b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>> @@ -11,12 +11,14 @@ maintainers:
>>>   
>>>   properties:
>>>     compatible:
>>> -    items:
>>> -      - enum:
>>> -          - apple,t8103-nvme-ans2
>>> -          - apple,t8112-nvme-ans2
>>> -          - apple,t6000-nvme-ans2
>>> -      - const: apple,nvme-ans2
>>> +    oneOf:
>>> +      - const: apple,t8015-nvme-ans2
>>> +      - items:
>>> +          - enum:
>>> +              - apple,t8103-nvme-ans2
>>> +              - apple,t8112-nvme-ans2
>>> +              - apple,t6000-nvme-ans2
>>> +          - const: apple,nvme-ans2
>>
>> When some months ago this pattern of generic fallback appeared, I
>> believe I commented it is bad idea. So now months later we have a proof
>> - generic fallback is useless and you should have been using SoC
>> specific compatibles from the start.
>>
>> Now it is just confusing and this broken pattern will be spreading more
>> and more, because you folks put generic compatibles everywhere.
> 
> I haven't commented on the dt-bindings yet because I suspect this patch 
> is wrong but haven't had time to test this yet.
> 
> I believe we want "apple,t8015-nvme-ans2", "apple,nvme-ans2" here and
> then use the code Nick added for "apple,nvme-ans2" by default and only
> enable additional features (NVMMU, linear submission queue) when we see
> the SoC-specific compatibles for t8103, t8112, and t6000. IIRC these
> newer SoCs still support the old way of submitting commands just fine
> and the new way was added at some point to add support for this weird
> integrated IOMMU.
> 
> I've already seen some strings about ANS3 somewhere which I suspect
> will be the controller in some future SoC (or maybe M3/M4 which we 
> haven't reverse engineered yet) that actually breaks compatibility.


This was 99% predictable and expected months/years ago when first Apple
M1 generic compatibles appeared. I just do not understand why so much
effort from reviewers has to go into explaining this and for arguing
over that, and eventually we are right.

> 
> It's too late to drop them here but if you're strongly opposed to these
> generic fallbacks we can just switch to only use tXXXX-nvme-ans3 at that
> point without making anything confusing. Same for any other new hardware
> blocks we reverse engineer and upstream.



Best regards,
Krzysztof

Re: [PATCH v2 7/9] nvme: apple: Add Apple A11 support

[Nick Chan]

On 19/8/2025 16:30, Christoph Hellwig wrote:
> On Mon, Aug 18, 2025 at 04:43:00PM +0800, Nick Chan wrote:
>>  };
>>  
>> +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.
> 
>> +
>> +	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.

> 
>> +	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

Re: [PATCH v2 1/9] dt-bindings: mailbox: apple,mailbox: Add ASC mailboxes on Apple A11 and T2

[Rob Herring (Arm)]

On Mon, 18 Aug 2025 16:42:54 +0800, Nick Chan wrote:
> Add bindings for ASC mailboxes as found on Apple A11 and T2 SoCs. These
> mailboxes are used for coprocessors including Secure Enclave Processor
> (SEP), the NVMe coprocessor and the system management controller.
> 
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 3/9] dt-bindings: iommu: apple,sart: Add Apple A11

[Rob Herring (Arm)]

On Mon, 18 Aug 2025 16:42:56 +0800, Nick Chan wrote:
> Add bindings for SARTv0 as found on Apple A11 SoC.
> 
> Reviewed-by: Sven Peter <sven@kernel.org>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  Documentation/devicetree/bindings/iommu/apple,sart.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 3/9] dt-bindings: iommu: apple,sart: Add Apple A11

[Rob Herring]

On Mon, Aug 18, 2025 at 04:42:56PM +0800, Nick Chan wrote:
> Add bindings for SARTv0 as found on Apple A11 SoC.
> 
> Reviewed-by: Sven Peter <sven@kernel.org>
> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
> ---
>  Documentation/devicetree/bindings/iommu/apple,sart.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/apple,sart.yaml b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
> index e87c1520fea60a2de549aa8a469fcded52e3b6e3..95d55ec688779bd52b8aef316a4c1ba7b7449574 100644
> --- a/Documentation/devicetree/bindings/iommu/apple,sart.yaml
> +++ b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
> @@ -29,6 +29,7 @@ description:
>  properties:
>    compatible:
>      oneOf:
> +      - const: apple,t8015-sart

Actually I take back my ack, move this to the existing 'enum' down 
below. With that change, you can keep the ack.

>        - items:
>            - const: apple,t8112-sart
>            - const: apple,t6000-sart
> 
> -- 
> 2.50.1
> 

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Sven Peter]

On 19.08.25 13:34, Krzysztof Kozlowski wrote:
> On 19/08/2025 12:01, Sven Peter wrote:
>> On 19.08.25 11:18, Krzysztof Kozlowski wrote:
>>> On Mon, Aug 18, 2025 at 04:42:59PM +0800, Nick Chan wrote:
>>>> Add ANS2 NVMe bindings for Apple A11 SoC.
>>>>
>>>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>>>> ---
>>>>    .../devicetree/bindings/nvme/apple,nvme-ans.yaml          | 15 +++++++++------
>>>>    1 file changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>>> index fc6555724e1858e8a16f6750302ff0ad9c4e5b88..4127d7b0a0f066fd0e144b32d1b676e3406b9d5a 100644
>>>> --- a/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>>> +++ b/Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
>>>> @@ -11,12 +11,14 @@ maintainers:
>>>>    
>>>>    properties:
>>>>      compatible:
>>>> -    items:
>>>> -      - enum:
>>>> -          - apple,t8103-nvme-ans2
>>>> -          - apple,t8112-nvme-ans2
>>>> -          - apple,t6000-nvme-ans2
>>>> -      - const: apple,nvme-ans2
>>>> +    oneOf:
>>>> +      - const: apple,t8015-nvme-ans2
>>>> +      - items:
>>>> +          - enum:
>>>> +              - apple,t8103-nvme-ans2
>>>> +              - apple,t8112-nvme-ans2
>>>> +              - apple,t6000-nvme-ans2
>>>> +          - const: apple,nvme-ans2
>>>
>>> When some months ago this pattern of generic fallback appeared, I
>>> believe I commented it is bad idea. So now months later we have a proof
>>> - generic fallback is useless and you should have been using SoC
>>> specific compatibles from the start.
>>>
>>> Now it is just confusing and this broken pattern will be spreading more
>>> and more, because you folks put generic compatibles everywhere.
>>
>> I haven't commented on the dt-bindings yet because I suspect this patch
>> is wrong but haven't had time to test this yet.
>>
>> I believe we want "apple,t8015-nvme-ans2", "apple,nvme-ans2" here and
>> then use the code Nick added for "apple,nvme-ans2" by default and only
>> enable additional features (NVMMU, linear submission queue) when we see
>> the SoC-specific compatibles for t8103, t8112, and t6000. IIRC these
>> newer SoCs still support the old way of submitting commands just fine
>> and the new way was added at some point to add support for this weird
>> integrated IOMMU.
>>
>> I've already seen some strings about ANS3 somewhere which I suspect
>> will be the controller in some future SoC (or maybe M3/M4 which we
>> haven't reverse engineered yet) that actually breaks compatibility.
> 
> 
> This was 99% predictable and expected months/years ago when first Apple
> M1 generic compatibles appeared. I just do not understand why so much
> effort from reviewers has to go into explaining this and for arguing
> over that, and eventually we are right.
> 

Right, so we're in a bad situation now because we didn't listen. That 
sucks and I'm sorry.

How do we move on from here now though?

I think for any new bindings it's pretty clear, we just do what we 
should've done from the very beginning:
Whenever we upstream a new device that has no bindings yet we only use 
tXXXX-whatever as compatible and use that as a fallback in case later 
(or earlier) SoCs work with the same driver.



Then we still have to deal with the stuff that's already upstream. I see 
two options here:


1) For situations like this one where the generic one just doesn't make 
any sense we deprecate "apple,nvme-ans2" in the binding and use
"apple,t8103-nvme-ans2" as the fallback instead, i.e. just
"apple,t8103-nvme-ans2" for M1, "apple,t6000-nvme-ans2", 
"apple,t8103-nvme-ans2" for M1 Pro, and just "apple,t8015-nvme-ans2" for 
A11.

We keep the generic one in the driver for now but also add
"apple,t8103-nvme-ans2". We then remove the generic one from all 
upstream DTS files but keep it inside the downstream files we ship to 
users for now to avoid pain with kernel upgrades/downgrades.
A year or two from now we can then delete the deprecated generic 
compatibles everywhere. This all has to be synced with OpenBSD and 
u-boot as well since both also use these bindings.
It's gonna be rather painful but this would clean up the entire mess.


2) We keep using the ones that are already upstream and just accept that 
the situation is a mess and add comment above all the bindings that we 
messed up and that this should not be used as pattern.
In this case that means it'll just be "apple,t8015-nvme-ans2" for A11 
without any fallback and we keep everything else the way it is.

I prefer option (2) but if you really want to get rid of all this mess 
I'll also work on (1).

Would either option work for you or do you have a better idea?



Thanks,


Sven

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Krzysztof Kozlowski]

On 21/08/2025 12:25, Sven Peter wrote:
> 
> 
> 1) For situations like this one where the generic one just doesn't make 
> any sense we deprecate "apple,nvme-ans2" in the binding and use
> "apple,t8103-nvme-ans2" as the fallback instead, i.e. just
> "apple,t8103-nvme-ans2" for M1, "apple,t6000-nvme-ans2", 
> "apple,t8103-nvme-ans2" for M1 Pro, and just "apple,t8015-nvme-ans2" for 
> A11.
> 
> We keep the generic one in the driver for now but also add
> "apple,t8103-nvme-ans2". We then remove the generic one from all 
> upstream DTS files but keep it inside the downstream files we ship to 
> users for now to avoid pain with kernel upgrades/downgrades.
> A year or two from now we can then delete the deprecated generic 
> compatibles everywhere. This all has to be synced with OpenBSD and 
> u-boot as well since both also use these bindings.
> It's gonna be rather painful but this would clean up the entire mess.
> 
> 
> 2) We keep using the ones that are already upstream and just accept that 
> the situation is a mess and add comment above all the bindings that we 
> messed up and that this should not be used as pattern.
> In this case that means it'll just be "apple,t8015-nvme-ans2" for A11 
> without any fallback and we keep everything else the way it is.
> 
> I prefer option (2) but if you really want to get rid of all this mess 


I also prefer option (2). That's the least disruptive option for users
and inconsistency in bindings naming is just inconsistency, no big deal.
You just need to remember not to grow the old items/pattern with generic
compatible.

Best regards,
Krzysztof

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Sven Peter]

On 21.08.25 15:09, Krzysztof Kozlowski wrote:
> On 21/08/2025 12:25, Sven Peter wrote:
>> 2) We keep using the ones that are already upstream and just accept that
>> the situation is a mess and add comment above all the bindings that we
>> messed up and that this should not be used as pattern.
>> In this case that means it'll just be "apple,t8015-nvme-ans2" for A11
>> without any fallback and we keep everything else the way it is.
>>
>> I prefer option (2) but if you really want to get rid of all this mess
> 
> 
> I also prefer option (2). That's the least disruptive option for users
> and inconsistency in bindings naming is just inconsistency, no big deal.
> You just need to remember not to grow the old items/pattern with generic
> compatible.

Okay, thanks. I'll make sure we keep those old items/patterns frozen and 
essentially start over for any new hardware.


Best,


Sven

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Janne Grunau]

On Thu, Aug 21, 2025 at 03:09:42PM +0200, Krzysztof Kozlowski wrote:
> On 21/08/2025 12:25, Sven Peter wrote:
> > 
> > 1) For situations like this one where the generic one just doesn't make 
> > any sense we deprecate "apple,nvme-ans2" in the binding and use
> > "apple,t8103-nvme-ans2" as the fallback instead, i.e. just
> > "apple,t8103-nvme-ans2" for M1, "apple,t6000-nvme-ans2", 
> > "apple,t8103-nvme-ans2" for M1 Pro, and just "apple,t8015-nvme-ans2" for 
> > A11.
> > 
> > We keep the generic one in the driver for now but also add
> > "apple,t8103-nvme-ans2". We then remove the generic one from all 
> > upstream DTS files but keep it inside the downstream files we ship to 
> > users for now to avoid pain with kernel upgrades/downgrades.
> > A year or two from now we can then delete the deprecated generic 
> > compatibles everywhere. This all has to be synced with OpenBSD and 
> > u-boot as well since both also use these bindings.
> > It's gonna be rather painful but this would clean up the entire mess.
> > 
> > 
> > 2) We keep using the ones that are already upstream and just accept that 
> > the situation is a mess and add comment above all the bindings that we 
> > messed up and that this should not be used as pattern.
> > In this case that means it'll just be "apple,t8015-nvme-ans2" for A11 
> > without any fallback and we keep everything else the way it is.
> > 
> > I prefer option (2) but if you really want to get rid of all this mess 
> 
> 
> I also prefer option (2). That's the least disruptive option for users
> and inconsistency in bindings naming is just inconsistency, no big deal.
> You just need to remember not to grow the old items/pattern with generic
> compatible.

This sounds to me like a mix of option 1) and 2).

All devices / SoCs already upstream will use the fixed current
compatibles and thus are following option 2)

New SoCs will have to use
    compatible = "apple,t6020-nvme-ans2", "apple,t8103-nvme-ans2";
using t6020 as example even though they will be using the same driver
code as "apple,nvme-ans2". Using t6020 as an example I planned to submit
today.
This will require adding the new fallback "apple,t8103-nvme-ans2"
compatible string to the driver.

Asking for clarification as I could image such driver changes will raise
questions from the driver maintainers.

Is there a way do document/annotate the generic compatibles as
deprecated / "do not use" for new devices?

Thanks
Janne

Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

[Krzysztof Kozlowski]

On 27/08/2025 14:25, Janne Grunau wrote:
> On Thu, Aug 21, 2025 at 03:09:42PM +0200, Krzysztof Kozlowski wrote:
>> On 21/08/2025 12:25, Sven Peter wrote:
>>>
>>> 1) For situations like this one where the generic one just doesn't make 
>>> any sense we deprecate "apple,nvme-ans2" in the binding and use
>>> "apple,t8103-nvme-ans2" as the fallback instead, i.e. just
>>> "apple,t8103-nvme-ans2" for M1, "apple,t6000-nvme-ans2", 
>>> "apple,t8103-nvme-ans2" for M1 Pro, and just "apple,t8015-nvme-ans2" for 
>>> A11.
>>>
>>> We keep the generic one in the driver for now but also add
>>> "apple,t8103-nvme-ans2". We then remove the generic one from all 
>>> upstream DTS files but keep it inside the downstream files we ship to 
>>> users for now to avoid pain with kernel upgrades/downgrades.
>>> A year or two from now we can then delete the deprecated generic 
>>> compatibles everywhere. This all has to be synced with OpenBSD and 
>>> u-boot as well since both also use these bindings.
>>> It's gonna be rather painful but this would clean up the entire mess.
>>>
>>>
>>> 2) We keep using the ones that are already upstream and just accept that 
>>> the situation is a mess and add comment above all the bindings that we 
>>> messed up and that this should not be used as pattern.
>>> In this case that means it'll just be "apple,t8015-nvme-ans2" for A11 
>>> without any fallback and we keep everything else the way it is.
>>>
>>> I prefer option (2) but if you really want to get rid of all this mess 
>>
>>
>> I also prefer option (2). That's the least disruptive option for users
>> and inconsistency in bindings naming is just inconsistency, no big deal.
>> You just need to remember not to grow the old items/pattern with generic
>> compatible.
> 
> This sounds to me like a mix of option 1) and 2).
> 
> All devices / SoCs already upstream will use the fixed current
> compatibles and thus are following option 2)
> 
> New SoCs will have to use
>     compatible = "apple,t6020-nvme-ans2", "apple,t8103-nvme-ans2";
> using t6020 as example even though they will be using the same driver
> code as "apple,nvme-ans2". Using t6020 as an example I planned to submit
> today.
> This will require adding the new fallback "apple,t8103-nvme-ans2"
> compatible string to the driver.
> 
> Asking for clarification as I could image such driver changes will raise
> questions from the driver maintainers.


I don't think they will question it. :) Just mention briefly in commit
msg of the binding reasons why style is different.


> 
> Is there a way do document/annotate the generic compatibles as
> deprecated / "do not use" for new devices?

You can deprecate existing compatibles ("deprecated: true"), but I think
that's not what is going to happen here and there is no specific syntax
to annotate that "do not grow this list, that's old style of doing things".

You can always add a comment like this. See also:
Documentation/devicetree/bindings/iommu/arm,smmu.yaml

(but this uses both comment and deprecated)

> 
> Thanks
> Janne


Best regards,
Krzysztof

Re: [PATCH v2 3/9] dt-bindings: iommu: apple,sart: Add Apple A11

[Krzysztof Kozlowski]

On 21/08/2025 00:20, Rob Herring wrote:
> On Mon, Aug 18, 2025 at 04:42:56PM +0800, Nick Chan wrote:
>> Add bindings for SARTv0 as found on Apple A11 SoC.
>>
>> Reviewed-by: Sven Peter <sven@kernel.org>
>> Signed-off-by: Nick Chan <towinchenmi@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/iommu/apple,sart.yaml | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/apple,sart.yaml b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
>> index e87c1520fea60a2de549aa8a469fcded52e3b6e3..95d55ec688779bd52b8aef316a4c1ba7b7449574 100644
>> --- a/Documentation/devicetree/bindings/iommu/apple,sart.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/apple,sart.yaml
>> @@ -29,6 +29,7 @@ description:
>>  properties:
>>    compatible:
>>      oneOf:
>> +      - const: apple,t8015-sart
> 
> Actually I take back my ack, move this to the existing 'enum' down 
> below. With that change, you can keep the ack.

What commit fails to explain (folks, please fix that) is that device is
incompatible with existing ones. Right?

Commit should describe the hardware or any unusual things here. Not
making devices compatible, while usually they were compatible, is the
unusual thing.
Best regards,
Krzysztof

Re: [PATCH v2 3/9] dt-bindings: iommu: apple,sart: Add Apple A11

[Sven Peter]

On 27.08.25 15:20, Krzysztof Kozlowski wrote:
> 
> What commit fails to explain (folks, please fix that) is that device is
> incompatible with existing ones. Right?

I've already applied this [1] but I can still rebase and change the 
commit message to "Add apple,t8015-sart for SARTv0 found on the Apple 
A11 SoC, which uses a different MMIO layout and is thus incompatible 
with the previously described versions".


Sven


[1] 
https://lore.kernel.org/all/175593408419.53541.12520438370584715457.b4-ty@kernel.org/