On 13/07/2023 15:23, Nate Drude wrote:
> The MXL8611X driver has custom bindings for configuring the LEDs
> and RGMII internal delays. This patch adds the documentation and
> defines necessary to configure it from the device tree.
>
> Signed-off-by: Nate Drude <nate.d@xxxxxxxxxxxxx>
> ---
> .../net/phy/mxl-8611x.txt | 37 +++++++++++++++++++
> include/dt-bindings/net/mxl-8611x.h | 27 ++++++++++++++
> 2 files changed, 64 insertions(+)
> create mode 100644 doc/device-tree-bindings/net/phy/mxl-8611x.txt
You did not CC DT maintainers, so kind of funny way to ask them to
review something.
In general I will not provide full review for projects other than Linux.
Submit bindings to the Linux, following proper Linux kernel process, if
you wish them to be fully reviewed.
> create mode 100644 include/dt-bindings/net/mxl-8611x.h
>
> diff --git a/doc/device-tree-bindings/net/phy/mxl-8611x.txt
> b/doc/device-tree-bindings/net/phy/mxl-8611x.txt
> new file mode 100644
> index 00000000000..462fdf61666
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/phy/mxl-8611x.txt
> @@ -0,0 +1,37 @@
> +* MaxLinear MXL8611x PHY Device Tree binding
> +
> +Required properties:
> +- reg: PHY address
> +
> +Optional properties:
> +- mxl-8611x,ledN_cfg: Register configuration for COM_EXT_LED0_CFG,
That's not correct vendor name. Neither property name - underscores are
not allowed. The property itself does not describe any feature or
hardware. We do not program registers in DT.
> + COM_EXT_LED1_CFG, and COM_EXT_LED2_CFG
> +- mxl-8611x,rx-internal-delay-ps: RGMII RX Clock Delay used only when
> PHY operates
> + in RGMII mode with internal delay (phy-mode is 'rgmii-id' or
> + 'rgmii-rxid') in pico-seconds.
> +- mxl-8611x,tx-internal-delay-ps-100m: RGMII TX Clock Delay used only
Use correct unit suffixes.
> when PHY operates
> + in 10/100M RGMII mode with internal delay (phy-mode is 'rgmii-id' or
> + 'rgmii-txid') in pico-seconds.
> +- mxl-8611x,tx-internal-delay-ps-1g: RGMII TX Clock Delay used only
> when PHY operates
> + in 1G RGMII mode with internal delay (phy-mode is 'rgmii-id' or
> + 'rgmii-txid') in pico-seconds.
Same problem.
> +
> +Example:
> +
> + ethernet-phy@0 {
> + reg = <0>;
> +
> + mxl-8611x,led0_cfg = <(
> + MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON |
> + MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON |
> + MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND
> + )>;
> + mxl-8611x,led1_cfg = <(
> + MXL8611X_LEDX_CFG_LINK_UP_10MB_ON |
> + MXL8611X_LEDX_CFG_LINK_UP_100MB_ON |
> + MXL8611X_LEDX_CFG_LINK_UP_1GB_ON
> + )>;
> + mxl-8611x,rx-internal-delay-ps = <0>;
> + mxl-8611x,tx-internal-delay-ps-100m = <2250>;
> + mxl-8611x,tx-internal-delay-ps-1g = <150>;
> + };
> diff --git a/include/dt-bindings/net/mxl-8611x.h
> b/include/dt-bindings/net/mxl-8611x.h
> new file mode 100644
> index 00000000000..cb0ec0f8bd0
> --- /dev/null
> +++ b/include/dt-bindings/net/mxl-8611x.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Device Tree constants for MaxLinear MXL8611x PHYs
> + *
> + * Copyright 2023 Variscite Ltd.
> + * Copyright 2023 MaxLinear Inc.
> + */
> +
> +#ifndef _DT_BINDINGS_MXL_8611X_H
> +#define _DT_BINDINGS_MXL_8611X_H
> +
> +#define MXL8611X_LEDX_CFG_TRAFFIC_ACT_BLINK_IND (1 << 13)
Register values are not bindings.
Best regards,
Krzysztof
