Message-ID: <8279536d-c4f4-477a-9823-3e7b452c3c33@kernel.org>
Date: Thu, 21 Aug 2025 12:25:13 +0200
From: Sven Peter <sven@...nel.org>
To: Krzysztof Kozlowski <krzk@...nel.org>, Nick Chan <towinchenmi@...il.com>
Cc: Janne Grunau <j@...nau.net>, Alyssa Rosenzweig <alyssa@...enzweig.io>,
 Neal Gompa <neal@...pa.dev>, Jassi Brar <jassisinghbrar@...il.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Hector Martin <marcan@...can.st>,
 Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
 Robin Murphy <robin.murphy@....com>, Keith Busch <kbusch@...nel.org>,
 Jens Axboe <axboe@...nel.dk>, Christoph Hellwig <hch@....de>,
 Sagi Grimberg <sagi@...mberg.me>, asahi@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, iommu@...ts.linux.dev,
 linux-nvme@...ts.infradead.org
Subject: Re: [PATCH v2 6/9] dt-bindings: nvme: apple,nvme-ans: Add Apple A11

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@...il.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

Powered by blists - more mailing lists