netdev - Re: [PATCH RFC leds + net-next v4 1/2] net: phy: add API for LEDs controlled by PHY HW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200728171128.61c7193b@dellmb.labs.office.nic.cz>
Date:   Tue, 28 Jul 2020 17:11:28 +0200
From:   Marek Behún <marek.behun@....cz>
To:     netdev@...r.kernel.org
Cc:     linux-leds@...r.kernel.org, Pavel Machek <pavel@....cz>,
        jacek.anaszewski@...il.com, Dan Murphy <dmurphy@...com>,
        Ondřej Jirman <megous@...ous.com>,
        Russell King <linux@...linux.org.uk>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Andrew Lunn <andrew@...n.ch>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC leds + net-next v4 1/2] net: phy: add API for LEDs
 controlled by PHY HW

On Tue, 28 Jul 2020 17:05:29 +0200
Marek Behún <marek.behun@....cz> wrote:

> @@ -736,6 +777,16 @@ struct phy_driver {
>  	int (*set_loopback)(struct phy_device *dev, bool enable);
>  	int (*get_sqi)(struct phy_device *dev);
>  	int (*get_sqi_max)(struct phy_device *dev);
> +
> +	/* PHY LED support */
> +	int (*led_init)(struct phy_device *dev, struct
> phy_device_led *led);
> +	int (*led_brightness_set)(struct phy_device *dev, struct
> phy_device_led *led,
> +				  enum led_brightness brightness);
> +	const char *(*led_iter_hw_mode)(struct phy_device *dev,
> struct phy_device_led *led,
> +					void **	iter);
> +	int (*led_set_hw_mode)(struct phy_device *dev, struct
> phy_device_led *led,
> +			       const char *mode);
> +	const char *(*led_get_hw_mode)(struct phy_device *dev,
> struct phy_device_led *led); };
>  #define to_phy_driver(d)
> container_of(to_mdio_common_driver(d),		\ struct
> phy_driver, mdiodrv)

The problem here is that the same code will have to be added to DSA
switch ops structure, which is not OK.

I wanted to put this into struct mdio_driver_common, so that all mdio
drivers would be able to have HW LEDs connected. But then I remembered
that not all DSA drivers are connected via MDIO, some are via SPI.

So maybe this could instead become part of LED API, instead of phydev
API. Structure
  struct hw_controlled_led
and
  struct hw_controlled_led_ops
could be offered by the LED API, which would also register the needed
trigger.

struct phydev, struct dsa_switch and other could then just contain
pointer to struct hw_controlled_led_ops...

Marek