* Re: [PATCH v4 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-6-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [6]: [PATCH v4 6/7] net: wwan: t9xx: Add AT & MBIM WWAN ports ---
commit 1739a55610bf7a44ce24874819e01c07236d9442
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add AT & MBIM WWAN ports
Add AT & MBIM ports to the port infrastructure.
The WWAN initialization method is responsible for creating the
corresponding ports using the WWAN framework infrastructure. The
implemented WWAN port operations are start, stop, tx, tx_blocking
and tx_poll.
> diff --git a/drivers/net/wwan/t9xx/mtk_port_io.c b/drivers/net/wwan/t9xx/mtk_port_io.c
> index e3a2de6d2f297..882254b74026f 100644
> --- a/drivers/net/wwan/t9xx/mtk_port_io.c
> +++ b/drivers/net/wwan/t9xx/mtk_port_io.c
[ ... ]
> @@ -234,6 +376,198 @@ static const struct port_ops port_internal_ops = {
> .recv = mtk_port_internal_recv,
> };
>
> +static int mtk_port_wwan_open(struct wwan_port *w_port)
[ ... ]
> +static int mtk_port_wwan_write(struct wwan_port *w_port, struct sk_buff *skb)
> +{
> + struct mtk_port *port = wwan_port_get_drvdata(w_port);
> + union user_buf user_buf;
> + int ret;
> +
> + if (unlikely(!skb->len)) {
> + consume_skb(skb);
> + return 0;
> + }
> +
> + port->info.flags &= ~PORT_F_BLOCKING;
> + user_buf.kbuf = (void *)skb->data;
> + ret = mtk_port_common_write(port, user_buf, skb->len, false);
> + if (ret < 0)
> + return ret;
[Severity: High]
If mtk_port_common_write() successfully sends the first few fragments but
fails on subsequent ones, returning a positive partial write count, does
this code silently drop the remaining data?
The condition ret < 0 is false for partial writes, so the SKB is consumed
and 0 is returned to the caller, falsely reporting complete success to the
WWAN core.
> +
> + consume_skb(skb);
> + return 0;
> +}
> +
> +static int mtk_port_wwan_write_blocking(struct wwan_port *w_port, struct sk_buff *skb)
> +{
> + struct mtk_port *port = wwan_port_get_drvdata(w_port);
> + union user_buf user_buf;
> + int ret;
> +
> + if (unlikely(!skb->len)) {
> + consume_skb(skb);
> + return 0;
> + }
> +
> + port->info.flags |= PORT_F_BLOCKING;
> + user_buf.kbuf = (void *)skb->data;
> + ret = mtk_port_common_write(port, user_buf, skb->len, false);
> + if (ret < 0)
> + return ret;
[Severity: High]
Can the same silent drop of partially written SKBs happen here?
> +
> + consume_skb(skb);
> + return 0;
> +}
[ ... ]
> +static void mtk_port_wwan_enable(struct mtk_port *port)
> +{
> + struct mtk_port_mngr *port_mngr;
> + int ret;
> +
> + port_mngr = port->port_mngr;
> +
> + if (test_bit(PORT_S_ENABLE, &port->status))
> + return;
> +
> + ret = mtk_port_ch_enable(port);
> + if (ret && ret != -EBUSY)
> + return;
> +
> + port->w_priv.w_port = wwan_create_port(port_mngr->ctrl_blk->mdev->dev,
> + port->w_priv.w_type,
> + &wwan_ops, NULL, port);
> + if (IS_ERR(port->w_priv.w_port)) {
[Severity: High]
Can a race condition occur here if wwan_create_port() fails and returns an
error pointer?
Once mtk_port_ch_enable() completes, hardware RX can start. If
wwan_create_port() fails, an ERR_PTR is stored in port->w_priv.w_port
without holding w_lock. Concurrently, the RX interrupt handler could call
mtk_port_wwan_recv():
mutex_lock(&port->w_priv.w_lock);
if (!port->w_priv.w_port) {
mutex_unlock(&port->w_priv.w_lock);
return -ENXIO;
}
wwan_port_rx(port->w_priv.w_port, skb);
Because an ERR_PTR is not NULL, mtk_port_wwan_recv() will pass it to
wwan_port_rx(), which will then dereference the error pointer.
> + dev_warn(port_mngr->ctrl_blk->mdev->dev,
> + "Failed to create wwan port for (%s)\n", port->info.name);
> + port->w_priv.w_port = NULL;
> + mtk_port_ch_disable(port);
> + return;
> + }
> +
> + set_bit(PORT_S_WR, &port->status);
> + set_bit(PORT_S_ENABLE, &port->status);
> +}
> +
^ permalink raw reply
* Re: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-5-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [5]: [PATCH v4 5/7] net: wwan: t9xx: Add FSM thread ---
commit 8cf81b126e77414e72ef40886e0728c165b2e588
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add FSM thread
This commit introduces a finite-state machine (FSM) thread for the MTK WWAN
driver to synchronize module actions. It handles events sequentially from a
queue and notifies other modules before and after state transitions.
> diff --git a/drivers/net/wwan/t9xx/mtk_fsm.c b/drivers/net/wwan/t9xx/mtk_fsm.c
> new file mode 100644
> index 0000000000000..daefb7581ce0f
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_fsm.c
[ ... ]
> +static int mtk_fsm_parse_hs2_msg(struct fsm_hs_info *hs_info)
> +{
> + struct mtk_md_fsm *fsm = container_of(hs_info, struct mtk_md_fsm, hs_info[hs_info->id]);
> + char *rt_data = ((struct sk_buff *)hs_info->rt_data)->data;
[Severity: High]
Does this iterate over the SKB payload assuming the entire payload is linearly
accessible? Networking core guidelines usually require checking boundaries
using pskb_may_pull() before direct dereferencing, as packets might be
fragmented.
[ ... ]
> + if (cur_ft_spt == RTFT_TYPE_MUST_SUPPORT)
> + if (query_rtft_action[ft_id])
> + ret = query_rtft_action[ft_id](fsm->mdev, rtft_entry->data);
> + if (ret < 0)
> + break;
> +
> + data_len = le32_to_cpu(rtft_entry->data_len);
> + if (data_len > hs_info->rt_data_len - offset - sizeof(*rtft_entry))
> + break;
[Severity: High]
Is the rtft_entry->data passed to the action callback (like
mtk_port_status_update) before data_len is checked against the remaining
buffer bounds? This appears to allow an out-of-bounds heap read if a
crafted message has a large port_cnt and truncated payload.
[ ... ]
> +static int mtk_fsm_send_hs3_msg(struct fsm_hs_info *hs_info)
> +{
> + struct mtk_md_fsm *fsm = container_of(hs_info, struct mtk_md_fsm, hs_info[hs_info->id]);
> + unsigned int data_len, msg_size = 0;
> + struct ctrl_msg_header *ctrl_msg_h;
> + struct sk_buff *skb;
> + int ret;
> +
> + skb = __dev_alloc_skb(RTFT_DATA_SIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
[Severity: High]
Does this leak uninitialized kernel heap memory to the WWAN device?
__dev_alloc_skb() does not zero the buffer, and when the structure is
populated in mtk_fsm_append_rtft_entries(), the 2-byte reserved field
(and potentially support_info if the switch falls through) appear to be
left uninitialized.
[ ... ]
> +static int mtk_fsm_sap_ctrl_msg_handler(void *__fsm, struct sk_buff *skb)
> +{
> + struct ctrl_msg_header *ctrl_msg_h;
> + struct mtk_md_fsm *fsm = __fsm;
> + struct fsm_hs_info *hs_info;
> + int ret;
> +
> + if (skb->len < sizeof(*ctrl_msg_h)) {
> + dev_kfree_skb(skb);
> + return -EINVAL;
> + }
> +
> + ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> + skb_pull(skb, sizeof(*ctrl_msg_h));
[Severity: High]
Can skb_pull() trigger a BUG() here if the SKB is fragmented? It seems the
payload is directly accessed and pulled without calling pskb_may_pull()
to guarantee the required bytes are in the linear data region.
[ ... ]
> + hs_info = &fsm->hs_info[HS_ID_SAP];
> + if (le32_to_cpu(ctrl_msg_h->id) != CTRL_MSG_HS2) {
> + dev_kfree_skb(skb);
> + return -EPROTO;
> + }
> +
> + hs_info->rt_data = skb;
> + hs_info->rt_data_len = skb->len;
> + ret = mtk_fsm_evt_submit(fsm->mdev, FSM_EVT_STARTUP,
> + hs_info->fsm_flag_hs2, hs_info, sizeof(*hs_info), 0);
> + if (ret == FSM_EVT_RET_FAIL)
> + dev_kfree_skb(skb);
[Severity: High]
If mtk_fsm_evt_submit() fails, the SKB is freed, but hs_info->rt_data
is not set to NULL. Could this dangling pointer cause a double-free later in
the error path of mtk_fsm_startup_act()?
Also, hs_info->rt_data is assigned without locking here. If consecutive
messages arrive, could this overwrite the pointer and leak the previous SKB?
When the FSM thread eventually parses the first event, it sets the pointer to
NULL, which would cause subsequent queued events to dereference NULL in
mtk_fsm_parse_hs2_msg().
[ ... ]
> +static void mtk_fsm_evt_cleanup(struct mtk_md_fsm *fsm, struct list_head *evtq)
> +{
> + struct mtk_fsm_evt *event, *tmp;
> +
> + list_for_each_entry_safe(event, tmp, evtq, entry) {
> + list_del(&event->entry);
> + mtk_fsm_evt_finish(fsm, event, FSM_EVT_RET_FAIL);
> + }
> +}
[Severity: Medium]
When flushing the event queue on device removal, does this cleanup miss
freeing the SKB currently stored in the shared fsm->hs_info[].rt_data if an
HS2 message was pending?
[ ... ]
> +int mtk_fsm_notifier_register(struct mtk_md_dev *mdev, enum mtk_user_id id,
> + void (*cb)(struct mtk_fsm_param *, void *data),
> + void *data, enum mtk_fsm_prio prio, bool is_pre)
> +{
[ ... ]
> + if (is_pre)
> + mtk_fsm_notifier_insert(notifier, &fsm->pre_notifiers);
> + else
> + mtk_fsm_notifier_insert(notifier, &fsm->post_notifiers);
[Severity: Medium]
Is it safe to modify these notifier lists globally without spinlock or mutex
protection? The FSM kthread traverses these lists in mtk_fsm_switch_state()
and could race with registration or unregistration.
[ ... ]
> +int mtk_fsm_evt_submit(struct mtk_md_dev *mdev,
> + enum mtk_fsm_evt_id id, enum mtk_fsm_flag flag,
> + void *data, unsigned int len, unsigned char mode)
> +{
[ ... ]
> + spin_unlock_irqrestore(&fsm->evtq_lock, flags);
> +
> + wake_up_process(fsm->fsm_handler);
[Severity: High]
Could a race condition occur here during teardown? If this thread is
preempted right after releasing the lock, mtk_fsm_exit() could execute,
stopping the kthread and setting fsm->fsm_handler = NULL. When this thread
resumes, calling wake_up_process(NULL) would trigger a kernel panic.
> diff --git a/drivers/net/wwan/t9xx/mtk_port.c b/drivers/net/wwan/t9xx/mtk_port.c
> --- a/drivers/net/wwan/t9xx/mtk_port.c
> +++ b/drivers/net/wwan/t9xx/mtk_port.c
[ ... ]
> int mtk_port_ch_disable(struct mtk_port *port)
> {
[ ... ]
> start_wait:
> ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> MTK_DFLT_TRB_TIMEOUT);
> if (ret == -ERESTARTSYS)
> goto start_wait;
[Severity: High]
If a signal is pending (e.g., SIGKILL), wait_event_interruptible_timeout()
immediately returns -ERESTARTSYS. Won't this retry loop cause an infinite
busy loop (100% CPU lockup) because the signal remains pending without
sleeping?
[ ... ]
> +static void mtk_port_disable(struct mtk_port_mngr *port_mngr)
> +{
> + struct mtk_port **ports;
> + int tbl_type;
> + int ret, idx;
> +
> + ports = kcalloc(port_mngr->port_cnt, sizeof(struct mtk_port *), GFP_KERNEL);
> + if (!ports)
> + return;
[Severity: High]
If kcalloc() fails under memory pressure, this aborts the disable sequence
silently. Would this leave hardware channels active during device teardown
and cause use-after-free crashes? It might be safer to iterate via
radix_tree_for_each_slot() without dynamic allocation.
^ permalink raw reply
* Re: [PATCH v4 4/7] net: wwan: t9xx: Add control port
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-4-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [4]: [PATCH v4 4/7] net: wwan: t9xx: Add control port ---
commit 8d0ed42502a8e2e3fe15beabcf8bda318350f746
Author: Jack Wu <jackbb_wu@compal.com>
Date: Thu Jul 9 10:53:26 2026 +0000
net: wwan: t9xx: Add control port
The control port consists of port I/O and port manager.
Port I/O provides a common operation as defined by "struct port_ops",
and the operation is managed by the "port manager". It provides
interfaces to internal users, the implemented internal interfaces are
open, close, write and recv_register.
> diff --git a/drivers/net/wwan/t9xx/mtk_port.c b/drivers/net/wwan/t9xx/mtk_port.c
> new file mode 100644
> index 0000000000000..06436422133ba
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_port.c
> @@ -0,0 +1,875 @@
[ ... ]
> +static void mtk_port_free_or_backup(struct mtk_port_mngr *port_mngr,
> + struct mtk_port *port, struct mtk_stale_list *s_list)
> +{
> + mutex_lock(&port_mngr_grp_mtx);
> + mtk_port_tbl_del(port_mngr, port);
> + if (port->info.type != PORT_TYPE_INTERNAL) {
> + if (test_bit(PORT_S_OPEN, &port->status)) {
> + list_add_tail(&port->stale_entry, &s_list->ports);
> + set_bit(PORT_S_ON_STALE_LIST, &port->status);
> + memcpy(port->dev_str, port_mngr->ctrl_blk->mdev->dev_str,
> + MTK_DEV_STR_LEN);
> + port->port_mngr = NULL;
> + }
> + kref_put(&port->kref, mtk_port_release);
> + } else {
> + mtk_port_release(&port->kref);
[Severity: High]
Does this directly bypass the kref API? By explicitly calling
mtk_port_release() rather than kref_put(), this forces the port to be freed
immediately, ignoring any active references internal clients might still hold.
Could this lead to a use-after-free during device removal or module unload?
> + }
> + mutex_unlock(&port_mngr_grp_mtx);
> +}
> +
> +static struct mtk_port *mtk_port_search_by_id(struct mtk_port_mngr *port_mngr, int rx_ch)
> +{
> + int tbl_type = MTK_PORT_TBL_TYPE(rx_ch);
> +
> + if (tbl_type < PORT_TBL_SAP || tbl_type >= PORT_TBL_MAX)
> + return NULL;
> +
> + return radix_tree_lookup(&port_mngr->port_tbl[tbl_type], MTK_CH_ID(rx_ch));
[Severity: High]
Is it safe to perform radix tree lookups without an RCU read-side critical
section?
Since mtk_port_release() uses a synchronous kfree() instead of kfree_rcu(),
if a port closure or device removal occurs concurrently, the traversal here
might access freed radix tree nodes or port structures.
> +}
> +
> +struct mtk_port *mtk_port_search_by_name(struct mtk_port_mngr *port_mngr, char *name)
> +{
> + int tbl_type = PORT_TBL_SAP;
> + struct radix_tree_iter iter;
> + struct mtk_port *port;
> + void __rcu **slot;
> +
> + do {
> + radix_tree_for_each_slot(slot, &port_mngr->port_tbl[tbl_type], &iter, 0) {
[Severity: High]
Similarly, does this radix tree iteration require rcu_read_lock() protection
to prevent traversing into memory that has been concurrently freed?
> + MTK_PORT_SEARCH_FROM_RADIX_TREE(port, slot);
> + MTK_PORT_INTERNAL_NODE_CHECK(port, slot, iter);
> + if (!strncmp(port->info.name, name, MTK_DFLT_PORT_NAME_LEN))
> + return port;
> + }
> + tbl_type++;
> + } while (tbl_type < PORT_TBL_MAX);
> +
> + return NULL;
> +}
[ ... ]
> +void mtk_port_stale_list_grp_cleanup(void)
> +{
> + struct mtk_stale_list *s_list, *next_s_list;
> + struct mtk_port *port, *next_port;
> +
> + mutex_lock(&port_mngr_grp_mtx);
> + list_for_each_entry_safe(s_list, next_s_list, &stale_list_grp, entry) {
> + list_del(&s_list->entry);
> +
> + list_for_each_entry_safe(port, next_port, &s_list->ports, stale_entry) {
> + clear_bit(PORT_S_ON_STALE_LIST, &port->status);
> + mtk_port_release(&port->kref);
[Severity: High]
This appears to be another instance of bypassing the kref API by calling
mtk_port_release() directly instead of kref_put().
> + }
> +
> + kfree(s_list);
[Severity: Medium]
Does this leak the s_list->dev_id? The list entries are freed here during
module unload, but there doesn't appear to be a corresponding call to
ida_free() to return the dev_id to the allocator.
> + }
> + mutex_unlock(&port_mngr_grp_mtx);
> +}
[ ... ]
> +void mtk_port_trb_init(struct mtk_port *port, struct trb *trb, enum mtk_trb_cmd_type cmd,
> + int (*trb_complete)(struct sk_buff *skb))
> +{
> + kref_init(&trb->kref);
> + trb->channel_id = port->info.rx_ch;
> + trb->status = MTK_DFLT_TRB_STATUS;
> + trb->priv = port;
[Severity: High]
Does this assignment need to take a reference to the port with kref_get()?
If a non-blocking transmission is submitted, the port could be closed and
freed while the TRB is still in the hardware DMA ring. When the DMA
interrupt finally fires, the async completion handler mtk_port_tx_complete()
will access trb->priv, potentially dereferencing a freed port pointer.
> + trb->cmd = cmd;
> + trb->trb_complete = trb_complete;
> +}
[ ... ]
> +int mtk_port_send_data(struct mtk_port *port, void *data)
> +{
> + struct mtk_port_mngr *port_mngr;
> + struct sk_buff *skb = data;
> + bool force_send;
> + struct trb *trb;
> + int ret, len;
> +
> + port_mngr = port->port_mngr;
> +
> + force_send = !!(port->info.flags & (PORT_F_BLOCKING | PORT_F_FORCE_SEND));
> + trb = (struct trb *)skb->cb;
> + mtk_port_trb_init(port, trb, TRB_CMD_TX, mtk_port_tx_complete);
> + len = skb->len;
> + kref_get(&trb->kref); /* kref count 1->2 */
> +
> + /* add ccci header */
> + mtk_port_add_header(skb);
> + ret = mtk_port_status_check(port);
> + if (!ret)
> + ret = port_mngr->ctrl_blk->ops->submit_skb(port_mngr->ctrl_blk->mdev,
> + skb, force_send);
> +
> + if (ret < 0) {
> + kref_put(&trb->kref, mtk_port_trb_free); /* kref count 2->1 */
> + kref_put(&trb->kref, mtk_port_trb_free); /* kref count 1->0 */
> + port->tx_seq--;
[Severity: Medium]
Can concurrent writers corrupt the tx_seq variable?
Because the mtk_port_internal_write() API doesn't seem to enforce locking,
multiple threads transmitting on the same port can induce a read-modify-write
race on this variable here and in mtk_port_add_header(). This could result in
duplicate or out-of-order sequence numbers being sent to the WWAN modem.
> + goto out;
> + }
[ ... ]
> +static int mtk_port_rx_dispatch_frag_skb(struct mtk_port *port, struct sk_buff *skb)
> +{
> + struct sk_buff *frag_skb, *frag_next;
> + int ret;
> +
> + frag_skb = skb_shinfo(skb)->frag_list;
> + skb->len -= skb->data_len;
> + skb->data_len = 0;
> + skb_shinfo(skb)->frag_list = NULL;
> +
> + ret = ports_ops[port->info.type]->recv(port, skb);
> + if (ret < 0) {
> + skb_shinfo(skb)->frag_list = frag_skb;
[Severity: High]
Can this lead to a use-after-free and a double-free on the skb?
If the port is not in an open state, mtk_port_internal_recv() explicitly calls
dev_kfree_skb_any(skb) and returns -ENXIO:
mtk_port_internal_recv() {
if (!test_bit(PORT_S_OPEN, &port->status))
goto drop_data;
...
drop_data:
dev_kfree_skb_any(skb);
return ret;
}
When the error returns here to mtk_port_rx_dispatch_frag_skb(), it accesses
the freed skb to update frag_list. It then propagates the error up to
mtk_port_rx_dispatch(), which jumps to the drop_data label and frees the skb
a second time.
> + return ret;
> + }
[ ... ]
> +static int mtk_port_rx_dispatch(struct sk_buff *skb, void *priv, bool force_recv)
> +{
[ ... ]
> + /* Support scatter gather transmission */
> + if (port->rx_mtu > port->rx_frag_size) {
> + ret = mtk_port_rx_dispatch_frag_skb(port, skb);
> + /* -EIO means partial data dispatch complete, does not goto drop flow */
> + if (ret < 0 && ret != -EIO)
> + goto drop_frag_skb;
> + } else {
> + ret = ports_ops[port->info.type]->recv(port, skb);
> + if (ret < 0)
> + goto drop_data;
> + }
> +
> + return ret;
> +
> +drop_frag_skb:
> + {
> + struct sk_buff *frag_skb, *tmp;
> +
> + frag_skb = skb_shinfo(skb)->frag_list;
> + while (frag_skb) {
> + tmp = frag_skb->next;
> + frag_skb->next = NULL;
> + dev_kfree_skb_any(frag_skb);
> + frag_skb = tmp;
> + }
> + skb_shinfo(skb)->frag_list = NULL;
> + }
> +drop_data:
> + dev_kfree_skb_any(skb);
[Severity: High]
This is where the second free of the skb occurs when propagating the error
from the recv callback.
> + return ret;
> +}
> +
> +int mtk_port_add_header(struct sk_buff *skb)
> +{
[ ... ]
> + ccci_h->packet_header = cpu_to_le32(0);
> + ccci_h->packet_len = cpu_to_le32(skb->len);
> + ccci_h->ex_msg = cpu_to_le32(0);
> + ccci_h->status = cpu_to_le32(FIELD_PREP(MTK_HDR_FLD_CHN, port->info.tx_ch) |
> + FIELD_PREP(MTK_HDR_FLD_SEQ, port->tx_seq++) |
[Severity: Medium]
This is the other side of the tx_seq data race where concurrent writers will
corrupt the sequence tracking.
> + FIELD_PREP(MTK_HDR_FLD_AST, 1));
> +
> + trb->status = MTK_TRB_HEADER_ADDED;
> +
> + return 0;
> +}
[ ... ]
> +int mtk_port_status_update(struct mtk_md_dev *mdev, void *data)
> +{
> + struct mtk_port_enum_msg *msg = data;
> + struct mtk_port_info *port_info;
> + struct mtk_port_mngr *port_mngr;
> + struct mtk_ctrl_blk *ctrl_blk;
> + struct mtk_port *port;
> + int port_id;
> + u16 ch_id;
> +
> + if (unlikely(!mdev || !msg))
> + return -EINVAL;
> +
> + ctrl_blk = mdev->ctrl_blk;
> + port_mngr = ctrl_blk->port_mngr;
> + if (le16_to_cpu(msg->version) != MTK_PORT_ENUM_VER ||
> + le32_to_cpu(msg->head_pattern) != MTK_PORT_ENUM_HEAD_PATTERN ||
> + le32_to_cpu(msg->tail_pattern) != MTK_PORT_ENUM_TAIL_PATTERN)
> + return -EPROTO;
> +
> + for (port_id = 0; port_id < le16_to_cpu(msg->port_cnt); port_id++) {
[Severity: High]
Is there a missing bounds check on msg->port_cnt here?
This loop relies strictly on the device-provided port_cnt value without
validating it against the actual size of the received msg->data payload.
A malformed or maliciously large count from the WWAN firmware could cause an
out-of-bounds memory read.
> + port_info = (struct mtk_port_info *)(msg->data +
> + (sizeof(*port_info) * port_id));
> + ch_id = FIELD_GET(MTK_INFO_FLD_CHID, le16_to_cpu(port_info->channel));
> + port = mtk_port_search_by_id(port_mngr, ch_id);
> + if (!port)
> + continue;
> + port->enable = FIELD_GET(MTK_INFO_FLD_EN, le16_to_cpu(port_info->channel));
> + }
> +
> + return 0;
> +}
> +
> +int mtk_port_ch_enable(struct mtk_port *port)
> +{
[ ... ]
> +start_wait:
> + ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> + MTK_DFLT_TRB_TIMEOUT);
> + if (ret == -ERESTARTSYS)
> + goto start_wait;
[Severity: High]
Could this create an infinite loop in kernel space?
If a process executing this function has a pending signal (such as being
killed), wait_event_interruptible_timeout() returns -ERESTARTSYS immediately
without sleeping. The unconditional goto forces a tight loop that never
returns to userspace to process the signal.
> + else if (!ret)
> + ret = -ETIMEDOUT;
> + else
> + ret = trb->status;
> +
> + kref_put(&trb->kref, mtk_port_trb_free);
> +
> + return ret;
> +}
> +
> +int mtk_port_ch_disable(struct mtk_port *port)
> +{
[ ... ]
> +start_wait:
> + ret = wait_event_interruptible_timeout(port->trb_wq, trb->status <= 0,
> + MTK_DFLT_TRB_TIMEOUT);
> + if (ret == -ERESTARTSYS)
> + goto start_wait;
[Severity: High]
This appears to have the same infinite loop risk as mtk_port_ch_enable() when
a signal is pending.
> + else if (!ret)
> + ret = -ETIMEDOUT;
> + else
> + ret = trb->status;
> +
> + kref_put(&trb->kref, mtk_port_trb_free);
> +
> + return ret;
> +}
> diff --git a/drivers/net/wwan/t9xx/mtk_port_io.c b/drivers/net/wwan/t9xx/mtk_port_io.c
> new file mode 100644
> index 0000000000000..e3a2de6d2f297
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_port_io.c
> @@ -0,0 +1,239 @@
[ ... ]
> +static int mtk_port_internal_recv(struct mtk_port *port, struct sk_buff *skb)
> +{
> + struct mtk_internal_port *priv;
> + int ret = -ENXIO;
> +
> + if (!test_bit(PORT_S_OPEN, &port->status))
> + goto drop_data;
> +
> + priv = &port->i_priv;
> + if (!priv->recv_cb || !priv->arg)
> + goto drop_data;
> +
> + ret = priv->recv_cb(priv->arg, skb);
> + return ret;
> +
> +drop_data:
> + dev_kfree_skb_any(skb);
[Severity: High]
This is where the skb is freed on the error path before returning -ENXIO,
leading to the use-after-free and double-free in the caller.
> + return ret;
> +}
> +
> +static int mtk_port_common_open(struct mtk_port *port)
> +{
> + int ret = 0;
> +
> + if (!test_bit(PORT_S_ENABLE, &port->status))
> + return -ENODEV;
> +
> + if (test_bit(PORT_S_OPEN, &port->status))
> + return -EBUSY;
> +
> + skb_queue_purge(&port->rx_skb_list);
> + set_bit(PORT_S_OPEN, &port->status);
[Severity: High]
Is this test_bit() and set_bit() sequence intended to provide mutual
exclusion?
Since this is not an atomic test-and-set operation, two threads could
concurrently observe PORT_S_OPEN as unset, simultaneously execute
skb_queue_purge() which corrupts the queue, and both proceed believing
they have successfully opened the port.
> + clear_bit(PORT_S_FLUSH, &port->status);
> +
> + return ret;
> +}
[ ... ]
^ permalink raw reply
* Re: [PATCH v4 3/7] net: wwan: t9xx: Add control DMA interface
From: Simon Horman @ 2026-07-16 9:34 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-3-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [3]: [PATCH v4 3/7] net: wwan: t9xx: Add control DMA interface ---
commit ee2f68157a30f68c2d68e46c572820b187896695
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add control DMA interface
This patch adds the Cross Layer Direct Memory Access (CLDMA) interface for
the control plane to translate data between the host and device. It
implements GPD ring traversal for TX and RX paths and hardware interrupt
handling.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_cldma.c b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
> index 0000000000000..3bab14d35d78d
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
> [ ... ]
> +static void mtk_cldma_tx_done_work(struct work_struct *work)
> +{
> [ ... ]
> + req = txq->req_pool + txq->free_idx;
> +
> + rmb(); /* ensure HWO setup done before HWO read */
> +
> + if (!req->data_vm_addr || (req->gpd->tx_gpd.gpd_flags & CLDMA_GPD_FLAG_HWO))
> + break;
[Severity: High]
Is a dma_rmb() needed after checking the hardware ownership flag? On weakly
ordered architectures, speculative execution might read payload or length
fields before the ownership check is fully ordered in memory.
> [ ... ]
> +static void mtk_cldma_rx_done_work(struct work_struct *work)
> +{
> [ ... ]
> +again:
> + for (i = 0; i < rxq->nr_gpds; i++) {
> + req = rxq->req_pool + rxq->free_idx;
> + if (!req->skb) {
> [ ... ]
> + }
> +
> + if (req->gpd->rx_gpd.gpd_flags & CLDMA_GPD_FLAG_HWO)
> + break;
> +
> + mtk_cldma_rx_skb_adjust(mdev, rxq, req);
[Severity: High]
Similarly, would we need a dma_rmb() here before attempting to read the
receive length out of the descriptor in mtk_cldma_rx_skb_adjust?
> + do {
> + ret = rxq->rx_done(req->skb, rxq->arg,
> + atomic_read(&rxq->need_exit) ? true : false);
> + if (ret == -EAGAIN)
> + usleep_range(1000, 2000);
> + else
> + req->skb = NULL;
> + } while (ret == -EAGAIN);
> +
> + ret = mtk_cldma_reload_rx_skb(mdev, rxq, req);
> + if (ret)
> + goto out;
[Severity: High]
If mtk_cldma_reload_rx_skb fails due to memory pressure, it jumps to out
without advancing rxq->free_idx. Will this cause the worker to permanently
stall the queue, as subsequent runs will constantly evaluate the same slot
without being able to make progress?
> [ ... ]
> +static void mtk_cldma_txq_free(struct cldma_drv_info *drv_info, u32 txqno)
> +{
> [ ... ]
> + for (j = 0; j < txq->nr_bds; j++) {
> + bd_dsc = req->bd_dsc_pool + j;
> + dma_pool_free(drv_info->bd_dma_pool, bd_dsc->bd,
> + bd_dsc->bd_dma_addr);
> + }
> + kfree(req->bd_dsc_pool);
> + dma_pool_free(drv_info->gpd_dma_pool, req->gpd, req->gpd_dma_addr);
> + txq->free_idx = (txq->free_idx + 1) % txq->nr_gpds;
> + }
> +
> + kfree(txq->req_pool);
> + kfree(txq);
> +}
[Severity: Critical]
Does this incorrectly free memory managed by devres? The arrays req_pool and
bd_dsc_pool are allocated using devm_kcalloc during initialization.
Calling kfree directly on devres-managed memory causes slab allocator
corruption and later double-frees when the device is detached. This same
pattern appears to be present in mtk_cldma_rxq_free as well.
> [ ... ]
> +static int mtk_cldma_reload_rx_skb(struct mtk_md_dev *mdev, struct rxq *rxq,
> + struct rx_req *req)
> +{
> [ ... ]
> +err_free_skb:
> + if (nr_bds) {
> [ ... ]
> + for (i = 0; i < nr_bds; i++) {
> + bd_dsc = req->bd_dsc_pool + i;
> + if (!bd_dsc->skb)
> + break;
> + if (!dma_mapping_error(mdev->dev, bd_dsc->data_dma_addr))
> + dma_unmap_single(mdev->dev, bd_dsc->data_dma_addr,
> + req->frag_size, DMA_FROM_DEVICE);
> + bd_dsc->data_dma_addr = 0;
> + bd_dsc->skb->next = NULL;
> + dev_kfree_skb_any(bd_dsc->skb);
[Severity: High]
Can this lead to a double free? After calling dev_kfree_skb_any here, the
pointer bd_dsc->skb is not set to NULL. During driver teardown,
mtk_cldma_rxq_free will iterate over this array, observe bd_dsc->skb is
still populated, and attempt to free it a second time.
> [ ... ]
> +static int mtk_cldma_txbuf_set(struct cldma_drv_info *drv_info, struct sk_buff *skb,
> + struct tx_req *req, int nr_bds)
> +{
> [ ... ]
> + } else {
> + req->data_dma_addr = dma_map_single(mdev->dev, skb->data,
> + skb->len, DMA_TO_DEVICE);
[Severity: High]
Does this code safely handle non-linear SKBs? When nr_bds is 0, this maps
skb->data using the full skb->len instead of skb_headlen(skb). Could this
cause an out-of-bounds read past the linear buffer for fragmented packets?
> [ ... ]
> +int mtk_cldma_submit_tx(void *dev, struct sk_buff *skb)
> +{
> [ ... ]
> + req->data_len = skb->len;
> + req->skb = skb;
> + req->data_vm_addr = skb->data;
> +
> + wmb(); /* ensure req and data msg set done before HWO setup */
> +
> + req->gpd->tx_gpd.gpd_flags |= CLDMA_GPD_FLAG_HWO;
[Severity: High]
Could there be a race condition here with mtk_cldma_tx_done_work?
By setting req->data_vm_addr before setting the CLDMA_GPD_FLAG_HWO flag, a
concurrent execution of the completion worker might observe data_vm_addr as
non-NULL but with the hardware ownership flag still cleared, misinterpreting
this intermediate state as a completed transmission.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 68dddd652effc..2f51e150750da 100644
> --- a/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> [ ... ]
> @@ -998,6 +1036,7 @@ static void mtk_pci_remove(struct pci_dev *pdev)
> }
>
> pci_clear_master(pdev);
> + mtk_pci_dev_exit(mdev);
> mtk_pci_free_irq(mdev);
[Severity: High]
Is it intended to call pci_clear_master before mtk_pci_dev_exit? Disabling
bus mastering before the hardware queues are fully shut down via MMIO
commands in mtk_pci_dev_exit might block pending hardware DMA transactions
or completion descriptors, which can lead to PCIe Master Aborts or hangs.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c b/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c
> index 0000000000000..55297cfcdbc61
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_trans_ctrl.c
> [ ... ]
> +static void mtk_ctrl_trb_handler(struct trb_srv *srv, struct trans_list *trans_list, u32 qno)
> +{
> [ ... ]
> + case TRB_CMD_TX:
> + err = mtk_cldma_submit_tx(trans->dev, skb);
> + if (err) {
> + if (trans_list->tx_burst_cnt[qno]) {
> + kick = true;
> + break;
> + }
> + if (err == -EAGAIN)
> + return;
[Severity: High]
If mtk_cldma_submit_tx returns -EAGAIN due to DMA mapping failures, this
returns immediately and leaves the skb on the list. Since the budget is not
consumed and the queue remains non-empty, mtk_ctrl_chs_is_busy_or_empty
will evaluate to false. Could this cause the mtk_ctrl_trb_thread loop to
spin continuously without sleeping, leading to a CPU lockup?
> [ ... ]
> +static int mtk_ctrl_trb_thread(void *args)
> +{
> + struct trb_srv *srv = args;
> +
> + for (;;) {
> + wait_event_interruptible(srv->trb_waitq,
> + !mtk_ctrl_chs_is_busy_or_empty(srv) ||
> + kthread_should_stop() || kthread_should_park());
[Severity: High]
Can this cause an infinite loop if a signal is delivered to the thread?
Using wait_event_interruptible without checking signal_pending(current) or
flushing signals means it will immediately return -ERESTARTSYS on every loop
iteration once a signal is pending, permanently burning a CPU core.
> [ ... ]
> +static void mtk_ctrl_remove_radix_tree(struct mtk_ctrl_trans *trans)
> +{
> + struct radix_tree_iter iter;
> + struct queue_info *queue;
> + void __rcu **slot;
> +
> + radix_tree_for_each_slot(slot, &trans->queue_tbl, &iter, 0) {
> + queue = radix_tree_deref_slot(slot);
> + if (!queue)
> + continue;
> + radix_tree_delete(&trans->queue_tbl, iter.index);
> + kfree(queue);
> + }
> +}
[Severity: High]
Is it safe to iterate over the radix tree and call radix_tree_delete without
holding rcu_read_lock? Because the tree operations use RCU for internal
node memory management, doing this locklessly might allow the grace period
to expire, causing a use-after-free when the iterator accesses nodes.
^ permalink raw reply
* Re: [PATCH v4 2/7] net: wwan: t9xx: Add control plane transaction layer
From: Simon Horman @ 2026-07-16 9:33 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-2-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [2]: [PATCH v4 2/7] net: wwan: t9xx: Add control plane transaction layer ---
commit a06b668b7ebf78322cb2957d8a6a11fdfb9235ad
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add control plane transaction layer
The control plane implements TX services that reside in the
transaction layer. The services receive the packets from the
port layer and call the corresponding DMA components to
transmit data to the device. Meanwhile, TX services receive
and manage the port control commands from the port layer.
The control plane implements RX services that reside in the
transaction layer. The services receive the downlink packets
from the modem and transfer the packets to the corresponding
port layer interfaces.
> diff --git a/drivers/net/wwan/t9xx/mtk_ctrl_plane.c b/drivers/net/wwan/t9xx/mtk_ctrl_plane.c
> new file mode 100644
> index 0000000000000..cf6079218e1ce
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/mtk_ctrl_plane.c
> @@ -0,0 +1,45 @@
[ ... ]
> +int mtk_ctrl_init(struct mtk_md_dev *mdev)
> +{
> + struct mtk_ctrl_blk *ctrl_blk;
> +
> + ctrl_blk = devm_kzalloc(mdev->dev, sizeof(*ctrl_blk), GFP_KERNEL);
> + if (!ctrl_blk)
> + return -ENOMEM;
> +
> + ctrl_blk->mdev = mdev;
> + mdev->ctrl_blk = ctrl_blk;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(mtk_ctrl_init);
[Severity: Medium]
Does this patch actually implement the TX and RX services described in the
commit message?
The commit message claims to implement control plane TX and RX services, but
this code merely introduces empty initialization stubs (mtk_ctrl_init() and
mtk_ctrl_exit()) that are currently dead code.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 72259bf1b6033..68dddd652effc 100644
> --- a/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> @@ -467,6 +468,7 @@ static u32 mtk_pci_ext_h2d_evt_hw_bits(u32 chs)
>
> SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DEVICE_RESET,
> DEV_EVT_H2D_DEVICE_RESET);
> +
> return LE32_TO_U32(cpu_to_le32(hw_bits));
> }
[Severity: High]
Does this code apply a double byte-swap on big-endian architectures?
Using cpu_to_le32() converts the bitmask, which is then cast to u32 and
passed to mtk_pci_write32() (wrapping iowrite32()). Because iowrite32()
intrinsically expects a host-endian value and applies its own conversion for
little-endian PCI memory, the value is byte-swapped twice, resulting in the
original big-endian layout being incorrectly written.
Additionally, this can cause ffs(hw_bits) in mtk_pci_send_ext_evt() to
calculate the wrong bit index on the already byte-swapped value.
^ permalink raw reply
* Re: [PATCH v4 1/7] net: wwan: t9xx: Add PCIe core
From: Simon Horman @ 2026-07-16 9:33 UTC (permalink / raw)
To: jackbb_wu
Cc: Simon Horman, loic.poulain, ryazanov.s.a, johannes, andrew+netdev,
davem, edumazet, kuba, pabeni, wen-zhi.huang, shi-wei.yeh,
Minano.tseng, matthias.bgg, angelogioacchino.delregno, corbet,
skhan, linux-kernel, netdev, linux-arm-kernel, linux-mediatek,
linux-doc
In-Reply-To: <20260709-t9xx_driver_v1-v4-1-a8c009d509c5@compal.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [1]: [PATCH v4 1/7] net: wwan: t9xx: Add PCIe core ---
commit 4564b6641dc4f4a3fb3d169ca45443e5d18c66b7
Author: Jack Wu <jackbb_wu@compal.com>
net: wwan: t9xx: Add PCIe core
Registers the T900 device driver with the kernel. Set up all
the fundamental configurations for the device: PCIe layer,
Modem Host Cross Core Interface (MHCCIF), Reset Generation
Unit (RGU), modem common control operations and build
infrastructure.
> diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
> index 0000000000000..72259bf1b6033
> --- /dev/null
> +++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
[ ... ]
> +static irqreturn_t mtk_pci_irq_msix(int irq, void *data)
> +{
> + struct mtk_pci_irq_desc *irq_desc = data;
> + struct mtk_md_dev *mdev = irq_desc->mdev;
> + struct mtk_pci_priv *priv;
> + u32 irq_state, irq_enable;
> +
> + priv = mdev->hw_priv;
> + irq_state = mtk_pci_mac_read32(priv, REG_MSIX_ISTATUS_HOST_GRP0_0);
> + irq_enable = mtk_pci_mac_read32(priv, REG_IMASK_HOST_MSIX_GRP0_0);
> + irq_state &= irq_enable;
> +
> + if (unlikely(!irq_state) ||
> + unlikely(!((irq_state & GENMASK(priv->irq_cnt - 1, 0)) &
> + irq_desc->msix_bits)))
> + return IRQ_NONE;
[Severity: High]
Does this logic silently drop hardware interrupts when fewer than 32 vectors are
allocated?
Because hardware source bits map to fixed high bit positions (such as bit 28
for MTK_IRQ_SRC_MHCCIF), systems that allocate fewer vectors will mask out
these high bits with GENMASK(priv->irq_cnt - 1, 0).
Evaluating !(0 & msix_bits) as true returns IRQ_NONE, which may eventually
disable the interrupt line.
[ ... ]
> +static int mtk_pci_request_irq_msix(struct mtk_md_dev *mdev,
> + int irq_cnt_allocated)
> +{
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + struct mtk_pci_irq_desc *irq_desc;
> + struct pci_dev *pdev;
> + int irq_cnt;
> + int ret, i;
> +
> + /* calculate the nearest 2's power number */
> + irq_cnt = BIT(fls(irq_cnt_allocated) - 1);
> + pdev = to_pci_dev(mdev->dev);
> + irq_desc = priv->irq_desc;
> + for (i = 0; i < irq_cnt; i++) {
> + irq_desc[i].mdev = mdev;
> + irq_desc[i].msix_bits = BIT(i);
> + snprintf(irq_desc[i].name, MTK_IRQ_NAME_LEN, "msix%d-%s", i, mdev->dev_str);
> + ret = pci_request_irq(pdev, i, mtk_pci_irq_msix, NULL,
> + &irq_desc[i], irq_desc[i].name);
[Severity: Medium]
Could this trigger a -Wformat-security compiler warning?
Passing a dynamic string variable directly as the format string to
pci_request_irq() can lead to format string vulnerabilities. Should this use
"%s", irq_desc[i].name instead?
[ ... ]
> +static u32 mtk_pci_ext_h2d_evt_hw_bits(u32 chs)
> +{
> + u32 hw_bits = 0;
> +
> + SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DEVICE_RESET,
> + DEV_EVT_H2D_DEVICE_RESET);
> + return LE32_TO_U32(cpu_to_le32(hw_bits));
> +}
[ ... ]
> +int mtk_pci_send_ext_evt(struct mtk_md_dev *mdev, u32 ch)
> +{
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + u32 rc_base, hw_bits;
> +
> + rc_base = priv->cfg->mhccif_rc_base_addr;
> +
> + /* Only allow one ch to be triggered at a time */
> + if (!is_power_of_2(ch)) {
> + dev_err(mdev->dev, "Unsupported ext evt ch=0x%08x\n", ch);
> + return -EINVAL;
> + }
> +
> + hw_bits = mtk_pci_ext_h2d_evt_hw_bits(ch);
> + mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_BSY, hw_bits);
> + mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_TCHNUM, ffs(hw_bits) - 1);
[Severity: Medium]
Will this break channel index calculation on big-endian systems?
If hw_bits has already been byte-swapped by cpu_to_le32() inside
mtk_pci_ext_h2d_evt_hw_bits(), performing ffs(hw_bits) - 1 on the byte-swapped
integer will result in a completely incorrect bit index.
[Severity: Medium]
What happens if the driver attempts to send an unmapped channel?
If ch is not mapped in mtk_pci_ext_h2d_evt_hw_bits(), hw_bits will evaluate
to 0. The calculation ffs(0) - 1 will then underflow to 0xFFFFFFFF. This
value is written directly to the hardware and might hang or crash the device
firmware.
[ ... ]
> +static int mtk_pci_bar_init(struct mtk_md_dev *mdev)
> +{
> + struct pci_dev *pdev = to_pci_dev(mdev->dev);
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + u32 bar[MTK_PCI_BAR_NUM];
> + int i, ret;
> +
> + for (i = 0; i < MTK_PCI_BAR_NUM; i++)
> + pci_read_config_dword(to_pci_dev(mdev->dev),
> + PCI_BASE_ADDRESS_0 + (i << 2), bar + i);
> +
> + ret = pcim_iomap_regions(pdev, MTK_REQUESTED_BARS, mdev->dev_str);
> + if (ret) {
> + dev_err(mdev->dev, "Failed to init MMIO. ret=%d\n", ret);
> + return ret;
> + }
> +
> + /* get ioremapped memory */
> + priv->mac_reg_base = pcim_iomap_table(pdev)[MTK_BAR_0_1_IDX];
> + priv->bar23_addr = pcim_iomap_table(pdev)[MTK_BAR_2_3_IDX];
> + if (!priv->mac_reg_base || !priv->bar23_addr) {
> + dev_err(mdev->dev, "Failed to init BAR.\n");
> + return -EINVAL;
> + }
> + /* We use MD view base address "0" to observe registers */
> + priv->ext_reg_base = priv->bar23_addr - ATR_PCIE_REG_TRSL_ADDR;
[Severity: Medium]
Is it safe to store an out-of-bounds pointer?
ATR_PCIE_REG_TRSL_ADDR is 0x10000000, and subtracting it here results in
priv->ext_reg_base pointing well outside the allocated object. This invokes
undefined behavior and might cause crashes or be optimized out by UBSAN.
Could the correct offset be calculated at the time of access instead?
[ ... ]
> +static void mtk_pci_remove(struct pci_dev *pdev)
> +{
> + struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
> + struct mtk_pci_priv *priv = mdev->hw_priv;
> + struct device *dev = &pdev->dev;
> +
> + mtk_pci_mask_irq(mdev, priv->mhccif_irq_id);
> +
> + if (mtk_pci_pldr(mdev)) {
> + dev_warn(dev, "Failed to execute PLDR, try external event\n");
> + mtk_pci_reset(mdev, RESET_MHCCIF);
> + }
> +
> + pci_clear_master(pdev);
> + mtk_pci_free_irq(mdev);
> + mtk_mhccif_exit(mdev);
[Severity: High]
Does this sequence result in the device being powered off before the OS
interrupt handlers and workqueues are synchronized?
Calling mtk_pci_pldr() executes the ACPI PXP._OFF method, cutting power.
If mtk_mhccif_exit() (which calls cancel_work_sync(&priv->mhccif_work))
or mtk_pci_free_irq() runs after this, a concurrently executing workqueue
or interrupt handler might attempt to read MMIO registers from a powered-off
device, causing bus errors and kernel panics.
[ ... ]
> +static pci_ers_result_t mtk_pci_error_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> + struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
> +
> + dev_err(mdev->dev, "AER detected: pci_channel_state_t=%d\n", state);
> +
> + /* Request a slot reset. */
> + return PCI_ERS_RESULT_CAN_RECOVER;
> +}
[Severity: Medium]
Will returning PCI_ERS_RESULT_CAN_RECOVER actually abort the slot reset?
The comment says "Request a slot reset", but CAN_RECOVER instructs the PCI
core to skip the reset. Furthermore, since the driver provides no .resume or
.mmio_enabled callbacks, the device remains in a corrupted state despite
being marked as recovered.
Shouldn't this return PCI_ERS_RESULT_NEED_RESET?
^ permalink raw reply
* Re: [PATCH v6 1/5] spi: dt-bindings: Add spi-device-addr peripheral property
From: Nuno Sá @ 2026-07-16 9:22 UTC (permalink / raw)
To: Mark Brown
Cc: Janani Sunil, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Jonathan Corbet, Shuah Khan, Marius Cristea, Marcus Folkesson,
Kent Gustavsson, linux-iio, devicetree, linux-kernel, linux-doc,
Janani Sunil, linux-spi, Kent Gustavsson
In-Reply-To: <157154d1-3995-454b-9e08-1527f1c5d409@sirena.org.uk>
On Wed, Jul 15, 2026 at 02:37:12PM +0100, Mark Brown wrote:
> On Wed, Jul 15, 2026 at 03:29:32PM +0200, Nuno Sá wrote:
> > On Wed, Jul 15, 2026 at 02:09:19PM +0100, Mark Brown wrote:
>
> > > This really isn't a generic SPI thing, if nothing else you need *far*
> > > more information in there about how exactly this would be put onto the
> > > bus. If it belongs anywhere outside of the specific device's binding it
> > > feels like it might be regmap.
>
> > Just for some context,
>
> > For the analog chip, it can share the same CS line with another 3
> > identical chips. It has two pins that depending on how they are set act
> > as the device address (so only one replies to a given transfer -
> > naturally the peripheral driver needs to setup the correct transfer
> > and that depends on these pins setup and hence dt property).
>
> > This property reflects that. Apparently some microchip chips are doing something
> > very similar so Conor proposed a generic property given that we would have at
> > least 3 users of it.
>
> You still need to work out how the ID appears in the byte stream that
> gets sent to/from the device, that's way more information than just a
> number and not something the byte stream SPI offers is going to cope
> well with.
Not sure if I'm following you. Those ID pins just become something you
need to set on the spi_xfer. Or in Janani case, she's using regmap
reg_base in order to set the right thing depending on the address. I
would image this is always something that peripherals need to address in
terms of how the message/stream needs to be set. So my understanding is
that this should be pretty much transparent for the spi core.
But I might be just focused on this usecase and missing the big picture.
Thx!
- Nuno Sá
^ permalink raw reply
* Re: [PATCH v5] arm64: errata: work around NVIDIA Olympus device store/load ordering
From: Vladimir Murzin @ 2026-07-16 9:15 UTC (permalink / raw)
To: Shanker Donthineni, Catalin Marinas, Will Deacon
Cc: Jason Gunthorpe, linux-arm-kernel, Mark Rutland, linux-kernel,
linux-doc, Vikram Sethi, Jason Sequeira
In-Reply-To: <20260715204856.3707449-1-sdonthineni@nvidia.com>
On 7/15/26 21:48, Shanker Donthineni wrote:
> On systems with NVIDIA Olympus cores, a Device-nGnR* load can be
> observed by a peripheral before an older, non-overlapping Device-nGnR*
> store to the same peripheral. This breaks the program-order guarantee
> that software expects for Device-nGnR* accesses and can leave a
> peripheral in an incorrect state.
>
> The erratum can occur only when all of the following apply:
>
> - A PE executes a Device-nGnR* store followed by a younger
> Device-nGnR* load.
> - The store is not a store-release.
> - The accesses target the same peripheral and do not overlap in bytes.
> - There is at most one intervening Device-nGnR* store in program
> order, and there are no intervening Device-nGnR* loads.
> - There is no DSB or full DMB between the store and the load.
> - Specific microarchitectural and timing conditions occur.
>
> Insert a DMB OSH immediately before each raw MMIO load on affected CPUs.
> As a full barrier, DMB OSH orders the older Device store before the
> younger Device load and prevents the erroneous observation.
>
> Add the barrier directly to the __raw_read*() helpers, independently of
> the existing device-load-acquire alternative. On affected CPUs this adds
> one DMB OSH per raw MMIO load, including each load used by
> memcpy_fromio(). On unaffected CPUs the alternative remains a NOP.
>
> Co-developed-by: Vikram Sethi <vsethi@nvidia.com>
> Signed-off-by: Vikram Sethi <vsethi@nvidia.com>
> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
> Link: https://lore.kernel.org/all/akPQ8F3OgER621UP@willie-the-truck/
> ---
> Changes since v4:
> - Reworked the workaround following Will Deacon's review: leave the raw
> MMIO write helpers unchanged and insert a DMB OSH before raw MMIO loads.
> - Use DMB OSH after hardware confirmation that it fixes T410-OLY-1027.
> - Dropped the separate memcpy_fromio() optimization patch because the
> benchmark showed no noticeable benefit over the per-load workaround.
> - Updated the cpucap, Kconfig help text, and commit messages for the
> load-side workaround.
>
> Changes since v3:
> - Split the workaround into two patches: the erratum fix (1/2) and the
> arm64 memset_io()/memcpy_toio() block writers (2/2).
> - Reworked the raw MMIO write helpers to use a direct base-register
> str*/stlr* alternative sequence instead of a per-write static branch.
> - Covered the write-combining __iowrite{32,64}_copy() path by patching
> dgh() to dmb osh on affected CPUs, keeping the contiguous STR groups
> and the ordering barrier outside the copy loop; the single-element
> case now uses a plain str* as well.
> - Added arm64 memset_io()/memcpy_toio() so the byte/word block writers
> take one trailing dmb osh instead of a per-store store-release.
> - Updated the commit messages to describe the offset-addressing
> trade-off.
>
> Changes since v2:
> - Reworked the raw MMIO write helpers so unaffected CPUs keep the
> existing offset-addressed STR sequence, while affected CPUs use the
> base-register STLR path.
> - Updated the commit message to match the code changes.
> - Rebased on top of the arm64 for-next/errata branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/errata
>
> Changes since v1:
> - Updated the commit message based on feedback from Vladimir Murzin.
>
> ---
> Documentation/arch/arm64/silicon-errata.rst | 2 ++
> arch/arm64/Kconfig | 22 +++++++++++++++++++++
> arch/arm64/include/asm/io.h | 16 ++++++++++++----
> arch/arm64/kernel/cpu_errata.c | 8 ++++++++
> arch/arm64/tools/cpucaps | 1 +
> 5 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index ad04d1cdc0f0..c4137f89acef 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -298,6 +298,8 @@ stable kernels.
> +----------------+-----------------+-----------------+-----------------------------+
> | NVIDIA | Carmel Core | N/A | NVIDIA_CARMEL_CNP_ERRATUM |
> +----------------+-----------------+-----------------+-----------------------------+
> +| NVIDIA | Olympus core | T410-OLY-1027 | NVIDIA_OLYMPUS_1027_ERRATUM |
> ++----------------+-----------------+-----------------+-----------------------------+
> | NVIDIA | Olympus core | T410-OLY-1029 | ARM64_ERRATUM_4118414 |
> +----------------+-----------------+-----------------+-----------------------------+
> | NVIDIA | T241 GICv3/4.x | T241-FABRIC-4 | N/A |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 10c69474f276..73e3e68db161 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1363,6 +1363,28 @@ config NVIDIA_CARMEL_CNP_ERRATUM
>
> If unsure, say Y.
>
> +config NVIDIA_OLYMPUS_1027_ERRATUM
> + bool "NVIDIA Olympus: device store/load ordering erratum"
> + default y
> + help
> + This option adds an alternative code sequence to work around an
> + NVIDIA Olympus core erratum where a Device-nGnR* store can be
> + observed by a peripheral after a younger Device-nGnR* load to the
> + same peripheral. This breaks the program order that drivers rely
> + on for MMIO and can leave a device in an incorrect state.
> +
> + The workaround inserts a DMB OSH immediately before raw MMIO loads.
> + The erratum cannot occur when a DMB that orders loads appears
> + between the store and load, preventing the younger load from being
> + observed before the older store.
> +
> + The alternatives framework patches in DMB OSH only when an affected
> + CPU is detected. Other CPUs execute a NOP in its place. Disabling
> + this option leaves the original MMIO read instruction stream
> + unchanged.
> +
> + If unsure, say Y.
> +
> config ROCKCHIP_ERRATUM_3568002
> bool "Rockchip 3568002: GIC600 can not access physical addresses higher than 4GB"
> default y
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 8cbd1e96fd50..6d6d54c1b74c 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -54,7 +54,9 @@ static __always_inline void __raw_writeq(u64 val, volatile void __iomem *addr)
> static __always_inline u8 __raw_readb(const volatile void __iomem *addr)
> {
> u8 val;
> - asm volatile(ALTERNATIVE("ldrb %w0, [%1]",
> + asm volatile(ALTERNATIVE("nop", "dmb osh",
> + ARM64_WORKAROUND_NVIDIA_OLYMPUS_1027)
> + ALTERNATIVE("ldrb %w0, [%1]",
> "ldarb %w0, [%1]",
> ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
> : "=r" (val) : "r" (addr));
> @@ -66,7 +68,9 @@ static __always_inline u16 __raw_readw(const volatile void __iomem *addr)
> {
> u16 val;
>
> - asm volatile(ALTERNATIVE("ldrh %w0, [%1]",
> + asm volatile(ALTERNATIVE("nop", "dmb osh",
> + ARM64_WORKAROUND_NVIDIA_OLYMPUS_1027)
> + ALTERNATIVE("ldrh %w0, [%1]",
> "ldarh %w0, [%1]",
> ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
> : "=r" (val) : "r" (addr));
> @@ -77,7 +81,9 @@ static __always_inline u16 __raw_readw(const volatile void __iomem *addr)
> static __always_inline u32 __raw_readl(const volatile void __iomem *addr)
> {
> u32 val;
> - asm volatile(ALTERNATIVE("ldr %w0, [%1]",
> + asm volatile(ALTERNATIVE("nop", "dmb osh",
> + ARM64_WORKAROUND_NVIDIA_OLYMPUS_1027)
> + ALTERNATIVE("ldr %w0, [%1]",
> "ldar %w0, [%1]",
> ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
> : "=r" (val) : "r" (addr));
> @@ -88,7 +94,9 @@ static __always_inline u32 __raw_readl(const volatile void __iomem *addr)
> static __always_inline u64 __raw_readq(const volatile void __iomem *addr)
> {
> u64 val;
> - asm volatile(ALTERNATIVE("ldr %0, [%1]",
> + asm volatile(ALTERNATIVE("nop", "dmb osh",
> + ARM64_WORKAROUND_NVIDIA_OLYMPUS_1027)
> + ALTERNATIVE("ldr %0, [%1]",
> "ldar %0, [%1]",
> ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
> : "=r" (val) : "r" (addr));
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 4b0d5d932897..740ba3d51e30 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -839,6 +839,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> ERRATA_MIDR_ALL_VERSIONS(MIDR_NVIDIA_CARMEL),
> },
> #endif
> +#ifdef CONFIG_NVIDIA_OLYMPUS_1027_ERRATUM
> + {
> + /* NVIDIA Olympus core */
> + .desc = "NVIDIA Olympus device store/load ordering erratum",
> + .capability = ARM64_WORKAROUND_NVIDIA_OLYMPUS_1027,
> + ERRATA_MIDR_ALL_VERSIONS(MIDR_NVIDIA_OLYMPUS),
> + },
> +#endif
> #ifdef CONFIG_ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
> {
> /*
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 811c2479e82d..8d919b6699f0 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -121,6 +121,7 @@ WORKAROUND_CAVIUM_TX2_219_TVM
> WORKAROUND_CLEAN_CACHE
> WORKAROUND_DEVICE_LOAD_ACQUIRE
> WORKAROUND_NVIDIA_CARMEL_CNP
Nitpick: this ^ line has changed recently, so patch might not apply
> +WORKAROUND_NVIDIA_OLYMPUS_1027
> WORKAROUND_PMUV3_IMPDEF_TRAPS
> WORKAROUND_QCOM_FALKOR_E1003
> WORKAROUND_QCOM_ORYON_CNTVOFF
> -- 2.54.0.windows.1
>
FWIW,
Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
Thanks
Vladimir
^ permalink raw reply
* Re: [PATCH v2 0/4] mm/hmm: Clarify notifier retry state and scope HMM timeouts
From: Lorenzo Stoakes (ARM) @ 2026-07-16 9:12 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Stanislav Kinsburskii, airlied, akhilesh, corbet,
dakr, jgg, kees, leon, liam, lizhi.hou, lyude, maarten.lankhorst,
mamin506, mhocko, mripard, nouveau, ogabbay, oleg, rppt, shuah,
simona, skhan, surenb, tzimmermann, vbabka, dri-devel, linux-mm,
linux-doc, linux-kernel, linux-kselftest, linux-rdma
In-Reply-To: <e9d0ade4-cf87-4e8d-90ec-50ddb5249b24@kernel.org>
On Thu, Jul 16, 2026 at 10:59:56AM +0200, David Hildenbrand (Arm) wrote:
> On 7/15/26 19:14, Andrew Morton wrote:
> > On Wed, 15 Jul 2026 09:39:37 -0700 Stanislav Kinsburskii <skinsburskii@gmail.com> wrote:
> >
> >>>
> >>> Please don't do this, this is completely impossible to track for review :)
> >>>
> >>> mm review is currently very difficult based on volumes, it'll become impossible
> >>> to manage if people sound fragments of series.
> >>>
> >>> Please just resend the whole thing at this point.
> >>>
> >>
> >> Well, I followed the guidance provided by Andrew, and I believe he
> >> applied all of the changes, including these, to the `mm` tree.
> >>
> >> Do you still want me to send v9 under these circumstances?
> >
> > Sure, if that's what reviewers prefer.
> >
> > This is a bit unfriendly to people who have already reviewed the code.
> > Which is one of the reasons why I respond to a new version with a
> > single diff showing reviewers (and the author, and myself) what changed
> > since the previous version.
>
> As much as I dislike fixup patches, I tolerate them in reply to the existing series.
>
> But having some random fixup series is just crazy, really.
>
> --
> Cheers,
>
> David
Yes.
Stanislav - also see Jason's suggestion - if you give a base commit then sashiko
seems to work better. Maybe look at using b4 which does all of this for you
automatically?
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 00/23] Introduce SCMI Telemetry support
From: David Hildenbrand (Arm) @ 2026-07-16 9:06 UTC (permalink / raw)
To: Cristian Marussi, Subrahmanya Lingappa
Cc: arm-scmi, linux-arm-kernel, linux-kernel, linux-doc, sudeep.holla,
james.quinlan, f.fainelli, vincent.guittot, etienne.carriere,
peng.fan, michal.simek, d-gole, jic23, elif.topuz, lukasz.luba,
philip.radford, brauner, souvik.chakravarty, leitao, kas,
puranjay, usama.arif, kernel-team
In-Reply-To: <alenP8e_2SFtgWw4@pluto>
Hi,
thanks for looking into this again, Subrahmanya,
Let me add some additional points to Cristian's reply.
>
>>
>> I agree that asking this series to grow a generic telemetry subsystem now is
>> too much. With only SCMI in-tree, it would be easy to create a "generic"
>> interface that is really just SCMI with different names. I also agree that raw
>> SHMTI is not just a debug path if the expected users are high-rate tooling.
>>
>> So let me narrow the ask.
>>
>> I am not asking for `drivers/telemetry/` in v6. I am asking that the SCMI
>> chardev/UAPI not be tied too closely to the current SCMI core storage layout.
>> In particular, I think v6 should try to:
>>
>> - copy descriptor/config/sample data through fixed and extensible UAPI
>> structs, not structs mirroring SCMI internal objects;
>
> If you mean by this adding a bit more of pad/reserved space scattered
> here and there, it is not a problem; also, one thing that is still missing
> and planned to be added is some ABI Versioning and related IOCTL,
> should not this be enough to address future possible UAPI expansions ?
>
> Regarding avoiding mapping internal SCMI object in the UAPI, I think I
> am already NOT blindly sharing internal structures but instead I copy
> explicitly internal representations into the UAPI fields....I think the
> only exception to this is the sample (scmi_tlm_de_sample) that I take
> care to keep aligned with the internals to avoid the overhead of the
> double copy...but it seems one of the few pretty much general concept
> that I felt confident to abstract no ?
>
> ...but of course this UAPI abstraction into which I copy the results
> is mapped into SCMI inspired concepts/abstractions...for the reasons
> we said....
Getting the UAPI right and future-proof is certainly the most critical thing.
Adding some more padding to keep it extensible is certainly doable.
I remember plans for having an ABI versioning though, would that still be
required (or could it even be deferred) if we add some more padding to the structs?
Or would ABI versioning just avoid that for now?
>
>> - treat DE IDs, group IDs and SHMTI IDs as protocol identifiers, not as
>> implicit indexes into kernel arrays;
>
> Not sure what you mean here... a shim layer used to acces info data
> indirectly instead of the SCMI data structures ?
>
> or some sort of layer/abstraction to be able to remap references
> internally ? again it is not easy to imagine an abstraction without any
> other example than SCMI...or you mean naming issue in the UAPI ?
I'm also having a hard time understanding this.
Subrahmanya, can you share an example of before vs. after to clarify? Thanks
>
>> - keep the TDCF/SHMTI parsing and SCMI command handling behind small
>> internal ops used by the chardev side;
>
> well..if you refer to the telemetry_ops, this is an SCMI driver so it
> has the structure of an SCMI driver, the Telemetry Operations exposed by
> the core SCMI stack are the internal ops themselves that you ask for in
> my opinion...I mean this is pretty much internal stuff that has nothing
> to do with the UAPI itself and I imagine that when some future protocol
> of yours, like the XXMI you mentioned, will appear you will write a new
> driver on your own subsystem that will implement the UAPI itself for
> your protocols (if you want to share that)...any possible future internal
> reworking due to possible common abstractions on our side can be carried
> out in the future...
Any internal refactorings should be had once required, not prematurely.
At this point it's not even clear when/how/if any internal user would want to
reuse some pieces.
>
>> - document the config model explicitly: per SCMI instance, last writer wins,
>> generation changes tell userspace that state changed;
>> - document the mmap contract: length, offsets, cache attributes, lifetime,
>> and reset/reconfiguration behaviour.
>
> Completely agreed on this...Documentation is still lacking, including
> extensive examples...I should have marked docs as RFC too...
+1
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 0/4] mm/hmm: Clarify notifier retry state and scope HMM timeouts
From: David Hildenbrand (Arm) @ 2026-07-16 8:59 UTC (permalink / raw)
To: Andrew Morton, Stanislav Kinsburskii
Cc: Lorenzo Stoakes (ARM), airlied, akhilesh, corbet, dakr, jgg, kees,
leon, liam, lizhi.hou, lyude, maarten.lankhorst, mamin506, mhocko,
mripard, nouveau, ogabbay, oleg, rppt, shuah, simona, skhan,
surenb, tzimmermann, vbabka, dri-devel, linux-mm, linux-doc,
linux-kernel, linux-kselftest, linux-rdma
In-Reply-To: <20260715101432.99957d2d33203fafe4af2e1c@linux-foundation.org>
On 7/15/26 19:14, Andrew Morton wrote:
> On Wed, 15 Jul 2026 09:39:37 -0700 Stanislav Kinsburskii <skinsburskii@gmail.com> wrote:
>
>>>
>>> Please don't do this, this is completely impossible to track for review :)
>>>
>>> mm review is currently very difficult based on volumes, it'll become impossible
>>> to manage if people sound fragments of series.
>>>
>>> Please just resend the whole thing at this point.
>>>
>>
>> Well, I followed the guidance provided by Andrew, and I believe he
>> applied all of the changes, including these, to the `mm` tree.
>>
>> Do you still want me to send v9 under these circumstances?
>
> Sure, if that's what reviewers prefer.
>
> This is a bit unfriendly to people who have already reviewed the code.
> Which is one of the reasons why I respond to a new version with a
> single diff showing reviewers (and the author, and myself) what changed
> since the previous version.
As much as I dislike fixup patches, I tolerate them in reply to the existing series.
But having some random fixup series is just crazy, really.
--
Cheers,
David
^ permalink raw reply
* [PATCH net-next V7 4/4] devlink: Apply eswitch mode boot defaults
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch, Jiri Pirko
In-Reply-To: <20260716084852.549909-1-mbloch@nvidia.com>
Apply parsed devlink_eswitch_mode= defaults after devlink registration
and after successful reload.
Mark the default as pending when a devlink instance is allocated. Before
devl_unlock() releases the instance lock, apply a pending default when
the instance is registered. Since the default is applied while the lock
is still held, userspace cannot race with it.
Clear the pending state before calling into the driver so the boot
default remains a one-shot operation even if the mode change fails.
For successful reloads that performed DRIVER_REINIT, devlink_reload()
already holds the devlink instance lock and the driver has completed
reload_up(). Clear the pending state and apply the default directly from
the reload path.
Treat an explicit userspace eswitch mode request as consuming the pending
default, and clear it when unregistering the devlink instance.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
net/devlink/core.c | 3 ++
net/devlink/default.c | 70 +++++++++++++++++++++++++++++++++++--
net/devlink/dev.c | 6 ++++
net/devlink/devl_internal.h | 5 +++
4 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index d791a8e4317d..02e6d3ef32f8 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -317,6 +317,7 @@ EXPORT_SYMBOL_GPL(devl_trylock);
void devl_unlock(struct devlink *devlink)
{
+ devlink_default_eswitch_mode_apply_pending(devlink);
mutex_unlock(&devlink->lock);
}
EXPORT_SYMBOL_GPL(devl_unlock);
@@ -429,6 +430,7 @@ void devl_unregister(struct devlink *devlink)
ASSERT_DEVLINK_REGISTERED(devlink);
devl_assert_locked(devlink);
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
devlink_rel_put(devlink);
@@ -490,6 +492,7 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
INIT_LIST_HEAD(&devlink->trap_group_list);
INIT_LIST_HEAD(&devlink->trap_policer_list);
INIT_RCU_WORK(&devlink->rwork, devlink_release);
+ devlink_default_eswitch_mode_init(devlink);
lockdep_register_key(&devlink->lock_key);
mutex_init(&devlink->lock);
lockdep_set_class(&devlink->lock, &devlink->lock_key);
diff --git a/net/devlink/default.c b/net/devlink/default.c
index 9b15b7b23e00..b80e3d0399e1 100644
--- a/net/devlink/default.c
+++ b/net/devlink/default.c
@@ -10,6 +10,7 @@
static char *devlink_default_esw_mode_param;
static bool devlink_default_esw_mode_match_all;
+static bool devlink_default_esw_mode_enabled;
static enum devlink_eswitch_mode devlink_default_esw_mode;
static LIST_HEAD(devlink_default_esw_mode_nodes);
@@ -154,6 +155,7 @@ static void __init devlink_default_eswitch_mode_nodes_clear(void)
}
devlink_default_esw_mode_match_all = false;
+ devlink_default_esw_mode_enabled = false;
}
static int __init devlink_default_eswitch_mode_parse(char *str)
@@ -180,14 +182,78 @@ static int __init devlink_default_eswitch_mode_parse(char *str)
return err;
err = devlink_default_eswitch_mode_handles_parse(handles);
- if (err)
+ if (err) {
devlink_default_eswitch_mode_nodes_clear();
- else
+ } else {
devlink_default_esw_mode = esw_mode;
+ devlink_default_esw_mode_enabled = true;
+ }
return err;
}
+static bool devlink_default_eswitch_mode_match(struct devlink *devlink)
+{
+ const char *bus_name = devlink_bus_name(devlink);
+ const char *dev_name = devlink_dev_name(devlink);
+ struct devlink_default_esw_mode_node *node;
+
+ if (devlink_default_esw_mode_match_all)
+ return true;
+
+ node = devlink_default_eswitch_mode_node_find(bus_name, dev_name);
+ return !!node;
+}
+
+void devlink_default_eswitch_mode_apply_locked(struct devlink *devlink)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ int err;
+
+ devl_assert_locked(devlink);
+
+ if (!devlink_default_eswitch_mode_match(devlink))
+ return;
+
+ if (!ops->eswitch_mode_set) {
+ if (!devlink_default_esw_mode_match_all)
+ devl_warn(devlink,
+ "devlink_eswitch_mode= selected this device but eswitch mode setting is not supported\n");
+ return;
+ }
+
+ err = devlink_eswitch_mode_set(devlink, devlink_default_esw_mode, NULL);
+ if (err)
+ devl_warn(devlink,
+ "Couldn't apply default eswitch mode, err %d\n",
+ err);
+}
+
+void devlink_default_eswitch_mode_apply_pending(struct devlink *devlink)
+{
+ devl_assert_locked(devlink);
+
+ if (!devlink->default_esw_mode_apply_pending ||
+ !__devl_is_registered(devlink))
+ return;
+
+ devlink->default_esw_mode_apply_pending = false;
+ devlink_default_eswitch_mode_apply_locked(devlink);
+}
+
+void devlink_default_eswitch_mode_init(struct devlink *devlink)
+{
+ devlink->default_esw_mode_apply_pending =
+ devlink_default_esw_mode_enabled;
+}
+
+void devlink_default_eswitch_mode_apply_pending_clear(struct devlink *devlink)
+{
+ devl_assert_locked(devlink);
+
+ devlink->default_esw_mode_apply_pending = false;
+}
+
static int __init devlink_default_eswitch_mode_setup(char *str)
{
devlink_default_esw_mode_param = str;
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 119ef105d0a7..6a8d4e1100c2 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -478,6 +478,11 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net,
return err;
WARN_ON(!(*actions_performed & BIT(action)));
+ if (*actions_performed & BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT)) {
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
+ devlink_default_eswitch_mode_apply_locked(devlink);
+ }
+
/* Catch driver on updating the remote action within devlink reload */
WARN_ON(memcmp(remote_reload_stats, devlink->stats.remote_reload_stats,
sizeof(remote_reload_stats)));
@@ -731,6 +736,7 @@ int devlink_nl_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info)
u16 mode;
if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
+ devlink_default_eswitch_mode_apply_pending_clear(devlink);
mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
err = devlink_eswitch_mode_set(devlink, mode, info->extack);
if (err)
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 8fde867f2c14..8270f91c9e84 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -58,6 +58,7 @@ struct devlink {
struct mutex lock;
struct lock_class_key lock_key;
u8 reload_failed:1;
+ u8 default_esw_mode_apply_pending:1;
refcount_t refcount;
struct rcu_work rwork;
struct devlink_rel *rel;
@@ -73,6 +74,10 @@ struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
const struct device_driver *dev_driver);
int devlink_default_eswitch_mode_cmdline_init(void);
void devlink_default_eswitch_mode_cleanup(void);
+void devlink_default_eswitch_mode_init(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_locked(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_pending(struct devlink *devlink);
+void devlink_default_eswitch_mode_apply_pending_clear(struct devlink *devlink);
#define devl_warn(devlink, format, args...) \
do { \
--
2.43.0
^ permalink raw reply related
* [PATCH net-next V7 3/4] devlink: Parse eswitch mode boot defaults
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch, Jiri Pirko
In-Reply-To: <20260716084852.549909-1-mbloch@nvidia.com>
Add devlink_eswitch_mode= kernel command line parsing for a default
eswitch mode.
The supported syntax selects either all devlink handles or one explicit
comma-separated handle list:
devlink_eswitch_mode=*=<mode>
devlink_eswitch_mode=<handle>[,<handle>...]=<mode>
where <mode> is one of legacy, switchdev or switchdev_inactive. All
selected handles receive the same mode. Assigning different modes to
different handle lists in the same parameter value is not supported.
Store the parsed selector and mode in devlink core so the default can be
applied by a downstream patch.
Document the devlink_eswitch_mode= syntax and duplicate handle handling.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
.../admin-guide/kernel-parameters.txt | 25 ++
.../networking/devlink/devlink-defaults.rst | 78 ++++++
Documentation/networking/devlink/index.rst | 1 +
net/devlink/Makefile | 2 +-
net/devlink/core.c | 7 +
net/devlink/default.c | 237 ++++++++++++++++++
net/devlink/devl_internal.h | 2 +
7 files changed, 351 insertions(+), 1 deletion(-)
create mode 100644 Documentation/networking/devlink/devlink-defaults.rst
create mode 100644 net/devlink/default.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5493a7f8f22..117300dd589c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1249,6 +1249,31 @@ Kernel parameters
dell_smm_hwmon.fan_max=
[HW] Maximum configurable fan speed.
+ devlink_eswitch_mode=
+ [NET]
+ Format:
+ <selector>=<mode>
+
+ <selector>:
+ * | <handle>[,<handle>...]
+
+ <handle>:
+ <bus-name>/<dev-name>
+
+ Configure default devlink eswitch mode for matching
+ devlink instances during device initialization.
+
+ <mode>:
+ legacy | switchdev | switchdev_inactive
+
+ Examples:
+ devlink_eswitch_mode=*=switchdev
+ devlink_eswitch_mode=pci/0000:08:00.0=switchdev
+ devlink_eswitch_mode=pci/0000:08:00.0,pci/0000:09:00.1=switchdev_inactive
+
+ See Documentation/networking/devlink/devlink-defaults.rst
+ for the full syntax.
+
dfltcc= [HW,S390]
Format: { on | off | def_only | inf_only | always }
on: s390 zlib hardware support for compression on
diff --git a/Documentation/networking/devlink/devlink-defaults.rst b/Documentation/networking/devlink/devlink-defaults.rst
new file mode 100644
index 000000000000..380c9e99210e
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-defaults.rst
@@ -0,0 +1,78 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================
+Devlink Eswitch Mode Defaults
+==============================
+
+Devlink eswitch mode defaults allow the eswitch mode to be provided on the
+kernel command line and applied to matching devlink instances during device
+initialization.
+
+The devlink device is selected by its devlink handle. For PCI devices this is
+the same handle shown by ``devlink dev show``, for example
+``pci/0000:08:00.0``.
+
+Kernel command line syntax
+==========================
+
+Defaults are specified with the ``devlink_eswitch_mode=`` kernel command line
+parameter.
+
+The general syntax is::
+
+ devlink_eswitch_mode=<selector>=<mode>
+
+``<selector>`` is either ``*`` or one or more devlink handles::
+
+ * | <bus-name>/<dev-name>[,<bus-name>/<dev-name>...]
+
+``*`` applies the mode to every devlink instance. All handles in the same
+selector receive the same eswitch mode.
+
+``<mode>`` is one of ``legacy``, ``switchdev`` or ``switchdev_inactive``.
+
+Syntax rules
+------------
+
+The following syntax rules apply:
+
+* Specify the default in one ``devlink_eswitch_mode=`` parameter. Repeated
+ ``devlink_eswitch_mode=`` parameters are not accumulated.
+* The ``devlink_eswitch_mode=`` value is limited by the kernel command line
+ size.
+* Whitespace is not allowed within the parameter value.
+* ``<selector>`` must be either ``*`` or a handle list. ``*`` cannot be
+ combined with explicit handles.
+* ``<bus-name>`` and ``<dev-name>`` must not be empty.
+* ``<dev-name>`` may contain ``:``. This allows PCI names such as
+ ``0000:08:00.0``.
+* Handles must not contain whitespace, ``*``, ``=`` or more than one ``/``.
+* A comma separates handles.
+* Comma-separated default assignments are not supported.
+* Duplicate handles are rejected and the devlink eswitch mode default is
+ ignored.
+
+The eswitch mode default corresponds to the userspace command::
+
+ devlink dev eswitch set <handle> mode <value>
+
+
+Examples
+========
+
+Set all devlink instances to switchdev mode::
+
+ devlink_eswitch_mode=*=switchdev
+
+Set one PCI devlink instance to switchdev mode::
+
+ devlink_eswitch_mode=pci/0000:08:00.0=switchdev
+
+Set two PCI devlink instances to switchdev inactive mode::
+
+ devlink_eswitch_mode=pci/0000:08:00.0,pci/0000:09:00.1=switchdev_inactive
+
+The following is invalid because comma-separated default assignments are not
+supported::
+
+ devlink_eswitch_mode=pci/0000:08:00.0=switchdev,pci/0000:09:00.0=switchdev_inactive
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 4745148fecf4..134d2f319922 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -56,6 +56,7 @@ general.
:maxdepth: 1
devlink-dpipe
+ devlink-defaults
devlink-eswitch-attr
devlink-flash
devlink-health
diff --git a/net/devlink/Makefile b/net/devlink/Makefile
index 8f2adb5e5836..99ca0ef7cf1e 100644
--- a/net/devlink/Makefile
+++ b/net/devlink/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y := core.o netlink.o netlink_gen.o dev.o port.o sb.o dpipe.o \
+obj-y := core.o netlink.o netlink_gen.o dev.o default.o port.o sb.o dpipe.o \
resource.o param.o region.o health.o trap.o rate.o linecard.o sh_dev.o
diff --git a/net/devlink/core.c b/net/devlink/core.c
index c53a42e17a58..d791a8e4317d 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -598,6 +598,10 @@ static int __init devlink_init(void)
{
int err;
+ err = devlink_default_eswitch_mode_cmdline_init();
+ if (err)
+ goto out;
+
err = register_pernet_subsys(&devlink_pernet_ops);
if (err)
goto out;
@@ -613,7 +617,10 @@ static int __init devlink_init(void)
out_unreg_pernet_subsys:
unregister_pernet_subsys(&devlink_pernet_ops);
out:
+ if (err)
+ devlink_default_eswitch_mode_cleanup();
WARN_ON(err);
+
return err;
}
diff --git a/net/devlink/default.c b/net/devlink/default.c
new file mode 100644
index 000000000000..9b15b7b23e00
--- /dev/null
+++ b/net/devlink/default.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "devl_internal.h"
+
+static char *devlink_default_esw_mode_param;
+static bool devlink_default_esw_mode_match_all;
+static enum devlink_eswitch_mode devlink_default_esw_mode;
+static LIST_HEAD(devlink_default_esw_mode_nodes);
+
+struct devlink_default_esw_mode_node {
+ struct list_head list;
+ char *bus_name;
+ char *dev_name;
+};
+
+static int __init
+devlink_default_eswitch_mode_to_value(const char *str,
+ enum devlink_eswitch_mode *mode)
+{
+ if (!strcmp(str, "legacy")) {
+ *mode = DEVLINK_ESWITCH_MODE_LEGACY;
+ return 0;
+ }
+ if (!strcmp(str, "switchdev")) {
+ *mode = DEVLINK_ESWITCH_MODE_SWITCHDEV;
+ return 0;
+ }
+ if (!strcmp(str, "switchdev_inactive")) {
+ *mode = DEVLINK_ESWITCH_MODE_SWITCHDEV_INACTIVE;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int __init
+devlink_default_eswitch_mode_handle_parse(char *handle, char **bus_name,
+ char **dev_name)
+{
+ char *slash;
+ char *p;
+
+ if (!*handle)
+ return -EINVAL;
+
+ for (p = handle; *p; p++) {
+ if (*p == '*' || *p == '=')
+ return -EINVAL;
+ }
+
+ slash = strchr(handle, '/');
+ if (!slash || slash == handle || !slash[1])
+ return -EINVAL;
+ if (strchr(slash + 1, '/'))
+ return -EINVAL;
+
+ *slash = '\0';
+
+ *bus_name = handle;
+ *dev_name = slash + 1;
+ return 0;
+}
+
+static struct devlink_default_esw_mode_node *
+devlink_default_eswitch_mode_node_find(const char *bus_name, const char *dev_name)
+{
+ struct devlink_default_esw_mode_node *node;
+
+ list_for_each_entry(node, &devlink_default_esw_mode_nodes, list) {
+ if (!strcmp(node->bus_name, bus_name) &&
+ !strcmp(node->dev_name, dev_name))
+ return node;
+ }
+
+ return NULL;
+}
+
+static int __init
+devlink_default_eswitch_mode_node_add(const char *bus_name, const char *dev_name)
+{
+ struct devlink_default_esw_mode_node *node;
+
+ if (devlink_default_eswitch_mode_node_find(bus_name, dev_name))
+ return -EEXIST;
+
+ node = kzalloc_obj(*node);
+ if (!node)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&node->list);
+ node->bus_name = kstrdup(bus_name, GFP_KERNEL);
+ node->dev_name = kstrdup(dev_name, GFP_KERNEL);
+ if (!node->bus_name || !node->dev_name) {
+ kfree(node->bus_name);
+ kfree(node->dev_name);
+ kfree(node);
+ return -ENOMEM;
+ }
+
+ list_add_tail(&node->list, &devlink_default_esw_mode_nodes);
+ return 0;
+}
+
+static int __init devlink_default_eswitch_mode_handles_parse(char *handles)
+{
+ char *handle;
+ int err;
+
+ if (!strcmp(handles, "*")) {
+ devlink_default_esw_mode_match_all = true;
+ return 0;
+ }
+
+ while ((handle = strsep(&handles, ",")) != NULL) {
+ char *bus_name;
+ char *dev_name;
+
+ err = devlink_default_eswitch_mode_handle_parse(handle, &bus_name,
+ &dev_name);
+ if (err)
+ return err;
+
+ err = devlink_default_eswitch_mode_node_add(bus_name, dev_name);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static void __init
+devlink_default_eswitch_mode_node_free(struct devlink_default_esw_mode_node *node)
+{
+ kfree(node->bus_name);
+ kfree(node->dev_name);
+ kfree(node);
+}
+
+static void __init devlink_default_eswitch_mode_nodes_clear(void)
+{
+ struct devlink_default_esw_mode_node *node_tmp;
+ struct devlink_default_esw_mode_node *node;
+
+ list_for_each_entry_safe(node, node_tmp,
+ &devlink_default_esw_mode_nodes, list) {
+ list_del(&node->list);
+ devlink_default_eswitch_mode_node_free(node);
+ }
+
+ devlink_default_esw_mode_match_all = false;
+}
+
+static int __init devlink_default_eswitch_mode_parse(char *str)
+{
+ enum devlink_eswitch_mode esw_mode;
+ char *separator;
+ char *handles;
+ char *mode;
+ int err;
+
+ if (!*str)
+ return -EINVAL;
+
+ separator = strrchr(str, '=');
+ if (!separator || separator == str || !separator[1])
+ return -EINVAL;
+
+ *separator = '\0';
+ handles = str;
+ mode = separator + 1;
+
+ err = devlink_default_eswitch_mode_to_value(mode, &esw_mode);
+ if (err)
+ return err;
+
+ err = devlink_default_eswitch_mode_handles_parse(handles);
+ if (err)
+ devlink_default_eswitch_mode_nodes_clear();
+ else
+ devlink_default_esw_mode = esw_mode;
+
+ return err;
+}
+
+static int __init devlink_default_eswitch_mode_setup(char *str)
+{
+ devlink_default_esw_mode_param = str;
+ return 1;
+}
+__setup("devlink_eswitch_mode=", devlink_default_eswitch_mode_setup);
+
+int __init devlink_default_eswitch_mode_cmdline_init(void)
+{
+ char *def;
+ int err;
+
+ if (!devlink_default_esw_mode_param)
+ return 0;
+
+ def = kstrdup(devlink_default_esw_mode_param, GFP_KERNEL);
+ if (!def) {
+ devlink_default_esw_mode_param = NULL;
+ pr_warn("devlink: devlink_eswitch_mode parameter ignored, failed to allocate memory\n");
+ return 0;
+ }
+
+ err = devlink_default_eswitch_mode_parse(def);
+ kfree(def);
+ if (err == -EEXIST) {
+ devlink_default_esw_mode_param = NULL;
+ pr_warn("devlink: duplicate eswitch mode handles ignored\n");
+ return 0;
+ } else if (err == -EINVAL) {
+ devlink_default_esw_mode_param = NULL;
+ pr_warn("devlink: invalid devlink_eswitch_mode parameter ignored\n");
+ return 0;
+ } else if (err == -ENOMEM) {
+ devlink_default_esw_mode_param = NULL;
+ pr_warn("devlink: devlink_eswitch_mode parameter ignored, failed to allocate memory\n");
+ return 0;
+ } else if (err) {
+ return err;
+ }
+
+ return 0;
+}
+
+void __init devlink_default_eswitch_mode_cleanup(void)
+{
+ devlink_default_eswitch_mode_nodes_clear();
+}
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index af43b7163f78..8fde867f2c14 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -71,6 +71,8 @@ extern struct genl_family devlink_nl_family;
struct devlink *__devlink_alloc(const struct devlink_ops *ops, size_t priv_size,
struct net *net, struct device *dev,
const struct device_driver *dev_driver);
+int devlink_default_eswitch_mode_cmdline_init(void);
+void devlink_default_eswitch_mode_cleanup(void);
#define devl_warn(devlink, format, args...) \
do { \
--
2.43.0
^ permalink raw reply related
* [PATCH net-next V7 2/4] devlink: Factor out eswitch mode setting
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch, Jiri Pirko
In-Reply-To: <20260716084852.549909-1-mbloch@nvidia.com>
Move the common eswitch mode set checks into a small helper and use it
from the netlink eswitch set command. This makes the same validation
available to the devlink core path that applies eswitch mode defaults.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
net/devlink/dev.c | 27 ++++++++++++++++++++-------
net/devlink/devl_internal.h | 3 +++
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index bcf001554e84..119ef105d0a7 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -702,6 +702,25 @@ int devlink_nl_eswitch_get_doit(struct sk_buff *skb, struct genl_info *info)
return genlmsg_reply(msg, info);
}
+int devlink_eswitch_mode_set(struct devlink *devlink,
+ enum devlink_eswitch_mode mode,
+ struct netlink_ext_ack *extack)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ int err;
+
+ devl_assert_locked(devlink);
+
+ if (!ops->eswitch_mode_set)
+ return -EOPNOTSUPP;
+
+ err = devlink_rates_check(devlink, devlink_rate_is_node, extack);
+ if (err)
+ return err;
+
+ return ops->eswitch_mode_set(devlink, mode, extack);
+}
+
int devlink_nl_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info)
{
struct devlink *devlink = devlink_nl_ctx(info)->devlink;
@@ -712,14 +731,8 @@ int devlink_nl_eswitch_set_doit(struct sk_buff *skb, struct genl_info *info)
u16 mode;
if (info->attrs[DEVLINK_ATTR_ESWITCH_MODE]) {
- if (!ops->eswitch_mode_set)
- return -EOPNOTSUPP;
- err = devlink_rates_check(devlink, devlink_rate_is_node,
- info->extack);
- if (err)
- return err;
mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
- err = ops->eswitch_mode_set(devlink, mode, info->extack);
+ err = devlink_eswitch_mode_set(devlink, mode, info->extack);
if (err)
return err;
}
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index cdf894ba5a9d..af43b7163f78 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -348,6 +348,9 @@ bool devlink_rate_is_node(const struct devlink_rate *devlink_rate);
int devlink_rates_check(struct devlink *devlink,
bool (*rate_filter)(const struct devlink_rate *),
struct netlink_ext_ack *extack);
+int devlink_eswitch_mode_set(struct devlink *devlink,
+ enum devlink_eswitch_mode mode,
+ struct netlink_ext_ack *extack);
/* Linecards */
unsigned int devlink_linecard_index(struct devlink_linecard *linecard);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next V7 1/4] net/mlx5: Clear FW reset-in-progress bit before reload
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch, Shay Drori, Moshe Shemesh
In-Reply-To: <20260716084852.549909-1-mbloch@nvidia.com>
mlx5 sets MLX5_FW_RESET_FLAGS_RESET_IN_PROGRESS when acknowledging a sync
reset request. This bit blocks devlink reload and other devlink operations
while the firmware reset is running, but it was kept set until after the
driver reload finished.
Clear the reset-in-progress bit once the reset unload flow is done and PCI
access is back, before reloading the device. For a reset initiated through
devlink, clear it before completing the reload waiter. For a reset reported
through an asynchronous firmware event, keep the unload flow outside
devl_lock, then take devl_lock before clearing the bit and reloading
through the devl-locked load helper.
Reviewed-by: Shay Drori <shayd@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
.../ethernet/mellanox/mlx5/core/fw_reset.c | 28 +++++++++++--------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index 07440c58713a..7283e5b49eed 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -238,24 +238,30 @@ static void mlx5_fw_reset_complete_reload(struct mlx5_core_dev *dev)
{
struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
struct devlink *devlink = priv_to_devlink(dev);
+ int err;
/* if this is the driver that initiated the fw reset, devlink completed the reload */
if (test_bit(MLX5_FW_RESET_FLAGS_PENDING_COMP, &fw_reset->reset_flags)) {
+ clear_bit(MLX5_FW_RESET_FLAGS_RESET_IN_PROGRESS,
+ &fw_reset->reset_flags);
complete(&fw_reset->done);
- } else {
- mlx5_sync_reset_unload_flow(dev, false);
- if (mlx5_health_wait_pci_up(dev))
- mlx5_core_err(dev, "reset reload flow aborted, PCI reads still not working\n");
- else
- mlx5_load_one(dev, true);
- devl_lock(devlink);
- devlink_remote_reload_actions_performed(devlink, 0,
- BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
- BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE));
- devl_unlock(devlink);
+ return;
}
+ mlx5_sync_reset_unload_flow(dev, false);
+ err = mlx5_health_wait_pci_up(dev);
+
+ devl_lock(devlink);
clear_bit(MLX5_FW_RESET_FLAGS_RESET_IN_PROGRESS, &fw_reset->reset_flags);
+ if (err)
+ mlx5_core_err(dev, "reset reload flow aborted, PCI reads still not working\n");
+ else
+ mlx5_load_one_devl_locked(dev, true);
+
+ devlink_remote_reload_actions_performed(devlink, 0,
+ BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
+ BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE));
+ devl_unlock(devlink);
}
static void mlx5_stop_sync_reset_poll(struct mlx5_core_dev *dev)
--
2.43.0
^ permalink raw reply related
* [PATCH net-next V7 0/4] devlink: Add boot-time eswitch mode defaults
From: Mark Bloch @ 2026-07-16 8:48 UTC (permalink / raw)
To: Jiri Pirko, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc,
Mark Bloch
This series adds a devlink_eswitch_mode= kernel command line parameter
for setting a default devlink eswitch mode during boot.
Following the discussion with Jakub[1] and the feedback on the RFC
postings, this version keeps the scope limited to a boot-time devlink
eswitch mode default only.
The option selects either all devlink handles or an explicit
comma-separated handle list:
devlink_eswitch_mode=*=switchdev
devlink_eswitch_mode=pci/0000:08:00.0,pci/0000:09:00.1=switchdev_inactive
The supported modes are legacy, switchdev and switchdev_inactive. The
selected mode is applied through the existing eswitch_mode_set() devlink
operation, the same operation used by the devlink eswitch mode command.
Registration may happen while a driver holds the devlink lock and
continues device initialization. Devlink core marks the default as
pending and applies it from devl_unlock() once the instance is
registered, before releasing the lock. This prevents userspace from
racing with the boot default.
After a successful reload that performed DRIVER_REINIT, devlink core
already holds the devlink instance lock and the driver completed
reload_up(), so the default is applied directly from the reload path.
Patch 1 clears the mlx5 FW reset-in-progress bit before reload.
Patch 2 factors the common eswitch mode set validation into a helper.
Patch 3 adds the devlink_eswitch_mode= parser and documentation.
Patch 4 applies parsed defaults from devlink core.
Changelog:
v6 -> v7:
- Added Reviewed-by tags from Jiri Pirko.
- Renamed the default eswitch mode functions to spell out eswitch instead
of esw.
- Renamed the per-devlink initialization helper to
devlink_default_eswitch_mode_init(), dropping instance from its name.
v5 -> v6:
- Dropped regular work and applied a pending default directly from
devl_unlock() while the devlink instance lock is still held.
- Dropped the driver API and mlx5-specific default application patches.
v4 -> v5:
- Moved the default eswitch mode code into a separate file, per Jiri's
comment.
- Dropped the delayed workqueue and switched to regular work triggered
via devl_unlock(), per Jiri's comment.
- Renamed some functions to better align with devlink code.
v3 -> v4:
- Rework registration time apply to use per devlink work queued from
devl_unlock(), instead of calling eswitch_mode_set() directly from
devl_register().
- Apply the default directly after successful DRIVER_REINIT devlink reload,
where the devlink lock is already held and reload_up() has completed.
- Add devl_apply_default_esw_mode() for drivers that know their exact ready
point.
- Drop the driver registration-ordering preparation patches that are no
longer needed with the async registration apply path.
v2 -> v3:
- Change the devlink_eswitch_mode= API syntax to use <selector>=<mode>
instead of [<selector>]:<mode>, following a comment from Randy Dunlap.
v1 -> v2:
- Move default eswitch mode application into devlink core. The default is
now applied during devlink registration and after a successful devlink
reload that performed DRIVER_REINIT.
- Remove the exported devl_apply_default_esw_mode() driver API and the mlx5
driver-side call to it.
- Skip devlink health recovery notifications while the devlink instance is
not registered, so drivers can move registration later without early
health work hitting registration assertions.
- Move mlx5 devlink registration after device initialization, including the
lightweight init path, so the core can apply the default through the
normal registration flow.
- Move the matching netdevsim and mlx5 unregister paths before object
teardown, so unregister notifications come from devl_unregister() and the
later object teardown paths run while the devlink instance is no longer
registered.
- Add registration-ordering preparation patches for netdevsim and octeontx2
AF/PF, so their eswitch state is ready before registration-time defaults
may call eswitch_mode_set().
[1] lore.kernel.org/r/20260502184153.4fd8d06f@kernel.org/
RFC v1: lore.kernel.org/r/20260506123739.1959770-1-mbloch@nvidia.com/
RFC v2: lore.kernel.org/r/20260510185424.2041415-1-mbloch@nvidia.com/
v1: lore.kernel.org/r/20260521072434.362624-1-tariqt@nvidia.com/
v2: lore.kernel.org/all/20260603193259.3412464-1-mbloch@nvidia.com/
v3: lore.kernel.org/all/20260605181030.3486619-1-mbloch@nvidia.com/
v4: lore.kernel.org/all/20260629182102.245150-1-mbloch@nvidia.com/
v5: lore.kernel.org/all/20260707174527.425134-1-mbloch@nvidia.com/
v6: lore.kernel.org/all/20260714061731.531849-1-mbloch@nvidia.com/
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
Mark Bloch (4):
net/mlx5: Clear FW reset-in-progress bit before reload
devlink: Factor out eswitch mode setting
devlink: Parse eswitch mode boot defaults
devlink: Apply eswitch mode boot defaults
.../admin-guide/kernel-parameters.txt | 25 ++
.../networking/devlink/devlink-defaults.rst | 78 +++++
Documentation/networking/devlink/index.rst | 1 +
.../ethernet/mellanox/mlx5/core/fw_reset.c | 28 +-
net/devlink/Makefile | 2 +-
net/devlink/core.c | 10 +
net/devlink/default.c | 303 ++++++++++++++++++
net/devlink/dev.c | 33 +-
net/devlink/devl_internal.h | 10 +
9 files changed, 471 insertions(+), 19 deletions(-)
create mode 100644 Documentation/networking/devlink/devlink-defaults.rst
create mode 100644 net/devlink/default.c
base-commit: f6f3b36c15ed44de1fbb44e645e4fae8c4a4453e
--
2.43.0
^ permalink raw reply
* [PATCH 3/3] hwmon: (pmbus/max34440): Add support for MAX34452
From: Alexis Czezar Torreno @ 2026-07-16 8:25 UTC (permalink / raw)
To: Guenter Roeck, Kun Yi, Nuno Sá, Jonathan Corbet, Shuah Khan
Cc: linux-hwmon, linux-kernel, linux-doc, Alexis Czezar Torreno,
Carlos Jones Jr
In-Reply-To: <20260716-max34451_fixes-v1-0-a941b27eaecb@analog.com>
From: Carlos Jones Jr <carlosjr.jones@analog.com>
Add support for Maxim MAX34452 PMBus 16-Channel V/I Monitor and
12-Channel Sequencer/Marginer. The device is similar to MAX34451
and shares the same configuration function.
The MAX34452 supports:
- 16 configurable voltage/current monitoring channels
- 5 temperature sensors (pages 16-20)
- Dynamic channel configuration via MFR_CHANNEL_CONFIG
- IOUT average monitoring
Signed-off-by: Carlos Jones Jr <carlosjr.jones@analog.com>
Co-Developed by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Documentation/hwmon/max34440.rst | 26 +++++++++++------
drivers/hwmon/pmbus/Kconfig | 5 ++--
drivers/hwmon/pmbus/max34440.c | 61 ++++++++++++++++++++++++++++++++--------
3 files changed, 71 insertions(+), 21 deletions(-)
diff --git a/Documentation/hwmon/max34440.rst b/Documentation/hwmon/max34440.rst
index e7421f4dbf38fc1436bbaeba71d4461a00f8cefb..866f22b88e9b360ff54aa52807484bb3e859e56e 100644
--- a/Documentation/hwmon/max34440.rst
+++ b/Documentation/hwmon/max34440.rst
@@ -65,6 +65,16 @@ Supported chips:
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max34451.pdf
+ * Maxim MAX34452
+
+ PMBus 16-Channel V/I Monitor and 12-Channel Sequencer/Marginer
+
+ Prefixes: 'max34452'
+
+ Addresses scanned: -
+
+ Datasheet: -
+
* Maxim MAX34460
PMBus 12-Channel Voltage Monitor & Sequencer
@@ -94,11 +104,11 @@ Description
This driver supports multiple devices: hardware monitoring for Maxim MAX34440
PMBus 6-Channel Power-Supply Manager, MAX34441 PMBus 5-Channel Power-Supply
Manager and Intelligent Fan Controller, and MAX34446 PMBus Power-Supply Data
-Logger; PMBus Voltage Monitor and Sequencers for MAX34451, MAX34460, and
-MAX34461; PMBus DC/DC Power Module ADPM12160, ADPM12200, and ADPM12250. The
-MAX34451 supports monitoring voltage or current of 12 channels based on GIN
-pins. The MAX34460 supports 12 voltage channels, and the MAX34461 supports 16
-voltage channels. The ADPM12160, ADPM12200, and ADPM12250 also monitors both
+Logger; PMBus Voltage Monitor and Sequencers for MAX34451, MAX34452, MAX34460,
+and MAX34461; PMBus DC/DC Power Module ADPM12160, ADPM12200, and ADPM12250. The
+MAX34451 and MAX34452 support monitoring voltage or current of 16 channels based
+on GIN pins. The MAX34460 supports 12 voltage channels, and the MAX34461 supports
+16 voltage channels. The ADPM12160, ADPM12200, and ADPM12250 also monitor both
input and output of voltage and current.
The driver is a client driver to the core PMBus driver. Please see
@@ -171,7 +181,7 @@ curr[1-6]_crit Critical maximum current. From IOUT_OC_FAULT_LIMIT
register.
curr[1-6]_max_alarm Current high alarm. From IOUT_OC_WARNING status.
curr[1-6]_crit_alarm Current critical high alarm. From IOUT_OC_FAULT status.
-curr[1-4]_average Historical average current (MAX34446/34451 only).
+curr[1-4]_average Historical average current (MAX34446/34451/34452 only).
curr[1-6]_highest Historical maximum current.
curr[1-6]_reset_history Write any value to reset history.
======================= ========================================================
@@ -223,7 +233,7 @@ temp[1-8]_reset_history Write any value to reset history.
.. note::
- - MAX34451 supports attribute groups in[1-16] (or curr[1-16] based on
- input pins) and temp[1-5].
+ - MAX34451 and MAX34452 support attribute groups in[1-16] (or curr[1-16]
+ based on input pins) and temp[1-5].
- MAX34460 supports attribute groups in[1-12] and temp[1-5].
- MAX34461 supports attribute groups in[1-16] and temp[1-5].
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index ae003a6eac30a7b0a022c5f7f467d8f81736c3f7..91fa1c19e87c9f2de789f2908e59ac00c228eb76 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -434,8 +434,9 @@ config SENSORS_MAX34440
tristate "Maxim MAX34440 and compatibles"
help
If you say yes here you get hardware monitoring support for Maxim
- MAX34440, MAX34441, MAX34446, MAX34451, MAX34460, and MAX34461.
- Other compatible are ADPM12160, and ADPM12200.
+ MAX34440, MAX34441, MAX34446, MAX34451, MAX34452, MAX34460, and
+ MAX34461. Other compatible devices are ADPM12160, ADPM12200, and
+ ADPM12250.
This driver can also be built as a module. If so, the module will
be called max34440.
diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
index 024109df26db568a4c230c9dbc45d69ba531dd8d..2e57af09f47809b0a0bddab27fc47cadfe35e0ef 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -23,6 +23,7 @@ enum chips {
max34441,
max34446,
max34451,
+ max34452,
max34460,
max34461,
};
@@ -116,6 +117,10 @@ static int max34440_read_word_data(struct i2c_client *client, int page,
return -ENXIO;
ret = -ENODATA;
break;
+ case PMBUS_VOUT_OV_WARN_LIMIT:
+ if (data->id == max34452)
+ return -ENXIO;
+ return -ENODATA;
case PMBUS_VIRT_READ_VOUT_MIN:
ret = pmbus_read_word_data(client, page, phase,
MAX34440_MFR_VOUT_MIN);
@@ -126,8 +131,8 @@ static int max34440_read_word_data(struct i2c_client *client, int page,
break;
case PMBUS_VIRT_READ_IOUT_AVG:
if (data->id != max34446 && data->id != max34451 &&
- data->id != adpm12160 && data->id != adpm12200 &&
- data->id != adpm12250)
+ data->id != max34452 && data->id != adpm12160 &&
+ data->id != adpm12200 && data->id != adpm12250)
return -ENXIO;
ret = pmbus_read_word_data(client, page, phase,
MAX34446_MFR_IOUT_AVG);
@@ -192,6 +197,10 @@ static int max34440_write_word_data(struct i2c_client *client, int page,
ret = pmbus_write_word_data(client, page, data->iout_oc_warn_limit,
word);
break;
+ case PMBUS_VOUT_OV_WARN_LIMIT:
+ if (data->id == max34452)
+ return -ENXIO;
+ return -ENODATA;
case PMBUS_VIRT_RESET_POUT_HISTORY:
ret = pmbus_write_word_data(client, page,
MAX34446_MFR_POUT_PEAK, 0);
@@ -212,8 +221,8 @@ static int max34440_write_word_data(struct i2c_client *client, int page,
ret = pmbus_write_word_data(client, page,
MAX34440_MFR_IOUT_PEAK, 0);
if (!ret && (data->id == max34446 || data->id == max34451 ||
- data->id == adpm12160 || data->id == adpm12200 ||
- data->id == adpm12250))
+ data->id == max34452 || data->id == adpm12160 ||
+ data->id == adpm12200 || data->id == adpm12250))
ret = pmbus_write_word_data(client, page,
MAX34446_MFR_IOUT_AVG, 0);
@@ -281,12 +290,13 @@ static int max34451_read_byte_data(struct i2c_client *client, int page, int reg)
case PMBUS_STATUS_BYTE:
case PMBUS_STATUS_OTHER:
/*
- * MAX34451/ADPM family do not support STATUS_BYTE or
+ * MAX34451/MAX34452/ADPM family do not support STATUS_BYTE or
* STATUS_OTHER registers. Accessing them triggers CML
* error and asserts ALERT.
*/
- if (data->id == max34451 || data->id == adpm12160 ||
- data->id == adpm12200 || data->id == adpm12250)
+ if (data->id == max34451 || data->id == max34452 ||
+ data->id == adpm12160 || data->id == adpm12200 ||
+ data->id == adpm12250)
return -ENXIO;
return -ENODATA;
default:
@@ -304,12 +314,13 @@ static int max34451_write_byte_data(struct i2c_client *client, int page,
case PMBUS_STATUS_BYTE:
case PMBUS_STATUS_OTHER:
/*
- * MAX34451/ADPM family do not support STATUS_BYTE or
+ * MAX34451/MAX34452/ADPM family do not support STATUS_BYTE or
* STATUS_OTHER registers. Writing to them triggers CML
* error and asserts ALERT.
*/
- if (data->id == max34451 || data->id == adpm12160 ||
- data->id == adpm12200 || data->id == adpm12250)
+ if (data->id == max34451 || data->id == max34452 ||
+ data->id == adpm12160 || data->id == adpm12200 ||
+ data->id == adpm12250)
return -ENXIO;
return -ENODATA;
default:
@@ -340,6 +351,7 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
if (rv < 0)
return rv;
+ /* MAX34452's latest MFR_REV is only at 0x0004, will skip this part */
if (rv >= MAX34451ETNA6_MFR_REV) {
max34451_na6 = true;
data->info.format[PSC_VOLTAGE_IN] = direct;
@@ -678,6 +690,32 @@ static struct pmbus_driver_info max34440_info[] = {
.write_word_data = max34440_write_word_data,
.page_change_delay = MAX34440_PAGE_CHANGE_DELAY,
},
+ [max34452] = {
+ .pages = 21,
+ .format[PSC_VOLTAGE_OUT] = direct,
+ .format[PSC_TEMPERATURE] = direct,
+ .format[PSC_CURRENT_OUT] = direct,
+ .m[PSC_VOLTAGE_OUT] = 1,
+ .b[PSC_VOLTAGE_OUT] = 0,
+ .R[PSC_VOLTAGE_OUT] = 3,
+ .m[PSC_CURRENT_OUT] = 1,
+ .b[PSC_CURRENT_OUT] = 0,
+ .R[PSC_CURRENT_OUT] = 2,
+ .m[PSC_TEMPERATURE] = 1,
+ .b[PSC_TEMPERATURE] = 0,
+ .R[PSC_TEMPERATURE] = 2,
+ /* func 0-15 is set dynamically before probing */
+ .func[16] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .func[17] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .func[19] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .func[20] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .read_byte_data = max34451_read_byte_data,
+ .read_word_data = max34440_read_word_data,
+ .write_byte_data = max34451_write_byte_data,
+ .write_word_data = max34440_write_word_data,
+ .page_change_delay = MAX34440_PAGE_CHANGE_DELAY,
+ },
[max34460] = {
.pages = 18,
.format[PSC_VOLTAGE_OUT] = direct,
@@ -761,7 +799,7 @@ static int max34440_probe(struct i2c_client *client)
data->iout_oc_fault_limit = MAX34440_IOUT_OC_FAULT_LIMIT;
data->iout_oc_warn_limit = MAX34440_IOUT_OC_WARN_LIMIT;
- if (data->id == max34451) {
+ if (data->id == max34451 || data->id == max34452) {
rv = max34451_set_supported_funcs(client, data);
if (rv)
return rv;
@@ -782,6 +820,7 @@ static const struct i2c_device_id max34440_id[] = {
{ .name = "max34441", .driver_data = max34441 },
{ .name = "max34446", .driver_data = max34446 },
{ .name = "max34451", .driver_data = max34451 },
+ { .name = "max34452", .driver_data = max34452 },
{ .name = "max34460", .driver_data = max34460 },
{ .name = "max34461", .driver_data = max34461 },
{ }
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] hwmon: (pmbus/max34440): add support for newer version of max34451
From: Alexis Czezar Torreno @ 2026-07-16 8:25 UTC (permalink / raw)
To: Guenter Roeck, Kun Yi, Nuno Sá, Jonathan Corbet, Shuah Khan
Cc: linux-hwmon, linux-kernel, linux-doc, Alexis Czezar Torreno
In-Reply-To: <20260716-max34451_fixes-v1-0-a941b27eaecb@analog.com>
The MAX34451 released a newer version called max34451etna8+.
This simply changes the direct format coefficients
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
drivers/hwmon/pmbus/max34440.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
index e56057e9273c1639bddbaf4e665ea159f1d7c3ed..024109df26db568a4c230c9dbc45d69ba531dd8d 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -59,6 +59,7 @@ enum chips {
#define MAX34440_IOUT_OC_FAULT_LIMIT 0x4A
#define MAX34451ETNA6_MFR_REV 0x0012
+#define MAX34451ETNA8_MFR_REV 0x0014
#define MAX34451_MFR_CHANNEL_CONFIG 0xe4
#define MAX34451_MFR_CHANNEL_CONFIG_SEL_MASK 0x3f
@@ -343,12 +344,24 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
max34451_na6 = true;
data->info.format[PSC_VOLTAGE_IN] = direct;
data->info.format[PSC_CURRENT_IN] = direct;
- data->info.m[PSC_VOLTAGE_IN] = 1;
+
data->info.b[PSC_VOLTAGE_IN] = 0;
- data->info.R[PSC_VOLTAGE_IN] = 3;
- data->info.m[PSC_CURRENT_IN] = 1;
data->info.b[PSC_CURRENT_IN] = 0;
- data->info.R[PSC_CURRENT_IN] = 2;
+ if (rv == MAX34451ETNA8_MFR_REV) {
+ data->info.m[PSC_VOLTAGE_IN] = 125;
+ data->info.R[PSC_VOLTAGE_IN] = 0;
+ data->info.m[PSC_VOLTAGE_OUT] = 125;
+ data->info.R[PSC_VOLTAGE_OUT] = 0;
+ data->info.m[PSC_CURRENT_IN] = 250;
+ data->info.R[PSC_CURRENT_IN] = -1;
+ data->info.m[PSC_CURRENT_OUT] = 250;
+ data->info.R[PSC_CURRENT_OUT] = -1;
+ } else {
+ data->info.m[PSC_VOLTAGE_IN] = 1;
+ data->info.R[PSC_VOLTAGE_IN] = 3;
+ data->info.m[PSC_CURRENT_IN] = 1;
+ data->info.R[PSC_CURRENT_IN] = 2;
+ }
data->iout_oc_fault_limit = PMBUS_IOUT_OC_FAULT_LIMIT;
data->iout_oc_warn_limit = PMBUS_IOUT_OC_WARN_LIMIT;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 1/3] hwmon: (pmbus/max34440): block unsupported VIN and IIN limit registers
From: Alexis Czezar Torreno @ 2026-07-16 8:25 UTC (permalink / raw)
To: Guenter Roeck, Kun Yi, Nuno Sá, Jonathan Corbet, Shuah Khan
Cc: linux-hwmon, linux-kernel, linux-doc, Alexis Czezar Torreno
In-Reply-To: <20260716-max34451_fixes-v1-0-a941b27eaecb@analog.com>
MAX34451 and ADPM chips do not support standard PMBus VIN/IIN limit
registers, manufacturer specific min/max registers, or undercurrent
or undertemperature fault limits. STATUS_BYTE and STATUS_OTHER are also
not available. Accessing these non-existent registers during driver
initialization triggers a CML error and asserts ALERT. Handled by
blocking these functions during read/write.
Fixes: 7a001dbab4ad ("hwmon: (pmbus/max34440) Add support for MAX34451.")
Fixes: 629cf8f6c23a ("hwmon: (pmbus/max34440) Add support for ADPM12160")
Fixes: 2e0b52f1ae88 ("hwmon: (pmbus/max34440): add support adpm12200")
Fixes: 479bfeba2eb6 ("hwmon: (pmbus/max34440): add support adpm12250")
Tested-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
drivers/hwmon/pmbus/max34440.c | 80 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
index 74876d2207fbe4014b8b54a9fd9682370fc3bbed..e56057e9273c1639bddbaf4e665ea159f1d7c3ed 100644
--- a/drivers/hwmon/pmbus/max34440.c
+++ b/drivers/hwmon/pmbus/max34440.c
@@ -88,6 +88,33 @@ static int max34440_read_word_data(struct i2c_client *client, int page,
ret = pmbus_read_word_data(client, page, phase,
data->iout_oc_warn_limit);
break;
+ case PMBUS_VIN_OV_FAULT_LIMIT:
+ case PMBUS_VIN_OV_WARN_LIMIT:
+ case PMBUS_VIN_UV_WARN_LIMIT:
+ case PMBUS_VIN_UV_FAULT_LIMIT:
+ case PMBUS_MFR_VIN_MIN:
+ case PMBUS_MFR_VIN_MAX:
+ case PMBUS_IIN_OC_WARN_LIMIT:
+ case PMBUS_IIN_OC_FAULT_LIMIT:
+ case PMBUS_MFR_IIN_MAX:
+ case PMBUS_MFR_VOUT_MIN:
+ case PMBUS_MFR_VOUT_MAX:
+ case PMBUS_IOUT_UC_FAULT_LIMIT:
+ case PMBUS_MFR_IOUT_MAX:
+ case PMBUS_UT_WARN_LIMIT:
+ case PMBUS_UT_FAULT_LIMIT:
+ case PMBUS_MFR_MAX_TEMP_1:
+ /*
+ * MAX34451/ADPM family do not support VIN/IIN limit registers,
+ * manufacturer-specific min/max registers, or undercurrent/
+ * undertemperature fault limits. Accessing these triggers CML
+ * error and asserts ALERT.
+ */
+ if (data->id == max34451 || data->id == adpm12160 ||
+ data->id == adpm12200 || data->id == adpm12250)
+ return -ENXIO;
+ ret = -ENODATA;
+ break;
case PMBUS_VIRT_READ_VOUT_MIN:
ret = pmbus_read_word_data(client, page, phase,
MAX34440_MFR_VOUT_MIN);
@@ -244,6 +271,51 @@ static int max34440_read_byte_data(struct i2c_client *client, int page, int reg)
return ret;
}
+static int max34451_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+ const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+ const struct max34440_data *data = to_max34440_data(info);
+
+ switch (reg) {
+ case PMBUS_STATUS_BYTE:
+ case PMBUS_STATUS_OTHER:
+ /*
+ * MAX34451/ADPM family do not support STATUS_BYTE or
+ * STATUS_OTHER registers. Accessing them triggers CML
+ * error and asserts ALERT.
+ */
+ if (data->id == max34451 || data->id == adpm12160 ||
+ data->id == adpm12200 || data->id == adpm12250)
+ return -ENXIO;
+ return -ENODATA;
+ default:
+ return -ENODATA;
+ }
+}
+
+static int max34451_write_byte_data(struct i2c_client *client, int page,
+ int reg, u8 byte)
+{
+ const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+ const struct max34440_data *data = to_max34440_data(info);
+
+ switch (reg) {
+ case PMBUS_STATUS_BYTE:
+ case PMBUS_STATUS_OTHER:
+ /*
+ * MAX34451/ADPM family do not support STATUS_BYTE or
+ * STATUS_OTHER registers. Writing to them triggers CML
+ * error and asserts ALERT.
+ */
+ if (data->id == max34451 || data->id == adpm12160 ||
+ data->id == adpm12200 || data->id == adpm12250)
+ return -ENXIO;
+ return -ENODATA;
+ default:
+ return -ENODATA;
+ }
+}
+
static int max34451_set_supported_funcs(struct i2c_client *client,
struct max34440_data *data)
{
@@ -363,7 +435,9 @@ static struct pmbus_driver_info max34440_info[] = {
.func[9] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT,
.func[10] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT,
.func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .read_byte_data = max34451_read_byte_data,
.read_word_data = max34440_read_word_data,
+ .write_byte_data = max34451_write_byte_data,
.write_word_data = max34440_write_word_data,
},
[adpm12200] = {
@@ -399,7 +473,9 @@ static struct pmbus_driver_info max34440_info[] = {
.func[10] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT,
.func[14] = PMBUS_HAVE_IOUT,
.func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .read_byte_data = max34451_read_byte_data,
.read_word_data = max34440_read_word_data,
+ .write_byte_data = max34451_write_byte_data,
.write_word_data = max34440_write_word_data,
},
[adpm12250] = {
@@ -433,7 +509,9 @@ static struct pmbus_driver_info max34440_info[] = {
.func[10] = PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT,
.func[14] = PMBUS_HAVE_IOUT,
.func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .read_byte_data = max34451_read_byte_data,
.read_word_data = max34440_read_word_data,
+ .write_byte_data = max34451_write_byte_data,
.write_word_data = max34440_write_word_data,
},
[max34440] = {
@@ -581,7 +659,9 @@ static struct pmbus_driver_info max34440_info[] = {
.func[18] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
.func[19] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
.func[20] = PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
+ .read_byte_data = max34451_read_byte_data,
.read_word_data = max34440_read_word_data,
+ .write_byte_data = max34451_write_byte_data,
.write_word_data = max34440_write_word_data,
.page_change_delay = MAX34440_PAGE_CHANGE_DELAY,
},
--
2.34.1
^ permalink raw reply related
* [PATCH 0/3] hwmon: (pmbus/max34440): Bug fixes and supporting new device
From: Alexis Czezar Torreno @ 2026-07-16 8:25 UTC (permalink / raw)
To: Guenter Roeck, Kun Yi, Nuno Sá, Jonathan Corbet, Shuah Khan
Cc: linux-hwmon, linux-kernel, linux-doc, Alexis Czezar Torreno,
Carlos Jones Jr
This series fixes register access issues in MAX34451/ADPM devices and adds
support for newer MAX34451 variants and the MAX34452 device.
The first patch addresses an issue where the driver attempts to access
unsupported registers on MAX34451 and ADPM devices, causing CML errors and
triggering ALERT.
The second patch adds support for the newer MAX34451ETNA8+ silicon revision
which uses different direct format coefficients for voltage and current
measurements.
The third patch introduces support for the MAX34452, a 16-channel voltage/
current monitor that shares a similar architecture with MAX34451.
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Alexis Czezar Torreno (2):
hwmon: (pmbus/max34440): block unsupported VIN and IIN limit registers
hwmon: (pmbus/max34440): add support for newer version of max34451
Carlos Jones Jr (1):
hwmon: (pmbus/max34440): Add support for MAX34452
Documentation/hwmon/max34440.rst | 26 ++++---
drivers/hwmon/pmbus/Kconfig | 5 +-
drivers/hwmon/pmbus/max34440.c | 150 ++++++++++++++++++++++++++++++++++++---
3 files changed, 162 insertions(+), 19 deletions(-)
---
base-commit: ca078d004cf58137bcf8cb24a8b271397431ba58
change-id: 20260716-max34451_fixes-788edb623116
Best regards,
--
Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
^ permalink raw reply
* Re: [PATCH v5 3/5] rpmsg: virtio_rpmsg_bus: get buffer size from config space
From: Arnaud POULIQUEN @ 2026-07-16 8:19 UTC (permalink / raw)
To: Mathieu Poirier, Tanmay Shah
Cc: andersson, corbet, skhan, linux-remoteproc, linux-doc,
linux-kernel
In-Reply-To: <ale0KIu6hXNvN0q9@p14s>
Hello Mathieu,
On 7/15/26 18:24, Mathieu Poirier wrote:
> On Fri, Jul 10, 2026 at 12:28:29PM -0700, Tanmay Shah wrote:
>> 512 bytes isn't always suitable for all case, let firmware
>> maker decide the best value from resource table.
>> enable by VIRTIO_RPMSG_F_BUFSZ feature bit.
>>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>> Changes in v5:
>>
>> - fix documentation about alignment of the buffer size
>> - change version field from u16 to u8
>> - remove buffer alignment check
>> - Separate buffer alignment vs MTU of a single buffer
>> - Use buffer alignment only to get next buffer address at alignment
>> boundary
>>
>> Changes in v4:
>>
>> - Introduce new patch to modify rpmsg.rst documentation
>> - check version is always 1.
>> - check size field is same as size of struct virtio_rpmsg_config
>> - introduce alignment field
>> - check alignment field is power of 2
>> - check tx and rx buf size is aligned with alignment passed in the
>> structure
>>
>> Changes in v3:
>>
>> - change version field from u16 to u8
>> - introduce size field in the rpmsg_virtio_config structure
>> - check version field is set to any non-zero value.
>> - check size field is not 0.
>> - Remove field for private config, as not needed for now.
>> - add documentation of rpmsg_virtio_config structure
>>
>> drivers/rpmsg/virtio_rpmsg_bus.c | 130 ++++++++++++++++++++++++-----
>> include/linux/rpmsg/virtio_rpmsg.h | 48 +++++++++++
>> 2 files changed, 159 insertions(+), 19 deletions(-)
>> create mode 100644 include/linux/rpmsg/virtio_rpmsg.h
>>
>> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
>> index c84b6cec9caf..5e5473f4adeb 100644
>> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
>> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
>> @@ -15,11 +15,13 @@
>> #include <linux/idr.h>
>> #include <linux/jiffies.h>
>> #include <linux/kernel.h>
>> +#include <linux/log2.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> #include <linux/rpmsg.h>
>> #include <linux/rpmsg/byteorder.h>
>> #include <linux/rpmsg/ns.h>
>> +#include <linux/rpmsg/virtio_rpmsg.h>
>> #include <linux/scatterlist.h>
>> #include <linux/slab.h>
>> #include <linux/sched.h>
>> @@ -39,7 +41,10 @@
>> * @tx_bufs: kernel address of tx buffers
>> * @num_rx_buf: total number of rx buffers
>> * @num_tx_buf: total number of tx buffers
>> - * @buf_size: size of one rx or tx buffer
>> + * @rx_buf_size: size of one rx buffer
>> + * @tx_buf_size: size of one tx buffer
>> + * @rx_buf_size_aligned: aligned size of one rx buffer
>> + * @tx_buf_size_aligned: aligned size of one tx buffer
>> * @last_tx_buf: index of last tx buffer used
>> * @bufs_dma: dma base addr of the buffers
>> * @tx_lock: protects svq and tx_bufs, to allow concurrent senders.
>> @@ -59,7 +64,10 @@ struct virtproc_info {
>> void *rx_bufs, *tx_bufs;
>> unsigned int num_rx_buf;
>> unsigned int num_tx_buf;
>> - unsigned int buf_size;
>> + unsigned int rx_buf_size;
>> + unsigned int tx_buf_size;
>> + unsigned int rx_buf_size_aligned;
>> + unsigned int tx_buf_size_aligned;
>> int last_tx_buf;
>> dma_addr_t bufs_dma;
>> struct mutex tx_lock;
>> @@ -68,9 +76,6 @@ struct virtproc_info {
>> wait_queue_head_t sendq;
>> };
>>
>> -/* The feature bitmap for virtio rpmsg */
>> -#define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */
>> -
>> /**
>> * struct rpmsg_hdr - common header for all rpmsg messages
>> * @src: source address
>> @@ -128,7 +133,7 @@ struct virtio_rpmsg_channel {
>> * processor.
>> */
>> #define MAX_RPMSG_NUM_BUFS (256)
>> -#define MAX_RPMSG_BUF_SIZE (512)
>> +#define DEFAULT_RPMSG_BUF_SIZE (512)
>>
>> /*
>> * Local addresses are dynamically allocated on-demand.
>> @@ -443,7 +448,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
>>
>> /* either pick the next unused tx buffer */
>> if (vrp->last_tx_buf < vrp->num_tx_buf)
>> - ret = vrp->tx_bufs + vrp->buf_size * vrp->last_tx_buf++;
>> + ret = vrp->tx_bufs + vrp->tx_buf_size_aligned * vrp->last_tx_buf++;
>> /* or recycle a used one */
>> else
>> ret = virtqueue_get_buf(vrp->svq, &len);
>> @@ -513,7 +518,7 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
>> * messaging), or to improve the buffer allocator, to support
>> * variable-length buffer sizes.
>> */
>> - if (len > vrp->buf_size - sizeof(struct rpmsg_hdr)) {
>> + if (len > vrp->tx_buf_size - sizeof(struct rpmsg_hdr)) {
>> dev_err(dev, "message is too big (%d)\n", len);
>> return -EMSGSIZE;
>> }
>> @@ -646,7 +651,7 @@ static ssize_t virtio_rpmsg_get_mtu(struct rpmsg_endpoint *ept)
>> struct rpmsg_device *rpdev = ept->rpdev;
>> struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
>>
>> - return vch->vrp->buf_size - sizeof(struct rpmsg_hdr);
>> + return vch->vrp->tx_buf_size - sizeof(struct rpmsg_hdr);
>> }
>>
>> static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>> @@ -672,7 +677,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>> * We currently use fixed-sized buffers, so trivially sanitize
>> * the reported payload length.
>> */
>> - if (len > vrp->buf_size ||
>> + if (len > vrp->rx_buf_size ||
>> msg_len > (len - sizeof(struct rpmsg_hdr))) {
>> dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
>> return -EINVAL;
>> @@ -705,7 +710,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
>> dev_warn_ratelimited(dev, "msg received with no recipient\n");
>>
>> /* publish the real size of the buffer */
>> - rpmsg_sg_init(&sg, msg, vrp->buf_size);
>> + rpmsg_sg_init(&sg, msg, vrp->rx_buf_size);
>>
>> /* add the buffer back to the remote processor's virtqueue */
>> err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, msg, GFP_KERNEL);
>> @@ -819,10 +824,13 @@ static int rpmsg_probe(struct virtio_device *vdev)
>> struct virtproc_info *vrp;
>> struct virtio_rpmsg_channel *vch = NULL;
>> struct rpmsg_device *rpdev_ns, *rpdev_ctrl;
>> + u16 rpmsg_buf_align = 0;
>> void *bufs_va;
>> int err = 0, i;
>> size_t total_buf_space;
>> bool notify;
>> + u8 version;
>> + u16 size;
>>
>> vrp = kzalloc_obj(*vrp);
>> if (!vrp)
>> @@ -854,9 +862,87 @@ static int rpmsg_probe(struct virtio_device *vdev)
>> else
>> vrp->num_tx_buf = MAX_RPMSG_NUM_BUFS;
>>
>> - vrp->buf_size = MAX_RPMSG_BUF_SIZE;
>> + /*
>> + * If VIRTIO_RPMSG_F_BUFSZ feature is supported, then configure buf
>> + * size from virtio device config space from the resource table.
>> + * If the feature is not supported, then assign default buf size.
>> + */
>> + if (virtio_has_feature(vdev, VIRTIO_RPMSG_F_BUFSZ)) {
>> + virtio_cread(vdev, struct virtio_rpmsg_config,
>> + version, &version);
>> +
>> + /* for now we support only v1 */
>> + if (version != RPMSG_VDEV_CONFIG_V1) {
>> + dev_err(&vdev->dev,
>> + "unsupported vdev config version %u\n", version);
>> + err = -EINVAL;
>> + goto vqs_del;
>> + }
>> +
>> + /* size of the config space must match */
>> + virtio_cread(vdev, struct virtio_rpmsg_config,
>> + size, &size);
>> + if (size != sizeof(struct virtio_rpmsg_config)) {
>> + dev_err(&vdev->dev, "invalid size of vdev config %u\n",
>> + size);
>> + err = -EINVAL;
>> + goto vqs_del;
>> + }
>>
>> - total_buf_space = (vrp->num_rx_buf + vrp->num_tx_buf) * vrp->buf_size;
>> + /*
>> + * Optional alignment applied to each buffer size and to the TX
>> + * buffer base address (e.g. to align buffers on a cache line).
>> + * It must be a power of two; zero means no extra alignment.
>> + */
>> + virtio_cread(vdev, struct virtio_rpmsg_config,
>> + rpmsg_buf_align, &rpmsg_buf_align);
>> + if (rpmsg_buf_align && !is_power_of_2(rpmsg_buf_align)) {
>> + dev_err(&vdev->dev,
>> + "bad vdev config: rpmsg_buf_align %u is not a power of 2\n",
>> + rpmsg_buf_align);
>> + err = -EINVAL;
>> + goto vqs_del;
>> + }
>> +
>> + /* note: tx and rx are defined from remote view */
>> + virtio_cread(vdev, struct virtio_rpmsg_config,
>> + txbuf_size, &vrp->rx_buf_size);
>> + virtio_cread(vdev, struct virtio_rpmsg_config,
>> + rxbuf_size, &vrp->tx_buf_size);
>> +
>> + /* The buffers must hold at least the rpmsg header */
>> + if (vrp->rx_buf_size < sizeof(struct rpmsg_hdr) ||
>> + vrp->tx_buf_size < sizeof(struct rpmsg_hdr)) {
>> + dev_err(&vdev->dev,
>> + "bad vdev config: rx buf sz = %u, tx buf sz = %u\n",
>> + vrp->rx_buf_size, vrp->tx_buf_size);
>> + err = -EINVAL;
>> + goto vqs_del;
>> + }
>> +
>> + if (rpmsg_buf_align) {
>> + vrp->rx_buf_size_aligned = ALIGN(vrp->rx_buf_size,
>> + rpmsg_buf_align);
>> + vrp->tx_buf_size_aligned = ALIGN(vrp->tx_buf_size,
>> + rpmsg_buf_align);
>> + } else {
>> + vrp->rx_buf_size_aligned = vrp->rx_buf_size;
>> + vrp->tx_buf_size_aligned = vrp->tx_buf_size;
>> + }
>> +
>> + dev_dbg(&vdev->dev,
>> + "vdev config: ver=%u, align=0x%x, rx sz = 0x%x, tx sz = 0x%x\n",
>> + version, rpmsg_buf_align, vrp->rx_buf_size,
>> + vrp->tx_buf_size);
>> + } else {
>> + vrp->rx_buf_size = DEFAULT_RPMSG_BUF_SIZE;
>> + vrp->tx_buf_size = DEFAULT_RPMSG_BUF_SIZE;
>> + vrp->rx_buf_size_aligned = vrp->rx_buf_size;
>> + vrp->tx_buf_size_aligned = vrp->tx_buf_size;
>> + }
>> +
>> + total_buf_space = (vrp->num_rx_buf * vrp->rx_buf_size_aligned) +
>> + (vrp->num_tx_buf * vrp->tx_buf_size_aligned);
>>
>> /* allocate coherent memory for the buffers */
>> bufs_va = dma_alloc_coherent(vdev->dev.parent,
>> @@ -873,15 +959,20 @@ static int rpmsg_probe(struct virtio_device *vdev)
>> /* first part of the buffers is dedicated for RX */
>> vrp->rx_bufs = bufs_va;
>>
>> - /* and second part is dedicated for TX */
>> - vrp->tx_bufs = bufs_va + vrp->num_rx_buf * vrp->buf_size;
>> + /*
>> + * Here buf_va is aligned to a page. Also rx buf size is aligned with
>> + * cache line alignment provided by the firmware, so tx buf's start
>> + * address is guranteed to be aligned with the alignment provided by
>> + * the firmware.
>> + */
>> + vrp->tx_bufs = bufs_va + (vrp->num_rx_buf * vrp->rx_buf_size_aligned);
>>
>> /* set up the receive buffers */
>> for (i = 0; i < vrp->num_rx_buf; i++) {
>> struct scatterlist sg;
>> - void *cpu_addr = vrp->rx_bufs + i * vrp->buf_size;
>> + void *cpu_addr = vrp->rx_bufs + i * vrp->rx_buf_size_aligned;
>>
>> - rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size);
>> + rpmsg_sg_init(&sg, cpu_addr, vrp->rx_buf_size);
>>
>> err = virtqueue_add_inbuf(vrp->rvq, &sg, 1, cpu_addr,
>> GFP_KERNEL);
>> @@ -964,8 +1055,8 @@ static int rpmsg_remove_device(struct device *dev, void *data)
>> static void rpmsg_remove(struct virtio_device *vdev)
>> {
>> struct virtproc_info *vrp = vdev->priv;
>> - unsigned int num_bufs = vrp->num_rx_buf + vrp->num_tx_buf;
>> - size_t total_buf_space = num_bufs * vrp->buf_size;
>> + size_t total_buf_space = (vrp->num_rx_buf * vrp->rx_buf_size_aligned) +
>> + (vrp->num_tx_buf * vrp->tx_buf_size_aligned);
>> int ret;
>>
>> virtio_reset_device(vdev);
>> @@ -991,6 +1082,7 @@ static struct virtio_device_id id_table[] = {
>>
>> static unsigned int features[] = {
>> VIRTIO_RPMSG_F_NS,
>> + VIRTIO_RPMSG_F_BUFSZ,
>> };
>>
>> static struct virtio_driver virtio_ipc_driver = {
>> diff --git a/include/linux/rpmsg/virtio_rpmsg.h b/include/linux/rpmsg/virtio_rpmsg.h
>> new file mode 100644
>> index 000000000000..735a947d5582
>> --- /dev/null
>> +++ b/include/linux/rpmsg/virtio_rpmsg.h
>> @@ -0,0 +1,48 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) Pinecone Inc. 2019
>> + * Copyright (C) Xiang Xiao <xiaoxiang@pinecone.net>
>> + * Copyright (C) Advanced Micro Devices, Inc. 2026
>> + */
>> +
>> +#ifndef _LINUX_VIRTIO_RPMSG_H
>> +#define _LINUX_VIRTIO_RPMSG_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/virtio_types.h>
>> +
>> +/* The feature bitmap for virtio rpmsg */
>> +#define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */
>> +#define VIRTIO_RPMSG_F_BUFSZ 1 /* RP get buffer size from config space */
>> +
>> +/* Version of struct virtio_rpmsg_config understood by this driver */
>> +#define RPMSG_VDEV_CONFIG_V1 1
>> +
>> +/**
>> + * struct virtio_rpmsg_config - config space for rpmsg virtio device
>> + *
>> + * @version: version of this structure, currently %RPMSG_VDEV_CONFIG_V1.
>> + * @size: size of this structure in bytes.
>> + * @rpmsg_buf_align: alignment in bytes for each buffer. Must be a power of
>> + * two. If 0 then no alignment will be done. This alignment
>> + * will not decide actual size of the buffer but will be
>> + * used to decided the start address of the buffer. The
>> + * actual size of the buffer can be different than the
>> + * aligned size of the buffer.
>
> Is there really a need to have a buffer size different from its alignment? It's
> not like the (small) delta between the buffer size and its alignment will be
> used for something else. I'm fine with a buffer alignment requirement but in
> those cases, the firmware should set the size of the buffer in accordance with
> its alignment requirement. Otherwise, the complexity needed to manage the
> discrpancy between the two yields a driver that is hard to maintain and prone to
> bugs.
I would be in favor to keep it.
The remote processor could expose some alignment constraints. The Linux
kernel
or another main processor OS can have its own constraints. As it
allocates the
buffers, it can decide the final alignment independently of the buffer
sizes.
An example is a remote processor without a cache and a main processor with a
cache. rpmsg_buf_align could be set to 0, but the main processor could
decide to
align buffer addresses on a cache line.
Thanks,
Arnaud
>
>> + * @txbuf_size: Tx buf size from remote's view. For Linux this is rx buf size.
>> + * @rxbuf_size: Rx buf size from remote's view. For Linux this is tx buf size.
>> + *
>> + * This is the configuration structure shared by the device and the driver,
>> + * read when %VIRTIO_RPMSG_F_BUFSZ is negotiated. The fields are laid out so
>> + * the structure is naturally 32-bit aligned.
>> + */
>> +struct virtio_rpmsg_config {
>> + u8 version;
>> + __virtio16 size;
>> + __virtio16 rpmsg_buf_align;
>> + /* The tx/rx individual buffer size (if VIRTIO_RPMSG_F_BUFSZ) */
>> + __virtio32 txbuf_size;
>> + __virtio32 rxbuf_size;
>> +} __packed;
>> +
>> +#endif /* _LINUX_VIRTIO_RPMSG_H */
>> --
>> 2.34.1
>>
>
^ permalink raw reply
* Re: [PATCH v4 net-next 7/7] net: ena: Implement gettimexattrs64 callback for PTP attributes
From: Arthur Kiyanovski @ 2026-07-16 8:11 UTC (permalink / raw)
To: Jacob Keller
Cc: Arthur Kiyanovski, David Miller, Jakub Kicinski, netdev,
Richard Cochran, Eric Dumazet, Paolo Abeni, David Woodhouse,
Thomas Gleixner, Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo,
David Woodhouse, Yonatan Sarna, Zorik Machulsky,
Alexander Matushevsky, Saeed Bshara, Matt Wilson, Anthony Liguori,
Nafea Bshara, Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <f8974bb1-e27d-480c-8f88-6aea3ef41ad8@intel.com>
On 2026-07-14 17:47:35-07:00, Jacob Keller wrote:
> On 7/13/2026 7:03 PM, Arthur Kiyanovski wrote:
>
> > Implement the gettimexattrs64 callback in the ENA driver to support
> > the PTP_SYS_OFFSET_EXTENDED_ATTRS ioctl.
> >
> > This enables applications to retrieve PHC timestamps with quality
> > attributes through the standard PTP ioctl interface.
> >
> > The ENA device currently reports only error_bound (valid bit set).
> > Other attributes are not reported (valid bits unset).
>
> Typically it would be a policy not to introduce new attributes which are
> not yet used, and add the other attributes once a user appears. However,
> I think it makes sense to have the full set of desired attributes
> especially given the ioctl interface limitations which would otherwise
> require a lot of reserved space or new ioctl numbers. Especially given
> the uAPI here has been discussed and changed heavily from previous patch
> iterations.
Thanks — that matches the reasoning. We defined the full
attribute set up front because of the ioctl interface
constraints you describe. The valid bitmask ensures that
userspace only consumes attributes that a driver explicitly
reports, while still allowing additional attributes to
become available in future drivers without further UAPI
changes.
^ permalink raw reply
* Re: [PATCH v4 net-next 1/7] ptp: Add ioctls for PHC timestamps with quality attributes
From: Arthur Kiyanovski @ 2026-07-16 8:09 UTC (permalink / raw)
To: Jacob Keller
Cc: Arthur Kiyanovski, David Miller, Jakub Kicinski, netdev,
Richard Cochran, Eric Dumazet, Paolo Abeni, David Woodhouse,
Thomas Gleixner, Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo,
David Woodhouse, Yonatan Sarna, Zorik Machulsky,
Alexander Matushevsky, Saeed Bshara, Matt Wilson, Anthony Liguori,
Nafea Bshara, Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <e58af0ae-ad06-4f77-8ebd-f3678d6dde66@intel.com>
On 2026-07-14 17:45:34-07:00, Jacob Keller wrote:
> On 7/13/2026 7:03 PM, Arthur Kiyanovski wrote:
>
> > Introduce two new ioctls that extend existing PTP timestamp interfaces
> > with clock quality information:
> >
> > - PTP_SYS_OFFSET_EXTENDED_ATTRS: Extends PTP_SYS_OFFSET_EXTENDED
> > - PTP_SYS_OFFSET_PRECISE_ATTRS: Extends PTP_SYS_OFFSET_PRECISE
> >
> > These ioctls provide quality attributes alongside timestamps:
> >
> > 1. error_bound: Maximum deviation from true time (nanoseconds), based
> > on device's internal clock state
> > 2. clock_status: Synchronization state (unknown, initializing,
> > synchronized, free-running, unreliable)
> > 3. timescale: Time reference (TAI, UTC, etc.)
> > 4. counter_value: Raw system counter (e.g. TSC ticks) captured by the
> > timekeeping core alongside each system timestamp
> > 5. counter_id: Identifies the counter source (e.g. TSC, ARM arch counter)
> >
> > This supports three use cases:
> >
> > 1. Managed PHC devices (e.g., ENA, vmclock) that maintain their own
> > synchronization and can report quality metrics directly to userspace
> > without requiring ptp4l
> >
> > 2. Applications that need complete time quality information in a single
> > call, regardless of how the PHC is synchronized
> >
> > 3. VMMs that need raw system counter values paired
> > with PTP timestamps for feed-forward clock calibration, avoiding the
> > feedback loop inherent in NTP-style synchronization
>
> I'm also wondering if this can expose device-known error bounds on
> timestamps even for devices which are operated as synchronized by ptp4l..
>
The intent is for these attributes to represent device-
known clock quality information. The mechanism is
deliberately synchronization-agnostic: drivers may report
any attributes they can meaningfully vouch for, regardless
of how the clock is being disciplined. Thus a device that
genuinely knows a hardware error bound could report it even
if the PHC is being adjusted by ptp4l.
In practice, however, when a PHC is disciplined entirely
from userspace the driver often has little or no visibility
into synchronization quality. The valid bitmask is designed
for exactly this case: a driver advertises only the
attributes it can populate, so anything it cannot determine
is simply reported as unavailable.
> > Timescale definitions use a Continuity/Discipline framework to describe
> > timeline properties and steering behavior consistently across all
> > entries.
> >
> > This implementation is based on the original RFC and the UAPI design
> > discussion linked below.
>
> Not a dig against this patch set, nor a request that you work to
> implement anything else, but I am beginning to wonder if/when it would
> make sense to transition from ioctl-based implementation to genetlink or
> something. We did something similar for ethtool ioctls a few years ago.
> I know the maintainer for PTP has some distaste for netlink and prefers
> the simplicity of the ioctls.. but I think we're moving past where the
> ioctls are "simple". Now that we have ynl tools, it has gotten easier to
> implement properly. It makes extending the API much easier for the
> future vs the array of ioctls we now carry for legacy implementations.
>
That's an interesting direction for future PTP UAPI
evolution, and ynl does make that path easier than it used
to be. For this series I've kept to the existing PTP
userspace API model and extended the current timestamping
interfaces in a backwards-compatible way; a move to a
netlink family would be a broader subsystem effort and feels
separate from this work.
> > diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
> > index 46d45f902486..88c2da6bc8c6 100644
> > --- a/include/uapi/linux/ptp_clock.h
> > +++ b/include/uapi/linux/ptp_clock.h
> > @@ -79,6 +79,137 @@
> > */
> > #define PTP_PEROUT_V1_VALID_FLAGS (0)
> >
> > +/*
> > + * Clock status values for struct ptp_clock_attrs.status
> > + */
> > +enum ptp_clock_status {
> > + /* Clock synchronization status cannot be reliably determined */
> > + PTP_CLOCK_STATUS_UNKNOWN = 0,
> > +
> > + /* Clock is acquiring synchronization */
> > + PTP_CLOCK_STATUS_INITIALIZING = 1,
> > +
> > + /* Clock is synchronized and maintained accurately by the device */
> > + PTP_CLOCK_STATUS_SYNCED = 2,
> > +
> > + /* Clock is drifting but remains within acceptable error bounds */
> > + PTP_CLOCK_STATUS_HOLDOVER = 3,
> > +
> > + /* Clock is drifting without adjustments or synchronization */
> > + PTP_CLOCK_STATUS_FREE_RUNNING = 4,
> > +
> > + /* Clock is unreliable, the error_bound value cannot be trusted */
> > + PTP_CLOCK_STATUS_UNRELIABLE = 5
> > +};
>
> Do you have any thought on how ptp4l synchronizing the clock should
> impact the clock status here? Is this intended purely for device/drivers
> which have their own synchronization and not for ones which expose a
> clock that is synchronized by userspace? Would it make sense to have a
> mode that is something like "this clock has been modified by userspace"
> after any call to the .adjtime or .adjfreq is made?
>
Regarding a "modified by userspace" mode, I wasn't
planning to add one. Whether adjtime() or adjfreq() was
invoked does not by itself describe the current
synchronization state or quality of the clock. A clock
disciplined from userspace may still be highly accurate. I'd
prefer to keep clock_status focused on clock quality
information that the driver can directly determine rather
than on how the clock is being controlled.
> > +
> > +/*
> > + * Clock timescale values for struct ptp_clock_attrs.timescale.
> > + *
> > + * These definitions describe the mathematical properties and reference
> > + * epochs of the timescale provided by the PHC.
> > + *
> > + * Discipline: Describes the frequency/phase steering behavior.
> > + * Continuity: Describes whether the timeline is uninterrupted.
> > + */
> > +enum ptp_clock_timescale {
> > + /* Unknown or unspecified timescale */
> > + PTP_TIMESCALE_UNKNOWN = 0,
> > +
> > + /********************* Absolute Atomic Timescales *********************
> > + * These timescales are continuous, monotonic standards based on atomic
> > + * physics. They do not experience phase jumps.
> > + **********************************************************************/
> > +
> > + /**
> > + * International Atomic Time (TAI)
> > + * Epoch: 1958-01-01 00:00:00.
> > + * Continuity: Strictly monotonic and continuous; no leap seconds.
> > + * Discipline: Primary atomic reference; no phase jumps.
> > + */
> > + PTP_TIMESCALE_TAI = 1,
> > +
> > + /**
> > + * Terrestrial Time (TT)
> > + * Epoch: 1958-01-01 00:00:00.
> > + * Continuity: Strictly monotonic and continuous; no leap seconds.
> > + * Discipline: Defined as TAI + 32.184s constant offset.
> > + */
> > + PTP_TIMESCALE_TT = 2,
> > +
> > + /**
> > + * Global Positioning System (GPS) Time
> > + * Epoch: 1980-01-06 00:00:00.
> > + * Continuity: Strictly monotonic and continuous; no leap seconds.
> > + * Discipline: Defined by the GPS constellation; fixed offset from TAI.
> > + */
> > + PTP_TIMESCALE_GPS = 3,
> > +
> > + /****************** UTC-Based Timescales (Civil Time) *****************
> > + * These timescales are derived from TAI but adjusted to align with
> > + * the Earth's rotation, primarily through leap seconds.
> > + **********************************************************************/
> > +
> > + /**
> > + * Coordinated Universal Time (UTC) - Wall-clock (CLOCK_REALTIME)
> > + * Epoch: 1970-01-01 00:00:00 (Unix epoch).
> > + * Continuity: Discontinuous; subject to 1-second leap second
> > + * phase jumps.
> > + * Discipline: Frequency steered; incorporates leap second corrections.
> > + *
> > + * Note: Leap-smeared UTC MUST NOT be advertised as PTP_TIMESCALE_UTC.
> > + * Smear algorithms are not standardized and the resulting timescale
> > + * is ambiguous. Implementations using smeared UTC MUST advertise
> > + * PTP_TIMESCALE_UNKNOWN or PTP_TIMESCALE_PROPRIETARY instead.
> > + */
> > + PTP_TIMESCALE_UTC = 4,
> > +
> > + /**
> > + * POSIX Time (Unix Time)
> > + * Epoch: 1970-01-01 00:00:00.
> > + * Continuity: Discontinuous; leap seconds handled by
> > + * repeating/skipping values.
> > + * Discipline: Follows UTC frequency steering and phase jumps.
> > + */
> > + PTP_TIMESCALE_POSIX = 5,
> > +
> > + /****************** System-Relative Monotonic Clocks ******************
> > + * These timescales are relative to a system event (like boot)
> > + * and are not synchronized to an external atomic standard.
> > + **********************************************************************/
> > +
> > + /**
> > + * Monotonic System Clock (CLOCK_MONOTONIC)
> > + * Epoch: Arbitrary (System boot time).
> > + * Continuity: Strictly monotonic; no leap seconds.
> > + * Discipline: Frequency steered to match system reference;
> > + * does not advance during suspend.
> > + */
> > + PTP_TIMESCALE_MONOTONIC = 6,
> > +
> > + /**
> > + * Raw Monotonic System Clock (CLOCK_MONOTONIC_RAW)
> > + * Epoch: Arbitrary (System boot time).
> > + * Continuity: Strictly monotonic; no leap seconds.
> > + * Discipline: Raw hardware oscillator; no frequency steering
> > + * or discipline.
> > + */
> > + PTP_TIMESCALE_MONOTONIC_RAW = 7,
> > +
> > + /**
> > + * Boot Time System Clock (CLOCK_BOOTTIME)
> > + * Epoch: Arbitrary (System boot time).
> > + * Continuity: Strictly monotonic and continuous; no leap seconds.
> > + * Discipline: Frequency steered to match system reference;
> > + * advances during suspend.
> > + */
> > + PTP_TIMESCALE_BOOTTIME = 8,
> > +
> > + /********************** Vendor-Specific Timescale *********************/
> > +
> > + /* A proprietary or vendor-specific timescale with custom rules. */
> > + PTP_TIMESCALE_PROPRIETARY = 9,
> > +};
>
> I appreciate the detailed explanations here as it helps to disambiguate
> the modes.
>
Thanks — glad the breakdown is helpful.
> > @@ -106,7 +350,11 @@ struct ptp_clock_caps {
> > /* Whether the clock supports adjust phase */
> > int adjust_phase;
> > int max_phase_adj; /* Maximum phase adjustment in nanoseconds. */
> > - int rsv[11]; /* Reserved for future use. */
> > + /* Whether the clock supports extended timestamps with attributes */
> > + int extended_attrs;
> > + /* Whether the clock supports precise cross-timestamps with attributes */
> > + int precise_attrs;
> > + int rsv[9]; /* Reserved for future use. */
>
> I do kind of wish we had opted for bit flags here given the number of
> ints being used as booleans.. :( A lot of wasted reserved space.
Agreed — a flags field would likely have scaled better. I
followed the existing ptp_clock_caps convention (one int per
capability) to stay consistent with the current UAPI
structure rather than mix two styles within the same struct.
^ permalink raw reply
* Re: [PATCH v21 06/14] dmaengine: qcom: bam_dma: add support for BAM locking
From: Bartosz Golaszewski @ 2026-07-16 7:43 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Jonathan Corbet, Thara Gopinath, Herbert Xu, David S. Miller,
Udit Tiwari, Md Sadre Alam, Dmitry Baryshkov,
Manivannan Sadhasivam, Bjorn Andersson, Peter Ujfalusi,
Michal Simek, Frank Li, Andy Gross, Neil Armstrong, dmaengine,
linux-doc, linux-kernel, linux-arm-msm, linux-crypto,
linux-arm-kernel, Bartosz Golaszewski, Bartosz Golaszewski,
Vinod Koul
In-Reply-To: <alewA9ubkYKHpnuj@linaro.org>
On Wed, 15 Jul 2026 18:06:27 +0200, Stephan Gerhold
<stephan.gerhold@linaro.org> said:
> On Wed, Jul 15, 2026 at 09:06:46PM +0530, Vinod Koul wrote:
>> On 15-07-26, 14:43, Bartosz Golaszewski wrote:
>> > On Tue, 14 Jul 2026 11:49:14 +0200, Stephan Gerhold
>
...
> I would avoid using struct dma_slave_config->dst_addr (like in v11) [1]
> though, since it's more a "address to dummy register" now than the
> actual dst_addr of the peripheral. That's why I suggested
> struct dma_slave_config->peripheral_config.
As in:
struct bam_dma_peripheral_config {
phys_addr_t scratchpad_addr;
};
struct dma_slave_config cfg = { .peripheral_config = &scratchpad_addr };
?
Bart
^ permalink raw reply
* Re: [linux-next:master] [crypto] 2f204fe718: stress-ng.af-alg.ops_per_sec 113.7% improvement
From: Oliver Sang @ 2026-07-16 7:04 UTC (permalink / raw)
To: Eric Biggers
Cc: oe-lkp, lkp, Herbert Xu, linux-doc, linux-crypto, oliver.sang
In-Reply-To: <20260715162749.GA1794078@google.com>
hi, Eric,
On Wed, Jul 15, 2026 at 04:27:49PM +0000, Eric Biggers wrote:
> On Wed, Jul 15, 2026 at 11:00:46PM +0800, kernel test robot wrote:
> >
> >
> > Hello,
> >
> >
> > we don't have enough knowledge to deeply analyze the impact of this commit
> > to stress-ng.af-alg performance tests.
> >
> > along with the major kpi improvement:
> >
> > 7756377 +113.7% 16578454 stress-ng.af-alg.ops_per_sec
> >
> > we also noticed lots of misc tests seem have changes, e.g.
> >
> > 697423 -9.5% 630908 ± 2% stress-ng.af-alg.aes_cipher_ops/sec
> > 245103 -100.0% 0.00 stress-ng.af-alg.blowfish_cipher_ops/sec
> > 369646 -100.0% 0.00 stress-ng.af-alg.camellia_cipher_ops/sec
> > 162192 -100.0% 0.00 stress-ng.af-alg.cast5_cipher_ops/sec
> > 115113 -100.0% 0.00 stress-ng.af-alg.cast6_cipher_ops/sec
> >
> > we don't know if these are expected behavior upon 2f204fe718.
> > so this report is just FYI what we observed in our tests.
> >
> >
> >
> > kernel test robot noticed a 113.7% improvement of stress-ng.af-alg.ops_per_sec on:
> >
> >
> > commit: 2f204fe718f5bf519013cc2536ad7bb2cbb51661 ("crypto: af_alg - Add af_alg_restrict sysctl, defaulting to 1")
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>
> That commit makes AF_ALG block access to some algorithms by default,
> overridable by a sysctl. Apparently these performance tests
> misinterpret that as a change in performance.
>
> It actually doesn't make much sense to do performance testing on AF_ALG
> at all anymore, given that AF_ALG is deprecated. A limited form of
> AF_ALG is being left in place just for backwards compatibility with a
> few programs. None of those programs use it in a performance-critical
> way, as far as I know.
>
> To the extent that it's still done at all, it shouldn't test random
> algorithms like "ecb(cast5)" that are obsolete and unused.
thanks a lot for education! we will disable the test of AF_ALG.
>
> - Eric
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox