* [PATCH] Requesting review of U-Boot mxl-8611x device tree bindings @ 2023-07-13 13:20 Nate Drude 2023-07-13 13:23 ` [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY Nate Drude 0 siblings, 1 reply; 5+ messages in thread From: Nate Drude @ 2023-07-13 13:20 UTC (permalink / raw) To: devicetree@vger.kernel.org; +Cc: Eran Matityahu I am working on adding a driver for the mxl-8611x ethernet PHY to U-Boot. While there is a Linux driver from the vendor, it is not ready for mainline and it does not currently have any device tree bindings. The U-Boot maintainers requested that I have the device tree bindings reviewed by the Linux devicetree@ to avoid any misalignment between U-Boot and Linux when the driver lands in Linux. I will send the bindings U-Boot patch In-Reply-To this email. Thanks, Nate ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY 2023-07-13 13:20 [PATCH] Requesting review of U-Boot mxl-8611x device tree bindings Nate Drude @ 2023-07-13 13:23 ` Nate Drude 2023-07-13 19:06 ` Krzysztof Kozlowski 0 siblings, 1 reply; 5+ messages in thread From: Nate Drude @ 2023-07-13 13:23 UTC (permalink / raw) To: devicetree@vger.kernel.org; +Cc: Eran Matityahu 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@variscite.com> --- .../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 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, + 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 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. + +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) +#define MXL8611X_LEDX_CFG_LINK_UP_FULL_DUPLEX_ON (1 << 12) +#define MXL8611X_LEDX_CFG_LINK_UP_HALF_DUPLEX_ON (1 << 11) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ACT_ON (1 << 10) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ACT_ON (1 << 9) +#define MXL8611X_LEDX_CFG_LINK_UP_TX_ON (1 << 8) +#define MXL8611X_LEDX_CFG_LINK_UP_RX_ON (1 << 7) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_ON (1 << 6) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_ON (1 << 5) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_ON (1 << 4) +#define MXL8611X_LEDX_CFG_LINK_UP_COLLISION (1 << 3) +#define MXL8611X_LEDX_CFG_LINK_UP_1GB_BLINK (1 << 2) +#define MXL8611X_LEDX_CFG_LINK_UP_100MB_BLINK (1 << 1) +#define MXL8611X_LEDX_CFG_LINK_UP_10MB_BLINK (1 << 0) + +#endif -- 2.40.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY 2023-07-13 13:23 ` [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY Nate Drude @ 2023-07-13 19:06 ` Krzysztof Kozlowski 2023-07-14 12:53 ` Nate Drude 0 siblings, 1 reply; 5+ messages in thread From: Krzysztof Kozlowski @ 2023-07-13 19:06 UTC (permalink / raw) To: Nate Drude, devicetree@vger.kernel.org; +Cc: Eran Matityahu 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@variscite.com> > --- > .../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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY 2023-07-13 19:06 ` Krzysztof Kozlowski @ 2023-07-14 12:53 ` Nate Drude 2023-07-16 16:31 ` Krzysztof Kozlowski 0 siblings, 1 reply; 5+ messages in thread From: Nate Drude @ 2023-07-14 12:53 UTC (permalink / raw) To: Krzysztof Kozlowski, devicetree@vger.kernel.org, Rob Herring, Conor Dooley Cc: Eran Matityahu, Nate Drude 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@variscite.com> >> --- >> .../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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY 2023-07-14 12:53 ` Nate Drude @ 2023-07-16 16:31 ` Krzysztof Kozlowski 0 siblings, 0 replies; 5+ messages in thread From: Krzysztof Kozlowski @ 2023-07-16 16:31 UTC (permalink / raw) To: Nate Drude, devicetree@vger.kernel.org, Rob Herring, Conor Dooley Cc: Eran Matityahu On 14/07/2023 14:53, Nate Drude wrote: > > Once again, thanks for your review. Hopefully when the driver patches > are sent for Linux this will have been helpful. Driver patches are not really necessary here and I would say are independent. So as I said - submit bindings to the Linux kernel, so they will get proper review. Otherwise it does not matter what I say here. It does not guarantee absolutely any stable bindings because whatever someone submits to Linux kernel will later overrule it. Therefore I do not see any benefit of your approach - you will not be able to create Linux kernel ABI (bindings) by sending binding patch to U-Boot. It does not work like that. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-16 16:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-13 13:20 [PATCH] Requesting review of U-Boot mxl-8611x device tree bindings Nate Drude 2023-07-13 13:23 ` [PATCH] dt-bindings: add device tree bindings for mxl-8611x PHY Nate Drude 2023-07-13 19:06 ` Krzysztof Kozlowski 2023-07-14 12:53 ` Nate Drude 2023-07-16 16:31 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).