* Re: [PATCH v8 08/22] RISC-V: Add Sscfg extension CSR definition
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-8-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:56 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> This adds the scountinhibit CSR definition and S-mode accessible hpmevent
> bits defined by smcdeleg/ssccfg. scountinhibit allows S-mode to start/stop
> counters directly from S-mode without invoking SBI calls to M-mode. It is
> also used to figure out the counters delegated to S-mode by the M-mode as
> well.
>
> Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
Does this need to be signed off by you?
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH v8 06/22] RISC-V: Add Smcntrpmf extension parsing
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-6-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:54 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> Smcntrpmf extension allows M-mode to enable privilege mode filtering
> for cycle/instret counters. However, the cyclecfg/instretcfg CSRs are
> available in Ssccfg only if Smcntrpmf is present.
>
> That's why, kernel needs to detect presence of Smcntrpmf extension and
> enable privilege mode filtering for cycle/instret counters.
>
> [...]
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH v8 05/22] RISC-V: Define indirect CSR access helpers
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-5-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:53 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> The indirect CSR requires multiple instructions to read/write CSR.
> Add a few helper functions for ease of usage.
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH v8 03/22] RISC-V: Add Sxcsrind ISA extension definition and parsing
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-3-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:51 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> The S[m|s]csrind extension extends the indirect CSR access mechanism
> defined in Smaia/Ssaia extensions.
>
> This patch just enables the definition and parsing.
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH v8 02/22] RISC-V: Add Sxcsrind ISA extension CSR definitions
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-2-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:50 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> This adds definitions of new CSRs and bits defined in Sxcsrind ISA
> extension. These CSR enables indirect accesses mechanism to access
> any CSRs in M-, S-, and VS-mode. The range of the select values
> and ireg will be define by the ISA extension using Sxcsrind extension.
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
Tested-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH v8 01/22] RISC-V: perf: fix resource cleanup on driver probe failure
From: Charlie Jenkins @ 2026-07-20 7:21 UTC (permalink / raw)
To: Atish Patra
Cc: Jiri Olsa, Paul Walmsley, Mark Rutland, Rob Herring, Anup Patel,
Namhyung Kim, Arnaldo Carvalho de Melo, Krzysztof Kozlowski,
Ian Rogers, Will Deacon, James Clark, linux-arm-kernel,
linux-riscv, linux-kernel, devicetree, linux-perf-users,
Conor Dooley
In-Reply-To: <20260701-counter_delegation-v8-1-7909f863a645@meta.com>
On Wed, 01 Jul 2026 01:46:49 -0700, Atish Patra <atish.patra@linux.dev> wrote:
> Sashiko pointed out various UAF and memory leak issues around
> pmu_sbi_device_probe() error paths.
>
> If the probe fails, here are list of cleanups needed.
> a. Already registered pmu must be freed
> b. per cpu IRQ must be released
> c. pmu_ctr_list data structure must be freed
> d. cpu hotplug state must be cleaned up only if added.
>
> [...]
Reviewed-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
--
- Charlie
^ permalink raw reply
* Re: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths
From: sashiko-bot @ 2026-07-20 7:20 UTC (permalink / raw)
To: Abdurrahman Hussain; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260720-nh-of-alias-overlay-v1-3-f1e5d9889b30@nexthop.ai>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Fallback to `__of_prop_dup()` for `/aliases` masks ENOMEM errors and bypasses null-termination safety checks, causing silent corruption and potential OOB reads.
Pre-existing issues:
- [Critical] Dangling pointer in target->np->deadprops leads to use-after-free and double-free if of_changeset_add_property() fails.
--
--- Patch [3]: [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths ---
commit 8741f5ffd2983da599c7be77cbf8c1af8325d4be
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
of/overlay: rewrite /aliases path values to live-tree paths
/aliases entries added by an overlay reference labeled nodes inside
the overlay via '&label' in the .dtso. dtc renders those references
as string paths at compile time, but the paths encode the overlay's
internal fragment layout (e.g. "/fragment@1/__overlay__/fpga@0/i2c@40000")
rather than the location where the node will live after apply.
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 654a70d5cb075..338786cef17ab 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -350,6 +350,19 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
> if (prop)
> return -EINVAL;
> new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> + } else if (target->np->parent &&
> + of_node_is_root(target->np->parent) &&
> + of_node_name_eq(target->np, "aliases")) {
> + /*
> + * /aliases property values are string paths in the same
> + * "/fragment@N/__overlay__/..." form as /__symbols__.
> + * Rewrite them to live-tree paths so of_alias_get_id() can
> + * resolve them; fall back to a raw dup for non-fragment
> + * values (e.g. legacy string aliases like "ttyS0").
> + */
> + new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
> + if (!new_prop)
> + new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
[Severity: High]
If dup_and_fixup_symbol_prop() fails because the string is not
null-terminated, does this fallback to __of_prop_dup() end up copying the
malformed string?
Because dup_and_fixup_symbol_prop() can return NULL for both memory
allocation failures and structural errors like non-null-terminated strings,
falling back to a raw duplicate here preserves the non-null-terminated
string.
Later, the reconfig notifier can trigger of_alias_create() which attempts to
read the property:
drivers/of/base.c:of_alias_create() {
...
if (owned)
np = of_find_node_by_path(pp->value);
...
}
Could this lead to an out-of-bounds read when of_find_node_by_path() treats
the non-null-terminated property value as a C string?
Additionally, does this mask memory allocation failures from
dup_and_fixup_symbol_prop() by falling back to a raw duplicate, injecting
unresolved paths directly into the live tree instead of failing the overlay
application?
> } else {
> new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
> }
[Severity: Critical]
This is a pre-existing issue, but while reviewing add_changeset_property(), I
noticed a problem further down in the same function.
When a property is added to a newly created node, it is linked into the
node's deadprops list before the changeset operation:
drivers/of/overlay.c:add_changeset_property() {
...
if (!prop) {
if (!target->in_livetree) {
new_prop->next = target->np->deadprops;
target->np->deadprops = new_prop;
}
ret = of_changeset_add_property(&ovcs->cset, target->np,
new_prop);
...
if (ret)
__of_prop_free(new_prop);
return ret;
}
If of_changeset_add_property() fails, perhaps due to an allocation error,
__of_prop_free() is called on new_prop, but new_prop is not unlinked from
target->np->deadprops.
Will this leave a dangling pointer in the deadprops list, causing a double
free later when the changeset is aborted and of_node_release() calls
property_list_free() on node->deadprops?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=3
^ permalink raw reply
* Re: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications
From: sashiko-bot @ 2026-07-20 7:17 UTC (permalink / raw)
To: Abdurrahman Hussain; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260720-nh-of-alias-overlay-v1-1-f1e5d9889b30@nexthop.ai>
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
New issues:
- [Critical] Unconditional dereference of `rd->old_prop` on OF_RECONFIG_UPDATE_PROPERTY causes a NULL pointer panic.
- [Critical] OF_RECONFIG_DETACH_NODE fails to remove dynamically allocated aliases, leading to a Memory Leak and Use-After-Free.
- [High] Lockless modification of the global `aliases_lookup` list causes data races and memory corruption.
- [High] Updating a boot-time alias creates duplicate entries mapping to the same ID, causing driver resource conflicts.
- [Medium] OF_RECONFIG_ATTACH_NODE fails to scan the node's properties, silently ignoring aliases added in a newly attached /aliases node.
- [Medium] The global `of_aliases` pointer mismanages node reference counting, causing leaks and weak references.
- [Low] The struct alias_prop::owned documentation contains factually incorrect and misleading claims regarding memory ownership.
Pre-existing issues:
- [Critical] Creating an alias from an empty property causes a NULL pointer dereference in of_find_node_by_path().
- [High] The alias parsing loop contains a 1-byte out-of-bounds read if the property name is an empty string.
--
--- Patch [1]: [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications ---
commit 172d1d81baebcd7bb4e961520fc11e6b4eadbc38
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
of: incrementally update /aliases lookup on reconfig notifications
This commit registers an OF reconfig notifier to mirror changes in the
/aliases node into the global aliases_lookup list. It extracts the alias
parsing logic into a common function and tracks whether entries are statically
or dynamically allocated. This ensures aliases defined in overlays are
correctly resolved by drivers.
Link: https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/20150630172131.D4E6CC4041A@trevor.secretlab.ca/
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 6e7a42dedad3f..d516523a71d5e 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
[ ... ]
> @@ -1915,6 +1915,152 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
> +/*
> + * Build an alias_prop for @pp using @dt_alloc as the storage allocator
> + * and add it to aliases_lookup. @owned is stored on the entry so the
> + * matching destroy path knows whether the alias_prop is a kmalloc'd
> + * struct that must be kfree()d (with a paired of_node_put on the
> + * target) or a memblock/dt_alloc'd struct that must be left alone.
> + *
> + * Pseudo-properties (name, phandle, ...) and alias names not ending in
> + * a numeric id are silently skipped.
> + */
> +static void of_alias_create(const struct property *pp,
> + void *(*dt_alloc)(u64 size, u64 align),
> + bool owned)
> +{
> + const char *start = pp->name;
> + const char *end = start + strlen(start);
> + struct device_node *np;
> + struct alias_prop *ap;
> + int id, len;
> +
> + if (is_pseudo_property(pp->name))
> + return;
> +
> + np = of_find_node_by_path(pp->value);
[Severity: Critical]
This is a pre-existing issue, but does this cause a NULL pointer
dereference if the property is empty?
If an overlay or boot DT defines a property in /aliases with no value,
pp->value is NULL. Since of_alias_create() passes pp->value directly to
of_find_node_by_path(), which passes it to strchr(), will this crash?
> + if (!np)
> + return;
> +
> + while (isdigit(*(end - 1)) && end > start)
> + end--;
[Severity: High]
This isn't a bug introduced by this patch, but does the loop condition
cause an out-of-bounds read if the property name is an empty string?
If start is empty, end equals start. The loop evaluates isdigit(*(end - 1))
before verifying end > start, causing an immediate read of the memory byte
preceding the buffer.
> + len = end - start;
[ ... ]
> +static void of_alias_destroy(const char *name)
> +{
> + struct alias_prop *ap, *tmp;
> +
> + list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
> + if (!ap->owned || strcmp(ap->alias, name) != 0)
> + continue;
[Severity: High]
If an overlay modifies the target of an existing boot-time alias, will
skipping the destruction of boot-time entries leave duplicate entries in
the list?
Since this skips entries where owned is false, the old boot-time alias
remains in aliases_lookup alongside the newly created overlay alias.
When of_alias_get_id() searches the list, could it return the same ID for
both devices?
> + list_del(&ap->link);
> + of_node_put(ap->np);
> + kfree(ap);
> + return;
> + }
> +}
[ ... ]
> +static int of_aliases_reconfig_notifier(struct notifier_block *nb,
> + unsigned long action, void *arg)
> +{
> + struct of_reconfig_data *rd = arg;
> +
> + /*
> + * Match /aliases structurally (name + root-parent) rather than by
> + * pointer against the of_aliases global — a system with no
> + * boot-time /aliases (of_aliases == NULL) can still acquire one
> + * from an overlay, and we must track its properties from the
> + * first ATTACH_NODE onward.
> + */
> + if (!rd->dn || !rd->dn->parent ||
> + !of_node_is_root(rd->dn->parent) ||
> + !of_node_name_eq(rd->dn, "aliases"))
> + return NOTIFY_DONE;
> +
> + switch (action) {
> + case OF_RECONFIG_ATTACH_NODE:
> + if (!of_aliases)
> + of_aliases = rd->dn;
[Severity: Medium]
Does assigning of_aliases here create a weak reference?
During boot-time scanning, of_alias_scan() acquires of_aliases with an
incremented reference count. Here, it is assigned without calling
of_node_get().
[Severity: Medium]
Also, if the system started without a boot-time /aliases node, does
attaching one fail to parse its properties?
Since this handler updates the of_aliases pointer but misses the
for_each_property_of_node() loop to actually create the aliases, it seems
the aliases in a newly attached node are silently ignored.
> + break;
> + case OF_RECONFIG_DETACH_NODE:
> + if (of_aliases == rd->dn)
> + of_aliases = NULL;
[Severity: Medium]
Does clearing the pointer here cause a reference leak since of_node_put()
is omitted?
[Severity: Critical]
Additionally, does skipping the removal of the dynamic aliases cause a
memory leak and use-after-free?
The DETACH_NODE handler clears the of_aliases pointer but skips removing
the corresponding alias_prop entries from aliases_lookup. When the detached
node and its properties are later freed, dangling entries will remain in
the global list. A subsequent alias update could iterate the list and call
strcmp() on freed pointer strings.
> + break;
> + case OF_RECONFIG_ADD_PROPERTY:
> + of_alias_create(rd->prop, alias_alloc, true);
[Severity: High]
Is it safe to modify aliases_lookup locklessly here?
The notifier processes dynamic changes and calls into of_alias_create()
which uses list_add_tail() on the global aliases_lookup list. OF reconfig
notifiers are intentionally executed after of_mutex has been unlocked, such
as in __of_changeset_apply_notify().
Could this race against other writers, or against driver probes calling
of_alias_get_id() which iterates the list expecting it to be protected by
of_mutex?
> + break;
> + case OF_RECONFIG_REMOVE_PROPERTY:
> + of_alias_destroy(rd->prop->name);
> + break;
> + case OF_RECONFIG_UPDATE_PROPERTY:
> + of_alias_destroy(rd->old_prop->name);
[Severity: Critical]
Can rd->old_prop be NULL here?
The of_update_property() API allows adding a property if it does not
already exist, and emits an OF_RECONFIG_UPDATE_PROPERTY event with
old_prop set to NULL. Would unconditionally dereferencing
rd->old_prop->name cause a panic in this scenario?
> + of_alias_create(rd->prop, alias_alloc, true);
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
[ ... ]
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 0ae16da066e2c..9d16765ae2c3f 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -17,6 +17,12 @@
> * @alias: Alias property name
> * @np: Pointer to device_node that the alias stands for
> * @id: Index value from end of alias name
> + * @owned: True if @alias was kstrdup'd and @np was of_node_get'd on
> + * insertion (overlay-time entries). False for entries built
> + * by of_alias_scan() at boot, where @alias points into the
> + * FDT and @np is an unreferenced pointer. The removal path
> + * uses this flag to decide whether it must kfree(@alias),
> + * of_node_put(@np), and kfree(the struct itself).
[Severity: Low]
Does this documentation accurately reflect the implementation?
The comment claims @alias is kstrdup'd on insertion and kfree'd on
removal. However, the code merely stores the pp->name pointer directly
and never calls kfree() on the alias string. It also mentions np is
unreferenced at boot time, but of_find_node_by_path() does take a
reference.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=1
^ permalink raw reply
* Re: [PATCH RFC 2/4] of/overlay: look up absolute target-paths absolutely
From: sashiko-bot @ 2026-07-20 7:15 UTC (permalink / raw)
To: Abdurrahman Hussain; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260720-nh-of-alias-overlay-v1-2-f1e5d9889b30@nexthop.ai>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Overlay changeset fragments' target and overlay nodes are leaked on the error paths in `init_overlay_changeset()`.
- [High] Calling `of_node_put()` under `devtree_lock` (a raw spinlock) can sleep in atomic context.
- [High] The `lan966x_pci` driver leaks overlay resources if `of_overlay_fdt_apply()` fails.
--
--- Patch [2]: [PATCH RFC 2/4] of/overlay: look up absolute target-paths absolutely ---
commit 8125ed0b64def60707430d565ce13659fa3c53dc
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Subject: of/overlay: look up absolute target-paths absolutely
This commit modifies find_target() to treat non-empty target-paths as
absolute when applying a device tree overlay. It allows overlays to modify
both a subtree under a base node and nodes at the device tree root.
> Treat target-path as absolute whenever it is non-empty. An empty
> target-path continues to mean "the target base itself", preserving
> the existing shape used by drivers/misc/lan966x_pci.c and its dtso
> (the only in-tree of_overlay_fdt_apply() caller today that passes a
> non-NULL base).
[Severity: High]
This isn't a regression introduced by this patch, but does the lan966x_pci
driver leak overlay resources if of_overlay_fdt_apply() fails?
The API contract for of_overlay_fdt_apply() states that if it fails during
of_overlay_apply(), it intentionally skips freeing the changeset and instead
populates *ret_ovcs_id, delegating the cleanup to the caller.
However, if lan966x_pci_load_overlay() encounters a failure, it immediately
returns the error to lan966x_pci_probe():
drivers/misc/lan966x_pci.c:lan966x_pci_probe() {
...
ret = lan966x_pci_load_overlay(data);
if (ret)
return ret;
...
}
This returns without invoking of_overlay_remove() on the returned ID, which
would permanently leak the overlay changeset structure, unflattened FDT
memory, and partially applied device tree nodes. Should lan966x_pci_probe()
clean up the overlay if it fails?
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 08d5351746be6..654a70d5cb075 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
[ ... ]
> @@ -709,23 +708,22 @@ static struct device_node *find_target(const struct device_node *info_node,
>
> ret = of_property_read_string(info_node, "target-path", &path);
> if (!ret) {
> - if (target_base) {
> - target_path = kasprintf(GFP_KERNEL, "%pOF%s", target_base, path);
> - if (!target_path)
> - return NULL;
> - node = of_find_node_by_path(target_path);
> - if (!node) {
> - pr_err("find target, node: %pOF, path '%s' not found\n",
> - info_node, target_path);
> - }
> - kfree(target_path);
> - } else {
> - node = of_find_node_by_path(path);
> - if (!node) {
> - pr_err("find target, node: %pOF, path '%s' not found\n",
> - info_node, path);
> - }
> - }
> + /*
> + * With a non-NULL @target_base, an empty target-path means
> + * "the target base itself" — this is the common
> + * of_overlay_fdt_apply(..., base) form used by e.g. the
> + * LAN966x PCI overlay. Any other target-path is looked up
> + * absolutely, so overlays that also need to reach the DT
> + * root (e.g. to add /aliases entries alongside a base-
> + * relative fragment) can do so with target-path="/aliases".
> + */
> + if (target_base && path[0] == '\0')
> + return of_node_get((struct device_node *)target_base);
> +
> + node = of_find_node_by_path(path);
> + if (!node)
> + pr_err("find target, node: %pOF, path '%s' not found\n",
> + info_node, path);
> return node;
> }
[Severity: High]
This is a pre-existing issue, but I noticed a memory leak when find_target()
returns NULL.
In init_overlay_changeset(), ovcs->count is only updated to the final fragment
count (cnt) at the end of the function. If find_target() returns NULL, the
function jumps to the error path err_out without updating ovcs->count, leaving
it at 0:
drivers/of/overlay.c:init_overlay_changeset() {
...
fragment->target = find_target(node, target_base);
if (!fragment->target) {
of_node_put(fragment->overlay);
ret = -EINVAL;
of_node_put(node);
goto err_out;
}
...
}
When the caller subsequently cleans up by calling free_overlay_changeset(), it
uses ovcs->count to iterate over the fragments and release their target and
overlay nodes:
drivers/of/overlay.c:free_overlay_changeset() {
...
for (i = 0; i < ovcs->count; i++) {
of_node_put(ovcs->fragments[i].target);
of_node_put(ovcs->fragments[i].overlay);
}
...
}
Since ovcs->count is 0, none of the previously processed fragments have their
node references released. Can we ensure ovcs->count is updated incrementally
or handle the cleanup of already-processed fragments on the error path?
[Severity: High]
I also noticed another pre-existing issue unrelated to this patch, but can
calling of_node_put() under devtree_lock cause a sleep in atomic context?
The devtree_lock is a raw spinlock. Several OF node iterators, such as
__of_get_next_child(), call of_node_put(prev) while holding this lock:
drivers/of/base.c:of_get_next_child() {
...
raw_spin_lock_irqsave(&devtree_lock, flags);
next = __of_get_next_child(node, prev);
raw_spin_unlock_irqrestore(&devtree_lock, flags);
...
}
drivers/of/base.c:__of_get_next_child() {
...
of_node_put(prev);
return next;
}
If the node being put is a dynamic device node (e.g., from an overlay) and
this is its final reference, of_node_put() calls kobject_release(), which
invokes of_node_release(). Inside of_node_release(), the code calls
fwnode_links_purge(), which acquires the fwnode_link_lock mutex. Taking a
mutex while holding a raw spinlock and with interrupts disabled is a fatal
scheduling-while-atomic bug.
Could this lead to deadlocks or system crashes when an overlay is detached
concurrently with device tree iteration?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=2
^ permalink raw reply
* Re: [PATCH v2] scsi: ufs: dt-bindings: Add missing mcq reg for qcom,sa8255p-ufshc
From: Krzysztof Kozlowski @ 2026-07-20 7:12 UTC (permalink / raw)
To: Shawn Guo
Cc: Martin K . Petersen, Manivannan Sadhasivam, Alim Akhtar,
Avri Altman, Bart Van Assche, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Kumar Dwivedi, Anjana Hari, linux-arm-msm,
linux-scsi, devicetree, linux-kernel, stable
In-Reply-To: <20260720023552.2667237-1-shengchao.guo@oss.qualcomm.com>
On Mon, Jul 20, 2026 at 10:35:52AM +0800, Shawn Guo wrote:
> Qualcomm UFS controller found on SoCs SA8255P/SA8797P has a MCQ I/O
> address space. It should be defined in the bindings even though Linux
> driver currently doesn't utilize it. Fix the binding before it gets
> adopted by DTS.
>
> Fixes: e2725ed2a7fb ("scsi: ufs: dt-bindings: Document bindings for SA8255P UFS Host Controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
> Changes for v2:
> - Recover the newline which got dropped by accident
> - Link to v1: https://lore.kernel.org/r/20260716083313.2367249-1-shengchao.guo@oss.qualcomm.com
>
> .../devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
qcom,sa8797p-ufshc should require it, but nevertheless:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert
From: sashiko-bot @ 2026-07-20 7:10 UTC (permalink / raw)
To: Abdurrahman Hussain; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260720-nh-of-alias-overlay-v1-4-f1e5d9889b30@nexthop.ai>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test claims to exercise patch 2's logic but bypasses it by passing NULL for target_base.
- [Medium] The test claims to exercise patch 3's logic but bypasses it by using an absolute live-tree path.
--
--- Patch [4]: [PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert ---
commit 1237920f0bd1d803a084dc2f03f9e63aaf5cd6a2
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
of: unittest: cover /aliases updates from overlay apply/revert
This commit adds an overlay and a test runner to verify alias creation and
removal dynamically during device tree overlay apply and revert operations.
It asserts that the alias is correctly added and then removed.
> exercises all three functional patches earlier in this series
> together: patch 1's reconfig notifier is what mutates aliases_lookup
> on apply/revert, patch 2 is what lets target-path="/aliases" resolve
> to the DT root when target_base is non-NULL, and patch 3 is what
> rewrites the fragment-internal path in the alias value so
> of_find_node_by_path() finds the live-tree target. Any one of the
> three missing turns the middle assertion (get_id -> 99) into
> -ENODEV.
[Severity: Medium]
Is this description of the test coverage accurate?
As detailed below, the test appears to bypass the logic for both patch 2 and
patch 3. The overlay uses an absolute path instead of an intra-overlay
reference (bypassing patch 3), and overlay_data_apply() passes NULL for
target_base (bypassing patch 2).
> diff --git a/drivers/of/unittest-data/overlay_alias.dtso b/drivers/of/unittest-data/overlay_alias.dtso
> new file mode 100644
> index 0000000000000..32532c80505a7
> --- /dev/null
> +++ b/drivers/of/unittest-data/overlay_alias.dtso
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +/plugin/;
> +
> +/* overlay_alias - declare an alias inside an overlay */
> +
> +&{/aliases} {
> + testcase-alias99 = "/testcase-data/overlay-node/test-bus/test-unittest100";
[Severity: Medium]
Does this overlay actually test the path rewrite logic from patch 3?
The testcase-alias99 property is defined using an absolute live-tree path
rather than an intra-overlay fragment reference. Because this is already a
valid absolute path, it seems of_alias_create() will successfully resolve it
via of_find_node_by_path() even if the rewrite logic from patch 3 is
missing.
> +};
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index e255f54f4d760..a9f4a0bb30872 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
[ ... ]
> @@ -3475,6 +3475,53 @@ static struct notifier_block of_nb = {
> .notifier_call = of_notify,
> };
>
> +static void __init of_unittest_overlay_alias(void)
> +{
> + const char *path = "/testcase-data/overlay-node/test-bus/test-unittest100";
> + struct device_node *np;
> + int ovcs_id = 0;
> + int id, ret;
> +
> + np = of_find_node_by_path(path);
> + if (!np) {
> + unittest(0, "could not find %s for alias test\n", path);
> + return;
> + }
> +
> + id = of_alias_get_id(np, "testcase-alias");
> + if (id != -ENODEV) {
> + unittest(0,
> + "of_alias_get_id() = %d before overlay, expected -ENODEV\n",
> + id);
> + goto out;
> + }
> +
> + ret = overlay_data_apply("overlay_alias", &ovcs_id);
[Severity: Medium]
Does this test actually exercise the target_base logic from patch 2?
Looking at overlay_data_apply(), it calls of_overlay_fdt_apply() with a
hardcoded NULL for target_base:
drivers/of/unittest.c:overlay_data_apply() {
...
ret = of_overlay_fdt_apply(info->dtbo_begin, size, &info->ovcs_id,
NULL);
...
}
When target_base is NULL, the pre-patch-2 code already resolves
target-path="/aliases" correctly via of_find_node_by_path(). This means the
test might pass and bypass patch 2 entirely, missing the intended test
coverage.
> + if (!ret) {
> + unittest(0, "overlay_alias apply failed\n");
> + goto out;
> + }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai?part=4
^ permalink raw reply
* Re: [PATCH v2 4/4] dt-bindings: power: supply: max17042: Allow monitored-battery
From: Krzysztof Kozlowski @ 2026-07-20 7:06 UTC (permalink / raw)
To: Vincent Cloutier
Cc: Sebastian Reichel, Hans de Goede, Marek Szyprowski,
Sebastian Krzyszkowiak, Purism Kernel Team, Rob Herring,
Conor Dooley, linux-pm, devicetree, linux-kernel,
Vincent Cloutier
In-Reply-To: <20260720011144.1280219-5-vincent.cloutier@icloud.com>
On Sun, Jul 19, 2026 at 09:11:40PM -0400, Vincent Cloutier wrote:
> From: Vincent Cloutier <vincent@cloutier.co>
>
> The MAX17042 family binding references the generic power-supply schema,
> but its additionalProperties restriction means generic properties must
> also be admitted explicitly.
With the same explanation, just change additionalProps into
unevaluatedProperties.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH V2] scsi: ufs: dt-bindings: Document the Maili UFS Controller
From: Krzysztof Kozlowski @ 2026-07-20 7:05 UTC (permalink / raw)
To: Nitin Rawat
Cc: krzk+dt, robh, andersson, mani, alim.akhtar, bvanassche,
avri.altman, conor+dt, linux-arm-msm, linux-kernel, devicetree,
linux-scsi
In-Reply-To: <20260719195833.136543-1-nitin.rawat@oss.qualcomm.com>
On Mon, Jul 20, 2026 at 01:28:33AM +0530, Nitin Rawat wrote:
> Document the UFS Controller on Maili SoC.
>
> Signed-off-by: Nitin Rawat <nitin.rawat@oss.qualcomm.com>
> ---
> Change from v1:
> 1. Addressed Krzysztof's comment to update 'qcom,maili-ufshc'
> to the allOf condition required for MCQ targets.
> ---
> Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [EXT] Re: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree
From: Joseph Guo @ 2026-07-20 7:03 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, Frank.Li, conor+dt, imx, devicetree
In-Reply-To: <20260720065549.C24301F000E9@smtp.kernel.org>
On 7/20/2026 2:55 PM, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Shared GPIO requested exclusively by two CAN transceivers causes probe failure for the second transceiver.
> - [Low] Use of undocumented, downstream-only device tree property `fsl,cd-gpio-wakeup-disable`.
> --
>
> --- Patch [2]: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree ---
> commit 8bbcb28d08a05e0f4c57023333492b4e023f7caf
> Author: Joseph Guo <qijian.guo@nxp.com>
>
> arm64: dts: freescale: add i.MX952 FRDM Board support device tree
>
> Add basic device tree support for i.MX952 FRDM Board. The board-common hardware
> description is placed in imx952-frdm.dtsi so that it can be shared with future
> i.MX952-based FRDM derivative boards.
>
>> diff --git a/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
>> new file mode 100644
>> index 0000000000000..b9ef6bec2bd5a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
>> @@ -0,0 +1,727 @@
> [ ... ]
>> + flexcan2_phy: can-phy0 {
>> + compatible = "nxp,tja1051";
>> + #phy-cells = <0>;
>> + max-bitrate = <8000000>;
>> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
>> + };
>> +
>> + flexcan3_phy: can-phy1 {
>> + compatible = "nxp,tja1051";
>> + #phy-cells = <0>;
>> + max-bitrate = <8000000>;
>> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
>
> [Severity: High]
> Does this cause a probe failure for the second transceiver?
>
> The first PHY node successfully requests this GPIO line, taking exclusive
> ownership. Because the phy-can-transceiver driver uses standard exclusive
> requests (without GPIOD_FLAGS_BIT_NONEXCLUSIVE), the second request for the
> exact same GPIO line fails with -EBUSY.
>
> This prevents the flexcan3_phy and the flexcan3 controller from probing and
> functioning.
>
This two CAN PHY did share same pin as their silent-gpio on hardware.
The same design on i.MX95 15x15 FRDM. This should be fine.
Regards,
Joseph
>> + };
> [ ... ]
>> +&usdhc2 {
>> + bus-width = <4>;
>> + cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
>> + pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
>> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
>> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
>> + pinctrl-3 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
>> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
>> + vmmc-supply = <®_usdhc2_vmmc>;
>> + fsl,cd-gpio-wakeup-disable;
>
> [Severity: Low]
> Is this out-of-tree vendor property invalid in the upstream kernel?
>
> The property fsl,cd-gpio-wakeup-disable does not exist in any upstream device
> tree bindings (such as fsl-imx-esdhc.yaml) and is not handled by any driver
> code in the upstream Linux tree. This acts as dead code and will trigger
> `make dtbs_check` validation warnings.
>
>> + status = "okay";
>> +};
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720-imx952_frdm-v1-0-6d145eaaa5c4@nxp.com?part=2
--
Best Regards,
Joseph
^ permalink raw reply
* [PATCH RFC 4/4] of: unittest: cover /aliases updates from overlay apply/revert
From: Abdurrahman Hussain @ 2026-07-20 7:02 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
In-Reply-To: <20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai>
Add overlay_alias.dtso, which declares `testcase-alias99 = ...` under
/aliases via the &{/aliases} shorthand, and an of_unittest_overlay_alias()
runner that:
1. asserts of_alias_get_id(target, "testcase-alias") is -ENODEV
before the overlay is applied,
2. applies the overlay and asserts the same call now returns 99,
3. removes the overlay and asserts we're back to -ENODEV.
Exercises all three functional patches earlier in this series
together: patch 1's reconfig notifier is what mutates aliases_lookup
on apply/revert, patch 2 is what lets target-path="/aliases" resolve
to the DT root when target_base is non-NULL, and patch 3 is what
rewrites the fragment-internal path in the alias value so
of_find_node_by_path() finds the live-tree target. Any one of the
three missing turns the middle assertion (get_id -> 99) into
-ENODEV.
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/of/unittest-data/Makefile | 2 ++
drivers/of/unittest-data/overlay_alias.dtso | 9 +++++
drivers/of/unittest.c | 51 +++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 01a966e39f23..0a8bd9a74283 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_OF_OVERLAY) += overlay.dtbo.o \
overlay_18.dtbo.o \
overlay_19.dtbo.o \
overlay_20.dtbo.o \
+ overlay_alias.dtbo.o \
overlay_bad_add_dup_node.dtbo.o \
overlay_bad_add_dup_prop.dtbo.o \
overlay_bad_phandle.dtbo.o \
@@ -87,6 +88,7 @@ apply_static_overlay_1 := overlay_0.dtbo \
overlay_18.dtbo \
overlay_19.dtbo \
overlay_20.dtbo \
+ overlay_alias.dtbo \
overlay_gpio_01.dtbo \
overlay_gpio_02a.dtbo \
overlay_gpio_02b.dtbo \
diff --git a/drivers/of/unittest-data/overlay_alias.dtso b/drivers/of/unittest-data/overlay_alias.dtso
new file mode 100644
index 000000000000..32532c80505a
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_alias.dtso
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+/* overlay_alias - declare an alias inside an overlay */
+
+&{/aliases} {
+ testcase-alias99 = "/testcase-data/overlay-node/test-bus/test-unittest100";
+};
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index e255f54f4d76..a9f4a0bb3087 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3475,6 +3475,53 @@ static struct notifier_block of_nb = {
.notifier_call = of_notify,
};
+static void __init of_unittest_overlay_alias(void)
+{
+ const char *path = "/testcase-data/overlay-node/test-bus/test-unittest100";
+ struct device_node *np;
+ int ovcs_id = 0;
+ int id, ret;
+
+ np = of_find_node_by_path(path);
+ if (!np) {
+ unittest(0, "could not find %s for alias test\n", path);
+ return;
+ }
+
+ id = of_alias_get_id(np, "testcase-alias");
+ if (id != -ENODEV) {
+ unittest(0,
+ "of_alias_get_id() = %d before overlay, expected -ENODEV\n",
+ id);
+ goto out;
+ }
+
+ ret = overlay_data_apply("overlay_alias", &ovcs_id);
+ if (!ret) {
+ unittest(0, "overlay_alias apply failed\n");
+ goto out;
+ }
+
+ id = of_alias_get_id(np, "testcase-alias");
+ unittest(id == 99,
+ "of_alias_get_id() = %d after overlay apply, expected 99\n", id);
+
+ ret = of_overlay_remove(&ovcs_id);
+ if (ret) {
+ unittest(0, "overlay_alias remove failed, ret = %d\n", ret);
+ goto out;
+ }
+
+ id = of_alias_get_id(np, "testcase-alias");
+ unittest(id == -ENODEV,
+ "of_alias_get_id() = %d after overlay remove, expected -ENODEV\n",
+ id);
+
+ unittest(1, "overlay alias test passed\n");
+out:
+ of_node_put(np);
+}
+
static void __init of_unittest_overlay_notify(void)
{
int ovcs_id;
@@ -3649,6 +3696,8 @@ static void __init of_unittest_overlay(void)
of_unittest_overlay_gpio();
+ of_unittest_overlay_alias();
+
of_unittest_remove_tracked_overlays();
of_unittest_overlay_notify();
@@ -3848,6 +3897,7 @@ OVERLAY_INFO_EXTERN(overlay_17);
OVERLAY_INFO_EXTERN(overlay_18);
OVERLAY_INFO_EXTERN(overlay_19);
OVERLAY_INFO_EXTERN(overlay_20);
+OVERLAY_INFO_EXTERN(overlay_alias);
OVERLAY_INFO_EXTERN(overlay_gpio_01);
OVERLAY_INFO_EXTERN(overlay_gpio_02a);
OVERLAY_INFO_EXTERN(overlay_gpio_02b);
@@ -3885,6 +3935,7 @@ static struct overlay_info overlays[] = {
OVERLAY_INFO(overlay_18, 0, 0),
OVERLAY_INFO(overlay_19, 0, 0),
OVERLAY_INFO(overlay_20, 0, 0),
+ OVERLAY_INFO(overlay_alias, 0, 0),
OVERLAY_INFO(overlay_gpio_01, 0, 0),
OVERLAY_INFO(overlay_gpio_02a, 0, 0),
OVERLAY_INFO(overlay_gpio_02b, 0, 0),
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 3/4] of/overlay: rewrite /aliases path values to live-tree paths
From: Abdurrahman Hussain @ 2026-07-20 7:02 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
In-Reply-To: <20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai>
/aliases entries added by an overlay reference labeled nodes inside
the overlay via '&label' in the .dtso. dtc renders those references
as string paths at compile time, but the paths encode the overlay's
internal fragment layout (e.g. "/fragment@1/__overlay__/fpga@0/i2c@40000")
rather than the location where the node will live after apply.
Currently only /__symbols__ has its property values rewritten from
overlay-internal paths to live-tree paths by dup_and_fixup_symbol_prop().
/aliases values fall through the plain __of_prop_dup() path and are
copied byte-for-byte, so of_find_node_by_path() on such a value returns
NULL, of_alias_get_id() reports -ENODEV — and the reconfig notifier
added earlier in this series sees uninterpretable paths and can't
populate aliases_lookup for overlay-declared aliases.
The values in /aliases follow the same textual convention as
/__symbols__, so we can reuse the existing rewriter. Detect the
/aliases target node and route its properties through
dup_and_fixup_symbol_prop() as well. Fall back to a raw dup when the
value is not a rewritable fragment path (legacy string aliases like
"ttyS0" that some out-of-tree code writes verbatim).
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/of/overlay.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 654a70d5cb07..338786cef17a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -350,6 +350,19 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
if (prop)
return -EINVAL;
new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+ } else if (target->np->parent &&
+ of_node_is_root(target->np->parent) &&
+ of_node_name_eq(target->np, "aliases")) {
+ /*
+ * /aliases property values are string paths in the same
+ * "/fragment@N/__overlay__/..." form as /__symbols__.
+ * Rewrite them to live-tree paths so of_alias_get_id() can
+ * resolve them; fall back to a raw dup for non-fragment
+ * values (e.g. legacy string aliases like "ttyS0").
+ */
+ new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop);
+ if (!new_prop)
+ new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
} else {
new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL);
}
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 2/4] of/overlay: look up absolute target-paths absolutely
From: Abdurrahman Hussain @ 2026-07-20 7:02 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
In-Reply-To: <20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai>
When of_overlay_fdt_apply() is called with a non-NULL target base,
find_target() currently concatenates the base's full path with every
fragment's target-path via "%pOF%s" — so target-path="" resolves to
the base itself (the intended common case), but target-path="/foo"
resolves to "<base>/foo" (never the DT root) and target-path="/" to
"<base>/" (never a valid node at all).
That makes it impossible for a two-fragment overlay to modify one
subtree under the base and one node at the DT root — a shape that
arises naturally when a PCI-attached device wants to declare its
peripherals under dev_of_node(&pdev->dev) AND add /aliases entries
so alias-aware drivers (i2c-xiic, spi, tty, ...) can pin bus numbers.
Treat target-path as absolute whenever it is non-empty. An empty
target-path continues to mean "the target base itself", preserving
the existing shape used by drivers/misc/lan966x_pci.c and its dtso
(the only in-tree of_overlay_fdt_apply() caller today that passes a
non-NULL base).
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/of/overlay.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 08d5351746be..654a70d5cb07 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -693,7 +693,6 @@ static struct device_node *find_target(const struct device_node *info_node,
const struct device_node *target_base)
{
struct device_node *node;
- char *target_path;
const char *path;
u32 val;
int ret;
@@ -709,23 +708,22 @@ static struct device_node *find_target(const struct device_node *info_node,
ret = of_property_read_string(info_node, "target-path", &path);
if (!ret) {
- if (target_base) {
- target_path = kasprintf(GFP_KERNEL, "%pOF%s", target_base, path);
- if (!target_path)
- return NULL;
- node = of_find_node_by_path(target_path);
- if (!node) {
- pr_err("find target, node: %pOF, path '%s' not found\n",
- info_node, target_path);
- }
- kfree(target_path);
- } else {
- node = of_find_node_by_path(path);
- if (!node) {
- pr_err("find target, node: %pOF, path '%s' not found\n",
- info_node, path);
- }
- }
+ /*
+ * With a non-NULL @target_base, an empty target-path means
+ * "the target base itself" — this is the common
+ * of_overlay_fdt_apply(..., base) form used by e.g. the
+ * LAN966x PCI overlay. Any other target-path is looked up
+ * absolutely, so overlays that also need to reach the DT
+ * root (e.g. to add /aliases entries alongside a base-
+ * relative fragment) can do so with target-path="/aliases".
+ */
+ if (target_base && path[0] == '\0')
+ return of_node_get((struct device_node *)target_base);
+
+ node = of_find_node_by_path(path);
+ if (!node)
+ pr_err("find target, node: %pOF, path '%s' not found\n",
+ info_node, path);
return node;
}
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 1/4] of: incrementally update /aliases lookup on reconfig notifications
From: Abdurrahman Hussain @ 2026-07-20 7:02 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
In-Reply-To: <20260720-nh-of-alias-overlay-v1-0-f1e5d9889b30@nexthop.ai>
/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.
Fix by registering an internal OF reconfig notifier from
of_core_init() that mirrors /aliases property changes into
aliases_lookup:
OF_RECONFIG_ADD_PROPERTY -> of_alias_create(pp, kzalloc, owned=true)
OF_RECONFIG_REMOVE_PROPERTY -> of_alias_destroy(name)
OF_RECONFIG_UPDATE_PROPERTY -> destroy + create
The reconfig notifier chain fires from both direct changesets and
overlay apply/revert, so the same code path covers runtime dt
modifications and overlay-declared aliases without any overlay-
specific hook in drivers/of/overlay.c. Grant Likely suggested this
shape on Geert Uytterhoeven's 2015 RFC [1]; Geert's original hook was
in dynamic.c directly.
Match the /aliases target node structurally (name == "aliases" and
parent == root) rather than by pointer against the of_aliases global.
A system with no boot-time /aliases has of_aliases == NULL, so an
overlay that creates /aliases from scratch would otherwise be missed
from the first ATTACH_NODE onward. The ATTACH/DETACH_NODE cases also
update of_aliases lazily so subsequent consumers see it.
Factor the per-property loop body of of_alias_scan() into
of_alias_create() so the boot-time scan and the runtime notifier share
one code path. Add a one-bit @owned flag to struct alias_prop tracking
whether the entry was kmalloc'd (overlay-time) or came from the boot-
time memblock allocator via of_alias_scan(). of_alias_destroy() skips
non-owned entries, so an overlay-driven UPDATE_PROPERTY against a
boot-time alias can't kfree() memblock storage — addressing the
allocator-mismatch worry Grant flagged on the 2015 series [2].
An @owned entry also holds an of_node_get() reference to its target,
released by of_alias_destroy(); this fixes a smaller leak Geert's
original of_alias_create() would have introduced for overlay targets.
Naming builds on Geert's original series:
- "of: Extract of_alias_create()" [3]
- "of: Add of_alias_destroy()" [4]
- "of/dynamic: Update list of aliases on aliases changes" [5]
Link: https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/ [1]
Link: https://lore.kernel.org/lkml/20150630172131.D4E6CC4041A@trevor.secretlab.ca/ [2]
Link: https://lore.kernel.org/lkml/1435675876-2159-2-git-send-email-geert+renesas@glider.be/ [3]
Link: https://lore.kernel.org/lkml/1435675876-2159-3-git-send-email-geert+renesas@glider.be/ [4]
Link: https://lore.kernel.org/lkml/1435675876-2159-4-git-send-email-geert+renesas@glider.be/ [5]
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/of/base.c | 184 ++++++++++++++++++++++++++++++++++++++----------
drivers/of/of_private.h | 7 ++
2 files changed, 155 insertions(+), 36 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e7a42dedad3..d516523a71d5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1915,6 +1915,152 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np,
ap->alias, ap->stem, ap->id, np);
}
+/*
+ * Build an alias_prop for @pp using @dt_alloc as the storage allocator
+ * and add it to aliases_lookup. @owned is stored on the entry so the
+ * matching destroy path knows whether the alias_prop is a kmalloc'd
+ * struct that must be kfree()d (with a paired of_node_put on the
+ * target) or a memblock/dt_alloc'd struct that must be left alone.
+ *
+ * Pseudo-properties (name, phandle, ...) and alias names not ending in
+ * a numeric id are silently skipped.
+ */
+static void of_alias_create(const struct property *pp,
+ void *(*dt_alloc)(u64 size, u64 align),
+ bool owned)
+{
+ const char *start = pp->name;
+ const char *end = start + strlen(start);
+ struct device_node *np;
+ struct alias_prop *ap;
+ int id, len;
+
+ if (is_pseudo_property(pp->name))
+ return;
+
+ np = of_find_node_by_path(pp->value);
+ if (!np)
+ return;
+
+ while (isdigit(*(end - 1)) && end > start)
+ end--;
+ len = end - start;
+
+ if (kstrtoint(end, 10, &id) < 0)
+ goto out_put;
+
+ ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
+ if (!ap)
+ goto out_put;
+ memset(ap, 0, sizeof(*ap) + len + 1);
+ ap->alias = start;
+ ap->owned = owned;
+ of_alias_add(ap, np, id, start, len);
+ return;
+
+out_put:
+ /*
+ * Boot-time entries reach here on parse failure; leaking the
+ * of_node_get() from of_find_node_by_path() is fine because the
+ * boot tree is never freed. Overlay-time entries need the put so
+ * the target node's refcount tracks the failed-parse case
+ * symmetrically with success.
+ */
+ if (owned)
+ of_node_put(np);
+}
+
+/*
+ * Reverse of of_alias_create(): find the owned alias_prop whose name
+ * matches @name, unlink it, and release everything it owns. Non-owned
+ * (boot-time) entries are skipped so an overlay-driven UPDATE against
+ * a boot-time alias can't kfree() memblock storage. That does mean an
+ * overlay updating a boot-time alias leaves two entries in
+ * aliases_lookup — deferred as a separate cleanup.
+ */
+static void of_alias_destroy(const char *name)
+{
+ struct alias_prop *ap, *tmp;
+
+ list_for_each_entry_safe(ap, tmp, &aliases_lookup, link) {
+ if (!ap->owned || strcmp(ap->alias, name) != 0)
+ continue;
+ list_del(&ap->link);
+ of_node_put(ap->np);
+ kfree(ap);
+ return;
+ }
+}
+
+static void *alias_alloc(u64 size, u64 align)
+{
+ return kzalloc(size, GFP_KERNEL);
+}
+
+/*
+ * OF reconfig notifier that mirrors /aliases property changes into
+ * aliases_lookup. Fires on both direct changesets and overlay
+ * apply/revert, so of_alias_get_id() returns the right id for aliases
+ * declared inside an overlay.
+ */
+static int of_aliases_reconfig_notifier(struct notifier_block *nb,
+ unsigned long action, void *arg)
+{
+ struct of_reconfig_data *rd = arg;
+
+ /*
+ * Match /aliases structurally (name + root-parent) rather than by
+ * pointer against the of_aliases global — a system with no
+ * boot-time /aliases (of_aliases == NULL) can still acquire one
+ * from an overlay, and we must track its properties from the
+ * first ATTACH_NODE onward.
+ */
+ if (!rd->dn || !rd->dn->parent ||
+ !of_node_is_root(rd->dn->parent) ||
+ !of_node_name_eq(rd->dn, "aliases"))
+ return NOTIFY_DONE;
+
+ switch (action) {
+ case OF_RECONFIG_ATTACH_NODE:
+ if (!of_aliases)
+ of_aliases = rd->dn;
+ break;
+ case OF_RECONFIG_DETACH_NODE:
+ if (of_aliases == rd->dn)
+ of_aliases = NULL;
+ break;
+ case OF_RECONFIG_ADD_PROPERTY:
+ of_alias_create(rd->prop, alias_alloc, true);
+ break;
+ case OF_RECONFIG_REMOVE_PROPERTY:
+ of_alias_destroy(rd->prop->name);
+ break;
+ case OF_RECONFIG_UPDATE_PROPERTY:
+ of_alias_destroy(rd->old_prop->name);
+ of_alias_create(rd->prop, alias_alloc, true);
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block of_aliases_nb = {
+ .notifier_call = of_aliases_reconfig_notifier,
+};
+
+static int __init of_aliases_reconfig_init(void)
+{
+ return of_reconfig_notifier_register(&of_aliases_nb);
+}
+
+/*
+ * of_alias_scan() runs from of_core_init() (core_initcall), so hook the
+ * reconfig notifier one initcall level later to guarantee the initial
+ * static scan is complete before any dynamic tracking begins.
+ */
+core_initcall_sync(of_aliases_reconfig_init);
+
/**
* of_alias_scan - Scan all properties of the 'aliases' node
* @dt_alloc: An allocator that provides a virtual address to memory
@@ -1950,42 +2096,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
if (!of_aliases)
return;
- for_each_property_of_node(of_aliases, pp) {
- const char *start = pp->name;
- const char *end = start + strlen(start);
- struct device_node *np;
- struct alias_prop *ap;
- int id, len;
-
- /* Skip those we do not want to proceed */
- if (is_pseudo_property(pp->name))
- continue;
-
- np = of_find_node_by_path(pp->value);
- if (!np)
- continue;
-
- /* walk the alias backwards to extract the id and work out
- * the 'stem' string */
- while (isdigit(*(end-1)) && end > start)
- end--;
- len = end - start;
-
- if (kstrtoint(end, 10, &id) < 0) {
- of_node_put(np);
- continue;
- }
-
- /* Allocate an alias_prop with enough space for the stem */
- ap = dt_alloc(sizeof(*ap) + len + 1, __alignof__(*ap));
- if (!ap) {
- of_node_put(np);
- continue;
- }
- memset(ap, 0, sizeof(*ap) + len + 1);
- ap->alias = start;
- of_alias_add(ap, np, id, start, len);
- }
+ for_each_property_of_node(of_aliases, pp)
+ of_alias_create(pp, dt_alloc, false);
}
/**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..9d16765ae2c3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -17,6 +17,12 @@
* @alias: Alias property name
* @np: Pointer to device_node that the alias stands for
* @id: Index value from end of alias name
+ * @owned: True if @alias was kstrdup'd and @np was of_node_get'd on
+ * insertion (overlay-time entries). False for entries built
+ * by of_alias_scan() at boot, where @alias points into the
+ * FDT and @np is an unreferenced pointer. The removal path
+ * uses this flag to decide whether it must kfree(@alias),
+ * of_node_put(@np), and kfree(the struct itself).
* @stem: Alias string without the index
*
* The structure represents one alias property of 'aliases' node as
@@ -27,6 +33,7 @@ struct alias_prop {
const char *alias;
struct device_node *np;
int id;
+ bool owned;
char stem[];
};
--
2.54.0
^ permalink raw reply related
* [PATCH RFC 0/4] of: teach overlay code to keep /aliases in sync
From: Abdurrahman Hussain @ 2026-07-20 7:02 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
/aliases entries added by a device-tree overlay are stored in the live
tree but never enter the global aliases_lookup list that of_alias_scan()
builds at boot. As a result, of_alias_get_id() returns -ENODEV for
aliases declared inside overlays, and any driver that relies on
alias-based numbering (i2c-xiic, spi, tty, mmc, ...) silently loses its
pinned id and falls back to auto-assignment.
The gap has been public since 2015 [1] and reproduces trivially: apply
an overlay that declares e.g. `i2c99 = &foo;`, ask
of_alias_get_id(foo, "i2c") -> -ENODEV. Bootlin's ELCE 2025 talk on
PCI DT overlays [2] enumerates "i2c muxes" as one of the subsystems
broken by dynamic overlays; alias pinning is the underlying cause.
The core fix (patch 1) is a reconfig notifier that mirrors /aliases
property changes into aliases_lookup. Two smaller overlay-code fixes
(patches 2 and 3) fall out of the same use case: without them, the
notifier alone can't actually resolve overlay-declared aliases.
Prior art
---------
Geert Uytterhoeven posted a 3-patch RFC in June 2015 [1] with the same
alias-tracking design shape. Grant Likely reviewed positively; merge
was gated on missing unittests and an object-lifetime concern the
author self-flagged, and the series was never reposted as non-RFC. Ten
years later, drivers/of/overlay.c still contains zero references to
aliases, of_alias_scan, or aliases_lookup.
Series contents
---------------
Patch 1 adds a reconfig notifier that mirrors /aliases property
changes into aliases_lookup. It also refactors of_alias_scan()'s
per-property body into a helper of_alias_create() shared by the
boot-time scan and the runtime notifier. A one-bit `owned` flag on
struct alias_prop distinguishes kmalloc'd (overlay-time) entries from
memblock-backed (boot-time) ones so the remove path can't kfree the
wrong storage — addressing the lifetime concern that stopped Geert in
2015. The notifier keys off structural properties of the target node
(name + root-parent) rather than the of_aliases global, so overlays
that create /aliases from scratch on a system without a boot-time
aliases node are covered too.
Patch 2 fixes find_target() so an overlay applied with a non-NULL
target base can still reach the DT root via target-path="/foo". The
current code unconditionally concatenates base + target-path via
"%pOF%s", so target-path="/aliases" resolves to "<base>/aliases" and
target-path="/" produces "<base>/" (never a valid node). After this
patch, an empty target-path continues to mean "the target base
itself" — preserving the shape used by drivers/misc/lan966x_pci.c,
the only in-tree caller of of_overlay_fdt_apply() that passes a
non-NULL base — while any non-empty target-path is looked up
absolutely.
Patch 3 rewrites /aliases property values from the overlay's internal
fragment path ("/fragment@N/__overlay__/...") to the live-tree path
that the node will occupy after apply. The overlay code already does
this for /__symbols__ via dup_and_fixup_symbol_prop(); /aliases uses
the same textual convention and can reuse the same helper. Without
this, patch 1's notifier receives paths that never resolve in the
live tree, so of_alias_get_id() still returns -ENODEV.
Patch 4 adds a unittest — overlay_alias.dtso plus
of_unittest_overlay_alias() — that applies an overlay declaring
`testcase-alias99` under /aliases and asserts of_alias_get_id() flips
from -ENODEV to 99 across the apply, and back to -ENODEV across the
revert.
Open items — feedback wanted before v1
--------------------------------------
Locking. of_alias_get_id() traverses aliases_lookup without a lock,
which was safe when the list was populated once at boot. Runtime
add/remove requires synchronization; this series does NOT yet add
any. Options:
(a) protect aliases_lookup with a spinlock (simple, sheds the
lockless-lookup contract);
(b) convert to list_head_rcu + synchronize_rcu on remove (keeps
lookup lockless, more churn);
(c) piggyback on of_mutex — but of_alias_get_id() is called from
driver probe context, so grabbing of_mutex there risks lock
inversion.
Currently leaning (b); happy to be talked out of it.
Verification
------------
Series was applied to v7.2-rc3 and boot-tested on an x86 platform
with two PCI-attached Xilinx FPGAs whose i2c-xiic controllers come
from driver-embedded DT overlays. Before this series, i2c bus
numbers auto-assigned regardless of what /aliases said and shifted
across boots depending on which FPGA won the probe race. After: 89
adapters, `/dev/i2c-N` numbering matches the overlay aliases exactly
and is stable across reboots, `sensors` reads live telemetry through
mux channels, and dmesg is free of WARN/BUG/Oops. Patch 4's unittest
passes.
[1] https://lore.kernel.org/lkml/1435675876-2159-1-git-send-email-geert+renesas@glider.be/
Geert Uytterhoeven, "[PATCH/RFC 0/3] of/overlay: Update aliases
when added or removed", 2015-06-30.
[2] Hervé Codina, "Using Device Tree Overlays to Support Complex PCI
Devices in Linux", ELCE 2025.
https://bootlin.com/pub/conferences/2025/elce/codina-pcie-dt-overlay.pdf
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (4):
of: incrementally update /aliases lookup on reconfig notifications
of/overlay: look up absolute target-paths absolutely
of/overlay: rewrite /aliases path values to live-tree paths
of: unittest: cover /aliases updates from overlay apply/revert
drivers/of/base.c | 184 ++++++++++++++++++++++------
drivers/of/of_private.h | 7 ++
drivers/of/overlay.c | 47 ++++---
drivers/of/unittest-data/Makefile | 2 +
drivers/of/unittest-data/overlay_alias.dtso | 9 ++
drivers/of/unittest.c | 51 ++++++++
6 files changed, 246 insertions(+), 54 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260719-nh-of-alias-overlay-6e5e0d56212b
Best regards,
--
Abdurrahman Hussain <abdurrahman@nexthop.ai>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree
From: sashiko-bot @ 2026-07-20 6:55 UTC (permalink / raw)
To: Joseph Guo; +Cc: robh, Frank.Li, conor+dt, imx, devicetree
In-Reply-To: <20260720-imx952_frdm-v1-2-6d145eaaa5c4@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Shared GPIO requested exclusively by two CAN transceivers causes probe failure for the second transceiver.
- [Low] Use of undocumented, downstream-only device tree property `fsl,cd-gpio-wakeup-disable`.
--
--- Patch [2]: [PATCH 2/2] arm64: dts: freescale: add i.MX952 FRDM Board support device tree ---
commit 8bbcb28d08a05e0f4c57023333492b4e023f7caf
Author: Joseph Guo <qijian.guo@nxp.com>
arm64: dts: freescale: add i.MX952 FRDM Board support device tree
Add basic device tree support for i.MX952 FRDM Board. The board-common hardware
description is placed in imx952-frdm.dtsi so that it can be shared with future
i.MX952-based FRDM derivative boards.
> diff --git a/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> new file mode 100644
> index 0000000000000..b9ef6bec2bd5a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx952-frdm.dtsi
> @@ -0,0 +1,727 @@
[ ... ]
> + flexcan2_phy: can-phy0 {
> + compatible = "nxp,tja1051";
> + #phy-cells = <0>;
> + max-bitrate = <8000000>;
> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
> + };
> +
> + flexcan3_phy: can-phy1 {
> + compatible = "nxp,tja1051";
> + #phy-cells = <0>;
> + max-bitrate = <8000000>;
> + silent-gpios = <&pcal6416 6 GPIO_ACTIVE_HIGH>;
[Severity: High]
Does this cause a probe failure for the second transceiver?
The first PHY node successfully requests this GPIO line, taking exclusive
ownership. Because the phy-can-transceiver driver uses standard exclusive
requests (without GPIOD_FLAGS_BIT_NONEXCLUSIVE), the second request for the
exact same GPIO line fails with -EBUSY.
This prevents the flexcan3_phy and the flexcan3 controller from probing and
functioning.
> + };
[ ... ]
> +&usdhc2 {
> + bus-width = <4>;
> + cd-gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
> + pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-3 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
> + vmmc-supply = <®_usdhc2_vmmc>;
> + fsl,cd-gpio-wakeup-disable;
[Severity: Low]
Is this out-of-tree vendor property invalid in the upstream kernel?
The property fsl,cd-gpio-wakeup-disable does not exist in any upstream device
tree bindings (such as fsl-imx-esdhc.yaml) and is not handled by any driver
code in the upstream Linux tree. This acts as dead code and will trigger
`make dtbs_check` validation warnings.
> + status = "okay";
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-imx952_frdm-v1-0-6d145eaaa5c4@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v5 2/3] dt-bindings: display: sn65dsi83: Add output data-lanes property
From: Wojciech Dubowik @ 2026-07-20 6:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Marek Vasut, dri-devel, devicetree
In-Reply-To: <20260720-international-industrious-hippogriff-dd24ee@quoll>
On Mon, Jul 20, 2026 at 07:51:10AM +0200, Krzysztof Kozlowski wrote:
> On Fri, Jul 17, 2026 at 01:50:00PM +0200, Wojciech Dubowik wrote:
> > From: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> >
> > Add an optional output lvds data lanes property with two allowed values.
> > The array <1 2 3 4> for standard layout and <4 3 2 1> for reversed lvds
> > output lanes. The latter informs the driver that reverse lvds config
> > option has to be set in config register for the respective output channel.
> >
> > Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
>
> Well, I am not doing the work twice (nothing in cover letter explains
> why I should, IOW, why you decided to drop my review).
Sorry. Missed it. I will add reviewed-by-you tag in the nest interation.
Regards,
Wojtek
>
> Dropping this patch from Patchwork.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v2 3/3] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Krzysztof Kozlowski @ 2026-07-20 6:54 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-4-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:32PM +0000, Eduard Bostina wrote:
+
> + patternProperties:
> + "^regulator@[0-6]$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + regulator-compatible:
> + $ref: /schemas/types.yaml#/definitions/string
You should not need this property, it is already in regulator.yaml.
> +
> + charger:
> + $ref: /schemas/power/supply/tps65217-charger.yaml#
unevaluatedProperties: false
> +
> + pwrbutton:
> + $ref: /schemas/input/ti,tps65217-pwrbutton.yaml#
unevaluatedProperties: false
> +
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + tps@24 {
pmic@24
Please make the example mostly complete - add rest of properties and nodes.
> + reg = <0x24>;
> + compatible = "ti,tps65217";
> +
> + backlight {
> + isel = <1>; /* 1 - ISET1, 2 ISET2 */
> + fdim = <100>; /* TPS65217_BL_FDIM_100HZ */
> + default-brightness = <50>;
> + };
> + };
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: input: Convert TI TPS65217 power button to DT schema
From: Krzysztof Kozlowski @ 2026-07-20 6:51 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-3-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:31PM +0000, Eduard Bostina wrote:
> Convert the Texas Instruments TPS65217 and TPS65218 Power Button
> bindings to DT schema.
>
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
> ---
> .../bindings/input/ti,tps65217-pwrbutton.yaml | 42 +++++++++++++++++++
> .../bindings/input/tps65218-pwrbutton.txt | 30 -------------
> 2 files changed, 42 insertions(+), 30 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/ti,tps65217-pwrbutton.yaml
> delete mode 100644 Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v10 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC
From: sashiko-bot @ 2026-07-20 6:50 UTC (permalink / raw)
To: Jianhua Lin; +Cc: media-ci, robh, conor+dt, devicetree
In-Reply-To: <20260720062829.7343-4-jianhua.lin@mediatek.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Medium] Missing DMA Mask Configuration for 34-bit IOVA support.
Pre-existing issues:
- [High] Use-After-Free when unbound while a V4L2 file descriptor is held open.
- [High] Use-After-Free due to uncanceled job timeout delayed work during unbind.
- [High] NULL Pointer Dereference due to interrupt request before memory-to-memory context initialization.
- [High] DMA Address Calculation Bug for 34-bit stall address extension.
--
--- Patch [3]: [PATCH v10 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC ---
commit 3c5efcbd1b6f6e5dab41b7f01500bcc550eb3c09
Author: Jianhua Lin <jianhua.lin@mediatek.com>
media: mediatek: jpeg: add compatible for MT8189 SoC
This commit adds new compatible strings and specific driver data to support
the JPEG encoder and decoder on the MT8189 SoC, which uses a 34-bit IOVA
address space.
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec4830811..14f2991a40535 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
[ ... ]
> @@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
> .multi_core = false,
> };
>
> +static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
> + .clks = mtk_jpeg_clocks,
> + .num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
> + .formats = mtk_jpeg_enc_formats,
> + .num_formats = MTK_JPEG_ENC_NUM_FORMATS,
> + .qops = &mtk_jpeg_enc_qops,
> + .irq_handler = mtk_jpeg_enc_irq,
> + .hw_reset = mtk_jpeg_enc_reset,
> + .m2m_ops = &mtk_jpeg_enc_m2m_ops,
> + .dev_name = "mtk-jpeg-enc",
> + .ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
> + .out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
> + .cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> + .support_34bit = true,
[Severity: Medium]
Will the 34-bit addressing hardware capability be silently disabled by the
kernel's DMA API?
The driver sets .support_34bit = true for the new MT8189 variants, intending
to allow the hardware to address up to 16GB.
However, the driver never calls dma_set_mask_and_coherent() or dma_set_mask()
in the probe sequence. Without this call, the kernel's DMA APIs assume the
default 32-bit mask (4GB) for platform devices.
Does the driver need to configure the DMA mask during initialization when
support_34bit is enabled?
[Severity: High]
This is a pre-existing issue, but does enabling .support_34bit = true here
activate a buggy DMA address calculation path in mtk_jpeg_set_enc_dst()?
Looking at drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c, the 34-bit
extension logic correctly calculates the base 2-bit extension:
addr_ext = FIELD_PREP(MTK_JPEG_ADDR_MASK, upper_32_bits(dma_addr));
But for the stall address, it erroneously writes addr_ext + size:
writel(addr_ext + size, base + JPEG_ENC_STALL_ADDR0_EXT);
Since size is the byte size of the buffer (e.g., 0x20000), wouldn't this write
an improperly large value into a register meant for the upper 2 bits of the
address, rather than calculating upper_32_bits(dma_addr + size) & 3?
Could this lead to memory corruption or IOMMU faults by incorrectly setting
the upper address bits for the hardware DMA?
> +};
> +
> +static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
> + .clks = mtk_jpeg_dec_clocks,
> + .num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
> + .formats = mtk_jpeg_dec_formats,
> + .num_formats = MTK_JPEG_DEC_NUM_FORMATS,
> + .qops = &mtk_jpeg_dec_qops,
> + .irq_handler = mtk_jpeg_dec_irq,
> + .hw_reset = mtk_jpeg_dec_reset,
> + .m2m_ops = &mtk_jpeg_dec_m2m_ops,
> + .dev_name = "mtk-jpeg-dec",
> + .ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
> + .out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> + .cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
> + .support_34bit = true,
> +};
[ ... ]
> @@ -1938,6 +1974,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
> .compatible = "mediatek,mtk-jpgenc",
> .data = &mtk_jpeg_drvdata,
> },
> + {
> + .compatible = "mediatek,mt8189-jpgenc",
> + .data = &mtk8189_jpegenc_drvdata,
> + },
[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
if the device is unbound while a userspace application holds the V4L2
character device file descriptor open?
The struct mtk_jpeg_dev pointer is allocated via devm_kzalloc() and its
lifetime is tied to the platform device. In mtk_jpeg_remove(), the driver
unregisters the V4L2 device and explicitly releases m2m_dev:
mtk_jpeg_remove() {
...
v4l2_m2m_release(jpeg->m2m_dev);
v4l2_device_unregister(&jpeg->v4l2_dev);
}
The core structure is freed immediately by devres when remove() returns.
When the application later closes the file, mtk_jpeg_release() is called and
accesses the devres-freed jpeg structure:
mtk_jpeg_release() {
struct mtk_jpeg_dev *jpeg = video_drvdata(file);
...
mutex_lock(&jpeg->lock);
...
}
Should the driver wait for file operations to complete or manage the lifetime
of the jpeg structure differently?
[Severity: High]
This isn't a bug introduced by this patch, but does unbinding the device while
a hardware job is in flight cause a use-after-free via the job timeout delayed
work?
mtk_jpeg_single_core_init() initializes a delayed work jpeg->job_timeout_work.
When hardware processing starts, this work is scheduled.
However, mtk_jpeg_remove() fails to call cancel_delayed_work_sync() for
jpeg->job_timeout_work.
If the device is unbound before the timeout expires, the struct mtk_jpeg_dev
will be freed by devres. When the timer eventually expires,
mtk_jpeg_job_timeout_work() will dereference the freed jpeg pointer.
Should mtk_jpeg_remove() cancel this work before returning?
[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of a NULL
pointer dereference if a hardware interrupt fires immediately after the IRQ
is requested?
In mtk_jpeg_single_core_init(), the driver calls devm_request_irq() to
register and enable the IRQ handler.
However, jpeg->m2m_dev is not initialized until mtk_jpeg_probe() calls
v4l2_m2m_init() later in the execution flow.
If the bootloader leaves the hardware in an active or errored state, the
interrupt could fire instantly. mtk_jpeg_enc_irq() or mtk_jpeg_dec_irq() will
invoke mtk_jpeg_enc_done(), which attempts to fetch the current context:
ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
Since jpeg->m2m_dev is still NULL at this point, will this dereference a NULL
pointer?
> + {
> + .compatible = "mediatek,mt8189-jpgdec",
> + .data = &mtk8189_jpegdec_drvdata,
> + },
> {
> .compatible = "mediatek,mt8195-jpgenc",
> .data = &mtk8195_jpegenc_drvdata,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720062829.7343-1-jianhua.lin@mediatek.com?part=3
^ permalink raw reply
* Re: [PATCH v2 1/3] ARM: dts: ti: am335x-guardian: Drop unused isink-en property
From: Krzysztof Kozlowski @ 2026-07-20 6:49 UTC (permalink / raw)
To: Eduard Bostina
Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
m-chawdhry
In-Reply-To: <20260718215632.3295049-2-egbostina@gmail.com>
On Sat, Jul 18, 2026 at 09:56:30PM +0000, Eduard Bostina wrote:
> The 'isink-en' property is not read by any driver, so remove it from the
"...is not documented and not read by any driver..."
> TPS65217 backlight node.
With this change:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ 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