* [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings
@ 2026-08-15 23:37 Abdurrahman Hussain
2026-08-15 23:48 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Abdurrahman Hussain @ 2026-08-15 23:37 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan
Cc: devicetree, linux-kernel, Abdurrahman Hussain
Platform-device i2c muxes (i2c-mux-reg, i2c-mux-gpio, ...) reference
the bus they multiplex with an i2c-parent phandle and hold a reference
on that adapter from probe until remove. Unlike muxes that are clients
on the parent bus, they live outside the adapter's device hierarchy,
so nothing orders their teardown against it: if the adapter's device
is unbound first — e.g. while reverting a device-tree overlay whose
changeset attached the mux nodes before the controller nodes —
i2c_del_adapter() blocks forever in
wait_for_completion(&adap->dev_released), waiting for a reference that
is only dropped later in the same teardown sequence.
Teach fw_devlink about i2c-parent so the core creates the
corresponding device links: consumers are unbound before the parent
adapter's device, and probe ordering no longer needs -EPROBE_DEFER.
A plain DEFINE_SIMPLE_PROP() cannot be used because the property has
two incompatible layouts: the i2c mux bindings hold a list of bare
phandles (i2c-demux-pinctrl takes several), while toshiba,tc9563 holds
a single phandle followed by an i2c slave-address cell. The two forms
are indistinguishable in the flattened tree, and a 0-cell parse of the
tc9563 form would read the slave address as a phandle, linking the
consumer to whatever node happens to carry that phandle value. Use a
custom parser that only takes entry 0 for toshiba,tc9563 nodes.
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Changes in v2:
- Use a custom parser instead of DEFINE_SIMPLE_PROP: toshiba,tc9563 puts
an i2c slave-address cell after the phandle, which a 0-cell parse
would misread as a phandle (Sashiko AI review, Rob). Kept the full
phandle-list parse for other users since i2c-demux-pinctrl takes
several parents; only tc9563 is limited to entry 0
- Move the Signed-off-by into the commit message proper; v1 had it below
the '---' cutter line where it would be dropped on apply
- Link to v1: https://patch.msgid.link/20260813-b4-of-property-add-i2c-parent-v1-1-a2487e920ce1@nexthop.ai
To: Rob Herring <robh@kernel.org>
To: Saravana Kannan <saravanak@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/of/property.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 72cf12907de0..e9ceb26c0300 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1471,6 +1471,22 @@ static struct device_node *parse_gpio_compat(struct device_node *np,
return sup_args.np;
}
+static struct device_node *parse_i2c_parent(struct device_node *np,
+ const char *prop_name, int index)
+{
+ if (strcmp(prop_name, "i2c-parent"))
+ return NULL;
+
+ /* toshiba,tc9563 is <phandle addr>; every other user is bare phandles */
+ if (of_device_is_compatible(np, "toshiba,tc9563")) {
+ if (index)
+ return NULL;
+ return of_parse_phandle(np, prop_name, 0);
+ }
+
+ return of_parse_phandle(np, prop_name, index);
+}
+
static struct device_node *parse_interrupts(struct device_node *np,
const char *prop_name, int index)
{
@@ -1562,6 +1578,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
{ .parse_prop = parse_resets, },
{ .parse_prop = parse_leds, },
{ .parse_prop = parse_backlight, },
+ { .parse_prop = parse_i2c_parent, },
{ .parse_prop = parse_panel, },
{ .parse_prop = parse_msi_parent, },
{ .parse_prop = parse_pses, },
---
base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
change-id: 20260813-b4-of-property-add-i2c-parent-168c6d7f1934
Best regards,
--
Abdurrahman Hussain <abdurrahman@nexthop.ai>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings
2026-08-15 23:37 [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings Abdurrahman Hussain
@ 2026-08-15 23:48 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-08-15 23:48 UTC (permalink / raw)
To: Abdurrahman Hussain; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The custom `i2c-parent` parser uses an incorrect compatible string (`toshiba,tc9563`) to detect TC9563 devices, bypassing the custom parsing logic for actual hardware.
--
--- Patch [1]: [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings ---
commit 53359fe4a5d6b17938d753a47bed08284aafa03a
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Subject: of: property: add i2c-parent to fw_devlink supplier bindings
This commit teaches fw_devlink about the i2c-parent property so consumers
are unbound before the parent adapter's device, ensuring proper teardown
ordering. It introduces a custom parser for i2c-parent to handle differing
layouts, specifically to accommodate the toshiba,tc9563 node which holds a
phandle followed by an i2c slave-address cell.
Link: https://patch.msgid.link/20260813-b4-of-property-add-i2c-parent-v1-1-a2487e920ce1@nexthop.ai
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 72cf12907de03..e9ceb26c03000 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
[ ... ]
> @@ -1471,6 +1471,22 @@ static struct device_node *parse_gpio_compat(struct device_node *np,
> return sup_args.np;
> }
>
> +static struct device_node *parse_i2c_parent(struct device_node *np,
> + const char *prop_name, int index)
> +{
> + if (strcmp(prop_name, "i2c-parent"))
> + return NULL;
> +
> + /* toshiba,tc9563 is <phandle addr>; every other user is bare phandles */
> + if (of_device_is_compatible(np, "toshiba,tc9563")) {
[Severity: High]
Is this the correct compatible string for the TC9563 PCIe switch?
According to the device tree bindings, actual hardware nodes for the
TC9563 use the compatible string "pci1179,0623" rather than
"toshiba,tc9563".
If the compatible string doesn't match, this check will fail during boot
and fall through to the bare phandle parser. Will this cause fw_devlink
to parse the second cell of the property (the I2C slave address, e.g.,
0x77) as a phandle?
If another node happens to have that value as its phandle, it appears
this could create a spurious device link that might block the PCIe
switch from probing or create a dependency cycle.
> + if (index)
> + return NULL;
> + return of_parse_phandle(np, prop_name, 0);
> + }
> +
> + return of_parse_phandle(np, prop_name, index);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-b4-of-property-add-i2c-parent-v2-1-239c6da9e097@nexthop.ai?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-15 23:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15 23:37 [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings Abdurrahman Hussain
2026-08-15 23:48 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox