All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Abdurrahman Hussain <abdurrahman@nexthop.ai>,
	conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH] of: property: add i2c-parent to fw_devlink supplier bindings
Date: Fri, 14 Aug 2026 13:15:10 -0500	[thread overview]
Message-ID: <20260814181510.GH1981719-robh@kernel.org> (raw)
In-Reply-To: <20260814010950.051261F000E9@smtp.kernel.org>

On Fri, Aug 14, 2026 at 01:09:49AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] fw_devlink incorrectly parses I2C slave addresses as phandles due to 0-cell supplier binding definition for `i2c-parent`.
> --
> 
> --- Patch [1]: [PATCH] of: property: add i2c-parent to fw_devlink supplier bindings ---
> commit 85f34a66eac871d9e36bccb6795b94b17ab065d4
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 
> of: property: add i2c-parent to fw_devlink supplier bindings
> 
> This commit teaches fw_devlink about i2c-parent so the core creates the
> corresponding device links. This ensures consumers are unbound before the
> parent adapter's device, removing the need for -EPROBE_DEFER.
> 
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1409,6 +1409,7 @@ DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
> >  DEFINE_SIMPLE_PROP(leds, "leds", NULL)
> >  DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
> >  DEFINE_SIMPLE_PROP(panel, "panel", NULL)
> > +DEFINE_SIMPLE_PROP(i2c_parent, "i2c-parent", NULL)
> 
> [Severity: High]
> Will this correctly handle device tree bindings where the i2c-parent
> property includes a slave address argument?
> 
> For example, platforms using devices like the Toshiba TC9563 PCIe switch
> might use a property such as: i2c-parent = <&qup_i2c 0x77>;
> 
> By passing NULL as the cells argument to DEFINE_SIMPLE_PROP(), the parser
> assumes a 0-cell list. When fw_devlink iterates over the property, it finds
> the first phandle correctly, but won't it erroneously interpret the slave
> address argument (0x77 / 119) as a second, independent 0-cell phandle?
> 
> If another unrelated device tree node happens to match this phandle ID,
> could this cause fw_devlink to link the consumer to the wrong device,
> potentially leading to spurious probe deferrals or circular dependencies?

I think you need custom parsing for this one to read the first cell and 
ignore the 2nd cell if present.

Rob

  reply	other threads:[~2026-08-14 18:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 21:09 [PATCH] of: property: add i2c-parent to fw_devlink supplier bindings Abdurrahman Hussain
2026-08-14  1:09 ` sashiko-bot
2026-08-14 18:15   ` Rob Herring [this message]
2026-08-14 19:13     ` Abdurrahman Hussain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260814181510.GH1981719-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=abdurrahman@nexthop.ai \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.