Re: [PATCH v4 7/9] bus: mhi: Capture DDR training data using command mode

[Kishore Batta <kishore.batta@oss.qualcomm.com>]

On 4/10/2026 2:57 AM, Jeff Hugo wrote:
> On 3/19/2026 12:31 AM, Kishore Batta wrote:
>> During early boot, devices may perform DDR training and produce training
>> data that can be reused on subsequent boots to reduce initialization
>> time. The sahara protocol provides a command mode flow to transfer this
>
> Sahara

ACK. Will correct it.
>
>> training data to the host, but the driver currently does not handle
>> command mode and drops the training payload.
>>
>> Add Sahara command mode support to retrieve DDR training data from the
>> device. When the device enters command mode and sends CMD_READY, query
>> the support command list and request DDR training data using EXECUTE and
>> EXECUTE_DATA. Allocate receive buffers based on the reported response
>> size and copy the raw payload directly from the MHI DL completion
>> callback.
>>
>> Store the captured training data in controller-scoped memory using 
>> devres,
>> so it remains available after sahara channel teardown. Also distinguish
>
> Sahara

ACK. Will correct it.
>
>> raw payload completion from control packets in the DL callback, avoiding
>> misinterpretation of training data as protocol messages, and requeue
>> the RX buffer after switching back to IMAGE_TX_PENDING to allow the
>> boot flow to continue.
>>
>> Signed-off-by: Kishore Batta <kishore.batta@oss.qualcomm.com>
>> ---
>>   drivers/bus/mhi/sahara/sahara.c | 328 
>> +++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 320 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/bus/mhi/sahara/sahara.c 
>> b/drivers/bus/mhi/sahara/sahara.c
>> index 
>> 0a0f578aaa47ab2c4ca0765666b392fb9936ddd5..c88f1220199ac4373d3552167870c19a0d5f23b9 
>> 100644
>> --- a/drivers/bus/mhi/sahara/sahara.c
>> +++ b/drivers/bus/mhi/sahara/sahara.c
>> @@ -5,11 +5,14 @@
>>    */
>>     #include <linux/devcoredump.h>
>> +#include <linux/device.h>
>> +#include <linux/device/devres.h>
>>   #include <linux/firmware.h>
>>   #include <linux/limits.h>
>>   #include <linux/mhi.h>
>>   #include <linux/minmax.h>
>>   #include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>>   #include <linux/overflow.h>
>>   #include <linux/sahara.h>
>>   #include <linux/types.h>
>> @@ -60,8 +63,16 @@
>>   #define SAHARA_RESET_LENGTH        0x8
>>   #define SAHARA_MEM_DEBUG64_LENGTH    0x18
>>   #define SAHARA_MEM_READ64_LENGTH    0x18
>> -
>> +#define SAHARA_COMMAND_READY_LENGTH    0x8
>> +#define SAHARA_COMMAND_EXEC_RESP_LENGTH    0x10
>> +#define SAHARA_COMMAND_EXECUTE_LENGTH    0xc
>> +#define SAHARA_COMMAND_EXEC_DATA_LENGTH    0xc
>> +#define SAHARA_SWITCH_MODE_LENGTH    0xc
>> +
>> +#define SAHARA_EXEC_CMD_GET_COMMAND_ID_LIST    0x8
>> +#define SAHARA_EXEC_CMD_GET_TRAINING_DATA    0x9
>>   #define SAHARA_DDR_TRAINING_IMG_ID    34
>
> Why is the indentation of this line messed up?

ACK. Will correct it.
>
>> +#define SAHARA_NUM_CMD_BUF SAHARA_NUM_TX_BUF
>>     struct sahara_packet {
>>       __le32 cmd;
>> @@ -97,6 +108,19 @@ struct sahara_packet {
>>               __le64 memory_address;
>>               __le64 memory_length;
>>           } memory_read64;
>> +        struct {
>> +            __le32 client_command;
>> +        } command_execute;
>> +        struct {
>> +            __le32 client_command;
>> +            __le32 response_length;
>> +        } command_execute_resp;
>> +        struct {
>> +            __le32 client_command;
>> +        } command_exec_data;
>> +        struct {
>> +            __le32 mode;
>> +        } mode_switch;
>>       };
>>   };
>>   @@ -163,6 +187,7 @@ struct sahara_context {
>>       struct work_struct        fw_work;
>>       struct work_struct        dump_work;
>>       struct work_struct        read_data_work;
>> +    struct work_struct        cmd_work;
>>       struct mhi_device        *mhi_dev;
>>       const char * const        *image_table;
>>       u32                table_size;
>> @@ -183,6 +208,24 @@ struct sahara_context {
>>       bool                is_mem_dump_mode;
>>       bool                non_streaming;
>>       const char            *fw_folder;
>> +    bool                is_cmd_mode;
>> +    bool                receiving_trng_data;
>
> You already spell out "receiving", spell out "training".  I don't 
> recall seeing "trng" before so it seems like a really uncommon shortform.


ACK. Will correct it.

>
>> +    size_t                trng_size;
>> +    size_t                trng_rcvd;
>> +    u32                trng_nbuf;
>> +    char                *cmd_buff[SAHARA_NUM_CMD_BUF];
>> +};