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
