netdev - Re: [PATCH net-next v1 2/3] net: phy: add API for LEDs controlled by ethernet PHY chips

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 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]

Date:   Sun, 4 Oct 2020 11:58:53 +0200
From:   Pavel Machek <pavel@....cz>
To:     Marek Beh??n <marek.behun@....cz>
Cc:     netdev@...r.kernel.org, linux-leds@...r.kernel.org,
        Dan Murphy <dmurphy@...com>,
        Ond??ej Jirman <megous@...ous.com>,
        Russell King <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>, linux-kernel@...r.kernel.org,
        Matthias Schiffer <matthias.schiffer@...tq-group.com>
Subject: Re: [PATCH net-next v1 2/3] net: phy: add API for LEDs controlled by
 ethernet PHY chips

Hi!

> Many an ethernet PHY supports various HW control modes for LEDs
> connected directly to the PHY chip.
> 
> This patch adds code for registering such LEDs when described in device
> tree and also adds a new private LED trigger called phydev-hw-mode.
> When this trigger is enabled for a LED, the various HW control modes
> which are supported by the PHY for given LED cat be get/set via hw_mode
> sysfs file.
> 
> A PHY driver wishing to utilize this API needs to implement all the
> methods in the phy_device_led_ops structure.
> 
> Signed-off-by: Marek Beh??n <marek.behun@....cz>


>  	select MDIO_I2C
>  
> +config PHY_LEDS
> +	bool
> +	default y if LEDS_TRIGGERS
> +
>  comment "MII PHY device drivers"
>  
>  config AMD_PHY

> +/* drivers/net/phy/phy_hw_led_mode.c
> + *

Stale comment.

> +	init_data.fwnode = &np->fwnode;
> +	init_data.devname_mandatory = true;
> +	snprintf(devicename, sizeof(devicename), "phy%d", phydev->phyindex);
> +	init_data.devicename = devicename;
> +
> +	ret = phydev->led_ops->led_init(phydev, led, &pdata);
> +	if (ret < 0)
> +		goto err_free;
> +
> +	ret = devm_led_classdev_register_ext(&phydev->mdio.dev, &led->cdev, &init_data);
> +	if (ret < 0)
> +		goto err_free;
> +
> +	led->flags |= PHY_DEVICE_LED_REGISTERED;
> +
> +	return 0;
> +err_free:
> +	devm_kfree(&phydev->mdio.dev, led);
> +	return ret;

devm should take care of freeing, right?

Plus, format comments to 80 colums. checkpatch no longer warns, but rule still exists.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html