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@xxxxxxxxxxxxxx> > > Use full first and last name. Your email suggests something more than > only "Frank". > OK , I will use Frank.Sae <Frank.Sae@xxxxxxxxxxxxxx> >> --- >> .../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@xxxxxxxxxxxxxx> >> + >> +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
