* Re: [PATCH v4] crypto: testmgr - Add test vectors for authenc(hmac(md5),cbc(aes))
From: Herbert Xu @ 2026-04-03 1:03 UTC (permalink / raw)
To: Aleksander Jan Bajkowski
Cc: davem, mcoquelin.stm32, alexandre.torgue, linux-crypto,
linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <20260303184916.69132-1-olek2@wp.pl>
On Tue, Mar 03, 2026 at 07:48:44PM +0100, Aleksander Jan Bajkowski wrote:
> Test vectors were generated starting from existing CBC(AES) test vectors
> (RFC3602, NIST SP800-38A) and adding HMAC(MD5) computed with Python
> script. Then, the results were double-checked on Mediatek MT7981 (safexcel)
> and NXP P2020 (talitos). Both platforms pass self-tests.
>
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
> v4:
> - rename aes-generic -> aes-lib
> v3:
> - correct sha384 -> md5 in description
> v2:
> - rebase and resolve conflicts
> ---
> crypto/testmgr.c | 7 ++
> crypto/testmgr.h | 255 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 262 insertions(+)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: aspeed/hash: Use memcpy_from_sglist() in aspeed_ahash_dma_prepare()
From: Herbert Xu @ 2026-04-03 1:08 UTC (permalink / raw)
To: Paul Louvel
Cc: Neal Liu, David S. Miller, Joel Stanley, Andrew Jeffery,
Thomas Petazzoni, linux-aspeed, linux-crypto, linux-arm-kernel,
linux-kernel
In-Reply-To: <20260327092418.10476-1-paul.louvel@bootlin.com>
On Fri, Mar 27, 2026 at 10:24:18AM +0100, Paul Louvel wrote:
> Replace scatterwalk_map_and_copy() with memcpy_from_sglist() in
> aspeed_ahash_dma_prepare(). The latter provides a simpler interface
> without requiring a direction parameter, making the code easier to
> read and less error-prone.
>
> No functional change intended.
>
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
> ---
> drivers/crypto/aspeed/aspeed-hace-hash.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next v2 10/11] net: airoha: Do not stop GDM port if it is shared
From: Jakub Kicinski @ 2026-04-03 1:12 UTC (permalink / raw)
To: lorenzo
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, robh,
krzk+dt, conor+dt, ansuelsmth, benjamin.larsson, linux-arm-kernel,
linux-mediatek, netdev, devicetree
In-Reply-To: <20260401-airoha-eth-multi-serdes-v2-10-ac427ae4beeb@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: airoha: Do not stop GDM port if it is shared
This patch introduces a port->users reference counter to prevent setting
the GDM port forward configuration to FE_PSE_PORT_DROP if another net_device
is still using the shared GDM port.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 0780fb8aacfb..3f9bce8c5370 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -1681,9 +1682,6 @@ static int airoha_dev_stop(struct net_device *netdev)
> for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> netdev_tx_reset_subqueue(netdev, i);
>
> - airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> - FE_PSE_PORT_DROP);
> -
> if (atomic_dec_and_test(&qdma->users)) {
> airoha_qdma_clear(qdma, REG_QDMA_GLOBAL_CFG,
> GLOBAL_CFG_TX_DMA_EN_MASK |
While this patch aims to support shared GDM ports, does the remaining
unconditional call to airoha_set_vip_for_gdm_port(dev, false) at the start
of airoha_dev_stop() break other active devices sharing the port?
static int airoha_dev_stop(struct net_device *netdev)
{
...
err = airoha_set_vip_for_gdm_port(dev, false);
...
}
This appears to clear the REG_FE_VIP_PORT_EN and REG_FE_IFC_PORT_EN hardware
bits for the physical port, which might instantly kill traffic for any other
active interface using the shared port.
Similarly, in airoha_dev_open() and airoha_dev_change_mtu(), the
REG_GDM_LEN_CFG (MTU) is overwritten unconditionally. If a device with a
smaller MTU is brought up, will it overwrite the shared port's MTU limit
and cause the hardware to silently drop larger packets for the other device?
[ ... ]
> @@ -1697,6 +1695,11 @@ static int airoha_dev_stop(struct net_device *netdev)
> }
> }
>
> + if (atomic_dec_and_test(&port->users))
> + airoha_set_gdm_port_fwd_cfg(qdma->eth,
> + REG_GDM_FWD_CFG(port->id),
> + FE_PSE_PORT_DROP);
> +
> return 0;
> }
Does moving this FE_PSE_PORT_DROP configuration to the end of airoha_dev_stop()
cause a race condition when stopping the device?
In airoha_dev_open(), the driver correctly enables the QDMA engine before
configuring the GDM port to forward packets. A safe teardown typically
mirrors this by first stopping the packet source (setting FE_PSE_PORT_DROP)
and then disabling the destination DMA engine.
By moving airoha_set_gdm_port_fwd_cfg() after the airoha_qdma_clear() block,
when both qdma->users and port->users reach zero, the QDMA engine and its
queues are shut down while the GDM port is still actively forwarding packets.
Could incoming packets be forwarded to a disabled DMA engine during this
window, risking hardware lockups or buffer leaks?
^ permalink raw reply
* Re: [PATCH net-next v2 09/11] net: airoha: Support multiple net_devices for a single FE GDM port
From: Jakub Kicinski @ 2026-04-03 1:12 UTC (permalink / raw)
To: lorenzo
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, robh,
krzk+dt, conor+dt, ansuelsmth, benjamin.larsson, linux-arm-kernel,
linux-mediatek, netdev, devicetree
In-Reply-To: <20260401-airoha-eth-multi-serdes-v2-9-ac427ae4beeb@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: airoha: Support multiple net_devices for a single FE GDM port
This commit introduces support for multiple net_devices connected to the
same Frame Engine (FE) GDM port via an external hardware arbiter. It
adds logic to parse device tree child nodes and instantiate separate
net_devices for TDM traffic from external SerDes.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 5e489e8593ba..0780fb8aacfb 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -614,9 +616,8 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> struct page *page = virt_to_head_page(e->buf);
> u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> - struct airoha_gdm_port *port;
> - struct net_device *netdev;
> - int data_len, len, p;
> + struct airoha_gdm_dev *dev;
> + int data_len, len;
>
> if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> break;
> @@ -633,12 +634,10 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> if (!len || data_len < len)
> goto free_frag;
>
> - p = airoha_qdma_get_gdm_port(eth, desc);
> - if (p < 0 || !eth->ports[p])
> + dev = airoha_qdma_get_gdm_dev(eth, desc);
> + if (IS_ERR(dev))
> goto free_frag;
If a scattered frame is being processed (q->skb is not NULL) and IS_ERR(dev)
triggers the goto free_frag, the error handler frees the skb:
free_frag:
if (q->skb) {
dev_kfree_skb(q->skb);
q->skb = NULL;
} else {
page_pool_put_full_page(q->page_pool, page, true);
}
Since the current fragment's page is not yet attached to q->skb, does this
skip the else branch and permanently leak the page pool buffer?
[ ... ]
> @@ -1747,9 +1748,7 @@ static int airoha_set_gdm2_loopback(struct airoha_gdm_dev *dev)
> airoha_fe_clear(eth, REG_FE_VIP_PORT_EN, BIT(AIROHA_GDM2_IDX));
> airoha_fe_clear(eth, REG_FE_IFC_PORT_EN, BIT(AIROHA_GDM2_IDX));
>
> - /* XXX: handle XSI_USB_PORT and XSI_PCE1_PORT */
> - nbq = port->id == AIROHA_GDM3_IDX && airoha_is_7581(eth) ? 4 : 0;
> - src_port = eth->soc->ops.get_src_port_id(port, nbq);
> + src_port = eth->soc->ops.get_src_port_id(port, dev->nbq);
> if (src_port < 0)
> return src_port;
With multiple net_devices instantiated for the same physical GDM port, does
the initialization of sibling devices overwrite shared hardware registers?
For example, when airoha_set_gdm2_loopback() uses src_port to configure
REG_FE_WAN_PORT, it explicitly clears the WAN port routing masks:
airoha_fe_rmw(eth, REG_FE_WAN_PORT,
WAN1_EN_MASK | WAN1_MASK | WAN0_MASK,
FIELD_PREP(WAN0_MASK, src_port));
Will the last initialized device clobber the WAN routing configurations and
hardware MAC filters for the others, causing their traffic to be dropped?
[ ... ]
> @@ -1970,7 +1969,8 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
> }
>
> fport = airoha_get_fe_port(dev);
> - msg1 = FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> + msg1 = FIELD_PREP(QDMA_ETH_TXMSG_NBOQ_MASK, dev->nbq) |
> + FIELD_PREP(QDMA_ETH_TXMSG_FPORT_MASK, fport) |
> FIELD_PREP(QDMA_ETH_TXMSG_METER_MASK, 0x7f);
>
> q = &qdma->q_tx[qid];
When multiple net_devices share a single QDMA instance and its TX rings
(qdma->q_tx), a TX queue stall might occur here.
If the shared hardware ring fills up, netif_tx_stop_queue() stops the
transmitting device's queue. Later, when descriptors are freed, the driver
only wakes the queue corresponding to the specific packet that just completed.
If multiple devices' TX queues are stopped, will the other devices' queues
remain stalled unless their specific packets happen to be processed?
[ ... ]
> @@ -2897,10 +2900,11 @@ bool airoha_is_valid_gdm_dev(struct airoha_eth *eth,
>
> static int airoha_alloc_gdm_device(struct airoha_eth *eth,
> struct airoha_gdm_port *port,
> - struct device_node *np)
> + int nbq, struct device_node *np)
> {
> - struct airoha_gdm_dev *dev;
> struct net_device *netdev;
> + struct airoha_gdm_dev *dev;
> + u8 index;
> int err;
[ ... ]
> + /* Allowed nbq for EN7581 on GDM3 port are 4 and 5 for PCIE0
> + * and PCIE1 respectively.
> + */
> + index = nbq;
> + if (airoha_is_7581(eth) && port->id == AIROHA_GDM3_IDX)
> + index -= 4;
> +
> + if (index >= ARRAY_SIZE(port->devs) || port->devs[index]) {
> + dev_err(eth->dev, "invalid nbq id: %d\n", nbq);
> + return -EINVAL;
> + }
There are two potential issues regarding the new index logic:
First, does this break probing for older device trees without child nodes?
When falling back to airoha_alloc_gdm_device(..., 0, np) with nbq = 0,
the index -= 4 operation for AIROHA_GDM3_IDX will cause the u8 index to
underflow to 252. The bounds check 252 >= 2 fails, returning -EINVAL.
Second, can an invalid device tree value bypass the bounds check?
The int nbq parameter is directly assigned to the u8 index. If a large value
like reg = <256> is provided, it truncates to 0, passing the index >= 2
check. The un-truncated int nbq is then stored in dev->nbq, potentially
causing out-of-bounds shifts when used in hardware macros.
[ ... ]
> @@ -2986,7 +3004,36 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
> if (err)
> return err;
>
> - return airoha_alloc_gdm_device(eth, port, np);
> + for_each_child_of_node(np, node) {
> + /* Multiple external serdes connected to the FE GDM port via an
> + * external arbiter.
> + */
> + const __be32 *nbq_ptr;
> +
> + if (!of_device_is_compatible(node, "airoha,eth-port"))
> + continue;
> +
> + d++;
> + if (!of_device_is_available(node))
> + continue;
> +
> + nbq_ptr = of_get_property(node, "reg", NULL);
> + if (!nbq_ptr) {
> + dev_err(eth->dev, "missing nbq id\n");
> + err = -EINVAL;
> + of_node_put(node);
> + break;
> + }
> +
> + err = airoha_alloc_gdm_device(eth, port, be32_to_cpup(nbq_ptr),
> + node);
> + if (err) {
> + of_node_put(node);
> + return err;
> + }
> + }
> +
> + return !d ? airoha_alloc_gdm_device(eth, port, 0, np) : 0;
> }
If nbq_ptr is missing, err = -EINVAL is set and the loop breaks. However,
since the child counter d was already incremented, the final statement
evaluates to 0 (success) because !d is false.
Does this swallow the -EINVAL error and cause a silent failure where the
driver continues probing without initializing the misconfigured GDM devices?
Additionally, the for_each_child_of_node() iterator automatically handles
taking and dropping the reference to node.
When node is passed to airoha_alloc_gdm_device(), it is directly assigned:
netdev->dev.of_node = np;
Since of_node_get() is not called, when the loop iteration finishes and drops
the reference, does this leave the net_device pointing to a freed device
tree node?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v4 net-next 03/14] net: enetc: add pre-boot initialization for i.MX94 switch
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-4-wei.fang@nxp.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.
---
net: enetc: add pre-boot initialization for i.MX94 switch
This commit adds pre-boot initialization logic for the i.MX94 switch
in the NETC block control driver. It configures external ports, MII
protocols, and MDIO PHY addresses, and handles the MAC selection
constraint between switch port 2 and ENETC 0.
> diff --git a/drivers/net/ethernet/freescale/enetc/netc_blk_ctrl.c b/drivers/net/ethernet/freescale/enetc/netc_blk_ctrl.c
> index 92a0f824dae7..c21230f7a7fb 100644
> --- a/drivers/net/ethernet/freescale/enetc/netc_blk_ctrl.c
> +++ b/drivers/net/ethernet/freescale/enetc/netc_blk_ctrl.c
[ ... ]
> +static int imx94_switch_link_config(struct netc_blk_ctrl *priv,
> + struct device_node *np,
> + bool *swp2_en)
> +{
> + struct device_node *ports;
> + int port_id, err = 0;
Since of_property_read_u32() expects a u32 pointer, does passing a
signed int pointer here trigger an incompatible pointer types warning
during compilation?
> +
> + ports = netc_get_switch_ports(np);
> + if (!ports)
> + return -ENODEV;
> +
> + for_each_available_child_of_node_scoped(ports, child) {
> + if (of_property_read_u32(child, "reg", &port_id) < 0) {
If the device tree provides a reg value with the MSB set, port_id
would be evaluated as a negative number.
[ ... ]
> static int imx94_netcmix_init(struct platform_device *pdev)
> {
> struct netc_blk_ctrl *priv = platform_get_drvdata(pdev);
> struct device_node *np = pdev->dev.of_node;
> + bool enetc0_en = false, swp2_en = false;
> u32 val;
> int err;
>
> for_each_child_of_node_scoped(np, child) {
> for_each_child_of_node_scoped(child, gchild) {
> - if (!of_device_is_compatible(gchild, "pci1131,e101"))
> - continue;
> -
> - err = imx94_enetc_link_config(priv, gchild);
> - if (err)
> - return err;
> + if (of_device_is_compatible(gchild, "pci1131,e101")) {
> + err = imx94_enetc_link_config(priv, gchild,
> + &enetc0_en);
> + if (err)
> + return err;
> + } else if (of_device_is_compatible(gchild,
> + "pci1131,eef2")) {
Is it possible for the switch node to be disabled in the device tree?
If so, should we check of_device_is_available() for gchild before proceeding,
similar to how it is checked for enetc0_en?
Without checking availability, if the disabled switch lacks a ports subnode,
imx94_switch_link_config() might return -ENODEV and abort the entire probe.
Alternatively, if the disabled switch node has a ports subnode with an
available port 2, it could spuriously set swp2_en = true and cause a false
conflict with ENETC 0.
> + err = imx94_switch_link_config(priv, gchild,
> + &swp2_en);
> + if (err)
> + return err;
> + }
> }
> }
[ ... ]
> @@ -610,6 +682,77 @@ static int imx94_enetc_mdio_phyaddr_config(struct netc_blk_ctrl *priv,
> return 0;
> }
>
> +static int imx94_ierb_enetc_init(struct netc_blk_ctrl *priv,
[ ... ]
> +static int imx94_switch_mdio_phyaddr_config(struct netc_blk_ctrl *priv,
> + struct device_node *np,
> + int port_id, u32 phy_mask)
> +{
> + int addr;
> +
> + /* The switch has 3 external ports at most */
> + if (port_id > 2)
> + return 0;
If port_id is negative due to the signed type, it will bypass this upper
bound check.
> +
> + addr = netc_get_phy_addr(np);
> + if (addr < 0) {
> + if (addr == -ENODEV)
> + return 0;
> +
> + return addr;
> + }
> +
> + if (phy_mask & BIT(addr)) {
> + dev_err(&priv->pdev->dev,
> + "Found same PHY address in EMDIO and switch node\n");
> + return -EINVAL;
> + }
> +
> + netc_reg_write(priv->ierb, IERB_LBCR(port_id),
> + LBCR_MDIO_PHYAD_PRTAD(addr));
Could this lead to an out-of-bounds register write prior to the array base
since port_id is negative?
> +
> + return 0;
> +}
> +
> +static int imx94_ierb_switch_init(struct netc_blk_ctrl *priv,
> + struct device_node *np,
> + u32 phy_mask)
> +{
> + struct device_node *ports;
> + int port_id, err = 0;
A similar signed int declaration is used here for port_id, which would run
into the same pointer type warning and negative value issues.
> +
> + ports = netc_get_switch_ports(np);
> + if (!ports)
> + return -ENODEV;
[ ... ]
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v4 net-next 04/14] net: enetc: add basic operations to the FDB table
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-5-wei.fang@nxp.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.
---
net: enetc: add basic operations to the FDB table
This commit adds FDB table management operations (Add, Delete, Update,
Query/Search) to the enetc driver using NTMP version 2.0. FDB entries
are managed via the command BD ring.
> diff --git a/include/linux/fsl/ntmp.h b/include/linux/fsl/ntmp.h
> index 916dc4fe7de3..a9f3e6cbf422 100644
> --- a/include/linux/fsl/ntmp.h
> +++ b/include/linux/fsl/ntmp.h
[ ... ]
> @@ -116,6 +158,31 @@ static inline int ntmp_rsst_query_entry(struct ntmp_user *user,
> return 0;
> }
>
> +static inline int ntmp_fdbt_add_entry(struct ntmp_user *user, u32 *entry_id,
> + const struct fdbt_keye_data *keye,
> + const struct fdbt_cfge_data *data)
> +{
> + return 0;
> +}
If CONFIG_NXP_NETC_LIB is disabled, will returning 0 here cause callers to
falsely assume the operation succeeded and proceed to use an uninitialized
entry_id pointer? Would it be safer to return an error code such as
-EOPNOTSUPP for stubs that have output parameters?
[ ... ]
> +static inline int ntmp_fdbt_search_port_entry(struct ntmp_user *user, int port,
> + u32 *resume_entry_id,
> + struct fdbt_entry_data *entry)
> +{
> + return 0;
> +}
Similar to the above, does returning 0 here leave both resume_entry_id and
entry uninitialized while signaling success? Since resume_entry_id is
typically used as an iterator, could this lead to an infinite loop or other
unpredictable behavior in the caller if they interpret the 0 return as a
successful read of valid hardware configuration data?
^ permalink raw reply
* Re: [PATCH v4 net-next 05/14] net: enetc: add support for the "Add" operation to VLAN filter table
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-6-wei.fang@nxp.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.
---
net: enetc: add support for the "Add" operation to VLAN filter table
This patch adds the ntmp_vft_add_entry() helper function to support adding
entries to the VLAN filter table. It allocates a DMA buffer, formats the
request, and transmits it via the command BD ring using netc_xmit_ntmp_cmd().
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
[ ... ]
> @@ -652,5 +655,52 @@ int ntmp_fdbt_search_port_entry(struct ntmp_user *user, int port,
> +int ntmp_vft_add_entry(struct ntmp_user *user, u16 vid,
> + const struct vft_cfge_data *cfge)
> +{
> + struct ntmp_dma_buf data = {
> + .dev = user->dev,
> + .size = sizeof(struct vft_req_ua),
> + };
> + struct vft_req_ua *req;
> + union netc_cbd cbd;
> + u32 len;
> + int err;
> +
> + err = ntmp_alloc_data_mem(&data, (void **)&req);
> + if (err)
> + return err;
> +
> + /* Request data */
> + ntmp_fill_crd(&req->crd, user->tbl.vft_ver, 0,
> + NTMP_GEN_UA_CFGEU);
> + req->ak.exact.vid = cpu_to_le16(vid);
> + req->cfge = *cfge;
> +
> + /* Request header */
> + len = NTMP_LEN(data.size, NTMP_STATUS_RESP_LEN);
> + ntmp_fill_request_hdr(&cbd, data.dma, len, NTMP_VFT_ID,
> + NTMP_CMD_ADD, NTMP_AM_EXACT_KEY);
> +
> + err = netc_xmit_ntmp_cmd(user, &cbd);
> + if (err)
> + dev_err(user->dev,
> + "Failed to add %s entry, vid: %u, err: %pe\n",
> + ntmp_table_name(NTMP_VFT_ID), vid, ERR_PTR(err));
> +
> + ntmp_free_data_mem(&data);
Does this code safely handle hardware timeouts?
If netc_xmit_ntmp_cmd() times out and returns an error, the pending
transaction is not explicitly aborted before ntmp_free_data_mem()
unconditionally frees the DMA buffer.
Could the hardware eventually process the pending command and perform a
DMA write of the response to the physical address of the freed buffer?
This might lead to silent memory corruption if that memory has already
been reallocated to another subsystem. Would it be safer to explicitly
abort the pending DMA transaction, reset the device on timeout, or
intentionally delay freeing the buffer until it is proven safe?
> +
> + return err;
> +}
^ permalink raw reply
* Re: [PATCH v4 net-next 06/14] net: enetc: add support for the "Update" operation to buffer pool table
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-7-wei.fang@nxp.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.
---
Subject: net: enetc: add support for the "Update" operation to buffer pool table
This patch adds the ntmp_bpt_update_entry() helper function to support
updating a specified entry in the buffer pool table. It provides the
ability to update buffer pool configuration data.
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp_private.h b/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> --- a/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp_private.h
[ ... ]
> +/* Buffer Pool Table Request Data Buffer Format of Update action */
> +struct bpt_req_update {
> + struct ntmp_req_by_eid rbe;
> + struct bpt_cfge_data cfge;
> +};
> +
[ ... ]
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
[ ... ]
> @@ -702,5 +708,38 @@ int ntmp_vft_add_entry(struct ntmp_user *user, u16 vid,
> }
> EXPORT_SYMBOL_GPL(ntmp_vft_add_entry);
>
> +int ntmp_bpt_update_entry(struct ntmp_user *user, u32 entry_id,
> + const struct bpt_cfge_data *cfge)
> +{
> + struct ntmp_dma_buf data = {
> + .dev = user->dev,
> + .size = sizeof(struct bpt_req_update),
> + };
> + struct bpt_req_update *req;
> + union netc_cbd cbd;
> + int err;
> +
> + err = ntmp_alloc_data_mem(&data, (void **)&req);
> + if (err)
> + return err;
> +
> + ntmp_fill_crd_eid(&req->rbe, user->tbl.bpt_ver, 0,
> + NTMP_GEN_UA_CFGEU | BPT_UA_BPSEU, entry_id);
Does this update action mask match the size of the request buffer?
The mask includes both NTMP_GEN_UA_CFGEU and BPT_UA_BPSEU, which instructs
the hardware to update both the configuration entry and the status entry.
However, struct bpt_req_update only contains the configuration data,
omitting the status data.
If the hardware attempts to read the missing BPSE data, could it reject
the command due to an undersized request length, or read beyond the
intended payload into zero-initialized DMA padding? This might overwrite
critical buffer pool tracking variables like amount_used_hwm.
If only the configuration is being updated, should the BPT_UA_BPSEU flag
be removed?
> + req->cfge = *cfge;
> + ntmp_fill_request_hdr(&cbd, data.dma, NTMP_LEN(data.size, 0),
> + NTMP_BPT_ID, NTMP_CMD_UPDATE, NTMP_AM_ENTRY_ID);
> +
> + err = netc_xmit_ntmp_cmd(user, &cbd);
> + if (err)
> + dev_err(user->dev,
> + "Failed to update %s entry 0x%x, err: %pe\n",
> + ntmp_table_name(NTMP_BPT_ID), entry_id, ERR_PTR(err));
> +
> + ntmp_free_data_mem(&data);
Can this result in a DMA use-after-free if the hardware command times out?
If netc_xmit_ntmp_cmd() times out waiting for the hardware to process the
command, it returns -ETIMEDOUT without resetting the hardware ring or
explicitly cancelling the command.
The descriptor remains active in the ring, and ntmp_free_data_mem()
unconditionally frees the DMA-mapped buffer. If the hardware eventually
processes the descriptor, it will read from the now-freed DMA memory.
Could this lead to an IOMMU fault or cause the hardware to read garbage
data if the page has been reallocated?
> +
> + return err;
> +}
^ permalink raw reply
* Re: [PATCH v4 net-next 07/14] net: enetc: add support for "Add" and "Delete" operations to IPFT
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-8-wei.fang@nxp.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.
---
net: enetc: add support for "Add" and "Delete" operations to IPFT
This commit adds support for the "Add" and "Delete" operations to the
ingress port filter table (IPFT). These interfaces will be used by both
the ENETC driver and the NETC switch driver.
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> index 3c59b355c142..960d5be8ec42 100644
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
[ ... ]
> @@ -468,6 +471,79 @@ int ntmp_rsst_query_entry(struct ntmp_user *user, u32 *table, int count)
> }
> EXPORT_SYMBOL_GPL(ntmp_rsst_query_entry);
>
> +/**
> + * ntmp_ipft_add_entry - add an entry into the ingress port filter table
> + * @user: target ntmp_user struct
> + * @entry: the entry data, entry->cfge (configuration element data) and
> + * entry->keye (key element data) are used as input. Since the entry ID
> + * is assigned by the hardware, so entry->entry_id is a returned value
> + * for the driver to use, the driver can update/delete/query the entry
> + * based on the entry_id.
> + *
> + * Return: 0 on success, otherwise a negative error code
> + */
> +int ntmp_ipft_add_entry(struct ntmp_user *user,
> + struct ipft_entry_data *entry)
> +{
[ ... ]
> + err = ntmp_alloc_data_mem(&data, (void **)&req);
> + if (err)
> + return err;
> +
> + ntmp_fill_crd(&req->crd, user->tbl.ipft_ver, NTMP_QA_ENTRY_ID,
> + NTMP_GEN_UA_CFGEU | NTMP_GEN_UA_STSEU);
> + req->ak.keye = entry->keye;
> + req->cfge = entry->cfge;
The request sets NTMP_GEN_UA_STSEU to indicate statistics data is present,
but struct ipft_req_ua does not appear to contain a match_count or STSE data
field before cfge.
Since the hardware expects STSE data before CFGE data (as seen in
struct ipft_resp_query), will this cause the hardware to misinterpret the
first 8 bytes of cfge as the match_count, and then read the actual filter
configuration from out-of-bounds or uninitialized memory?
> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp_private.h b/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> index 7d50af7745c7..6d519d59a433 100644
> --- a/drivers/net/ethernet/freescale/enetc/ntmp_private.h
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp_private.h
[ ... ]
> +/* Ingress Port Filter Table Request Data Buffer Format of Update and
> + * Add actions
> + */
> +struct ipft_req_ua {
> + struct ntmp_cmn_req_data crd;
> + union ipft_access_key ak;
> + struct ipft_cfge_data cfge;
> +};
Does this struct need an STSE data field before cfge to match the request
header flags?
> diff --git a/include/linux/fsl/ntmp.h b/include/linux/fsl/ntmp.h
> index a54945dcdc61..efda35e5e22c 100644
> --- a/include/linux/fsl/ntmp.h
> +++ b/include/linux/fsl/ntmp.h
[ ... ]
> @@ -66,6 +68,94 @@ struct maft_entry_data {
> struct maft_cfge_data cfge;
> };
>
> +struct ipft_pld_byte {
> + u8 data;
> + u8 mask;
> +};
> +
> +struct ipft_keye_data {
> + __le16 precedence;
> + __le16 resv0[3];
> + __le16 frm_attr_flags;
> +#define IPFT_FAF_OVLAN BIT(2)
> +#define IPFT_FAF_IVLAN BIT(3)
> +#define IPFT_FAF_IP_HDR BIT(7)
> +#define IPFT_FAF_IP_VER6 BIT(8)
> +#define IPFT_FAF_L4_CODE GENMASK(11, 10)
> +#define IPFT_FAF_TCP_HDR 1
> +#define IPFT_FAF_UDP_HDR 2
> +#define IPFT_FAF_SCTP_HDR 3
> +#define IPFT_FAF_WOL_MAGIC BIT(12)
> + __le16 frm_attr_flags_mask;
> + __le16 dscp;
> +#define IPFT_DSCP GENMASK(5, 0)
> +#define IPFT_DSCP_MASK GENMASK(11, 0)
Does IPFT_DSCP_MASK completely overlap with IPFT_DSCP?
If it is defined as GENMASK(11, 0) instead of GENMASK(11, 6), using
FIELD_PREP with this mask will not shift the value into the upper bits,
which might overwrite the actual DSCP value in bits 0-5.
^ permalink raw reply
* Re: [PATCH v4 net-next 09/14] net: dsa: add NETC switch tag support
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-10-wei.fang@nxp.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.
---
net: dsa: add NETC switch tag support
Adds support for the proprietary NXP NETC switch tag used in NXP NETC
switches version 4.3 or later. The tag has 3 types (Forward, To_Port,
To_Host) and supports a varying length between 6 and 14 bytes.
> diff --git a/net/dsa/tag_netc.c b/net/dsa/tag_netc.c
> new file mode 100644
> index 000000000000..addd41f7f1b6
> --- /dev/null
> +++ b/net/dsa/tag_netc.c
[ ... ]
> +static void *netc_fill_common_tp_tag(struct sk_buff *skb,
> + struct net_device *ndev,
> + u8 subtype, int tag_len)
> +{
> + struct dsa_port *dp = dsa_user_to_port(ndev);
> + u16 queue = skb_get_queue_mapping(skb);
> + u8 ipv = netdev_txq_to_tc(ndev, queue);
What happens here if the interface doesn't have traffic classes configured?
By default, netdev_txq_to_tc() returns -1. Because ipv is an unsigned 8-bit
integer, this casts to 255.
When this is later passed to FIELD_PREP(NETC_TAG_IPV, ipv) in
netc_fill_common_tag(), the macro blindly masks the variable, silently
forcing the IPV field to 7 (the maximum priority) for all default,
unconfigured traffic. Valid TC values greater than 7 would also be silently
truncated.
Does this code bypass the intended QoS mechanisms?
> + void *tag;
> +
> + skb_push(skb, tag_len);
> + dsa_alloc_etype_header(skb, tag_len);
> +
> + tag = dsa_etype_header_pos_tx(skb);
> + memset(tag + NETC_TAG_CMN_LEN, 0, tag_len - NETC_TAG_CMN_LEN);
> + netc_fill_common_tag(tag, NETC_TAG_TO_PORT, subtype,
> + dp->ds->index, dp->index, ipv);
Will this break communication for standard single-switch setups?
The driver passes dp->ds->index as the hardware switch ID here, which
defaults to 0 for the first switch in a DSA tree. However, on the receive
path, frames stamped with switch ID 0 are rejected.
If the physical switch hardware uses an ID greater than 0, frames will fail
the lookup in dsa_conduit_find_user() on receive because the software index
is 0.
> +
> + return tag;
> +}
[ ... ]
> +static struct sk_buff *netc_rcv(struct sk_buff *skb,
> + struct net_device *ndev)
> +{
> + struct netc_tag_cmn *tag_cmn;
> + int tag_len, sw_id, port;
[ ... ]
> + if (tag_cmn->qos & NETC_TAG_QV)
> + skb->priority = FIELD_GET(NETC_TAG_IPV, tag_cmn->qos);
> +
> + sw_id = NETC_TAG_SWITCH & tag_cmn->switch_port;
This isn't a bug, but is there a reason to use a raw bitwise AND for sw_id
instead of using FIELD_GET(NETC_TAG_SWITCH, ...) to match how the port
field is extracted just below this?
> + /* ENETC VEPA switch ID (0) is not supported yet */
> + if (!sw_id) {
> + dev_warn_ratelimited(&ndev->dev,
> + "VEPA switch ID is not supported yet\n");
> +
> + return NULL;
> + }
> +
> + port = FIELD_GET(NETC_TAG_PORT, tag_cmn->switch_port);
> + skb->dev = dsa_conduit_find_user(ndev, sw_id, port);
[ ... ]
^ permalink raw reply
* Re: [PATCH v4 net-next 10/14] net: dsa: netc: introduce NXP NETC switch driver for i.MX94
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-11-wei.fang@nxp.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.
---
net: dsa: netc: introduce NXP NETC switch driver for i.MX94
This commit introduces the initial NXP NETC switch driver for the i.MX94
series. It supports basic probe and remove functionality for the integrated
802.1Q Ethernet switch. The switch includes 3 user ports and 1 CPU port
connected to an internal ENETC.
> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> new file mode 100644
> index 000000000000..5828fd3e342e
> --- /dev/null
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> +static int netc_port_create_emdio_bus(struct netc_port *np,
> + struct device_node *node)
> +{
[ ... ]
> + err = devm_of_mdiobus_register(dev, bus, node);
> + if (err)
> + return dev_err_probe(dev, err,
> + "Cannot register EMDIO bus\n");
> +
> + np->emdio = bus;
> +
> + return 0;
> +}
[ ... ]
> +static int netc_init_switch_id(struct netc_switch *priv)
> +{
> + struct netc_switch_regs *regs = &priv->regs;
> + struct dsa_switch *ds = priv->ds;
> +
> + /* The value of 0 is reserved for the VEPA switch and cannot
> + * be used.
> + */
> + if (ds->index > SWCR_SWID || !ds->index) {
> + dev_err(priv->dev, "Switch index %d out of range\n",
> + ds->index);
> + return -ERANGE;
> + }
Does this check cause the probe to fail unconditionally for standard
single-switch topologies?
The DSA core typically assigns ds->index = 0 by default for the first switch.
Because !ds->index evaluates to true for index 0, this setup function will
always fail unless the dsa,member property is explicitly overridden in the
device tree.
Could the driver translate the hardware ID internally, for example by writing
ds->index + 1 to NETC_SWCR, rather than forcing this hardware-specific
restriction onto the software DSA index?
> +
> + netc_base_wr(regs, NETC_SWCR, ds->index);
> +
> + return 0;
> +}
[ ... ]
> +static bool netc_port_is_emdio_consumer(struct device_node *node)
> +{
> + struct device_node *mdio_node;
> +
> + /* If the port node has phy-handle property and it does
> + * not contain a mdio child node, then the port is the
> + * EMDIO consumer.
> + */
> + mdio_node = of_get_child_by_name(node, "mdio");
> + if (!mdio_node)
> + return true;
> +
> + of_node_put(mdio_node);
> +
> + return false;
> +}
> +
> +/* Currently, phylink_of_phy_connect() is called by dsa_user_create(),
> + * so if the switch uses the external MDIO controller (like the EMDIO
> + * function) to manage the external PHYs. The MDIO bus may not be
> + * created when phylink_of_phy_connect() is called, so it will return
> + * an error and cause the switch driver to fail to probe.
> + * This workaround can be removed when DSA phylink_of_phy_connect()
> + * calls are moved from probe() to ndo_open().
> + */
> +static int netc_switch_check_emdio_is_ready(struct device *dev)
> +{
[ ... ]
> + for_each_available_child_of_node_scoped(ports, child) {
> + /* If the node does not have phy-handle property, then
> + * the port does not connect to a PHY, so the port is
> + * not the EMDIO consumer.
> + */
> + phy_node = of_parse_phandle(child, "phy-handle", 0);
> + if (!phy_node)
> + continue;
> +
> + if (!netc_port_is_emdio_consumer(child)) {
> + of_node_put(phy_node);
> + continue;
> + }
> +
> + phydev = of_phy_find_device(phy_node);
> + of_node_put(phy_node);
> + if (!phydev) {
> + err = -EPROBE_DEFER;
> + goto out;
> + }
Could this logic cause an infinite probe deferral loop for ports sharing an
internal MDIO bus?
The netc_port_is_emdio_consumer() function assumes any port node without an
mdio child is an EMDIO consumer waiting for an external controller. However,
if multiple sibling ports on the same switch share a single internal MDIO bus
defined under one port, the other ports will lack an mdio child node.
This would cause those ports to be mistakenly treated as external EMDIO
consumers, calling of_phy_find_device(). Because the internal MDIO bus is not
registered until later in netc_setup(), the PHY device cannot be found, and
the driver returns -EPROBE_DEFER.
Since netc_setup() is never reached, the MDIO bus is never registered,
causing an infinite deferral loop. Could the driver more accurately identify
whether the PHY belongs to its own switch before deferring?
> +
> + put_device(&phydev->mdio.dev);
> + }
> +
> +out:
> + of_node_put(ports);
> +
> + return err;
> +}
> +
> +static int netc_switch_pci_init(struct pci_dev *pdev)
> +{
[ ... ]
> + regs->base = pci_ioremap_bar(pdev, NETC_REGS_BAR);
> + if (!regs->base) {
> + err = -ENXIO;
> + dev_err(dev, "pci_ioremap_bar() failed\n");
> + goto release_mem_regions;
> + }
[ ... ]
> +static void netc_switch_pci_destroy(struct pci_dev *pdev)
> +{
> + struct netc_switch *priv = pci_get_drvdata(pdev);
> +
> + iounmap(priv->regs.base);
> + pci_release_mem_regions(pdev);
> + pci_disable_device(pdev);
> +}
Does unmapping the memory here introduce a use-after-unmap issue when the
MDIO bus is unregistered?
The driver registers the MDIO bus using devm_of_mdiobus_register(), meaning
devres cleans it up asynchronously after the remove function returns or when
probe fails.
However, in netc_switch_pci_destroy(), the driver explicitly unmaps the PCI
I/O memory via iounmap(). Since netc_switch_pci_destroy() is called
synchronously inside the remove function and probe error paths, the memory is
unmapped before the MDIO bus is unregistered by devres.
When devres eventually unregisters the MDIO bus, it detaches PHY devices,
which often perform MDIO transactions during removal. Because the underlying
memory priv->regs.base is already unmapped, these MDIO read/write callbacks
could trigger a synchronous external abort or kernel panic.
Would it be better to use pcim_iomap() to tie the memory mapping lifecycle to
devres, ensuring it outlives the MDIO bus?
^ permalink raw reply
* Re: [PATCH v4 net-next 11/14] net: dsa: netc: add phylink MAC operations
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-12-wei.fang@nxp.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.
---
net: dsa: netc: add phylink MAC operations
Different versions of NETC switches have different numbers of ports and
MAC capabilities, so add .phylink_get_caps() to struct netc_switch_info.
In addition, related interfaces of struct phylink_mac_ops are added.
> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 5828fd3e342e..f11f5d0f6a6d 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> +static void netc_port_set_mac_mode(struct netc_port *np,
> + unsigned int mode,
> + phy_interface_t phy_mode)
> +{
> + u32 mask = PM_IF_MODE_IFMODE | PM_IF_MODE_REVMII;
> + u32 val = 0;
> +
> + switch (phy_mode) {
> + case PHY_INTERFACE_MODE_RGMII:
[ ... ]
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + val |= IFMODE_SGMII;
> + break;
> + default:
> + break;
> + }
> +
> + netc_mac_port_rmw(np, NETC_PM_IF_MODE(0), mask, val);
> +}
In imx94_switch_phylink_get_caps(), PHY_INTERFACE_MODE_1000BASEX is
advertised as supported for ports 0 and 1. Does it need to be handled
in this switch statement? As written, it appears it will fall through
to the default case and clear the IFMODE bits, leaving the MAC in an
undefined mode.
[ ... ]
> +static void netc_port_mac_rx_enable(struct netc_port *np)
> +{
> + netc_port_rmw(np, NETC_POR, PCR_RXDIS, 0);
> + netc_mac_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_RX_EN,
> + PM_CMD_CFG_RX_EN);
> +}
Pseudo ports (like the CPU port) bypass PMAC register accesses during
graceful stop via is_netc_pseudo_port(). Should there be a similar check
here, and in netc_port_set_mac_mode(), to prevent accessing physical MAC
registers on pseudo ports?
Additionally, when np->caps.pmac is true, netc_port_mac_rx_graceful_stop()
disables the preemption MAC via NETC_PM_CMD_CFG(1). Should this function
re-enable it when bringing the link back up?
> +static void netc_port_wait_rx_empty(struct netc_port *np, int mac)
> +{
> + u32 val;
> +
> + if (read_poll_timeout(netc_port_rd, val, val & PM_IEVENT_RX_EMPTY,
> + 100, 10000, false, np, NETC_PM_IEVENT(mac)))
> + dev_warn(np->switch_priv->dev,
> + "MAC %d of swp%d RX is not empty\n", mac,
> + np->dp->index);
> +}
Is NETC_PM_IEVENT a sticky write-1-to-clear register? If the RX FIFO had
previously emptied during normal operation, could this bit already be set,
causing read_poll_timeout() to return immediately without waiting?
> +static void netc_port_mac_rx_graceful_stop(struct netc_port *np)
> +{
> + u32 val;
> +
> + if (is_netc_pseudo_port(np))
> + goto check_rx_busy;
> +
> + if (np->caps.pmac) {
> + netc_port_rmw(np, NETC_PM_CMD_CFG(1), PM_CMD_CFG_RX_EN, 0);
> + netc_port_wait_rx_empty(np, 1);
> + }
> +
> + netc_port_rmw(np, NETC_PM_CMD_CFG(0), PM_CMD_CFG_RX_EN, 0);
> + netc_port_wait_rx_empty(np, 0);
> +
> +check_rx_busy:
> + if (read_poll_timeout(netc_port_rd, val, !(val & PSR_RX_BUSY),
> + 100, 10000, false, np, NETC_PSR))
> + dev_warn(np->switch_priv->dev, "swp%d RX is busy\n",
> + np->dp->index);
> +
> + netc_port_rmw(np, NETC_POR, PCR_RXDIS, PCR_RXDIS);
> +}
For pseudo ports, the code jumps directly to check_rx_busy and polls for
PSR_RX_BUSY to clear. Since PCR_RXDIS is only set after the polling
completes, will the RX pipeline continue accepting traffic and cause this
poll to always timeout on an active link?
> +static void netc_mac_link_up(struct phylink_config *config,
> + struct phy_device *phy, unsigned int mode,
> + phy_interface_t interface, int speed,
> + int duplex, bool tx_pause, bool rx_pause)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> + struct netc_port *np;
> +
> + np = NETC_PORT(dp->ds, dp->index);
> + netc_port_set_speed(np, speed);
> +
> + if (phy_interface_mode_is_rgmii(interface))
> + netc_port_set_rgmii_mac(np, speed, duplex);
> +
> + if (interface == PHY_INTERFACE_MODE_RMII ||
> + interface == PHY_INTERFACE_MODE_REVMII ||
> + interface == PHY_INTERFACE_MODE_MII)
> + netc_port_set_rmii_mii_mac(np, speed, duplex);
> +
> + netc_port_mac_rx_enable(np);
> +}
The callback imx94_switch_phylink_get_caps() advertises MAC_ASYM_PAUSE and
MAC_SYM_PAUSE capabilities. Is it expected that tx_pause and rx_pause are
ignored here? If flow control is implemented in a subsequent patch, should
the advertisement be deferred until then?
> +static void netc_mac_link_down(struct phylink_config *config,
> + unsigned int mode,
> + phy_interface_t interface)
> +{
> + struct dsa_port *dp = dsa_phylink_to_port(config);
> +
> + netc_port_mac_rx_graceful_stop(NETC_PORT(dp->ds, dp->index));
> +}
This correctly stops the RX pipeline, but should the TX path
(PM_CMD_CFG_TX_EN or PCR_TXDIS) also be disabled when the link goes down?
^ permalink raw reply
* Re: [PATCH v4 net-next 12/14] net: dsa: netc: add more basic functions support
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-13-wei.fang@nxp.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.
---
net: dsa: netc: add more basic functions support
This patch expands the NETC switch driver with several foundational
features, including FDB and MDB management, STP state handling, MTU
configuration, port setup/teardown, and host flooding support.
At this stage, the driver operates only in standalone port mode. Each
port uses VLAN 0 as its PVID, meaning ingress frames are internally
assigned VID 0 regardless of whether they arrive tagged or untagged.
Note that this does not inject a VLAN 0 header into the frame, the VID
is used purely for subsequent VLAN processing within the switch.
> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> +static int netc_add_standalone_vlan_entry(struct netc_switch *priv)
> +{
> + u32 bitmap_stg = VFT_STG_ID(0) | netc_available_port_bitmap(priv);
Does putting all standalone ports into a single shared VLAN 0 break the DSA
isolation requirements?
If a user manually adds a static FDB entry to a standalone port, it is
programmed into VLAN 0. Because all standalone ports reside in this VLAN, a
frame arriving on one standalone port with a matching destination MAC might
be hardware-switched directly to another standalone port, bypassing the CPU.
[ ... ]
> +static int netc_port_set_fdb_entry(struct netc_port *np,
> + const unsigned char *addr, u16 vid)
> +{
[ ... ]
> + /* If the entry already exists, but not on this port, we need to
> + * update the port bitmap. In general, it should only be valid
> + * for multicast or broadcast address.
> + */
> + port_bitmap ^= BIT(port);
> + entry->cfge.port_bitmap = cpu_to_le32(port_bitmap);
Can this XOR operation cause multi-port floods during unicast MAC migrations?
When a unicast MAC address moves to a new port, the bridging layer issues an
ADD for the new port. The XOR operation leaves the old port bit set alongside
the new port bit. Does the driver need to differentiate between unicast and
multicast addresses and explicitly overwrite the bitmap for unicast entries?
[ ... ]
> +static int netc_port_del_fdb_entry(struct netc_port *np,
> + const unsigned char *addr, u16 vid)
> +{
> + struct netc_switch *priv = np->switch_priv;
> + struct ntmp_user *ntmp = &priv->ntmp;
> + struct netc_fdb_entry *entry;
> + int port = np->dp->index;
> + u32 port_bitmap;
> + int err = 0;
> +
> + mutex_lock(&priv->fdbt_lock);
> +
> + entry = netc_lookup_fdb_entry(priv, addr, vid);
> + if (unlikely(!entry))
> + goto unlock_fdbt;
Will this prevent dynamically learned FDB entries from being deleted?
Since the CPU port is configured with MLO_HW, the switch hardware dynamically
learns MAC addresses, but they are not added to the software priv->fdb_list.
If a user attempts to delete a dynamic entry, the software lookup will fail
here and return 0 (success), leaving the stale dynamically learned entry
active in hardware.
[ ... ]
> +static int netc_port_change_mtu(struct dsa_switch *ds,
> + int port, int mtu)
> +{
> + u32 max_frame_size = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> + struct netc_port *np = NETC_PORT(ds, port);
> +
> + if (dsa_is_cpu_port(ds, port))
> + max_frame_size += NETC_TAG_MAX_LEN;
Does this double-count the DSA tag overhead for the CPU port?
The DSA core computes the CPU port's MTU by adding the tagger overhead before
calling the switch's .port_change_mtu() function. If the maximum MTU of 9582
bytes is requested, the CPU port receives an MTU of 9596. Adding
NETC_TAG_MAX_LEN again here results in 9628 bytes, which exceeds the
hardware limit of 9600 (NETC_MAX_FRAME_LEN) and might lead to truncation.
[ ... ]
> +static int netc_port_fdb_add(struct dsa_switch *ds, int port,
> + const unsigned char *addr, u16 vid,
> + struct dsa_db db)
> +{
> + struct netc_port *np = NETC_PORT(ds, port);
> +
> + /* Currently, we only support standalone port mode, so all VLANs
> + * should be converted to NETC_STANDALONE_PVID.
> + */
> + return netc_port_set_fdb_entry(np, addr, NETC_STANDALONE_PVID);
> +}
Is it safe to silently override user-requested VLAN IDs to VLAN 0?
If a user explicitly attempts to configure a MAC address for a specific VLAN,
the hardware is programmed for VLAN 0 and returns success. Should this
instead check if the requested vid is not 0 and not NETC_STANDALONE_PVID,
and return -EOPNOTSUPP to inform the networking stack?
[ ... ]
> +static void netc_port_set_host_flood(struct dsa_switch *ds, int port,
> + bool uc, bool mc)
> +{
> + struct netc_port *np = NETC_PORT(ds, port);
> +
> + if (np->uc == uc && np->mc == mc)
> + return;
> +
> + /* IPFT does not support in-place updates to the KEYE element,
> + * so we need to delete the old IPFT entry and then add a new
> + * one.
> + */
> + if (np->host_flood)
> + netc_port_remove_host_flood(np);
> +
> + if (netc_port_add_host_flood_rule(np, uc, mc))
> + dev_err(ds->dev, "Failed to add host flood rule on port %d\n",
> + port);
> +}
Could this permanently lose the host flood configuration if the new rule
addition fails?
If the call to netc_port_add_host_flood_rule() fails (for example, due to
-ENOMEM), the function logs an error and returns, leaving the port entirely
without a host flood rule. Should the driver revert to the previous
configuration on failure to avoid blackholing traffic meant for the CPU?
^ permalink raw reply
* Re: [PATCH v4 net-next 13/14] net: dsa: netc: initialize buffer bool table and implement flow-control
From: Jakub Kicinski @ 2026-04-03 1:17 UTC (permalink / raw)
To: wei.fang
Cc: Jakub Kicinski, claudiu.manoil, vladimir.oltean, xiaoning.wang,
andrew+netdev, davem, edumazet, pabeni, robh, krzk+dt, conor+dt,
f.fainelli, frank.li, chleroy, horms, linux, andrew, netdev,
linux-kernel, devicetree, linuxppc-dev, linux-arm-kernel, imx
In-Reply-To: <20260331113025.1566878-14-wei.fang@nxp.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.
---
net: dsa: netc: initialize buffer bool table and implement flow-control
This commit adds buffer pool initialization and flow control support for the
NXP netc switch. It maps frame priorities to specific buffer pools and
configures transmission pause thresholds.
> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index 3609d83ac363..688dceb486c2 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> @@ -386,6 +388,15 @@ static void netc_port_fixed_config(struct netc_port *np)
> /* Enable L2 and L3 DOS */
> netc_port_rmw(np, NETC_PCR, PCR_L2DOSE | PCR_L3DOSE,
> PCR_L2DOSE | PCR_L3DOSE);
> +
> + /* Set the quanta value of TX PAUSE frame */
> + netc_mac_port_wr(np, NETC_PM_PAUSE_QUANTA(0), pqnt);
> +
> + /* When a quanta timer counts down and reaches this value,
> + * the MAC sends a refresh PAUSE frame with the programmed
> + * full quanta value if a pause condition still exists.
> + */
> + netc_mac_port_wr(np, NETC_PM_PAUSE_TRHESH(0), qth);
This isn't a bug, but there's a typo in the macro name (TRHESH instead of
THRESH).
[ ... ]
> +static u32 netc_get_buffer_pool_num(struct netc_switch *priv)
> +{
> + /* The BPCAPR register is a read only register, the hardware
> + * guarantees that the BPCAPR_NUM_BP field will not be 0.
> + */
> + return netc_base_rd(&priv->regs, NETC_BPCAPR) & BPCAPR_NUM_BP;
> +}
[ ... ]
> +static void netc_ipv_to_buffer_pool_mapping(struct netc_switch *priv)
> +{
> + int bp_per_port = priv->num_bp / priv->info->num_ports;
> + int q = NETC_IPV_NUM / bp_per_port;
> + int r = NETC_IPV_NUM % bp_per_port;
If the hardware register read in netc_get_buffer_pool_num() returns a value
smaller than priv->info->num_ports (for instance, during a bus error or
reset state), bp_per_port will evaluate to 0.
Can this lead to a division by zero panic here?
Should priv->num_bp be validated against the number of ports before
performing this calculation to ensure the driver fails gracefully if the
hardware returns an unexpected value?
[ ... ]
> diff --git a/drivers/net/dsa/netc/netc_switch_hw.h b/drivers/net/dsa/netc/netc_switch_hw.h
> index c6a0c0a8ff8a..1e1c0d279a21 100644
> --- a/drivers/net/dsa/netc/netc_switch_hw.h
> +++ b/drivers/net/dsa/netc/netc_switch_hw.h
[ ... ]
> @@ -145,6 +152,12 @@ enum netc_stg_stage {
> #define NETC_PM_IEVENT(a) (0x1040 + (a) * 0x400)
> #define PM_IEVENT_RX_EMPTY BIT(6)
>
> +#define NETC_PM_PAUSE_QUANTA(a) (0x1054 + (a) * 0x400)
> +#define PAUSE_QUANTA_PQNT GENMASK(15, 0)
> +
> +#define NETC_PM_PAUSE_TRHESH(a) (0x1064 + (a) * 0x400)
> +#define PAUSE_TRHESH_QTH GENMASK(15, 0)
This isn't a bug, but there's a typo in the macro names here as well
(TRHESH instead of THRESH).
^ permalink raw reply
* Re: [PATCH v1 1/1] scsi: ufs: rockchip: Drop unused include
From: Martin K. Petersen @ 2026-04-03 2:05 UTC (permalink / raw)
To: Shawn Lin, linux-scsi, linux-arm-kernel, linux-rockchip,
linux-kernel, Andy Shevchenko
Cc: Martin K . Petersen, James E.J. Bottomley, Heiko Stuebner
In-Reply-To: <20260320215606.3236516-1-andriy.shevchenko@linux.intel.com>
On Fri, 20 Mar 2026 22:56:06 +0100, Andy Shevchenko wrote:
> This driver includes the legacy header <linux/gpio.h> but does
> not use any symbols from it. Drop the inclusion.
>
>
Applied to 7.1/scsi-queue, thanks!
[1/1] scsi: ufs: rockchip: Drop unused include
https://git.kernel.org/mkp/scsi/c/8ad1ddc50d15
--
Martin K. Petersen
^ permalink raw reply
* Re: [PATCH v5 00/12] scsi: ufs: Add TX Equalization support for UFS 5.0
From: Martin K. Petersen @ 2026-04-03 2:05 UTC (permalink / raw)
To: avri.altman, bvanassche, beanhuo, peter.wang, mani, Can Guo
Cc: Martin K . Petersen, linux-scsi, Matthias Brugger,
AngeloGioacchino Del Regno, linux-kernel, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20260325152154.1604082-1-can.guo@oss.qualcomm.com>
On Wed, 25 Mar 2026 08:21:42 -0700, Can Guo wrote:
> The UFS v5.0 and UFSHCI v5.0 standards have published, introducing support
> for HS-G6 (46.6 Gbps per lane) through the new UniPro V3.0 interconnect
> layer and M-PHY V6.0 physical layer specifications. To achieve reliable
> operation at these higher speeds, UniPro V3.0 introduces TX Equalization
> and Pre-Coding mechanisms that are essential for signal integrity.
>
> This patch series implements TX Equalization support in the UFS core
> driver as specified in UFSHCI v5.0, along with the necessary vendor
> operations and a reference implementation for Qualcomm UFS host
> controllers.
>
> [...]
Applied to 7.1/scsi-queue, thanks!
[01/12] scsi: ufs: core: Introduce a new ufshcd vops negotiate_pwr_mode()
https://git.kernel.org/mkp/scsi/c/d3eba21c7170
[02/12] scsi: ufs: core: Pass force_pmc to ufshcd_config_pwr_mode() as a parameter
https://git.kernel.org/mkp/scsi/c/c91c83671642
[03/12] scsi: ufs: core: Add UFS_HS_G6 and UFS_HS_GEAR_MAX to enum ufs_hs_gear_tag
https://git.kernel.org/mkp/scsi/c/6669ab18c223
[04/12] scsi: ufs: core: Add support for TX Equalization
https://git.kernel.org/mkp/scsi/c/03e5d38e2f98
[05/12] scsi: ufs: core: Add debugfs entries for TX Equalization params
https://git.kernel.org/mkp/scsi/c/10c40143f369
[06/12] scsi: ufs: core: Add helpers to pause and resume command processing
https://git.kernel.org/mkp/scsi/c/dc5dcac53278
[07/12] scsi: ufs: core: Add support to retrain TX Equalization via debugfs
https://git.kernel.org/mkp/scsi/c/adbabdcf0db0
[08/12] scsi: ufs: ufs-qcom: Fixup PAM-4 TX L0_L1_L2_L3 adaptation pattern length
https://git.kernel.org/mkp/scsi/c/53c94067efa2
[09/12] scsi: ufs: ufs-qcom: Implement vops tx_eqtr_notify()
https://git.kernel.org/mkp/scsi/c/385b95893e79
[10/12] scsi: ufs: ufs-qcom: Implement vops get_rx_fom()
https://git.kernel.org/mkp/scsi/c/26605db7604d
[11/12] scsi: ufs: ufs-qcom: Implement vops apply_tx_eqtr_settings()
https://git.kernel.org/mkp/scsi/c/16cbdc830877
[12/12] scsi: ufs: ufs-qcom: Enable TX Equalization
https://git.kernel.org/mkp/scsi/c/57b7943fd87f
--
Martin K. Petersen
^ permalink raw reply
* Re: [PATCH v2] iommu/arm-smmu: Use pm_runtime in fault handlers
From: Pranjal Shrivastava @ 2026-04-03 2:20 UTC (permalink / raw)
To: Prakash Gupta
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Clark, Connor Abbott,
linux-arm-msm, linux-arm-kernel, iommu, linux-kernel,
Akhil P Oommen, Pratyush Brahma
In-Reply-To: <20260313-smmu-rpm-v2-1-8c2236b402b0@oss.qualcomm.com>
On Fri, Mar 13, 2026 at 03:53:53PM +0530, Prakash Gupta wrote:
> Commit d4a44f0750bb ("iommu/arm-smmu: Invoke pm_runtime across the driver")
> enabled pm_runtime for the arm-smmu device. On systems where the SMMU
> sits in a power domain, all register accesses must be done while the
> device is runtime active to avoid unclocked register reads and
> potential NoC errors.
>
> So far, this has not been an issue for most SMMU clients because
> stall-on-fault is enabled by default. While a translation fault is
> being handled, the SMMU stalls further translations for that context
> bank, so the fault handler would not race with a powered-down
> SMMU.
>
> Adreno SMMU now disables stall-on-fault in the presence of fault
> storms to avoid saturating SMMU resources and hanging the GMU. With
> stall-on-fault disabled, the SMMU can generate faults while its power
> domain may no longer be enabled, which makes unclocked accesses to
> fault-status registers in the SMMU fault handlers possible.
>
> Guard the context and global fault handlers with pm_runtime_get_if_active()
> and pm_runtime_put_autosuspend() so that all SMMU fault register accesses
> are done with the SMMU powered. In case pm_runtime is not active we can
> safely ignore the fault as for pm runtime resume the smmu device is
> reset and fault registers are cleared.
>
> Fixes: b13044092c1e ("drm/msm: Temporarily disable stall-on-fault after a page fault")
> Co-developed-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
> Signed-off-by: Prakash Gupta <prakash.gupta@oss.qualcomm.com>
> ---
> Changes in v2:
> - Switched from arm_smmu_rpm_get()/arm_smmu_rpm_put() wrappers to
> pm_runtime_get_if_active()/pm_runtime_put_autosuspend() APIs
> - Added support for smmu->impl->global_fault callback in global fault handler
> - Remove threaded irq context fault restriction to allow modifying stall
> mode for adreno smmu
> - Link to v1: https://patch.msgid.link/20260127-smmu-rpm-v1-1-2ef2f4c85305@oss.qualcomm.com
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 60 +++++++++++++++++++++++------------
> 1 file changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 5e690cf85ec9..f4c46491a03d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -462,10 +462,20 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
> int idx = smmu_domain->cfg.cbndx;
> int ret;
>
> + if (!pm_runtime_get_if_active(smmu->dev))
Note that the pm_runtime_get_if_active(dev) only returns a positive
value if the device state is exactly RPM_ACTIVE. If the device is in the
middle of its runtime_suspend callback, its state is RPM_SUSPENDING.
Thus, if a fault races with the suspend callback, we'll return IRQ_NONE
and the suspend callback doesn't seem to be disabling interrupts.
This isn't any better if we're in a fault-storm caused by
level-triggered interrupts, we'd simply keep entering this handler and
return.
I believe we should clear / handle any pending faults/interrupts or
atleast mask interrupt in the suspend handler to avoid this situation.
> + return IRQ_NONE;
> +
Maybe we could add another wrapped-helper to maintain consistency:
static inline int arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
{
if (!pm_runtime_enabled(smmu->dev))
return 1; // Assume active/powered if RPM is not used
return pm_runtime_get_if_active(smmu->dev);
}
This returns -EINVAL otherwise which isn't a problem for the if
condition but slightly cleaner.
> + if (smmu->impl && smmu->impl->context_fault) {
> + ret = smmu->impl->context_fault(irq, dev);
> + goto out_power_off;
> + }
> +
We've moved impl-specific handlers here, I don't see a functional change.
This looks fine.
> arm_smmu_read_context_fault_info(smmu, idx, &cfi);
>
> - if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT))
> - return IRQ_NONE;
> + if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT)) {
> + ret = IRQ_NONE;
> + goto out_power_off;
> + }
>
> ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
> cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
> @@ -480,7 +490,12 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
> ret == -EAGAIN ? 0 : ARM_SMMU_RESUME_TERMINATE);
> }
>
> - return IRQ_HANDLED;
> + ret = IRQ_HANDLED;
> +
> +out_power_off:
> + pm_runtime_put_autosuspend(smmu->dev);
Nit: Please use arm_smmu_rpm_put() here.. while at it, I guess we can
also bring back pm_runtime_put_autosuspend() in arm_smmu_rpm_put() since
it is updated now to also mark last busy.
> +
> + return ret;
> }
>
> static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
> @@ -489,14 +504,25 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
> struct arm_smmu_device *smmu = dev;
> static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
> DEFAULT_RATELIMIT_BURST);
> + int ret;
> +
> + if (!pm_runtime_get_if_active(smmu->dev))
> + return IRQ_NONE;
> +
Same here.
> + if (smmu->impl && smmu->impl->global_fault) {
> + ret = smmu->impl->global_fault(irq, dev);
> + goto out_power_off;
> + }
>
> gfsr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
> gfsynr0 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR0);
> gfsynr1 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR1);
> gfsynr2 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR2);
>
> - if (!gfsr)
> - return IRQ_NONE;
> + if (!gfsr) {
> + ret = IRQ_NONE;
> + goto out_power_off;
> + }
>
> if (__ratelimit(&rs)) {
> if (IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT) &&
> @@ -513,7 +539,11 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
> }
>
> arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, gfsr);
> - return IRQ_HANDLED;
> + ret = IRQ_HANDLED;
> +
> +out_power_off:
> + pm_runtime_put_autosuspend(smmu->dev);
> + return ret;
> }
>
> static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> @@ -683,7 +713,6 @@ static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
> enum io_pgtable_fmt fmt;
> struct iommu_domain *domain = &smmu_domain->domain;
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> - irqreturn_t (*context_fault)(int irq, void *dev);
>
> mutex_lock(&smmu_domain->init_mutex);
> if (smmu_domain->smmu)
> @@ -850,19 +879,14 @@ static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
> */
> irq = smmu->irqs[cfg->irptndx];
>
> - if (smmu->impl && smmu->impl->context_fault)
> - context_fault = smmu->impl->context_fault;
> - else
> - context_fault = arm_smmu_context_fault;
> -
> if (smmu->impl && smmu->impl->context_fault_needs_threaded_irq)
> ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
> - context_fault,
> + arm_smmu_context_fault,
> IRQF_ONESHOT | IRQF_SHARED,
> "arm-smmu-context-fault",
> smmu_domain);
> else
> - ret = devm_request_irq(smmu->dev, irq, context_fault, IRQF_SHARED,
> + ret = devm_request_irq(smmu->dev, irq, arm_smmu_context_fault, IRQF_SHARED,
> "arm-smmu-context-fault", smmu_domain);
>
> if (ret < 0) {
> @@ -2125,7 +2149,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> int num_irqs, i, err;
> u32 global_irqs, pmu_irqs;
> - irqreturn_t (*global_fault)(int irq, void *dev);
>
> smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> if (!smmu) {
> @@ -2205,18 +2228,13 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> smmu->num_context_irqs = smmu->num_context_banks;
> }
>
> - if (smmu->impl && smmu->impl->global_fault)
> - global_fault = smmu->impl->global_fault;
> - else
> - global_fault = arm_smmu_global_fault;
> -
> for (i = 0; i < global_irqs; i++) {
> int irq = platform_get_irq(pdev, i);
>
> if (irq < 0)
> return irq;
>
> - err = devm_request_irq(dev, irq, global_fault, IRQF_SHARED,
> + err = devm_request_irq(dev, irq, arm_smmu_global_fault, IRQF_SHARED,
> "arm-smmu global fault", smmu);
> if (err)
> return dev_err_probe(dev, err,
>
Thanks,
Praan
^ permalink raw reply
* Re: [Question mpam mpam/snapshot+extras/v6.18-rc1] Question with Configuring iommu_group in 'task'
From: Qinxin Xia @ 2026-04-03 2:44 UTC (permalink / raw)
To: Ben Horgan
Cc: amitsinght, baisheng.gao, baolin.wang, carl, dave.martin, david,
dfustini, fenghuay, gshan, james.morse, jonathan.cameron, kobak,
lcherian, linux-arm-kernel, linux-kernel, peternewman,
punit.agrawal, quic_jiles, reinette.chatre, rohit.mathew, scott,
sdonthineni, xhao, zengheng4, Linuxarm
In-Reply-To: <08aacacc-e3e5-4e97-858b-cbcbdb9a1fcf@arm.com>
On 2026/3/27 18:47:49, Ben Horgan <ben.horgan@arm.com> wrote:
> Hi Qinxin,
>
> On 3/27/26 10:21, Qinxin Xia wrote:
>>
>> Hello everyone!
>>
>> In earlier versions, mpam supports the configuration of iommu_groups.
>>
>> 823 static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
>> 824 char *buf, size_t nbytes,
>> loff_t off)
>> 825 {
>> 826 struct rdtgroup *rdtgrp;
>> 827 int iommu_group_id;
>> 828 bool is_iommu;
>> 829 char *pid_str;
>> 830 int ret = 0;
>> 831 pid_t pid;
>> 832
>> 833 rdtgrp = rdtgroup_kn_lock_live(of->kn);
>> 834 if (!rdtgrp) {
>> 835 rdtgroup_kn_unlock(of->kn);
>> 836 return -ENOENT;
>> 837 }
>> 838 rdt_last_cmd_clear();
>> 839
>> 840 if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
>> 841 rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
>> 842 ret = -EINVAL;
>> 843 rdt_last_cmd_puts("Pseudo-locking in progress\n");
>> 844 goto unlock;
>> 845 }
>> 846
>> 847 while (buf && buf[0] != '\0' && buf[0] != '\n') {
>> 848 pid_str = strim(strsep(&buf, ","));
>> 849
>> 850 is_iommu = string_is_iommu_group(pid_str, &iommu_group_id);
>>
>> What puzzles me is why we would put it under 'task'—this seems a little
>> strange to users.It seems they are not related.Why don't we add a new
>> interface like 'iommu'?
>
> I think it is likely that this interface would change if upstream support is added.
>
I have done some work in this direction before, and I will release an
RFC later for further discussion.:-)
>>
>> 851 if (is_iommu)
>> 852 ret = rdtgroup_move_iommu(iommu_group_id, rdtgrp, of);
>> 853 else if (kstrtoint(pid_str, 0, &pid)) {
>> 854 rdt_last_cmd_printf("Task list parsing error pid %s\n", pid_str);
>> 855 ret = -EINVAL;
>> 856 break;
>> 857 }
>> 858
>> 859 if (pid < 0) {
>> 860 rdt_last_cmd_printf("Invalid pid %d\n", pid);
>> 861 ret = -EINVAL;
>> 862 break;
>> 863 }
>> 864
>>
>> In future glue versions, will you re-enable support for iommu_group, and
>> if so, will the configuration scheme be changed?
>
> Please can you let us know about your usecase so that we can get more information to decide
> what the best interface would be?
>
> Thanks,
>
> Ben
>
>
>
We want to use the iommu mpam to implement stream control for different
PCIe devices. By limiting the access bandwidth of some PCIe devices, the
high-priority service devices can obtain more resources.
--
Thanks,
Qinxin
^ permalink raw reply
* ✅ PASS: Test report for for-kernelci (7.0.0-rc6, upstream-arm-next, 129fdb45)
From: cki-project @ 2026-04-03 3:33 UTC (permalink / raw)
To: will, linux-arm-kernel, catalin.marinas
Hi, we tested your kernel and here are the results:
Overall result: PASSED
Merge: OK
Compile: OK
Test: OK
Tested-by: CKI Project <cki-project@redhat.com>
Kernel information:
Commit message: Merge remote-tracking branch 'origin/nocache-cleanup' into for-kernelci
You can find all the details about the test run at
https://datawarehouse.cki-project.org/kcidb/checkouts/redhat:2427004144
If you find a failure unrelated to your changes, please ask the test maintainer to review it.
This will prevent the failures from being incorrectly reported in the future.
Please reply to this email if you have any questions about the tests that we
ran or if you have any suggestions on how to make future tests more effective.
,-. ,-.
( C ) ( K ) Continuous
`-',-.`-' Kernel
( I ) Integration
`-'
______________________________________________________________________________
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx{91,93}-phyboard-segin: Add peb-av-18 overlay
From: Frank Li @ 2026-04-03 4:00 UTC (permalink / raw)
To: Florijan Plohl
Cc: Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, imx, linux-arm-kernel,
devicetree, linux-kernel, upstream
In-Reply-To: <20260402070826.970012-1-florijan.plohl@norik.com>
On Thu, Apr 02, 2026 at 09:08:26AM +0200, Florijan Plohl wrote:
> Add overlay for the PEB-AV-18 adapter on phyBOARD-Segin-i.MX91/93.
> The supported LCD is Powertip PH800480T032-ZHC19 panel (AC220).
>
> Signed-off-by: Florijan Plohl <florijan.plohl@norik.com>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 4 +
> .../imx91-phyboard-segin-peb-av-18.dtso | 142 ++++++++++++++++++
> .../imx93-phyboard-segin-peb-av-18.dtso | 142 ++++++++++++++++++
> 3 files changed, 288 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx91-phyboard-segin-peb-av-18.dtso
> create mode 100644 arch/arm64/boot/dts/freescale/imx93-phyboard-segin-peb-av-18.dtso
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index bae24b53bce6..8f5b3996b678 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -437,17 +437,21 @@ dtb-$(CONFIG_ARCH_MXC) += imx93-kontron-bl-osm-s.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-nash.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin.dtb
>
> +imx91-phyboard-segin-peb-av-18-dtbs += imx91-phyboard-segin.dtb imx91-phyboard-segin-peb-av-18.dtbo
> imx93-phyboard-nash-jtag-dtbs += imx93-phyboard-nash.dtb imx93-phyboard-nash-jtag.dtbo
> imx93-phyboard-nash-peb-wlbt-07-dtbs += imx93-phyboard-nash.dtb imx93-phyboard-nash-peb-wlbt-07.dtbo
> imx93-phyboard-nash-pwm-fan-dtbs += imx93-phyboard-nash.dtb imx93-phyboard-nash-pwm-fan.dtbo
> imx93-phyboard-segin-peb-av-02-dtbs += imx93-phyboard-segin.dtb imx93-phyboard-segin-peb-av-02.dtbo
> +imx93-phyboard-segin-peb-av-18-dtbs += imx93-phyboard-segin.dtb imx93-phyboard-segin-peb-av-18.dtbo
> imx93-phyboard-segin-peb-eval-01-dtbs += imx93-phyboard-segin.dtb imx93-phyboard-segin-peb-eval-01.dtbo
> imx93-phyboard-segin-peb-wlbt-05-dtbs += imx93-phyboard-segin.dtb imx93-phyboard-segin-peb-wlbt-05.dtbo
> imx93-phycore-rpmsg-dtbs += imx93-phyboard-nash.dtb imx93-phyboard-segin.dtb imx93-phycore-rpmsg.dtbo
> +dtb-$(CONFIG_ARCH_MXC) += imx91-phyboard-segin-peb-av-18.dtb
https://sashiko.dev/#/patchset/20260402070826.970012-1-florijan.plohl%40norik.com
"should this be kept with the other imx91 dtb definition"
Frank Li
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-nash-jtag.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-nash-peb-wlbt-07.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-nash-pwm-fan.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin-peb-av-02.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin-peb-av-18.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin-peb-eval-01.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phyboard-segin-peb-wlbt-05.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx93-phycore-rpmsg.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx91-phyboard-segin-peb-av-18.dtso b/arch/arm64/boot/dts/freescale/imx91-phyboard-segin-peb-av-18.dtso
> new file mode 100644
> index 000000000000..ec6ef2e5a11a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx91-phyboard-segin-peb-av-18.dtso
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2026 PHYTEC Messtechnik GmbH
> + *
> + * Author: Florijan Plohl <florijan.plohl@norik.com>
> + */
> +
> +#include <dt-bindings/clock/imx93-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include "imx91-pinfunc.h"
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&{/} {
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + brightness-levels = <0 4 8 16 32 64 128 255>;
> + default-brightness-level = <5>;
> + power-supply = <®_vcc_3v3_con>;
> + pwms = <&pwm7 0 5000000 0>;
> + };
> +
> + panel {
> + compatible = "powertip,ph800480t032-zhc19";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_panel>;
> +
> + backlight = <&backlight>;
> + enable-gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> + power-supply = <®_vcc_3v3_con>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&dpi_to_panel>;
> + };
> + };
> + };
> +
> + pwm7: pwm-7 {
> + compatible = "pwm-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm7>;
> + gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
> + #pwm-cells = <3>;
> + };
> +
> + reg_vcc_3v3_con: regulator-vcc-3v3-con {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC3V3_CON";
> + regulator-max-microvolt = <3300000>;
> + regulator-min-microvolt = <3300000>;
> + };
> +};
> +
> +&dpi_bridge {
> + status = "okay";
> +};
> +
> +&dpi_to_panel {
> + remote-endpoint = <&panel_in>;
> + bus-width = <18>;
> +};
> +
> +&lcdif {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcdif>;
> + assigned-clocks = <&clk IMX93_CLK_VIDEO_PLL>;
> + assigned-clock-rates = <27272728>;
> + status = "okay";
> +};
> +
> +&lpi2c2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + touchscreen@41 {
> + compatible = "ilitek,ili2130";
> + reg = <0x41>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_touchscreen>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio4 1 GPIO_ACTIVE_LOW>;
> + touchscreen-size-x = <800>;
> + touchscreen-size-y = <480>;
> + wakeup-source;
> + };
> +};
> +
> +&media_blk_ctrl {
> + status = "okay";
> +};
> +
> +&iomuxc {
> + pinctrl_lcdif: lcdifgrp {
> + fsl,pins = <
> + MX91_PAD_GPIO_IO00__MEDIAMIX_DISP_CLK 0x50e
> + MX91_PAD_GPIO_IO01__MEDIAMIX_DISP_DE 0x50e
> + MX91_PAD_GPIO_IO02__MEDIAMIX_DISP_VSYNC 0x50e
> + MX91_PAD_GPIO_IO03__MEDIAMIX_DISP_HSYNC 0x50e
> + MX91_PAD_GPIO_IO04__MEDIAMIX_DISP_DATA0 0x50e
> + MX91_PAD_GPIO_IO05__MEDIAMIX_DISP_DATA1 0x50e
> + MX91_PAD_GPIO_IO06__MEDIAMIX_DISP_DATA2 0x50e
> + MX91_PAD_GPIO_IO07__MEDIAMIX_DISP_DATA3 0x50e
> + MX91_PAD_GPIO_IO08__MEDIAMIX_DISP_DATA4 0x50e
> + MX91_PAD_GPIO_IO09__MEDIAMIX_DISP_DATA5 0x51e
> + MX91_PAD_GPIO_IO10__MEDIAMIX_DISP_DATA6 0x50e
> + MX91_PAD_GPIO_IO11__MEDIAMIX_DISP_DATA7 0x50e
> + MX91_PAD_GPIO_IO12__MEDIAMIX_DISP_DATA8 0x50e
> + MX91_PAD_GPIO_IO13__MEDIAMIX_DISP_DATA9 0x50e
> + MX91_PAD_GPIO_IO14__MEDIAMIX_DISP_DATA10 0x50e
> + MX91_PAD_GPIO_IO15__MEDIAMIX_DISP_DATA11 0x50e
> + MX91_PAD_GPIO_IO16__MEDIAMIX_DISP_DATA12 0x506
> + MX91_PAD_GPIO_IO17__MEDIAMIX_DISP_DATA13 0x506
> + MX91_PAD_GPIO_IO18__MEDIAMIX_DISP_DATA14 0x506
> + MX91_PAD_GPIO_IO19__MEDIAMIX_DISP_DATA15 0x506
> + MX91_PAD_GPIO_IO20__MEDIAMIX_DISP_DATA16 0x506
> + MX91_PAD_GPIO_IO21__MEDIAMIX_DISP_DATA17 0x506
> + >;
> + };
> +
> + pinctrl_panel: panelgrp {
> + fsl,pins = <
> + MX91_PAD_CCM_CLKO4__GPIO4_IO29 0x1133e
> + >;
> + };
> +
> + pinctrl_pwm7: pwm7grp {
> + fsl,pins = <
> + MX91_PAD_CCM_CLKO3__GPIO4_IO28 0x1133e
> + >;
> + };
> +
> + pinctrl_touchscreen: touchscreengrp {
> + fsl,pins = <
> + MX91_PAD_ENET1_MDIO__GPIO4_IO1 0x11e
> + MX91_PAD_ENET1_RD2__GPIO4_IO12 0x1133e
> + >;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx93-phyboard-segin-peb-av-18.dtso b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin-peb-av-18.dtso
> new file mode 100644
> index 000000000000..189b0f0472d2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-phyboard-segin-peb-av-18.dtso
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2026 PHYTEC Messtechnik GmbH
> + *
> + * Author: Florijan Plohl <florijan.plohl@norik.com>
> + */
> +
> +#include <dt-bindings/clock/imx93-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include "imx93-pinfunc.h"
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&{/} {
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + brightness-levels = <0 4 8 16 32 64 128 255>;
> + default-brightness-level = <5>;
> + power-supply = <®_vcc_3v3_con>;
> + pwms = <&pwm7 0 5000000 0>;
> + };
> +
> + panel {
> + compatible = "powertip,ph800480t032-zhc19";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_panel>;
> +
> + backlight = <&backlight>;
> + enable-gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> + power-supply = <®_vcc_3v3_con>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&dpi_to_panel>;
> + };
> + };
> + };
> +
> + pwm7: pwm-7 {
> + compatible = "pwm-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm7>;
> + gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;
> + #pwm-cells = <3>;
> + };
> +
> + reg_vcc_3v3_con: regulator-vcc-3v3-con {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC3V3_CON";
> + regulator-max-microvolt = <3300000>;
> + regulator-min-microvolt = <3300000>;
> + };
> +};
> +
> +&dpi_bridge {
> + status = "okay";
> +};
> +
> +&dpi_to_panel {
> + remote-endpoint = <&panel_in>;
> + bus-width = <18>;
> +};
> +
> +&lcdif {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcdif>;
> + assigned-clocks = <&clk IMX93_CLK_VIDEO_PLL>;
> + assigned-clock-rates = <27272728>;
> + status = "okay";
> +};
> +
> +&lpi2c2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + touchscreen@41 {
> + compatible = "ilitek,ili2130";
> + reg = <0x41>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_touchscreen>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio4 1 GPIO_ACTIVE_LOW>;
> + touchscreen-size-x = <800>;
> + touchscreen-size-y = <480>;
> + wakeup-source;
> + };
> +};
> +
> +&media_blk_ctrl {
> + status = "okay";
> +};
> +
> +&iomuxc {
> + pinctrl_lcdif: lcdifgrp {
> + fsl,pins = <
> + MX93_PAD_GPIO_IO00__MEDIAMIX_DISP_CLK 0x50e
> + MX93_PAD_GPIO_IO01__MEDIAMIX_DISP_DE 0x50e
> + MX93_PAD_GPIO_IO02__MEDIAMIX_DISP_VSYNC 0x50e
> + MX93_PAD_GPIO_IO03__MEDIAMIX_DISP_HSYNC 0x50e
> + MX93_PAD_GPIO_IO04__MEDIAMIX_DISP_DATA00 0x50e
> + MX93_PAD_GPIO_IO05__MEDIAMIX_DISP_DATA01 0x50e
> + MX93_PAD_GPIO_IO06__MEDIAMIX_DISP_DATA02 0x50e
> + MX93_PAD_GPIO_IO07__MEDIAMIX_DISP_DATA03 0x50e
> + MX93_PAD_GPIO_IO08__MEDIAMIX_DISP_DATA04 0x50e
> + MX93_PAD_GPIO_IO09__MEDIAMIX_DISP_DATA05 0x51e
> + MX93_PAD_GPIO_IO10__MEDIAMIX_DISP_DATA06 0x50e
> + MX93_PAD_GPIO_IO11__MEDIAMIX_DISP_DATA07 0x50e
> + MX93_PAD_GPIO_IO12__MEDIAMIX_DISP_DATA08 0x50e
> + MX93_PAD_GPIO_IO13__MEDIAMIX_DISP_DATA09 0x50e
> + MX93_PAD_GPIO_IO14__MEDIAMIX_DISP_DATA10 0x50e
> + MX93_PAD_GPIO_IO15__MEDIAMIX_DISP_DATA11 0x50e
> + MX93_PAD_GPIO_IO16__MEDIAMIX_DISP_DATA12 0x506
> + MX93_PAD_GPIO_IO17__MEDIAMIX_DISP_DATA13 0x506
> + MX93_PAD_GPIO_IO18__MEDIAMIX_DISP_DATA14 0x506
> + MX93_PAD_GPIO_IO19__MEDIAMIX_DISP_DATA15 0x506
> + MX93_PAD_GPIO_IO20__MEDIAMIX_DISP_DATA16 0x506
> + MX93_PAD_GPIO_IO21__MEDIAMIX_DISP_DATA17 0x506
> + >;
> + };
> +
> + pinctrl_panel: panelgrp {
> + fsl,pins = <
> + MX93_PAD_CCM_CLKO4__GPIO4_IO29 0x1133e
> + >;
> + };
> +
> + pinctrl_pwm7: pwm7grp {
> + fsl,pins = <
> + MX93_PAD_CCM_CLKO3__GPIO4_IO28 0x1133e
> + >;
> + };
> +
> + pinctrl_touchscreen: touchscreengrp {
> + fsl,pins = <
> + MX93_PAD_ENET1_MDIO__GPIO4_IO01 0x11e
> + MX93_PAD_ENET1_RD2__GPIO4_IO12 0x1133e
> + >;
> + };
> +};
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH] arm64: pi: validate bootargs before parsing them
From: Pengpeng Hou @ 2026-04-03 3:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: linux-arm-kernel, linux-kernel, pengpeng
get_bootargs_cmdline() fetches the raw bootargs property from the FDT
and immediately calls strlen() on it before later passing the same
pointer into the early command-line parser. Flat DT properties are
external boot input, and this path does not prove that bootargs is
NUL-terminated within its declared bounds.
Use fdt_stringlist_get() so malformed unterminated bootargs are
rejected before the local parser treats them as C strings.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
arch/arm64/kernel/pi/idreg-override.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
index bc57b290e5e7..310ed279ef26 100644
--- a/arch/arm64/kernel/pi/idreg-override.c
+++ b/arch/arm64/kernel/pi/idreg-override.c
@@ -373,11 +373,11 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt, int node)
if (node < 0)
return NULL;
- prop = fdt_getprop(fdt, node, bootargs, NULL);
+ prop = fdt_stringlist_get(fdt, node, bootargs, 0, NULL);
if (!prop)
return NULL;
- return strlen(prop) ? prop : NULL;
+ return *prop ? prop : NULL;
}
static __init void parse_cmdline(const void *fdt, int chosen)
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* [PATCH] ARM: compressed: validate memory node device_type strings
From: Pengpeng Hou @ 2026-04-03 3:55 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-kernel, pengpeng
fdt_check_mem_start() walks memory nodes in the compressed-kernel early
FDT and fetches device_type directly with fdt_getprop() before
immediately comparing it with strcmp(). Raw FDT properties are external
boot input, and this path does not prove that device_type is
NUL-terminated within its declared bounds.
Use fdt_stringlist_get() so malformed unterminated device_type
properties are rejected before they are used as C strings.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
arch/arm/boot/compressed/fdt_check_mem_start.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/compressed/fdt_check_mem_start.c b/arch/arm/boot/compressed/fdt_check_mem_start.c
index aa856567fd33..7c433ef33bcb 100644
--- a/arch/arm/boot/compressed/fdt_check_mem_start.c
+++ b/arch/arm/boot/compressed/fdt_check_mem_start.c
@@ -106,7 +106,7 @@ uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt)
/* Walk all memory nodes and regions */
for (offset = fdt_next_node(fdt, -1, NULL); offset >= 0;
offset = fdt_next_node(fdt, offset, NULL)) {
- type = fdt_getprop(fdt, offset, "device_type", NULL);
+ type = fdt_stringlist_get(fdt, offset, "device_type", 0, NULL);
if (!type || strcmp(type, "memory"))
continue;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH 3/3] drm: lcdif: Wait for vblank before disabling DMA
From: Krzysztof Hałasa @ 2026-04-03 4:36 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: dri-devel, imx, linux-arm-kernel, linux-kernel, Marek Vasut,
Stefan Agner, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Lucas Stach, Marco Felsch,
Liu Ying
In-Reply-To: <ac6eQcDS8lr-A4ia@shepard>
Hi Paul,
Paul Kocialkowski <paulk@sys-base.io> writes:
> Interestingly I tried to keep the clocks always on as an experiment and it
> had the opposite effect: the DMA engine would get confused every time after the
> first mode set and disable. So for some reason the disabling of the clocks seems
> to mitigate the issue rather than aggravate it.
Interesting. Fortunately we have a workaround.
>> > + ret = readl_poll_timeout(lcdif->base + LCDC_V8_CTRLDESCL0_5,
>> > + reg, !(reg & CTRLDESCL0_5_SHADOW_LOAD_EN),
>> > + 0, 36000); /* Wait ~2 frame times max */
>>
>> I guess this comment is not necessarily correct - at 2160p30 one frame =
>> ca. 33 ms. Still works, though (I guess anything more than one frame is
>> enough). I don't know how long a frame on HDMI (or LVDS, MIPI etc.) can
>> take. 30 FPS on 2160p is common because the i.MX8MP can't display 2160p60.
>
> Honestly I think we're good assuming 30 fps (33 ms) is a lower bound.
> And the current 36 ms goes even beyond, so I think it's fine.
Right. It is just the comment in the code which is not exactly true.
I.e., we "wait for at least 1 complete frame time". I guess.
Also, the 25 ms in the patch (commit) message is no longer accurate.
>> Also, found an issue. Perhaps unrelated? Powered the board without HDMI
>> connected. Then connected 1080p60 display. It came in 1024x768, console
>> 64x24 :-)
>
> That looks more related to a failure to fetch the EDID from the monitor.
> 1024x768 is the default fallback that is used in this situation.
> Maybe check if there is something wrong with the DDC lines from the hdmi
> controller, maybe pinmux etc.
No no no, I did that on purpose - the monitor was really disconnected at
the boot time. Only then (but before starting weston) i reconnected it.
But it indeed appears to be a separate issue, a software one - a mutex
deadlock on access to clocks and power management. Both enable and
disable paths interfere there. "I will post a patch when I have a patch
to post" :-)
Thanks,
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ permalink raw reply
* Re: [PATCH] iommu/rockchip: fix page table allocation flags for v2 IOMMU
From: Simon Xue @ 2026-04-03 4:40 UTC (permalink / raw)
To: Jonas Karlman, Midgy BALON
Cc: iommu, joro, will, robin.murphy, heiko, linux-arm-kernel,
linux-rockchip, linux-kernel, stable
In-Reply-To: <89ed223d-1a2c-447d-9f21-76969e668855@rock-chips.com>
在 2026/4/1 18:22, Simon Xue 写道:
> Hi Jonas,
>
> 在 2026/4/1 16:41, Jonas Karlman 写道:
>> Hi Simon,
>>
>> On 4/1/2026 9:48 AM, Simon wrote:
>>> Hi Midgy,
>>>
>>> 在 2026/3/31 15:50, Midgy BALON 写道:
>>>> commit 2a7e6400f72b ("iommu: rockchip: Allocate tables from all
>>>> available memory for IOMMU v2") removed GFP_DMA32 from
>>>> iommu_data_ops_v2, reasoning that RK356x and RK3588 IOMMU v2 hardware
>>>> supports up to 40-bit physical addresses for page tables. However, the
>>>> RK3568 IOMMU page-table walker uses a 32-bit AXI bus: it cannot access
>>>> physical addresses above 4 GB regardless of the address encoding
>>>> range.
>>>>
>>>> On boards with more than 4 GB of RAM (e.g. 8 GB LPDDR4X), removing
>>>> GFP_DMA32 causes two distinct failure modes:
>>>>
>>>> 1. Direct allocation above 4 GB: iommu_alloc_pages_sz() may return
>>>> memory above 0x100000000. The hardware page-table walker
>>>> issues a
>>>> bus error trying to dereference those addresses, causing an IOMMU
>>>> fault on the first DMA transaction.
>>> Which IP block is hitting this? We'd like to take a look on our end.
>> I have seen reports that the NPU MMU on RK3568/RK3566 is having some
>> issue using DTE/PTE with >32-bit addresses, maybe it uses a different
>> MMU hw revision or has some hw errata?
>>
>> From my own testing at least the VOP2 MMU on RK3568 (and RK3588) was
>> able to handle 40-bit addressable DTE/PTE, hence the original commit
>> 2a7e6400f72b ("iommu: rockchip: Allocate tables from all available
>> memory for IOMMU v2").
>>
>> As also mentioned in my reply at [1], maybe the NPU MMU has some hw
>> limitation or errata and may need to use a different compatible.
>
> Yes, We are checking internally whether different IOMMU versions
> integrated.
>
> I will share what we find once we have results.
>
We internally checked that the RK356x SoCs integrate two different IOMMU
versions (v1.0 and v2.0), like NPU and ISP use the v1.0 IOMMU.
Both versions can map 40-bit physical pages, but v1.0 does not support
placing the first-level page table above 4 GB.
To fix this, I think we need to land this patch first:
https://lore.kernel.org/all/20260310105303.128859-1-xxm@rock-chips.com/
Then on top of that, we can add a new compatible string to distinguish
the IOMMU versions.
>> [1]
>> https://lore.kernel.org/r/3cd63b3d-1c5e-4a11-856e-c4aeb5d97d55@kwiboo.se/
>>
>> Regards,
>> Jonas
>>
>>>> 2. SWIOTLB bounce-buffer poisoning: without GFP_DMA32, page tables
>>>> land
>>>> above the SWIOTLB window. dma_map_single() with DMA_BIT_MASK(32)
>>>> then bounces them into a buffer below 4 GB.
>>>> rk_dte_get_page_table()
>>>> returns phys_to_virt() of the bounce buffer address; PTEs are
>>>> written
>>>> there; the next dma_sync_single_for_device(DMA_TO_DEVICE)
>>>> copies the
>>>> original (zero) data back over the bounce buffer, silently
>>>> erasing the
>>>> freshly written PTEs. The IOMMU faults because every PTE
>>>> reads as zero.
>>> This probably need a separate patch. One way to fix it would be to
>>> track the
>>> original L2 page table base addresses in struct rk_iommu_domain,
>>> then have rk_dte_get_page_table() return the tracked address instead of
>>> deriving it from the DTE.
>>>
>>>> Restore GFP_DMA32 (and DMA_BIT_MASK(32)) for iommu_data_ops_v2, which
>>>> currently only serves "rockchip,rk3568-iommu" in mainline.
>>>>
>>>> Tested on Radxa ROCK 3B (RK3568, 8 GB LPDDR4X):
>>>> - MobileNetV1 via RKNN: 5.8 ms/inference (IOMMU mode)
>>>> - YOLOv5s 640x640 via RKNN: ~57 ms/inference (IOMMU mode)
>>>> - No IOMMU faults, correct inference results
>>>>
>>>> Fixes: 2a7e6400f72b ("iommu: rockchip: Allocate tables from all
>>>> available memory for IOMMU v2")
>>>> Cc: stable@vger.kernel.org
>>>> Cc: Jonas Karlman <jonas@kwiboo.se>
>>>> Signed-off-by: Midgy BALON <midgy971@gmail.com>
>>>> ---
>>>> drivers/iommu/rockchip-iommu.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/rockchip-iommu.c
>>>> b/drivers/iommu/rockchip-iommu.c
>>>> index 85f3667e797..8b45db29471 100644
>>>> --- a/drivers/iommu/rockchip-iommu.c
>>>> +++ b/drivers/iommu/rockchip-iommu.c
>>>> @@ -1358,8 +1358,8 @@ static struct rk_iommu_ops iommu_data_ops_v2 = {
>>>> .pt_address = &rk_dte_pt_address_v2,
>>>> .mk_dtentries = &rk_mk_dte_v2,
>>>> .mk_ptentries = &rk_mk_pte_v2,
>>>> - .dma_bit_mask = DMA_BIT_MASK(40),
>>>> - .gfp_flags = 0,
>>>> + .dma_bit_mask = DMA_BIT_MASK(32),
>>>> + .gfp_flags = GFP_DMA32,
>>>> };
>>>> static const struct of_device_id rk_iommu_dt_ids[] = {
>>
^ permalink raw reply
* Re: [PATCH 3/3] drm: lcdif: Wait for vblank before disabling DMA
From: Krzysztof Hałasa @ 2026-04-03 4:43 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: dri-devel, imx, linux-arm-kernel, linux-kernel, Marek Vasut,
Stefan Agner, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Lucas Stach, Marco Felsch,
Liu Ying
In-Reply-To: <m3ldf4mzh4.fsf@t19.piap.pl>
Krzysztof Hałasa <khalasa@piap.pl> writes:
> Also, the 25 ms in the patch (commit) message is no longer accurate.
Aaah, it's before 7 a.m. here :-)
On the second try I was finally able to parse that sentence. Perhaps.
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
^ 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