Netdev List
 help / color / mirror / Atom feed
From: "Wu. JackBB (GSM)" <JackBB_Wu@compal.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "loic.poulain@oss.qualcomm.com" <loic.poulain@oss.qualcomm.com>,
	"ryazanov.s.a@gmail.com" <ryazanov.s.a@gmail.com>,
	"johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"wen-zhi.huang@mediatek.com" <wen-zhi.huang@mediatek.com>,
	"shi-wei.yeh@mediatek.com" <shi-wei.yeh@mediatek.com>,
	"Minano.tseng@mediatek.com" <Minano.tseng@mediatek.com>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"angelogioacchino.delregno@collabora.com"
	<angelogioacchino.delregno@collabora.com>,
	"horms@kernel.org" <horms@kernel.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"skhan@linuxfoundation.org" <skhan@linuxfoundation.org>,
	"Yu. Robert (GSM)" <robert_yu@compal.com>,
	"Chang. Jeff (GSM)" <Jeff_Chang@compal.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: RE: [External Mail] Re: [PATCH v6 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports
Date: Tue, 18 Aug 2026 12:41:12 +0000	[thread overview]
Message-ID: <c2cba1d9cf9f454bad9a72fee67ce2ed@compal.com> (raw)
In-Reply-To: <20260817223658.3688816-1-kuba@kernel.org>

On Mon, 17 Aug 2026 at 22:36, Jakub Kicinski <kuba@kernel.org> wrote:
> [Severity: Low]
> Should the error from mtk_port_enable_by_type() be handled or at least
> logged here?

Will fix in v7. Will add dev_err logging when
mtk_port_enable_by_type() returns an error.

> [Severity: Medium]
> Can a short write be silently reported to userspace as a complete write
> here (and in mtk_port_wwan_write_blocking() below)?
...
> Both tx callbacks only test ret < 0, then consume the skb and return 0.

Will fix in v7. Will treat a positive return smaller than skb->len
as an error (return -EIO) in both mtk_port_wwan_write() and
mtk_port_wwan_write_blocking().

> [Severity: Medium]
> Does this early return need a compensating mtk_port_ch_disable(), the way
> the wwan_create_port() failure path below does?
...
> queue not allocated and returns -EINVAL.  Would that leave the AT/MBIM
> channel permanently unusable for the lifetime of the device?

Will fix in v7. Will call mtk_port_ch_disable() on the error path
when mtk_port_ch_enable() fails, to clean up usr_cnt and any
partially allocated queue resources.

> [Severity: Medium]
> Can the RX path dereference the ERR_PTR that is stored into
> port->w_priv.w_port before it is validated?
...
> would using a local variable and only publishing the validated pointer
> under w_lock be more consistent here?

Will fix in v7. Will use a local variable for wwan_create_port()
result and only publish the validated pointer to w_priv.w_port
under w_lock, consistent with mtk_port_wwan_disable().

> [Severity: Low]
> This isn't a bug, but would passing a wwan_port_caps here be preferable to
> NULL?
...
> frag_len to the tx MTU and headroom_len to sizeof(struct mtk_ccci_header),
> as t7xx_port_wwan.c does, would bound that allocation and drop the extra
> copy.

Will fix in v7. Will pass wwan_port_caps with frag_len set to
port->info.tx_mtu and headroom_len to sizeof(struct mtk_ccci_header),
matching the t7xx approach.

> [Severity: Low]
> Should these bits be set before wwan_create_port() rather than after?
...
> such as ModemManager can open() the node before these set_bit() calls run.

Will fix in v7. Will move set_bit(PORT_S_WR) and set_bit(PORT_S_ENABLE)
before wwan_create_port(), and clear them on failure.

Thanks.

--
Jack

  reply	other threads:[~2026-08-18 12:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11  7:14 [PATCH v6 0/7] net: wwan: t9xx: Add MediaTek T9XX WWAN driver Jack Wu via B4 Relay
2026-08-11  7:14 ` [PATCH v6 1/7] net: wwan: t9xx: Add PCIe core Jack Wu via B4 Relay
2026-08-17 22:35   ` Jakub Kicinski
2026-08-11  7:14 ` [PATCH v6 2/7] net: wwan: t9xx: Add control plane transaction layer Jack Wu via B4 Relay
2026-08-17 22:36   ` Jakub Kicinski
2026-08-18 12:22     ` [External Mail] " Wu. JackBB (GSM)
2026-08-11  7:14 ` [PATCH v6 3/7] net: wwan: t9xx: Add control DMA interface Jack Wu via B4 Relay
2026-08-17 22:36   ` Jakub Kicinski
2026-08-18 12:24     ` [External Mail] " Wu. JackBB (GSM)
2026-08-11  7:14 ` [PATCH v6 4/7] net: wwan: t9xx: Add control port Jack Wu via B4 Relay
2026-08-11  7:14 ` [PATCH v6 5/7] net: wwan: t9xx: Add FSM thread Jack Wu via B4 Relay
2026-08-17 22:36   ` Jakub Kicinski
2026-08-11  7:14 ` [PATCH v6 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports Jack Wu via B4 Relay
2026-08-17 22:36   ` Jakub Kicinski
2026-08-18 12:41     ` Wu. JackBB (GSM) [this message]
2026-08-11  7:14 ` [PATCH v6 7/7] net: wwan: t9xx: Add maintainers entry Jack Wu via B4 Relay
2026-08-17 22:38   ` Jakub Kicinski
2026-08-17 22:39 ` [PATCH v6 0/7] net: wwan: t9xx: Add MediaTek T9XX WWAN driver Jakub Kicinski
2026-08-18 12:20   ` [External Mail] " Wu. JackBB (GSM)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c2cba1d9cf9f454bad9a72fee67ce2ed@compal.com \
    --to=jackbb_wu@compal.com \
    --cc=Jeff_Chang@compal.com \
    --cc=Minano.tseng@mediatek.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=loic.poulain@oss.qualcomm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=robert_yu@compal.com \
    --cc=ryazanov.s.a@gmail.com \
    --cc=shi-wei.yeh@mediatek.com \
    --cc=skhan@linuxfoundation.org \
    --cc=wen-zhi.huang@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox