On Wed, Jun 12, 2024 at 11:05:38AM +0800, Slark Xiao wrote: > > 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? > There is a build dependency with the MHI patch. So I'll just take both patches through MHI tree once I get an ACK from WWAN maintainers. > >>> 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. > Just send v2 of both patches. There are some comments in the MHI patch as well. > 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? > >>> Yeah, just use link_id directly as it will be 0 by default. - Mani -- மணிவண்ணன் சதாசிவம்
