Hi Krzysztof,
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - realtek,rtd1295-usb2phy
> > + - realtek,rtd1312c-usb2phy
> > + - realtek,rtd1315e-usb2phy
> > + - realtek,rtd1319-usb2phy
> > + - realtek,rtd1319d-usb2phy
> > + - realtek,rtd1395-usb2phy
> > + - realtek,rtd1395-usb2phy-2port
> > + - realtek,rtd1619-usb2phy
> > + - realtek,rtd1619b-usb2phy
> > + - const: realtek,usb2phy
>
> That's not what your driver is saying... This patchset has random set of
> changes.
>
> I suggest to drop "realtek,usb2phy".
Okay, I will remove it.
> > +
> > + reg:
> > + items:
> > + - description: PHY data registers
> > + - description: PHY control registers
> > +
> > + "#phy-cells":
> > + const: 0
> > +
> > + nvmem-cells:
> > + maxItems: 2
> > + description:
> > + Phandles to nvmem cell that contains the trimming data.
> > + If unspecified, default value is used.
> > +
> > + nvmem-cell-names:
> > + items:
> > + - const: usb-dc-cal
> > + - const: usb-dc-dis
> > + description:
> > + The following names, which correspond to each nvmem-cells.
> > + usb-dc-cal is the driving level for each phy specified via efuse.
> > + usb-dc-dis is the disconnection level for each phy specified via efuse.
> > +
> > + realtek,inverse-hstx-sync-clock:
> > + description:
> > + For one of the phys of RTD1619b SoC, the synchronous clock of the
> > + high-speed tx must be inverted. So this property is used to set the
> > + inverted clock.
>
> Drop last sentence, it is redundant.
Okay
> > + type: boolean
> > +
> > + realtek,driving-level:
> > + description:
> > + Each board or port may have a different driving capability. This
> > + will adjust the driving level value if the value is not the default.
>
> I don't understand it. What is "driving capability" and if each port can have it
> different, why do you need property for this?
Sorry. I didn't express myself clearly.
"Driving capability" refers to the magnitude of the Dp/Dm output swing.
For a different board or port, the original magnitude maybe not meet the specification.
In this situation we can adjust the value to meet the specification.
> You mention some default - why it is not expressed as "default: xx"?
The default is mean hardware default value.
I can add the default value.
> What do the values mean?
The description can be modified to:
description:
Control the magnitude of High speed Dp/Dm output swing.
For a different board or port, the original magnitude maybe not meet
the specification. In this situation we can adjust the value to meet
the specification.
$ref: /schemas/types.yaml#/definitions/uint32
default: 8
minimum: 0
maximum: 31
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 31
> > +
> > + realtek,driving-compensate:
> > + description:
> > + For RTD1315e SoC, the driving level can be adjusted by reading the
> > + efuse table. Therefore, this property provides drive compensation
> for
> > + different boards with different drive capabilities.
>
> if driving level can be read from nvmem, why do you have realtek,driving-level
> in the first place? Don't you miss here some allOf making this constrained per
> variant?
>
> "Therefore" means "for that reason" or "as a consequence". How is this
> property a consequence of reading driving level from efuse? Is it then mutually
> exclusive with "realtek,driving-level"? But your schema does not express it.
No, it's not that complicated.
In our case, not all ICs have programming efuse.
The value "realtek,deiving-level" is for non-programmed efuse ICs.
In the programmed efuse IC, we use the value of efuse to instead of "realtek,driving-level".
If the magnitude of High speed Dp/Dm output swing still not meet the specification,
then we can set the value "driving-compensate" to meet the specification.
The description can be modified to:
realtek,driving-compensate:
description:
For RTD1315e SoC, the driving level can be adjusted by reading the
efuse table. This property provides drive compensation.
If the magnitude of High speed Dp/Dm output swing still not meet the
specification, then we can set this value to meet the specification.
$ref: /schemas/types.yaml#/definitions/int32
minimum: -8
maximum: 8
> > + $ref: /schemas/types.yaml#/definitions/int32
> > + minimum: -8
> > + maximum: 8
> > +
> > + realtek,disconnection-compensate:
> > + description:
> > + This adjusts the disconnection level compensation for the different
> > + boards with different disconnection level.
> > + $ref: /schemas/types.yaml#/definitions/int32
> > + minimum: -8
> > + maximum: 8
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - "#phy-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + usb_port0_usb2phy: usb-phy@13214 {
> > + compatible = "realtek,rtd1319d-usb2phy", "realtek,usb2phy";
> > + reg = <0x13214 0x4>, <0x28280 0x4>;
> > + #phy-cells = <0>;
> > +
> > + realtek,driving-level = <0xe>;
>
> Why this example is so empty? You have at least 6 more properties which
> should be shown here.
I will add more examples in here.
examples:
- |
usb_port0_usb2phy: usb-phy@13214 {
compatible = "realtek,rtd1319d-usb2phy";
reg = <0x13214 0x4>, <0x28280 0x4>;
#phy-cells = <0>;
nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";
realtek,driving-level = <0xe>;
};
- |
port0_usb2phy: usb-phy@13214 {
compatible = "realtek,rtd1619b-usb2phy";
reg = <0x13214 0x4>, <0x28280 0x4>;
#phy-cells = <0>;
nvmem-cells = <&otp_usb_port0_dc_cal>, <&otp_usb_port0_dc_dis>;
nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";
realtek,inverse-hstx-sync-clock;
realtek,driving-level = <0xa>;
realtek,driving-compensate = <(-1)>;
realtek,disconnection-compensate = <(-1)>;
};
port1_usb2phy: usb-phy@13c14 {
compatible = "realtek,rtd1619b-usb2phy";
reg = <0x13c14 0x4>, <0x31280 0x4>;
#phy-cells = <0>;
nvmem-cells = <&otp_usb_port1_dc_cal>, <&otp_usb_port1_dc_dis>;
nvmem-cell-names = "usb-dc-cal", "usb-dc-dis";
realtek,driving-level = <8>;
realtek,driving-compensate = <1>;
realtek,disconnection-compensate = <0>;
};
Thanks,
Stanley
