On Thu, Jul 23, 2020 at 08:13:19PM +0200, Marek Behún wrote:

This patch adds support for controlling the LEDs connected to several
families of Marvell PHYs via Linux' LED API. These families are:
88E1112, 88E1121R, 88E1240, 88E1340S, 88E1510 and 88E1545. More can be
added.

The code reads LEDs definitions from the device-tree node of the PHY.

Since the LEDs can be controlled by hardware, we add one LED-private LED
trigger named "hw-control". This trigger is private and displayed only
for Marvell PHY LEDs.

When this driver is activated, another sysfs file is created in that
LEDs sysfs directory, names "hw_control". This file contains space
separated list of possible HW controlled modes for this LED. The one
which is selected is enclosed by square brackets. To change HW control
mode the user has to write the name of desired mode to this "hw_control"
file.

This patch does not yet add support for compound LED modes. This could
be achieved via the LED multicolor framework (which is not yet in
upstream).

Settings such as HW blink rate or pulse stretch duration are not yet
supported, nor are LED polarity settings.

Hi Marek

I expect some of this should be moved into the phylib core. We don't
want each PHY inventing its own way to do this. The core should
provide a framework and the PHY driver fills in the gaps.

Take a look at for example mscc_main.c and its LED information. It has
pretty similar hardware to the Marvell. And microchip.c also has LED
handling, etc.

+static int _marvell_led_brightness_set(struct led_classdev *cdev, enum led_brightness brightness,
+				       bool check_trigger)

Please avoid _ functions. 

+{
+	struct phy_device *phydev = to_phy_device(cdev->dev->parent);
+	struct marvell_phy_led *led = to_marvell_phy_led(cdev);
+	u8 val;
+
+	/* don't do anything if HW control is enabled */
+	if (check_trigger && cdev->trigger == &marvell_hw_led_trigger)
+		return 0;

I thought the brightness file disappeared when a trigger takes
over. So is this possible?

      Andrew