diff mbox series

[net-next,v5,3/3] leds: trigger: netdev: expose hw_control status via sysfs

Message ID 20230619204700.6665-4-ansuelsmth@gmail.com (mailing list archive)
State Accepted
Commit b655892ffd6d89b0c7407e099c40dbde82ee3f03
Delegated to: Netdev Maintainers
Headers show
Series leds: trigger: netdev: add additional modes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 23 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Christian Marangi June 19, 2023, 8:47 p.m. UTC
Expose hw_control status via sysfs for the netdev trigger to give
userspace better understanding of the current state of the trigger and
the LED.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/leds/trigger/ledtrig-netdev.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Kalesh Anakkur Purayil June 20, 2023, 3:50 a.m. UTC | #1
On Tue, Jun 20, 2023 at 2:18 AM Christian Marangi <ansuelsmth@gmail.com>
wrote:

> Expose hw_control status via sysfs for the netdev trigger to give
> userspace better understanding of the current state of the trigger and
> the LED.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/leds/trigger/ledtrig-netdev.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/leds/trigger/ledtrig-netdev.c
> b/drivers/leds/trigger/ledtrig-netdev.c
> index 2c1c9e95860e..32b66703068a 100644
> --- a/drivers/leds/trigger/ledtrig-netdev.c
> +++ b/drivers/leds/trigger/ledtrig-netdev.c
> @@ -406,6 +406,16 @@ static ssize_t interval_store(struct device *dev,
>
>  static DEVICE_ATTR_RW(interval);
>
> +static ssize_t hw_control_show(struct device *dev,
> +                              struct device_attribute *attr, char *buf)
> +{
> +       struct led_netdev_data *trigger_data =
> led_trigger_get_drvdata(dev);
> +
> +       return sprintf(buf, "%d\n", trigger_data->hw_control);
>
[Kalesh]: How about using sysfs_emit?

> +}
> +
> +static DEVICE_ATTR_RO(hw_control);
> +
>  static struct attribute *netdev_trig_attrs[] = {
>         &dev_attr_device_name.attr,
>         &dev_attr_link.attr,
> @@ -417,6 +427,7 @@ static struct attribute *netdev_trig_attrs[] = {
>         &dev_attr_rx.attr,
>         &dev_attr_tx.attr,
>         &dev_attr_interval.attr,
> +       &dev_attr_hw_control.attr,
>         NULL
>  };
>  ATTRIBUTE_GROUPS(netdev_trig);
> --
> 2.40.1
>
>
>
Andrew Lunn June 20, 2023, 1:21 p.m. UTC | #2
>     +static ssize_t hw_control_show(struct device *dev,
>     +                              struct device_attribute *attr, char *buf)
>     +{
>     +       struct led_netdev_data *trigger_data = led_trigger_get_drvdata
>     (dev);
>     +
>     +       return sprintf(buf, "%d\n", trigger_data->hw_control);
> 
> [Kalesh]: How about using sysfs_emit? 

Currently, there are zero instances of sysfs_emit() in ledtrig-netdev,
and in any files in drivers/leds/trigger/, or even drivers/leds.

So i think it would be better to keep this consistent with the rest of
the code in this file, and have a follow up patchset which reviews and
converts the 52 sprintf() in drivers/leds.

	 Andrew
Kalesh Anakkur Purayil June 20, 2023, 2:25 p.m. UTC | #3
On Tue, Jun 20, 2023 at 6:51 PM Andrew Lunn <andrew@lunn.ch> wrote:

> >     +static ssize_t hw_control_show(struct device *dev,
> >     +                              struct device_attribute *attr, char
> *buf)
> >     +{
> >     +       struct led_netdev_data *trigger_data =
> led_trigger_get_drvdata
> >     (dev);
> >     +
> >     +       return sprintf(buf, "%d\n", trigger_data->hw_control);
> >
> > [Kalesh]: How about using sysfs_emit?
>
> Currently, there are zero instances of sysfs_emit() in ledtrig-netdev,
> and in any files in drivers/leds/trigger/, or even drivers/leds.
>
> So i think it would be better to keep this consistent with the rest of
> the code in this file, and have a follow up patchset which reviews and
> converts the 52 sprintf() in drivers/leds.
>
Sounds good, thank you

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

>
>          Andrew
>
Lee Jones June 21, 2023, 2:55 p.m. UTC | #4
On Mon, 19 Jun 2023, Christian Marangi wrote:

> Expose hw_control status via sysfs for the netdev trigger to give
> userspace better understanding of the current state of the trigger and
> the LED.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/leds/trigger/ledtrig-netdev.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Acked-by: Lee Jones <lee@kernel.org>
diff mbox series

Patch

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 2c1c9e95860e..32b66703068a 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -406,6 +406,16 @@  static ssize_t interval_store(struct device *dev,
 
 static DEVICE_ATTR_RW(interval);
 
+static ssize_t hw_control_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct led_netdev_data *trigger_data = led_trigger_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", trigger_data->hw_control);
+}
+
+static DEVICE_ATTR_RO(hw_control);
+
 static struct attribute *netdev_trig_attrs[] = {
 	&dev_attr_device_name.attr,
 	&dev_attr_link.attr,
@@ -417,6 +427,7 @@  static struct attribute *netdev_trig_attrs[] = {
 	&dev_attr_rx.attr,
 	&dev_attr_tx.attr,
 	&dev_attr_interval.attr,
+	&dev_attr_hw_control.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(netdev_trig);