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

[Nick Chan <towinchenmi@gmail.com>]

On 17/8/2025 18:47, Sven Peter wrote:
> On 11.08.25 15:50, Nick Chan wrote:
>> 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 | 228 +++++++++++++++++++++++++++++++---------------
> 
> [...]
> 
>>   }
>>   
>>   static void apple_nvme_rtkit_crashed(void *cookie, const void *crashlog, size_t crashlog_size)
>> @@ -284,21 +294,8 @@ static void apple_nvme_submit_cmd(struct apple_nvme_queue *q,
>>   				  struct nvme_command *cmd)
>>   {
> 
> Please just create a separate submit function here.
> There's just not much code that's shared between the two variants.
Will do in v2.

> 
> [...]
> 
>>   }
>>   
>> @@ -587,10 +618,17 @@ static inline void apple_nvme_handle_cqe(struct apple_nvme_queue *q,
>>   {
>>   	struct apple_nvme *anv = queue_to_apple_nvme(q);
>>   	struct nvme_completion *cqe = &q->cqes[idx];
>> -	__u16 command_id = READ_ONCE(cqe->command_id);
>>   	struct request *req;
>>   
>> -	apple_nvmmu_inval(q, command_id);
>> +	if (!anv->hw->has_lsq_nvmmu)
>> +		cqe->command_id--;
>> +
>> +	__u16 command_id = READ_ONCE(cqe->command_id);
>> +
>> +	if (anv->hw->has_lsq_nvmmu)
>> +		apple_nvmmu_inval(q, command_id);
>> +	else
>> +		command_id++;
> 
> This entire block here looks weird. First you decrease the command_id
> directly inside the shared memory structure, then you read it with
> READ_ONCE to a local variable only to increase it again. Why?
Thanks for spotting! Looks like this is merely an artifact of how the code
is hacked to work that slipped under my radar, so I will remove the useless
codes in v2.

> 
> 
> 
> Sven

Nick Chan