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

Nick Chan posted 9 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11
Posted by Nick Chan 1 month, 2 weeks ago
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
Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11
Posted by Krzysztof Kozlowski 1 month, 2 weeks ago
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
Posted by Sven Peter 1 month, 2 weeks ago
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
Posted by Krzysztof Kozlowski 1 month, 2 weeks ago
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 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11
Posted by Sven Peter 1 month, 1 week ago
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
Posted by Krzysztof Kozlowski 1 month, 1 week ago
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
Posted by Janne Grunau 1 month, 1 week ago
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
Posted by Krzysztof Kozlowski 1 month, 1 week ago
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 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11
Posted by Sven Peter 1 month, 1 week ago
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