diff mbox series

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

Message ID 20210319155710.2793637-3-m.tretter@pengutronix.de
State New
Headers show
Series None | expand

Commit Message

Michael Tretter March 19, 2021, 3:57 p.m. UTC 
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(+)

Comments

Michael Tretter March 19, 2021, 4:14 p.m. UTC | #1 
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
> 
>
kernel test robot March 19, 2021, 6:14 p.m. UTC | #2 
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
kernel test robot March 20, 2021, 12:46 a.m. UTC | #3 
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
kernel test robot March 20, 2021, 3:41 a.m. UTC | #4 
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
diff mbox series

Patch

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))