| Message ID | 20240612093941.359904-1-slark_xiao@163.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/2] bus: mhi: host: Import mux_id item | expand |
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@163.com/ > Signed-off-by: Slark Xiao <slark_xiao@163.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
At 2024-06-13 05:54:03, "Sergey Ryazanov" <ryazanov.s.a@gmail.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@163.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
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); } static void mhi_mbim_remove(struct mhi_device *mhi_dev)
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. Signed-off-by: Slark Xiao <slark_xiao@163.com> --- drivers/net/wwan/mhi_wwan_mbim.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)