diff mbox series

[net-next] net: phy: micrel: Adding LED feature for LAN8814 PHY

Message ID 20220628054925.14198-1-Divya.Koppera@microchip.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: phy: micrel: Adding LED feature for LAN8814 PHY | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 150 this patch: 10
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang fail Errors and warnings before: 13 this patch: 11
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 156 this patch: 10
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 132 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Divya Koppera June 28, 2022, 5:49 a.m. UTC
LED support for extended mode where
LED 1: Enhanced Mode 5 (10M/1000M/Activity)
LED 2: Enhanced Mode 4 (100M/1000M/Activity)

By default it supports KSZ9031 LED mode

Signed-off-by: Divya Koppera <Divya.Koppera@microchip.com>
---
 drivers/net/phy/micrel.c | 71 +++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 15 deletions(-)

Comments

Andrew Lunn June 28, 2022, 2:33 p.m. UTC | #1
On Tue, Jun 28, 2022 at 11:19:25AM +0530, Divya Koppera wrote:
> LED support for extended mode where
> LED 1: Enhanced Mode 5 (10M/1000M/Activity)
> LED 2: Enhanced Mode 4 (100M/1000M/Activity)
> 
> By default it supports KSZ9031 LED mode

You need to update the binding documentation.

    Andrew
Russell King (Oracle) June 29, 2022, 1:44 p.m. UTC | #2
On Tue, Jun 28, 2022 at 04:33:53PM +0200, Andrew Lunn wrote:
> On Tue, Jun 28, 2022 at 11:19:25AM +0530, Divya Koppera wrote:
> > LED support for extended mode where
> > LED 1: Enhanced Mode 5 (10M/1000M/Activity)
> > LED 2: Enhanced Mode 4 (100M/1000M/Activity)
> > 
> > By default it supports KSZ9031 LED mode
> 
> You need to update the binding documentation.

What happened to "use the LEDs interface, don't invent private bindings
for PHY LEDs" ?

Does this mean the private bindings are now acceptable and I can
resubmit my 88x3310 LED support code?
Andrew Lunn June 29, 2022, 5:01 p.m. UTC | #3
On Wed, Jun 29, 2022 at 02:44:15PM +0100, Russell King (Oracle) wrote:
> On Tue, Jun 28, 2022 at 04:33:53PM +0200, Andrew Lunn wrote:
> > On Tue, Jun 28, 2022 at 11:19:25AM +0530, Divya Koppera wrote:
> > > LED support for extended mode where
> > > LED 1: Enhanced Mode 5 (10M/1000M/Activity)
> > > LED 2: Enhanced Mode 4 (100M/1000M/Activity)
> > > 
> > > By default it supports KSZ9031 LED mode
> > 
> > You need to update the binding documentation.
> 
> What happened to "use the LEDs interface, don't invent private bindings
> for PHY LEDs" ?
> 
> Does this mean the private bindings are now acceptable and I can
> resubmit my 88x3310 LED support code?

I hummed and harded about this case. The binding is there, documented
and in use. It comes from the times before we started pushing back on
vendor LED configuration methods. All this patch does is extend the
existing binding to another device of the same family. It seemed
unfair to reject it. The mess of a vendor proprietary binding exists,
and this does not make it worse.

For 88x3310 there is no president set, it should be done via Linux
LEDs.

	Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 22139901f01c..297e58d49159 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -209,6 +209,9 @@ 
 #define PTP_TSU_INT_STS_PTP_RX_TS_OVRFL_INT_	BIT(1)
 #define PTP_TSU_INT_STS_PTP_RX_TS_EN_		BIT(0)
 
+#define LAN8814_LED_CTRL_1			0x0
+#define LAN8814_LED_CTRL_1_KSZ9031_LED_MODE_	BIT(6)
+
 /* PHY Control 1 */
 #define MII_KSZPHY_CTRL_1			0x1e
 #define KSZ8081_CTRL1_MDIX_STAT			BIT(4)
@@ -308,6 +311,10 @@  struct kszphy_priv {
 	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
 };
 
+static const struct kszphy_type lan8814_type = {
+	.led_mode_reg		= ~LAN8814_LED_CTRL_1,
+};
+
 static const struct kszphy_type ksz8021_type = {
 	.led_mode_reg		= MII_KSZPHY_CTRL_2,
 	.has_broadcast_disable	= true,
@@ -1688,6 +1695,30 @@  static int kszphy_suspend(struct phy_device *phydev)
 	return genphy_suspend(phydev);
 }
 
+static void kszphy_parse_led_mode(struct phy_device *phydev)
+{
+	const struct kszphy_type *type = phydev->drv->driver_data;
+	const struct device_node *np = phydev->mdio.dev.of_node;
+	struct kszphy_priv *priv = phydev->priv;
+	int ret;
+
+	if (type && type->led_mode_reg) {
+		ret = of_property_read_u32(np, "micrel,led-mode",
+					   &priv->led_mode);
+
+		if (ret)
+			priv->led_mode = -1;
+
+		if (priv->led_mode > 3) {
+			phydev_err(phydev, "invalid led mode: 0x%02x\n",
+				   priv->led_mode);
+			priv->led_mode = -1;
+		}
+	} else {
+		priv->led_mode = -1;
+	}
+}
+
 static int kszphy_resume(struct phy_device *phydev)
 {
 	int ret;
@@ -1720,7 +1751,6 @@  static int kszphy_probe(struct phy_device *phydev)
 	const struct device_node *np = phydev->mdio.dev.of_node;
 	struct kszphy_priv *priv;
 	struct clk *clk;
-	int ret;
 
 	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1730,20 +1760,7 @@  static int kszphy_probe(struct phy_device *phydev)
 
 	priv->type = type;
 
-	if (type && type->led_mode_reg) {
-		ret = of_property_read_u32(np, "micrel,led-mode",
-				&priv->led_mode);
-		if (ret)
-			priv->led_mode = -1;
-
-		if (priv->led_mode > 3) {
-			phydev_err(phydev, "invalid led mode: 0x%02x\n",
-				   priv->led_mode);
-			priv->led_mode = -1;
-		}
-	} else {
-		priv->led_mode = -1;
-	}
+	kszphy_parse_led_mode(phydev);
 
 	clk = devm_clk_get(&phydev->mdio.dev, "rmii-ref");
 	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
@@ -2815,8 +2832,23 @@  static int lan8814_ptp_probe_once(struct phy_device *phydev)
 	return 0;
 }
 
+static void lan8814_setup_led(struct phy_device *phydev, int val)
+{
+	int temp;
+
+	temp = lan8814_read_page_reg(phydev, 5, LAN8814_LED_CTRL_1);
+
+	if (val)
+		temp |= LAN8814_LED_CTRL_1_KSZ9031_LED_MODE_;
+	else
+		temp &= ~LAN8814_LED_CTRL_1_KSZ9031_LED_MODE_;
+
+	lan8814_write_page_reg(phydev, 5, LAN8814_LED_CTRL_1, temp);
+}
+
 static int lan8814_config_init(struct phy_device *phydev)
 {
+	struct kszphy_priv *lan8814 = phydev->priv;
 	int val;
 
 	/* Reset the PHY */
@@ -2850,11 +2882,15 @@  static int lan8814_release_coma_mode(struct phy_device *phydev)
 	gpiod_set_consumer_name(gpiod, "LAN8814 coma mode");
 	gpiod_set_value_cansleep(gpiod, 0);
 
+	if (lan8814->led_mode >= 0)
+		lan8814_setup_led(phydev, lan8814->led_mode);
+
 	return 0;
 }
 
 static int lan8814_probe(struct phy_device *phydev)
 {
+	const struct kszphy_type *type = phydev->drv->driver_data;
 	struct kszphy_priv *priv;
 	u16 addr;
 	int err;
@@ -2867,6 +2903,10 @@  static int lan8814_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	priv->type = type;
+
+	kszphy_parse_led_mode(phydev);
+
 	/* Strap-in value for PHY address, below register read gives starting
 	 * phy address value
 	 */
@@ -3068,6 +3108,7 @@  static struct phy_driver ksphy_driver[] = {
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.name		= "Microchip INDY Gigabit Quad PHY",
 	.config_init	= lan8814_config_init,
+	.driver_data	= &lan8814_type,
 	.probe		= lan8814_probe,
 	.soft_reset	= genphy_soft_reset,
 	.read_status	= ksz9031_read_status,