[PATCH net-next 0/3] Add support for configuring Marvell 10G PHY LEDs

[PATCH net-next 0/3] Add support for configuring Marvell 10G PHY LEDs

[Russell King - ARM Linux admin]

Hi,

This series makes it possible to configure the PHY LEDs on the Marvell
88x3310 and similar PHYs.

We introduce a new DT property called "marvell,led-mode" which allows
the register values (up to four) for the LEDs to be given and programmed
into the PHY by the driver.

 .../devicetree/bindings/net/marvell,10g.yaml       | 31 +++++++++++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts  |  2 +
 drivers/net/phy/marvell10g.c                       | 62 +++++++++++++++++++---
 3 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH net-next 0/3] Add support for configuring Marvell 10G PHY LEDs

[Russell King - ARM Linux admin]

Hi,

This series makes it possible to configure the PHY LEDs on the Marvell
88x3310 and similar PHYs.

We introduce a new DT property called "marvell,led-mode" which allows
the register values (up to four) for the LEDs to be given and programmed
into the PHY by the driver.

 .../devicetree/bindings/net/marvell,10g.yaml       | 31 +++++++++++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts  |  2 +
 drivers/net/phy/marvell10g.c                       | 62 +++++++++++++++++++---
 3 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King]

Add a DT bindings document for the Marvell 10G driver, which will
augment the generic ethernet PHY binding by having LED mode
configuration.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
new file mode 100644
index 000000000000..da597fc5314d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Alaska X family Ethernet PHYs
+
+maintainers:
+  - Russell King <rmk+kernel@armlinux.org.uk>
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+properties:
+  marvell,led-mode:
+    description: |
+      An array of one to four 16-bit integers to write to the PHY LED
+      configuration registers.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint16-array
+      - minItems: 1
+        maxItems: 4
+
+examples:
+  - |
+    ethernet-phy@0 {
+        reg = <0>;
+        compatible = "ethernet-phy-ieee802.3-c45";
+        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
+    };
-- 
2.20.1

[PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King]

Add a DT bindings document for the Marvell 10G driver, which will
augment the generic ethernet PHY binding by having LED mode
configuration.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
new file mode 100644
index 000000000000..da597fc5314d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Alaska X family Ethernet PHYs
+
+maintainers:
+  - Russell King <rmk+kernel@armlinux.org.uk>
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+properties:
+  marvell,led-mode:
+    description: |
+      An array of one to four 16-bit integers to write to the PHY LED
+      configuration registers.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint16-array
+      - minItems: 1
+        maxItems: 4
+
+examples:
+  - |
+    ethernet-phy@0 {
+        reg = <0>;
+        compatible = "ethernet-phy-ieee802.3-c45";
+        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
+    };
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 2/3] net: phy: marvell10g: add support for configuring LEDs

[Russell King]

Add support for configuring the LEDs. Macchiatobin has an oddity in
that the left LED goes out when the cable is connected, and flashes
when there's link activity. This is because the reset default for
the LED outputs assume that the LED is connected to supply, not to
ground. Add support for configuring the LED modes and polarities.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/marvell10g.c | 62 ++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 9a4e12a2af07..88720fa5c611 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -25,6 +25,7 @@
 #include <linux/ctype.h>
 #include <linux/hwmon.h>
 #include <linux/marvell_phy.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/sfp.h>
 
@@ -74,6 +75,8 @@ enum {
 struct mv3310_priv {
 	struct device *hwmon_dev;
 	char *hwmon_name;
+	u8 num_leds;
+	u16 led_mode[4];
 };
 
 #ifdef CONFIG_HWMON
@@ -238,6 +241,43 @@ static const struct sfp_upstream_ops mv3310_sfp_ops = {
 	.module_insert = mv3310_sfp_insert,
 };
 
+static int mv3310_leds_write(struct phy_device *phydev)
+{
+	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	int i, ret;
+
+	for (i = 0; i < priv->num_leds; i++) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, 0xf020 + i,
+				    priv->led_mode[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int mv3310_fw_config(struct phy_device *phydev)
+{
+	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	struct device_node *node;
+	int ret;
+
+	node = phydev->mdio.dev.of_node;
+	if (!node)
+		return 0;
+
+	ret = of_property_read_variable_u16_array(node, "marvell,led-mode",
+				priv->led_mode, 1, ARRAY_SIZE(priv->led_mode));
+	if (ret == -EINVAL)
+		ret = 0;
+	if (ret < 0)
+		return ret;
+
+	priv->num_leds = ret;
+
+	return 0;
+}
+
 static int mv3310_probe(struct phy_device *phydev)
 {
 	struct mv3310_priv *priv;
@@ -248,6 +288,20 @@ static int mv3310_probe(struct phy_device *phydev)
 	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
 		return -ENODEV;
 
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&phydev->mdio.dev, priv);
+
+	ret = mv3310_fw_config(phydev);
+	if (ret < 0)
+		return ret;
+
+	ret = mv3310_leds_write(phydev);
+	if (ret < 0)
+		return ret;
+
 	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_BOOT);
 	if (ret < 0)
 		return ret;
@@ -258,12 +312,6 @@ static int mv3310_probe(struct phy_device *phydev)
 		return -ENODEV;
 	}
 
-	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	dev_set_drvdata(&phydev->mdio.dev, priv);
-
 	ret = mv3310_hwmon_probe(phydev);
 	if (ret)
 		return ret;
@@ -316,7 +364,7 @@ static int mv3310_config_init(struct phy_device *phydev)
 	    phydev->interface != PHY_INTERFACE_MODE_10GBASER)
 		return -ENODEV;
 
-	return 0;
+	return mv3310_leds_write(phydev);
 }
 
 static int mv3310_get_features(struct phy_device *phydev)
-- 
2.20.1

[PATCH net-next 2/3] net: phy: marvell10g: add support for configuring LEDs

[Russell King]

Add support for configuring the LEDs. Macchiatobin has an oddity in
that the left LED goes out when the cable is connected, and flashes
when there's link activity. This is because the reset default for
the LED outputs assume that the LED is connected to supply, not to
ground. Add support for configuring the LED modes and polarities.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/marvell10g.c | 62 ++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 9a4e12a2af07..88720fa5c611 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -25,6 +25,7 @@
 #include <linux/ctype.h>
 #include <linux/hwmon.h>
 #include <linux/marvell_phy.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/sfp.h>
 
@@ -74,6 +75,8 @@ enum {
 struct mv3310_priv {
 	struct device *hwmon_dev;
 	char *hwmon_name;
+	u8 num_leds;
+	u16 led_mode[4];
 };
 
 #ifdef CONFIG_HWMON
@@ -238,6 +241,43 @@ static const struct sfp_upstream_ops mv3310_sfp_ops = {
 	.module_insert = mv3310_sfp_insert,
 };
 
+static int mv3310_leds_write(struct phy_device *phydev)
+{
+	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	int i, ret;
+
+	for (i = 0; i < priv->num_leds; i++) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, 0xf020 + i,
+				    priv->led_mode[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int mv3310_fw_config(struct phy_device *phydev)
+{
+	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	struct device_node *node;
+	int ret;
+
+	node = phydev->mdio.dev.of_node;
+	if (!node)
+		return 0;
+
+	ret = of_property_read_variable_u16_array(node, "marvell,led-mode",
+				priv->led_mode, 1, ARRAY_SIZE(priv->led_mode));
+	if (ret == -EINVAL)
+		ret = 0;
+	if (ret < 0)
+		return ret;
+
+	priv->num_leds = ret;
+
+	return 0;
+}
+
 static int mv3310_probe(struct phy_device *phydev)
 {
 	struct mv3310_priv *priv;
@@ -248,6 +288,20 @@ static int mv3310_probe(struct phy_device *phydev)
 	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
 		return -ENODEV;
 
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&phydev->mdio.dev, priv);
+
+	ret = mv3310_fw_config(phydev);
+	if (ret < 0)
+		return ret;
+
+	ret = mv3310_leds_write(phydev);
+	if (ret < 0)
+		return ret;
+
 	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MV_PMA_BOOT);
 	if (ret < 0)
 		return ret;
@@ -258,12 +312,6 @@ static int mv3310_probe(struct phy_device *phydev)
 		return -ENODEV;
 	}
 
-	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	dev_set_drvdata(&phydev->mdio.dev, priv);
-
 	ret = mv3310_hwmon_probe(phydev);
 	if (ret)
 		return ret;
@@ -316,7 +364,7 @@ static int mv3310_config_init(struct phy_device *phydev)
 	    phydev->interface != PHY_INTERFACE_MODE_10GBASER)
 		return -ENODEV;
 
-	return 0;
+	return mv3310_leds_write(phydev);
 }
 
 static int mv3310_get_features(struct phy_device *phydev)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next 3/3] arm64: dts: configure Macchiatobin 10G PHY LED modes

[Russell King]

Configure the Macchiatobin 10G PHY LED modes to correct their polarity.
We keep the existing LED behaviours, but switch their polarity to
reflect how they are connected. Tweak the LED modes as well to be:

left:  off          = no link
       solid green  = RJ45 link up (not SFP+ cage)
       flash green  = traffic
right: off          = no link
       solid green  = 10G
       solid yellow = 1G
       flash green  = 100M
       flash yellow = 10M

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index d06f5ab7ddab..87a3149a4261 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -21,12 +21,14 @@
 		compatible = "ethernet-phy-ieee802.3-c45";
 		reg = <0>;
 		sfp = <&sfp_eth0>;
+		marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
 	};
 
 	phy8: ethernet-phy@8 {
 		compatible = "ethernet-phy-ieee802.3-c45";
 		reg = <8>;
 		sfp = <&sfp_eth1>;
+		marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
 	};
 };
 
-- 
2.20.1

[PATCH net-next 3/3] arm64: dts: configure Macchiatobin 10G PHY LED modes

[Russell King]

Configure the Macchiatobin 10G PHY LED modes to correct their polarity.
We keep the existing LED behaviours, but switch their polarity to
reflect how they are connected. Tweak the LED modes as well to be:

left:  off          = no link
       solid green  = RJ45 link up (not SFP+ cage)
       flash green  = traffic
right: off          = no link
       solid green  = 10G
       solid yellow = 1G
       flash green  = 100M
       flash yellow = 10M

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index d06f5ab7ddab..87a3149a4261 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -21,12 +21,14 @@
 		compatible = "ethernet-phy-ieee802.3-c45";
 		reg = <0>;
 		sfp = <&sfp_eth0>;
+		marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
 	};
 
 	phy8: ethernet-phy@8 {
 		compatible = "ethernet-phy-ieee802.3-c45";
 		reg = <8>;
 		sfp = <&sfp_eth1>;
+		marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
 	};
 };
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, 27 Feb 2020 09:52:36 +0000, Russell King wrote:
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Documentation/devicetree/bindings/net/marvell,10g.example.dts:18.13-23: Warning (reg_format): /example-0/ethernet-phy@0:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

See https://patchwork.ozlabs.org/patch/1245687
Please check and re-submit.

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, 27 Feb 2020 09:52:36 +0000, Russell King wrote:
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Documentation/devicetree/bindings/net/marvell,10g.example.dts:18.13-23: Warning (reg_format): /example-0/ethernet-phy@0:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

See https://patchwork.ozlabs.org/patch/1245687
Please check and re-submit.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, Feb 27, 2020 at 3:52 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> new file mode 100644
> index 000000000000..da597fc5314d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0+

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Alaska X family Ethernet PHYs
> +
> +maintainers:
> +  - Russell King <rmk+kernel@armlinux.org.uk>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  marvell,led-mode:
> +    description: |
> +      An array of one to four 16-bit integers to write to the PHY LED
> +      configuration registers.

This is for what to blink or turn on for? I thought we had something
generic for configuring PHY LEDs specifically?

> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> +      - minItems: 1
> +        maxItems: 4
> +
> +examples:
> +  - |
> +    ethernet-phy@0 {
> +        reg = <0>;

This needs to be under an 'mdio' node with #address-cells and
#size-cells set correctly.

> +        compatible = "ethernet-phy-ieee802.3-c45";
> +        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
> +    };
> --
> 2.20.1
>

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, Feb 27, 2020 at 3:52 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
>
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> new file mode 100644
> index 000000000000..da597fc5314d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0+

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Alaska X family Ethernet PHYs
> +
> +maintainers:
> +  - Russell King <rmk+kernel@armlinux.org.uk>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  marvell,led-mode:
> +    description: |
> +      An array of one to four 16-bit integers to write to the PHY LED
> +      configuration registers.

This is for what to blink or turn on for? I thought we had something
generic for configuring PHY LEDs specifically?

> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> +      - minItems: 1
> +        maxItems: 4
> +
> +examples:
> +  - |
> +    ethernet-phy@0 {
> +        reg = <0>;

This needs to be under an 'mdio' node with #address-cells and
#size-cells set correctly.

> +        compatible = "ethernet-phy-ieee802.3-c45";
> +        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
> +    };
> --
> 2.20.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 11:08:58AM -0600, Rob Herring wrote:
> On Thu, 27 Feb 2020 09:52:36 +0000, Russell King wrote:
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
> Documentation/devicetree/bindings/net/marvell,10g.example.dts:18.13-23: Warning (reg_format): /example-0/ethernet-phy@0:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

It looks like your bot has made a mistake, or I don't understand the
error messages. It seems to be trying to treat the example as a PCI
device, but it isn't, it is a PHY device.

I don't think that's something I can fix, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 11:08:58AM -0600, Rob Herring wrote:
> On Thu, 27 Feb 2020 09:52:36 +0000, Russell King wrote:
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
> Documentation/devicetree/bindings/net/marvell,10g.example.dts:18.13-23: Warning (reg_format): /example-0/ethernet-phy@0:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> Documentation/devicetree/bindings/net/marvell,10g.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'

It looks like your bot has made a mistake, or I don't understand the
error messages. It seems to be trying to treat the example as a PCI
device, but it isn't, it is a PHY device.

I don't think that's something I can fix, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 11:13:41AM -0600, Rob Herring wrote:
> On Thu, Feb 27, 2020 at 3:52 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> > new file mode 100644
> > index 000000000000..da597fc5314d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> > @@ -0,0 +1,31 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> 
> Dual license new bindings please:
> 
> (GPL-2.0-only OR BSD-2-Clause)
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Alaska X family Ethernet PHYs
> > +
> > +maintainers:
> > +  - Russell King <rmk+kernel@armlinux.org.uk>
> > +
> > +allOf:
> > +  - $ref: ethernet-phy.yaml#
> > +
> > +properties:
> > +  marvell,led-mode:
> > +    description: |
> > +      An array of one to four 16-bit integers to write to the PHY LED
> > +      configuration registers.
> 
> This is for what to blink or turn on for? I thought we had something
> generic for configuring PHY LEDs specifically?

I see nothing in ethernet-phy.yaml.

Yes, it configures which conditions cause the LED to illuminate and/or
blink, what blink rate and polarity of the pin.

> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > +      - minItems: 1
> > +        maxItems: 4
> > +
> > +examples:
> > +  - |
> > +    ethernet-phy@0 {
> > +        reg = <0>;
> 
> This needs to be under an 'mdio' node with #address-cells and
> #size-cells set correctly.

I wish these things were documented somewhere... I'm pretty sure this
passed validation when I wrote it.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 11:13:41AM -0600, Rob Herring wrote:
> On Thu, Feb 27, 2020 at 3:52 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> > new file mode 100644
> > index 000000000000..da597fc5314d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> > @@ -0,0 +1,31 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> 
> Dual license new bindings please:
> 
> (GPL-2.0-only OR BSD-2-Clause)
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Alaska X family Ethernet PHYs
> > +
> > +maintainers:
> > +  - Russell King <rmk+kernel@armlinux.org.uk>
> > +
> > +allOf:
> > +  - $ref: ethernet-phy.yaml#
> > +
> > +properties:
> > +  marvell,led-mode:
> > +    description: |
> > +      An array of one to four 16-bit integers to write to the PHY LED
> > +      configuration registers.
> 
> This is for what to blink or turn on for? I thought we had something
> generic for configuring PHY LEDs specifically?

I see nothing in ethernet-phy.yaml.

Yes, it configures which conditions cause the LED to illuminate and/or
blink, what blink rate and polarity of the pin.

> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > +      - minItems: 1
> > +        maxItems: 4
> > +
> > +examples:
> > +  - |
> > +    ethernet-phy@0 {
> > +        reg = <0>;
> 
> This needs to be under an 'mdio' node with #address-cells and
> #size-cells set correctly.

I wish these things were documented somewhere... I'm pretty sure this
passed validation when I wrote it.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Andrew Lunn]

> > > +    allOf:
> > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > +      - minItems: 1
> > > +        maxItems: 4
> > > +
> > > +examples:
> > > +  - |
> > > +    ethernet-phy@0 {
> > > +        reg = <0>;
> > 
> > This needs to be under an 'mdio' node with #address-cells and
> > #size-cells set correctly.
> 
> I wish these things were documented somewhere... I'm pretty sure this
> passed validation when I wrote it.

Documentation/devicetree/bindings/net/mdio.yaml

Rob, is there a way to express the hierarchy between yaml files and
properties? Can we say that a phy, as defined by ethernet-phy.yaml
should always be inside an MDIO bus as defined in mdio.yaml?

Thanks
	Andrew

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Andrew Lunn]

> > > +    allOf:
> > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > +      - minItems: 1
> > > +        maxItems: 4
> > > +
> > > +examples:
> > > +  - |
> > > +    ethernet-phy@0 {
> > > +        reg = <0>;
> > 
> > This needs to be under an 'mdio' node with #address-cells and
> > #size-cells set correctly.
> 
> I wish these things were documented somewhere... I'm pretty sure this
> passed validation when I wrote it.

Documentation/devicetree/bindings/net/mdio.yaml

Rob, is there a way to express the hierarchy between yaml files and
properties? Can we say that a phy, as defined by ethernet-phy.yaml
should always be inside an MDIO bus as defined in mdio.yaml?

Thanks
	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 06:36:36PM +0100, Andrew Lunn wrote:
> > > > +    allOf:
> > > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > > +      - minItems: 1
> > > > +        maxItems: 4
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    ethernet-phy@0 {
> > > > +        reg = <0>;
> > > 
> > > This needs to be under an 'mdio' node with #address-cells and
> > > #size-cells set correctly.
> > 
> > I wish these things were documented somewhere... I'm pretty sure this
> > passed validation when I wrote it.
> 
> Documentation/devicetree/bindings/net/mdio.yaml

I'm not sure that makes it any more obvious.  Maybe it's obvious to
those who understand yaml, but for the rest of us, it isn't.

> Rob, is there a way to express the hierarchy between yaml files and
> properties? Can we say that a phy, as defined by ethernet-phy.yaml
> should always be inside an MDIO bus as defined in mdio.yaml?

and yes, it isn't even referenced from ethernet-phy.yaml, so how one
would know to even look there.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 06:36:36PM +0100, Andrew Lunn wrote:
> > > > +    allOf:
> > > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > > +      - minItems: 1
> > > > +        maxItems: 4
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    ethernet-phy@0 {
> > > > +        reg = <0>;
> > > 
> > > This needs to be under an 'mdio' node with #address-cells and
> > > #size-cells set correctly.
> > 
> > I wish these things were documented somewhere... I'm pretty sure this
> > passed validation when I wrote it.
> 
> Documentation/devicetree/bindings/net/mdio.yaml

I'm not sure that makes it any more obvious.  Maybe it's obvious to
those who understand yaml, but for the rest of us, it isn't.

> Rob, is there a way to express the hierarchy between yaml files and
> properties? Can we say that a phy, as defined by ethernet-phy.yaml
> should always be inside an MDIO bus as defined in mdio.yaml?

and yes, it isn't even referenced from ethernet-phy.yaml, so how one
would know to even look there.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Florian Fainelli]

On 2/27/20 1:52 AM, Russell King wrote:
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

We have been kicking the ball for way too long but there really ought to
be a standardized binding to configure LED modes for a PHY. Something
that we previously discussed here without making much progress because
the LED maintainer was not involved:

http://patchwork.ozlabs.org/patch/1146609/
http://patchwork.ozlabs.org/patch/1146610/
http://patchwork.ozlabs.org/patch/1146611/
http://patchwork.ozlabs.org/patch/1146612/

What you are proposing here is just a plain configuration interface via
Device Tree, which is really borderline. It gets the job done, and it is
extremely easy to maintain and use because people just stick in their
register value in there, but boy, what a poor abstraction that is.

Maybe you can resume where Matthias left and improve upon his patch
series, if nothing else for the binding and PHY layer integration?

> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> new file mode 100644
> index 000000000000..da597fc5314d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Alaska X family Ethernet PHYs
> +
> +maintainers:
> +  - Russell King <rmk+kernel@armlinux.org.uk>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  marvell,led-mode:
> +    description: |
> +      An array of one to four 16-bit integers to write to the PHY LED
> +      configuration registers.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> +      - minItems: 1
> +        maxItems: 4
> +
> +examples:
> +  - |
> +    ethernet-phy@0 {
> +        reg = <0>;
> +        compatible = "ethernet-phy-ieee802.3-c45";
> +        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
> +    };
> 


-- 
Florian

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Florian Fainelli]

On 2/27/20 1:52 AM, Russell King wrote:
> Add a DT bindings document for the Marvell 10G driver, which will
> augment the generic ethernet PHY binding by having LED mode
> configuration.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

We have been kicking the ball for way too long but there really ought to
be a standardized binding to configure LED modes for a PHY. Something
that we previously discussed here without making much progress because
the LED maintainer was not involved:

http://patchwork.ozlabs.org/patch/1146609/
http://patchwork.ozlabs.org/patch/1146610/
http://patchwork.ozlabs.org/patch/1146611/
http://patchwork.ozlabs.org/patch/1146612/

What you are proposing here is just a plain configuration interface via
Device Tree, which is really borderline. It gets the job done, and it is
extremely easy to maintain and use because people just stick in their
register value in there, but boy, what a poor abstraction that is.

Maybe you can resume where Matthias left and improve upon his patch
series, if nothing else for the binding and PHY layer integration?

> ---
>  .../devicetree/bindings/net/marvell,10g.yaml  | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,10g.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,10g.yaml b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> new file mode 100644
> index 000000000000..da597fc5314d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,10g.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,10g.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Alaska X family Ethernet PHYs
> +
> +maintainers:
> +  - Russell King <rmk+kernel@armlinux.org.uk>
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  marvell,led-mode:
> +    description: |
> +      An array of one to four 16-bit integers to write to the PHY LED
> +      configuration registers.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> +      - minItems: 1
> +        maxItems: 4
> +
> +examples:
> +  - |
> +    ethernet-phy@0 {
> +        reg = <0>;
> +        compatible = "ethernet-phy-ieee802.3-c45";
> +        marvell,led-mode = /bits/ 16 <0x0129 0x095d 0x0855>;
> +    };
> 


-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 09:44:35AM -0800, Florian Fainelli wrote:
> On 2/27/20 1:52 AM, Russell King wrote:
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> We have been kicking the ball for way too long but there really ought to
> be a standardized binding to configure LED modes for a PHY. Something
> that we previously discussed here without making much progress because
> the LED maintainer was not involved:
> 
> http://patchwork.ozlabs.org/patch/1146609/
> http://patchwork.ozlabs.org/patch/1146610/
> http://patchwork.ozlabs.org/patch/1146611/
> http://patchwork.ozlabs.org/patch/1146612/
> 
> What you are proposing here is just a plain configuration interface via
> Device Tree, which is really borderline. It gets the job done, and it is
> extremely easy to maintain and use because people just stick in their
> register value in there, but boy, what a poor abstraction that is.
> 
> Maybe you can resume where Matthias left and improve upon his patch
> series, if nothing else for the binding and PHY layer integration?

That series is way too simplistic, and would not allow for a
usable configuration for a four-speed PHY such as this one.

The proposed binding in those patches makes the assumption that
the only time that a LED shall blink is when there is traffic.

LED configuration is highly PHY specific.

For the 88x3310, we have around 31 different conditions that the LED
can blink for, or be solid for, the blink rate, and the polarity -
each LED is controlled by 13 bits in total, and then there's the "dual"
modes for bi-color LEDs which cause other of the LED configuration
registers to be ignored.  In other words, it's rather complex.

We could choose to limit the complexity, but then that risks making
it useless for certain boards - such as the Macchiatobin board, where
the dual modes can't be used due to the way the LEDs are wired - see
the last patch, where I describe how the LEDs are configured to
behave, which is the sanest organisation I could come up with which
doesn't result in mixing up various modes.


In any case, I do not wish to add to my patch backlog right now.  Maybe
when the backlog is smaller, I'll consider it, but not before.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Russell King - ARM Linux admin]

On Thu, Feb 27, 2020 at 09:44:35AM -0800, Florian Fainelli wrote:
> On 2/27/20 1:52 AM, Russell King wrote:
> > Add a DT bindings document for the Marvell 10G driver, which will
> > augment the generic ethernet PHY binding by having LED mode
> > configuration.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> We have been kicking the ball for way too long but there really ought to
> be a standardized binding to configure LED modes for a PHY. Something
> that we previously discussed here without making much progress because
> the LED maintainer was not involved:
> 
> http://patchwork.ozlabs.org/patch/1146609/
> http://patchwork.ozlabs.org/patch/1146610/
> http://patchwork.ozlabs.org/patch/1146611/
> http://patchwork.ozlabs.org/patch/1146612/
> 
> What you are proposing here is just a plain configuration interface via
> Device Tree, which is really borderline. It gets the job done, and it is
> extremely easy to maintain and use because people just stick in their
> register value in there, but boy, what a poor abstraction that is.
> 
> Maybe you can resume where Matthias left and improve upon his patch
> series, if nothing else for the binding and PHY layer integration?

That series is way too simplistic, and would not allow for a
usable configuration for a four-speed PHY such as this one.

The proposed binding in those patches makes the assumption that
the only time that a LED shall blink is when there is traffic.

LED configuration is highly PHY specific.

For the 88x3310, we have around 31 different conditions that the LED
can blink for, or be solid for, the blink rate, and the polarity -
each LED is controlled by 13 bits in total, and then there's the "dual"
modes for bi-color LEDs which cause other of the LED configuration
registers to be ignored.  In other words, it's rather complex.

We could choose to limit the complexity, but then that risks making
it useless for certain boards - such as the Macchiatobin board, where
the dual modes can't be used due to the way the LEDs are wired - see
the last patch, where I describe how the LEDs are configured to
behave, which is the sanest organisation I could come up with which
doesn't result in mixing up various modes.


In any case, I do not wish to add to my patch backlog right now.  Maybe
when the backlog is smaller, I'll consider it, but not before.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, Feb 27, 2020 at 11:36 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > +    allOf:
> > > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > > +      - minItems: 1
> > > > +        maxItems: 4
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    ethernet-phy@0 {
> > > > +        reg = <0>;
> > >
> > > This needs to be under an 'mdio' node with #address-cells and
> > > #size-cells set correctly.
> >
> > I wish these things were documented somewhere... I'm pretty sure this
> > passed validation when I wrote it.
>
> Documentation/devicetree/bindings/net/mdio.yaml
>
> Rob, is there a way to express the hierarchy between yaml files and
> properties? Can we say that a phy, as defined by ethernet-phy.yaml
> should always be inside an MDIO bus as defined in mdio.yaml?

We can link a child schema into a parent schema, but not the other way
around. So you can do something like this in mdio.yaml:

  "^ethernet-phy@[0-9a-f]+$":
    type: object
    allOf:
      - $ref: ethernet-phy.yaml#

That happens to work in this case since there's a common compatible
string for ethernet phys, but doesn't scale in the general case. Note
that ethernet-phy.yaml would need a couple of changes too. Also, this
should also be expanded to other possible node names like 'switch'.

I've had some thoughts of defining a pseudo property '$parent' or
something to be able to express constraints such as to what bus a
device has to be on. Currently, we rely on the overlap of the bus
schemas checking the bus specific aspects of the bus child nodes. I'm
also not really convinced that putting say an I2C device under a SPI
bus node is a problem we need to check for.


I'm not sure how any of this would help on examples compiling and
validating correctly. In example-schema.yaml, it mentions all the
problems I see: dtc fails, validation fails, bus node requirements,
and include file requirements:

  # Examples are now compiled with dtc and validated against the schemas
  #
  # Examples have a default #address-cells and #size-cells value of 1. This can
  # be overridden or an appropriate parent bus node should be shown (such as on
  # i2c buses).
  #
  # Any includes used have to be explicitly included.

Rob

Re: [PATCH net-next 1/3] dt-bindings: net: add dt bindings for marvell10g driver

[Rob Herring]

On Thu, Feb 27, 2020 at 11:36 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > +    allOf:
> > > > +      - $ref: /schemas/types.yaml#/definitions/uint16-array
> > > > +      - minItems: 1
> > > > +        maxItems: 4
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    ethernet-phy@0 {
> > > > +        reg = <0>;
> > >
> > > This needs to be under an 'mdio' node with #address-cells and
> > > #size-cells set correctly.
> >
> > I wish these things were documented somewhere... I'm pretty sure this
> > passed validation when I wrote it.
>
> Documentation/devicetree/bindings/net/mdio.yaml
>
> Rob, is there a way to express the hierarchy between yaml files and
> properties? Can we say that a phy, as defined by ethernet-phy.yaml
> should always be inside an MDIO bus as defined in mdio.yaml?

We can link a child schema into a parent schema, but not the other way
around. So you can do something like this in mdio.yaml:

  "^ethernet-phy@[0-9a-f]+$":
    type: object
    allOf:
      - $ref: ethernet-phy.yaml#

That happens to work in this case since there's a common compatible
string for ethernet phys, but doesn't scale in the general case. Note
that ethernet-phy.yaml would need a couple of changes too. Also, this
should also be expanded to other possible node names like 'switch'.

I've had some thoughts of defining a pseudo property '$parent' or
something to be able to express constraints such as to what bus a
device has to be on. Currently, we rely on the overlap of the bus
schemas checking the bus specific aspects of the bus child nodes. I'm
also not really convinced that putting say an I2C device under a SPI
bus node is a problem we need to check for.


I'm not sure how any of this would help on examples compiling and
validating correctly. In example-schema.yaml, it mentions all the
problems I see: dtc fails, validation fails, bus node requirements,
and include file requirements:

  # Examples are now compiled with dtc and validated against the schemas
  #
  # Examples have a default #address-cells and #size-cells value of 1. This can
  # be overridden or an appropriate parent bus node should be shown (such as on
  # i2c buses).
  #
  # Any includes used have to be explicitly included.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel