| Message ID | 20230427001541.18704-6-ansuelsmth@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | leds: introduce new LED hw control APIs | expand |
> @@ -168,7 +174,7 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, > size_t size, enum led_trigger_netdev_modes attr) > { > struct led_netdev_data *trigger_data = led_trigger_get_drvdata(dev); > - unsigned long state; > + unsigned long state, new_mode = trigger_data->mode; > int ret; > int bit; > > @@ -186,12 +192,18 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, > return -EINVAL; > } > > - cancel_delayed_work_sync(&trigger_data->work); > - > if (state) > - set_bit(bit, &trigger_data->mode); > + set_bit(bit, &new_mode); > else > - clear_bit(bit, &trigger_data->mode); > + clear_bit(bit, &new_mode); > + > + ret = validate_requested_mode(trigger_data, new_mode); > + if (ret) > + return ret; > + > + cancel_delayed_work_sync(&trigger_data->work); > + > + trigger_data->mode = new_mode; > > set_baseline_state(trigger_data); I think you need to hold the trigger_data lock here, otherwise there are potential race conditions. Andrew
diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c index 115f2bae9eee..81e0b0083f2f 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -91,6 +91,12 @@ static void set_baseline_state(struct led_netdev_data *trigger_data) } } +static int validate_requested_mode(struct led_netdev_data *trigger_data, + unsigned long mode) +{ + return 0; +} + static ssize_t device_name_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -168,7 +174,7 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, size_t size, enum led_trigger_netdev_modes attr) { struct led_netdev_data *trigger_data = led_trigger_get_drvdata(dev); - unsigned long state; + unsigned long state, new_mode = trigger_data->mode; int ret; int bit; @@ -186,12 +192,18 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf, return -EINVAL; } - cancel_delayed_work_sync(&trigger_data->work); - if (state) - set_bit(bit, &trigger_data->mode); + set_bit(bit, &new_mode); else - clear_bit(bit, &trigger_data->mode); + clear_bit(bit, &new_mode); + + ret = validate_requested_mode(trigger_data, new_mode); + if (ret) + return ret; + + cancel_delayed_work_sync(&trigger_data->work); + + trigger_data->mode = new_mode; set_baseline_state(trigger_data);
Introduce function to validate the requested mode in preparation for hw control support. Currently everything is handled in software so every mode is validated and accepted. Requested mode are always validated before making any change, to follow this rework the attr_store function to set the mode to a temp variable and then apply the new mode only if accepted and validated. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/leds/trigger/ledtrig-netdev.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)