Received-SPF: pass (google.com: domain of linux-kernel+bounces-211658-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <87aecf24-cdbb-70d2-a3d1-8d1cacf18401@quicinc.com>
Date: Wed, 12 Jun 2024 08:19:13 -0600
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH v2 1/2] bus: mhi: host: Import mux_id item
Content-Language: en-US
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Slark Xiao
	<slark_xiao@163.com>
CC: <loic.poulain@linaro.org>, <ryazanov.s.a@gmail.com>,
        <johannes@sipsolutions.net>, <netdev@vger.kernel.org>,
        <mhi@lists.linux.dev>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20240612093842.359805-1-slark_xiao@163.com>
 <20240612094609.GA58302@thinkpad>
From: Jeffrey Hugo <quic_jhugo@quicinc.com>
In-Reply-To: <20240612094609.GA58302@thinkpad>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 6/12/2024 3:46 AM, Manivannan Sadhasivam wrote:
> On Wed, Jun 12, 2024 at 05:38:42PM +0800, Slark Xiao wrote:
> 
> Subject could be improved:
> 
> bus: mhi: host: Add configurable mux_id for MBIM mode
> 
>> For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
>> This would lead to device can't ping outside successfully.
>> Also MBIM side would report "bad packet session (112)".
>> So we add a default mux_id value for SDX72. And this value
>> would be transferred to wwan mbim side.
>>
>> Signed-off-by: Slark Xiao <slark_xiao@163.com>
>> ---
>>   drivers/bus/mhi/host/pci_generic.c | 3 +++
>>   include/linux/mhi.h                | 2 ++
>>   2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> index 0b483c7c76a1..9e9adf8320d2 100644
>> --- a/drivers/bus/mhi/host/pci_generic.c
>> +++ b/drivers/bus/mhi/host/pci_generic.c
>> @@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
>>   	unsigned int dma_data_width;
>>   	unsigned int mru_default;
>>   	bool sideband_wake;
>> +	unsigned int mux_id;
>>   };
>>   
>>   #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
>> @@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>   	.dma_data_width = 32,
>>   	.mru_default = 32768,
>>   	.sideband_wake = false,
>> +	.mux_id = 112,
>>   };
>>   
>>   static const struct mhi_channel_config mhi_mv3x_channels[] = {
>> @@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>   	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
>>   	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
>>   	mhi_cntrl->mru = info->mru_default;
>> +	mhi_cntrl->link_id = info->mux_id;
> 
> Again, 'link_id' is just a WWAN term. Use 'mux_id' here also.

Does this really belong in MHI?  If this was DT, I don't think we would 
put this value in DT, but rather have the driver (MBIM) detect the 
device and code in the required value.

Furthermore, if this is included in MHI, it seems to be a property of 
the channel, and not the controller.

-Jeff