At 2024-06-12 06:46:33, "Sergey Ryazanov" <ryazanov.s.a@xxxxxxxxx> wrote: >On 11.06.2024 04:36, Slark Xiao wrote: >> +More maintainer to this second patch list. >> >> At 2024-06-08 06:28:48, "Sergey Ryazanov" <ryazanov.s.a@xxxxxxxxx> wrote: >>> Hello Slark, >>> >>> without the first patch it is close to impossible to understand this >>> one. Next time please send such tightly connected patches to both >>> mailing lists. >>> >> Sorry for this mistake since it's my first commit about committing code to 2 >> difference area: mhi and mbim. Both the maintainers are difference. >> In case a new version commit would be created, I would like to ask if >> should I add both side maintainers on these 2 patches ? > >No worries. We finally got both sides of the puzzle. BTW, looks like the >first patch still lacks Linux netdev mailing list in the CC. > >Usually maintainers are responsible for applying patches to their >dedicated repositories (trees), and then eventually for sending them in >batch to the main tree. So, if a work consists of two patches, it is >better to apply them together to one of the trees. Otherwise, it can >cause a build failure in one tree due to lack of required changes that >have been applied to other. Sometimes contributors even specify a >preferred tree in a cover letter. However, it is still up to maintainers >to make a decision which tree is better when a work changes several >subsystems. > Thanks for your detailed explanation. Since this change was modified mainly on mhi side, I prefer to commit it to mhi side. @loic @mani, what's your opinion? >>> On 07.06.2024 13:03, 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. >>>> >>>> Signed-off-by: Slark Xiao <slark_xiao@xxxxxxx> >>> >>> Since it a but fix, it needs a 'Fixes:' tag. >>> >> Actually, I thought it's a fix for common SDX72 product. But now I think >> it should be only meet for my SDX72 MBIM product. Previous commit >> has not been applied. So there is no commit id for "Fixes". >> But I think I shall include that patch in V2 version. >> Please ref: >> https://lore.kernel.org/lkml/20240520070633.308913-1-slark_xiao@xxxxxxx/ > >There are nothing to fix yet. Great. Then you can resend the Foxconn >SDX72 introduction work as a series that also includes these mux id >changes. Just rename this specific patch to something less terrifying. >Mean, remove the "Fix" word from the subject, please. > >Looks like "net: wwan: mhi: make default data link id configurable" >subject also summarize the reason of the change. > Currently I don't know if my previous commit which has been reviewed still be effective. Since this link_id changes only works for MBIM mode of SDX72. If keeps the commit of [1], then I will update this patch with v2 version which just update the subject . If not, then this SDX72 series would have 3 patches: [1] + first patch + second patch[v2](or 2 patches: combine [1] with first patch + second patch[v2]). Please let me know which solution would be better. Thanks. >>>> --- >>>> 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..4ca5c845394b 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 ? mhi_dev->mhi_cntrl->link_id : 0); >>> >>> Is it possible to drop the ternary operator and pass the link_id directly? >>> >>>> } >>>> >>>> static void mhi_mbim_remove(struct mhi_device *mhi_dev) > >-- >Sergey [1] - https://lore.kernel.org/lkml/20240520070633.308913-1-slark_xiao@xxxxxxx/