On Thu, Apr 13, 2006 at 19:32:00 +0200, Julio M. Merino Vidal wrote:

> Attached is a patch to fix the get/putwschar functions.  It:
> 
> - Removes the getwschar and putwschar accessops from the wsdisplay
>   devices.
[...]
> - Gets rid of the WSDISPLAY_CHARFUNCS kernel option; useless.

As these changes are pretty mechanical, it would be very helpful if
they were in a separate file to make review easier.



> Index: dev/ic/vga.c

> @@ -771,6 +760,7 @@ int
>  vga_ioctl(void *v, void *vs, u_long cmd, caddr_t data, int flag, struct lwp *l)
>  {
>  	struct vga_config *vc = v;
> +	struct vgascreen *scr = vs;
>  	const struct vga_funcs *vf = vc->vc_funcs;
>  
>  	switch (cmd) {
> @@ -791,6 +781,14 @@ vga_ioctl(void *v, void *vs, u_long cmd,
>  		vga_set_video(vc, *(int *)data == WSDISPLAYIO_VIDEO_ON);
>  		return 0;
>  
> +	case WSDISPLAYIO_GETWSCHAR:
> +		return pcdisplay_getwschar((struct pcdisplayscreen *)scr,
> +		    (struct wsdisplay_char *)data);

Please, don't use casts like thas.  &scr->pcs is shorter and is easier
to read and understand (see below).


> +
> +	case WSDISPLAYIO_PUTWSCHAR:
> +		return pcdisplay_putwschar((struct pcdisplayscreen *)scr,
> +		    (struct wsdisplay_char *)data);

Ditto.


>  #ifdef WSDISPLAY_CUSTOM_BORDER
>  	case WSDISPLAYIO_GBORDER:
>  		return (vga_getborder(vc, (u_int *)data));
> @@ -1417,27 +1415,6 @@ vga_putchar(void *c, int row, int col, u
>  	pcdisplay_putchar(c, row, col, uc, attr);
>  }
>  
> -
> -#ifdef WSDISPLAY_CHARFUNCS
> -int
> -vga_getwschar(void *cookie, struct wsdisplay_char *wschar)
> -{
> -	struct vgascreen *scr = cookie;
> -
> -	if (scr == NULL) return 0;
> -	return (pcdisplay_getwschar(&scr->pcs, wschar));

This is the correct way.




> Index: dev/wscons/wsdisplay_vcons.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/wscons/wsdisplay_vcons.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 wsdisplay_vcons.c
> --- dev/wscons/wsdisplay_vcons.c	19 Feb 2006 03:51:03 -0000	1.5
> +++ dev/wscons/wsdisplay_vcons.c	13 Apr 2006 17:27:43 -0000
> @@ -60,6 +60,7 @@ __KERNEL_RCSID(0, "$NetBSD: wsdisplay_vc
>  static void vcons_dummy_init_screen(void *, struct vcons_screen *, int, 
>  	    long *);
>  
> +static int  vcons_ioctl(void *, void *, u_long, caddr_t, int, struct lwp *);
>  static int  vcons_alloc_screen(void *, const struct wsscreen_descr *, void **, 
>  	    int *, int *, long *);
>  static void vcons_free_screen(void *, void *);
> @@ -87,8 +88,8 @@ static void vcons_putchar(void *, int, i
>  static void vcons_cursor(void *, int, int, int);
>  
>  /* support for readin/writing text buffers. For wsmoused */
> -static int  vcons_putwschar(void *, struct wsdisplay_char *);
> -static int  vcons_getwschar(void *, struct wsdisplay_char *);
> +static int  vcons_putwschar(struct rasops_info *, struct wsdisplay_char *);
> +static int  vcons_getwschar(struct rasops_info *, struct wsdisplay_char *);

Is rasops_info semantically correct way to refer to a vcons screen?
Admittedly I haven't checked it thoroughly, but I suspect - no.
Michael?


> @@ -361,6 +366,34 @@ vcons_redraw_screen(struct vcons_screen 
>  }
>  
>  static int
> +vcons_ioctl(void *v, void *vs, u_long cmd, caddr_t data, int flag,
> +	struct lwp *l)
> +{
> +	struct vcons_data *vd = v;
> +	int error;
> +
> +	switch (cmd) {
> +	case WSDISPLAYIO_GETWSCHAR:
> +		error = vcons_getwschar(vd->cookie,
> +			(struct wsdisplay_char *)data);
> +		break;

vd->cookie is driver's cookie to vcons.  It cannot be rasops (see
prototype in the previous hunk).  E.g. isgfb passes struct
igsfb_devconfig as cookie.


> +
> +	case WSDISPLAYIO_PUTWSCHAR:
> +		error = vcons_putwschar(vd->cookie,
> +			(struct wsdisplay_char *)data);
> +		break;

Ditto.


> +
> +	default:
> +		if (vd->ioctl != NULL)
> +			error = (*vd->ioctl)(v, vs, cmd, data, flag, l);
> +		else
> +			error = EINVAL;
> +	}
> +
> +	return error;
> +}

Use EPASSTHROUGH instead.  Upper layers will handle it.



SY, Uwe
-- 
uwe@ptc.spbu.ru                         |       Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/          |       Ist zu Grunde gehen