Re: [RFC PATCH 2/2] fbcon: expose cursor blink interval via sysfs

Tomi Valkeinen
2015-01-26T12:08:43Z

[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]

On 24/01/15 03:19, Scot Doyle wrote:
> The fbcon cursor, when set to blink, is hardcoded to toggle display state
> five times per second. Expose this setting via
> /sys/class/graphics/fbcon/cursor_blink_ms
>
> Values written to the interface set the approximate time interval in
> milliseconds between cursor toggles, from 1 to 32767. Since the interval
> is stored internally as a number of jiffies, the millisecond value read
> from the interface may not exactly match the entered value.
>
> An outstanding blink timer is reset after a new value is entered.
>
> If the cursor blink is disabled, either via the 'cursor_blink' boolean
> setting or some other mechanism, the 'cursor_blink_ms' setting may still
> be modified. The new value will be used if the blink is reactivated.
>
> Tested with intelfb.
>
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> ---
> drivers/video/console/fbcon.c | 75 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 7a2030b..0ddfcf6 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -3495,11 +3495,86 @@ err:
> return count;
> }
>
> +static ssize_t show_cursor_blink_ms(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fb_info *info;
> + struct fbcon_ops *ops;
> + int idx, ms = -1;
> +
> + if (fbcon_has_exited)
> + return 0;

Why not return an error here?

> +
> + console_lock();
> + idx = con2fb_map[fg_console];
> +
> + if (idx == -1 || registered_fb[idx] == NULL)
> + goto err;

Same here?

> + info = registered_fb[idx];
> + ops = info->fbcon_par;
> +
> + if (!ops)
> + goto err;

And here.

> +
> + ms = jiffies_to_msecs(ops->blink_jiffies);
> +
> +err:
> + console_unlock();
> + return snprintf(buf, PAGE_SIZE, "%d\n", ms);
> +}
> +
> +static ssize_t store_cursor_blink_ms(struct device *device,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct fb_info *info;
> + struct fbcon_ops *ops;
> + int idx;
> + unsigned long ms;
> +
> + if (fbcon_has_exited)
> + return count;
> +
> + console_lock();
> + idx = con2fb_map[fg_console];
> +
> + if (idx == -1 || registered_fb[idx] == NULL)
> + goto err;
> +
> + info = registered_fb[idx];
> +
> + if (!info->fbcon_par)
> + goto err;
> +
> + ops = info->fbcon_par;
> +
> + if (!ops)
> + goto err;

Here also all the above look like errors to me, so why not return an error?

> +
> + if (!kstrtoul(buf, 0, &ms)) {
> + ms = min_t(unsigned long, ms, SHRT_MAX);
> + ops->blink_jiffies = max_t(int, msecs_to_jiffies(ms), 1);
> +
> + if (info->queue.func == fb_flashcursor &&
> + ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
> + fbcon_del_cursor_timer(info);
> + fbcon_add_cursor_timer(info);
> + }
> + }
> +
> +err:
> + console_unlock();
> + return count;
> +}
> +
> static struct device_attribute device_attrs[] = {
> __ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
> __ATTR(rotate_all, S_IWUSR, NULL, store_rotate_all),
> __ATTR(cursor_blink, S_IRUGO|S_IWUSR, show_cursor_blink,
> store_cursor_blink),
> + __ATTR(cursor_blink_ms, S_IRUGO|S_IWUSR, show_cursor_blink_ms,
> + store_cursor_blink_ms),
> };
>
> static int fbcon_init_device(void)
>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]