netdev - Re:Re: [PATCH v2 2/2] net: wwan: mhi: make default data link id configurable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <269323e2.35a6.19038c60d00.Coremail.slark_xiao@163.com>
Date: Fri, 21 Jun 2024 11:10:21 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Sergey Ryazanov" <ryazanov.s.a@...il.com>
Cc: manivannan.sadhasivam@...aro.org, loic.poulain@...aro.org, 
	johannes@...solutions.net, quic_jhugo@...cinc.com, 
	netdev@...r.kernel.org, mhi@...ts.linux.dev, 
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re:Re: [PATCH v2 2/2] net: wwan: mhi: make default data link id
 configurable










At 2024-06-13 05:54:03, "Sergey Ryazanov" <ryazanov.s.a@...il.com> wrote:
>Hello Slark, Manivannan,
>
>On 12.06.2024 12:39, Slark Xiao wrote:
>> For SDX72 MBIM device, 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 link id default value for these SDX72 products which
>> works in MBIM mode.
>
>The patch itself looks good to me except a tiny nitpick (see below). 
>Meanwhile, I can not understand when we should merge it. During the V1 
>discussion, It was mentioned that we need this change specifically for 
>Foxconn SDX72 modem. Without any actual users the configurable default 
>data link id is a dead code.
>
>According to the ARM MSM patchwork [1], the main Foxconn SDX72 
>introducing patch is (a) not yet merged, (b) no more applicable. So, as 
>far as I understand, it should be resend. In this context, a best way to 
>merge the modem support is to prepend the modem introduction patch with 
>these changes forming a series:
>1/3: bus: mhi: host: Import mux_id item
>2/3: net: wwan: mhi: make default data link id configurable
>3/3: bus: mhi: host: Add Foxconn SDX72 related support
>
>And merge the series as whole, when everything will be ready. This will 
>help us to avoid partially merged work and will keep the modem support 
>introduction clear.
>

Yes, currently these 3 patches would be merged by Mani at the same time.
So I think there is no build failure risk.

>Manivannan, could you share the main [1] Foxconn SDX72 introduction 
>patch status, and your thoughts regarding the merging process?

We were discussing another patch in last weeks. And we still have not
reached a consensus. Let's focus on that patch firstly.
And Mani, please let us know about the merging process since the new
merge window is open or will open soon?

Thanks
>
>
>1. 
>https://patchwork.kernel.org/project/linux-arm-msm/patch/20240520070633.308913-1-slark_xiao@163.com/
>
>> Signed-off-by: Slark Xiao <slark_xiao@....com>
>> ---
>>   drivers/net/wwan/mhi_wwan_mbim.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/wwan/mhi_wwan_mbim.c b/drivers/net/wwan/mhi_wwan_mbim.c
>> index 3f72ae943b29..c731fe20814f 100644
>> --- a/drivers/net/wwan/mhi_wwan_mbim.c
>> +++ b/drivers/net/wwan/mhi_wwan_mbim.c
>> @@ -618,7 +618,8 @@ static int mhi_mbim_probe(struct mhi_device *mhi_dev, const struct mhi_device_id
>>   	mbim->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
>>   
>>   	/* Register wwan link ops with MHI controller representing WWAN instance */
>> -	return wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_mbim_wwan_ops, mbim, 0);
>> +	return wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_mbim_wwan_ops, mbim,
>> +		mhi_dev->mhi_cntrl->link_id);
>
>Just a nitpick. The second line had better be aligned with the opening 
>bracket:
>
>return wwan_register_ops(&cntrl->...
>                          mhi_dev->...
>
>>   }
>>   
>>   static void mhi_mbim_remove(struct mhi_device *mhi_dev)
>
>--
>Sergey