[PATCH 0/2] net: phy: dp83867: Configure LED modes via device tree

[PATCH 0/2] net: phy: dp83867: Configure LED modes via device tree

[Michael Tretter]

Hello,

The dp83867 has 4 LED pins, which can be multiplexed with different functions
of the phy.

This series adds a device tree binding to describe the multiplexing of the
functions to the LEDs and implements the binding for the dp83867 phy.

I found existing bindings for configuring the LED modes for other phys:

In Documentation/devicetree/bindings/net/micrel.txt, the binding is not
flexible enough for the use case in the dp83867, because there is a value for
each LED configuration, which would be a lot of values for the dp83867.

In Documentation/devicetree/bindings/net/mscc-phy-vsc8532.txt, there is a
separate property for each LED, which would work, but I found rather
unintuitive compared to how clock bindings etc. work.

The new binding defines two properties: one for the led names and another
property for the modes of the LEDs with defined values in the same order.
Currently, the binding is specific to the dp83867, but I guess that the
binding could be made more generic and used for other phys, too.

Let me know, what you think.

Michael

Michael Tretter (1):
  dt-bindings: dp83867: Add binding for LED mode configuration

Thomas Haemmerle (1):
  net: phy: dp83867: add support for changing LED modes

 .../devicetree/bindings/net/ti,dp83867.yaml   | 24 ++++++++
 drivers/net/phy/dp83867.c                     | 57 +++++++++++++++++++
 include/dt-bindings/net/ti-dp83867.h          | 16 ++++++
 3 files changed, 97 insertions(+)

-- 
2.29.2

[PATCH 1/2] dt-bindings: dp83867: Add binding for LED mode configuration

[Michael Tretter]

The DP83867 supports four configurable LED pins. Describe the
multiplexing of functions to the LEDs via device tree.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 .../devicetree/bindings/net/ti,dp83867.yaml   | 24 +++++++++++++++++++
 include/dt-bindings/net/ti-dp83867.h          | 16 +++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
index 047d757e8d82..d09e1cf42462 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
@@ -106,6 +106,30 @@ properties:
       Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h for applicable
       values.
 
+  ti,dp83867-led-mode-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description: |
+      A list of led name strings sorted in the same order as the
+      ti,dp83867-led-modes property.
+    items:
+      anyOf:
+        items:
+          - const: led-0
+          - const: led-1
+          - const: led-2
+          - const: led-gpio
+    maxItems: 4
+
+  ti,dp83867-led-modes:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      The DP83867 supports four configurable LED pins. Several functions can
+      be multiplexed onto the LEDs for different modes of operation.
+
+      Must contain an entry for each entry in ti,dp83867-led-mode-names.
+      See dt-bindings/net/ti-dp83867.h for applicable values.
+    maxItems: 4
+
 required:
   - reg
 
diff --git a/include/dt-bindings/net/ti-dp83867.h b/include/dt-bindings/net/ti-dp83867.h
index 6fc4b445d3a1..fc3891f37fd1 100644
--- a/include/dt-bindings/net/ti-dp83867.h
+++ b/include/dt-bindings/net/ti-dp83867.h
@@ -50,4 +50,20 @@
 #define DP83867_CLK_O_SEL_REF_CLK		0xC
 /* Special flag to indicate clock should be off */
 #define DP83867_CLK_O_SEL_OFF			0xFFFFFFFF
+
+/* LED configuration */
+#define DP83867_LED_LINK_EST			0x0
+#define DP83867_LED_RX_TX_ACT			0x1
+#define DP83867_LED_TX_ACT			0x2
+#define DP83867_LED_RX_ACT			0x3
+#define DP83867_LED_COLLISION_DET		0x4
+#define DP83867_LED_1000_BT_LINK		0x5
+#define DP83867_LED_100_BT_LINK			0x6
+#define DP83867_LED_10_BT_LINK			0x7
+#define DP83867_LED_10_100_BT_LINK		0x8
+#define DP83867_LED_100_1000_BT_LINK		0x9
+#define DP83867_LED_FULL_DUPLEX			0xa
+#define DP83867_LED_LINK_EST_RX_TX_ACT		0xb
+#define DP83867_LED_RX_TX_ERR			0xd
+#define DP83867_LED_RX_ERR			0xe
 #endif
-- 
2.29.2

[PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[Michael Tretter]

From: Thomas Haemmerle <thomas.haemmerle@wolfvision.net>

The DP83867 supports four configurable LEDs. Several functions can be
multiplexed to these LEDs. The multiplexing can be configured in the
LEDCR1 register.

Add support for changing the multiplexing of the LEDs via device tree.

Signed-off-by: Thomas Haemmerle <thomas.haemmerle@wolfvision.net>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/net/phy/dp83867.c | 57 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 9bd9a5c0b1db..dfcac95941f3 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -25,6 +25,7 @@
 #define MII_DP83867_MICR	0x12
 #define MII_DP83867_ISR		0x13
 #define DP83867_CFG2		0x14
+#define DP83867_LEDCR1		0x18
 #define DP83867_CFG3		0x1e
 #define DP83867_CTRL		0x1f
 
@@ -138,6 +139,12 @@
 #define DP83867_DOWNSHIFT_4_COUNT	4
 #define DP83867_DOWNSHIFT_8_COUNT	8
 
+/* LEDCR1 bits */
+#define DP83867_LEDCR1_LED_0_SEL	GENMASK(3, 0)
+#define DP83867_LEDCR1_LED_1_SEL	GENMASK(7, 4)
+#define DP83867_LEDCR1_LED_2_SEL	GENMASK(11, 8)
+#define DP83867_LEDCR1_LED_GPIO_SEL	GENMASK(15, 12)
+
 /* CFG3 bits */
 #define DP83867_CFG3_INT_OE			BIT(7)
 #define DP83867_CFG3_ROBUST_AUTO_MDIX		BIT(9)
@@ -154,6 +161,14 @@ enum {
 	DP83867_PORT_MIRROING_DIS,
 };
 
+enum {
+	DP83867_LED_0,
+	DP83867_LED_1,
+	DP83867_LED_2,
+	DP83867_LED_GPIO,
+	DP83867_LED_MAX,
+};
+
 struct dp83867_private {
 	u32 rx_id_delay;
 	u32 tx_id_delay;
@@ -165,6 +180,7 @@ struct dp83867_private {
 	bool set_clk_output;
 	u32 clk_output_sel;
 	bool sgmii_ref_clk_en;
+	u32 led_mode[DP83867_LED_MAX];
 };
 
 static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -521,6 +537,27 @@ static int dp83867_verify_rgmii_cfg(struct phy_device *phydev)
 }
 
 #if IS_ENABLED(CONFIG_OF_MDIO)
+static int dp83867_of_led_mode_read(struct device_node *of_node,
+				    const char *led_name, u32 *mode)
+{
+	u32 tmp;
+	int index;
+	int err;
+
+	index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
+					 led_name);
+	err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
+					 index, tmp);
+	if (err)
+		return err;
+	if (tmp == 0xc || tmp >= 0xf)
+		return -EINVAL;
+
+	*mode = tmp;
+
+	return 0;
+}
+
 static int dp83867_of_init(struct phy_device *phydev)
 {
 	struct dp83867_private *dp83867 = phydev->priv;
@@ -614,6 +651,15 @@ static int dp83867_of_init(struct phy_device *phydev)
 		return -EINVAL;
 	}
 
+	dp83867_of_led_mode_read(of_node, "led-0",
+				 &dp83867->led_mode[DP83867_LED_0]);
+	dp83867_of_led_mode_read(of_node, "led-1",
+				 &dp83867->led_mode[DP83867_LED_1]);
+	dp83867_of_led_mode_read(of_node, "led-2",
+				 &dp83867->led_mode[DP83867_LED_2]);
+	dp83867_of_led_mode_read(of_node, "led-gpio",
+				 &dp83867->led_mode[DP83867_LED_GPIO]);
+
 	return 0;
 }
 #else
@@ -632,6 +678,11 @@ static int dp83867_probe(struct phy_device *phydev)
 	if (!dp83867)
 		return -ENOMEM;
 
+	dp83867->led_mode[DP83867_LED_0] = DP83867_LED_LINK_EST;
+	dp83867->led_mode[DP83867_LED_1] = DP83867_LED_1000_BT_LINK;
+	dp83867->led_mode[DP83867_LED_2] = DP83867_LED_RX_TX_ACT;
+	dp83867->led_mode[DP83867_LED_GPIO] = DP83867_LED_100_BT_LINK;
+
 	phydev->priv = dp83867;
 
 	return dp83867_of_init(phydev);
@@ -792,6 +843,12 @@ static int dp83867_config_init(struct phy_device *phydev)
 		phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_SGMIICTL, val);
 	}
 
+	val = FIELD_PREP(DP83867_LEDCR1_LED_0_SEL, dp83867->led_mode[DP83867_LED_0]) |
+	      FIELD_PREP(DP83867_LEDCR1_LED_1_SEL, dp83867->led_mode[DP83867_LED_1]) |
+	      FIELD_PREP(DP83867_LEDCR1_LED_2_SEL, dp83867->led_mode[DP83867_LED_2]) |
+	      FIELD_PREP(DP83867_LEDCR1_LED_GPIO_SEL, dp83867->led_mode[DP83867_LED_GPIO]);
+	phy_write(phydev, DP83867_LEDCR1, val);
+
 	val = phy_read(phydev, DP83867_CFG3);
 	/* Enable Interrupt output INT_OE in CFG3 register */
 	if (phy_interrupt_is_valid(phydev))
-- 
2.29.2

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[Michael Tretter]

On Fri, 19 Mar 2021 16:57:10 +0100, Michael Tretter wrote:
> From: Thomas Haemmerle <thomas.haemmerle@wolfvision.net>
> 
> The DP83867 supports four configurable LEDs. Several functions can be
> multiplexed to these LEDs. The multiplexing can be configured in the
> LEDCR1 register.
> 
> Add support for changing the multiplexing of the LEDs via device tree.
> 
> Signed-off-by: Thomas Haemmerle <thomas.haemmerle@wolfvision.net>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  drivers/net/phy/dp83867.c | 57 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 9bd9a5c0b1db..dfcac95941f3 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -25,6 +25,7 @@
>  #define MII_DP83867_MICR	0x12
>  #define MII_DP83867_ISR		0x13
>  #define DP83867_CFG2		0x14
> +#define DP83867_LEDCR1		0x18
>  #define DP83867_CFG3		0x1e
>  #define DP83867_CTRL		0x1f
>  
> @@ -138,6 +139,12 @@
>  #define DP83867_DOWNSHIFT_4_COUNT	4
>  #define DP83867_DOWNSHIFT_8_COUNT	8
>  
> +/* LEDCR1 bits */
> +#define DP83867_LEDCR1_LED_0_SEL	GENMASK(3, 0)
> +#define DP83867_LEDCR1_LED_1_SEL	GENMASK(7, 4)
> +#define DP83867_LEDCR1_LED_2_SEL	GENMASK(11, 8)
> +#define DP83867_LEDCR1_LED_GPIO_SEL	GENMASK(15, 12)
> +
>  /* CFG3 bits */
>  #define DP83867_CFG3_INT_OE			BIT(7)
>  #define DP83867_CFG3_ROBUST_AUTO_MDIX		BIT(9)
> @@ -154,6 +161,14 @@ enum {
>  	DP83867_PORT_MIRROING_DIS,
>  };
>  
> +enum {
> +	DP83867_LED_0,
> +	DP83867_LED_1,
> +	DP83867_LED_2,
> +	DP83867_LED_GPIO,
> +	DP83867_LED_MAX,
> +};
> +
>  struct dp83867_private {
>  	u32 rx_id_delay;
>  	u32 tx_id_delay;
> @@ -165,6 +180,7 @@ struct dp83867_private {
>  	bool set_clk_output;
>  	u32 clk_output_sel;
>  	bool sgmii_ref_clk_en;
> +	u32 led_mode[DP83867_LED_MAX];
>  };
>  
>  static int dp83867_ack_interrupt(struct phy_device *phydev)
> @@ -521,6 +537,27 @@ static int dp83867_verify_rgmii_cfg(struct phy_device *phydev)
>  }
>  
>  #if IS_ENABLED(CONFIG_OF_MDIO)
> +static int dp83867_of_led_mode_read(struct device_node *of_node,
> +				    const char *led_name, u32 *mode)
> +{
> +	u32 tmp;
> +	int index;
> +	int err;
> +
> +	index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
> +					 led_name);
> +	err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
> +					 index, tmp);

This should have been &tmp.

I will wait for some more review feedback by others especially for the dt
bindings and send a v2.

> +	if (err)
> +		return err;
> +	if (tmp == 0xc || tmp >= 0xf)
> +		return -EINVAL;
> +
> +	*mode = tmp;
> +
> +	return 0;
> +}
> +
>  static int dp83867_of_init(struct phy_device *phydev)
>  {
>  	struct dp83867_private *dp83867 = phydev->priv;
> @@ -614,6 +651,15 @@ static int dp83867_of_init(struct phy_device *phydev)
>  		return -EINVAL;
>  	}
>  
> +	dp83867_of_led_mode_read(of_node, "led-0",
> +				 &dp83867->led_mode[DP83867_LED_0]);
> +	dp83867_of_led_mode_read(of_node, "led-1",
> +				 &dp83867->led_mode[DP83867_LED_1]);
> +	dp83867_of_led_mode_read(of_node, "led-2",
> +				 &dp83867->led_mode[DP83867_LED_2]);
> +	dp83867_of_led_mode_read(of_node, "led-gpio",
> +				 &dp83867->led_mode[DP83867_LED_GPIO]);
> +
>  	return 0;
>  }
>  #else
> @@ -632,6 +678,11 @@ static int dp83867_probe(struct phy_device *phydev)
>  	if (!dp83867)
>  		return -ENOMEM;
>  
> +	dp83867->led_mode[DP83867_LED_0] = DP83867_LED_LINK_EST;
> +	dp83867->led_mode[DP83867_LED_1] = DP83867_LED_1000_BT_LINK;
> +	dp83867->led_mode[DP83867_LED_2] = DP83867_LED_RX_TX_ACT;
> +	dp83867->led_mode[DP83867_LED_GPIO] = DP83867_LED_100_BT_LINK;
> +
>  	phydev->priv = dp83867;
>  
>  	return dp83867_of_init(phydev);
> @@ -792,6 +843,12 @@ static int dp83867_config_init(struct phy_device *phydev)
>  		phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_SGMIICTL, val);
>  	}
>  
> +	val = FIELD_PREP(DP83867_LEDCR1_LED_0_SEL, dp83867->led_mode[DP83867_LED_0]) |
> +	      FIELD_PREP(DP83867_LEDCR1_LED_1_SEL, dp83867->led_mode[DP83867_LED_1]) |
> +	      FIELD_PREP(DP83867_LEDCR1_LED_2_SEL, dp83867->led_mode[DP83867_LED_2]) |
> +	      FIELD_PREP(DP83867_LEDCR1_LED_GPIO_SEL, dp83867->led_mode[DP83867_LED_GPIO]);
> +	phy_write(phydev, DP83867_LEDCR1, val);
> +
>  	val = phy_read(phydev, DP83867_CFG3);
>  	/* Enable Interrupt output INT_OE in CFG3 register */
>  	if (phy_interrupt_is_valid(phydev))
> -- 
> 2.29.2
> 
> 

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[-- Attachment #1: Type: text/plain, Size: 3260 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/phy/dp83867.c: In function 'dp83867_of_led_mode_read':
>> drivers/net/phy/dp83867.c:550:14: warning: passing argument 4 of 'of_property_read_u32_index' makes pointer from integer without a cast [-Wint-conversion]
     550 |       index, tmp);
         |              ^~~
         |              |
         |              u32 {aka unsigned int}
   In file included from drivers/net/phy/dp83867.c:11:
   include/linux/of.h:311:28: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'u32' {aka 'unsigned int'}
     311 |            u32 index, u32 *out_value);
         |                       ~~~~~^~~~~~~~~
>> drivers/net/phy/dp83867.c:549:8: warning: 'tmp' is used uninitialized in this function [-Wuninitialized]
     549 |  err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     550 |       index, tmp);
         |       ~~~~~~~~~~~


vim +/of_property_read_u32_index +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
 > 549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67276 bytes --]

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/net/phy/dp83867.c: In function 'dp83867_of_led_mode_read':
>> drivers/net/phy/dp83867.c:550:14: warning: passing argument 4 of 'of_property_read_u32_index' makes pointer from integer without a cast [-Wint-conversion]
     550 |       index, tmp);
         |              ^~~
         |              |
         |              u32 {aka unsigned int}
   In file included from drivers/net/phy/dp83867.c:11:
   include/linux/of.h:311:28: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'u32' {aka 'unsigned int'}
     311 |            u32 index, u32 *out_value);
         |                       ~~~~~^~~~~~~~~
>> drivers/net/phy/dp83867.c:549:8: warning: 'tmp' is used uninitialized in this function [-Wuninitialized]
     549 |  err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     550 |       index, tmp);
         |       ~~~~~~~~~~~


vim +/of_property_read_u32_index +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
 > 549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 67276 bytes --]

Re: [PATCH 0/2] net: phy: dp83867: Configure LED modes via device tree

[Andrew Lunn]

On Fri, Mar 19, 2021 at 04:57:08PM +0100, Michael Tretter wrote:
> Hello,
> 
> The dp83867 has 4 LED pins, which can be multiplexed with different functions
> of the phy.
> 
> This series adds a device tree binding to describe the multiplexing of the
> functions to the LEDs and implements the binding for the dp83867 phy.
> 
> I found existing bindings for configuring the LED modes for other phys:
> 
> In Documentation/devicetree/bindings/net/micrel.txt, the binding is not
> flexible enough for the use case in the dp83867, because there is a value for
> each LED configuration, which would be a lot of values for the dp83867.
> 
> In Documentation/devicetree/bindings/net/mscc-phy-vsc8532.txt, there is a
> separate property for each LED, which would work, but I found rather
> unintuitive compared to how clock bindings etc. work.
> 
> The new binding defines two properties: one for the led names and another
> property for the modes of the LEDs with defined values in the same order.
> Currently, the binding is specific to the dp83867, but I guess that the
> binding could be made more generic and used for other phys, too.

There is some work going on to manage PHY LEDs just like other LEDs in
Linux, using /sys/class/leds.

Please try to help out with that work, rather than adding yet another
DT binding.

   Andrew

Re: [PATCH 1/2] dt-bindings: dp83867: Add binding for LED mode configuration

[Rob Herring]

On Fri, 19 Mar 2021 16:57:09 +0100, Michael Tretter wrote:
> The DP83867 supports four configurable LED pins. Describe the
> multiplexing of functions to the LEDs via device tree.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  .../devicetree/bindings/net/ti,dp83867.yaml   | 24 +++++++++++++++++++
>  include/dt-bindings/net/ti-dp83867.h          | 16 +++++++++++++
>  2 files changed, 40 insertions(+)
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,dp83867.yaml: properties:ti,dp83867-led-mode-names:items: 'anyOf' conditional failed, one must be fixed:
	{'anyOf': {'items': [{'const': 'led-0'}, {'const': 'led-1'}, {'const': 'led-2'}, {'const': 'led-gpio'}]}} is not of type 'array'
	{'items': [{'const': 'led-0'}, {'const': 'led-1'}, {'const': 'led-2'}, {'const': 'led-gpio'}]} is not of type 'array'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,dp83867.yaml: properties:ti,dp83867-led-mode-names:items: {'anyOf': {'items': [{'const': 'led-0'}, {'const': 'led-1'}, {'const': 'led-2'}, {'const': 'led-gpio'}]}} is not of type 'array'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,dp83867.yaml: properties:ti,dp83867-led-mode-names:items: 'oneOf' conditional failed, one must be fixed:
	{'anyOf': {'items': [{'const': 'led-0'}, {'const': 'led-1'}, {'const': 'led-2'}, {'const': 'led-gpio'}]}} is not of type 'array'
	{'items': [{'const': 'led-0'}, {'const': 'led-1'}, {'const': 'led-2'}, {'const': 'led-gpio'}]} is not of type 'array'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,dp83867.yaml: ignoring, error in schema: properties: ti,dp83867-led-mode-names: items
warning: no schema found in file: ./Documentation/devicetree/bindings/net/ti,dp83867.yaml

See https://patchwork.ozlabs.org/patch/1455937

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-s031-20210318 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-277-gc089cd2d-dirty
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/dp83867.c:550:49: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *out_value @@     got unsigned int [usertype] tmp @@
   drivers/net/phy/dp83867.c:550:49: sparse:     expected unsigned int [usertype] *out_value
   drivers/net/phy/dp83867.c:550:49: sparse:     got unsigned int [usertype] tmp
>> drivers/net/phy/dp83867.c:550:49: sparse: sparse: non size-preserving integer to pointer cast

vim +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
   549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35327 bytes --]

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2736 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-s031-20210318 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-277-gc089cd2d-dirty
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/dp83867.c:550:49: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *out_value @@     got unsigned int [usertype] tmp @@
   drivers/net/phy/dp83867.c:550:49: sparse:     expected unsigned int [usertype] *out_value
   drivers/net/phy/dp83867.c:550:49: sparse:     got unsigned int [usertype] tmp
>> drivers/net/phy/dp83867.c:550:49: sparse: sparse: non size-preserving integer to pointer cast

vim +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
   549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35327 bytes --]

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3460 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-randconfig-r023-20210318 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/phy/dp83867.c:550:14: warning: incompatible integer to pointer conversion passing 'u32' (aka 'unsigned int') to parameter of type 'u32 *' (aka 'unsigned int *'); take the address with & [-Wint-conversion]
                                            index, tmp);
                                                   ^~~
                                                   &
   include/linux/of.h:311:28: note: passing argument to parameter 'out_value' here
                                          u32 index, u32 *out_value);
                                                          ^
>> drivers/net/phy/dp83867.c:550:14: warning: variable 'tmp' is uninitialized when used here [-Wuninitialized]
                                            index, tmp);
                                                   ^~~
   drivers/net/phy/dp83867.c:543:9: note: initialize the variable 'tmp' to silence this warning
           u32 tmp;
                  ^
                   = 0
   2 warnings generated.


vim +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
   549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35941 bytes --]

Re: [PATCH 2/2] net: phy: dp83867: add support for changing LED modes

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3539 bytes --]

Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net/master ipvs/master net-next/master v5.12-rc3 next-20210319]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-randconfig-r023-20210318 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project fcc1ce00931751ac02498986feb37744e9ace8de)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/740c2de62bc36c66a54a8c152a65ae2ebf805515
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Tretter/net-phy-dp83867-Configure-LED-modes-via-device-tree/20210320-000027
        git checkout 740c2de62bc36c66a54a8c152a65ae2ebf805515
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/phy/dp83867.c:550:14: warning: incompatible integer to pointer conversion passing 'u32' (aka 'unsigned int') to parameter of type 'u32 *' (aka 'unsigned int *'); take the address with & [-Wint-conversion]
                                            index, tmp);
                                                   ^~~
                                                   &
   include/linux/of.h:311:28: note: passing argument to parameter 'out_value' here
                                          u32 index, u32 *out_value);
                                                          ^
>> drivers/net/phy/dp83867.c:550:14: warning: variable 'tmp' is uninitialized when used here [-Wuninitialized]
                                            index, tmp);
                                                   ^~~
   drivers/net/phy/dp83867.c:543:9: note: initialize the variable 'tmp' to silence this warning
           u32 tmp;
                  ^
                   = 0
   2 warnings generated.


vim +550 drivers/net/phy/dp83867.c

   538	
   539	#if IS_ENABLED(CONFIG_OF_MDIO)
   540	static int dp83867_of_led_mode_read(struct device_node *of_node,
   541					    const char *led_name, u32 *mode)
   542	{
   543		u32 tmp;
   544		int index;
   545		int err;
   546	
   547		index = of_property_match_string(of_node, "ti,dp83867-led-mode-names",
   548						 led_name);
   549		err = of_property_read_u32_index(of_node, "ti,dp83867-led-modes",
 > 550						 index, tmp);
   551		if (err)
   552			return err;
   553		if (tmp == 0xc || tmp >= 0xf)
   554			return -EINVAL;
   555	
   556		*mode = tmp;
   557	
   558		return 0;
   559	}
   560	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35941 bytes --]

Re: [PATCH 0/2] net: phy: dp83867: Configure LED modes via device tree

[Michael Tretter]

On Fri, 19 Mar 2021 22:19:44 +0100, Andrew Lunn wrote:
> On Fri, Mar 19, 2021 at 04:57:08PM +0100, Michael Tretter wrote:
> > The dp83867 has 4 LED pins, which can be multiplexed with different functions
> > of the phy.
> > 
> > This series adds a device tree binding to describe the multiplexing of the
> > functions to the LEDs and implements the binding for the dp83867 phy.
> > 
> > I found existing bindings for configuring the LED modes for other phys:
> > 
> > In Documentation/devicetree/bindings/net/micrel.txt, the binding is not
> > flexible enough for the use case in the dp83867, because there is a value for
> > each LED configuration, which would be a lot of values for the dp83867.
> > 
> > In Documentation/devicetree/bindings/net/mscc-phy-vsc8532.txt, there is a
> > separate property for each LED, which would work, but I found rather
> > unintuitive compared to how clock bindings etc. work.
> > 
> > The new binding defines two properties: one for the led names and another
> > property for the modes of the LEDs with defined values in the same order.
> > Currently, the binding is specific to the dp83867, but I guess that the
> > binding could be made more generic and used for other phys, too.
> 
> There is some work going on to manage PHY LEDs just like other LEDs in
> Linux, using /sys/class/leds.
> 
> Please try to help out with that work, rather than adding yet another
> DT binding.

Oh, thanks. That's even better.

For reference: https://lore.kernel.org/netdev/20190813191147.19936-1-mka@chromium.org/

Michael