Re: [PATCH 17/17] i3c: mipi-i3c-hci: Add HDR-DDR support

[Adrian Hunter <adrian.hunter@intel.com>]

On 14/09/2026 14:54, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The HDR Command Code's RNW bit (bit 7) is incorrectly masked out when constructing the transfer descriptor, causing HDR read commands to be sent as write commands on the bus.
> --
> 
> commit f1f61dea5df0e3000b1b519aa78a3886fbb61942
> Author: Adrian Hunter <adrian.hunter@intel.com>
> Subject: i3c: mipi-i3c-hci: Add HDR-DDR support
> 
> This patch adds support for HDR-DDR private transfers in the MIPI I3C HCI
> driver. It encodes HDR-DDR Command Descriptors, advertises HDR-DDR capability
> when supported by the controller, and enforces HDR-DDR transfer constraints.
> 
>> diff --git a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
>> index 9910df53c013d..78ad2ec2bd936 100644
>> --- a/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
>> +++ b/drivers/i3c/master/mipi-i3c-hci/cmd_v1.c
>> @@ -215,14 +215,30 @@ static int hci_cmd_v1_prep_ccc(struct i3c_hci *hci,
>>  
>>  static void hci_cmd_v1_prep_i3c_xfer(struct i3c_hci *hci,
>>  				     struct i3c_dev_desc *dev,
>> -				     struct hci_xfer *xfer)
>> +				     struct hci_xfer *xfer,
>> +				     enum i3c_xfer_mode xfer_mode)
>>  {
>>  	struct i3c_hci_dev_data *dev_data = i3c_dev_get_master_data(dev);
>>  	unsigned int dat_idx = dev_data->dat_idx;
>> -	enum hci_cmd_mode mode = get_i3c_mode(hci);
>> +	enum hci_cmd_mode mode;
>>  	u8 *data = xfer->data;
>>  	unsigned int data_len = xfer->data_len;
>>  	bool rnw = xfer->rnw;
>> +	u32 cp_cmd = 0;
>> +
>> +	if (xfer_mode == I3C_SDR) {
>> +		mode = get_i3c_mode(hci);
>> +	} else {
>> +		/*
>> +		 * HDR-DDR is the only advertised HDR Mode, so a non-SDR
>> +		 * transfer uses the HDR-DDR Command Code encoding: CP marks CMD
>> +		 * as valid, CMD carries bits[6:0], and RNW carries bit[7]. CP
>> +		 * and CMD occupy the same descriptor bits in both command
>> +		 * formats.
>> +		 */
>> +		mode = MODE_I3C_HDR_DDR;
>> +		cp_cmd = CMD_R0_CP | CMD_R0_CMD(xfer->hdr_cmd & I3C_HDR_CMD_CODE);
>                 ^^^^^^^^
> 
> [Severity: High]
> Does masking xfer->hdr_cmd with I3C_HDR_CMD_CODE unintentionally strip the RNW
> bit (bit 7) from the 8-bit HDR Command Code before it is placed into the
> descriptor?

No - it is intentional.

> 
> Since CMD_R0_CMD occupies an 8-bit field (bits 14:7) in the descriptor and
> xfer->hdr_cmd is masked with 0x7F, bit 14 of the descriptor will always be
> forced to 0.
> 
> If the hardware sends the 8-bit CMD field exactly as provided onto the bus
> rather than automatically merging in CMD_R0_RNW (bit 29), will this cause HDR
> read transfers to be transmitted as write commands? This could lead to bus
> protocol errors or timeouts, as the target device will expect to receive data
> instead of providing it.
> 
>>  	}
>>  
>>  	xfer->cmd_tid = hci_get_tid();
> 


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c