netdev - Re: [PATCH RFC 2/7] net: Add helpers for netdev LEDs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <f194b4a4-0bbf-4cd0-81c2-b0fbe5ac8a8a@lunn.ch>
Date: Fri, 22 Mar 2024 19:56:20 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Josua Mayer <josua@...id-run.com>
Cc: Florian Fainelli <f.fainelli@...il.com>,
	Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Russell King <linux@...linux.org.uk>,
	Gregory Clement <gregory.clement@...tlin.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC 2/7] net: Add helpers for netdev LEDs

On Fri, Mar 22, 2024 at 06:33:28PM +0000, Josua Mayer wrote:
> Am 17.03.24 um 22:45 schrieb Andrew Lunn:
> > Add a set of helpers for parsing the standard device tree properties
> > for LEDs are part of an ethernet device, and registering them with the
> > LED subsystem. This code can be used by any sort of netdev driver, DSA
> > switch or pure switchdev switch driver.
> >
> > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> > ---
> > ...
> >
> > +struct netdev_leds_ops {
> > +	int (*brightness_set)(struct net_device *ndev, u8 led,
> > +			      enum led_brightness brightness);
> > +	int (*blink_set)(struct net_device *ndev, u8 led,
> > +			 unsigned long *delay_on,  unsigned long *delay_off);
> > +	int (*hw_control_is_supported)(struct net_device *ndev, u8 led,
> > +				       unsigned long flags);
> > +	int (*hw_control_set)(struct net_device *ndev, u8 led,
> > +			      unsigned long flags);
> > +	int (*hw_control_get)(struct net_device *ndev, u8 led,
> > +			      unsigned long *flags);
> > +};
> I noticed phy.h calls the "flags" argument "rules" instead,
> perhaps that is more suitable.

The naming is a bit inconsistent. include/linux/leds.h uses:

        /*
         * Check if the LED driver supports the requested mode provided by the
         * defined supported trigger to setup the LED to hw control mode.
         *
         * Return 0 on success. Return -EOPNOTSUPP when the passed flags are not
         * supported and software fallback needs to be used.
         * Return a negative error number on any other case  for check fail due
         * to various reason like device not ready or timeouts.
         */
        int                     (*hw_control_is_supported)(struct led_classdev *led_cdev,
                                                           unsigned long flags);
        /*
         * Activate hardware control, LED driver will use the provided flags
         * from the supported trigger and setup the LED to be driven by hardware
         * following the requested mode from the trigger flags.
         * Deactivate hardware blink control by setting brightness to LED_OFF via
         * the brightness_set() callback.
         *
         * Return 0 on success, a negative error number on flags apply fail.
         */
        int                     (*hw_control_set)(struct led_classdev *led_cdev,
                                                  unsigned long flags);
        /*
         * Get from the LED driver the current mode that the LED is set in hw
         * control mode and put them in flags.
         * Trigger can use this to get the initial state of a LED already set in
         * hardware blink control.
         *
         * Return 0 on success, a negative error number on failing parsing the
         * initial mode. Error from this function is NOT FATAL as the device
         * may be in a not supported initial state by the attached LED trigger.
         */
        int                     (*hw_control_get)(struct led_classdev *led_cdev,
                                                  unsigned long *flags);

So if anything, phy.h should really change to use flags.

   Andrew