linux-kernel - Re: [PATCH v6 1/2] dt-bindings: i3c: Add adi-i3c-master

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250721-large-daffy-vole-d2d25d@kuoka>
Date: Mon, 21 Jul 2025 09:37:39 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Jorge Marques <jorge.marques@...log.com>, 
	Alexandre Belloni <alexandre.belloni@...tlin.com>, Frank Li <Frank.Li@....com>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	linux-i3c@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	gastmaier@...il.com
Subject: Re: [PATCH v6 1/2] dt-bindings: i3c: Add adi-i3c-master

On Sun, Jul 20, 2025 at 06:27:26PM -0500, Rob Herring wrote:
> > +description: |
> > +  FPGA-based I3C controller designed to interface with I3C and I2C peripherals,
> > +  implementing a subset of the I3C-basic specification. The IP core is tested
> > +  on arm, microblaze, and arm64 architectures.
> > +
> > +  https://analogdevicesinc.github.io/hdl/library/i3c_controller
> > +
> > +maintainers:
> > +  - Jorge Marques <jorge.marques@...log.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: adi,i3c-master-v1
> 
> If you want to use version numbers, they need to correlate to something 
> and you need to document what that is. I don't see anything in the above 
> link about a version 1. Kind of feels like you just made it up.

I asked already at v4 to document the naming/versioning, which was a
result of one of previous discussions, in the binding description. :/

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 1
> > +    items:
> > +      - description: The AXI interconnect clock, drives the register map.
> > +      - description: The I3C controller clock. AXI clock drives all logic if not provided.
> 
> Is that a description of how the h/w works? The controller clock input 
> can literally be left disconnected? If 1 clock source drives both 
> inputs, then the binding should reflect that.

This was explained in reply, but never made as proper explanation to the binding.

Jorge,
When you answer to a review about uncertain pieces like that, usually
outcome of the discussion must end up also in new patch - either in
commit msg or better in the binding itself. I also asked about this -
documenting the outcode - in v4.

Best regards,
Krzysztof