All of lore.kernel.org
 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: 27+ 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
2026-08-11  7:14 ` Jack Wu via B4 Relay
2026-08-11  7:14 ` [PATCH v6 1/7] net: wwan: t9xx: Add PCIe core Jack Wu
2026-08-11  7:14   ` 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
2026-08-11  7:14   ` 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
2026-08-11  7:14   ` 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
2026-08-11  7:14   ` Jack Wu via B4 Relay
2026-08-11  7:14 ` [PATCH v6 5/7] net: wwan: t9xx: Add FSM thread Jack Wu
2026-08-11  7:14   ` 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
2026-08-11  7:14   ` 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
2026-08-11  7:14   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.