Re: [PATCH v1 2/2] net: wwan: Fix SDX72 ping failure issue

From: Sergey Ryazanov
Date: Tue Jun 11 2024 - 18:46:15 EST


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.

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.

---
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