[net-next,03/11] net: phy: Add a binding for PHY LEDs

Message ID 20230307170046.28917-4-ansuelsmth@gmail.com
State New
Headers
Series net: Add basic LED support for switch/phy |

Commit Message

Christian Marangi March 7, 2023, 5 p.m. UTC
  From: Andrew Lunn <andrew@lunn.ch>

Define common binding parsing for all PHY drivers with LEDs using
phylib. Parse the DT as part of the phy_probe and add LEDs to the
linux LED class infrastructure. For the moment, provide a dummy
brightness function, which will later be replaced with a call into the
PHY driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 89 ++++++++++++++++++++++++++++++++++++
 include/linux/phy.h          | 16 +++++++
 2 files changed, 105 insertions(+)
  

Comments

Christian Marangi March 7, 2023, 6 p.m. UTC | #1
On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > Define common binding parsing for all PHY drivers with LEDs using
> > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > linux LED class infrastructure. For the moment, provide a dummy
> > brightness function, which will later be replaced with a call into the
> > PHY driver.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Hi Christian
> 
> Since you are submitting this, you need to add your own Signed-off-by:
> after mine.
> 

Tought it was needed only for patch where I have put any change. (case
of 2 patch in this series where there was a whitespace error and had to
change a binding)

Think I need do to this for every other patch right?
  
Andrew Lunn March 7, 2023, 11:17 p.m. UTC | #2
On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Define common binding parsing for all PHY drivers with LEDs using
> phylib. Parse the DT as part of the phy_probe and add LEDs to the
> linux LED class infrastructure. For the moment, provide a dummy
> brightness function, which will later be replaced with a call into the
> PHY driver.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Hi Christian

Since you are submitting this, you need to add your own Signed-off-by:
after mine.

      Andrew
  
Andrew Lunn March 8, 2023, 12:54 a.m. UTC | #3
On Tue, Mar 07, 2023 at 07:00:08PM +0100, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> > On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > 
> > > Define common binding parsing for all PHY drivers with LEDs using
> > > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > > linux LED class infrastructure. For the moment, provide a dummy
> > > brightness function, which will later be replaced with a call into the
> > > PHY driver.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > Hi Christian
> > 
> > Since you are submitting this, you need to add your own Signed-off-by:
> > after mine.
> > 
> 
> Tought it was needed only for patch where I have put any change. (case
> of 2 patch in this series where there was a whitespace error and had to
> change a binding)
> 
> Think I need do to this for every other patch right?

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

says:

   Any further SoBs (Signed-off-by:’s) following the author’s SoB are
   from people handling and transporting the patch, but were not
   involved in its development. SoB chains should reflect the real
   route a patch took as it was propagated to the maintainers and
   ultimately to Linus, with the first SoB entry signalling primary
   authorship of a single author.

So yes, you need to add your Signed-off-by to all my patches,
independent of if you make changes or not.

	    Andrew
  

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9ba8f973f26f..8acade42615c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -19,10 +19,12 @@ 
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/mii.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_led_triggers.h>
@@ -658,6 +660,7 @@  struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 	device_initialize(&mdiodev->dev);
 
 	dev->state = PHY_DOWN;
+	INIT_LIST_HEAD(&dev->led_list);
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
@@ -2964,6 +2967,85 @@  static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+/* Dummy implementation until calls into PHY driver are added */
+static int phy_led_set_brightness(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	return 0;
+}
+
+static int of_phy_led(struct phy_device *phydev,
+		      struct device_node *led)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_init_data init_data = {};
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+	int err;
+
+	phyled = devm_kzalloc(dev, sizeof(*phyled), GFP_KERNEL);
+	if (!phyled)
+		return -ENOMEM;
+
+	phyled->phydev = phydev;
+	cdev = &phyled->led_cdev;
+	INIT_LIST_HEAD(&phyled->led_list);
+
+	err = of_property_read_u32(led, "reg", &phyled->index);
+	if (err)
+		return err;
+
+	cdev->brightness_set_blocking = phy_led_set_brightness;
+	cdev->max_brightness = 1;
+	init_data.devicename = dev_name(&phydev->mdio.dev);
+	init_data.fwnode = of_fwnode_handle(led);
+
+	err = devm_led_classdev_register_ext(dev, cdev, &init_data);
+	if (err)
+		return err;
+
+	list_add(&phyled->led_list, &phydev->led_list);
+
+	return 0;
+}
+
+static int of_phy_leds(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct device_node *leds, *led;
+	int err;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (!node)
+		return 0;
+
+	leds = of_get_child_by_name(node, "leds");
+	if (!leds)
+		return 0;
+
+	for_each_available_child_of_node(leds, led) {
+		err = of_phy_led(phydev, led);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void phy_leds_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+
+	list_for_each_entry(phyled, &phydev->led_list, led_list) {
+		cdev = &phyled->led_cdev;
+		devm_led_classdev_unregister(dev, cdev);
+	}
+}
+
 /**
  * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
  * @fwnode: pointer to the mdio_device's fwnode
@@ -3142,6 +3224,11 @@  static int phy_probe(struct device *dev)
 	/* Set the state to READY by default */
 	phydev->state = PHY_READY;
 
+	/* Get the LEDs from the device tree, and instantiate standard
+	 * LEDs for them.
+	 */
+	of_phy_leds(phydev);
+
 out:
 	/* Assert the reset signal */
 	if (err)
@@ -3156,6 +3243,8 @@  static int phy_remove(struct device *dev)
 {
 	struct phy_device *phydev = to_phy_device(dev);
 
+	phy_leds_remove(phydev);
+
 	cancel_delayed_work_sync(&phydev->state_queue);
 
 	mutex_lock(&phydev->lock);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fbeba4fee8d4..1b1efe120f0f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -14,6 +14,7 @@ 
 #include <linux/compiler.h>
 #include <linux/spinlock.h>
 #include <linux/ethtool.h>
+#include <linux/leds.h>
 #include <linux/linkmode.h>
 #include <linux/netlink.h>
 #include <linux/mdio.h>
@@ -595,6 +596,7 @@  struct macsec_ops;
  * @phy_num_led_triggers: Number of triggers in @phy_led_triggers
  * @led_link_trigger: LED trigger for link up/down
  * @last_triggered: last LED trigger for link speed
+ * @led_list: list of PHY LED structures
  * @master_slave_set: User requested master/slave configuration
  * @master_slave_get: Current master/slave advertisement
  * @master_slave_state: Current master/slave configuration
@@ -690,6 +692,7 @@  struct phy_device {
 
 	struct phy_led_trigger *led_link_trigger;
 #endif
+	struct list_head led_list;
 
 	/*
 	 * Interrupt number for this PHY
@@ -825,6 +828,19 @@  struct phy_plca_status {
 	bool pst;
 };
 
+/* phy_led: An LED driven by the PHY
+ *
+ * phydev: Pointer to the PHY this LED belongs to
+ * led_cdev: Standard LED class structure
+ * index: Number of the LED
+ */
+struct phy_led {
+	struct list_head led_list;
+	struct phy_device *phydev;
+	struct led_classdev led_cdev;
+	u32 index;
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *