Re: Re: [PATCH v3 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC

[韦尚娟 <weishangjuan@eswincomputing.com>]

Hi, Krzysztof Kozlowski,

I apologize for the inconvenience caused by sending patches multiple times due to my incomplete
understanding of your previous version's response. I have two questions about YAML files that I would 
like to confirm with you. The questions are as follows.


> -----原始邮件-----
> 发件人: "Krzysztof Kozlowski" <krzk@kernel.org>
> 发送时间:2025-07-03 17:51:47 (星期四)
> 收件人: weishangjuan@eswincomputing.com, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, yong.liang.choong@linux.intel.com, vladimir.oltean@nxp.com, jszhang@kernel.org, jan.petrous@oss.nxp.com, prabhakar.mahadev-lad.rj@bp.renesas.com, inochiama@gmail.com, boon.khai.ng@altera.com, dfustini@tenstorrent.com, 0x1207@gmail.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org
> 抄送: ningyu@eswincomputing.com, linmin@eswincomputing.com, lizhi2@eswincomputing.com
> 主题: Re: [PATCH v3 1/2] dt-bindings: ethernet: eswin: Document for EIC7700 SoC
> 
> On 03/07/2025 11:19, weishangjuan@eswincomputing.com wrote:
> > From: Shangjuan Wei <weishangjuan@eswincomputing.com>
> > 
> > Add ESWIN EIC7700 Ethernet controller, supporting clock
> > configuration, delay adjustment and speed adaptive functions.
> > 
> > Signed-off-by: Zhi Li <lizhi2@eswincomputing.com>
> > Signed-off-by: Shangjuan Wei <weishangjuan@eswincomputing.com>
> > ---
> >  .../bindings/net/eswin,eic7700-eth.yaml       | 175 ++++++++++++++++++
> >  1 file changed, 175 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > new file mode 100644
> > index 000000000000..04b4c7bfbb5b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
> > @@ -0,0 +1,175 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/eswin,eic7700-eth.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Eswin EIC7700 SOC Eth Controller
> > +
> > +maintainers:
> > +  - Shuang Liang <liangshuang@eswincomputing.com>
> > +  - Zhi Li <lizhi2@eswincomputing.com>
> > +  - Shangjuan Wei <weishangjuan@eswincomputing.com>
> > +
> > +description:
> > +  The eth controller registers are part of the syscrg block on
> > +  the EIC7700 SoC.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - eswin,eic7700-qos-eth
> > +  required:
> > +    - compatible
> > +
> > +allOf:
> > +  - $ref: snps,dwmac.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: eswin,eic7700-qos-eth
> > +      - const: snps,dwmac-5.20
> > +
> > +  reg:
> > +    minItems: 1
> 
> Nope. Changelog does not explain that, it is not correct and no one ever
> requested something like that. See also writing bindings about constraints.

I have reviewed the writing method from other YAML files in the source code, 
and they all use “reg: maxItems: 1 ” instead of “reg: minItems: 1”. So we also
need to use “reg: maxItems: 1 ” in our YAML file. Is this understanding correct?

> > +
> > +  interrupt-names:
> > +    const: macirq
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  phy-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum:
> > +      - rgmii
> > +      - rgmii-rxid
> > +      - rgmii-txid
> > +      - rgmii-id
> > +
> > +  phy-handle:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: Reference to the PHY device
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    maxItems: 2
> > +    contains:
> > +      enum:
> > +        - stmmaceth
> > +        - tx
> 
> Not much changed, nothing explained in the changelog in cover letter.
> 

For clocks and clock-names, other YAML files have no minItems
and maxItems. Remove minItems and maxItems from
clocks and clock-names and as we have fix 2 clocks. Add description in clocks:items. 
Ref yaml: sophgo,sg2044-dwmac.yaml, starfive,jh7110-dwmac.yaml

All the changes will be added in cover letter in the next version. Is this understanding correct?

>
> You got already feedback that you keep pushing same code without fixing
> anything. You don't respond to feedback. You don't address it.
> 
> What is left for me? Start treating us seriously. I am not going to
> review the rest.
> 
> Respond to previous feedback with acknowledging that you understood it
> or further questions if you did not understand it, but you made thorough
> research on other bindings and example schema how to do it.
> 
> NAK
> 
> Best regards,
> Krzysztof