| Message ID | 20190805165453.3989-17-alexandru.ardelean@analog.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | net: phy: adin: add support for Analog Devices PHYs | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
> + adi,rx-internal-delay: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > + default value is 0 (which represents 2 ns) > + enum: [ 0, 1, 2, 6, 7 ] We want these numbers to be in ns. So the default value would actually be 2. The driver needs to convert the number in DT to a value to poke into a PHY register. Please rename the property adi,rx-internal-delay-ns. > + > + adi,tx-internal-delay: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > + default value is 0 (which represents 2 ns) > + enum: [ 0, 1, 2, 6, 7 ] Same here. > + > + adi,fifo-depth: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + When operating in RMII mode, this option configures the FIFO depth. > + See `dt-bindings/net/adin.h`. > + enum: [ 0, 1, 2, 3, 4, 5 ] Units? You should probably rename this adi,fifo-depth-bits and list the valid values in bits. > + > + adi,eee-enabled: > + description: | > + Advertise EEE capabilities on power-up/init (default disabled) > + type: boolean It is not the PHY which decides this. The MAC indicates if it is EEE capable to phylib. phylib looks into the PHY registers to determine if the PHY supports EEE. phylib will then enable EEE advertisement. Please remove this, and ensure EEE is disabled by default. Andrew
On Mon, Aug 05, 2019 at 07:54:53PM +0300, Alexandru Ardelean wrote: > This change adds bindings for the Analog Devices ADIN PHY driver, detailing > all the properties implemented by the driver. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > .../devicetree/bindings/net/adi,adin.yaml | 93 +++++++++++++++++++ > MAINTAINERS | 2 + > include/dt-bindings/net/adin.h | 26 ++++++ > 3 files changed, 121 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml > create mode 100644 include/dt-bindings/net/adin.h > > diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml > new file mode 100644 > index 000000000000..fcf884bb86f7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml > @@ -0,0 +1,93 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/adi,adin.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADIN1200/ADIN1300 PHY > + > +maintainers: > + - Alexandru Ardelean <alexandru.ardelean@analog.com> > + > +description: | > + Bindings for Analog Devices Industrial Ethernet PHYs > + > +properties: > + compatible: > + description: | > + Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case > + Clause 45 will be used to access device management registers. If > + unspecified, Clause 22 will be used. Use this only when MDIO supports > + Clause 45 access, but there is no other way to determine this. > + enum: > + - ethernet-phy-ieee802.3-c45 It is valid to list ethernet-phy-ieee802.3-c22, it is just not required. So maybe you should list it here to keep the DT validater happy? Andrew
On Mon, 2019-08-05 at 16:27 +0200, Andrew Lunn wrote: > [External] > > On Mon, Aug 05, 2019 at 07:54:53PM +0300, Alexandru Ardelean wrote: > > This change adds bindings for the Analog Devices ADIN PHY driver, detailing > > all the properties implemented by the driver. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > .../devicetree/bindings/net/adi,adin.yaml | 93 +++++++++++++++++++ > > MAINTAINERS | 2 + > > include/dt-bindings/net/adin.h | 26 ++++++ > > 3 files changed, 121 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml > > create mode 100644 include/dt-bindings/net/adin.h > > > > diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml > > b/Documentation/devicetree/bindings/net/adi,adin.yaml > > new file mode 100644 > > index 000000000000..fcf884bb86f7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml > > @@ -0,0 +1,93 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/adi,adin.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Analog Devices ADIN1200/ADIN1300 PHY > > + > > +maintainers: > > + - Alexandru Ardelean <alexandru.ardelean@analog.com> > > + > > +description: | > > + Bindings for Analog Devices Industrial Ethernet PHYsphy > > + > > +properties: > > + compatible: > > + description: | > > + Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case > > + Clause 45 will be used to access device management registers. If > > + unspecified, Clause 22 will be used. Use this only when MDIO supports > > + Clause 45 access, but there is no other way to determine this. > > + enum: > > + - ethernet-phy-ieee802.3-c45 > > It is valid to list ethernet-phy-ieee802.3-c22, it is just not > required. So maybe you should list it here to keep the DT validater happy? ack > > Andrew
On Mon, 2019-08-05 at 16:11 +0200, Andrew Lunn wrote: > [External] > > > + adi,rx-internal-delay: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode > > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > > + default value is 0 (which represents 2 ns) > > + enum: [ 0, 1, 2, 6, 7 ] > > We want these numbers to be in ns. So the default value would actually > be 2. The driver needs to convert the number in DT to a value to poke > into a PHY register. Please rename the property adi,rx-internal-delay-ns. ack; also, good point about ns units and PHY driver to convert it to reg values; > > > + > > + adi,tx-internal-delay: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode > > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > > + default value is 0 (which represents 2 ns) > > + enum: [ 0, 1, 2, 6, 7 ] > > Same here. > > > + > > + adi,fifo-depth: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + When operating in RMII mode, this option configures the FIFO depth. > > + See `dt-bindings/net/adin.h`. > > + enum: [ 0, 1, 2, 3, 4, 5 ] > > Units? You should probably rename this adi,fifo-depth-bits and list > the valid values in bits. units are bits; will adapt this > > > + > > + adi,eee-enabled: > > + description: | > > + Advertise EEE capabilities on power-up/init (default disabled) > > + type: boolean > > It is not the PHY which decides this. The MAC indicates if it is EEE > capable to phylib. phylib looks into the PHY registers to determine if > the PHY supports EEE. phylib will then enable EEE > advertisement. Please remove this, and ensure EEE is disabled by > default. ack; will remove > > Andrew
On Mon, 2019-08-05 at 16:11 +0200, Andrew Lunn wrote: > [External] > > > + adi,rx-internal-delay: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode > > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > > + default value is 0 (which represents 2 ns) > > + enum: [ 0, 1, 2, 6, 7 ] > > We want these numbers to be in ns. So the default value would actually > be 2. The driver needs to convert the number in DT to a value to poke > into a PHY register. Please rename the property adi,rx-internal-delay-ns. > I just realized: this will probably have to be pico-seconds. Some delays are 1.60 ns, which are not easy to represent in in ns in DT. The values here are actually the register values corresponding to the delays. > > + > > + adi,tx-internal-delay: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode > > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > > + default value is 0 (which represents 2 ns) > > + enum: [ 0, 1, 2, 6, 7 ] > > Same here. > > > + > > + adi,fifo-depth: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + When operating in RMII mode, this option configures the FIFO depth. > > + See `dt-bindings/net/adin.h`. > > + enum: [ 0, 1, 2, 3, 4, 5 ] > > Units? You should probably rename this adi,fifo-depth-bits and list > the valid values in bits. > > > + > > + adi,eee-enabled: > > + description: | > > + Advertise EEE capabilities on power-up/init (default disabled) > > + type: boolean > > It is not the PHY which decides this. The MAC indicates if it is EEE > capable to phylib. phylib looks into the PHY registers to determine if > the PHY supports EEE. phylib will then enable EEE > advertisement. Please remove this, and ensure EEE is disabled by > default. > > Andrew
On Mon, Aug 5, 2019 at 7:55 AM Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > This change adds bindings for the Analog Devices ADIN PHY driver, detailing > all the properties implemented by the driver. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > .../devicetree/bindings/net/adi,adin.yaml | 93 +++++++++++++++++++ > MAINTAINERS | 2 + > include/dt-bindings/net/adin.h | 26 ++++++ > 3 files changed, 121 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml > create mode 100644 include/dt-bindings/net/adin.h > > diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml > new file mode 100644 > index 000000000000..fcf884bb86f7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml > @@ -0,0 +1,93 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/adi,adin.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADIN1200/ADIN1300 PHY > + > +maintainers: > + - Alexandru Ardelean <alexandru.ardelean@analog.com> > + > +description: | > + Bindings for Analog Devices Industrial Ethernet PHYs > + Needs an: allOf: - $ref: ethernet-phy.yaml# > +properties: > + compatible: > + description: | > + Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case > + Clause 45 will be used to access device management registers. If > + unspecified, Clause 22 will be used. Use this only when MDIO supports > + Clause 45 access, but there is no other way to determine this. > + enum: > + - ethernet-phy-ieee802.3-c45 Then you can drop 'compatible' from here as it is covered by the above schema. > + > + adi,phy-mode-internal: > + $ref: /schemas/types.yaml#/definitions/string This has to be under an 'allOf' or it doesn't actually work. Same below. > + description: | No special formatting here, you can drop '|'. > + The internal mode of the PHY. This assumes that there is a PHY converter > + in-between the MAC & PHY. > + enum: [ "rgmii", "rgmii-id", "rgmii-txid", "rgmii-rxid", "rmii", "mii" ] Don't need quotes here. > + > + adi,rx-internal-delay: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > + default value is 0 (which represents 2 ns) Use 'default: 0' to specify defaults. > + enum: [ 0, 1, 2, 6, 7 ] > + > + adi,tx-internal-delay: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode > + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` > + default value is 0 (which represents 2 ns) > + enum: [ 0, 1, 2, 6, 7 ] > + > + adi,fifo-depth: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + When operating in RMII mode, this option configures the FIFO depth. > + See `dt-bindings/net/adin.h`. > + enum: [ 0, 1, 2, 3, 4, 5 ] > + > + adi,eee-enabled: Isn't there a standard property for EEE control? > + description: | > + Advertise EEE capabilities on power-up/init (default disabled) > + type: boolean > + > + adi,disable-energy-detect: > + description: | > + Disables Energy Detect Powerdown Mode (default disabled, i.e energy detect > + is enabled if this property is unspecified) > + type: boolean > + > + reset-gpios: > + description: | > + GPIO to reset the PHY > + see Documentation/devicetree/bindings/gpio/gpio.txt. Active high or low? > + > +examples: > + - | > + ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c45"; > + reg = <0>; > + }; Not really anything specific to this binding. Drop it. > + - | > + #include <dt-bindings/net/adin.h> > + ethernet-phy@1 { > + reg = <1>; > + adi,phy-mode-internal = "rgmii-id"; > + > + adi,rx-internal-delay = <ADIN1300_RGMII_1_80_NS>; > + adi,tx-internal-delay = <ADIN1300_RGMII_2_20_NS>; > + }; > + - | > + #include <dt-bindings/net/adin.h> > + ethernet-phy@2 { > + reg = <2>; > + phy-mode = "rmii"; > + > + adi,fifo-depth = <ADIN1300_RMII_16_BITS>; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index faf5723610c8..6ffbb266dee4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -944,6 +944,8 @@ L: netdev@vger.kernel.org > W: http://ez.analog.com/community/linux-device-drivers > S: Supported > F: drivers/net/phy/adin.c > +F: include/dt-bindings/net/adin.h > +F: Documentation/devicetree/bindings/net/adi,adin.yaml > > ANALOG DEVICES INC ADIS DRIVER LIBRARY > M: Alexandru Ardelean <alexandru.ardelean@analog.com> > diff --git a/include/dt-bindings/net/adin.h b/include/dt-bindings/net/adin.h > new file mode 100644 > index 000000000000..4c3afa550c59 > --- /dev/null > +++ b/include/dt-bindings/net/adin.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/** > + * Device Tree constants for Analog Devices Industrial Ethernet PHYs > + * > + * Copyright 2019 Analog Devices Inc. > + */ > + > +#ifndef _DT_BINDINGS_ADIN_H > +#define _DT_BINDINGS_ADIN_H > + > +/* RGMII internal delay settings for rx and tx for ADIN1300 */ > +#define ADIN1300_RGMII_1_60_NS 0x1 > +#define ADIN1300_RGMII_1_80_NS 0x2 > +#define ADIN1300_RGMII_2_00_NS 0x0 > +#define ADIN1300_RGMII_2_20_NS 0x6 > +#define ADIN1300_RGMII_2_40_NS 0x7 > + > +/* RMII fifo depth values */ > +#define ADIN1300_RMII_4_BITS 0x0 > +#define ADIN1300_RMII_8_BITS 0x1 > +#define ADIN1300_RMII_12_BITS 0x2 > +#define ADIN1300_RMII_16_BITS 0x3 > +#define ADIN1300_RMII_20_BITS 0x4 > +#define ADIN1300_RMII_24_BITS 0x5 > + > +#endif > -- > 2.20.1 >
diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml new file mode 100644 index 000000000000..fcf884bb86f7 --- /dev/null +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml @@ -0,0 +1,93 @@ +# SPDX-License-Identifier: GPL-2.0+ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/adi,adin.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADIN1200/ADIN1300 PHY + +maintainers: + - Alexandru Ardelean <alexandru.ardelean@analog.com> + +description: | + Bindings for Analog Devices Industrial Ethernet PHYs + +properties: + compatible: + description: | + Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case + Clause 45 will be used to access device management registers. If + unspecified, Clause 22 will be used. Use this only when MDIO supports + Clause 45 access, but there is no other way to determine this. + enum: + - ethernet-phy-ieee802.3-c45 + + adi,phy-mode-internal: + $ref: /schemas/types.yaml#/definitions/string + description: | + The internal mode of the PHY. This assumes that there is a PHY converter + in-between the MAC & PHY. + enum: [ "rgmii", "rgmii-id", "rgmii-txid", "rgmii-rxid", "rmii", "mii" ] + + adi,rx-internal-delay: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` + default value is 0 (which represents 2 ns) + enum: [ 0, 1, 2, 6, 7 ] + + adi,tx-internal-delay: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h` + default value is 0 (which represents 2 ns) + enum: [ 0, 1, 2, 6, 7 ] + + adi,fifo-depth: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + When operating in RMII mode, this option configures the FIFO depth. + See `dt-bindings/net/adin.h`. + enum: [ 0, 1, 2, 3, 4, 5 ] + + adi,eee-enabled: + description: | + Advertise EEE capabilities on power-up/init (default disabled) + type: boolean + + adi,disable-energy-detect: + description: | + Disables Energy Detect Powerdown Mode (default disabled, i.e energy detect + is enabled if this property is unspecified) + type: boolean + + reset-gpios: + description: | + GPIO to reset the PHY + see Documentation/devicetree/bindings/gpio/gpio.txt. + +examples: + - | + ethernet-phy@0 { + compatible = "ethernet-phy-ieee802.3-c45"; + reg = <0>; + }; + - | + #include <dt-bindings/net/adin.h> + ethernet-phy@1 { + reg = <1>; + adi,phy-mode-internal = "rgmii-id"; + + adi,rx-internal-delay = <ADIN1300_RGMII_1_80_NS>; + adi,tx-internal-delay = <ADIN1300_RGMII_2_20_NS>; + }; + - | + #include <dt-bindings/net/adin.h> + ethernet-phy@2 { + reg = <2>; + phy-mode = "rmii"; + + adi,fifo-depth = <ADIN1300_RMII_16_BITS>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index faf5723610c8..6ffbb266dee4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -944,6 +944,8 @@ L: netdev@vger.kernel.org W: http://ez.analog.com/community/linux-device-drivers S: Supported F: drivers/net/phy/adin.c +F: include/dt-bindings/net/adin.h +F: Documentation/devicetree/bindings/net/adi,adin.yaml ANALOG DEVICES INC ADIS DRIVER LIBRARY M: Alexandru Ardelean <alexandru.ardelean@analog.com> diff --git a/include/dt-bindings/net/adin.h b/include/dt-bindings/net/adin.h new file mode 100644 index 000000000000..4c3afa550c59 --- /dev/null +++ b/include/dt-bindings/net/adin.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/** + * Device Tree constants for Analog Devices Industrial Ethernet PHYs + * + * Copyright 2019 Analog Devices Inc. + */ + +#ifndef _DT_BINDINGS_ADIN_H +#define _DT_BINDINGS_ADIN_H + +/* RGMII internal delay settings for rx and tx for ADIN1300 */ +#define ADIN1300_RGMII_1_60_NS 0x1 +#define ADIN1300_RGMII_1_80_NS 0x2 +#define ADIN1300_RGMII_2_00_NS 0x0 +#define ADIN1300_RGMII_2_20_NS 0x6 +#define ADIN1300_RGMII_2_40_NS 0x7 + +/* RMII fifo depth values */ +#define ADIN1300_RMII_4_BITS 0x0 +#define ADIN1300_RMII_8_BITS 0x1 +#define ADIN1300_RMII_12_BITS 0x2 +#define ADIN1300_RMII_16_BITS 0x3 +#define ADIN1300_RMII_20_BITS 0x4 +#define ADIN1300_RMII_24_BITS 0x5 + +#endif
This change adds bindings for the Analog Devices ADIN PHY driver, detailing all the properties implemented by the driver. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- .../devicetree/bindings/net/adi,adin.yaml | 93 +++++++++++++++++++ MAINTAINERS | 2 + include/dt-bindings/net/adin.h | 26 ++++++ 3 files changed, 121 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml create mode 100644 include/dt-bindings/net/adin.h