mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Patil, Shubham Sanjay" <shubpati@amd.com>
To: Rob Herring <robh@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: Wed, 7 Oct 2026 11:33:03 +0530	[thread overview]
Message-ID: <8547c8c6-502f-4dca-a82f-893a5568f7d6@amd.com> (raw)
In-Reply-To: <arPlFMhcL7Jxh4wP@SMW015318>

Hi Rob, Just a reminder !

On 9/23/2026 8:11 PM, Frank Li wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Wed, Sep 23, 2026 at 12:43:22PM +0100, Conor Dooley wrote:
>> On Tue, Sep 22, 2026 at 05:21:26PM -0500, Frank Li wrote:
>>> 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.
>>
>> It's not unusual for FPGA IPs to provide extreme flexibility so that the
>> consumption of resources can be kept to a minimum, often to the point of
>> excessiveness. E.g. for a ethernet MAC IP each individual statistic might
>> have a configuration option.
>>
>>>
>>> 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.
>>
>> I'm not at all in favour of speculatively adding properties for
>> configuration properties of FPGA IPs, particularly given the likelihood
>> of abuse by others. Without multiple demonstrated users, things shouldn't
>> be considered for being made common anyway.
> 
> Rob:
>          What's your opinion? I predict we will met similar problem when
> I3C become popluar.
>          Do you perfer use vendor property fistly or direct add common
> property?
>          There will be back compatible problem if change vendor property to
> common property.
> 
> Frank


  reply	other threads:[~2026-10-07  6:03 UTC|newest]

Thread overview: 21+ 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
2026-09-23 11:33                             ` Patil, Shubham Sanjay
2026-09-23 11:43                             ` Conor Dooley
2026-09-23 14:41                               ` Frank Li
2026-10-07  6:03                                 ` Patil, Shubham Sanjay [this message]
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=8547c8c6-502f-4dca-a82f-893a5568f7d6@amd.com \
    --to=shubpati@amd.com \
    --cc=Frank.Li@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=conor+dt@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®