[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8fa89dac-6859-af93-0dc0-ffcb42b5bb30@motor-comm.com>
Date: Fri, 6 Jan 2023 17:17:04 +0800
From: "Frank.Sae" <Frank.Sae@...or-comm.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Peter Geis <pgwipeout@...il.com>, Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc: xiaogang.fan@...or-comm.com, fei.zhang@...or-comm.com,
hua.sun@...or-comm.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v1 1/3] dt-bindings: net: Add Motorcomm yt8xxx
ethernet phy Driver bindings
Hi Krzysztof Kozlowski,
On 2023/1/6 16:26, Krzysztof Kozlowski wrote:
> On 05/01/2023 08:30, Frank wrote:
>> Add a YAML binding document for the Motorcom yt8xxx Ethernet phy driver.
>>
>
> Subject: drop second, redundant "Driver bindings".
Change Subject from
dt-bindings: net: Add Motorcomm yt8xxx ethernet phy Driver bindings
to
dt-bindings: net: Add Motorcomm yt8xxx ethernet phy
?
>
>> Signed-off-by: Frank <Frank.Sae@...or-comm.com>
>
> Use full first and last name. Your email suggests something more than
> only "Frank".
>
OK , I will use Frank.Sae <Frank.Sae@...or-comm.com>
>> ---
>> .../bindings/net/motorcomm,yt8xxx.yaml | 180 ++++++++++++++++++
>> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
>> MAINTAINERS | 1 +
>> 3 files changed, 183 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> new file mode 100644
>> index 000000000000..337a562d864c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
>> @@ -0,0 +1,180 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/motorcomm,yt8xxx.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MotorComm yt8xxx Ethernet PHY
>> +
>> +maintainers:
>> + - frank <frank.sae@...or-comm.com>
>> +
>> +description: |
>> + Bindings for MotorComm yt8xxx PHYs.
>
> Instead describe the hardware. No need to state the obvious that these
> are bindings.
>
I will fix.
>> + yt8511 will be supported later.
>
> Bindings should be complete. Your driver support is not relevant here.
I will fix.
>
>> +
>> +allOf:
>> + - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> + motorcomm,clk-out-frequency:
>
> Use property suffixes matching the type.
>
>> + description: clock output in Hertz on clock output pin.
>
> Drop "Hertz". It should be obvious from the suffix.
>
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> Drop.
>
> Anyway, does it fit standard clock-frequency property?
>
>> + enum: [0, 25000000, 125000000]
>> + default: 0
>> +
Yes, I will fix.
>> + motorcomm,rx-delay-basic:
>> + description: |
>> + Tristate, setup the basic RGMII RX Clock delay of PHY.
>> + This basic delay is fixed at 2ns (1000Mbps) or 8ns (100Mbps、10Mbps).
>> + This basic delay usually auto set by hardware according to the voltage
>> + of RXD0 pin (low = 0, turn off; high = 1, turn on).
>> + If not exist, this delay is controlled by hardware.
>
> I don't understand that at all. What "not exist"? There is no verb and
> no subject.
>
> The type and description are really unclear.
>
>> + 0: turn off; 1: turn on.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [0, 1]
>
> So this is bool?
>
This basic delay can be controlled by hardware or dts.
Default value depends on power on strapping, according to the voltage
of RXD0 pin (low = 0, turn off; high = 1, turn on).
"not exist" means that This basic delay is controlled by hardware,
and software don't change this.
I will fix.
>> +
>> + motorcomm,rx-delay-additional-ps:
>> + description: |
>> + Setup the additional RGMII RX Clock delay of PHY defined in pico seconds.
>> + RGMII RX Clock Delay = rx-delay-basic + rx-delay-additional-ps.
>> + enum:
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists