Re: [PATCH v2] of: property: add i2c-parent to fw_devlink supplier bindings

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 <[email protected]>
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