Hello Maciek-
Some architectural questions below:
On Thu, Oct 01, 2015 at 04:04:31PM +0200, Maciek Borzecki wrote:
> The patch adds LED triggers for indicating an activity on a selected
> device. The drivers that intend to use triggers need to register
> respective devices using ledtrig_dev_add(). Triggers are generated by
> explicitly calling ledtrig_dev_activity().
>
> Signed-off-by: Maciek Borzecki <maciek.borzecki@xxxxxxxxx>
> ---
[..]
> +struct ledtrig_dev_data {
> + char name[MAX_NAME_LEN];
> + dev_t dev;
> + struct led_trigger *trig;
> + struct list_head node;
> +};
> +
> +/**
> + * ledtrig_dev_activity - signal activity on device
> + * @dev: device
> + *
> + * Fires a trigger assigned to @dev device.
> + */
> +void ledtrig_dev_activity(dev_t dev)
It seems a bit strange to me to associate a device LED trigger with
dev_t. Some devices don't expose a dev node, some devices expose
multiple dev nodes...
Is there a reason why you are not tying to the device model?
> +{
> + struct ledtrig_dev_data *dev_trig;
> +
> + if (!down_read_trylock(&devs_list_lock))
> + return;
> +
> + list_for_each_entry(dev_trig, &devs_list, node) {
> + if (dev_trig->dev == dev) {
> + led_trigger_blink_oneshot(dev_trig->trig,
> + &blink_delay,
> + &blink_delay,
> + 0);
> + break;
> + }
> + }
> + up_read(&devs_list_lock);
> +}
> +EXPORT_SYMBOL(ledtrig_dev_activity);
Not _GPL?
Josh
Attachment:
signature.asc
Description: PGP signature
