[External Mail] Re: [PATCH v6 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports

[Wu. JackBB (GSM)]

On Mon, 17 Aug 2026 at 22:36, Jakub Kicinski <kuba at 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