From: Frank Li <Frank.li@oss.nxp.com>
To: Conor Dooley <conor@kernel.org>
Cc: Michal Simek <michal.simek@amd.com>,
Shubham Patil <shubhamsanjay.patil@amd.com>,
alexandre.belloni@bootlin.com, Frank.Li@kernel.org,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
linux-i3c@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, git@amd.com
Subject: Re: [PATCH v3 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties
Date: Tue, 22 Sep 2026 17:21:26 -0500 [thread overview]
Message-ID: <arL_Zgi1r1IFpNIO@SMW015318> (raw)
In-Reply-To: <20260922-unadvised-stalling-5cbeff8f982a@spud>
On Tue, Sep 22, 2026 at 10:29:31PM +0100, Conor Dooley wrote:
> On Tue, Sep 22, 2026 at 01:35:04PM -0500, Frank Li wrote:
> > On Tue, Sep 22, 2026 at 06:24:52PM +0100, Conor Dooley wrote:
> > > On Tue, Sep 22, 2026 at 11:25:30AM -0500, Frank Li wrote:
> > > > On Tue, Sep 22, 2026 at 10:39:23AM +0100, Conor Dooley wrote:
> > > > > On Tue, Sep 22, 2026 at 10:38:17AM +0100, Conor Dooley wrote:
> > > > > > On Tue, Sep 22, 2026 at 10:33:44AM +0100, Conor Dooley wrote:
> > > > > > > On Thu, Sep 17, 2026 at 01:23:19PM +0200, Michal Simek wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > On 9/10/26 17:44, Frank Li wrote:
> > > > > > > > > On Thu, Sep 10, 2026 at 12:36:45PM +0100, Conor Dooley wrote:
> > > > > > > > > > On Wed, Sep 09, 2026 at 11:21:41AM -0500, Frank Li wrote:
> > > > > > > > > > > On Tue, Sep 08, 2026 at 06:54:31PM +0100, Conor Dooley wrote:
> > > > > > > > > > > > On Tue, Sep 08, 2026 at 03:12:55PM +0530, Shubham Patil wrote:
> > > > > > > > > > > > > In-Band Interrupt and Hot-Join are synthesis-time options of the AXI I3C
> > > > > > > > > > > > > IP. Describe them with two boolean properties.
> > > > > > > > > > > > >
> > > > > > > > > > > > > A Hot-Join request is acknowledged by the IBI machinery, so a hot-join
> > > > > > > > > > > > > capable design is always IBI capable as well. Both events are reported
> > > > > > > > > > > > > through the controller interrupt, which is therefore required whenever
> > > > > > > > > > > > > the capability is present.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > Changes in V3:
> > > > > > > > > > > > > - Move in-band-interrupt-capable and hot-join-capable into the common
> > > > > > > > > > > > > i3c.yaml schema and drop the xlnx, prefix.
> > > > > > > > > > > > > - Keep dependencies in the AMD binding.
> > > > > > > > > > > > > - Update the commit description accordingly.
> > > > > > > > > > > > > - Conor Dooley acked v2 with the xlnx,-prefixed properties in the AMD
> > > > > > > > > > > > > binding [1].
> > > > > > > > > > > > > That Acked-by is not carried here: the names lost the vendor prefix
> > > > > > > > > > > > > and the definitions moved to i3c.yaml after Frank Li's comment.
> > > > > > > > > > > > > [1]:https://lore.kernel.org/all/20260824-tightwad-impose-495476599087@spud/
> > > > > > > > > > > >
> > > > > > > > > > > > I disagree with Frank. These properties make sense for Xilinx because it
> > > > > > > > > > > > is an FPGA IP and synthesis options impact this. For other devices, this
> > > > > > > > > > > > should be determined from the compatible.
> > > > > > > > > > > > Please revert to how things were done in v2, especially as no rationale
> > > > > > > > > > > > was provided for why these should be common.
> > > > > > > > > > >
> > > > > > > > > > > It is common problems, when IP intergrate by SOC, which may defeature some
> > > > > > > > > > > part, It is not appeared now just because IBI and HJ have not enabled
> > > > > > > > > > > widely.
> > > > > > > > > > >
> > > > > > > > > > > IBI and HJ is optional features of I3C. Ideally it should be indicated by
> > > > > > > > > > > some registers. But not all vendor implement provide this CAP registers.
> > > > > > > > > > >
> > > > > > > > > > > IBI and HJ depend on some slow clocks, which monitor SDA line change.
> > > > > > > > > > > Some instances of IP may not have such slow clocks. Some IP's IBI and HJ
> > > > > > > > > > > use seperate IRQ line, but these irq line may not connect of difference
> > > > > > > > > > > instances.
> > > > > > > > > >
> > > > > > > > > > All of this should be able to be dealt with by appropriate use of
> > > > > > > > > > specific compatibles.
> > > > > > > > >
> > > > > > > > > I understand compatible can cover most cases. Need variance for property.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > like previous SPI vendor customized property, we takes efforts to convert
> > > > > > > > > > > to common one and also meet back compatiblity problem at convert. I don't
> > > > > > > > > > > want to do it again. This kind property is most likely as below.
> > > > > > > > > >
> > > > > > > > > > What SPI controller specific properties are you talking about here?
> > > > > > > > > > There are relatively few properties in spi-controller.yaml, and none of
> > > > > > > > > > them deal with these kinds of capabilities.
> > > > > > > > >
> > > > > > > > > num-cs vs fsl,espi-num-chipselects. Total number CS of IP is fixed, but
> > > > > > > > > some instances have not route all CS to pad.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > default: decide by comaptible string or hardware cap
> > > > > > > > > > > force-disabled: force disable for some reason, like, miss connect irq line
> > > > > > > > > > > or missed some clock, or IP bugs, or board desgin's some level shift chip
> > > > > > > > > > > broken IBI/HJ timing requirements.
> > > > > > > > > >
> > > > > > > > > > Of these, only the last would be a valid reason for having a property
> > > > > > > > > > for it. Missing interrupts, clocks or IP bugs should all be dealt with
> > > > > > > > > > using device specific compatibles.
> > > > > > > > > > If board wiring causes the breakage, the property may be more
> > > > > > > > > > appropriate at the i3c device level rather than the controller given
> > > > > > > > > > that wiring to some devices on the bus may not have the problems?
> > > > > > > > >
> > > > > > > > > I3C.yaml is for both master controller and devices now. I3C is bus, which
> > > > > > > > > connect many devices, if wiring issue, whole bus can't support IBI. And
> > > > > > > > > if any broken devices happen at address arbitation, whole bus can't support
> > > > > > > > > IBI.
> > > > > > > > >
> > > > > > > > > at beging, I suggest 3 state,
> > > > > > > > >
> > > > > > > > > [default, enable, disable], but now I think IBI_broken, HJ_broken is more
> > > > > > > > > reasonable to disable it, default value should be set by compatible
> > > > > > > > > string or DCR of I3C regiser.
> > > > > > > > >
> > > > > > > > > And some I3C device may be failure to work with IBI even DCR of I3C register
> > > > > > > > > show it support IBI.
> > > > > > > > >
> > > > > > > > > I don't want to appear two similar property between vendor and common, like
> > > > > > > > > num-cs vs fsl,espi-num-chipselects.
> > > > > > > > >
> > > > > > > > > such as IBI-broken can be used for controller and devices case.
> > > > > > > > >
> > > > > > > > > > That said, I think that problem should be dealt with when it arises,
> > > > > > > > > > rather than starting a trend of adding capabilities properties at the
> > > > > > > > > > controller level when I am not convinced that there's going to be other
> > > > > > > > > > users in the same vein.
> > > > > > > > >
> > > > > > >
> > > > > > > > > Understand, I3C is realtive new protocal. 'IBI-broken' is more easy
> > > > > > > > > understand, logically equial to in-band-interrupt-capable.
> > > > > > > > Conor: Any update on this one? I think this thread is stuck at this stage.
> > > > > > >
> > > > > > > I didn't think there was any need to reply. I took the first sentence of
> > > > > > > this snippet to be acceptance of what I was saying.
> > > > > > >
> > > > > > > If yous desperately want to have a generic property, the negative
> > > > > > > connotation of the "-broken" is probably better in that it'd be more
> > > > > > > likely to make people set stuff by compatible rather than have to use a
> > > > > > > property with that word in it.
> > > > > >
> > > > > > That said, technically this platform could be both
> > > > > > "amd,in-band-interrupt-capable" and "ibi-broken" at the same time, since
> > > > > > one describes the way the IP has been compiled and the other wiring. I'm
> > > > > > not entirely sure whether conflating the two is possible? Depends on if
> > > > > > your IP's programming model changes if the option is enabled even if it
> > > > > > cannot be used. Not beyond the realms of possibility.
> > > > > >
> > > > > > Also, ibi-broken doesn't work for your platform, since the default
> > > > > > before this patch is no ibi and requiring a property for no ibi would be
> > > > > > an ABI break.
> > > > >
> > > > > The perils of FPGA IP I suppose, and not being quite careful enough to
> > > > > document all of the relevant options from the start. Been guilty of that
> > > > > myself.
> > > > >
> > > >
> > > > Check impliment code
> > > >
> > > > + /*
> > > > + * The interrupt only carries IBI events, so it is only described for
> > > > + * designs synthesized with that feature.
> > > > + */
> > > > + if (master->ibi_capable) {
> > > > + xi3c_master_init_ibi_ops(master);
> > > > +
> > > > + master->irq = platform_get_irq(pdev, 0);
> > > > + if (master->irq < 0)
> > > > + return master->irq;
> > > > +
> > > > + ret = devm_request_irq(master->dev, master->irq,
> > > > + xi3c_master_irq_handler, IRQF_NO_AUTOEN,
> > > > + dev_name(master->dev), master);
> > > > + if (ret)
> > > > + return dev_err_probe(master->dev, ret,
> > > > + "Failed to request IRQ\n");
> > > > + }
> > > >
> > > > They can use master->irq is determinate if support IBI.
> > >
> > > That seems viable. Where does that leave them for hot join though?
> >
> > No reason to keep hj because the difference between HJ and IBI is the
> > address when address arbitration. If hardware support IBI, it should support
> > HJ. HJ just use special address 0x2.
>
> For this IP, they appear to be controlled by different configuration time
> parameters, so I don't think conflating the two is the right thing to do
> here:
> https://docs.amd.com/r/en-US/pg439-axi-i3c/Configuration-Tab
> Per this doc, and the original commit, hotjoin configuration is not
> permitted when ibi is not supported but can be enabled and disabled
> separately when it is.
I am not sure why provide such flexiblity, only difference is that compare
address 2 or other value.
This feature may be quite common for i3c, if other soft IP provider such
flexiblity, still common property is the better than vendor specific one.
Frank
next prev parent reply other threads:[~2026-09-22 22:21 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-08 9:42 [PATCH v3 0/3] i3c: master: amd: Add IBI and hot-join support Shubham Patil
2026-09-08 9:42 ` [PATCH v3 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties Shubham Patil
2026-09-08 17:54 ` Conor Dooley
2026-09-09 16:21 ` Frank Li
2026-09-10 11:36 ` Conor Dooley
2026-09-10 15:44 ` Frank Li
2026-09-17 11:23 ` Michal Simek
2026-09-22 9:33 ` Conor Dooley
2026-09-22 9:38 ` Conor Dooley
2026-09-22 9:39 ` Conor Dooley
2026-09-22 16:25 ` Frank Li
2026-09-22 17:24 ` Conor Dooley
2026-09-22 18:35 ` Frank Li
2026-09-22 21:29 ` Conor Dooley
2026-09-22 22:21 ` Frank Li [this message]
2026-09-23 11:33 ` Patil, Shubham Sanjay
2026-09-23 11:43 ` Conor Dooley
2026-09-23 14:41 ` Frank Li
2026-09-08 9:42 ` [PATCH v3 2/3] i3c: master: amd: Add support for in-band interrupts Shubham Patil
2026-09-08 9:42 ` [PATCH v3 3/3] i3c: master: amd: Add hot-join support Shubham Patil
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=arL_Zgi1r1IFpNIO@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=alexandre.belloni@bootlin.com \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=git@amd.com \
--cc=krzk+dt@kernel.org \
--cc=linux-i3c@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.simek@amd.com \
--cc=robh@kernel.org \
--cc=shubhamsanjay.patil@amd.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®