Hi Krzysztof,
On 7/13/23 2:06 PM, Krzysztof Kozlowski wrote:
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.
I appreciate your review, and sorry for the CC mistake. I added the
other maintainers.
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.
Thanks, I'll need to reconsider how to abstract this from register
values. Forget about this for now, I will split the LED support into a
separate effort.
+ 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.
Can you please provide feedback on these updated bindings?
- maxlinear,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.
- maxlinear,tx-internal-delay-100m-ps: RGMII TX Clock Delay used only
when PHY operates
in 10/100M RGMII mode with internal delay (phy-mode is 'rgmii-id' or
'rgmii-txid') in pico-seconds.
- maxlinear,tx-internal-delay-1g-ps: 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.
+
+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
Once again, thanks for your review. Hopefully when the driver patches
are sent for Linux this will have been helpful.
Sincerely,
Nate
