* Re: [PATCH net-next v6 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
From: Maxime Chevallier @ 2026-05-25 19:18 UTC (permalink / raw)
To: Meghana Malladi, liuhangbin, h-mittal1, haokexin, vadim.fedorenko,
devnexen, horms, jacob.e.keller, arnd, afd, basharath, parvathi,
vladimir.oltean, rogerq, danishanwar, pabeni, kuba, edumazet,
davem, andrew+netdev
Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra
In-Reply-To: <20260525182700.3135858-2-m-malladi@ti.com>
Hi Meghana,
On 5/25/26 20:26, Meghana Malladi wrote:
> From: MD Danish Anwar <danishanwar@ti.com>
>
> Introduce QoS infrastructure for Frame Preemption (FPE) support in
> the ICSSG Ethernet driver.
>
> prueth_qos_iet tracks FPE enable/active state and verify state machine
> status via firmware-reported enum icssg_ietfpe_verify_states.
> icssg_config_ietfpe() configures IET FPE in firmware, triggers
> verify state machine based on ethtool MAC Merge parameters.
> Polls firmware verify status up to 3 times with verify_time_ms intervals
> and driver handles timeout by logging error and returning.
> In case of any failure during configuration for enable/disable,
> IET FPE falls back to disabled state.
>
> For MQPRIO qdisc support all queues are express by default later
> gets override by user-provided preemptible_tcs bitmask via tc qdisc mask
> Preempt mask configuration: Maps traffic classes to queue express/preemptible
> state and applied only when FPE is active (Tx enabled)
>
> Verify state machine re-triggers on link up/down events based on
> fpe_enabled and fpe_active flags, and for memory protection, fpe_lock
> serializes all FPE state mutations, preventing races between ethtool
> config, qdisc setup, and link events
>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
There's quite a lot of checkpatch output for this patch, I'll let you
browse through it :)
Also keep in mind that we still recommend 80 chars on net.
Besides that, I have a few comments, see below :)
> ---
>
> v6-v5:
> - Balance fpe_lock mutex lifecycle on all error paths
> - Remove deadcode inside icssg_iet_set_preempt_mask()
> - Add fallback label inside icssg_config_ietfpe() to fix
> stale state machine handling.
> - Replace netdev_err with netdev_info and netdev_dbg wherever
> applicable
> - Remove EXPORT_SYMBOL_GPL for icssg_config_ietfpe()
> - Remove redundant code inside icssg_qos_init()
> - qdisc deletion path clears per-queue map as well.
> - Protect all read/writes to p_mqprio with a mutex
> All the above changes address the comments raised by sashiko
>
> drivers/net/ethernet/ti/Makefile | 3 +-
> drivers/net/ethernet/ti/icssg/icssg_common.c | 1 +
> drivers/net/ethernet/ti/icssg/icssg_config.h | 9 -
> drivers/net/ethernet/ti/icssg/icssg_prueth.c | 6 +
> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 2 +
> drivers/net/ethernet/ti/icssg/icssg_qos.c | 269 +++++++++++++++++++
> drivers/net/ethernet/ti/icssg/icssg_qos.h | 66 +++++
> 7 files changed, 346 insertions(+), 10 deletions(-)
> create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
> create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h
>
> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> index f4276c9a77620..d19bcd25c9d07 100644
> --- a/drivers/net/ethernet/ti/Makefile
> +++ b/drivers/net/ethernet/ti/Makefile
> @@ -46,6 +46,7 @@ icssg-y := icssg/icssg_common.o \
> icssg/icssg_config.o \
> icssg/icssg_mii_cfg.o \
> icssg/icssg_stats.o \
> - icssg/icssg_ethtool.o
> + icssg/icssg_ethtool.o \
> + icssg/icssg_qos.o
>
> obj-$(CONFIG_TI_ICSS_IEP) += icssg/icss_iep.o
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index a28a608f9bf4b..c3ee97e96cd50 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -1724,6 +1724,7 @@ void prueth_netdev_exit(struct prueth *prueth,
>
> netif_napi_del(&emac->napi_rx);
>
> + mutex_destroy(&emac->qos.iet.fpe_lock);
> pruss_release_mem_region(prueth->pruss, &emac->dram);
> free_netdev(emac->ndev);
> prueth->emac[mac] = NULL;
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 60d69744ffae2..1ac202f855ed4 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -323,13 +323,4 @@ struct prueth_fdb_slot {
> u8 fid;
> u8 fid_c2;
> } __packed;
> -
> -enum icssg_ietfpe_verify_states {
> - ICSSG_IETFPE_STATE_UNKNOWN = 0,
> - ICSSG_IETFPE_STATE_INITIAL,
> - ICSSG_IETFPE_STATE_VERIFYING,
> - ICSSG_IETFPE_STATE_SUCCEEDED,
> - ICSSG_IETFPE_STATE_FAILED,
> - ICSSG_IETFPE_STATE_DISABLED
> -};
> #endif /* __NET_TI_ICSSG_CONFIG_H */
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 591be5c8056b4..39f379df923bf 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -392,6 +392,8 @@ static void emac_adjust_link(struct net_device *ndev)
> } else {
> icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
> }
> +
> + icssg_qos_link_state_update(ndev);
> }
>
> if (emac->link) {
> @@ -1652,6 +1654,7 @@ static const struct net_device_ops emac_netdev_ops = {
> .ndo_hwtstamp_get = icssg_ndo_get_ts_config,
> .ndo_hwtstamp_set = icssg_ndo_set_ts_config,
> .ndo_xsk_wakeup = prueth_xsk_wakeup,
> + .ndo_setup_tc = icssg_qos_ndo_setup_tc,
> };
>
> static int prueth_netdev_init(struct prueth *prueth,
> @@ -1686,6 +1689,8 @@ static int prueth_netdev_init(struct prueth *prueth,
>
> INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
>
> + icssg_qos_init(ndev);
> +
> ret = pruss_request_mem_region(prueth->pruss,
> port == PRUETH_PORT_MII0 ?
> PRUSS_MEM_DRAM0 : PRUSS_MEM_DRAM1,
> @@ -1793,6 +1798,7 @@ static int prueth_netdev_init(struct prueth *prueth,
> free:
> pruss_release_mem_region(prueth->pruss, &emac->dram);
> free_ndev:
> + mutex_destroy(&emac->qos.iet.fpe_lock);
> emac->ndev = NULL;
> prueth->emac[mac] = NULL;
> free_netdev(ndev);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index df93d15c5b786..85f7017d2c8e7 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -44,6 +44,7 @@
> #include "icssg_config.h"
> #include "icss_iep.h"
> #include "icssg_switch_map.h"
> +#include "icssg_qos.h"
>
> #define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN)
> #define PRUETH_MIN_PKT_SIZE (VLAN_ETH_ZLEN)
> @@ -254,6 +255,7 @@ struct prueth_emac {
> struct bpf_prog *xdp_prog;
> struct xdp_attachment_info xdpi;
> int xsk_qid;
> + struct prueth_qos qos;
> };
>
> /* The buf includes headroom compatible with both skb and xdpf */
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> new file mode 100644
> index 0000000000000..2781abf39e9bb
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Texas Instruments ICSSG PRUETH QoS submodule
> + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include "icssg_prueth.h"
> +#include "icssg_switch_map.h"
> +
> +static void icssg_iet_set_preempt_mask(struct prueth_emac *emac)
> +{
> + void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> + struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
> + struct tc_mqprio_qopt *qopt = &p_mqprio->mqprio.qopt;
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int prempt_mask = 0, i;
> + u8 tc, num_tc;
> +
> + if (!iet->preemptible_tcs)
> + goto reset_hw;
> +
> + if (iet->fpe_active) {
> + /* Configure the queues based on the preemptible tc map set by the user */
> + num_tc = p_mqprio->mqprio.qopt.num_tc;
> + for (tc = 0; tc < num_tc; tc++) {
> + /* check if the tc is preemptive or not */
> + if (iet->preemptible_tcs & BIT(tc)) {
> + for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> + /* Set all the queues in this tc as preemptive queues */
> + writeb(BIT(4), config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> + }
> + } else {
> + /* Set all the queues in this tc as express queues */
> + for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> + writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> + prempt_mask |= BIT(i);
> + }
> + }
> + netdev_set_tc_queue(emac->ndev, tc, qopt->count[tc], qopt->offset[tc]);
> + }
> + writeb(prempt_mask, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
> + return;
> + }
> +
> +reset_hw:
> + /* Reset to default: all queues as express */
> + for (i = 0; i < ICSSG_MAX_TC_QUEUES; i++)
> + writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> + writeb(ICSSG_EXPRESS_Q_MASK_ALL, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
> +}
> +
> +static int icssg_iet_verify_wait(struct prueth_emac *emac)
> +{
> + void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int try = 3;
> +
> + do {
> + msleep(iet->verify_time_ms);
> + iet->verify_status = readb(config + PRE_EMPTION_VERIFY_STATUS);
> + if (iet->verify_status == ICSSG_IETFPE_STATE_SUCCEEDED)
> + return 0;
> + } while (--try > 0);
You can replace that whole loop with a readb_poll_timeout, to avoid
open-coding it.
> +
> + netdev_err(emac->ndev, "MAC Verify timeout\n");
> + return -ETIMEDOUT;
> +}
> +
> +/* Direct synchronous configuration of IET FPE.
> + * Caller must hold iet->fpe_lock.
> + */
> +int icssg_config_ietfpe(struct net_device *ndev, bool enable)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int ret;
> + u8 val;
> +
> + lockdep_assert_held(&iet->fpe_lock);
> +
> + if (!netif_running(ndev)) {
> + netdev_dbg(ndev, "cannot change IET/FPE state when interface is down\n");
> + return 0;
> + }
> +
> + /* Update FPE Tx enable bit (PRE_EMPTION_ENABLE_TX) if
> + * fpe_enabled is set to enable MM in Tx direction
> + */
> + writeb(enable ? 1 : 0, config + PRE_EMPTION_ENABLE_TX);
> +
> + /* If FPE is to be enabled, first configure MAC Verify state
> + * machine in firmware as firmware kicks the Verify process
> + * as soon as ICSSG_EMAC_PORT_PREMPT_TX_ENABLE command is
> + * received.
> + */
> + if (enable && iet->mac_verify_configure) {
> + writeb(1, config + PRE_EMPTION_ENABLE_VERIFY);
> + writew(iet->tx_min_frag_size + ETH_FCS_LEN,
> + config + PRE_EMPTION_ADD_FRAG_SIZE_LOCAL);
> + writel(iet->verify_time_ms, config + PRE_EMPTION_VERIFY_TIME);
> + } else {
> + writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
> + iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
> + }
> +
> + /* Send command to enable FPE Tx side. Rx is always enabled */
> + ret = icssg_set_port_state(emac,
> + enable ? ICSSG_EMAC_PORT_PREMPT_TX_ENABLE :
> + ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
> + if (ret) {
> + netdev_err(ndev, "TX preempt %s command failed\n",
> + str_enable_disable(enable));
> + goto fallback;
> + }
> +
> + if (enable && iet->mac_verify_configure) {
> + ret = icssg_iet_verify_wait(emac);
> + if (ret) {
> + netdev_err(ndev, "MAC Verification failed with timeout\n");
> + goto disable_tx;
> + }
> + } else if (enable) {
> + /* Give firmware some time to update PRE_EMPTION_ACTIVE_TX state */
> + usleep_range(100, 200);
> + }
> +
> + if (enable) {
> + val = readb(config + PRE_EMPTION_ACTIVE_TX);
> + if (val != 1) {
> + netdev_err(ndev,
> + "Firmware fails to activate IET/FPE\n");
> + ret = -EIO;
> + goto disable_tx;
> + }
> + iet->fpe_active = true;
> + } else {
> + iet->fpe_active = false;
> + }
> +
> + icssg_iet_set_preempt_mask(emac);
> + netdev_info(ndev, "IET FPE %s successfully\n",
> + str_enable_disable(iet->fpe_active));
> + return ret;
> +
> +disable_tx:
> + icssg_set_port_state(emac, ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
> +fallback:
> + writeb(0, config + PRE_EMPTION_ENABLE_TX);
> + writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
> + iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
> + iet->fpe_active = false;
> + return ret;
> +}
> +
> +void icssg_qos_init(struct net_device *ndev)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> +
> + mutex_init(&iet->fpe_lock);
> + /* Set default values to prevent garbage values during .get_mm() */
> + mutex_lock(&iet->fpe_lock);
> + iet->verify_time_ms = ICSSG_IET_MAX_VERIFY_TIME;
> + iet->tx_min_frag_size = ETH_ZLEN;
> + mutex_unlock(&iet->fpe_lock);
> +}
> +EXPORT_SYMBOL_GPL(icssg_qos_init);
> +
> +static int icssg_iet_change_preemptible_tcs(struct prueth_emac *emac)
> +{
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int ret;
> +
> + mutex_lock(&iet->fpe_lock);
> + ret = icssg_config_ietfpe(emac->ndev, iet->fpe_enabled);
> + mutex_unlock(&iet->fpe_lock);
> +
> + return ret;
> +}
> +
> +static int emac_tc_query_caps(struct net_device *ndev, void *type_data)
> +{
> + struct tc_query_caps_base *base = type_data;
> +
> + switch (base->type) {
> + case TC_SETUP_QDISC_MQPRIO: {
> + struct tc_mqprio_caps *caps = base->caps;
> +
> + caps->validate_queue_counts = true;
> + return 0;
> + }
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int emac_tc_setup_mqprio(struct net_device *ndev, void *type_data)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
> + struct tc_mqprio_qopt_offload *mqprio = type_data;
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int ret;
> +
> + /* Validate parameters */
> + if (mqprio->qopt.num_tc > ICSSG_MAX_TC_QUEUES) {
> + netdev_err(ndev, "Number of traffic classes (%u) exceeds hardware limit\n",
> + mqprio->qopt.num_tc);
> + return -EINVAL;
> + }
> +
> + if (mqprio->flags & TC_MQPRIO_F_SHAPER) {
> + netdev_err(ndev, "traffic shaping is not supported\n");
> + return -EINVAL;
Maybe -EOPNOTSUPP ?
> + }
> +
> + if (mqprio->flags & (TC_MQPRIO_F_MIN_RATE | TC_MQPRIO_F_MAX_RATE)) {
> + netdev_err(ndev, "per-queue rate limiting is not supported\n");
> + return -EINVAL;
Same
> + }
> +
> + if (!mqprio->qopt.num_tc) {
> + netdev_reset_tc(ndev);
> + } else {
> + netdev_set_num_tc(ndev, mqprio->qopt.num_tc);
> + }
> +
> + mutex_lock(&iet->fpe_lock);
> + if (!mqprio->qopt.num_tc) {
> + iet->preemptible_tcs = 0;
> + } else {
> + memcpy(&p_mqprio->mqprio, mqprio, sizeof(*mqprio));
> + iet->preemptible_tcs = mqprio->preemptible_tcs;
> + }
> + mutex_unlock(&iet->fpe_lock);
> +
> + netdev_dbg(ndev, "dev->num_tc %u dev->real_num_tx_queues %u\n",
> + ndev->num_tc, ndev->real_num_tx_queues);
> +
> + ret = icssg_iet_change_preemptible_tcs(emac);
> + return ret;
Simplify this with :
return icssg_iet_change_preemptible_tcs(emac);
> +}
> +
> +int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
> + void *type_data)
> +{
> + switch (type) {
> + case TC_QUERY_CAPS:
> + return emac_tc_query_caps(ndev, type_data);
> + case TC_SETUP_QDISC_MQPRIO:
> + return emac_tc_setup_mqprio(ndev, type_data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +EXPORT_SYMBOL_GPL(icssg_qos_ndo_setup_tc);
> +
> +void icssg_qos_link_state_update(struct net_device *ndev)
> +{
> + struct prueth_emac *emac = netdev_priv(ndev);
> + struct prueth_qos_iet *iet = &emac->qos.iet;
> + int ret;
> +
> + ret = icssg_iet_change_preemptible_tcs(emac);
> + if (ret)
> + netdev_dbg(ndev, "IET FPE %s failed\n",
> + str_enable_disable(iet->fpe_active));
> +}
> +EXPORT_SYMBOL_GPL(icssg_qos_link_state_update);
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
> new file mode 100644
> index 0000000000000..9355e96bbcda8
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#ifndef __NET_TI_ICSSG_QOS_H
> +#define __NET_TI_ICSSG_QOS_H
> +
> +#include <linux/atomic.h>
> +#include <linux/netdevice.h>
> +#include <net/pkt_sched.h>
> +
> +#define ICSSG_MAX_TC_QUEUES 8
> +#define ICSSG_EXPRESS_Q_MASK_ALL 0xFF
> +#define ICSSG_IET_MAX_VERIFY_TIME 128
> +#define ICSSG_IET_MIN_VERIFY_TIME 1
> +
> +/**
> + * enum icssg_ietfpe_verify_states - status of MAC Merge Verify returned by firmware
> + * @ICSSG_IETFPE_STATE_UNKNOWN:
> + * verification status is unknown
> + * @ICSSG_IETFPE_STATE_INITIAL:
> + * Firmware returns this if verify state diagram is idle
> + * @ICSSG_IETFPE_STATE_VERIFYING:
> + * Firmware returns this if verification is ongoing
> + * @ICSSG_IETFPE_STATE_SUCCEEDED:
> + * Firmware returns this if verify state diagram completes verification
> + * @ICSSG_IETFPE_STATE_FAILED:
> + * Firmware returns this if verify state diagram fails during verification
> + * @ICSSG_IETFPE_STATE_DISABLED:
> + * verification is disabled by the driver
> + */
> +enum icssg_ietfpe_verify_states {
> + ICSSG_IETFPE_STATE_UNKNOWN = 0,
> + ICSSG_IETFPE_STATE_INITIAL,
> + ICSSG_IETFPE_STATE_VERIFYING,
> + ICSSG_IETFPE_STATE_SUCCEEDED,
> + ICSSG_IETFPE_STATE_FAILED,
> + ICSSG_IETFPE_STATE_DISABLED
> +};
> +
> +struct prueth_qos_mqprio {
> + struct tc_mqprio_qopt_offload mqprio;
> +};
> +
> +struct prueth_qos_iet {
> + bool fpe_enabled;
> + bool mac_verify_configure;
> + u32 tx_min_frag_size;
> + u32 verify_time_ms;
> + bool fpe_active;
> + enum icssg_ietfpe_verify_states verify_status;
> + struct mutex fpe_lock;
Checkpatch already says it, but you need to document what this
mutex is protecting.
> + u8 preemptible_tcs;
> +};
> +
> +struct prueth_qos {
> + struct prueth_qos_iet iet;
> + struct prueth_qos_mqprio mqprio;
> +};
> +
> +void icssg_qos_init(struct net_device *ndev);
> +void icssg_qos_link_state_update(struct net_device *ndev);
> +int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
> + void *type_data);
> +int icssg_config_ietfpe(struct net_device *ndev, bool enable);
> +#endif /* __NET_TI_ICSSG_QOS_H */
Thanks,
Maxime
^ permalink raw reply
* [PATCH v4 5/5] iommu/amd: Fail probe on ATS configuration failure
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava
In-Reply-To: <20260525184347.4059549-1-praan@google.com>
Update the AMD IOMMU driver to handle ATS configuration and enablement
more strictly. Specifically, update the device probe to fail if
pci_prepare_ats() returns an error. This ensures that any ATS-capable
master reaching the attach phase is guaranteed to have a valid config.
Additionally, update pdev_enable_cap_ats() to WARN_ON() if pci_enable_ats
fails. Since earlier checks in the probe phase preclude config-related
failures, any failure during hardware enablement is considered a kernel
bug.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/amd/iommu.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 84cad43dc188..1dddb08e7b22 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -570,10 +570,16 @@ static inline int pdev_enable_cap_ats(struct pci_dev *pdev)
if (amd_iommu_iotlb_sup &&
(dev_data->flags & AMD_IOMMU_DEVICE_FLAG_ATS_SUP)) {
ret = pci_enable_ats(pdev, PAGE_SHIFT);
- if (!ret) {
- dev_data->ats_enabled = 1;
- dev_data->ats_qdep = pci_ats_queue_depth(pdev);
- }
+ /*
+ * pci_enable_ats() should not fail here because earlier checks
+ * have already verified support and configuration.
+ */
+ if (WARN_ON(ret))
+ return ret;
+
+ dev_data->ats_enabled = 1;
+ dev_data->ats_qdep = pci_ats_queue_depth(pdev);
+ ret = 0;
}
return ret;
@@ -2502,8 +2508,17 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
else
dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
- if (dev_is_pci(dev))
- pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
+ if (dev_is_pci(dev)) {
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ if (pci_ats_supported(pdev)) {
+ ret = pci_prepare_ats(pdev, PAGE_SHIFT);
+ if (ret) {
+ iommu_dev = ERR_PTR(ret);
+ goto out_err;
+ }
+ }
+ }
out_err:
return iommu_dev;
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* [PATCH v4 3/5] iommu/arm-smmu-v3: Fix ATS state tracking
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava
In-Reply-To: <20260525184347.4059549-1-praan@google.com>
The SMMUv3 driver currently has a two-phase commit in its ATS enablement
flow. During arm_smmu_attach_prepare(), it predicts whether ATS will be
enabled using arm_smmu_ats_supported() and accordingly increments
nr_ats_masters and merges ATS invalidations into the domain's invs array.
However, the actual hardware enablement via pci_enable_ats() happens
later in arm_smmu_attach_commit(). If this call to pci_enable_ats fails,
the SMMU driver's ATS state tracking remains polluted, i.e., the driver
tracks ATS enabled on a master that is not actually using ATS. This leads
to an incorrect nr_ats_masters and triggers a warning in the PCI core
during detach:
[ 127.925080] ------------[ cut here ]------------
[ 127.925084] WARNING: drivers/pci/ats.c:132 at pci_disable_ats+0x94/0xa8, CPU#42: iova_stress/12240
[ 127.949761] Modules linked in: vfat fat dummy bridge stp llc cdc_ncm cdc_eem cdc_ether usbnet mii xhci_pci xhci_hcd ehci_pci ehci_hcd
[ 127.961760] CPU: 42 UID: 0 PID: 12240 Comm: iova_stress Not tainted 7.1.0-smp-DEV #4 PREEMPTLAZY
[ 127.970619] Hardware name: <REDACTED>
[ 127.977655] pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 127.984603] pc : pci_disable_ats+0x94/0xa8
[ 127.988687] lr : arm_smmu_attach_prepare+0x104/0x310
...
[ 128.068169] Call trace:
[ 128.070603] pci_disable_ats+0x94/0xa8 (P)
[ 128.074688] arm_smmu_attach_prepare+0x104/0x310
[ 128.079292] arm_smmu_attach_dev_ste+0x128/0x1e0
[ 128.083899] arm_smmu_attach_dev_blocked+0x50/0x88
[ 128.088677] __iommu_attach_device+0x30/0x138
[ 128.093026] __iommu_group_set_domain_internal+0xdc/0x228
[ 128.098412] __iommu_take_dma_ownership+0x118/0x150
[ 128.103278] iommu_group_claim_dma_owner+0x48/0x80
[ 128.108056] vfio_container_attach_group+0xc8/0x1b0
[ 128.112927] vfio_group_fops_unl_ioctl+0x578/0x968
[ 128.117706] __arm64_sys_ioctl+0x90/0xe8
The issue was exposed under heavy load when running a VFIO-based DMA
map stress test (iova_stress).
Fix this by ensuring that all failable ATS configuration happens
early during device discovery. Update arm_smmu_probe_device() to call
pci_prepare_ats() only if ATS is supported and fail the probe if
pci_prepare_ats() returns an error, ensuring that any master that reaches
the attach phase is guaranteed to have a valid ATS configuration.
Additionally, update arm_smmu_enable_ats() to use the WARN() macro.
Since earlier checks now preclude configuration-related failures,
any failure during hardware enablement is a noisy kernel bug or fatal
hardware error that should be reported with a backtrace while
allowing the driver to continue in a balanced software state.
Fixes: 7497f4211f4f ("iommu/arm-smmu-v3: Make changing domains be hitless for ATS")
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index e8d7dbe495f0..1d96064d314b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3065,8 +3065,14 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
* ATC invalidation of PASID 0 causes the entire ATC to be flushed.
*/
arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
- if (pci_enable_ats(pdev, stu))
- dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
+
+ /*
+ * Any failure at this point is a kernel bug. pci_ats_supported()
+ * and pci_prepare_ats() have already verified the hardware capability
+ * and programmed the STU. Thus, pci_enable_ats() should not fail here.
+ */
+ WARN(pci_enable_ats(pdev, stu),
+ "Failed to enable ATS (STU %zu)\n", stu);
}
static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
@@ -4264,9 +4270,16 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
master->stall_enabled = true;
if (dev_is_pci(dev)) {
- unsigned int stu = __ffs(smmu->pgsize_bitmap);
+ struct pci_dev *pdev = to_pci_dev(dev);
- pci_prepare_ats(to_pci_dev(dev), stu);
+ if (pci_ats_supported(pdev)) {
+ unsigned int stu = __ffs(smmu->pgsize_bitmap);
+ int ret;
+
+ ret = pci_prepare_ats(pdev, stu);
+ if (ret)
+ return ERR_PTR(ret);
+ }
}
return &smmu->iommu;
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* [PATCH v4 4/5] iommu/vt-d: Fail probe on ATS configuration failure
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava
In-Reply-To: <20260525184347.4059549-1-praan@google.com>
Update the Intel VT-d driver to handle ATS configuration and enablement
more strictly. Specifically, update the device probe to fail if
pci_prepare_ats() returns an error. This ensures that any ATS-capable
master reaching the attach phase is guaranteed to have a valid config.
Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
fails. Since earlier checks in the probe phase preclude config-related
failures, any failure during hardware enablement is considered a kernel
bug.
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/iommu/intel/iommu.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4d0e65bc131d..22308e4911e1 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -873,8 +873,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
if (!pci_ats_page_aligned(pdev))
return;
- if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
- info->ats_enabled = 1;
+ /*
+ * pci_enable_ats() should not fail here because earlier checks
+ * have already verified support and configuration.
+ */
+ if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
+ return;
+
+ info->ats_enabled = 1;
}
static void iommu_disable_pci_ats(struct device_domain_info *info)
@@ -3288,7 +3294,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
dev_iommu_priv_set(dev, info);
if (pdev && pci_ats_supported(pdev)) {
- pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+ ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+ if (ret)
+ goto free;
+
ret = device_rbtree_insert(iommu, info);
if (ret)
goto free;
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* [PATCH v4 2/5] PCI/ATS: Validate STU for VFs in pci_prepare_ats()
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava,
Jason Gunthorpe
In-Reply-To: <20260525184347.4059549-1-praan@google.com>
While every PCI Function that implements ATS has an independent ATS
Extended Capability structure with a Read/Write Smallest Translation
Unit (STU) field, the kernel manages SR-IOV ATS by requiring the IOMMU
driver to configure the STU on the Physical Function (PF) before any
any Virtual Functions (VFs) are created.
Currently, pci_prepare_ats() bails out early for VFs, assuming that the
PF has already been correctly prepared. However, this creates a potential
mismatch if a VF is subsequently prepared with a different page shift.
Update pci_prepare_ats() to validate that the requested page shift (ps)
matches the STU already configured in the associated PF. This ensures
early detection of incompatible configurations and maintains the kernel's
policy of consistent STU sizing across all functions associated with a
given SMMU.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/pci/ats.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index a5fa7745bce8..54319854bfd8 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -73,8 +73,13 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
if (ps < PCI_ATS_MIN_STU)
return -EINVAL;
- if (dev->is_virtfn)
+ if (dev->is_virtfn) {
+ struct pci_dev *pdev = pci_physfn(dev);
+
+ if (pdev->ats_stu != ps)
+ return -EINVAL;
return 0;
+ }
dev->ats_stu = ps;
ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* [PATCH v4 1/5] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava,
Jason Gunthorpe
In-Reply-To: <20260525184347.4059549-1-praan@google.com>
Update pci_ats_supported() to additionally check the associated PF's
status when called on a VF. This ensures that PF-level quirks and
untrusted status are correctly propagated to VFs, providing a robust
support check that aligns with the kernel's PF-centric ATS configuration
model and is immune to the timing of VF-specific fixups.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
drivers/pci/ats.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index ec6c8dbdc5e9..a5fa7745bce8 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -40,10 +40,13 @@ void pci_ats_init(struct pci_dev *dev)
*/
bool pci_ats_supported(struct pci_dev *dev)
{
- if (!dev->ats_cap)
+ if (!dev->ats_cap || dev->untrusted)
return false;
- return (dev->untrusted == 0);
+ if (dev->is_virtfn)
+ return pci_ats_supported(pci_physfn(dev));
+
+ return true;
}
EXPORT_SYMBOL_GPL(pci_ats_supported);
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* [PATCH v4 0/5] iommu: Standardize ATS robustness and state tracking
From: Pranjal Shrivastava @ 2026-05-25 18:43 UTC (permalink / raw)
To: iommu, linux-pci, linux-arm-kernel, linux-kernel
Cc: Joerg Roedel, Will Deacon, Bjorn Helgaas, David Woodhouse,
Lu Baolu, Robin Murphy, Suravee Suthikulpanit, Jason Gunthorpe,
Nicolin Chen, David Matlack, Samiullah Khawaja, Daniel Mentz,
Pasha Tatashin, Mostafa Saleh, Pranjal Shrivastava
The primary motivation for this series is an ATS state mismatch observed
under heavy load (via iova_stress). A failure in pci_enable_ats() leaves
IOMMU drivers like arm-smmu-v3 with inconsistent state leading to PCI core
warnings during device detach.
While David's recent work [1] addressed a discovery race for specific
quirked devices by moving them to the HEADER phase, gaps remained
regarding how Virtual Functions (VFs) inherit state from their Physical
Functions (PFs). Specifically, pci_ats_supported() did not account for
PF-level quirked status, and pci_prepare_ats() lacked STU validation for
VFs.
Based on discussion with Jason in v3 it was decided that IOMMU drivers
should explicitly check pci_ats_supported before calling pci_prepare_ats
and the device probe should fail if pci_prepare_ats fails. Since these
early gates preclude software configuration errors, any remaining failure
during pci_enable_ats() is treated as a kernel bug & reported via WARN().
This series standardizes this pattern across ARM SMMUv3, Intel VT-d, &
AMD IOMMU drivers.
[1] https://lore.kernel.org/linux-pci/20260403222750.1215002-1-dmatlack@google.com/
[v4]
- Standardized the pattern across Intel VT-d and AMD IOMMU drivers.
- Replaced the SMMUv3 ats_prepared gate with a fatal probe-fail logic.
- Utilized WARN() macros for runtime enablement failures in all drivers.
- Collected R-b tags from Jason and Sami.
[v3] https://lore.kernel.org/all/20260519135323.1558777-1-praan@google.com/
[v2] https://lore.kernel.org/all/20260504163842.2692314-1-praan@google.com/
Pranjal Shrivastava (5):
PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs
PCI/ATS: Validate STU for VFs in pci_prepare_ats()
iommu/arm-smmu-v3: Fix ATS state tracking
iommu/vt-d: Fail probe on ATS configuration failure
iommu/amd: Fail probe on ATS configuration failure
drivers/iommu/amd/iommu.c | 27 ++++++++++++++++-----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 21 +++++++++++++---
drivers/iommu/intel/iommu.c | 15 +++++++++---
drivers/pci/ats.c | 14 ++++++++---
4 files changed, 61 insertions(+), 16 deletions(-)
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply
* Re: [PATCH] KVM: arm64: Preserve all guest ZCR_EL2.LEN values
From: Mark Rutland @ 2026-05-25 18:36 UTC (permalink / raw)
To: Mark Brown
Cc: Marc Zyngier, Oliver Upton, Joey Gouly, Steffen Eiden,
Suzuki K Poulose, Catalin Marinas, Will Deacon, linux-arm-kernel,
kvmarm, linux-kernel
In-Reply-To: <20260522-kvm-arm64-fix-zcr-len-nv-v1-1-ec254e9078cf@kernel.org>
On Fri, May 22, 2026 at 07:00:04PM +0100, Mark Brown wrote:
> Since b3d29a823099 ("KVM: arm64: nv: Handle ZCR_EL2 traps") when guests
> write to ZCR_EL2 we have clamped the value of ZCR_EL2.LEN to be at most
> that configuring the maximum guest VL. This is not the behaviour the
> architecture documents for ZCR_EL2.LEN, the expectation is that all bits
> will be read as written. Further, writing values larger than the largest
> available vector length is part of the documented procedure for enumerating
> the supported vector lengths so we expect to see this happen in practice.
>
> The reasoning for the current behaviour is not specifically articulated, my
> best guess is that it is intended to ensure that the guest can not see an
> effective VL greater than the maximum that has been configured. This can
> instead be achieved by configuring ZCR_EL2 when loading guest state:
>
> - When running at EL0 or EL1 configure ZCR_EL2.LEN to the minimum of the
> guest ZCR_EL2.LEN and vcpu_sve_max_vq(vcpu)-1.
> - When running at EL2 configure the maximum VL for the guest in
> ZCR_EL2.LEN like we do for non-nested guests and load the guest
> ZCR_EL2 into ZCR_EL1.
>
> This will ensure that the guest sees both the ZCR_EL2.LEN value which it
> wrote and the effective VL that resulting from the values it has configured
> in ZCR_ELx.LEN.
>
> Currently all other bits in ZCR_EL2 are either RES0 or RAZ/WI, values
> written are sanitised based on this.
>
> Fixes: b3d29a823099 ("KVM: arm64: nv: Handle ZCR_EL2 traps")
> Signed-off-by: Mark Brown <broonie@kernel.org>
For context, I mentioned this potential problem to Mark, and described
this possible solution at:
https://lore.kernel.org/linux-arm-kernel/af4bWxiOogfPz_dp@J2N7QTR9R3/
I said:
AFAICT, none of the values for the SMCR_ELx.LEN and ZCR_ELx.LEN fields
are reserved or unallocated. Thus all the bits of those fields should
be stateful, and a read should observe the last value written,
regardless of the effective value of the field.
[...]
Either what we're doing is wrong, or the architcture requires a
clarification to say that values corresponding to unimplmented vector
lengths are reserved.
Have we sought feedback from architects? While I said "*or* the
architcture requires a clarification", I think it should be clarified
more explicitly either way given that the pattern is unusual.
Given this is particularly subtle, please keep me in the loop when
speaking with architects about this.
Mark.
^ permalink raw reply
* [PATCH net-next v6 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
From: Meghana Malladi @ 2026-05-25 18:26 UTC (permalink / raw)
To: liuhangbin, h-mittal1, haokexin, vadim.fedorenko, devnexen, horms,
jacob.e.keller, m-malladi, arnd, afd, basharath, parvathi,
vladimir.oltean, rogerq, danishanwar, pabeni, kuba, edumazet,
davem, andrew+netdev
Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra
In-Reply-To: <20260525182700.3135858-1-m-malladi@ti.com>
From: MD Danish Anwar <danishanwar@ti.com>
Introduce QoS infrastructure for Frame Preemption (FPE) support in
the ICSSG Ethernet driver.
prueth_qos_iet tracks FPE enable/active state and verify state machine
status via firmware-reported enum icssg_ietfpe_verify_states.
icssg_config_ietfpe() configures IET FPE in firmware, triggers
verify state machine based on ethtool MAC Merge parameters.
Polls firmware verify status up to 3 times with verify_time_ms intervals
and driver handles timeout by logging error and returning.
In case of any failure during configuration for enable/disable,
IET FPE falls back to disabled state.
For MQPRIO qdisc support all queues are express by default later
gets override by user-provided preemptible_tcs bitmask via tc qdisc mask
Preempt mask configuration: Maps traffic classes to queue express/preemptible
state and applied only when FPE is active (Tx enabled)
Verify state machine re-triggers on link up/down events based on
fpe_enabled and fpe_active flags, and for memory protection, fpe_lock
serializes all FPE state mutations, preventing races between ethtool
config, qdisc setup, and link events
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
v6-v5:
- Balance fpe_lock mutex lifecycle on all error paths
- Remove deadcode inside icssg_iet_set_preempt_mask()
- Add fallback label inside icssg_config_ietfpe() to fix
stale state machine handling.
- Replace netdev_err with netdev_info and netdev_dbg wherever
applicable
- Remove EXPORT_SYMBOL_GPL for icssg_config_ietfpe()
- Remove redundant code inside icssg_qos_init()
- qdisc deletion path clears per-queue map as well.
- Protect all read/writes to p_mqprio with a mutex
All the above changes address the comments raised by sashiko
drivers/net/ethernet/ti/Makefile | 3 +-
drivers/net/ethernet/ti/icssg/icssg_common.c | 1 +
drivers/net/ethernet/ti/icssg/icssg_config.h | 9 -
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 6 +
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 2 +
drivers/net/ethernet/ti/icssg/icssg_qos.c | 269 +++++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_qos.h | 66 +++++
7 files changed, 346 insertions(+), 10 deletions(-)
create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index f4276c9a77620..d19bcd25c9d07 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -46,6 +46,7 @@ icssg-y := icssg/icssg_common.o \
icssg/icssg_config.o \
icssg/icssg_mii_cfg.o \
icssg/icssg_stats.o \
- icssg/icssg_ethtool.o
+ icssg/icssg_ethtool.o \
+ icssg/icssg_qos.o
obj-$(CONFIG_TI_ICSS_IEP) += icssg/icss_iep.o
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index a28a608f9bf4b..c3ee97e96cd50 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -1724,6 +1724,7 @@ void prueth_netdev_exit(struct prueth *prueth,
netif_napi_del(&emac->napi_rx);
+ mutex_destroy(&emac->qos.iet.fpe_lock);
pruss_release_mem_region(prueth->pruss, &emac->dram);
free_netdev(emac->ndev);
prueth->emac[mac] = NULL;
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 60d69744ffae2..1ac202f855ed4 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -323,13 +323,4 @@ struct prueth_fdb_slot {
u8 fid;
u8 fid_c2;
} __packed;
-
-enum icssg_ietfpe_verify_states {
- ICSSG_IETFPE_STATE_UNKNOWN = 0,
- ICSSG_IETFPE_STATE_INITIAL,
- ICSSG_IETFPE_STATE_VERIFYING,
- ICSSG_IETFPE_STATE_SUCCEEDED,
- ICSSG_IETFPE_STATE_FAILED,
- ICSSG_IETFPE_STATE_DISABLED
-};
#endif /* __NET_TI_ICSSG_CONFIG_H */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 591be5c8056b4..39f379df923bf 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -392,6 +392,8 @@ static void emac_adjust_link(struct net_device *ndev)
} else {
icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
}
+
+ icssg_qos_link_state_update(ndev);
}
if (emac->link) {
@@ -1652,6 +1654,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_hwtstamp_get = icssg_ndo_get_ts_config,
.ndo_hwtstamp_set = icssg_ndo_set_ts_config,
.ndo_xsk_wakeup = prueth_xsk_wakeup,
+ .ndo_setup_tc = icssg_qos_ndo_setup_tc,
};
static int prueth_netdev_init(struct prueth *prueth,
@@ -1686,6 +1689,8 @@ static int prueth_netdev_init(struct prueth *prueth,
INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
+ icssg_qos_init(ndev);
+
ret = pruss_request_mem_region(prueth->pruss,
port == PRUETH_PORT_MII0 ?
PRUSS_MEM_DRAM0 : PRUSS_MEM_DRAM1,
@@ -1793,6 +1798,7 @@ static int prueth_netdev_init(struct prueth *prueth,
free:
pruss_release_mem_region(prueth->pruss, &emac->dram);
free_ndev:
+ mutex_destroy(&emac->qos.iet.fpe_lock);
emac->ndev = NULL;
prueth->emac[mac] = NULL;
free_netdev(ndev);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index df93d15c5b786..85f7017d2c8e7 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -44,6 +44,7 @@
#include "icssg_config.h"
#include "icss_iep.h"
#include "icssg_switch_map.h"
+#include "icssg_qos.h"
#define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN)
#define PRUETH_MIN_PKT_SIZE (VLAN_ETH_ZLEN)
@@ -254,6 +255,7 @@ struct prueth_emac {
struct bpf_prog *xdp_prog;
struct xdp_attachment_info xdpi;
int xsk_qid;
+ struct prueth_qos qos;
};
/* The buf includes headroom compatible with both skb and xdpf */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
new file mode 100644
index 0000000000000..2781abf39e9bb
--- /dev/null
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Texas Instruments ICSSG PRUETH QoS submodule
+ * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#include "icssg_prueth.h"
+#include "icssg_switch_map.h"
+
+static void icssg_iet_set_preempt_mask(struct prueth_emac *emac)
+{
+ void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
+ struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
+ struct tc_mqprio_qopt *qopt = &p_mqprio->mqprio.qopt;
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int prempt_mask = 0, i;
+ u8 tc, num_tc;
+
+ if (!iet->preemptible_tcs)
+ goto reset_hw;
+
+ if (iet->fpe_active) {
+ /* Configure the queues based on the preemptible tc map set by the user */
+ num_tc = p_mqprio->mqprio.qopt.num_tc;
+ for (tc = 0; tc < num_tc; tc++) {
+ /* check if the tc is preemptive or not */
+ if (iet->preemptible_tcs & BIT(tc)) {
+ for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
+ /* Set all the queues in this tc as preemptive queues */
+ writeb(BIT(4), config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
+ }
+ } else {
+ /* Set all the queues in this tc as express queues */
+ for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
+ writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
+ prempt_mask |= BIT(i);
+ }
+ }
+ netdev_set_tc_queue(emac->ndev, tc, qopt->count[tc], qopt->offset[tc]);
+ }
+ writeb(prempt_mask, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
+ return;
+ }
+
+reset_hw:
+ /* Reset to default: all queues as express */
+ for (i = 0; i < ICSSG_MAX_TC_QUEUES; i++)
+ writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
+ writeb(ICSSG_EXPRESS_Q_MASK_ALL, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
+}
+
+static int icssg_iet_verify_wait(struct prueth_emac *emac)
+{
+ void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int try = 3;
+
+ do {
+ msleep(iet->verify_time_ms);
+ iet->verify_status = readb(config + PRE_EMPTION_VERIFY_STATUS);
+ if (iet->verify_status == ICSSG_IETFPE_STATE_SUCCEEDED)
+ return 0;
+ } while (--try > 0);
+
+ netdev_err(emac->ndev, "MAC Verify timeout\n");
+ return -ETIMEDOUT;
+}
+
+/* Direct synchronous configuration of IET FPE.
+ * Caller must hold iet->fpe_lock.
+ */
+int icssg_config_ietfpe(struct net_device *ndev, bool enable)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int ret;
+ u8 val;
+
+ lockdep_assert_held(&iet->fpe_lock);
+
+ if (!netif_running(ndev)) {
+ netdev_dbg(ndev, "cannot change IET/FPE state when interface is down\n");
+ return 0;
+ }
+
+ /* Update FPE Tx enable bit (PRE_EMPTION_ENABLE_TX) if
+ * fpe_enabled is set to enable MM in Tx direction
+ */
+ writeb(enable ? 1 : 0, config + PRE_EMPTION_ENABLE_TX);
+
+ /* If FPE is to be enabled, first configure MAC Verify state
+ * machine in firmware as firmware kicks the Verify process
+ * as soon as ICSSG_EMAC_PORT_PREMPT_TX_ENABLE command is
+ * received.
+ */
+ if (enable && iet->mac_verify_configure) {
+ writeb(1, config + PRE_EMPTION_ENABLE_VERIFY);
+ writew(iet->tx_min_frag_size + ETH_FCS_LEN,
+ config + PRE_EMPTION_ADD_FRAG_SIZE_LOCAL);
+ writel(iet->verify_time_ms, config + PRE_EMPTION_VERIFY_TIME);
+ } else {
+ writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
+ iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
+ }
+
+ /* Send command to enable FPE Tx side. Rx is always enabled */
+ ret = icssg_set_port_state(emac,
+ enable ? ICSSG_EMAC_PORT_PREMPT_TX_ENABLE :
+ ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
+ if (ret) {
+ netdev_err(ndev, "TX preempt %s command failed\n",
+ str_enable_disable(enable));
+ goto fallback;
+ }
+
+ if (enable && iet->mac_verify_configure) {
+ ret = icssg_iet_verify_wait(emac);
+ if (ret) {
+ netdev_err(ndev, "MAC Verification failed with timeout\n");
+ goto disable_tx;
+ }
+ } else if (enable) {
+ /* Give firmware some time to update PRE_EMPTION_ACTIVE_TX state */
+ usleep_range(100, 200);
+ }
+
+ if (enable) {
+ val = readb(config + PRE_EMPTION_ACTIVE_TX);
+ if (val != 1) {
+ netdev_err(ndev,
+ "Firmware fails to activate IET/FPE\n");
+ ret = -EIO;
+ goto disable_tx;
+ }
+ iet->fpe_active = true;
+ } else {
+ iet->fpe_active = false;
+ }
+
+ icssg_iet_set_preempt_mask(emac);
+ netdev_info(ndev, "IET FPE %s successfully\n",
+ str_enable_disable(iet->fpe_active));
+ return ret;
+
+disable_tx:
+ icssg_set_port_state(emac, ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
+fallback:
+ writeb(0, config + PRE_EMPTION_ENABLE_TX);
+ writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
+ iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
+ iet->fpe_active = false;
+ return ret;
+}
+
+void icssg_qos_init(struct net_device *ndev)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+
+ mutex_init(&iet->fpe_lock);
+ /* Set default values to prevent garbage values during .get_mm() */
+ mutex_lock(&iet->fpe_lock);
+ iet->verify_time_ms = ICSSG_IET_MAX_VERIFY_TIME;
+ iet->tx_min_frag_size = ETH_ZLEN;
+ mutex_unlock(&iet->fpe_lock);
+}
+EXPORT_SYMBOL_GPL(icssg_qos_init);
+
+static int icssg_iet_change_preemptible_tcs(struct prueth_emac *emac)
+{
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int ret;
+
+ mutex_lock(&iet->fpe_lock);
+ ret = icssg_config_ietfpe(emac->ndev, iet->fpe_enabled);
+ mutex_unlock(&iet->fpe_lock);
+
+ return ret;
+}
+
+static int emac_tc_query_caps(struct net_device *ndev, void *type_data)
+{
+ struct tc_query_caps_base *base = type_data;
+
+ switch (base->type) {
+ case TC_SETUP_QDISC_MQPRIO: {
+ struct tc_mqprio_caps *caps = base->caps;
+
+ caps->validate_queue_counts = true;
+ return 0;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int emac_tc_setup_mqprio(struct net_device *ndev, void *type_data)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
+ struct tc_mqprio_qopt_offload *mqprio = type_data;
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int ret;
+
+ /* Validate parameters */
+ if (mqprio->qopt.num_tc > ICSSG_MAX_TC_QUEUES) {
+ netdev_err(ndev, "Number of traffic classes (%u) exceeds hardware limit\n",
+ mqprio->qopt.num_tc);
+ return -EINVAL;
+ }
+
+ if (mqprio->flags & TC_MQPRIO_F_SHAPER) {
+ netdev_err(ndev, "traffic shaping is not supported\n");
+ return -EINVAL;
+ }
+
+ if (mqprio->flags & (TC_MQPRIO_F_MIN_RATE | TC_MQPRIO_F_MAX_RATE)) {
+ netdev_err(ndev, "per-queue rate limiting is not supported\n");
+ return -EINVAL;
+ }
+
+ if (!mqprio->qopt.num_tc) {
+ netdev_reset_tc(ndev);
+ } else {
+ netdev_set_num_tc(ndev, mqprio->qopt.num_tc);
+ }
+
+ mutex_lock(&iet->fpe_lock);
+ if (!mqprio->qopt.num_tc) {
+ iet->preemptible_tcs = 0;
+ } else {
+ memcpy(&p_mqprio->mqprio, mqprio, sizeof(*mqprio));
+ iet->preemptible_tcs = mqprio->preemptible_tcs;
+ }
+ mutex_unlock(&iet->fpe_lock);
+
+ netdev_dbg(ndev, "dev->num_tc %u dev->real_num_tx_queues %u\n",
+ ndev->num_tc, ndev->real_num_tx_queues);
+
+ ret = icssg_iet_change_preemptible_tcs(emac);
+ return ret;
+}
+
+int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
+ void *type_data)
+{
+ switch (type) {
+ case TC_QUERY_CAPS:
+ return emac_tc_query_caps(ndev, type_data);
+ case TC_SETUP_QDISC_MQPRIO:
+ return emac_tc_setup_mqprio(ndev, type_data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+EXPORT_SYMBOL_GPL(icssg_qos_ndo_setup_tc);
+
+void icssg_qos_link_state_update(struct net_device *ndev)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int ret;
+
+ ret = icssg_iet_change_preemptible_tcs(emac);
+ if (ret)
+ netdev_dbg(ndev, "IET FPE %s failed\n",
+ str_enable_disable(iet->fpe_active));
+}
+EXPORT_SYMBOL_GPL(icssg_qos_link_state_update);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
new file mode 100644
index 0000000000000..9355e96bbcda8
--- /dev/null
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#ifndef __NET_TI_ICSSG_QOS_H
+#define __NET_TI_ICSSG_QOS_H
+
+#include <linux/atomic.h>
+#include <linux/netdevice.h>
+#include <net/pkt_sched.h>
+
+#define ICSSG_MAX_TC_QUEUES 8
+#define ICSSG_EXPRESS_Q_MASK_ALL 0xFF
+#define ICSSG_IET_MAX_VERIFY_TIME 128
+#define ICSSG_IET_MIN_VERIFY_TIME 1
+
+/**
+ * enum icssg_ietfpe_verify_states - status of MAC Merge Verify returned by firmware
+ * @ICSSG_IETFPE_STATE_UNKNOWN:
+ * verification status is unknown
+ * @ICSSG_IETFPE_STATE_INITIAL:
+ * Firmware returns this if verify state diagram is idle
+ * @ICSSG_IETFPE_STATE_VERIFYING:
+ * Firmware returns this if verification is ongoing
+ * @ICSSG_IETFPE_STATE_SUCCEEDED:
+ * Firmware returns this if verify state diagram completes verification
+ * @ICSSG_IETFPE_STATE_FAILED:
+ * Firmware returns this if verify state diagram fails during verification
+ * @ICSSG_IETFPE_STATE_DISABLED:
+ * verification is disabled by the driver
+ */
+enum icssg_ietfpe_verify_states {
+ ICSSG_IETFPE_STATE_UNKNOWN = 0,
+ ICSSG_IETFPE_STATE_INITIAL,
+ ICSSG_IETFPE_STATE_VERIFYING,
+ ICSSG_IETFPE_STATE_SUCCEEDED,
+ ICSSG_IETFPE_STATE_FAILED,
+ ICSSG_IETFPE_STATE_DISABLED
+};
+
+struct prueth_qos_mqprio {
+ struct tc_mqprio_qopt_offload mqprio;
+};
+
+struct prueth_qos_iet {
+ bool fpe_enabled;
+ bool mac_verify_configure;
+ u32 tx_min_frag_size;
+ u32 verify_time_ms;
+ bool fpe_active;
+ enum icssg_ietfpe_verify_states verify_status;
+ struct mutex fpe_lock;
+ u8 preemptible_tcs;
+};
+
+struct prueth_qos {
+ struct prueth_qos_iet iet;
+ struct prueth_qos_mqprio mqprio;
+};
+
+void icssg_qos_init(struct net_device *ndev);
+void icssg_qos_link_state_update(struct net_device *ndev);
+int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
+ void *type_data);
+int icssg_config_ietfpe(struct net_device *ndev, bool enable);
+#endif /* __NET_TI_ICSSG_QOS_H */
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
From: Meghana Malladi @ 2026-05-25 18:27 UTC (permalink / raw)
To: liuhangbin, h-mittal1, haokexin, vadim.fedorenko, devnexen, horms,
jacob.e.keller, m-malladi, arnd, afd, basharath, parvathi,
vladimir.oltean, rogerq, danishanwar, pabeni, kuba, edumazet,
davem, andrew+netdev
Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra
In-Reply-To: <20260525182700.3135858-1-m-malladi@ti.com>
From: MD Danish Anwar <danishanwar@ti.com>
Add driver support for viewing and changing the MAC Merge sublayer
parameters via ethtool ops: .set_mm(), .get_mm() and .get_mm_stats().
The minimum size of non-final mPacket fragments supported by the firmware
without leading errors is 64 Bytes (including FCS). Verify time bounded to
1-128 ms per 802.3-2018 clause 30.14.1.6. Add a check to ensure
user passed tx_min_frag_size argument via ethtool, honors this.
Add pa stats registers to check statistics for preemption, which can be
dumped using ethtool ops.
Fix emac_get_stat_by_name() to return u64 instead of int and return 0 on
error instead of -EINVAL. This prevents invalid stat lookups from corrupting
output stats with signed error codes cast to u64. Error conditions are still
logged via netdev_err().
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
v6-v5:
- Initialize tx_min_frag_size and verify_time with valid values
to avoid returning corrupted values during get_mm()
- Fix rx_min_frag_size to ETH_ZLEN excluding ETH_FCS_LEN
- Fix return codes for emac_set_mm()
All the above changes address the comments raised by sashiko
drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 106 ++++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 7 +-
drivers/net/ethernet/ti/icssg/icssg_qos.h | 46 ++++++++
drivers/net/ethernet/ti/icssg/icssg_stats.c | 4 +-
drivers/net/ethernet/ti/icssg/icssg_stats.h | 7 +-
.../net/ethernet/ti/icssg/icssg_switch_map.h | 5 +
6 files changed, 168 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
index b715af21d23ac..ee940051644d6 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
@@ -294,6 +294,109 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
return 0;
}
+static int emac_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ enum icssg_ietfpe_verify_states verify_status;
+
+ if (emac->is_sr1)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&iet->fpe_lock);
+ state->tx_enabled = iet->fpe_enabled;
+ state->tx_min_frag_size = iet->tx_min_frag_size;
+ state->tx_active = iet->fpe_active;
+ state->verify_enabled = iet->mac_verify_configure;
+ state->verify_time = iet->verify_time_ms;
+ verify_status = iet->verify_status;
+ mutex_unlock(&iet->fpe_lock);
+
+ state->rx_min_frag_size = ETH_ZLEN;
+ state->pmac_enabled = true;
+
+ switch (verify_status) {
+ case ICSSG_IETFPE_STATE_DISABLED:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+ break;
+ case ICSSG_IETFPE_STATE_INITIAL:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_INITIAL;
+ break;
+ case ICSSG_IETFPE_STATE_VERIFYING:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_VERIFYING;
+ break;
+ case ICSSG_IETFPE_STATE_SUCCEEDED:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
+ break;
+ case ICSSG_IETFPE_STATE_FAILED:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
+ break;
+ default:
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
+ break;
+ }
+
+ /* 802.3-2018 clause 30.14.1.6, says that the aMACMergeVerifyTime
+ * variable has a range between 1 and 128 ms inclusive. Limit to that.
+ */
+ state->max_verify_time = ETHTOOL_MM_MAX_VERIFY_TIME_MS;
+
+ return 0;
+}
+
+static int emac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+ struct prueth_qos_iet *iet = &emac->qos.iet;
+ int err;
+
+ if (emac->is_sr1)
+ return -EOPNOTSUPP;
+
+ if (!cfg->pmac_enabled) {
+ NL_SET_ERR_MSG_MOD(extack, "preemptible MAC is always enabled");
+ return -EOPNOTSUPP;
+ }
+
+ err = icssg_qos_validate_tx_min_frag_size(cfg->tx_min_frag_size, extack);
+ if (err)
+ return err;
+
+ err = icssg_qos_validate_verify_time(cfg->verify_time, extack);
+ if (err)
+ return err;
+
+ mutex_lock(&iet->fpe_lock);
+ iet->verify_time_ms = cfg->verify_time;
+ iet->tx_min_frag_size = cfg->tx_min_frag_size;
+ iet->fpe_enabled = cfg->tx_enabled;
+ iet->mac_verify_configure = cfg->verify_enabled;
+ err = icssg_config_ietfpe(ndev, cfg->tx_enabled);
+ mutex_unlock(&iet->fpe_lock);
+
+ return err;
+}
+
+static void emac_get_mm_stats(struct net_device *ndev,
+ struct ethtool_mm_stats *s)
+{
+ struct prueth_emac *emac = netdev_priv(ndev);
+
+ if (emac->is_sr1)
+ return;
+
+ if (!emac->prueth->pa_stats)
+ return;
+
+ /* MACMergeHoldCount stats is not tracked by the firmware */
+ s->MACMergeFrameAssOkCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_OK");
+ s->MACMergeFrameAssErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_ERR");
+ s->MACMergeFragCountRx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_RX");
+ s->MACMergeFragCountTx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_TX");
+ s->MACMergeFrameSmdErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_BAD_FRAG");
+}
+
const struct ethtool_ops icssg_ethtool_ops = {
.get_drvinfo = emac_get_drvinfo,
.get_msglevel = emac_get_msglevel,
@@ -317,5 +420,8 @@ const struct ethtool_ops icssg_ethtool_ops = {
.set_eee = emac_set_eee,
.nway_reset = emac_nway_reset,
.get_rmon_stats = emac_get_rmon_stats,
+ .get_mm = emac_get_mm,
+ .set_mm = emac_set_mm,
+ .get_mm_stats = emac_get_mm_stats,
};
EXPORT_SYMBOL_GPL(icssg_ethtool_ops);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 85f7017d2c8e7..61320c252bec2 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -45,6 +45,7 @@
#include "icss_iep.h"
#include "icssg_switch_map.h"
#include "icssg_qos.h"
+#include "icssg_stats.h"
#define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN)
#define PRUETH_MIN_PKT_SIZE (VLAN_ETH_ZLEN)
@@ -58,8 +59,8 @@
#define ICSSG_MAX_RFLOWS 8 /* per slice */
-#define ICSSG_NUM_PA_STATS 32
-#define ICSSG_NUM_MIIG_STATS 60
+#define ICSSG_NUM_PA_STATS ARRAY_SIZE(icssg_all_pa_stats)
+#define ICSSG_NUM_MIIG_STATS ARRAY_SIZE(icssg_all_miig_stats)
/* Number of ICSSG related stats */
#define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
#define ICSSG_NUM_STANDARD_STATS 31
@@ -460,7 +461,7 @@ int emac_fdb_flow_id_updated(struct prueth_emac *emac);
void icssg_stats_work_handler(struct work_struct *work);
void emac_update_hardware_stats(struct prueth_emac *emac);
-int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name);
+u64 emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name);
/* Common functions */
void prueth_cleanup_rx_chns(struct prueth_emac *emac,
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
index 9355e96bbcda8..87ca031afcaa4 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_qos.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
@@ -13,6 +13,7 @@
#define ICSSG_EXPRESS_Q_MASK_ALL 0xFF
#define ICSSG_IET_MAX_VERIFY_TIME 128
#define ICSSG_IET_MIN_VERIFY_TIME 1
+#define ICSSG_IET_MAX_TX_MIN_FRAG_SIZE 252
/**
* enum icssg_ietfpe_verify_states - status of MAC Merge Verify returned by firmware
@@ -63,4 +64,49 @@ void icssg_qos_link_state_update(struct net_device *ndev);
int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
void *type_data);
int icssg_config_ietfpe(struct net_device *ndev, bool enable);
+static inline int icssg_qos_validate_tx_min_frag_size(u32 min_frag_size,
+ struct netlink_ext_ack *extack)
+{
+ /* Firmware takes min_frag_size including FCS length.
+ * The firmware requires the fragment size (including FCS) to be
+ * a multiple of 64 bytes. Since 64 bytes = ETH_ZLEN + ETH_FCS_LEN,
+ * valid user-facing values are: 60, 124, 188, 252.
+ */
+
+ if (min_frag_size < ETH_ZLEN) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "tx_min_frag_size must be at least 60 bytes");
+ return -EINVAL;
+ }
+
+ if (min_frag_size > ICSSG_IET_MAX_TX_MIN_FRAG_SIZE) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "tx_min_frag_size must not exceed 252 bytes");
+ return -EINVAL;
+ }
+
+ if ((min_frag_size + ETH_FCS_LEN) % (ETH_ZLEN + ETH_FCS_LEN)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "tx_min_frag_size must be a multiple of 64 bytes minus 4");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static inline int icssg_qos_validate_verify_time(u32 verify_time_ms,
+ struct netlink_ext_ack *extack)
+{
+ /* 802.3-2018 clause 30.14.1.6: aMACMergeVerifyTime must be
+ * between 1 and 128 ms inclusive
+ */
+ if (verify_time_ms < ICSSG_IET_MIN_VERIFY_TIME ||
+ verify_time_ms > ICSSG_IET_MAX_VERIFY_TIME) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "verify_time must be between 1 and 128 ms");
+ return -EINVAL;
+ }
+
+ return 0;
+}
#endif /* __NET_TI_ICSSG_QOS_H */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.c b/drivers/net/ethernet/ti/icssg/icssg_stats.c
index 7159baa0155cf..cfdb6f5dc5da1 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.c
@@ -74,7 +74,7 @@ void icssg_stats_work_handler(struct work_struct *work)
}
EXPORT_SYMBOL_GPL(icssg_stats_work_handler);
-int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name)
+u64 emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name)
{
int i;
@@ -91,5 +91,5 @@ int emac_get_stat_by_name(struct prueth_emac *emac, char *stat_name)
}
netdev_err(emac->ndev, "Invalid stats %s\n", stat_name);
- return -EINVAL;
+ return 0;
}
diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
index 5ec0b38e0c67d..8073deac35c3e 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
@@ -8,8 +8,6 @@
#ifndef __NET_TI_ICSSG_STATS_H
#define __NET_TI_ICSSG_STATS_H
-#include "icssg_prueth.h"
-
#define STATS_TIME_LIMIT_1G_MS 25000 /* 25 seconds @ 1G */
struct miig_stats_regs {
@@ -189,6 +187,11 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
ICSSG_PA_STATS(FW_INF_DROP_PRIOTAGGED),
ICSSG_PA_STATS(FW_INF_DROP_NOTAG),
ICSSG_PA_STATS(FW_INF_DROP_NOTMEMBER),
+ ICSSG_PA_STATS(FW_PREEMPT_BAD_FRAG),
+ ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_ERR),
+ ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_TX),
+ ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_OK),
+ ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_RX),
ICSSG_PA_STATS(FW_RX_EOF_SHORT_FRMERR),
ICSSG_PA_STATS(FW_RX_B0_DROP_EARLY_EOF),
ICSSG_PA_STATS(FW_TX_JUMBO_FRM_CUTOFF),
diff --git a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
index 7e053b8af3ece..855fd4ed0b3f6 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
@@ -256,6 +256,11 @@
#define FW_INF_DROP_PRIOTAGGED 0x0148
#define FW_INF_DROP_NOTAG 0x0150
#define FW_INF_DROP_NOTMEMBER 0x0158
+#define FW_PREEMPT_BAD_FRAG 0x0160
+#define FW_PREEMPT_ASSEMBLY_ERR 0x0168
+#define FW_PREEMPT_FRAG_CNT_TX 0x0170
+#define FW_PREEMPT_ASSEMBLY_OK 0x0178
+#define FW_PREEMPT_FRAG_CNT_RX 0x0180
#define FW_RX_EOF_SHORT_FRMERR 0x0188
#define FW_RX_B0_DROP_EARLY_EOF 0x0190
#define FW_TX_JUMBO_FRM_CUTOFF 0x0198
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v6 0/2] Add Frame Preemption MAC Merge support for ICSSG
From: Meghana Malladi @ 2026-05-25 18:26 UTC (permalink / raw)
To: liuhangbin, h-mittal1, haokexin, vadim.fedorenko, devnexen, horms,
jacob.e.keller, m-malladi, arnd, afd, basharath, parvathi,
vladimir.oltean, rogerq, danishanwar, pabeni, kuba, edumazet,
davem, andrew+netdev
Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra
This patch series adds QoS support to the ICSSG PRUETH driver.
The first patch implements mqprio qdisc handling and TC offload hooks
so userspace can request TC mappings and queue counts.
It also integrates a driver-side mechanism to program the firmware
with the IET/FPE preemption mask and to kick the firmware verify state
machine when frame preemption is enabled. The second patch adds ethtool
perations for the MAC Merge (Frame Preemption) sublayer, exposing .get_mm,
.set_mm and .get_mm_stats so admins can view and change MAC Merge
parameters and retrieve preemption statistics.
v5: https://lore.kernel.org/all/20260430111723.497113-1-m-malladi@ti.com/
MD Danish Anwar (2):
net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
drivers/net/ethernet/ti/Makefile | 3 +-
drivers/net/ethernet/ti/icssg/icssg_common.c | 1 +
drivers/net/ethernet/ti/icssg/icssg_config.h | 9 -
drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 106 +++++++
drivers/net/ethernet/ti/icssg/icssg_prueth.c | 6 +
drivers/net/ethernet/ti/icssg/icssg_prueth.h | 9 +-
drivers/net/ethernet/ti/icssg/icssg_qos.c | 269 ++++++++++++++++++
drivers/net/ethernet/ti/icssg/icssg_qos.h | 112 ++++++++
drivers/net/ethernet/ti/icssg/icssg_stats.c | 4 +-
drivers/net/ethernet/ti/icssg/icssg_stats.h | 7 +-
.../net/ethernet/ti/icssg/icssg_switch_map.h | 5 +
11 files changed, 514 insertions(+), 17 deletions(-)
create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h
base-commit: 830d8771ae3c7bc90a62dde76a6556e612529fbc
--
2.43.0
^ permalink raw reply
* Re: [PATCH] ARM: mm: fix kexec and hibernation with CONFIG_CPU_TTBR0_PAN
From: Florian Fainelli @ 2026-05-25 18:15 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel, bcm-kernel-feedback-list, Catalin Marinas,
Linus Walleij, Russell King, Russell King (Oracle), Kees Cook,
open list
In-Reply-To: <CAD++jL=QxG=e62aOBXCzuud-nMEKtiUd-6_iUAudB0DJRWJwzw@mail.gmail.com>
On 5/25/2026 6:35 AM, Linus Walleij wrote:
> Hi Florian,
>
> thanks for digging in and finding this!
>
> On Sat, May 23, 2026 at 2:08 AM Florian Fainelli
> <florian.fainelli@broadcom.com> wrote:
>
>> Commit 7af5b901e847 ("ARM: 9358/2: Implement PAN for LPAE by TTBR0
>> page table walks disablement") implemented PAN for LPAE kernels by
>> setting TTBCR.EPD0 on every kernel entry, disabling TTBR0 page-table
>> walks while running in kernel mode. The commit correctly updated
>> cpu_suspend() in arch/arm/kernel/suspend.c, but missed two other code
>> paths that switch the CPU to the identity mapping before jumping to
>> low-PA (TTBR0-range) physical addresses:
>>
>> 1. setup_mm_for_reboot() in arch/arm/mm/idmap.c, used by the kexec
>> reboot path. With TTBCR.EPD0 still set, the subsequent branch to
>> the identity-mapped cpu_v7_reset causes a PrefetchAbort because the
>> TTBR0 page-table walk needed to resolve the identity-mapped address
>> is disabled. This manifests as a hard hang or "bad PC value" panic
>> on LPAE kernels booted on CPUs that strictly enforce EPD0 for
>> instruction fetch (e.g. Cortex-A53 in AArch32 mode) while the same
>> image may accidentally work on Cortex-A15 due to microarchitectural
>> differences in EPD0 enforcement.
>>
>> 2. arch_restore_image() in arch/arm/kernel/hibernate.c, which calls
>> cpu_switch_mm(idmap_pgd, &init_mm) directly without going through
>> setup_mm_for_reboot(), leaving TTBCR.EPD0 set while the identity
>> mapping is active.
>>
>> Fix both sites by calling uaccess_save_and_enable() before switching
>> to the identity mapping, mirroring what the original commit did for
>> cpu_suspend().
>>
>> Fixes: 7af5b901e847 ("ARM: 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Assisted-by: Cursor:claude-sonnet-4.6
>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
>
> Please put this patch into Russell's patch tracker.
Thank you, now done:
https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9476/1
--
Florian
^ permalink raw reply
* Re: [PATCH RFC bpf-next 3/8] bpf: add BPF_JIT_KASAN for KASAN instrumentation of JITed programs
From: Emil Tsalapatis @ 2026-05-25 18:01 UTC (permalink / raw)
To: Alexis Lothoré, Emil Tsalapatis, Alexei Starovoitov
Cc: Andrey Konovalov, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Jiri Olsa,
John Fastabend, David S. Miller, David Ahern, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, X86 ML, H. Peter Anvin,
Shuah Khan, Maxime Coquelin, Alexandre Torgue, Andrey Ryabinin,
Alexander Potapenko, Dmitry Vyukov, Vincenzo Frascino,
Andrew Morton, ebpf, Bastien Curutchet, Thomas Petazzoni,
Xu Kuohai, bpf, LKML, Network Development,
open list:KERNEL SELFTEST FRAMEWORK, linux-stm32,
linux-arm-kernel, kasan-dev, linux-mm
In-Reply-To: <DIRMYUXFYOVY.1LQV9E3OHGGBK@bootlin.com>
On Mon May 25, 2026 at 5:05 AM EDT, Alexis Lothoré wrote:
> On Fri May 22, 2026 at 7:13 PM CEST, Emil Tsalapatis wrote:
>> On Fri May 22, 2026 at 10:14 AM EDT, Alexis Lothoré wrote:
>>> On Tue Apr 14, 2026 at 4:38 PM CEST, Alexei Starovoitov wrote:
>>>> On Tue, Apr 14, 2026 at 6:24 AM Alexis Lothoré
>>>> <alexis.lothore@bootlin.com> wrote:
>>>>>
>>>>> On Tue Apr 14, 2026 at 12:20 AM CEST, Andrey Konovalov wrote:
>>>>> > On Mon, Apr 13, 2026 at 8:29 PM Alexis Lothoré (eBPF Foundation)
>>>>> > <alexis.lothore@bootlin.com> wrote:
>>>
>>> [...]
>>>
>>>>> >> +config BPF_JIT_KASAN
>>>>> >> + bool
>>>>> >> + depends on HAVE_EBPF_JIT_KASAN
>>>>> >> + default y if BPF_JIT && KASAN_GENERIC
>>>>> >
>>>>> > Should this be "depends on KASAN && KASAN_GENERIC"?
>>>>>
>>>>> Meaning, making it an explicit user-selectable option ?
>>>>>
>>>>> If so, the current design choice is voluntary and based on the feedback
>>>>> received on the original RFC, where I have been suggested to
>>>>> automatically enable the KASAN instrumentation in BPF programs if KASAN
>>>>> support is enabled in the kernel ([1]). But if a user-selectable toggle
>>>>> is eventually a better solution, I'm fine with changing it.
>>>>
>>>> Let's not add more config knobs.
>>>> Even this patch looks redundant.
>>>> Inside JIT do instrumentation when KASAN_GENERIC is set.
>>>
>>> (with quite some delay) I think it would be better to keep this new
>>> BPF_JIT_KASAN, because aside from the possibility to use it in
>>> bpf_jit_comp.c, it allows to update tests affected by KASAN
>>> instrumentation in a nicer way. For example, the test_loader subtests
>>> that monitor JITted instructions are confused by KASAN. I can either
>>> skip them or make them smarter when KASAN is enabled for BPF, but in
>>> both cases, it would be nicer to just adapt the behavior based on a
>>> generic CONFIG_BPF_JIT_KASAN, rather than sprinkling some "if
>>> jit_enabled AND CONFIG_KASAN_GENERIC AND ARCH_X86" in selftests. That
>>> still does not make it a config knob, that just creates an internal
>>> Kconfig option that is automatically turned on when KASAN and JIT are
>>> enabled at build time.
>>
>> Having a togglable config knob gives us the option to set up KASAN for
>> the kernel but not for BPF, and I don't see why we'd want that. Imo we are
>> already paying the cost of KASAN for the rest of the kernel, there is no
>> incentive to not run it for the BPF JIT. Having to eat the complexity cost
>> in the selftests seems reasonable if the alternative means a cleaner
>> interface for the user (preventing them from choosing an unreasonable
>> combination of options).
>
> Again, this does not expose a togglable knob, this is a purely internal
> kconfig, automatically enabled if CONFIG_KASAN_GENERIC is set and if the
> architecture-specific Kconfig defines HAVE_EBPF_JIT_KASAN (since we want
> it for x86 only), and there would be no way to enable KASAN for kernel
> only and not for BPF, or the other way around. What I am proposing is
> just an internal, architecture-agnostice kconfig to avoid conditioning
> some selftests to any architecture.
Makes sense, in this case this seems reasonable.
>
> Alexis
^ permalink raw reply
* Re: [PATCH v6 2/4] pwm: sun8i: Add H616 PWM support
From: Uwe Kleine-König @ 2026-05-25 17:58 UTC (permalink / raw)
To: Richard Genoud
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Philipp Zabel, Paul Kocialkowski,
Thomas Petazzoni, John Stultz, Joao Schim, bigunclemax, linux-pwm,
devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <20260416134037.3160537-3-richard.genoud@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 35888 bytes --]
Hello Richard,
I cannot say much about the clk part of this driver and thus would like
to get some input from the clk people. Can you please add them to the
list of recipents for the next revision?
On Thu, Apr 16, 2026 at 03:40:35PM +0200, Richard Genoud wrote:
> Add driver for Allwinner H616 PWM controller, supporting up to 6
> channels.
> Those channels output can be either a PWM signal output or a clock
> output, thanks to the bypass.
>
> The channels are paired (0/1, 2/3 and 4/5) and each pair has a
> prescaler/mux/gate.
> Moreover, each channel has its own prescaler and bypass.
>
> The clock provider part of this driver is needed not only because the
> H616 PWM controller provides also clocks when bypass is enabled, but
> really because pwm-clock isn't fit to handle all cases here.
> pwm-clock would work if the 100MHz clock is requested, but if a lower
> clock is requested (like 24MHz), it will request a 42ns period to the
> PWM driver which will happily serve, with the 100MHz clock as input a
> 25MHz frequency and a duty cycle adjustable in the range [0-4]/4,
> because that is a sane thing to do for a PWM.
> The information missing is that a real clock is resquested, not a PWM.
>
> Tested-by: John Stultz <jstultz@google.com>
> Tested-by: Joao Schim <joao@schimsalabim.eu>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> ---
> drivers/pwm/Kconfig | 12 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-sun8i.c | 938 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 951 insertions(+)
> create mode 100644 drivers/pwm/pwm-sun8i.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 6f3147518376..c4fd682860d6 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -736,6 +736,18 @@ config PWM_SUN4I
> To compile this driver as a module, choose M here: the module
> will be called pwm-sun4i.
>
> +config PWM_SUN8I
> + tristate "Allwinner sun8i/sun50i PWM support"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + depends on HAS_IOMEM && COMMON_CLK
> + help
> + Generic PWM framework driver for Allwinner H616 SoCs.
> + It supports generic PWM, but can also provides a plain clock.
> + The AC300 PHY integrated in H616 SoC needs such a clock.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-sun8i.
> +
> config PWM_SUNPLUS
> tristate "Sunplus PWM support"
> depends on ARCH_SUNPLUS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 0dc0d2b69025..ba2e0ec7fc17 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o
> obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
> obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
> obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
> +obj-$(CONFIG_PWM_SUN8I) += pwm-sun8i.o
> obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o
> obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
> obj-$(CONFIG_PWM_TH1520) += pwm_th1520.o
> diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
> new file mode 100644
> index 000000000000..8f1023e3a2e5
> --- /dev/null
> +++ b/drivers/pwm/pwm-sun8i.c
> @@ -0,0 +1,938 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Allwinner sun8i Pulse Width Modulation Controller
> + *
> + * (C) Copyright 2025 Richard Genoud, Bootlin <richard.genoud@bootlin.com>
> + *
> + * Based on drivers/pwm/pwm-sun4i.c with Copyright:
> + *
> + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@bootlin.com>
> + *
> + * Limitations:
> + * - As the channels are paired (0/1, 2/3, 4/5), they share the same clock
> + * source and prescaler(div_m), but they also have their own prescaler(div_k)
> + * and bypass.
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/limits.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +#include <linux/time.h>
> +
> +/* PWM IRQ Enable Register */
> +#define SUN8I_PWM_PIER 0x0
> +
> +/* PWM IRQ Status Register */
> +#define SUN8I_PWM_PISR 0x4
> +
> +/* PWM Capture IRQ Enable Register */
> +#define SUN8I_PWM_CIER 0x10
> +
> +/* PWM Capture IRQ Status Register */
> +#define SUN8I_PWM_CISR 0x14
> +
> +/* PWMCC Pairs Clock Configuration Registers */
> +#define SUN8I_PWM_PCCR(pair) (0x20 + ((pair) * 0x4))
> +#define SUN8I_PWM_PCCR_SRC_SHIFT 7
> +#define SUN8I_PWM_PCCR_SRC_MASK 1
> +#define SUN8I_PWM_PCCR_GATE_BIT 4
> +#define SUN8I_PWM_PCCR_BYPASS_BIT(chan) ((chan) % 2 + 5)
> +#define SUN8I_PWM_PCCR_DIV_M_SHIFT 0
> +
> +/* PWMCC Pairs Dead Zone Control Registers */
> +#define SUN8I_PWM_PDZCR(pair) (0x30 + ((pair) * 0x4))
> +
> +/* PWM Enable Register */
> +#define SUN8I_PWM_PER 0x40
> +#define SUN8I_PWM_ENABLE(chan) BIT(chan)
> +
> +/* PWM Capture Enable Register */
> +#define SUN8I_PWM_CER 0x44
> +
> +/* PWM Control Register */
> +#define SUN8I_PWM_PCR(chan) (0x60 + (chan) * 0x20)
> +#define SUN8I_PWM_PCR_PRESCAL_K_SHIFT 0
> +#define SUN8I_PWM_PCR_PRESCAL_K_WIDTH 8
> +#define SUN8I_PWM_PCR_ACTIVE_STATE BIT(8)
> +
> +/* PWM Period Register */
> +#define SUN8I_PWM_PPR(chan) (0x64 + (chan) * 0x20)
> +#define SUN8I_PWM_PPR_PERIOD_MASK GENMASK(31, 16)
> +#define SUN8I_PWM_PPR_DUTY_MASK GENMASK(15, 0)
> +#define SUN8I_PWM_PPR_PERIOD_VALUE(reg) (FIELD_GET(SUN8I_PWM_PPR_PERIOD_MASK, reg) + 1)
> +#define SUN8I_PWM_PPR_DUTY_VALUE(reg) FIELD_GET(SUN8I_PWM_PPR_DUTY_MASK, reg)
> +#define SUN8I_PWM_PPR_PERIOD(prd) FIELD_PREP(SUN8I_PWM_PPR_PERIOD_MASK, (prd) - 1)
> +#define SUN8I_PWM_DUTY(dty) FIELD_PREP(SUN8I_PWM_PPR_DUTY_MASK, dty)
> +#define SUN8I_PWM_PPR_PERIOD_MAX (FIELD_MAX(SUN8I_PWM_PPR_PERIOD_MASK) + 1)
> +
> +/* PWM Count Register */
> +#define SUN8I_PWM_PCNTR(chan) (0x68 + (chan) * 0x20)
> +
> +/* PWM Capture Control Register */
> +#define SUN8I_PWM_CCR(chan) (0x6c + (chan) * 0x20)
> +
> +/* PWM Capture Rise Lock Register */
> +#define SUN8I_PWM_CRLR(chan) (0x70 + (chan) * 0x20)
> +
> +/* PWM Capture Fall Lock Register */
> +#define SUN8I_PWM_CFLR(chan) (0x74 + (chan) * 0x20)
> +
> +#define SUN8I_PWM_PAIR_IDX(chan) ((chan) >> 1)
> +
> +/*
> + * Block diagram of the PWM clock controller:
> + *
> + * _____ ______ ________
> + * OSC24M --->| | | | | |
> + * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
> + * |_____| |______| |________|
> + * ________
> + * | |
> + * +->| /div_k |---> SUN8I_PWM_clock_x
> + * | |________|
> + * | ______
> + * | | |
> + * +-->| Gate |----> SUN8I_PWM_bypass_clock_x
> + * | |______|
> + * SUN8I_PWM_clock_src_xy ---+ ________
> + * | | |
> + * +->| /div_k |---> SUN8I_PWM_clock_y
> + * | |________|
> + * | ______
> + * | | |
> + * +-->| Gate |----> SUN8I_PWM_bypass_clock_y
> + * |______|
> + *
> + * NB: when the bypass is set, all the PWM logic is bypassed.
> + * So, the duty cycle and polarity can't be modified (we just have a clock).
> + * The bypass in PWM mode is used to achieve a 1/2 relative duty cycle with the
> + * fastest clock.
> + *
> + * SUN8I_PWM_clock_x/y serve for the PWM purpose.
> + * SUN8I_PWM_bypass_clock_x/y serve for the clock-provider purpose.
> + *
> + */
> +
> +/*
> + * Table used for /div_m (diviser before obtaining SUN8I_PWM_clock_src_xy)
> + * It's actually CLK_DIVIDER_POWER_OF_TWO, but limited to /256
> + */
> +#define CLK_TABLE_DIV_M_ENTRY(i) { \
> + .val = (i), .div = 1 << (i) \
> +}
> +
> +static const struct clk_div_table clk_table_div_m[] = {
> + CLK_TABLE_DIV_M_ENTRY(0),
> + CLK_TABLE_DIV_M_ENTRY(1),
> + CLK_TABLE_DIV_M_ENTRY(2),
> + CLK_TABLE_DIV_M_ENTRY(3),
> + CLK_TABLE_DIV_M_ENTRY(4),
> + CLK_TABLE_DIV_M_ENTRY(5),
> + CLK_TABLE_DIV_M_ENTRY(6),
> + CLK_TABLE_DIV_M_ENTRY(7),
> + CLK_TABLE_DIV_M_ENTRY(8),
> + { /* sentinel */ }
> +};
> +
> +#define SUN8I_PWM_XY_SRC_GATE(_pair, _reg) \
> +struct clk_gate gate_xy_##_pair = { \
> + .reg = (void *)(_reg), \
> + .bit_idx = SUN8I_PWM_PCCR_GATE_BIT, \
> + .hw.init = &(struct clk_init_data){ \
> + .ops = &clk_gate_ops, \
> + } \
> +}
> +
> +#define SUN8I_PWM_XY_SRC_MUX(_pair, _reg) \
> +struct clk_mux mux_xy_##_pair = { \
> + .reg = (void *)(_reg), \
> + .shift = SUN8I_PWM_PCCR_SRC_SHIFT, \
> + .mask = SUN8I_PWM_PCCR_SRC_MASK, \
> + .flags = CLK_MUX_ROUND_CLOSEST, \
> + .hw.init = &(struct clk_init_data){ \
> + .ops = &clk_mux_ops, \
> + } \
> +}
> +
> +#define SUN8I_PWM_XY_SRC_DIV(_pair, _reg) \
> +struct clk_divider rate_xy_##_pair = { \
> + .reg = (void *)(_reg), \
> + .shift = SUN8I_PWM_PCCR_DIV_M_SHIFT, \
> + .table = clk_table_div_m, \
> + .hw.init = &(struct clk_init_data){ \
> + .ops = &clk_divider_ops, \
> + } \
> +}
> +
> +#define SUN8I_PWM_X_DIV(_idx, _reg) \
> +struct clk_divider rate_x_##_idx = { \
> + .reg = (void *)(_reg), \
> + .shift = SUN8I_PWM_PCR_PRESCAL_K_SHIFT, \
> + .width = SUN8I_PWM_PCR_PRESCAL_K_WIDTH, \
> + .hw.init = &(struct clk_init_data){ \
> + .ops = &clk_divider_ops, \
> + } \
> +}
> +
> +#define SUN8I_PWM_X_BYPASS_GATE(_idx) \
> +struct clk_gate gate_x_bypass_##_idx = { \
> + .reg = (void *)SUN8I_PWM_PER, \
> + .bit_idx = _idx, \
> + .hw.init = &(struct clk_init_data){ \
> + .ops = &clk_gate_ops, \
> + } \
> +}
> +
> +#define SUN8I_PWM_XY_CLK_SRC(_pair, _reg) \
> + static SUN8I_PWM_XY_SRC_MUX(_pair, _reg); \
> + static SUN8I_PWM_XY_SRC_GATE(_pair, _reg); \
> + static SUN8I_PWM_XY_SRC_DIV(_pair, _reg)
> +
> +#define SUN8I_PWM_X_CLK(_idx) \
> + static SUN8I_PWM_X_DIV(_idx, SUN8I_PWM_PCR(_idx))
> +
> +#define SUN8I_PWM_X_BYPASS_CLK(_idx) \
> + SUN8I_PWM_X_BYPASS_GATE(_idx)
> +
> +#define REF_CLK_XY_SRC(_pair) \
> + { \
> + .name = "pwm-clk-src" #_pair, \
> + .mux_hw = &mux_xy_##_pair.hw, \
> + .gate_hw = &gate_xy_##_pair.hw, \
> + .rate_hw = &rate_xy_##_pair.hw, \
> + }
> +
> +#define REF_CLK_X(_idx, _pair) \
> + { \
> + .name = "pwm-clk" #_idx, \
> + .parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
> + .num_parents = 1, \
> + .rate_hw = &rate_x_##_idx.hw, \
> + .flags = CLK_SET_RATE_PARENT, \
> + }
> +
> +#define REF_CLK_BYPASS(_idx, _pair) \
> + { \
> + .name = "pwm-clk-bypass" #_idx, \
> + .parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
> + .num_parents = 1, \
> + .gate_hw = &gate_x_bypass_##_idx.hw, \
> + .flags = CLK_SET_RATE_PARENT, \
> + }
> +
> +/*
> + * SUN8I_PWM_clock_src_xy generation:
> + * _____ ______ ________
> + * OSC24M --->| | | | | |
> + * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
> + * |_____| |______| |________|
> + */
> +SUN8I_PWM_XY_CLK_SRC(01, SUN8I_PWM_PCCR(0));
> +SUN8I_PWM_XY_CLK_SRC(23, SUN8I_PWM_PCCR(1));
> +SUN8I_PWM_XY_CLK_SRC(45, SUN8I_PWM_PCCR(2));
> +
> +/*
> + * SUN8I_PWM_clock_x_div generation:
> + * ________
> + * | | SUN8I_PWM_clock_x/y
> + * SUN8I_PWM_clock_src_xy -->| /div_k |--------------->
> + * |________|
> + */
> +SUN8I_PWM_X_CLK(0);
> +SUN8I_PWM_X_CLK(1);
> +SUN8I_PWM_X_CLK(2);
> +SUN8I_PWM_X_CLK(3);
> +SUN8I_PWM_X_CLK(4);
> +SUN8I_PWM_X_CLK(5);
> +
> +/*
> + * SUN8I_PWM_bypass_clock_xy generation:
> + * ______
> + * | |
> + * SUN8I_PWM_clock_src_xy --->| Gate |-------> SUN8I_PWM_bypass_clock_x
> + * |______|
> + *
> + * The gate is actually SUN8I_PWM_PER register.
> + */
> +SUN8I_PWM_X_BYPASS_CLK(0);
> +SUN8I_PWM_X_BYPASS_CLK(1);
> +SUN8I_PWM_X_BYPASS_CLK(2);
> +SUN8I_PWM_X_BYPASS_CLK(3);
> +SUN8I_PWM_X_BYPASS_CLK(4);
> +SUN8I_PWM_X_BYPASS_CLK(5);
> +
> +struct clk_pwm_data {
> + const char *name;
> + const char **parent_names;
> + unsigned int num_parents;
> + struct clk_hw *mux_hw;
> + struct clk_hw *rate_hw;
> + struct clk_hw *gate_hw;
> + unsigned long flags;
> +};
> +
> +/* Indexes of REF_CLK_BYPASS and REF_CLK_XY_SRC in the array */
> +#define CLK_BYPASS_IDX(sun8i_chip, chan) ((sun8i_chip)->data->npwm + (chan))
> +#define CLK_XY_SRC_IDX(sun8i_chip, chan) \
> + ((sun8i_chip)->data->npwm * 2 + SUN8I_PWM_PAIR_IDX(chan))
> +static struct clk_pwm_data pwmcc_data[] = {
> + REF_CLK_X(0, 01),
> + REF_CLK_X(1, 01),
> + REF_CLK_X(2, 23),
> + REF_CLK_X(3, 23),
> + REF_CLK_X(4, 45),
> + REF_CLK_X(5, 45),
> + REF_CLK_BYPASS(0, 01),
> + REF_CLK_BYPASS(1, 01),
> + REF_CLK_BYPASS(2, 23),
> + REF_CLK_BYPASS(3, 23),
> + REF_CLK_BYPASS(4, 45),
> + REF_CLK_BYPASS(5, 45),
> + REF_CLK_XY_SRC(01),
> + REF_CLK_XY_SRC(23),
> + REF_CLK_XY_SRC(45),
> + { /* sentinel */ }
> +};
> +
> +enum sun8i_pwm_mode {
> + SUN8I_PWM_MODE_NONE,
> + SUN8I_PWM_MODE_PWM,
> + SUN8I_PWM_MODE_CLK,
> +};
> +
> +struct sun8i_pwm_data {
> + unsigned int npwm;
> +};
> +
> +struct sun8i_pwm_channel {
> + struct clk *pwm_clk;
> + enum sun8i_pwm_mode mode;
> +};
> +
> +struct clk_pwm_pdata {
> + struct clk_hw_onecell_data *hw_data;
> + spinlock_t lock;
> + void __iomem *reg;
> +};
> +
> +struct sun8i_pwm_chip {
> + struct clk_pwm_pdata *clk_pdata;
> + struct sun8i_pwm_channel *channels;
> + struct clk *bus_clk;
> + struct reset_control *rst;
> + void __iomem *base;
> + const struct sun8i_pwm_data *data;
> +};
> +
> +struct sun8i_pwm_waveform {
> + u8 enabled:1;
> + u8 active_state:1;
> + u8 bypass_en:1;
> + u16 duty_ticks;
> + u32 period_ticks;
> + unsigned long clk_rate;
> +};
> +
> +static inline struct sun8i_pwm_chip *sun8i_pwm_from_chip(const struct pwm_chip *chip)
> +{
> + return pwmchip_get_drvdata(chip);
> +}
> +
> +static inline u32 sun8i_pwm_readl(struct sun8i_pwm_chip *sun8i_chip,
> + unsigned long offset)
> +{
> + return readl(sun8i_chip->base + offset);
> +}
> +
> +static inline void sun8i_pwm_writel(struct sun8i_pwm_chip *sun8i_chip,
> + u32 val, unsigned long offset)
> +{
> + writel(val, sun8i_chip->base + offset);
> +}
> +
> +static void sun8i_pwm_set_bypass(struct sun8i_pwm_chip *sun8i_chip,
> + unsigned int idx, bool en_bypass)
> +{
> + unsigned long flags, reg_offset;
> + u32 val;
> +
> + spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> + reg_offset = SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(idx));
> + val = sun8i_pwm_readl(sun8i_chip, reg_offset);
> + if (en_bypass)
> + val |= BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
> + else
> + val &= ~BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
> +
> + sun8i_pwm_writel(sun8i_chip, val, reg_offset);
> +
> + spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
> +}
> +
> +static int sun8i_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +
> + scoped_guard(spinlock_irqsave, &sun8i_chip->clk_pdata->lock) {
> + if (chan->mode == SUN8I_PWM_MODE_CLK)
> + return -EBUSY;
> + chan->mode = SUN8I_PWM_MODE_PWM;
> + }
> +
> + return clk_prepare_enable(chan->pwm_clk);
> +}
> +
> +static void sun8i_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +
> + clk_disable_unprepare(chan->pwm_clk);
> + chan->mode = SUN8I_PWM_MODE_NONE;
> +}
> +
> +static int sun8i_pwm_read_waveform(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + void *_wfhw)
> +{
> + struct sun8i_pwm_waveform *wfhw = _wfhw;
> + struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> + u32 val;
> +
> + wfhw->clk_rate = clk_get_rate(chan->pwm_clk);
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
> + wfhw->enabled = !!(SUN8I_PWM_ENABLE(pwm->hwpwm) & val);
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(pwm->hwpwm)));
> + wfhw->bypass_en = !!(val & BIT(SUN8I_PWM_PCCR_BYPASS_BIT(pwm->hwpwm)));
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
> + wfhw->active_state = !!(val & SUN8I_PWM_PCR_ACTIVE_STATE);
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PPR(pwm->hwpwm));
> + wfhw->duty_ticks = SUN8I_PWM_PPR_DUTY_VALUE(val);
> + wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_VALUE(val);
> +
> + dev_dbg(pwmchip_parent(chip),
> + "pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
> + pwm->hwpwm,
> + wfhw->enabled ? "enabled" : "disabled",
> + wfhw->bypass_en ? "enabled" : "disabled",
> + wfhw->active_state ? "normal" : "inversed",
> + wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
> +
> + return 0;
> +}
> +
> +static int sun8i_pwm_round_waveform_fromhw(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + const void *_wfhw,
> + struct pwm_waveform *wf)
> +{
> + const struct sun8i_pwm_waveform *wfhw = _wfhw;
> + u64 tmp, resolution;
> +
> + dev_dbg(pwmchip_parent(chip),
> + "pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
> + pwm->hwpwm,
> + wfhw->enabled ? "enabled" : "disabled",
> + wfhw->bypass_en ? "enabled" : "disabled",
> + wfhw->active_state ? "normal" : "inversed",
> + wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
> +
> + wf->duty_offset_ns = 0;
> +
> + if (!wfhw->enabled || !wfhw->clk_rate) {
> + wf->period_length_ns = 0;
> + wf->duty_length_ns = 0;
> + return 0;
> + }
> +
> + if (wfhw->bypass_en) {
> + wf->period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
> + wfhw->clk_rate);
> + wf->duty_length_ns = DIV_ROUND_UP_ULL(wf->period_length_ns, 2);
> + return 0;
> + }
> +
> + tmp = NSEC_PER_SEC * (u64)wfhw->period_ticks;
> + wf->period_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
> +
> + tmp = NSEC_PER_SEC * (u64)wfhw->duty_ticks;
> + wf->duty_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
> + if (!wfhw->active_state) {
> + /*
> + * For inverted polarity, we have to fix cases where
> + * computed duty_length_ns > requested duty_length_ns
> + * For that, we subtract the actual resolution of the PWM
> + * registers
> + */
> + wf->duty_offset_ns = wf->duty_length_ns;
> + wf->duty_length_ns = wf->period_length_ns - wf->duty_length_ns;
> +
> + resolution = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wfhw->clk_rate);
> +
> + if (wf->duty_offset_ns >= resolution)
> + wf->duty_offset_ns -= resolution;
This looks wrong. If you have
clk_rate = 300000
period_ticks = 3000
duty_ticks = 440
active_state = 0
you're supposed to report:
.period_length_ns = roundup(3000 * 1000000000 / 300000) = 10000000
.duty_length_ns = roundup((3000 - 440) * 1000000000 / 300000) = 8533334
.duty_offset_ns = roundup(440 * 1000000000 / 300000) = 1466667
> + }
> +
> + dev_dbg(pwmchip_parent(chip),
> + "pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
> + pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
> + wf->duty_offset_ns);
Can you please only use a single output that has both the register state
and the waveform representation? Currently the waveform part is also
skipped on the early returns above. Also please use "%lld/%lld [+%lld]"
to represent the waveform.
> + return 0;
> +}
> +
> +static int sun8i_pwm_write_waveform(struct pwm_chip *chip,
> + struct pwm_device *pwm, const void *_wfhw)
> +{
> + const struct sun8i_pwm_waveform *wfhw = _wfhw;
> + struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> + unsigned long flags;
> + u32 val;
> + int ret;
> +
> + ret = clk_set_rate(chan->pwm_clk, wfhw->clk_rate);
> + if (ret)
> + return ret;
> +
> + sun8i_pwm_set_bypass(sun8i_chip, pwm->hwpwm, wfhw->bypass_en);
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
> + if (wfhw->active_state)
> + val |= SUN8I_PWM_PCR_ACTIVE_STATE;
> + else
> + val &= ~SUN8I_PWM_PCR_ACTIVE_STATE;
> + sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PCR(pwm->hwpwm));
> +
> + val = SUN8I_PWM_DUTY(wfhw->duty_ticks);
> + val |= SUN8I_PWM_PPR_PERIOD(wfhw->period_ticks);
> + sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PPR(pwm->hwpwm));
> +
> + spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> + val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
> + if (wfhw->enabled)
> + val |= SUN8I_PWM_ENABLE(pwm->hwpwm);
> + else
> + val &= ~SUN8I_PWM_ENABLE(pwm->hwpwm);
> + sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PER);
> +
> + spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
> +
> + return 0;
> +}
> +
> +static int sun8i_pwm_round_waveform_tohw(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + const struct pwm_waveform *wf,
> + void *_wfhw)
> +{
> + struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> + struct sun8i_pwm_waveform *wfhw = _wfhw;
> + unsigned long max_rate;
> + long calc_rate;
> + u64 period_ratio, double_duty_ratio, freq, duty_cycle;
> +
> + dev_dbg(pwmchip_parent(chip),
> + "pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
> + pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
> + wf->duty_offset_ns);
> +
> + if (wf->period_length_ns == 0) {
> + wfhw->enabled = 0;
> + return 0;
> + }
> +
> + wfhw->enabled = 1;
> +
> + duty_cycle = wf->duty_length_ns;
> + if (wf->duty_length_ns + wf->duty_offset_ns < wf->period_length_ns)
> + wfhw->active_state = 1;
> + else
> + wfhw->active_state = 0;
Please look at pwm-stm32.c on how to determine if inversed polarity
should be used or not.
> + dev_dbg(pwmchip_parent(chip), "polarity: %s\n",
> + wfhw->active_state ? "normal" : "inversed");
> +
> + /*
> + * Lowest possible period case:
> + * Without bypass, the lowest possible period is when:
> + * duty cycle = 1 and period cycle = 2 (0x10001 in period register)
> + * E.g. if the input clock is 100MHz, we have a lowest period of 20ns.
> + * Now, with the bypass, the period register is ignored and we directly
> + * have the 100MHz clock as PWM output, that can act as a 10ns period
> + * with 5ns duty.
> + * So, to detect this lowest period case, just get the maximum possible
> + * rate from chan->pwm_clk and compare it with requested period and
> + * duty_cycle.
> + *
> + * But, to get the maximum possible rate, we have to use U32_MAX instead
> + * of (unsigned long)-1.
> + * This is because clk_round_rate() uses ultimately DIV_ROUND_UP_ULL()
> + * that in turn do_div(n,base). And base is uint32_t divisor.
> + */
> + max_rate = clk_round_rate(chan->pwm_clk, U32_MAX);
> +
> + dev_dbg(pwmchip_parent(chip), "max_rate: %ld Hz\n", max_rate);
> +
> + period_ratio = mul_u64_u64_div_u64(wf->period_length_ns,
> + max_rate, NSEC_PER_SEC);
> + double_duty_ratio = mul_u64_u64_div_u64(duty_cycle, (u64)max_rate * 2,
> + NSEC_PER_SEC);
> + if (period_ratio == 1) {
> + if (double_duty_ratio == 0)
> + /* requested period and duty are too small */
> + return -EINVAL;
> + /*
> + * If the requested period is to small to be generated by the
> + * PWM, but matches the highest clock with a
> + * duty_cycle >= period*2, just bypass the PWM logic
> + */
> + freq = div64_u64(NSEC_PER_SEC, wf->period_length_ns);
> + wfhw->bypass_en = true;
> + } else {
> + wfhw->bypass_en = false;
> + freq = div64_u64(NSEC_PER_SEC * (u64)SUN8I_PWM_PPR_PERIOD_MAX,
> + wf->period_length_ns);
> + /*
> + * Same remark as above, this is to prevent a value to big for
> + * clk_round_rate() to handle
> + */
> + if (freq > U32_MAX)
> + freq = U32_MAX;
> + }
> +
> + dev_dbg(pwmchip_parent(chip), "bypass: %s\n",
> + wfhw->bypass_en ? "enabled" : "disabled");
> +
> + calc_rate = clk_round_rate(chan->pwm_clk, freq);
> + if (calc_rate <= 0)
> + return calc_rate ? calc_rate : -EINVAL;
> +
> + dev_dbg(pwmchip_parent(chip), "calc_rate: %ld Hz\n", calc_rate);
> +
> + wfhw->period_ticks = mul_u64_u64_div_u64(calc_rate,
> + wf->period_length_ns,
> + NSEC_PER_SEC);
> + if (wfhw->period_ticks > SUN8I_PWM_PPR_PERIOD_MAX)
> + wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_MAX;
> +
> + /* min value in period register is 1 */
> + if (wfhw->period_ticks == 0)
> + return -EINVAL;
> +
> + wfhw->duty_ticks = mul_u64_u64_div_u64(calc_rate, duty_cycle,
> + NSEC_PER_SEC);
> +
> + if (wfhw->duty_ticks > wfhw->period_ticks)
> + wfhw->duty_ticks = wfhw->period_ticks;
> +
> + if (!wfhw->active_state)
> + wfhw->duty_ticks = wfhw->period_ticks - wfhw->duty_ticks;
> +
> + dev_dbg(pwmchip_parent(chip),
> + "pwm%u period_ticks=%u duty_cycle=%llu duty_ticks=%u\n",
> + pwm->hwpwm, wfhw->period_ticks, duty_cycle, wfhw->duty_ticks);
As for the fromhw callback: Please emit a single line here and stick to
the common format for *wf.
> +
> + wfhw->clk_rate = calc_rate;
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops sun8i_pwm_ops = {
> + .request = sun8i_pwm_request,
> + .free = sun8i_pwm_free,
> + .sizeof_wfhw = sizeof(struct sun8i_pwm_waveform),
> + .round_waveform_tohw = sun8i_pwm_round_waveform_tohw,
> + .round_waveform_fromhw = sun8i_pwm_round_waveform_fromhw,
> + .read_waveform = sun8i_pwm_read_waveform,
> + .write_waveform = sun8i_pwm_write_waveform,
> +};
> +
> +static struct clk_hw *sun8i_pwm_of_clk_get(struct of_phandle_args *clkspec,
> + void *data)
> +{
> + struct sun8i_pwm_chip *sun8i_chip = data;
> + struct clk_hw_onecell_data *hw_data = sun8i_chip->clk_pdata->hw_data;
> + unsigned int idx = clkspec->args[0];
> + struct sun8i_pwm_channel *chan;
> + struct clk_hw *ret_clk = NULL;
> + unsigned long flags;
> +
> + if (idx >= sun8i_chip->data->npwm)
> + return ERR_PTR(-EINVAL);
> +
> + chan = &sun8i_chip->channels[idx];
> +
> + spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> + if (chan->mode == SUN8I_PWM_MODE_PWM) {
> + ret_clk = ERR_PTR(-EBUSY);
> + } else {
> + chan->mode = SUN8I_PWM_MODE_CLK;
> + ret_clk = hw_data->hws[CLK_BYPASS_IDX(sun8i_chip, idx)];
> + }
> + spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
> +
> + if (IS_ERR(ret_clk))
> + goto out;
> +
> + sun8i_pwm_set_bypass(sun8i_chip, idx, true);
> +out:
> + return ret_clk;
> +}
> +
> +static int sun8i_add_composite_clk(struct clk_pwm_data *data,
> + void __iomem *reg, spinlock_t *lock,
> + struct device *dev, struct clk_hw **hw)
> +{
> + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *rate_ops = NULL;
> + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *rate_hw = NULL;
> + struct device_node *node = dev->of_node;
> +
> + if (data->mux_hw) {
> + struct clk_mux *mux;
> +
> + mux_hw = data->mux_hw;
> + mux = to_clk_mux(mux_hw);
> + mux->lock = lock;
> + mux_ops = mux_hw->init->ops;
> + mux->reg = (uintptr_t)mux->reg + reg;
> + }
> +
> + if (data->gate_hw) {
> + struct clk_gate *gate;
> +
> + gate_hw = data->gate_hw;
> + gate = to_clk_gate(gate_hw);
> + gate->lock = lock;
> + gate_ops = gate_hw->init->ops;
> + gate->reg = (uintptr_t)gate->reg + reg;
> + }
> +
> + if (data->rate_hw) {
> + struct clk_divider *rate;
> +
> + rate_hw = data->rate_hw;
> + rate = to_clk_divider(rate_hw);
> + rate_ops = rate_hw->init->ops;
> + rate->lock = lock;
> + rate->reg = (uintptr_t)rate->reg + reg;
> +
> + if (rate->table) {
> + const struct clk_div_table *clkt;
> + int table_size = 0;
> +
> + for (clkt = rate->table; clkt->div; clkt++)
> + table_size++;
> + rate->width = order_base_2(table_size);
> + }
> + }
> +
> + /*
> + * Retrieve the parent clock names from DTS for pwm-clk-srcxy
> + */
> + if (!data->parent_names) {
> + data->num_parents = of_clk_get_parent_count(node);
> + if (data->num_parents == 0)
> + return -ENOENT;
> +
> + data->parent_names = devm_kzalloc(dev,
> + sizeof(*data->parent_names),
> + GFP_KERNEL);
> + for (unsigned int i = 0; i < data->num_parents; i++)
> + data->parent_names[i] = of_clk_get_parent_name(node, i);
> + }
> +
> + *hw = clk_hw_register_composite(dev, data->name, data->parent_names,
> + data->num_parents, mux_hw,
> + mux_ops, rate_hw, rate_ops,
> + gate_hw, gate_ops, data->flags);
> +
> + return PTR_ERR_OR_ZERO(*hw);
> +}
> +
> +static int sun8i_pwm_init_clocks(struct platform_device *pdev,
> + struct sun8i_pwm_chip *sun8i_chip)
> +{
> + struct clk_pwm_pdata *pdata;
> + struct device *dev = &pdev->dev;
> + int num_clocks = 0;
> + int ret;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return dev_err_probe(dev, -ENOMEM,
> + "Failed to allocate clk_pwm_pdata\n");
> +
> + while (pwmcc_data[num_clocks].name)
> + num_clocks++;
> +
> + pdata->hw_data = devm_kzalloc(dev, struct_size(pdata->hw_data, hws, num_clocks),
> + GFP_KERNEL);
> + if (!pdata->hw_data)
> + return dev_err_probe(dev, -ENOMEM,
> + "Failed to allocate hw clocks\n");
> +
> + pdata->hw_data->num = num_clocks;
> + pdata->reg = sun8i_chip->base;
> +
> + spin_lock_init(&pdata->lock);
> +
> + for (int i = 0; i < num_clocks; i++) {
> + struct clk_hw **hw = &pdata->hw_data->hws[i];
> +
> + ret = sun8i_add_composite_clk(&pwmcc_data[i], pdata->reg,
> + &pdata->lock, dev, hw);
If you register the unregister call here, cleanup gets a bit easier.
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Failed to register hw clock %s\n",
> + pwmcc_data[i].name);
> + for (i--; i >= 0; i--)
> + clk_hw_unregister_composite(pdata->hw_data->hws[i]);
> + return ret;
> + }
> + }
> +
> + sun8i_chip->clk_pdata = pdata;
If you copy the content of *pdata to sun8i_chip you can save an
allocation here and several pointer dereferences during runtime.
> +
> + return 0;
> +}
> +
> +static void sun8i_pwm_unregister_clk(void *data)
> +{
> + struct clk_hw_onecell_data *hw_data = data;
> +
> + for (unsigned int i = 0; i < hw_data->num; i++)
> + clk_hw_unregister_composite(hw_data->hws[i]);
> +}
> +
> +static int sun8i_pwm_probe(struct platform_device *pdev)
> +{
> + const struct sun8i_pwm_data *data;
> + struct device *dev = &pdev->dev;
> + struct sun8i_pwm_chip *sun8i_chip;
> + struct pwm_chip *chip;
> + int ret;
> +
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return dev_err_probe(dev, -ENODEV,
> + "Missing specific data structure\n");
> +
> + chip = devm_pwmchip_alloc(dev, data->npwm, sizeof(*sun8i_chip));
> + if (IS_ERR(chip))
> + return dev_err_probe(dev, PTR_ERR(chip),
> + "Failed to allocate pwmchip\n");
> +
> + sun8i_chip = sun8i_pwm_from_chip(chip);
> + sun8i_chip->data = data;
> + sun8i_chip->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(sun8i_chip->base))
> + return dev_err_probe(dev, PTR_ERR(sun8i_chip->base),
> + "Failed to get PWM base address\n");
> +
> + sun8i_chip->bus_clk = devm_clk_get_enabled(dev, "bus");
> + if (IS_ERR(sun8i_chip->bus_clk))
> + return dev_err_probe(dev, PTR_ERR(sun8i_chip->bus_clk),
> + "Failed to get bus clock\n");
> +
> + sun8i_chip->channels = devm_kmalloc_array(dev, data->npwm,
> + sizeof(*(sun8i_chip->channels)),
> + GFP_KERNEL);
I wonder if the clk framework ensures that after
devm_of_clk_release_provider() (i.e. the unregister part of
devm_of_clk_add_hw_provider()) sun8i_pwm_of_clk_get() isn't called any
more. If not this might trigger a use-after-free.
> + if (!sun8i_chip->channels)
> + return dev_err_probe(dev, -ENOMEM,
> + "Failed to allocate %d channels array\n",
> + data->npwm);
> +
> + chip->ops = &sun8i_pwm_ops;
Apart from the clk_get_rate() call in the callbacks you could set
.atomic=true.
Also use clk_rate_exclusive_get() to prevent that the parent clk rate
changes while the hardware emits a waveform. (Then also the need to have
clk_rate in sun8i_pwm_waveform goes away.)
> +
> + ret = sun8i_pwm_init_clocks(pdev, sun8i_chip);
> + if (ret)
> + return ret;
> +
> + for (unsigned int i = 0; i < data->npwm; i++) {
> + struct sun8i_pwm_channel *chan = &sun8i_chip->channels[i];
> + struct clk_hw **hw = &sun8i_chip->clk_pdata->hw_data->hws[i];
> +
> + chan->pwm_clk = devm_clk_hw_get_clk(dev, *hw, NULL);
> + if (IS_ERR(chan->pwm_clk)) {
> + ret = dev_err_probe(dev, PTR_ERR(chan->pwm_clk),
> + "Failed to register PWM clock %d\n", i);
> + return ret;
Please shorten that to
return dev_err_probe(....);
> + }
> + chan->mode = SUN8I_PWM_MODE_NONE;
> + }
> +
> + ret = devm_of_clk_add_hw_provider(dev, sun8i_pwm_of_clk_get, sun8i_chip);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to add HW clock provider\n");
> +
> + ret = devm_add_action_or_reset(dev, sun8i_pwm_unregister_clk,
> + sun8i_chip->clk_pdata->hw_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to add devm action\n");
> +
> + /* Deassert reset */
> + sun8i_chip->rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> + if (IS_ERR(sun8i_chip->rst))
> + return dev_err_probe(dev, PTR_ERR(sun8i_chip->rst),
> + "Failed to get reset control\n");
> +
> + ret = devm_pwmchip_add(dev, chip);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
> +
> + platform_set_drvdata(pdev, chip);
this isn't used, so please drop it.
> + return 0;
> +}
> +
> +static const struct sun8i_pwm_data sun50i_h616_pwm_data = {
> + .npwm = 6,
> +};
Do you expect that other variants of this hardware will appear that have
!= 6 channels? If not, please use the 6 directly in .probe().
> +static const struct of_device_id sun8i_pwm_dt_ids[] = {
> + {
> + .compatible = "allwinner,sun50i-h616-pwm",
> + .data = &sun50i_h616_pwm_data,
> + }, {
> + /* sentinel */
> + }
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_pwm_dt_ids);
> +
> +static struct platform_driver sun8i_pwm_driver = {
> + .driver = {
> + .name = "sun8i-pwm",
> + .of_match_table = sun8i_pwm_dt_ids,
> + },
> + .probe = sun8i_pwm_probe,
> +};
> +module_platform_driver(sun8i_pwm_driver);
> +
> +MODULE_AUTHOR("Richard Genoud <richard.genoud@bootlin.com>");
> +MODULE_DESCRIPTION("Allwinner sun8i PWM driver");
> +MODULE_LICENSE("GPL");
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] spi: zynq-qspi: Remove redundant clock enables in setup
From: Gaetan Florio @ 2026-05-25 17:46 UTC (permalink / raw)
To: broonie
Cc: michal.simek, linux-spi, linux-arm-kernel, linux-kernel,
Gaetan Florio
The QSPI clocks are already enabled in probe() with clk_prepare_enable().
zynq_qspi_setup_op() enables them again with clk_enable(), but the extra
enable is never balanced. This leaves the clock enable count elevated and
can trigger warnings when unbinding the driver.
Remove the redundant clock enables from the setup callback.
Signed-off-by: Gaetan Florio <gaetansjo@gmail.com>
---
drivers/spi/spi-zynq-qspi.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index 5f23e902b..2aa2c9621 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -437,21 +437,10 @@ static int zynq_qspi_setup_op(struct spi_device *spi)
{
struct spi_controller *ctlr = spi->controller;
struct zynq_qspi *qspi = spi_controller_get_devdata(ctlr);
- int ret;
if (ctlr->busy)
return -EBUSY;
- ret = clk_enable(qspi->refclk);
- if (ret)
- return ret;
-
- ret = clk_enable(qspi->pclk);
- if (ret) {
- clk_disable(qspi->refclk);
- return ret;
- }
-
zynq_qspi_write(qspi, ZYNQ_QSPI_ENABLE_OFFSET,
ZYNQ_QSPI_ENABLE_ENABLE_MASK);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] ARM: OMAP2+: Add CFI type for omap4_finish_suspend
From: Nathan Chancellor @ 2026-05-25 16:55 UTC (permalink / raw)
To: kernel test robot
Cc: Mithil Bavishi, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Russell King, llvm, oe-kbuild-all,
Sami Tolvanen, Kees Cook, linux-arm-kernel, linux-omap,
linux-kernel
In-Reply-To: <202605241332.fXvCYBR2-lkp@intel.com>
On Sun, May 24, 2026 at 02:01:31PM +0800, kernel test robot wrote:
> Hi Mithil,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on tmlind-omap/for-next]
> [also build test ERROR on linus/master v7.1-rc4 next-20260522]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Mithil-Bavishi/ARM-OMAP2-Add-CFI-type-for-omap4_finish_suspend/20260523-073206
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
> patch link: https://lore.kernel.org/r/20260522233036.12277-1-bavishimithil%40gmail.com
> patch subject: [PATCH v2] ARM: OMAP2+: Add CFI type for omap4_finish_suspend
> config: arm-defconfig (https://download.01.org/0day-ci/archive/20260524/202605241332.fXvCYBR2-lkp@intel.com/config)
> compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 5bac06718f502014fade905512f1d26d578a18f3)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260524/202605241332.fXvCYBR2-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202605241332.fXvCYBR2-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> arch/arm/mach-omap2/sleep44xx.S:61:1: error: invalid instruction
> SYM_TYPED_FUNC_START(omap4_finish_suspend)
> ^
This report and the GCC one
https://lore.kernel.org/202605252315.UhjCBtGA-lkp@intel.com/
are both pointing out that cfi_types.h needs to be included in
arch/arm/mach-omap2/sleep44xx.S for SYM_TYPED_FUNC_START to always be
expanded properly. I tested adding
#include <linux/cfi_types.h>
as the first include line and both issues were resolved.
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v8 01/10] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588
From: Conor Dooley @ 2026-05-25 16:54 UTC (permalink / raw)
To: Damon Ding
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <20260525125331.140059-2-damon.ding@rock-chips.com>
[-- Attachment #1: Type: text/plain, Size: 3652 bytes --]
On Mon, May 25, 2026 at 08:53:22PM +0800, Damon Ding wrote:
> RK3588 eDP controller requires HCLK_VO1 to access the VO1 GRF
> registers and enable the video datapath.
>
> Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
> phandle reference, which allowed the eDP to work without explicitly
> managing the hclk_vo1 clock. However, this is not safe or explicit.
>
> To make the clock dependency explicit, enforce per-SoC clock-names
> requirements:
> - RK3288: 2 clocks (dp, pclk)
> - RK3399: 3 clocks (dp, pclk, grf)
> - RK3588: 3 clocks (dp, pclk, hclk)
>
> Do not reuse the 'grf' clock name for RK3588 because it represents
> a different clock with distinct control logic:
> - The 'grf' clock is only for GRF register access and is toggled
> dynamically during register access.
> - The 'hclk' clock controls both GRF access and video datapath
> gating, and must remain enabled during probe.
>
> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>
> ---
>
> Changes in v4:
> - Modify the commit msg.
>
> Changes in v5:
> - Enforce the correct third clock name on a per-compatible basis.
> - Modify the commit msg simultaneously.
>
> Changes in v6:
> - Expand more detail commit msg about using hclk instead of grf clock.
>
> Changes in v7:
> - List all valid clock names at the top level, and constrain the clock
> count for each platform with minItems/maxItems in allOf.
>
> Changes in v8:
> - Fix indentation to 10 for enum in clock-names property.
> ---
> .../rockchip/rockchip,analogix-dp.yaml | 34 ++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> index d99b23b88cc5..a1ab7a77bdd3 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> @@ -26,7 +26,9 @@ properties:
> items:
> - const: dp
> - const: pclk
> - - const: grf
> + - enum:
> + - grf
> + - hclk
>
> power-domains:
> maxItems: 1
> @@ -60,6 +62,32 @@ required:
> allOf:
> - $ref: /schemas/display/bridge/analogix,dp.yaml#
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3288-dp
> + then:
> + properties:
> + clocks:
> + maxItems: 2
> + clock-names:
> + maxItems: 2
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - rockchip,rk3399-edp
> + then:
> + properties:
> + clocks:
> + minItems: 3
> + clock-names:
> + minItems: 3
If you go back to v6, you'll see that I never asked you to remove the
explict clock-names from here or below. Only the one from the 3288
section. The minItems was an addition, not a replacement.
pw-bot: changes-requested
Cheers,
Conor.
> +
> - if:
> properties:
> compatible:
> @@ -68,6 +96,10 @@ allOf:
> - rockchip,rk3588-edp
> then:
> properties:
> + clocks:
> + minItems: 3
> + clock-names:
> + minItems: 3
> resets:
> minItems: 2
> reset-names:
> --
> 2.34.1
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2] iommu: Allow device driver to use its own PASID space for SVA
From: Jason Gunthorpe @ 2026-05-25 16:47 UTC (permalink / raw)
To: Joonwon Kang
Cc: kevin.tian, Alexander.Grest, alexander.shishkin, amhetre,
baolu.lu, bp, dave.hansen, easwar.hariharan, hpa, iommu,
jacob.jun.pan, joro, jpb, kas, kees, linux-arm-kernel,
linux-kernel, mingo, nicolinc, peterz, praan, robin.murphy,
ryasuoka, smostafa, sohil.mehta, tglx, will, x86, xin
In-Reply-To: <20260525152924.524721-1-joonwonkang@google.com>
On Mon, May 25, 2026 at 03:29:24PM +0000, Joonwon Kang wrote:
> Currently, the only known expected user of the new kAPI is our team. Since
> I test if the patch resolves our problem before sending it, I believe it
> should be good enough. Do you mean more than our team by "accompanied
> users"?
He means you cannot send patches like this that only serve OOT drivers
to the mainline kernel.
Jason
^ permalink raw reply
* [PATCH v2 2/2] i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)
From: Vincent Jardin @ 2026-05-25 16:43 UTC (permalink / raw)
To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
Shawn Guo, Stefan Eichenberger
Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
stable
In-Reply-To: <20260525-for-upstream-i2c-lx2160-fix-v1-v2-0-26a3cc8cd055@free.fr>
SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the
interrupt-driven block-read state machine rejects it as -EPROTO. Worse,
it returns without a NACK+STOP: the next receive cycle has already
started, so the target keeps holding SDA and the bus stays stuck until a
power cycle of this i2c controller.
Accept count=0: NACK the in-flight dummy byte (TXAK) and set msg->len to
2 so i2c_imx_isr_read_continue() emits STOP via its normal last-byte
path. The dummy byte is discarded; block-read callers only consume
buf[0..count-1].
Reading I2DR has likewise already armed the next byte on the
count > I2C_SMBUS_BLOCK_MAX error path, so NACK it (TXAK) before aborting
with -EPROTO; otherwise the failing transfer's STOP cannot complete and
the bus stays held.
The atomic path regressed earlier (v3.16) and is fixed separately; this
patch covers only the v6.13 state-machine rework.
Fixes: 5f5c2d4579ca ("i2c: imx: prevent rescheduling in non dma mode")
Cc: <stable@vger.kernel.org> # v6.13+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
drivers/i2c/busses/i2c-imx.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 14107e1ad413..8db8d2e10f5c 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1061,11 +1061,28 @@ static inline enum imx_i2c_state i2c_imx_isr_read_continue(struct imx_i2c_struct
static inline void i2c_imx_isr_read_block_data_len(struct imx_i2c_struct *i2c_imx)
{
u8 len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
+ unsigned int temp;
if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) {
+ /*
+ * SMBus 3.1 6.5.7: support count byte of 0.
+ * I2C_SMBUS_BLOCK_MAX case should not hold the SDA either.
+ * So NACK it (TXAK) to not hold the bus.
+ */
+ temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+ temp |= I2CR_TXAK;
+ imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+
+ if (len == 0) {
+ i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = 0;
+ i2c_imx->msg->len = 2;
+ return;
+ }
+
i2c_imx->isr_result = -EPROTO;
i2c_imx->state = IMX_I2C_STATE_FAILED;
wake_up(&i2c_imx->queue);
+ return;
}
i2c_imx->msg->len += len;
i2c_imx->msg->buf[i2c_imx->msg_buf_idx++] = len;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
From: Vincent Jardin @ 2026-05-25 16:43 UTC (permalink / raw)
To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
Shawn Guo, Stefan Eichenberger
Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
stable
In-Reply-To: <20260525-for-upstream-i2c-lx2160-fix-v1-v2-0-26a3cc8cd055@free.fr>
SMBus 3.1 6.5.7 allows a Block Read byte count of 0, but the atomic
(polling) path rejects it as -EPROTO. Worse, it returns without a
NACK+STOP: the next receive cycle has already started, so the target
keeps holding SDA and the bus stays stuck until a power cycle for
this i2c controller.
Reading I2DR to obtain the count likewise arms the next byte on the
count > I2C_SMBUS_BLOCK_MAX path, which also returned -EPROTO directly
and left the bus held.
Handle both: NACK the in-flight dummy byte (TXAK) and extend msgs->len so
the existing last-byte handling emits STOP; the dummy byte is discarded.
A count of 0 is a valid empty block read; a count above
I2C_SMBUS_BLOCK_MAX is still reported as -EPROTO, but only after the bus
has been released.
The interrupt-driven path has the same flaw from a later commit and is
fixed separately, as it carries a different Fixes: tag and stable range.
Fixes: 8e8782c71595 ("i2c: imx: add SMBus block read support")
Cc: <stable@vger.kernel.org> # v3.16+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
drivers/i2c/busses/i2c-imx.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index a208fefd3c3b..14107e1ad413 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1415,6 +1415,7 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
int i, result;
unsigned int temp;
int block_data = msgs->flags & I2C_M_RECV_LEN;
+ int block_err = 0;
result = i2c_imx_prepare_read(i2c_imx, msgs, false);
if (result)
@@ -1436,8 +1437,20 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
*/
if ((!i) && block_data) {
len = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
- if ((len == 0) || (len > I2C_SMBUS_BLOCK_MAX))
- return -EPROTO;
+ if ((len == 0) || (len > I2C_SMBUS_BLOCK_MAX)) {
+ /*
+ * SMBus 3.1 6.5.7: support count byte of 0.
+ * I2C_SMBUS_BLOCK_MAX case should not hold the SDA either.
+ */
+ if (len > I2C_SMBUS_BLOCK_MAX)
+ block_err = -EPROTO;
+ temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+ temp |= I2CR_TXAK;
+ imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
+ msgs->buf[0] = 0;
+ msgs->len = 2;
+ continue;
+ }
dev_dbg(&i2c_imx->adapter.dev,
"<%s> read length: 0x%X\n",
__func__, len);
@@ -1485,7 +1498,7 @@ static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx,
"<%s> read byte: B%d=0x%X\n",
__func__, i, msgs->buf[i]);
}
- return 0;
+ return block_err;
}
static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
From: Vincent Jardin @ 2026-05-25 16:43 UTC (permalink / raw)
To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
Sascha Hauer, Fabio Estevam, Wolfram Sang, Kaushal Butala,
Shawn Guo, Stefan Eichenberger
Cc: linux-i2c, imx, linux-arm-kernel, linux-kernel, Vincent Jardin,
stable
In-Reply-To: <20260525-for-upstream-i2c-lx2160-fix-v1-v1-0-f30ab53dd97c@free.fr>
i2c-imx rejects a SMBus Block Read byte count of 0 (valid per SMBus 3.1
6.5.7) and it returns without a NACK+STOP, leaving the target
holding SDA so the bus is stuck until a power cycle occur.
The same bug is occuring with two independently introduced spots, so the
fix is two patches with their respective Fixes: tags and backport ranges:
1/2 atomic/polling path Fixes: 8e8782c71595 v3.16+
2/2 IRQ-driven state machine Fixes: 5f5c2d4579ca v6.13+
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
Changes in v2:
- Handle when count > I2C_SMBUS_BLOCK_MAX the same way as count == 0
Reported by the Sashiko AI review on v1.
---
Vincent Jardin (2):
i2c: imx: fix locked bus on SMBus block-read of 0 (atomic)
i2c: imx: fix locked bus on SMBus block-read of 0 (IRQ)
drivers/i2c/busses/i2c-imx.c | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
---
base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
change-id: 20260525-for-upstream-i2c-lx2160-fix-v1-0cba0a0093e5
Best regards,
--
Vincent Jardin <vjardin@free.fr>
^ permalink raw reply
* [PATCH 2/2] gpio: rockchip: fix teardown bugs and resource leaks
From: Marco Scardovi (scardracs) @ 2026-05-25 16:39 UTC (permalink / raw)
To: linusw, brgl, heiko
Cc: linux-gpio, linux-rockchip, linux-arm-kernel, linux-kernel,
Marco Scardovi (scardracs)
In-Reply-To: <20260525164230.43307-1-scardracs@disroot.org>
This commit addresses several teardown issues and resource leaks in the
driver's remove path and error handling:
1. Debounce clock reference leak: The debounce clock (bank->db_clk) was
obtained using of_clk_get() which increments the clock's reference
count, but clk_put() was never called. A devm action is now registered
to cleanly release it on unbind. Note that of_clk_get(..., 1) remains
necessary over devm_clk_get() because the DT binding does not define
clock-names, precluding name-based lookup.
2. Unregistered chained IRQ handler: The chained IRQ handler was not
disconnected in remove(). If a stray interrupt fired after the driver
was removed, the kernel would attempt to execute a stale handler,
leading to a panic. This is fixed by clearing the handler in remove().
3. IRQ domain leak: The linear IRQ domain and its generic chips were
allocated manually during probe but never removed. The IRQ domain is
now removed during driver teardown to free the associated generic chips
and mappings.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
drivers/gpio/gpio-rockchip.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 33580093a4e7..c804f970d823 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -638,10 +638,17 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank)
return ret;
}
+static void rockchip_clk_put(void *data)
+{
+ struct clk *clk = data;
+
+ clk_put(clk);
+}
+
static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
{
struct resource res;
- int id = 0;
+ int id = 0, ret;
if (of_address_to_resource(bank->of_node, 0, &res)) {
dev_err(bank->dev, "cannot find IO resource for bank\n");
@@ -673,6 +680,13 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
dev_err(bank->dev, "cannot find debounce clk\n");
return -EINVAL;
}
+
+ ret = devm_add_action_or_reset(bank->dev, rockchip_clk_put,
+ bank->db_clk);
+ if (ret) {
+ dev_err(bank->dev, "failed to register debounce clk action\n");
+ return ret;
+ }
break;
case GPIO_TYPE_V1:
bank->gpio_regs = &gpio_regs_v1;
@@ -789,6 +803,9 @@ static void rockchip_gpio_remove(struct platform_device *pdev)
{
struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
+ irq_set_chained_handler_and_data(bank->irq, NULL, NULL);
+ if (bank->domain)
+ irq_domain_remove(bank->domain);
gpiochip_remove(&bank->gpio_chip);
}
--
2.54.0
^ permalink raw reply related
* [PATCH 1/2] gpio: rockchip: convert bank->clk to devm_clk_get_enabled()
From: Marco Scardovi (scardracs) @ 2026-05-25 16:39 UTC (permalink / raw)
To: linusw, brgl, heiko
Cc: linux-gpio, linux-rockchip, linux-arm-kernel, linux-kernel,
Marco Scardovi (scardracs)
In-Reply-To: <20260525164230.43307-1-scardracs@disroot.org>
The bank->clk was previously obtained via of_clk_get() and manually
prepared/enabled. However, it was missing a corresponding clk_put() in
both the error paths and the remove function, leading to a reference leak.
Convert the allocation to devm_clk_get_enabled(), which also properly
propagates failures from clk_prepare_enable() that were previously ignored.
The GPIO bank device uses the same OF node as the previous of_clk_get()
call, so devm_clk_get_enabled(dev, NULL) correctly resolves the same
clock provider entry.
This change fixes the reference leak and simplifies the code by removing
the manual clk_disable_unprepare() calls in the probe error paths and
in the remove function.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
drivers/gpio/gpio-rockchip.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 44d7ebd12724..33580093a4e7 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -656,11 +656,10 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
if (!bank->irq)
return -EINVAL;
- bank->clk = of_clk_get(bank->of_node, 0);
+ bank->clk = devm_clk_get_enabled(bank->dev, NULL);
if (IS_ERR(bank->clk))
return PTR_ERR(bank->clk);
- clk_prepare_enable(bank->clk);
id = readl(bank->reg_base + gpio_regs_v2.version_id);
switch (id) {
@@ -672,7 +671,6 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank)
bank->db_clk = of_clk_get(bank->of_node, 1);
if (IS_ERR(bank->db_clk)) {
dev_err(bank->dev, "cannot find debounce clk\n");
- clk_disable_unprepare(bank->clk);
return -EINVAL;
}
break;
@@ -751,7 +749,6 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
ret = rockchip_gpiolib_register(bank);
if (ret) {
- clk_disable_unprepare(bank->clk);
mutex_unlock(&bank->deferred_lock);
return ret;
}
@@ -792,7 +789,6 @@ static void rockchip_gpio_remove(struct platform_device *pdev)
{
struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
- clk_disable_unprepare(bank->clk);
gpiochip_remove(&bank->gpio_chip);
}
--
2.54.0
^ permalink raw reply related
* [PATCH 0/2] gpio: rockchip: fix resource leaks and teardown bugs
From: Marco Scardovi (scardracs) @ 2026-05-25 16:39 UTC (permalink / raw)
To: linusw, brgl, heiko
Cc: linux-gpio, linux-rockchip, linux-arm-kernel, linux-kernel,
Marco Scardovi (scardracs)
This series fixes several resource leaks and teardown issues in the
rockchip gpio driver probe error paths and remove() callback.
The patches included in this series:
1. Convert the main clock (bank->clk) to use devm_clk_get_enabled(),
simplifying the code and fixing an existing clk_put() leak.
2. Fix several teardown bugs and resource leaks:
- Fix a reference leak for the debounce clock (bank->db_clk)
by properly releasing it with a devm action.
- Fix a potential kernel panic on module unload by unregistering the
chained IRQ handler (rockchip_irq_demux) in the remove() callback.
- Fix an IRQ domain and generic chip memory leak by calling
irq_domain_remove() during module unload.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
Marco Scardovi (scardracs) (2):
gpio: rockchip: convert bank->clk to devm_clk_get_enabled()
gpio: rockchip: fix teardown bugs and resource leaks
drivers/gpio/gpio-rockchip.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH net-next] net: airoha: bind WLAN-bound flows on PPE driver L2 cache miss
From: Lorenzo Bianconi @ 2026-05-25 16:32 UTC (permalink / raw)
To: Jihong Min
Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-arm-kernel, linux-mediatek,
linux-kernel
In-Reply-To: <82d0e4a5-76b1-4c86-a153-2500c60e8063@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 12161 bytes --]
>
>
> On 5/26/26 00:19, Lorenzo Bianconi wrote:
> >>
> >>
> >> On 5/25/26 17:09, Lorenzo Bianconi wrote:
> >>>> The Linux bridge FDB can resolve a destination station to WDMA even when
> >>>> the Airoha PPE driver's L2 offload cache has no entry for that MAC pair.
> >>>> The normal bind path only checks the PPE driver's L2 offload cache, so an
> >>>> unbound PPE hit for WLAN egress can stay unbound even though the bridge
> >>>> already knows the right output path, unless a later offload event fills
> >>>> that PPE driver cache.
> >>>>
> >>>> This matters for bridge-visible WLAN egress, such as wired-to-WLAN
> >>>> forwarding or WLAN peer forwarding across another BSS, radio or MLO link.
> >>>> Same-link or same-radio intra-BSS forwarding can stay inside the WLAN
> >>>> datapath and is not covered.
> >>>
> >>> Hi Jihong,
> >>
> >> Hi, Lorenzo.
> >>
> >>>
> >>> In order to offload L2 flows, I assume you are using the OpenWrt bridger
> >>> package, right?
> >>
> >> Actually, no.
> >>
> >> I am using Fanboy's OpenWrt `test` build for the Lumen W1700K2,
> >> together with several of my other patches. It does not include the
> >> bridger package specifically. It now uses native nft-based offloading
> >> with `kmod-br-netfilter`.
> >
> > according to my understanding this is not merged yet, right? I guess the
> > patches should be based on official/accepted code.
> >
>
> Yes, you are right. I just checked current net-next and realized that the
> bridge/L2 nft_flow_offload pieces used in my test environment are not
> merged upstream yet. Sorry, I should have checked this before submitting
> the patch.
ack, no worries ;)
>
> Since I cannot properly test the current upstream code plus bridger at the
> moment, I will put this patch on hold.
>
> I will also check whether this should instead be fixed on the
> nft_flow_offload side.
ack.
Regards,
Lorenzo
>
>
> Sincerely,
> Jihong Min
>
> >>
> >>> IIUC the issue you want to resolve is we are not adding PPE L2 entries for
> >>> the specified cases (same-link or same-radio intra-BSS forwarding), correct?
> >>
> >> No. As written in the patch message, this specifically addresses
> >> bridge-visible WLAN egress, such as:
> >>
> >> 1. wired-to-WLAN forwarding
> >> 2. WLAN peer forwarding across another BSS, radio, or MLO link
> >>
> >> Same-link or same-radio intra-BSS forwarding can stay inside the WLAN
> >> datapath and is not covered by this patch, although it did show poor
> >> performance, whether that is due to shared airtime or not. That case
> >> appears to belong to the Wi-Fi stack/driver datapath, such as the
> >> mac80211/mt76/mt7996 path, rather than to this Airoha PPE fallback path.
> >
> > according to my understanding the l2 nft-based offloading solution should
> > add the missing info to PPE flow-table. As I pointed out, it should be
> > in-sync with hw flow-table. It seems a bug in the nft code to me.
> >
> >>
> >>> Using this approach, we are breaking the assumption PPE flow-table and hw
> >>> flow-table are in sync. If the issue is the one described above, why not
> >>> fixing the problem directly in the bridger package?
> >>
> >> Again, this problem exists in an environment without bridger.
> >
> > In order to offload L2 traffic bridger is mandatory. Do you mean the issue
> > occurs even on L3 scenario?
> >
> >>
> >>> Moreover, I see you developed the patch using Codex:gpt-5.5. Have you tested it
> >>> on a real hw?
> >>
> >> Yes. This has been tested on my Lumen W1700K2 with the environment
> >> described above. MLO Wi-Fi P2P communication and some wired-to-WLAN
> >> cases were indeed left unbound by PPE. CPU usage was high, and the
> >> unbound throughput was close to 50% of what this patch achieves now.
> >
> > ack
> >
> > Regards,
> > Lorenzo
> >
> >>
> >>>
> >>> Some comments inline.
> >>>
> >>> Regards,
> >>> Lorenzo
> >>>>>
> >>>> Before touching the PPE table, resolve the destination MAC through the
> >>>> bridge device above the ingress netdev. If the PPE driver's L2 offload
> >>>> cache lookup misses, bind the hardware flow to the resolved CDM4/WDMA
> >>>> path.
> >>>>
> >>>> Assisted-by: Codex:gpt-5.5
> >>>> Signed-off-by: Jihong Min <hurryman2212@gmail.com>
> >>>> ---
> >>>> drivers/net/ethernet/airoha/airoha_ppe.c | 138 +++++++++++++++++++----
> >>>> 1 file changed, 119 insertions(+), 19 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
> >>>> index 26da519236bf..ea932e6d87f6 100644
> >>>> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> >>>> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
> >>>> @@ -803,65 +803,163 @@ static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
> >>>> }
> >>>>
> >>>> static int
> >>>> -airoha_ppe_foe_commit_subflow_entry(struct airoha_ppe *ppe,
> >>>> - struct airoha_flow_table_entry *e,
> >>>> - u32 hash, bool rx_wlan)
> >>>> +airoha_ppe_foe_commit_subflow(struct airoha_ppe *ppe,
> >>>> + const struct airoha_foe_entry *bridge,
> >>>
> >>> maybe l2_hwe instead of bridge?
> >>>
> >>>> + u32 hash, bool rx_wlan)
> >>>> {
> >>>> u32 mask = AIROHA_FOE_IB1_BIND_PACKET_TYPE | AIROHA_FOE_IB1_BIND_UDP;
> >>>> struct airoha_foe_entry *hwe_p, hwe;
> >>>> - struct airoha_flow_table_entry *f;
> >>>> int type;
> >>>>
> >>>> hwe_p = airoha_ppe_foe_get_entry_locked(ppe, hash);
> >>>> if (!hwe_p)
> >>>> return -EINVAL;
> >>>>
> >>>> - f = kzalloc_obj(*f, GFP_ATOMIC);
> >>>> - if (!f)
> >>>> - return -ENOMEM;
> >>>> -
> >>>> - hlist_add_head(&f->l2_subflow_node, &e->l2_flows);
> >>>> - f->type = FLOW_TYPE_L2_SUBFLOW;
> >>>> - f->hash = hash;
> >>>> -
> >>>> memcpy(&hwe, hwe_p, sizeof(*hwe_p));
> >>>> - hwe.ib1 = (hwe.ib1 & mask) | (e->data.ib1 & ~mask);
> >>>> + hwe.ib1 = (hwe.ib1 & mask) | (bridge->ib1 & ~mask);
> >>>>
> >>>> type = FIELD_GET(AIROHA_FOE_IB1_BIND_PACKET_TYPE, hwe.ib1);
> >>>> if (type >= PPE_PKT_TYPE_IPV6_ROUTE_3T) {
> >>>> - memcpy(&hwe.ipv6.l2, &e->data.bridge.l2, sizeof(hwe.ipv6.l2));
> >>>> - hwe.ipv6.ib2 = e->data.bridge.ib2;
> >>>> + memcpy(&hwe.ipv6.l2, &bridge->bridge.l2,
> >>>> + sizeof(hwe.ipv6.l2));
> >>>> + hwe.ipv6.ib2 = bridge->bridge.ib2;
> >>>> /* setting smac_id to 0xf instruct the hw to keep original
> >>>> * source mac address
> >>>> */
> >>>> hwe.ipv6.l2.src_mac_hi = FIELD_PREP(AIROHA_FOE_MAC_SMAC_ID,
> >>>> 0xf);
> >>>> } else {
> >>>> - memcpy(&hwe.bridge.l2, &e->data.bridge.l2,
> >>>> + memcpy(&hwe.bridge.l2, &bridge->bridge.l2,
> >>>> sizeof(hwe.bridge.l2));
> >>>> - hwe.bridge.ib2 = e->data.bridge.ib2;
> >>>> + hwe.bridge.ib2 = bridge->bridge.ib2;
> >>>> if (type == PPE_PKT_TYPE_IPV4_HNAPT)
> >>>> memcpy(&hwe.ipv4.new_tuple, &hwe.ipv4.orig_tuple,
> >>>> sizeof(hwe.ipv4.new_tuple));
> >>>> }
> >>>>
> >>>> - hwe.bridge.data = e->data.bridge.data;
> >>>> - airoha_ppe_foe_commit_entry(ppe, &hwe, hash, rx_wlan);
> >>>> + hwe.bridge.data = bridge->bridge.data;
> >>>> +
> >>>> + return airoha_ppe_foe_commit_entry(ppe, &hwe, hash, rx_wlan);
> >>>> +}
> >>>> +
> >>>> +static int
> >>>> +airoha_ppe_foe_commit_subflow_entry(struct airoha_ppe *ppe,
> >>>> + struct airoha_flow_table_entry *e,
> >>>> + u32 hash, bool rx_wlan)
> >>>> +{
> >>>> + struct airoha_flow_table_entry *f;
> >>>> + int err;
> >>>> +
> >>>> + f = kzalloc_obj(*f, GFP_ATOMIC);
> >>>> + if (!f)
> >>>> + return -ENOMEM;
> >>>> +
> >>>> + err = airoha_ppe_foe_commit_subflow(ppe, &e->data, hash, rx_wlan);
> >>>> + if (err) {
> >>>> + kfree(f);
> >>>> + return err;
> >>>> + }
> >>>> +
> >>>> + hlist_add_head(&f->l2_subflow_node, &e->l2_flows);
> >>>> + f->type = FLOW_TYPE_L2_SUBFLOW;
> >>>> + f->hash = hash;
> >>>>
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +static bool
> >>>> +airoha_ppe_foe_prepare_wdma_subflow_dev(struct airoha_ppe *ppe,
> >>>> + struct net_device *dev,
> >>>> + struct airoha_flow_data *data,
> >>>> + struct airoha_foe_entry *hwe)
> >>>> +{
> >>>> + u32 pse_port;
> >>>> + int err;
> >>>> +
> >>>> + err = airoha_ppe_foe_entry_prepare(ppe->eth, hwe, dev,
> >>>> + PPE_PKT_TYPE_BRIDGE, data, 0);
> >>>> + if (err)
> >>>> + return false;
> >>>> +
> >>>> + pse_port = FIELD_GET(AIROHA_FOE_IB2_PSE_PORT, hwe->bridge.ib2);
> >>>> + if (pse_port != FE_PSE_PORT_CDM4)
> >>>> + return false;
> >>>> +
> >>>> + return true;
> >>>
> >>> return pse_port == FE_PSE_PORT_CDM4;
> >>>
> >>>> +}
> >>>> +
> >>>> +static struct net_device *
> >>>> +airoha_ppe_foe_get_bridge_master(struct net_device *dev)
> >>>> +{
> >>>> + struct net_device *master = NULL;
> >>>> +
> >>>> + rcu_read_lock();
> >>>> + master = netdev_master_upper_dev_get_rcu(dev);
> >>>> + if (master && netif_is_bridge_master(master))
> >>>> + dev_hold(master);
> >>>> + else
> >>>> + master = NULL;
> >>>> + rcu_read_unlock();
> >>>> +
> >>>> + return master;
> >>>> +}
> >>>> +
> >>>> +static bool
> >>>> +airoha_ppe_foe_prepare_wdma_subflow(struct airoha_ppe *ppe,
> >>>> + struct sk_buff *skb,
> >>>> + struct airoha_foe_entry *hwe)
> >>>> +{
> >>>> + struct ethhdr *eh = eth_hdr(skb);
> >>>> + struct airoha_flow_data data = {};
> >>>> + struct net_device *master;
> >>>> +
> >>>> + if (!is_valid_ether_addr(eh->h_source) ||
> >>>> + !is_valid_ether_addr(eh->h_dest))
> >>>> + return false;
> >>>> +
> >>>> + ether_addr_copy(data.eth.h_dest, eh->h_dest);
> >>>> + ether_addr_copy(data.eth.h_source, eh->h_source);
> >>>> +
> >>>> + if (!skb->dev)
> >>>> + return false;
> >>>> +
> >>>> + /* WLAN egress unbound hits can arrive before flowtable creates the
> >>>> + * L2 master flow normally used for subflow binding. Resolve only
> >>>> + * through the bridge master so dev_fill_forward_path() must use the
> >>>> + * bridge FDB for the destination MAC. Calling the ingress AP netdev
> >>>> + * directly can describe the source station's WDMA path and would
> >>>> + * corrupt Wi-Fi-to-wired flows whose real egress is not WDMA.
> >>>> + */
> >>>> + master = airoha_ppe_foe_get_bridge_master(skb->dev);
> >>>> + if (!master)
> >>>> + return false;
> >>>> +
> >>>> + if (airoha_ppe_foe_prepare_wdma_subflow_dev(ppe, master, &data,
> >>>> + hwe)) {
> >>>> + dev_put(master);
> >>>> + return true;
> >>>> + }
> >>>> +
> >>>> + dev_put(master);
> >>>> + return false;
> >>>
> >>> maybe something like:
> >>>
> >>> ret = airoha_ppe_foe_prepare_wdma_subflow_dev();
> >>> dev_put(master);
> >>>
> >>> return ret;
> >>>
> >>>> +}
> >>>> +
> >>>> static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe,
> >>>> struct sk_buff *skb,
> >>>> u32 hash, bool rx_wlan)
> >>>> {
> >>>> + struct airoha_foe_entry wdma_hwe = {};
> >>>> struct airoha_flow_table_entry *e;
> >>>> struct airoha_foe_bridge br = {};
> >>>> struct airoha_foe_entry *hwe;
> >>>> bool commit_done = false;
> >>>> + bool wdma_ready = false;
> >>>> struct hlist_node *n;
> >>>> u32 index, state;
> >>>>
> >>>> + wdma_ready = airoha_ppe_foe_prepare_wdma_subflow(ppe, skb,
> >>>> + &wdma_hwe);
> >>>> +
> >>>> spin_lock_bh(&ppe_lock);
> >>>>
> >>>> hwe = airoha_ppe_foe_get_entry_locked(ppe, hash);
> >>>> @@ -899,6 +997,8 @@ static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe,
> >>>> airoha_l2_flow_table_params);
> >>>> if (e)
> >>>> airoha_ppe_foe_commit_subflow_entry(ppe, e, hash, rx_wlan);
> >>>> + else if (wdma_ready)
> >>>> + airoha_ppe_foe_commit_subflow(ppe, &wdma_hwe, hash, rx_wlan);
> >>>> unlock:
> >>>> spin_unlock_bh(&ppe_lock);
> >>>> }
> >>>> --
> >>>> 2.53.0
> >>>>
> >>
> >> All inline code-style review comments will be addressed in the next
> >> submission of the patch set, together with the responses to Sashiko's
> >> review, if any.
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ 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