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.
Manivannan, could you share the main [1] Foxconn SDX72 introduction
patch status, and your thoughts regarding the merging process?
1.
https://patchwork.kernel.org/project/linux-arm-msm/patch/20240520070633.308913-1-slark_xiao@xxxxxxx/
Signed-off-by: Slark Xiao <slark_xiao@xxxxxxx>
---
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
