Re: [PATCH v3 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 10049 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]