Re: Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets

[Conor Dooley <conor@kernel.org>]

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

On Fri, May 22, 2026 at 10:52:27AM +0800, 李志 wrote:
> 
> 
> 
> > -----Original Messages-----
> > From: "Conor Dooley" <conor@kernel.org>
> > Send time:Tuesday, 19/05/2026 17:55:27
> > To: 李志 <lizhi2@eswincomputing.com>
> > Cc: sashiko-reviews@lists.linux.dev, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> > Subject: Re: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > 
> > On Tue, May 19, 2026 at 05:51:05PM +0800, 李志 wrote:
> > > 
> > > 
> > > 
> > > > -----Original Messages-----
> > > > From: "Conor Dooley" <conor@kernel.org>
> > > > Send time:Tuesday, 19/05/2026 17:11:59
> > > > To: sashiko-reviews@lists.linux.dev
> > > > Cc: lizhi2@eswincomputing.com, robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
> > > > Subject: Re: [PATCH net v2 1/5] dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > > > 
> > > > On Tue, May 19, 2026 at 02:23:33AM +0000, sashiko-bot@kernel.org wrote:
> > > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > > - [Medium] Missing `minItems: 4` constraint makes the newly added optional cells mandatory, breaking schema backward compatibility.
> > > > > --
> > > > > 
> > > > > commit 2992483df88af2823c275fc86187f7e38acbee5c
> > > > > Author: Zhi Li <lizhi2@eswincomputing.com>
> > > > > 
> > > > > dt-bindings: ethernet: eswin: add optional TXD and RXD delay register offsets
> > > > > 
> > > > > This commit documents two optional cells in the eswin,hsp-sp-csr property
> > > > > for the TXD and RXD delay control register offsets.
> > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > > > --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > > > > 
> > > > > [ ... ]
> > > > > 
> > > > > > @@ -82,6 +91,8 @@ properties:
> > > > > >            - description: Offset of AXI clock controller Low-Power request
> > > > > >                           register
> > > > > >            - description: Offset of register controlling TX/RX clock delay
> > > > > > +          - description: Optional offset of register controlling TXD delay
> > > > > > +          - description: Optional offset of register controlling RXD delay
> > > > > 
> > > > > Will this make the two new offsets strictly required instead of optional?
> > > > > 
> > > > > In dt-schema, when an items list defines a tuple, minItems implicitly
> > > > > defaults to the total number of items in that list. By increasing the list
> > > > > from 4 to 6 items without adding a minItems: 4 constraint to the inner items
> > > > > list, won't validation now strictly require exactly 6 items?
> > > > > 
> > > > > Could this cause dt_binding_check to fail for existing device trees that
> > > > > correctly specify only the original 4 elements?
> > > > 
> > > > Looks like a valid complaint to me.
> > > > pw-bot: changes-requested
> > > > 
> > > You're right, this is a valid issue.
> > > 
> > > I'll fix it in v3 by adding only minItems: 4 to preserve backward
> > > compatibility with existing 4-cell device trees.
> > > 
> > > Should I drop your Acked-by in v3 due to the schema change, or should
> > > it be retained?
> > 
> > Just fix it, and you can retain.
> 
> Hi Conor,
> 
> I noticed that the v2 series has already been applied to net.git:
> https://lore.kernel.org/lkml/177935821264.4013498.4647676520552839707.git-patchwork-notify@kernel.org/
> 
> Since the missing minItems: 4 breaks backward compatibility for
> existing 4-cell device trees, I plan to send a new single-patch series
> targeting net to add the missing constraint.
> 
> Just wanted to confirm that this is the preferred approach.

That sounds like the right approach to me.

Thanks,
Conor.

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