* Re: [RFC PATCH net-next 1/7] xfrm: allow packet offload drivers to own transmit
From: sashiko-bot @ 2026-06-15 4:58 UTC (permalink / raw)
To: Jihong Min; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260614040032.1567994-2-hurryman2212@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypassing `xfrm_dev_offload_ok()` allows packets to be transmitted without applying mandatory outer IPsec transformations for nested SAs.
- [High] Unchecked dereference of `x->xso.dev` races with asynchronous device unregistration, leading to a deterministic kernel panic.
--
commit c38a10fca161e28cf54507d8d670ee75e47687df
Author: Jihong Min <hurryman2212@gmail.com>
xfrm: allow packet offload drivers to own transmit
Packet offload drivers can currently program state and validate whether an
skb can be offloaded, but they cannot take ownership of a packet that needs
driver-specific transmit preparation before the regular XFRM output path
continues. This commit adds an optional xdo_dev_packet_xmit callback for
drivers to consume the skb and return the final transmit status.
[ ... ]
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index cc35c2fcbbe09..9f11559b02210 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -770,6 +770,17 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
> }
>
> if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> +#ifdef CONFIG_XFRM_OFFLOAD
> + const struct xfrmdev_ops *ops;
> +#endif
> +
> +#ifdef CONFIG_XFRM_OFFLOAD
> + ops = x->xso.dev->xfrmdev_ops;
[Severity: High]
Can this dereference of x->xso.dev crash if the device is concurrently
unregistered?
When a device is unregistered, xfrm_dev_state_flush() calls
xfrm_dev_state_free(), which clears x->xso.dev to null before updating
x->xso.type.
If xfrm_output() sees x->xso.type == XFRM_DEV_OFFLOAD_PACKET but reads
x->xso.dev after it was cleared, won't this result in a null pointer
dereference for in-flight packets?
> + /* Callback validates, consumes skb and returns final TX status. */
> + if (ops && ops->xdo_dev_packet_xmit)
> + return ops->xdo_dev_packet_xmit(skb, x);
[Severity: Critical]
Does this early return bypass outer IPsec transformations for nested
associations?
By returning early here, the code skips the call to xfrm_dev_offload_ok(),
which performs validation including checking !xdst->child->xfrm. This check
normally ensures there are no nested IPsec transformations pending.
If a packet has nested IPsec associations, such as transport over tunnel,
would the hardware driver transmit the packet out of the physical port
applying only the innermost transformation, leaving the outer transformations
completely bypassed?
> +#endif
> +
> if (!xfrm_dev_offload_ok(skb, x)) {
> XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> kfree_skb(skb);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=1
^ permalink raw reply
* Re: [RFC PATCH net-next 5/7] net: airoha: add QDMA support for SOE packets
From: sashiko-bot @ 2026-06-15 4:58 UTC (permalink / raw)
To: Jihong Min; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260614040032.1567994-6-hurryman2212@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Accessing `skb` after handing it off to the QDMA hardware TX ring causes a Use-After-Free vulnerability.
Pre-existing issues:
- [High] Using `dma_map_single()` with `skb_frag_address()` instead of `skb_frag_dma_map()` causes mapping failures or kernel panics on systems with high memory.
- [High] Unnecessarily re-evaluating `dev` and `netdev` from scattered RX descriptors causes invalid metadata extraction, resulting in misrouted or dropped packets.
--
commit a3fa3b0477739d5fa5e7176d3210e1a5b7b0f8df
Author: Jihong Min <hurryman2212@gmail.com>
net: airoha: add QDMA support for SOE packets
Add QDMA RX/TX plumbing for SOE packets, including the SOE RX ring,
coherent RX slots, SOE completion decoding, and the private QDMA submit
helper used by the SOE provider. Wire the Ethernet netdev feature and
lifetime hooks through the SOE stubs.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -822,92 +822,210 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
[ ... ]
> len = FIELD_GET(QDMA_DESC_LEN_MASK, desc_ctrl);
> - data_len = q->skb ? AIROHA_RX_LEN(q->buf_size) : e->dma_len;
> + data_len = q->skb && !q->rx_coherent ?
> + AIROHA_RX_LEN(q->buf_size) :
> + e->dma_len;
> if (!len || data_len < len)
> goto free_frag;
>
> - dev = airoha_qdma_get_gdm_dev(eth, desc);
> - if (IS_ERR(dev))
> - goto free_frag;
> + msg0 = le32_to_cpu(READ_ONCE(desc->msg0));
> + msg1 = le32_to_cpu(READ_ONCE(desc->msg1));
> + msg2 = le32_to_cpu(READ_ONCE(desc->msg2));
> +#if IS_REACHABLE(CONFIG_NET_AIROHA_SOE)
> + soe_pkt = partial_soe || airoha_qdma_rx_is_soe(msg0);
> +#endif
> + if (!soe_pkt) {
> + dev = airoha_qdma_get_gdm_dev(eth, desc);
> + if (IS_ERR(dev))
> + goto free_frag;
> + netdev = netdev_from_priv(dev);
> + rx_dev = netdev;
> + }
[Severity: High]
This is a pre-existing issue, but does re-evaluating dev and netdev on every
descriptor in a scattered chain overwrite the valid metadata from the first
descriptor?
If the hardware only writes msg0 and msg1 to the first descriptor and
zeroes them out in subsequent ones, re-evaluating here might corrupt DSA
routing and flow hashing by using zeroed data.
[ ... ]
> @@ -2176,6 +2552,110 @@ int airoha_get_fe_port(struct airoha_gdm_dev *dev)
[ ... ]
> +int airoha_qdma_xmit_skb(struct airoha_gdm_dev *dev, struct sk_buff *skb,
> + u32 msg0, u32 msg1, u32 msg2)
> +{
[ ... ]
> + while (true) {
> + struct airoha_qdma_desc *desc = &q->desc[index];
> + skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
> + dma_addr_t addr;
> + u32 val;
> +
> + addr = dma_map_single(netdev->dev.parent, data, len,
> + DMA_TO_DEVICE);
[Severity: High]
This is a pre-existing issue copied into this new function (and also present
in airoha_dev_xmit), but does using dma_map_single() with skb_frag_address()
risk mapping failures or kernel panics on systems with high memory?
On architectures with HIGHMEM, page_address() returns NULL for pages not
mapped into the kernel's virtual address space. Calling dma_map_single() on
a NULL address could map garbage or trigger a kernel oops. Should this use
skb_frag_dma_map() instead?
[ ... ]
> @@ -2293,13 +2779,24 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
> q->txq_stopped = true;
> }
>
> - if (netif_xmit_stopped(txq) || !netdev_xmit_more())
> + if (netif_xmit_stopped(txq) || !netdev_xmit_more() ||
> + soe_decrypt_skb)
> airoha_qdma_rmw(qdma, REG_TX_CPU_IDX(qid),
> TX_RING_CPU_IDX_MASK,
> FIELD_PREP(TX_RING_CPU_IDX_MASK, index));
>
> spin_unlock_bh(&q->lock);
>
> + /* SOE decrypt flow binding needs the final egress netdev and QDMA
> + * descriptor context. The SOE RX path marks only candidate packets; bind
> + * only after the current plaintext packet has been queued and kicked so
> + * the newly bound decrypt entry cannot race the CPU-transmitted packet.
> + */
> +#if IS_REACHABLE(CONFIG_NET_AIROHA_SOE)
> + if (soe_decrypt_skb)
> + airoha_ppe_soe_xmit_skb(&dev->eth->ppe->dev, skb, netdev);
> +#endif
> +
> return NETDEV_TX_OK;
[Severity: High]
Could this cause a use-after-free by accessing the skb after it has been
handed off to the hardware?
Once the transmission queue spinlock is released and the hardware doorbell
is kicked via airoha_qdma_rmw(), the skb ownership is fully transferred.
The transmission could complete concurrently on another CPU, which would
invoke dev_kfree_skb_any(skb) in the TX completion handler before
airoha_ppe_soe_xmit_skb() is able to safely access skb->mark.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=5
^ permalink raw reply
* Re: [RFC PATCH net-next 3/7] arm64: dts: airoha: add EN7581 SOE node
From: sashiko-bot @ 2026-06-15 4:58 UTC (permalink / raw)
To: Jihong Min; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260614040032.1567994-4-hurryman2212@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing `x->xso.dev` without a NULL check in `xfrm_output` can lead to a kernel panic.
--
commit c38a10fca161e28cf54507d8d670ee75e47687df
Author: Jihong Min <hurryman2212@gmail.com>
xfrm: allow packet offload drivers to own transmit
This commit adds an optional xdo_dev_packet_xmit() callback for packet
offload drivers. It allows devices to perform driver-specific transmit
preparation instead of continuing through the regular XFRM output path.
Drivers implementing this callback will consume the skb and return the
final transmit status.
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index cc35c2fcbbe09..9f11559b02210 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -770,6 +770,17 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
> }
>
> if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> +#ifdef CONFIG_XFRM_OFFLOAD
> + const struct xfrmdev_ops *ops;
> +#endif
> +
> +#ifdef CONFIG_XFRM_OFFLOAD
> + ops = x->xso.dev->xfrmdev_ops;
[Severity: High]
Is it possible for x->xso.dev to be NULL here?
During device unregistration, xfrm_dev_state_free() clears xso->dev to NULL
(via WRITE_ONCE) before updating xso->type to XFRM_DEV_OFFLOAD_UNSPECIFIED.
Because xfrm_output() processes packets concurrently without locking, could
a data race occur where the transmit path observes xso.type as
XFRM_DEV_OFFLOAD_PACKET but then reads xso.dev as NULL?
If this happens, wouldn't evaluating ops = x->xso.dev->xfrmdev_ops cause a
NULL pointer dereference regression?
The adjacent xfrm_dev_offload_ok() function seems to anticipate this
concurrent modification by explicitly checking for a NULL dev. Should a
similar NULL check be added here?
> + /* Callback validates, consumes skb and returns final TX status. */
> + if (ops && ops->xdo_dev_packet_xmit)
> + return ops->xdo_dev_packet_xmit(skb, x);
> +#endif
> +
> if (!xfrm_dev_offload_ok(skb, x)) {
> XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> kfree_skb(skb);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=3
^ permalink raw reply
* Re: [RFC PATCH net-next 6/7] net: airoha: add PPE support for SOE flows
From: sashiko-bot @ 2026-06-15 4:58 UTC (permalink / raw)
To: Jihong Min; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260614040032.1567994-7-hurryman2212@gmail.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [Critical] Type confusion from blindly casting `f->cookie` allows unprivileged users to crash the kernel via TC flower.
- [High] The driver destructively overwrites `skb->mark` with private hardware metadata, breaking user-defined firewall marks.
- [High] Lack of IPv4 fragment validation in `airoha_ppe_soe_fill_inner_ipv4_data` leads to garbage L4 port extraction.
- [High] Lockless updates of the `soe_meta` array cause data races and torn reads, corrupting IPsec state.
- [High] Overlapping identical memory pointers are passed to `memcpy`, triggering undefined behavior.
- [High] Hardware polling on 16K+ table entries inside a spinlock triggers CPU soft lockups.
- [Medium] Massive contiguous memory allocations via `devm_kzalloc` risk persistent driver initialization failures.
Pre-existing issues:
- [High] A shadowed error variable in `airoha_ppe_flush_entries` silently masks hardware initialization failures.
--
commit 00c38592fc258c1ad98cdbaaadb9925cf653ba43
Author: Jihong Min <hurryman2212@gmail.com>
net: airoha: add PPE support for SOE flows
Add PPE metadata handling for SOE flows so decrypted packets can carry
their original FOE/SA context until the normal egress path is known, and
so XFRM flowtable entries can be programmed with SOE SA and hop
information.
> diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
> index 91bcc55a6ac67..faa5f04d4c7b3 100644
> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
> @@ -578,6 +604,57 @@ static int airoha_ppe_foe_entry_set_ipv6_tuple(struct airoha_foe_entry *hwe,
> +static int airoha_ppe_soe_fill_inner_ipv4_data(struct sk_buff *skb,
> + struct airoha_flow_data *data,
> + int *type, int *l4proto)
> +{
> + unsigned int ip_offset = ETH_HLEN;
> + union {
> + struct tcphdr tcp;
> + struct udphdr udp;
> + } ports;
> + struct iphdr iph_buf, *iph;
> + unsigned int l4_offset;
> + struct udphdr *udp;
> + struct tcphdr *tcp;
> +
> + if (skb_headlen(skb) < ETH_HLEN)
> + return -EINVAL;
> +
> + memcpy(&data->eth, skb->data, ETH_HLEN);
> + if (data->eth.h_proto != htons(ETH_P_IP))
> + return -EAFNOSUPPORT;
> +
> + iph = skb_header_pointer(skb, ip_offset, sizeof(iph_buf), &iph_buf);
> + if (!iph || iph->ihl < 5 || iph->version != 4)
> + return -EINVAL;
> +
> + l4_offset = ip_offset + iph->ihl * 4;
[Severity: High]
Does this code handle IPv4 fragments safely?
If iph->frag_off indicates a trailing fragment, the L4 header is not present.
Because there is no ip_is_fragment(iph) check before calculating l4_offset,
the function will read arbitrary payload data, misinterpret it as ports,
and program incorrect tuples into the hardware offload tables, leading to
corrupted flows and dropped fragments.
[ ... ]
> @@ -868,16 +1096,129 @@ airoha_ppe_foe_commit_subflow_entry(struct airoha_ppe *ppe,
> +static void airoha_ppe_soe_meta_store(struct airoha_ppe_soe_meta *meta,
> + u32 key_hash, u16 foe_hash, u8 sa_index,
> + u8 hop)
> {
> + u8 seen = 1;
> +
> + if (READ_ONCE(meta->valid) &&
> + READ_ONCE(meta->key_hash) == key_hash &&
> + READ_ONCE(meta->foe_hash) == foe_hash &&
> + READ_ONCE(meta->sa_index) == sa_index &&
> + READ_ONCE(meta->hop) == hop &&
> + time_before_eq(jiffies, READ_ONCE(meta->expires)))
> + seen = min_t(u8, READ_ONCE(meta->seen) + 1, U8_MAX);
> +
> + WRITE_ONCE(meta->key_hash, key_hash);
> + WRITE_ONCE(meta->foe_hash, foe_hash);
> + WRITE_ONCE(meta->sa_index, sa_index);
> + WRITE_ONCE(meta->hop, hop);
> + WRITE_ONCE(meta->seen, seen);
> + WRITE_ONCE(meta->expires,
> + jiffies + msecs_to_jiffies(AIROHA_PPE_SOE_META_TIMEOUT_MS));
> + WRITE_ONCE(meta->valid, 1);
[Severity: High]
Can these lockless updates cause torn reads or data races?
Because there are no CPU memory barriers (like smp_store_release) and no
locks to prevent concurrent hash bucket collisions, writes are performed
sequentially.
Does this mean valid = 1 could be reordered before the data is visible to
other CPUs?
> +}
> +
> +void airoha_ppe_soe_mark_skb(struct airoha_ppe_dev *dev, struct sk_buff *skb,
> + u16 hash, u8 sa_index, u8 hop)
> +{
> + struct airoha_ppe *ppe;
> + u32 ppe_hash_mask;
> +
> + if (!dev || !skb)
> + return;
> +
> + ppe = dev->priv;
> + if (!ppe || !ppe->soe_meta)
> + return;
> +
> + ppe_hash_mask = airoha_ppe_get_total_num_entries(ppe) - 1;
> + if (hash > ppe_hash_mask)
> + return;
> +
> + /* SOE decrypt completion is CPU-visible before normal routing has
> + * selected the plaintext egress netdev. Keep the original encrypted FOE
> + * hash and SA hop briefly on the skb so airoha_dev_xmit() can finish
> + * the PPE entry once the final egress descriptor is known.
> + */
> + airoha_ppe_soe_meta_store(&ppe->soe_meta[hash], hash, hash, sa_index,
> + hop);
> + ppe->foe_check_time[hash] = 0;
> +
> + skb->mark &= ~(AIROHA_PPE_SOE_MARK_MAGIC_MASK |
> + AIROHA_PPE_SOE_MARK_HASH_MASK);
> + skb->mark |= AIROHA_PPE_SOE_MARK_MAGIC |
> + FIELD_PREP(AIROHA_PPE_SOE_MARK_HASH_MASK, hash);
[Severity: High]
Does this destructively overwrite the user-space visible skb->mark?
Network drivers are forbidden from repurposing the global UAPI skb->mark
field for internal cross-stack state tracking.
Could this destroy any existing fwmark applied by userspace
(via iptables, nftables, or tc), breaking policy routing and firewall rules?
> +}
> +
> +bool airoha_ppe_soe_skb_marked(struct sk_buff *skb)
> +{
> + return skb && ((skb->mark & AIROHA_PPE_SOE_MARK_MAGIC_MASK) ==
> + AIROHA_PPE_SOE_MARK_MAGIC);
> +}
> +
> +void airoha_ppe_soe_xmit_skb(struct airoha_ppe_dev *dev, struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> + struct airoha_foe_entry entry, tmpl, *hwe;
> + struct airoha_flow_data data = {};
> + struct airoha_ppe_soe_meta *meta;
> + u32 ppe_hash_mask, key_hash;
> + struct airoha_gdm_dev *gdm;
> + struct airoha_ppe *ppe;
> + unsigned long expires;
> + u16 hash;
> + int err, l4proto, type;
> + u8 sa_index, hop;
> + u8 seen;
> +
> + if (!dev || !skb || !netdev)
> + return;
> +
> + if ((skb->mark & AIROHA_PPE_SOE_MARK_MAGIC_MASK) !=
> + AIROHA_PPE_SOE_MARK_MAGIC)
> + return;
> +
> + ppe = dev->priv;
> + if (!ppe || !ppe->soe_meta)
> + goto clear_mark;
> +
> + ppe_hash_mask = airoha_ppe_get_total_num_entries(ppe) - 1;
> + key_hash = FIELD_GET(AIROHA_PPE_SOE_MARK_HASH_MASK, skb->mark);
> + if (key_hash > ppe_hash_mask)
> + goto clear_mark;
> +
> + meta = &ppe->soe_meta[key_hash];
> + if (!READ_ONCE(meta->valid))
> + goto clear_mark;
[Severity: High]
Are these reads gated by valid without acquire barriers?
Without smp_load_acquire, a reading CPU can observe valid == 1 while reading
torn, mixed data from two racing packets. This programs the hardware with an
incorrect SA index, breaking IPsec tunnel isolation.
[ ... ]
> +clear_mark:
> + skb->mark &= ~(AIROHA_PPE_SOE_MARK_MAGIC_MASK |
> + AIROHA_PPE_SOE_MARK_HASH_MASK);
[Severity: High]
Is the hijacked mark consumed and zeroed out on the transmit path here?
This finalizes the loss of the original fwmark.
> +}
> +
> +void airoha_ppe_soe_flush_sa(struct airoha_ppe *ppe, u8 sa_index)
> +{
> + u32 num_entries, hash;
> +
> + if (!ppe)
> + return;
> +
> + num_entries = airoha_ppe_get_total_num_entries(ppe);
> +
> + spin_lock_bh(&ppe_lock);
[Severity: High]
Is it safe to hold this spinlock continuously across a massive loop that
interacts with hardware?
When flushing IPsec state, this loops over every PPE table entry (up to
24,576 entries) while continuously holding a single spin_lock_bh(&ppe_lock).
During the iteration of SRAM entries, airoha_ppe_foe_get_entry_locked()
issues MMIO reads via read_poll_timeout_atomic(), which can delay up to
100us per entry.
Multiplying this delay by thousands of loop iterations can block the CPU for
tens to hundreds of milliseconds. Can this trigger watchdog soft lockup
warnings and drop unrelated network traffic?
> + for (hash = 0; hash < num_entries; hash++) {
> + struct airoha_foe_entry *hwe;
> + u32 state, type;
> +
> + hwe = airoha_ppe_foe_get_entry_locked(ppe, hash);
> + if (!hwe)
> + continue;
> +
> + state = FIELD_GET(AIROHA_FOE_IB1_BIND_STATE, hwe->ib1);
> + if (state != AIROHA_FOE_STATE_BIND)
> + continue;
> +
> + type = FIELD_GET(AIROHA_FOE_IB1_BIND_PACKET_TYPE, hwe->ib1);
> + if (type != PPE_PKT_TYPE_IPV4_HNAPT &&
> + type != PPE_PKT_TYPE_IPV4_ROUTE)
> + continue;
> +
> + if (!(hwe->ipv4.data & AIROHA_FOE_TUNNEL))
> + continue;
> +
> + if (FIELD_GET(AIROHA_FOE_ACTDP, hwe->ipv4.data) != sa_index)
> + continue;
> +
> + /* NAT-T data and IKE control both use UDP/4500. A stale SOE
> + * bound entry can otherwise keep sending later IKE_AUTH packets
> + * to the SOE path after the SA has been deleted.
> + */
> + hwe->ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
> + hwe->ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
> + AIROHA_FOE_STATE_INVALID);
> + airoha_ppe_foe_commit_entry(ppe, hwe, hash, false);
[Severity: High]
Does this pass overlapping identical memory pointers to memcpy(), triggering
undefined behavior?
The pointer returned by airoha_ppe_foe_get_entry_locked() points directly to
the ppe->foe array.
This exact pointer is then passed as hwe into airoha_ppe_foe_commit_entry(),
which recalculates the destination pointing to the exact same address:
struct airoha_foe_entry *hwe = ppe->foe + hash * sizeof(*hwe);
...
memcpy(&hwe->d, &e->d, sizeof(*hwe) - sizeof(hwe->ib1));
The C standard forbids overlapping pointers in memcpy(), which can trigger
fortify source panics or silent data corruption.
> + }
> + spin_unlock_bh(&ppe_lock);
> +}
[ ... ]
> @@ -1057,6 +1551,7 @@ static int airoha_ppe_entry_idle_time(struct airoha_ppe *ppe,
> static int airoha_ppe_flow_offload_replace(struct airoha_eth *eth,
> struct flow_cls_offload *f)
> {
> + const struct flow_offload_tuple *tuple = (const void *)f->cookie;
[Severity: Critical]
Could this lead to a kernel panic from a type confusion if the rule was
added via TC flower?
The driver accepts generic FLOW_CLS_REPLACE commands, which can come from the
Netfilter flowtable or TC flower. This blindly casts f->cookie to
struct flow_offload_tuple *.
However, if the rule was added via TC flower, cookie actually points to a
struct cls_fl_filter.
The driver then passes this invalid pointer to
airoha_ppe_foe_entry_set_soe_info(), which reads a garbage xmit_type.
If it coincidentally matches FLOW_OFFLOAD_XMIT_XFRM, the code calls
airoha_soe_xfrm_ppe_info(tuple->dst_cache), dereferencing a random garbage
pointer. This allows any user capable of configuring TC flower rules to
trigger an oops.
> struct flow_rule *rule = flow_cls_offload_flow_rule(f);
> struct airoha_flow_table_entry *e;
[ ... ]
> @@ -1350,16 +1851,26 @@ static int airoha_ppe_flow_offload_cmd(struct airoha_eth *eth,
> return -EOPNOTSUPP;
> }
>
> -static int airoha_ppe_flush_sram_entries(struct airoha_ppe *ppe)
> +static int airoha_ppe_flush_entries(struct airoha_ppe *ppe)
> {
> + u32 ppe_num_entries = airoha_ppe_get_total_num_entries(ppe);
> u32 sram_num_entries = airoha_ppe_get_total_sram_num_entries(ppe);
> struct airoha_foe_entry *hwe = ppe->foe;
> int i, err = 0;
>
> + memset(hwe, 0, ppe_num_entries * sizeof(*hwe));
> + if (ppe->foe_stats) {
> + u32 ppe_num_stats_entries =
> + airoha_ppe_get_total_num_stats_entries(ppe);
> +
> + memset(ppe->foe_stats, 0,
> + ppe_num_stats_entries * sizeof(*ppe->foe_stats));
> + }
> + dma_wmb();
> +
> for (i = 0; i < sram_num_entries; i++) {
> int err;
>
> - memset(&hwe[i], 0, sizeof(*hwe));
> err = airoha_ppe_foe_commit_sram_entry(ppe, i);
> if (err)
> break;
> }
>
> return err;
[Severity: High]
This is a pre-existing issue, but does this shadowed error variable silently
mask hardware initialization failures?
Inside this initialization loop, a block-scoped int err; shadows the
function-scoped err = 0;.
If airoha_ppe_foe_commit_sram_entry() fails (e.g., due to a hardware poll
timeout), the loop breaks, but the function returns the outer err which
remains 0.
Does this silently ignore hardware initialization errors, leaving the PPE
table in an inconsistent state?
> }
[ ... ]
> @@ -1601,12 +2143,20 @@ int airoha_ppe_init(struct airoha_eth *eth)
> return -ENOMEM;
> }
>
> - ppe->foe_check_time = devm_kzalloc(eth->dev, ppe_num_entries,
> - GFP_KERNEL);
> + ppe->foe_check_time =
> + devm_kzalloc(eth->dev,
> + ppe_num_entries * sizeof(*ppe->foe_check_time),
> + GFP_KERNEL);
> if (!ppe->foe_check_time)
> return -ENOMEM;
>
> - err = airoha_ppe_flush_sram_entries(ppe);
> + ppe->soe_meta = devm_kzalloc(eth->dev,
> + ppe_num_entries * sizeof(*ppe->soe_meta),
> + GFP_KERNEL);
[Severity: Medium]
Could this fail on systems with fragmented memory or high uptime?
Allocating nearly 600 KB (an order-8 contiguous allocation) without a
fallback is extremely prone to failure and will prevent the network interface
from probing. Could this use devm_kvzalloc() to allow virtual memory
fallbacks?
> + if (!ppe->soe_meta)
> + return -ENOMEM;
> +
> + err = airoha_ppe_flush_entries(ppe);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=6
^ permalink raw reply
* Re: [RFC PATCH net-next 7/7] net: airoha: add SOE XFRM packet offload support
From: sashiko-bot @ 2026-06-15 4:58 UTC (permalink / raw)
To: Jihong Min; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260614040032.1567994-8-hurryman2212@gmail.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [Critical] Asynchronous XFRM state GC causes a Use-After-Free of the conduit network device private data when an SA is offloaded via a DSA user port.
- [Critical] Forcefully clearing `sa->inflight` during SA teardown prematurely signals completion, causing packet misrouting and context UAF.
- [High] DSA proxy scan hardcodes `init_net`, causing `xfrmdev_ops` to leak on interfaces moved to other network namespaces.
- [High] Hardware-decrypted packets are injected into an Ethernet device via `netif_rx()` without a valid MAC header.
- [High] Out-of-place TX offload loses critical SKB metadata, breaking QoS and netfilter marks.
- [High] Cleared Security Associations remain active in hardware because `AIROHA_SOE_SA_CMD_VLD` is unconditionally set.
- [High] The `xdo_dev_state_free` callback performs sleeping operations, causing a kernel BUG when the XFRM state is released in softirq context.
- [High] RX offload is unconditionally broken for DSA user ports due to a strict `x->xso.dev != rx_dev` check and lack of DSA tag parsing.
- [High] Unverified RX packets have their protocol unconditionally overwritten and are irrevocably truncated, corrupting non-IPv4 traffic.
- [High] TX offload expects `skb->data` to point to the IP header, but it points to the MAC header, causing TX offload failure.
- [Medium] Inefficient DSA proxy scanning causes redundant O(N^2) netdev feature updates system-wide.
--
commit c8afd29ed4385ffd75d9f77b7cea01b7c16511f3
Author: Jihong Min <hurryman2212@gmail.com>
Subject: net: airoha: add SOE XFRM packet offload support
Add the EN7581 Secure Offload Engine provider. The provider programs ESP
SAs, exposes NETIF_F_HW_ESP through xfrmdev_ops, submits encrypt and
decrypt packets through the QDMA SOE path, and handles SOE completion
delivery.
> diff --git a/drivers/net/ethernet/airoha/airoha_soe.c b/drivers/net/ethernet/airoha/airoha_soe.c
> new file mode 100644
> index 0000000000000..3a240ed44d7fc
> --- /dev/null
> +++ b/drivers/net/ethernet/airoha/airoha_soe.c
[ ... ]
> +static int airoha_soe_program_sa_locked(struct airoha_soe *soe,
> + unsigned int index,
> + const struct airoha_soe_sa_cfg *cfg)
> +{
> + void __iomem *base = soe->base;
> +
> + writel(cfg->cmd | AIROHA_SOE_SA_CMD_VLD, base + AIROHA_SOE_SA_CMD);
[Severity: High]
Does the unconditional use of AIROHA_SOE_SA_CMD_VLD defeat the teardown
zeroing?
When an XFRM state is destroyed, airoha_soe_clear_sa_locked() attempts to
invalidate the slot by passing a zero-initialized airoha_soe_sa_cfg, but
this unconditionally applies the VLD bit before writing to the hardware
command register.
Could this leave the hardware SA slot active with an SPI of 0, allowing a
malicious packet carrying SPI 0 to match this slot and bypass security
checks?
[ ... ]
> +static void airoha_soe_abort_sa(struct airoha_soe_sa *sa)
> +{
> + LIST_HEAD(rx_queue);
> + LIST_HEAD(tx_queue);
> +
> + if (!sa)
> + return;
> +
> + spin_lock_bh(&sa->lock);
> + sa->dead = true;
> + airoha_soe_forget_rx_ctx_list(sa);
> + list_splice_init(&sa->tx_queue, &tx_queue);
> + list_splice_init(&sa->rx_queue, &rx_queue);
> + sa->inflight = 0;
> + complete(&sa->idle);
> + spin_unlock_bh(&sa->lock);
[Severity: Critical]
Does forcefully zeroing sa->inflight and signaling sa->idle destroy the
synchronization accounting while packets are still in hardware?
During XFRM state deletion, airoha_soe_abort_sa() breaks the synchronization
in airoha_soe_free_sa(), causing the wait timeout to return instantly. The
SA index is then reallocated to a new XFRM state while old packets are
still in flight.
When the hardware delivers the completion, airoha_soe_rx_skb() pops a
tracking context from the new SA's queue based solely on the reused index,
which applies incorrect XFRM states and corrupts context accounting.
[ ... ]
> +static void airoha_soe_free_sa(struct airoha_soe_sa *sa)
> +{
> + LIST_HEAD(rx_queue);
> + LIST_HEAD(tx_queue);
> + struct airoha_soe *soe;
> +
> + if (!sa)
> + return;
> +
> + soe = sa->soe;
> + airoha_soe_mark_sa_dead(sa);
> + if (!wait_for_completion_timeout(&sa->idle, AIROHA_SOE_SA_FREE_TIMEOUT))
> + dev_warn(soe->dev,
[Severity: High]
Is it safe to perform sleeping operations during XFRM state destruction?
The xdo_dev_state_free callback is executed synchronously from
__xfrm_state_destroy, which can be invoked in atomic or softirq context
when the last reference is dropped. This often happens when a packet is
dropped in the RX path and the network stack releases its routing entry.
Since airoha_soe_free_sa() waits for a completion and calls
synchronize_rcu(), executing these in softirq context would cause a
scheduling while atomic BUG.
[ ... ]
> +static int airoha_soe_prepare_ip_headers(struct sk_buff *skb)
> +{
> + unsigned int hdr_len;
> +
> + if (!pskb_may_pull(skb, 1))
> + return -EINVAL;
> +
> + switch (skb->data[0] & 0xf0) {
> + case 0x40:
> + hdr_len = sizeof(struct iphdr);
[Severity: High]
Will this incorrectly read the first byte of the MAC header instead of the
IP version field?
The airoha_soe_xmit() function serves as the xdo_dev_packet_xmit callback,
which is invoked by the core network stack immediately before handing the
packet to the driver's transmit routine. At this stage, the MAC header has
been added and the data pointer points to the Ethernet MAC header.
Reading the first byte checks the destination MAC address instead, causing
offload to randomly fail and drop packets unless the destination MAC
happens to start with 0x4 or 0x6.
[ ... ]
> +static bool airoha_soe_rx_parse_ipv4(struct sk_buff *skb,
> + struct airoha_soe_rx_info *info)
> +{
[ ... ]
> + if (skb->protocol != htons(ETH_P_IP)) {
> + if (!pskb_may_pull(skb, 1) || (skb->data[0] >> 4) != 4)
> + return false;
> +
> + skb->protocol = htons(ETH_P_IP);
> + }
[ ... ]
> +bool airoha_soe_rx_plain_skb(struct airoha_gdm_dev *dev, struct sk_buff *skb,
> + struct net_device *rx_dev, u16 foe_hash,
> + u32 foe_reason, bool foe_valid)
> +{
[ ... ]
> + skb_reset_network_header(skb);
> + if (!airoha_soe_rx_parse_ipv4(skb, &info))
> + return false;
> +
> + if (skb->len != info.packet_len && pskb_trim(skb, info.packet_len))
> + return false;
[Severity: High]
Can this corrupt non-IPv4 traffic by irrevocably truncating the packet?
In airoha_soe_rx_plain_skb(), the protocol is overwritten and the packet is
truncated via pskb_trim() before verifying that the packet belongs to an
offloaded ESP SA.
If a non-IPv4 packet has 0x4 in the upper nibble of its first byte,
airoha_soe_rx_parse_ipv4() treats it as IPv4, modifies the protocol, and
truncates it. If xfrm_input_state_lookup() later fails, the corrupted
packet is returned to the network stack, leading to stack confusion.
[ ... ]
> + daddr.a4 = ip_hdr(skb)->daddr;
> + x = xfrm_input_state_lookup(dev_net(rx_dev), skb->mark, &daddr,
> + info.spi, IPPROTO_ESP, AF_INET);
> + if (!x)
> + return false;
> +
> + if (x->xso.dir != XFRM_DEV_OFFLOAD_IN)
> + goto put_state;
> + if (x->xso.type != XFRM_DEV_OFFLOAD_PACKET)
> + goto put_state;
> + if (x->xso.dev != rx_dev)
> + goto put_state;
[Severity: High]
Will this strict check reject offloaded ESP packets for DSA user ports?
For DSA SAs, the offload device is the DSA user port. However, inbound ESP
packets are intercepted at the physical conduit device before DSA unpacks
the tags, meaning the received device is the conduit. The packet is ignored
for hardware decrypt offload because the conduit does not match the user
port.
Furthermore, intercepting right after eth_type_trans() means the data
points to the DSA tag rather than the IP header, which would cause IPv4
parsing to fail.
[ ... ]
> +static bool airoha_soe_complete_out(struct sk_buff *skb,
> + struct airoha_soe_ctx *ctx)
> +{
> + struct dst_entry *dst = ctx->dst;
> + struct net *net;
> + int err;
> +
> + ctx->dst = NULL;
[ ... ]
> + skb->protocol = htons(ETH_P_IP);
> + skb_dst_drop(skb);
> + skb_dst_set(skb, dst);
> + skb->ignore_df = 1;
> + net = dev_net(dst->dev);
> + kfree(ctx);
> + dst_output(net, NULL, skb);
[Severity: High]
Does this reinjection drop critical SKB metadata?
It looks like only the routing entry is restored before calling
dst_output(), failing to carry over metadata like the packet mark,
priority, socket association, and VLAN TCI from the original SKB.
Could this cause the transmitted outer IPsec packet to lose its QoS
classifications and socket accounting contexts?
[ ... ]
> +static bool airoha_soe_complete_in(struct sk_buff *skb,
> + struct airoha_soe_ctx *ctx)
> +{
[ ... ]
> + skb->dev = rx_dev;
> + skb->mark = ctx->rx.mark;
> + skb->ip_summed = CHECKSUM_NONE;
> + skb_reset_mac_header(skb);
> + skb_reset_mac_len(skb);
> + skb->pkt_type = PACKET_HOST;
> + skb->encapsulation = 0;
> + skb_dst_drop(skb);
[ ... ]
> + kfree(ctx);
> + netif_rx(skb);
[Severity: High]
Will injecting a MAC-less L3 packet via netif_rx() cause issues in the
network stack?
The function strips the dummy L2 header but sets the MAC header to point
directly to the IP header. Since the network stack expects packets arriving
on physical Ethernet devices to possess a valid MAC header, reading the IP
header as an Ethernet header breaks protocol semantics.
Could this poison the bridge FDB with garbage MAC addresses derived from
IP fields?
[ ... ]
> +static int airoha_soe_xfrm_state_add(struct net_device *dev,
> + struct xfrm_state *x,
> + struct netlink_ext_ack *extack)
> +{
[ ... ]
> + state->dev = gdm_dev;
> + state->soe = soe;
[Severity: Critical]
Could this lead to a Use-After-Free of the conduit network device private
data?
When an XFRM state is added to a DSA user port, the private data is
assigned a pointer to the conduit device without taking a reference on the
conduit netdev via dev_hold().
If the conduit device is unregistered, the DSA user ports are destroyed,
triggering an asynchronous XFRM state garbage collection. The background GC
could then dereference the dangling state pointer in
airoha_soe_xfrm_state_free() if the conduit device was fully freed.
[ ... ]
> +static void airoha_soe_dsa_proxy_enable(struct net_device *dev)
> +{
> + struct net_device *conduit;
> +
> + conduit = airoha_soe_dsa_conduit_get(dev);
> + if (!conduit)
> + return;
> +
> + if (dev->xfrmdev_ops && dev->xfrmdev_ops != &airoha_soe_dsa_xfrmdev_ops)
> + goto out;
[Severity: Medium]
Does this condition fail to exit early if the ops are already set?
Since airoha_soe_netdev_event() calls airoha_soe_dsa_proxy_scan(true) on
every NETDEV_REGISTER event, the lack of an early return when the ops
pointer is already equal to airoha_soe_dsa_xfrmdev_ops causes it to proceed
and call netdev_update_features() redundantly.
This means adding any unrelated interface to the system triggers feature
recalculations on all existing DSA ports.
[ ... ]
> +static void airoha_soe_dsa_proxy_scan(bool enable)
> +{
> + struct net_device *dev;
> +
> + for_each_netdev(&init_net, dev) {
> + if (enable)
> + airoha_soe_dsa_proxy_enable(dev);
> + else
> + airoha_soe_dsa_proxy_clear(dev);
> + }
> +}
[Severity: High]
Is it intentional to restrict this loop to init_net?
If a DSA user interface is moved to a different network namespace, it
appears this loop will permanently skip it. During module unload,
airoha_soe_deinit() calls this scan to clear the hook pointers, but failing
to reach interfaces outside init_net leaves dangling pointers to
airoha_soe_dsa_xfrmdev_ops.
Could this result in a Use-After-Free and kernel panic upon a subsequent
network namespace cleanup or XFRM operation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: install DT overlays via dtbs_install
From: Krzysztof Kozlowski @ 2026-06-15 5:00 UTC (permalink / raw)
To: Vishwas Udupa, Bjorn Andersson
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-arm-msm, devicetree, snb, kbajaj
In-Reply-To: <20260428123725.3457865-1-vudupa@qti.qualcomm.com>
On 28/04/2026 14:37, Vishwas Udupa wrote:
> +dtb-$(CONFIG_ARCH_QCOM) += monaco-el2.dtbo
> dtb-$(CONFIG_ARCH_QCOM) += qcs8550-aim300-aiot.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs9100-ride.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs9100-ride-r3.dtb
> @@ -192,6 +203,7 @@ qcs9100-ride-r3-el2-dtbs := qcs9100-ride-r3.dtb lemans-el2.dtbo
>
> dtb-$(CONFIG_ARCH_QCOM) += qcs9100-ride-el2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs9100-ride-r3-el2.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += lemans-el2.dtbo
el2 is not a overlay which should be runtime chosen by bootloader. You
are supposed to just choose proper DTB.
Same in other places for el2.
You install ONLY the DTBOs which are for real mezzanines and addons/hats.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: remoteproc: qcom,pas: add thermal mitigation properties
From: Krzysztof Kozlowski @ 2026-06-15 5:03 UTC (permalink / raw)
To: Gaurav Kohli
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano, Amit Kucheria,
Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, linux-pm,
linux-hardening, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <ec65893d-873a-4a62-b0e2-5008b2130545@oss.qualcomm.com>
On 13/06/2026 13:05, Gaurav Kohli wrote:
>
>> register something.
>>
>> I don't understand the problem and explanation in the binding is
>> basically non-existing.
>>
>> Remember that all lists and indices ARE FIXED, so driver knows exactly
>> which index means what.
>>
>
> thanks for review, shall i use driver data, which is basically pas data
> structure like below:
>
> static const struct qcom_pas_data {
> .crash_reason_smem = 601,
> .firmware_name = "cdsp.mdt",
> .tmd_names = (const char *[]){"xyz", NULL},
> .num_tmds = 1,
>
> Is something like above acceptable? and this will also help to filter
> tmd names as well?
Yes, assuming this is per SoC.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 0/2] cpufreq: spacemit: Add cpufreq support for K1 SoC
From: Viresh Kumar @ 2026-06-15 5:07 UTC (permalink / raw)
To: Shuwei Wu
Cc: Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Yixun Lan, linux-pm, linux-kernel, linux-riscv, spacemit,
devicetree
In-Reply-To: <20260612-shadow-deps-v3-0-2f3ba88611ff@mailbox.org>
On 12-06-26, 17:51, Shuwei Wu wrote:
> Changes in v3:
> - Add a K1-specific cpufreq driver for the shared-rail, dual-clock topology
Why ?
> - Use one shared CPU OPP table and one cpufreq policy for all CPUs
> - Link to v2: https://lore.kernel.org/r/20260410-shadow-deps-v2-0-4e16b8c0f60e@mailbox.org
--
viresh
^ permalink raw reply
* Re: [PATCH v6 02/11] ARM: dts: renesas: r8a7740: Add clocks for FSI
From: Bui Duc Phuc @ 2026-06-15 5:17 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kuninori Morimoto, Mark Brown, Liam Girdwood, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Jaroslav Kysela,
Takashi Iwai, linux-sound, linux-renesas-soc, devicetree,
linux-kernel
In-Reply-To: <CAABR9nEgE=jGTSS7snPxyRDgRj6qiFDDkbm0MBM40RpLy_nESQ@mail.gmail.com>
Dear all,
Sorry for sending two additional emails regarding FSIA. They were not
in HTML format and therefore were rejected by the mailing list.
To avoid further confusion, I would like to summarize my findings here
and also include some additional observations regarding FSIB.
1. Regarding FSIA
The 12.288 MHz oscillator (OSC X8) only provides a reference/system
clock and does not by itself determine whether the FSI operates in
Master or Slave mode.
The Master/Slave relationship is determined by which device drives the
BCLK and LRCLK signals. Therefore, assuming that the current MCLK
source remains valid,
switching the FSI to Master mode would primarily require rerouting
BCLK and LRCLK, together with the corresponding software configuration
changes.
Alternatively, the codec MCLK could also be sourced directly from the
FSI MCLK output by rerouting the MCLK connection.
In that configuration, the FSI would provide all audio clocks (MCLK,
BCLK, and LRCLK) and operate as the clock master.
2. Regarding FSIB
I could not find any FSIB-related connections in the available
Armadillo board schematic. The schematic only shows FSIA.
Therefore, there appear to be two possible explanations:
1. The available hardware design documentation is incomplete, or
2. The connection between FSIB and HDMI is implemented internally
inside the SoC,
so the FSIB signals are not exposed in the board schematic.
Personally, I believe the second explanation is more likely.
At present, the only signal related to FSIB that I can identify in the
schematic is FSIACK, which is shared between FSIA and FSIB.
I cannot find any of the other FSIB signals. In addition, after
tracing the Linux 4.2 source code, I found that for the FSIB-to-HDMI
path,
only one FSIB-related pin is configured: ' fsib_mclk_in '
Furthermore, when the audio stream is started and the clock rate is
configured, the driver calls: fsi_clk_set_rate_cpg() rather than
the external clock configuration function (fsi_clk_set_rate_external()).
At the moment, I have not investigated the HDMI subsystem in depth,
and I do not know how the internal connection between HDMI
and FSIB is implemented inside the SoC. In addition, the original HDMI
driver source code has already been removed, so HDMI is not supported.
Currently, FSIB is not ready for testing on the current kernel.
As I also mentioned in the cover letter:
> - FSI master mode is currently compile-tested only. Full verification
> requires a dedicated HDMI driver (FSIB) or hardware modifications
> (resoldering board resistors) (FSIA).
3. Regarding Geert's question
> I have one more general question.
> arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dts configures
> audio for FSI (fsia_pins), but does not fill in a clock-frequency
> in fsiack_clk. Instead, it fills in 12.288 MHz in fsibck_clk, while
> the schematics call it FSIACK.
> Apparently the FSIACK pin is shared with FSIBCK on R-Mobile A1, so
> which function is used depends on pin control. However, the DTS does
> not perform any pin configuration for this pin?
>
Based on the current schematic and source code analysis, I think that
if we continue to support FSIB for this use case,
the DTS should explicitly configure the FSIB clock and pinmux, for example:
&fsibck_clk {
clock-frequency = <12288000>;
};
fsia_pins: sounda {
groups = "fsia_sclk_in",
"fsia_mclk_out",
"fsia_data_in_1",
"fsia_data_out_0";
function = "fsia";
};
+ fsib_pins: soundb {
+ groups = "fsib_mclk_in";
+ function = "fsib";
+ };
This configuration appears to match the current resistor population
shown in the schematic:
FSIA operates in Slave mode & FSIB operates in Master mode.
However, this conclusion is based only on the currently available
schematic and source code analysis.
Additional hardware documentation would be helpful to better
understand the internal HDMI/FSIB clock architecture and to confirm
this assumption.
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH 1/2] spi: dt-bindings: qcom,spi-geni-qcom: Add property to force GSI mode
From: Krzysztof Kozlowski @ 2026-06-15 5:22 UTC (permalink / raw)
To: Pengyu Luo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel
In-Reply-To: <20260614083424.464132-1-mitltlatltl@gmail.com>
On 14/06/2026 10:34, Pengyu Luo wrote:
> Some devices (such as gaokun3) do not disable FIFO mode, causing the
> driver to fallback to FIFO mode by default. However, these platforms
> also support GSI mode, which is highly preferred for certain
> peripherals like SPI touchscreens to improve performance.
>
> Introduce the "qcom,force-gsi-mode" device property to hint and force
> the controller into GSI mode during initialization.
You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.
And even differently named property does not feel correct here - just
choose GSI in the driver...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/2] cpufreq: spacemit: Add K1 cpufreq driver
From: Viresh Kumar @ 2026-06-15 5:24 UTC (permalink / raw)
To: Shuwei Wu
Cc: Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Yixun Lan, linux-pm, linux-kernel, linux-riscv, spacemit,
devicetree
In-Reply-To: <20260612-shadow-deps-v3-1-2f3ba88611ff@mailbox.org>
On 12-06-26, 17:51, Shuwei Wu wrote:
> K1 has two CPU cluster clocks but one shared CPU voltage rail. Use one
> cpufreq policy for all CPUs, let the OPP core handle the shared regulator
> and the cluster0 clock, and update the cluster1 clock explicitly.
If the clusters can do DVFS individually (as they have separate clocks), this
should be represented as two separate policies. Isn't it ?
--
viresh
^ permalink raw reply
* Re: [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state
From: Maulik Shah (mkshah) @ 2026-06-15 5:26 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Stephan Gerhold
In-Reply-To: <37390682-ca24-402c-bf45-7f6bfb87f4a8@oss.qualcomm.com>
On 6/11/2026 4:11 PM, Konrad Dybcio wrote:
> On 5/26/26 12:54 PM, Maulik Shah wrote:
>> There are two modes PDC irqchip can work in
>> - pass through mode
>> - secondary controller mode
>>
>> Secondary mode is supported depending on SoC using PDC HW Version v3.0
>> or higher.
>
> [...]
>
>> base-commit: 550604d6c9b9efc8d068aff94dc301694a7afdee
>> change-id: 20260522-hamoa_pdc-1517acc2dcd4
>> prerequisite-message-id: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
>> prerequisite-patch-id: 152df6e30f70eb1b45909ce2793bc4277554b7ff
>> prerequisite-patch-id: 118bd4216e0386e7214133f53153684947fa8dd3
>> prerequisite-patch-id: 1f2f272d8ad1f7930d462e6349bc49de815e1ba1
>> prerequisite-patch-id: 3754ffdf536206353f74953fd6d39804ff7787d4
>
> This does depend on the changes you made on the driver, but not on
> the 30 bundled DT changes - let's just point to the actual dependencies
>
> Konrad
Yes, i let b4 handle it and it added all 30 changes since the dependent series had both both driver / DT changes.
Will remove this dependency as driver part of the dependency series is already merged in.
Thanks,
Maulik
^ permalink raw reply
* [PATCH v10 0/2] Add Support for LTC3220 18 Channel LED Driver
From: Edelweise Escala @ 2026-06-15 5:26 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-leds, devicetree, linux-kernel, Edelweise Escala,
Conor Dooley
The LTC3220/LTC3220-1 is a multi-display LED driver, which contains a
high-efficiency, low-noise charge pump to provide power to up to
18 LED current sources. The LEDs are individually configurable to
64-step linear brightness control, blinking and gradation control
via 2-wire I2C interface. The blinking and gradation configuration
is shared across all LED.
LTC3220 has a quick write function which allows changing the brightness
on all LEDS simultaneously when the brightness is changed on led 1.
For this leds are aggregated in the device tree and on probe we check
if led-sources exist to enable quick write.
We would like to know if this approach is alright?
Another way we might want to know is, is it alright to just make a
virtual led for the quick write function. Changing brightness on
the virtual led will change the brightness for all.
Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
Changes in v10:
- Remove volatile reg
- Add regacache_sync on resume
- Add delays on reset
- Remove early return and let software rset even if there is reset
- Set to max brightness before blink
- Two pass validation approach to avoid premature sysfs exposure
- Use pointer arithmetic for container_of
- Remove unused macros
- Add missing property header
- Link to v9: https://lore.kernel.org/r/20260528-ltc3220-driver-v9-0-69450fc213cb@analog.com
Changes in v9:
- Add address-cells and size-cells in required
- Restrict to reg 1 if led-sources is present
- Make set brightness only change brighntess for nomal mode,
blink and gradation is now handled only in its respective triggers.
- Fix blink to change mode on blink set
- Make pattern clear only change brightness and not change gradation
mode so it won't affect other leds.
- Add checks if ever there is duplicate reg in the bindings.
- Add REGMAP_I2C on Kconfig
- Rebase on Latest next
- Link to v8: https://lore.kernel.org/r/20260519-ltc3220-driver-v8-0-e1771c1733f7@analog.com
Changes in v8:
- Add '>' on descriptions with multiple paragraph
- Change led registers to hex
- Change cache type to REGCACHE_FLAT_S
- Link to v7: https://lore.kernel.org/r/20260508-ltc3220-driver-v7-0-0f092ba54f23@analog.com
Changes in v7:
- Add cahcetype on regmap
- Dropped struct ltc3220_state *ltc3220_state and use container_of()
- Rename ltc3220_state to ltc3220
- Dropped functions only used once and used inline instead.
- Loop variable changes
- Consistent decalaration for container_of
- Tabbing fixes
- Link to v6: https://lore.kernel.org/r/20260417-ltc3220-driver-v6-0-18157871eddd@analog.com
Changes in v6:
- Fix commit message
- Add manufacturer on Kconfig and improve description
- Rearrange register map and bitmask and improve naming
- Use regmap, also use update bits of regmap to avoid unnecessary
structs
- Alignment and spacing fixes
- Use Define for magic naumbers
- Fix blink calculation
- Add comments on aggregated LED
- Fix variable name to something more understandable like i to led_index
- Link to v5: https://lore.kernel.org/r/20260126-ltc3220-driver-v5-0-152a30e98ab7@analog.com
Changes in v5:
- Missed rename on bindings filename in MAINTAINERS file
- Link to v4: https://lore.kernel.org/linux-leds/20260126-ltc3220-driver-v4-0-c59517206c24@analog.com
Changes in v4:
- Rename leds-ltc3220.yaml to adi,ltc3220.yaml
- Add Reviewed-by: Conor Dooley <conor.dooley@microchip.com> on
adi,ltc3220.yaml
Other V1 comments I think already addressed
- Subject commit message was already changed to match hardware
- Fixed wrapping after description
- Dropped "Bindings for" in descriptions and improved description to match hardware
- Dropped adi,ltc3220-1
- Dropped redundant description on reset-gpios
- Dropped adi,force-cpo-level
- Dropped adi,quick-write in favor of aggregated LED
- Used consistent quotes ^led@([1-9]|1[0-8])$
- Fixed wrapping on error messages
- Link to v3: https://lore.kernel.org/r/20260120-ltc3220-driver-v3-0-fef612ec4faa@analog.com
Changes in v3:
- Dropped quick-write on bindings and added aggregated led instead.
- Add aggregated led example.
- Modify quick write to check if there is aggregated led, if there is
aggregated led enable quick write.
- Use DEFINE_SIMPLE_DEV_PM_OPS instead of SIMPLE_DEV_PM_OPS.
- Link to v2: https://lore.kernel.org/r/20260112-ltc3220-driver-v2-0-d043058fc4df@analog.com
Changes in v2:
leds-ltc3220.yaml changes
- Fix wrapping on description
- Improve description and commit messge to describe hardware
- Drop ltc3220-1
- Drop charge pump
ltc3220.c changes
- Fix wrapping
- Drop ltc3220-1
- Drop devname_mandatory
- Link to v1: https://lore.kernel.org/r/20260106-ltc3220-driver-v1-0-73601d6f1649@analog.com
---
Edelweise Escala (2):
dt-bindings: leds: Add LTC3220 18 channel LED Driver
leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
.../devicetree/bindings/leds/adi,ltc3220.yaml | 131 ++++++
MAINTAINERS | 8 +
drivers/leds/Kconfig | 13 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-ltc3220.c | 467 +++++++++++++++++++++
5 files changed, 620 insertions(+)
---
base-commit: 25025253476a64c186592d952c27f24bc3490e42
change-id: 20260106-ltc3220-driver-f9ab6cc9d1e4
Best regards,
--
Edelweise Escala <edelweise.escala@analog.com>
^ permalink raw reply
* [PATCH v10 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver
From: Edelweise Escala @ 2026-06-15 5:26 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-leds, devicetree, linux-kernel, Edelweise Escala,
Conor Dooley
In-Reply-To: <20260615-ltc3220-driver-v10-0-916562c88598@analog.com>
LTC3220 is a multi-display LED driver with I2C interface.
The LTC3220 provides individual brightness control (64-step),
blinking, and gradation features for up to 18 LED outputs.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
.../devicetree/bindings/leds/adi,ltc3220.yaml | 131 +++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 138 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
new file mode 100644
index 000000000000..c859490e9108
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/adi,ltc3220.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC3220 LED Driver
+
+maintainers:
+ - Edelweise Escala <edelweise.escala@analog.com>
+
+description: >
+ The LTC3220 is a multi-display LED driver, which contains a high-efficiency,
+ low-noise charge pump to provide power to up to 18 LED current sources.
+ The LEDs are individually configurable to 64-step linear brightness control,
+ blinking and gradation control via 2-wire I2C interface.
+
+ For more product information please see the link below
+ https://www.analog.com/en/products/ltc3220.html
+
+properties:
+ compatible:
+ const: adi,ltc3220
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ reset-gpios:
+ maxItems: 1
+
+patternProperties:
+ '^led@([1-9a-f]|1[0-2])$':
+ type: object
+ $ref: /schemas/leds/common.yaml#
+ unevaluatedProperties: false
+ properties:
+ reg:
+ description:
+ Output channel for the LED (1-18 maps to LED outputs D1-D18).
+ Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
+ For aggregated LED control, define only one LED node with reg = <0x1>
+ and use led-sources to list all controlled outputs. Only reg 1 should
+ be present when using led-sources.
+ minimum: 1
+ maximum: 18
+
+ required:
+ - reg
+
+ if:
+ required:
+ - led-sources
+ then:
+ properties:
+ reg:
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - '#address-cells'
+ - '#size-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ // Independent LEDs
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@1c {
+ compatible = "adi,ltc3220";
+ reg = <0x1c>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
+
+ led@1 {
+ reg = <0x1>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <1>;
+ };
+
+ led@2 {
+ reg = <0x2>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <2>;
+ };
+
+ led@3 {
+ reg = <0x3>;
+ function = LED_FUNCTION_INDICATOR;
+ function-enumerator = <3>;
+ };
+ };
+ };
+
+ - |
+ // Aggregated LED
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@1c {
+ compatible = "adi,ltc3220";
+ reg = <0x1c>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@1 {
+ reg = <0x1>;
+ led-sources = <0x1 0x2 0x3 0x4 0x5 0x6 0x7 0x8 0x9 0xa 0xb 0xc 0xd 0xe 0xf 0x10 0x11 0x12>;
+ function = LED_FUNCTION_BACKLIGHT;
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16..c8a242577d2f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15223,6 +15223,13 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
F: drivers/iio/temperature/ltc2983.c
+LTC3220 LED DRIVER
+M: Edelweise Escala <edelweise.escala@analog.com>
+L: linux-leds@vger.kernel.org
+S: Maintained
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
+
LTC4282 HARDWARE MONITOR DRIVER
M: Nuno Sa <nuno.sa@analog.com>
L: linux-hwmon@vger.kernel.org
--
2.43.0
^ permalink raw reply related
* [PATCH v10 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
From: Edelweise Escala @ 2026-06-15 5:26 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-leds, devicetree, linux-kernel, Edelweise Escala
In-Reply-To: <20260615-ltc3220-driver-v10-0-916562c88598@analog.com>
Add driver for the LTC3220 18-channel LED driver
with I2C interface, individual brightness control, and hardware-assisted
blink/gradation features.
Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
MAINTAINERS | 1 +
drivers/leds/Kconfig | 13 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ltc3220.c | 467 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 482 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index c8a242577d2f..0f553ada61d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15229,6 +15229,7 @@ L: linux-leds@vger.kernel.org
S: Maintained
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
+F: drivers/leds/leds-ltc3220.c
LTC4282 HARDWARE MONITOR DRIVER
M: Nuno Sa <nuno.sa@analog.com>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f4a0a3c8c870..31b1e3ff094c 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -1000,6 +1000,19 @@ config LEDS_ST1202
Say Y to enable support for LEDs connected to LED1202
LED driver chips accessed via the I2C bus.
+config LEDS_LTC3220
+ tristate "LED Driver for Analog Devices Inc. LTC3220"
+ depends on I2C && LEDS_CLASS
+ select REGMAP_I2C
+ help
+ Say Y to enable support for the Analog Devices LTC3220
+ 18-channel LED controller with I2C interface.
+ The driver supports individual LED brightness control (64 steps),
+ hardware-assisted blinking and gradation effects.
+
+ To compile this driver as a module, choose M here: the module will
+ be called leds-ltc3220.
+
config LEDS_TPS6105X
tristate "LED support for TI TPS6105X"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7db3768912ca..a68244bd50fb 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o
obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o
obj-$(CONFIG_LEDS_LP8864) += leds-lp8864.o
obj-$(CONFIG_LEDS_LT3593) += leds-lt3593.o
+obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
new file mode 100644
index 000000000000..f95174ae451b
--- /dev/null
+++ b/drivers/leds/leds-ltc3220.c
@@ -0,0 +1,467 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LTC3220 18-Channel LED Driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ *
+ * Author: Edelweise Escala <edelweise.escala@analog.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+/* LTC3220 Registers */
+#define LTC3220_COMMAND_REG 0x00
+#define LTC3220_QUICK_WRITE_MASK BIT(0)
+#define LTC3220_SHUTDOWN_MASK BIT(3)
+
+#define LTC3220_ULED_REG(x) (0x01 + (x))
+#define LTC3220_LED_CURRENT_MASK GENMASK(5, 0)
+#define LTC3220_LED_MODE_MASK GENMASK(7, 6)
+
+#define LTC3220_GRAD_BLINK_REG 0x13
+#define LTC3220_GRADATION_MASK GENMASK(2, 0)
+#define LTC3220_GRADATION_DIRECTION_MASK BIT(0)
+#define LTC3220_GRADATION_PERIOD_MASK GENMASK(2, 1)
+#define LTC3220_BLINK_MASK GENMASK(4, 3)
+
+#define LTC3220_NUM_LEDS 18
+
+#define LTC3220_GRADATION_RAMP_TIME_240MS 240
+#define LTC3220_GRADATION_RAMP_TIME_480MS 480
+
+#define LTC3220_BLINK_ON_156MS 156
+#define LTC3220_BLINK_ON_625MS 625
+#define LTC3220_BLINK_PERIOD_1250MS 1250
+#define LTC3220_BLINK_PERIOD_2500MS 2500
+
+#define LTC3220_BLINK_SHORT_ON_TIME BIT(0)
+#define LTC3220_BLINK_LONG_PERIOD BIT(1)
+
+enum ltc3220_led_mode {
+ LTC3220_NORMAL_MODE,
+ LTC3220_BLINK_MODE,
+ LTC3220_GRADATION_MODE,
+};
+
+enum ltc3220_blink_mode {
+ LTC3220_BLINK_MODE_625MS_1250MS,
+ LTC3220_BLINK_MODE_156MS_1250MS,
+ LTC3220_BLINK_MODE_625MS_2500MS,
+ LTC3220_BLINK_MODE_156MS_2500MS
+};
+
+enum ltc3220_gradation_mode {
+ LTC3220_GRADATION_MODE_DISABLED,
+ LTC3220_GRADATION_MODE_240MS_RAMP_TIME,
+ LTC3220_GRADATION_MODE_480MS_RAMP_TIME,
+ LTC3220_GRADATION_MODE_960MS_RAMP_TIME
+};
+
+static const struct regmap_config ltc3220_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = LTC3220_GRAD_BLINK_REG,
+ .cache_type = REGCACHE_FLAT_S,
+};
+
+struct ltc3220_uled_cfg {
+ struct led_classdev led_cdev;
+ u8 reg_value;
+ u8 led_index;
+};
+
+struct ltc3220 {
+ struct ltc3220_uled_cfg uled_cfg[LTC3220_NUM_LEDS];
+ struct regmap *regmap;
+ bool is_aggregated;
+};
+
+/*
+ * Set LED brightness. Hardware supports 0-63 brightness levels.
+ * Mode switching (blink/gradation) is handled through dedicated callbacks.
+ */
+static int ltc3220_set_led_data(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+ led_cdev);
+ struct ltc3220 *ltc3220 = container_of(uled_cfg - uled_cfg->led_index, struct ltc3220,
+ uled_cfg[0]);
+ int ret;
+
+ ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+ brightness);
+ if (ret)
+ return ret;
+
+ uled_cfg->reg_value = brightness;
+
+ /*
+ * When aggregated LED mode is enabled, writing to LED 1 updates all
+ * LEDs simultaneously via quick-write mode. Update cached values for
+ * all LEDs to reflect the synchronized state.
+ * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for how
+ * to configure aggregated LED mode.
+ */
+ if (ltc3220->is_aggregated && uled_cfg->led_index == 0) {
+ for (int i = 0; i < LTC3220_NUM_LEDS; i++)
+ ltc3220->uled_cfg[i].reg_value = brightness;
+ }
+
+ return 0;
+}
+
+static enum led_brightness ltc3220_get_led_data(struct led_classdev *led_cdev)
+{
+ struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+ led_cdev);
+
+ return uled_cfg->reg_value;
+}
+
+/*
+ * LTC3220 pattern support for hardware-assisted breathing/gradation.
+ * The hardware supports 3 gradation ramp time 240ms, 480ms, 960ms)
+ * and can ramp up or down.
+ *
+ * Pattern array interpretation:
+ * pattern[0].brightness = start brightness (0-63)
+ * pattern[0].delta_t = ramp time in milliseconds
+ * pattern[1].brightness = end brightness (0-63)
+ * pattern[1].delta_t = (optional, can be 0 or same as pattern[0].delta_t)
+ */
+static int ltc3220_pattern_set(struct led_classdev *led_cdev,
+ struct led_pattern *pattern,
+ u32 len, int repeat)
+{
+ struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+ led_cdev);
+ struct ltc3220 *ltc3220 = container_of(uled_cfg - uled_cfg->led_index, struct ltc3220,
+ uled_cfg[0]);
+ u8 gradation_period;
+ u8 start_brightness;
+ u8 end_brightness;
+ u8 gradation_val;
+ bool is_increasing;
+ int ret;
+
+ if (len != 2)
+ return -EINVAL;
+
+ start_brightness = pattern[0].brightness & LTC3220_LED_CURRENT_MASK;
+ end_brightness = pattern[1].brightness & LTC3220_LED_CURRENT_MASK;
+
+ is_increasing = end_brightness > start_brightness;
+
+ if (pattern[0].delta_t == 0)
+ gradation_period = LTC3220_GRADATION_MODE_DISABLED;
+ else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_240MS)
+ gradation_period = LTC3220_GRADATION_MODE_240MS_RAMP_TIME;
+ else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_480MS)
+ gradation_period = LTC3220_GRADATION_MODE_480MS_RAMP_TIME;
+ else
+ gradation_period = LTC3220_GRADATION_MODE_960MS_RAMP_TIME;
+
+ gradation_val = FIELD_PREP(LTC3220_GRADATION_PERIOD_MASK, gradation_period);
+ gradation_val |= FIELD_PREP(LTC3220_GRADATION_DIRECTION_MASK, is_increasing);
+
+ ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+ LTC3220_GRADATION_MASK, gradation_val);
+ if (ret)
+ return ret;
+
+ ret = ltc3220_set_led_data(led_cdev, start_brightness);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+ FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_GRADATION_MODE) |
+ end_brightness);
+ if (ret)
+ return ret;
+
+ uled_cfg->reg_value = end_brightness;
+
+ return 0;
+}
+
+static int ltc3220_pattern_clear(struct led_classdev *led_cdev)
+{
+ struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+ led_cdev);
+ struct ltc3220 *ltc3220 = container_of(uled_cfg - uled_cfg->led_index, struct ltc3220,
+ uled_cfg[0]);
+ int ret;
+
+ ret = regmap_update_bits(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+ LTC3220_LED_MODE_MASK, LTC3220_NORMAL_MODE);
+ if (ret)
+ return ret;
+
+ return ltc3220_set_led_data(led_cdev, LED_OFF);
+}
+
+/*
+ * LTC3220 has a global blink configuration that affects all LEDs.
+ * This implementation allows per-LED blink requests, but the blink timing
+ * will be shared across all LEDs. The delay values are mapped to the
+ * hardware's discrete blink rates.
+ */
+static int ltc3220_blink_set(struct led_classdev *led_cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+ led_cdev);
+ struct ltc3220 *ltc3220 = container_of(uled_cfg - uled_cfg->led_index, struct ltc3220,
+ uled_cfg[0]);
+ u8 blink_mode = 0;
+ int ret;
+
+ if (*delay_on <= LTC3220_BLINK_ON_156MS)
+ blink_mode = LTC3220_BLINK_SHORT_ON_TIME;
+
+ if (*delay_on + *delay_off > LTC3220_BLINK_PERIOD_1250MS)
+ blink_mode |= LTC3220_BLINK_LONG_PERIOD;
+
+ switch (blink_mode) {
+ case LTC3220_BLINK_MODE_625MS_1250MS:
+ *delay_on = LTC3220_BLINK_ON_625MS;
+ *delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_625MS;
+ break;
+ case LTC3220_BLINK_MODE_156MS_1250MS:
+ *delay_on = LTC3220_BLINK_ON_156MS;
+ *delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_156MS;
+ break;
+ case LTC3220_BLINK_MODE_625MS_2500MS:
+ *delay_on = LTC3220_BLINK_ON_625MS;
+ *delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_625MS;
+ break;
+ case LTC3220_BLINK_MODE_156MS_2500MS:
+ *delay_on = LTC3220_BLINK_ON_156MS;
+ *delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_156MS;
+ break;
+ }
+
+ ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+ LTC3220_BLINK_MASK, FIELD_PREP(LTC3220_BLINK_MASK, blink_mode));
+ if (ret)
+ return ret;
+
+ /* Ensure LED has non-zero brightness before enabling blink mode */
+ if (uled_cfg->reg_value == 0) {
+ ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+ led_cdev->max_brightness);
+ if (ret)
+ return ret;
+
+ uled_cfg->reg_value = led_cdev->max_brightness;
+ }
+
+ return regmap_update_bits(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+ LTC3220_LED_MODE_MASK,
+ FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_BLINK_MODE));
+}
+
+static void ltc3220_reset_gpio_action(void *data)
+{
+ struct gpio_desc *reset_gpio = data;
+
+ gpiod_set_value_cansleep(reset_gpio, 1);
+}
+
+static int ltc3220_reset(struct ltc3220 *ltc3220, struct i2c_client *client)
+{
+ struct gpio_desc *reset_gpio;
+ int ret;
+
+ reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(reset_gpio))
+ return dev_err_probe(&client->dev, PTR_ERR(reset_gpio), "Failed on reset GPIO\n");
+
+ if (reset_gpio) {
+ usleep_range(10000, 12000);
+ gpiod_set_value_cansleep(reset_gpio, 0);
+ usleep_range(10000, 12000);
+
+ ret = devm_add_action_or_reset(&client->dev, ltc3220_reset_gpio_action,
+ reset_gpio);
+ if (ret)
+ return ret;
+ }
+
+ ret = regmap_write(ltc3220->regmap, LTC3220_COMMAND_REG, 0);
+ if (ret)
+ return ret;
+
+ for (int i = 0; i < LTC3220_NUM_LEDS; i++) {
+ ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(i), 0);
+ if (ret)
+ return ret;
+ }
+
+ return regmap_write(ltc3220->regmap, LTC3220_GRAD_BLINK_REG, 0);
+}
+
+static int ltc3220_suspend(struct device *dev)
+{
+ struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
+
+ return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+ LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
+}
+
+static int ltc3220_resume(struct device *dev)
+{
+ struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
+ int ret;
+
+ ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+ LTC3220_SHUTDOWN_MASK, 0);
+ if (ret)
+ return ret;
+
+ return regcache_sync(ltc3220->regmap);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ltc3220_pm_ops, ltc3220_suspend, ltc3220_resume);
+
+static int ltc3220_probe(struct i2c_client *client)
+{
+ struct ltc3220 *ltc3220;
+ bool aggregated_led_found = false;
+ int num_leds = 0;
+ u8 led_index = 0;
+ int ret;
+
+ ltc3220 = devm_kzalloc(&client->dev, sizeof(*ltc3220), GFP_KERNEL);
+ if (!ltc3220)
+ return -ENOMEM;
+
+ ltc3220->regmap = devm_regmap_init_i2c(client, <c3220_regmap_config);
+ if (IS_ERR(ltc3220->regmap))
+ return dev_err_probe(&client->dev, PTR_ERR(ltc3220->regmap),
+ "Failed to initialize regmap\n");
+
+ i2c_set_clientdata(client, ltc3220);
+
+ ret = ltc3220_reset(ltc3220, client);
+ if (ret)
+ return dev_err_probe(&client->dev, ret, "Failed to reset device\n");
+
+ /* First pass: validate configuration and set up LED structures */
+ device_for_each_child_node_scoped(&client->dev, child) {
+ struct ltc3220_uled_cfg *led;
+ u32 source;
+
+ ret = fwnode_property_read_u32(child, "reg", &source);
+ if (ret)
+ return dev_err_probe(&client->dev, ret, "Couldn't read LED address\n");
+
+ if (!source || source > LTC3220_NUM_LEDS)
+ return dev_err_probe(&client->dev, -EINVAL, "LED address out of range\n");
+
+ if (fwnode_property_present(child, "led-sources")) {
+ if (source != 1)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Aggregated LED out of range\n");
+
+ if (aggregated_led_found)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "One Aggregated LED only\n");
+
+ aggregated_led_found = true;
+ ltc3220->is_aggregated = true;
+
+ ret = regmap_update_bits(ltc3220->regmap,
+ LTC3220_COMMAND_REG,
+ LTC3220_QUICK_WRITE_MASK,
+ LTC3220_QUICK_WRITE_MASK);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to set quick write mode\n");
+ }
+
+ num_leds++;
+
+ /* LED node reg/index/address goes from 1 to 18 */
+ led_index = source - 1;
+ led = <c3220->uled_cfg[led_index];
+
+ if (led->led_cdev.dev)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Duplicate LED reg %u found", source);
+
+ led->led_index = led_index;
+ led->reg_value = 0;
+ led->led_cdev.brightness_set_blocking = ltc3220_set_led_data;
+ led->led_cdev.brightness_get = ltc3220_get_led_data;
+ led->led_cdev.max_brightness = 63;
+ led->led_cdev.blink_set = ltc3220_blink_set;
+ led->led_cdev.pattern_set = ltc3220_pattern_set;
+ led->led_cdev.pattern_clear = ltc3220_pattern_clear;
+ }
+
+ /*
+ * Aggregated LED mode uses hardware quick-write to control all 18 LEDs
+ * simultaneously. This is mutually exclusive with individual LED control.
+ * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for details
+ * on how to configure aggregated LED mode.
+ */
+ if (aggregated_led_found && num_leds > 1)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Aggregated LED must be the only LED node\n");
+
+ /* Second pass: register LEDs after validation */
+ device_for_each_child_node_scoped(&client->dev, child) {
+ struct led_init_data init_data = {};
+ struct ltc3220_uled_cfg *led;
+ u32 source;
+
+ ret = fwnode_property_read_u32(child, "reg", &source);
+ if (ret)
+ return ret;
+
+ init_data.fwnode = child;
+ init_data.devicename = "ltc3220";
+
+ led_index = source - 1;
+ led = <c3220->uled_cfg[led_index];
+
+ ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev, &init_data);
+ if (ret)
+ return dev_err_probe(&client->dev, ret, "Failed to register LED class\n");
+ }
+
+ return 0;
+}
+
+static const struct of_device_id ltc3220_of_match[] = {
+ { .compatible = "adi,ltc3220" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ltc3220_of_match);
+
+static struct i2c_driver ltc3220_led_driver = {
+ .driver = {
+ .name = "ltc3220",
+ .of_match_table = ltc3220_of_match,
+ .pm = pm_sleep_ptr(<c3220_pm_ops),
+ },
+ .probe = ltc3220_probe,
+};
+module_i2c_driver(ltc3220_led_driver);
+
+MODULE_AUTHOR("Edelweise Escala <edelweise.escala@analog.com>");
+MODULE_DESCRIPTION("LED driver for LTC3220 controllers");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 4/8] arm64: dts: qcom: kodiak: Enable CDSP & Modem cooling
From: Gaurav Kohli @ 2026-06-15 5:32 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano, Amit Kucheria,
Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, linux-pm,
linux-hardening, Manaf Meethalavalappu Pallikunhi
In-Reply-To: <pf6rymdtbfcqqzt6vnn6gc3uuod4vlfpw42dxxag33aom7lltu@6ymibirvwfnc>
On 6/9/2026 4:27 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 09, 2026 at 03:52:59PM +0530, Gaurav Kohli wrote:
>> Unlike the CPU, the CDSP/Modem does not throttle its speed automatically
>> when it reaches high temperatures in kodiak.
>>
>> Set up CDSP cooling by throttling the cdsp when it reaches 100°C and
>> for modem when it reaches to 95°C.
>>
>> Remove inherited mdmss cooling-map nodes for Non Modem kodiak variant.
>
> Why? If it is a GNSS-only MPSS, does it not provide any thermal
> mitigation mechanisms? Does ADSP provide those? WPSS?
>
Hi Dmitry,
Thank you for the review.
Since the remoteproc_mpss node doesn't exist on these boards, the
cooling-maps that reference it cause DT compilation errors. That's why
we need to remove the inherited cooling-maps from the SoC DTSI.
/delete-node/ &remoteproc_mpss;
Regarding thermal mitigation for other subsystems:
->CDSP and Modem are the primary heat sources based on our internal
thermal testing and evaluation.
->ADSP and WPSS have lower power consumption and don't typically reach
thermal thresholds that require active mitigation
->For this, I'm checking with our internal team to confirm if ADSP/WPSS
provide any TMD mechanism across all targets.
>>
>> Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/kodiak.dtsi | 127 ++++++++++++++++++++-
>> .../boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts | 17 +++
>
> So, you removed those for Radxa Q6A, but not forRB3 Gen2. Why?
>
Ack, this is a miss. will fix this.
>> .../dts/qcom/qcs6490-thundercomm-minipc-g1iot.dts | 17 +++
>> .../boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts | 17 +++
>> .../boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 18 +++
>> .../boot/dts/qcom/sc7280-herobrine-wifi-sku.dtsi | 16 +++
>> 6 files changed, 208 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>> index fa540d8c2615..d345add2d8c8 100644
>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>> @@ -3427,6 +3427,9 @@ remoteproc_mpss: remoteproc@4080000 {
>> qcom,smem-states = <&modem_smp2p_out 0>;
>> qcom,smem-state-names = "stop";
>>
>> + #cooling-cells = <3>;
>> + tmd-names = "pa", "modem";
>> +
>> status = "disabled";
>>
>> glink-edge {
>> @@ -4787,6 +4790,9 @@ remoteproc_cdsp: remoteproc@a300000 {
>> qcom,smem-states = <&cdsp_smp2p_out 0>;
>> qcom,smem-state-names = "stop";
>>
>> + #cooling-cells = <2>;
>> + tmd-names = "cdsp_sw";
>
> I'm going to review only this DT, the comments apply to the rest of
> them.
>
> So, we have two cases, CDSP and MPSS. Why does CDSP have only 2 cells?
> Just because tmd-names has only one name? What if we add another
> mitigation (which can be added in the firmware), do we suddenly have to
> change number of cells and all the cooling devices to reflect it?
>
As Cdsp has only one relevant tmd to mitigate, so we have used cooling
cells as 2. But i will change this to 3 as this is backward compatible.
> Finally. If I understand correctly, these mitigtion mechanisms are
> provided by the firmware. Firmware differs between the boards. Vendors
> (in theory) can change them. Why do we list these names here, in the SoC
> DT?
>
Below are the main reason for this, replied in other thread also.
Please guide, if i can use qcom_pas_data to define names.
Following Daniel's series [1], the thermal framework supports
mapping multiple cooling devices per remoteproc/device via indexed
cooling-cells.
1) The thermal framework's cooling-maps reference
cooling devices by index (for #cooling-cells = <3>). Without tmd-names,
there's no way to know which index corresponds to which TMD, as firmware
may return tmd-names in any order.
below are the changes post new thermal mapping changes:
DT: tmd-names = "cdsp_sw", "xyz";
Firmware: ["cdsp_sw", "xyz1", "xyz2",]
Driver registers: Only "cdsp_sw" (index 0) and "xyz" (index 1)
This allows cooling-maps like below:
cooling-device = <&remoteproc 0 ...> // "cdsp_sw"
cooling-device = <&remoteproc 1 ...> // "xyz"
2) Not all firmware-provided TMDs should be
exposed as cooling devices. The tmd-names property acts as a filter,
allowing board-specific DT to select only the relevant TMDs for that
platform.
[1]
https://lore.kernel.org/all/20260526140802.1059293-12-daniel.lezcano@oss.qualcomm.com/
Shall i use pas data to define tmd-names instead of dt ?
>> +
>> status = "disabled";
>>
>> glink-edge {
>> + cooling-maps {
>> + map0 {
>> + trip = <&mdmss0_alert1>;
>> + cooling-device = <&remoteproc_mpss 0 0 2>;
>
> What does this mean? I assume that the first cell is one of the
> mechanisms. What is the difference between them? Do we really need to
> list them one by one here?
>
Let me check, if i can document different tmd's somewhere:
-> modem tmd used for Modem Processor mitigation.
-> pa is used for Power Amplifier mitigation.
And we need to list them for binding purpose mainly.
> What do other cells mean? Why are they 0 and 2 rather than
> THERMAL_NO_LIMIT? How does one come with those values? This should all
> be documented and explained somewhere.
>
Will change to THERMAL_NO_LIMIT. Let me check, if i can use
qli doc for documentation.
>> + };
>> +
>> + map1 {
>> + trip = <&mdmss0_alert1>;
>> + cooling-device = <&remoteproc_mpss 1 0 2>;
>> + };
>> + };
>> };
>>
>
^ permalink raw reply
* Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio irq property
From: Krzysztof Kozlowski @ 2026-06-15 5:32 UTC (permalink / raw)
To: Xianwei Zhao, Conor Dooley
Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
linux-amlogic, linux-gpio, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <a79e58b5-3a11-4593-847d-ba92527549bf@amlogic.com>
On 15/06/2026 04:47, Xianwei Zhao wrote:
> Hi Conor,
> Thanks for your review.
>
> On 2026/6/12 01:39, Conor Dooley wrote:
>> Subject:
>> Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio
>> irq property
>> From:
>> Conor Dooley <conor@kernel.org>
>> Date:
>> 2026/6/12 01:39
>>
>> To:
>> xianwei.zhao@amlogic.com
>> CC:
>> Linus Walleij <linusw@kernel.org>, Rob Herring <robh@kernel.org>,
>> Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley
>> <conor+dt@kernel.org>, Neil Armstrong <neil.armstrong@linaro.org>, Kevin
>> Hilman <khilman@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>,
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
>> linux-amlogic@lists.infradead.org, linux-gpio@vger.kernel.org,
>> devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>> linux-arm-kernel@lists.infradead.org
>>
>>
>>
>> On Thu, Jun 11, 2026 at 07:54:33AM +0000, Xianwei Zhao via B4 Relay wrote:
>>> From: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>>
>>> Add the hw-irq property for each GPIO bank and enable interrupt-parent
>>> for pinctrl so that gpiod_to_irq() can translate GPIO lines to IRQs.
>> Uhhhhh, what? Why can't you just use the normal interrupts property?
>>
>
> The interrupt cannot be used directly because the GPIO bank only
> provides an IRQ base, which does not have a one-to-one mapping with the
> actual hardware interrupts.
>
> On Amlogic SoCs, GPIO interrupts are handled through a mux. Multiple
> GPIO pins are mapped to a limited number of real interrupt sources. The
> implementation can be found here:
>
> https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-meson-gpio.c
>
> To use a GPIO interrupt, an unused hardware interrupt must first be
> allocated, and then the corresponding mux register must be configured.
> This allocation and mapping are already implemented in the existing driver.
>
> In that driver, the mapping is performed dynamically rather than simply
> calculating:
>
> irq = irq_start + gpio_offset
>
> If the interrupt is used directly, only the GPIO index can be obtained.
If it is performed dynamically, then it is not suitable for DT.
You still did not explain what hardware aspect exactly is described by
"hw-irq".
> The real interrupt number cannot be derived by simply adding an offset,
> because the hardware interrupt must be allocated first. Pre-allocating
> all interrupts during initialization would prevent later GPIOs from
> obtaining available interrupt sources.
>
> Perhaps other names would be more appropriate here, such as "irq_start".
>
>>> Signed-off-by: Xianwei Zhao<xianwei.zhao@amlogic.com>
>>> ---
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
From: Krzysztof Kozlowski @ 2026-06-15 5:34 UTC (permalink / raw)
To: Jakub Szczudlo
Cc: linux-iio, andy, antoniu.miclaus, conor+dt, devicetree, dlechner,
duje, jic23, jishnu.prakash, jorge.marques, krzk+dt, linusw,
linux-kernel, marcelo.schmitt, mazziesaccount, mike.looijmans,
nuno.sa, robh, sakari.ailus, wens, joshua.crofts1
In-Reply-To: <20260613190957.654798-2-jakubszczudlo40@gmail.com>
On Sat, Jun 13, 2026 at 09:09:55PM +0200, Jakub Szczudlo wrote:
> Register layouts are the same as for ADS1100 but ADS1110 have different
> datarates and have internal voltage reference that is always 2.048V
>
> Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
> ---
> .../devicetree/bindings/iio/adc/ti,ads1100.yaml | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state
From: Maulik Shah (mkshah) @ 2026-06-15 5:34 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <ahWLPy8vg_neYgrX@linaro.org>
On 5/26/2026 5:29 PM, Stephan Gerhold wrote:
> On Tue, May 26, 2026 at 04:24:36PM +0530, Maulik Shah wrote:
[...]
>>
>> The series has been tested on x1e80100 CRD with both old and new firmware
>> and also on kaanapali.
>>
>
> Tested how?
>
> I recommend testing with the tlmm-test module Bjorn added, in all
> supported configurations, to make sure you don't introduce regressions
> for one of them. It would be also good to provide the test results here
> in the cover letter.
>
Sneh tested via tlmm-test and figured some issues with test module [1].
The test module fixes are merged now and I will add results in v3 cover letter.
[1] https://lore.kernel.org/linux-arm-msm/20260529-tlmm_test_changes-v1-0-88bfdccb4369@oss.qualcomm.com/
Thanks,
Maulik
^ permalink raw reply
* Re: [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
From: Maulik Shah (mkshah) @ 2026-06-15 5:34 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <73f700f2-7cba-4832-bd06-e82a7fd51a7a@oss.qualcomm.com>
On 6/11/2026 4:35 PM, Konrad Dybcio wrote:
> On 5/26/26 12:54 PM, Maulik Shah wrote:
>> All PDC irqchip supports pass through mode in which both Direct SPIs and
>> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
>>
>> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
>> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
>> still works same as pass through mode without latching at PDC even in
>> secondary controller mode.
>>
>> All the SoCs so far default uses pass through mode with the exception of
>> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
>> boards whereas it may be set to pass through mode for IoT-EVK boards.
>> The mode configuration is done in firmware and initially shipped windows
>> firmware did not have SCM interface to read or modify the PDC mode.
>> Later only write access is opened up for non secure world.
>>
>> Using the write access available add changes to modify the PDC mode to
>> pass through mode via SCM write. When the write fails (on older firmware)
>> assume to work in secondary mode.
>>
>> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> ---
>
> [...]
>
>> +static inline bool pdc_pin_uses_seconary_mode(int pin_out)
>
> Please add a comment somewhere near here, repeating what you said in
> the previous commit message (about the SPIs being mapped first, followed
> by GPIO-as-SPIs)
Sure. Will add comment in v3.
Thanks,
Maulik
^ permalink raw reply
* Re: [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
From: Maulik Shah (mkshah) @ 2026-06-15 5:35 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <ahWQmTr-9a33b9FY@linaro.org>
On 5/26/2026 5:52 PM, Stephan Gerhold wrote:
> On Tue, May 26, 2026 at 04:24:41PM +0530, Maulik Shah wrote:
[...]
>> static const struct pdc_cfg pdc_cfg_v3_2 = {
>> + .gpio_irq_sts = GENMASK(5, 5),
>> + .gpio_irq_mask = GENMASK(4, 4),
>
> BIT(5) / BIT(4) would be clearer here in my opinion.
GENMASK gives uniformity.
>
>> .irq_enable = GENMASK(3, 3),
>> .irq_type = GENMASK(2, 0),
>> };
>> [...]
>> @@ -184,6 +204,14 @@ static u32 pdc_reg_read(int reg, u32 i)
>> return readl_relaxed(pdc->base + reg + i * sizeof(u32));
>> }
>>
>> +static inline bool pdc_pin_uses_seconary_mode(int pin_out)
>> +{
>> + if (pdc->mode == PDC_SECONDARY_MODE && pin_out >= pdc->num_spis)
>> + return true;
>> +
>> + return false;
>
> Can put this in one line:
>
> return pdc->mode == PDC_SECONDARY_MODE && pin_out >= pdc->num_spis;
>
>> +}
Sure.
>> +
[...]
>> +
>> +static void pdc_clear_gpio_cfg(int pin_out)
>> +{
>> + unsigned long gpio_sts;
>> +
>> + if (pdc->version < PDC_VERSION_3_0)
>> + return;
>> +
>> + gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
>> + gpio_sts &= ~pdc->cfg->gpio_irq_sts;
>> + pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_sts);
>
> Is this guaranteed to be called sequentially, i.e. not in parallel on
> another CPU? Otherwise, you need to add the lock here to make sure the
> read-modify-write doesn't race with another CPU.
Right. with irq_desc->lock held it will be called sequentially and no locking
needed.
>
> Note that since the irq_cfg_reg is also used in qcom_pdc_gic_set_type()
> it would be safest to add the lock there as well (although since PDC has
> IRQCHIP_SET_TYPE_MASKED it's probably unlikely to be called in parallel
> with another irqchip operation for the same IRQ). In my patch, I handled
> this for all users using a new pdc_update_irq_cfg() function [1].
>
> [1]: https://github.com/stephan-gh/linux/commit/59ca2a7335ede83e4a7cf02704dd7c469c725c14
>
>> +}
[...]
>> +static void qcom_pdc_ack(struct irq_data *d)
>> +{
>> + if (pdc_pin_uses_seconary_mode(d->hwirq) && !irqd_is_level_type(d))
>> + pdc->clear_gpio(d->hwirq);
>> +}
>
> You might need a write memory barrier here and/or read-back here to make
> sure the write is complete before the interrupt is unmasked in the GIC.
> IIRC I added this in my patch after seeing some test tlmm-test failure..
I did not see any need for barries and all tlmm-test passed.
[...]
>>
>> + pdc->unmask_gpio = pdc_unmask_gpio_cfg;
>> + pdc->clear_gpio = pdc_clear_gpio_cfg;
>
> What is the purpose of these function pointers if you always assign the
> same function?
I have updated them in v3 to be assigned only for secondary mode.
Thanks,
Maulik
^ permalink raw reply
* Re: [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper
From: Maulik Shah (mkshah) @ 2026-06-15 5:35 UTC (permalink / raw)
To: Thomas Gleixner, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <878q8v7ij8.ffs@fw13>
On 6/3/2026 8:55 PM, Thomas Gleixner wrote:
> On Tue, May 26 2026 at 16:24, Maulik Shah wrote:
>
[...]
>>
>> index = FIELD_GET(GENMASK(31, 5), pin_out);
>> mask = FIELD_GET(GENMASK(4, 0), pin_out);
>>
>> + raw_spin_lock_irqsave(&pdc->lock, flags);
>
> guard()
Ack. Will update in v3.
Thanks,
Maulik
^ permalink raw reply
* Re: [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper
From: Maulik Shah (mkshah) @ 2026-06-15 5:36 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <ahWTZj727RCNZPR9@linaro.org>
On 5/26/2026 6:04 PM, Stephan Gerhold wrote:
> On Tue, May 26, 2026 at 04:24:39PM +0530, Maulik Shah wrote:
>> pdc->enable_intr() function already points to respective version
>> specific enable function. pdc_enable_intr() now only kept as wrapper.
>> Remove the wrapper and invoke pdc->enable_intr() from caller.
>>
>> Locking in pdc_enable_intr() applies lock to all pdc->enable_intr()
>> however its only required for pdc_enable_intr_bank() which uses
>> a shared bank across all interrupts. pdc_enable_intr_cfg() do not
>> required locking as IRQ_CFG registers are one per interrupt. Move
>> locking accordingly.
>
> Well, pdc_enable_intr_cfg() is still a read-modify-write. If two CPUs
> read IRQ_i_CFG at the same time and modify different bits (e.g. enable
> and type bits) then write back the modified register one of the
> modifications will get lost. Can we be sure that this won't happen?
Two or more CPUs can not handle/configure same IRQ at the same time
as the irq_desc->lock would serialize all irqchip callbacks for same irq.
Thanks,
Maulik
^ permalink raw reply
* RE: [PATCH 4/5] arm64: dts: exynos: add initial support for Samsung Exynos8855 smdk
From: Alim Akhtar @ 2026-06-15 5:36 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260612162335.1F9361F000E9@smtp.kernel.org>
> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Friday, June 12, 2026 9:54 PM
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: robh@kernel.org; conor+dt@kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 4/5] arm64: dts: exynos: add initial support for Samsung
> Exynos8855 smdk
>
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s)
> to consider:
> - [High] Missing Required Property in UART node `serial_0`
> - [Medium] Device Tree Topology Error: `cluster1` skips `core2`
> - [Low] Device Tree Binding Violation in `timer` node interrupts
> - [Low] DT Binding Violation: Missing `-gpio-bank` suffix on GPIO bank nodes
> - [Low] Device Tree Syntax Error: `gic` node defines `#size-cells = <1>`
> without `#address-cells = <1>`
> --
Thanks, Will fix these in next version
> Sashiko AI review · https://protect2.fireeye.com/v1/url?k=f6873d03-
> 970c2830-f686b64c-000babff9bb7-7861d1909a20c345&q=1&e=c55a9d49-
> 67b5-4ef3-896c-
> 48615db6c678&u=https%3A%2F%2Fsashiko.dev%2F%23%2Fpatchset%2F202
> 60612163020.411761-1-alim.akhtar%40samsung.com%3Fpart%3D4
^ permalink raw reply
* Re: [PATCH v2 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc
From: Maulik Shah (mkshah) @ 2026-06-15 5:36 UTC (permalink / raw)
To: Thomas Gleixner, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <87bjdr7ikt.ffs@fw13>
On 6/3/2026 8:54 PM, Thomas Gleixner wrote:
> On Tue, May 26 2026 at 16:24, Maulik Shah wrote:
>> @@ -221,9 +231,9 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
>> {
>> unsigned long flags;
>>
>> - raw_spin_lock_irqsave(&pdc_lock, flags);
>> - __pdc_enable_intr(d->hwirq, on);
>> - raw_spin_unlock_irqrestore(&pdc_lock, flags);
>> + raw_spin_lock_irqsave(&pdc->lock, flags);
>
> While at it please convert this to:
>
> guard(raw_spinlock_irqsave)(...);
>
> The _irqsave is not really required when invoked from the irqchip
> callbacks with irq desc lock held and interrupts disabled, but that's
> also magically invoked from other contexts. So you could spare that
> irqsave by wrapping the other callsite into:
>
> guard(irq)() or scoped_guard(irq)
Ack. Will update in v3.
>
>> + for (i = 0; i < pdc->region_cnt; i++) {
>
> for (int i = 0; ....
Ack. Will update in v3.
>
>> + if (pin >= pdc->region[i].pin_base &&
>> + pin < pdc->region[i].pin_base + pdc->region[i].cnt)
>> + return &pdc->region[i];
>>
>> + raw_spin_lock_init(&pdc->lock);
>> +
>> pdc_domain = irq_domain_create_hierarchy(parent_domain,
>> IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
>> PDC_MAX_IRQS,
>
> Please fix up the coding style here according to
>
> https://docs.kernel.org/process/maintainer-tip.html
Ack. Will update in v3.
Thanks,
Maulik
^ 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