[RFC PATCH 2/5] CELL bogus_console port to hvc_console backend driver

Tue Dec 6 04:27:07 EST 2005

Hi Ryan.

On Dec 4, 2005, at 3:12 PM, Ryan S. Arnold wrote:

> This patch shuffles around some data-type declarations and moves some
> functions out of include/asm-ppc64/hvconsole.h and into a new
> drivers/char/hvc_console.h file.
>
> Signed-off-by: Ryan S. Arnold <rsa at us.ibm.com>
>
> diff -uNr linux-2.6.14-rc5/drivers/char/hvc_console.c 
> linux-2.6.14-rc5-cbe-fss/drivers/char/hvc_console.c
> --- linux-2.6.14-rc5/drivers/char/hvc_console.c	2005-10-20 
> 02:23:05.000000000 -0400
> +++ linux-2.6.14-rc5-cbe-fss/drivers/char/hvc_console.c	2005-12-02 
> 17:20:58.095207576 -0500
> @@ -40,7 +40,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/delay.h>
>  #include <asm/uaccess.h>
> -#include <asm/hvconsole.h>
> +#include "hvc_console.h"
>
>  #define HVC_MAJOR	229
>  #define HVC_MINOR	0
> @@ -61,11 +61,6 @@
>   */
>  #define HVC_ALLOC_TTY_ADAPTERS	8

Above should be in .h file, and consistent with console (or more).

>
> -#define N_OUTBUF	16
> -#define N_INBUF		16
> -
> -#define __ALIGNED__	__attribute__((__aligned__(8)))
> -
>  static struct tty_driver *hvc_driver;
>  static struct task_struct *hvc_task;
>
> @@ -76,22 +71,6 @@
>  static int sysrq_pressed;
>  #endif
>
> -struct hvc_struct {
> -	spinlock_t lock;
> -	int index;
> -	struct tty_struct *tty;
> -	unsigned int count;
> -	int do_wakeup;
> -	char outbuf[N_OUTBUF] __ALIGNED__;
> -	int n_outbuf;
> -	uint32_t vtermno;
> -	struct hv_ops *ops;
> -	int irq_requested;
> -	int irq;
> -	struct list_head next;
> -	struct kobject kobj; /* ref count & hvc_struct lifetime */
> -};
> -
>  /* dynamic list of hvc_struct instances */
>  static struct list_head hvc_structs = LIST_HEAD_INIT(hvc_structs);
>
> @@ -136,7 +115,6 @@
>  	return hp;
>  }
>
> -
>  /*
>   * Initial console vtermnos for console API usage prior to full 
> console
>   * initialization.  Any vty adapter outside this range will not have 
> usable
> @@ -154,6 +132,7 @@
>
>  void hvc_console_print(struct console *co, const char *b, unsigned 
> count)
>  {
> +	/* This [16] should probably use a #define */

N_OUTBUF perhaps?

>  	char c[16] __ALIGNED__;
>  	unsigned i = 0, n = 0;
>  	int r, donecr = 0, index = co->index;
> diff -uNr linux-2.6.14-rc5/drivers/char/hvc_console.h 
> linux-2.6.14-rc5-cbe-fss/drivers/char/hvc_console.h
> --- linux-2.6.14-rc5/drivers/char/hvc_console.h	1969-12-31 
> 19:00:00.000000000 -0500
> +++ linux-2.6.14-rc5-cbe-fss/drivers/char/hvc_console.h	2005-12-02 
> 17:27:07.733180280 -0500
> @@ -0,0 +1,83 @@
> +/*
> + * hvc_console.h
> + * Copyright (C) 2005 IBM Corporation
> + *
> + * Author(s):
> + * 	Ryan S. Arnold <rsa at us.ibm.com>
> + *
> + * hvc_console header information:
> + *      moved here from include/asm-ppc64/hvconsole.h
> + *      and drivers/char/hvc_console.c
> + *
> + * This program is free software; you can redistribute it and/or 
> modify
> + * it under the terms of the GNU General Public License as published 
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  
> 02111-1307 USA
> + */
> +
> +#ifndef HVC_CONSOLE_H
> +#define HVC_CONSOLE_H
> +
> +#include <linux/spinlock.h>
> +#include <linux/list.h>
> +#include <linux/kobject.h>
> +
> +/*
> + * This is the max number of console adapters that can/will be found 
> as
> + * console devices on first stage console init.  Any number beyond 
> this range
> + * can't be used as a console device but is still a valid tty device.
> + */
> +#define MAX_NR_HVC_CONSOLES	16
> +
> +/*
> + * This is a design shortcoming, the number '16' is a vio required 
> buffer
> + * size.  This should be changeable per architecture, but hvc_struct 
> relies
> + * upon it and that struct is used by all hvc_console backend 
> drivers.  This
> + * needs to be fixed.
> + */
> +#define N_OUTBUF	16
> +#define N_INBUF		16
> +
> +#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
> +

A little bit generic for a .h file used by multiple files ... but see 
next comment.

> +/* implemented by a low level driver */
> +struct hv_ops {
> +	int (*get_chars)(uint32_t vtermno, char *buf, int count);
> +	int (*put_chars)(uint32_t vtermno, const char *buf, int count);
> +};
> +

> +struct hvc_struct {
> +	spinlock_t lock;
> +	int index;
> +	struct tty_struct *tty;
> +	unsigned int count;
> +	int do_wakeup;
> +	char outbuf[N_OUTBUF] __ALIGNED__;
> +	int n_outbuf;
> +	uint32_t vtermno;
> +	struct hv_ops *ops;
> +	int irq_requested;
> +	int irq;
> +	struct list_head next;
> +	struct kobject kobj; /* ref count & hvc_struct lifetime */
> +};

Why are you putting the full structure definition in the .h file
instead of just declaring the struct?   It only encourages clients
to dig into the structure instead of treating it as magic cookie.

> +
> +/* Register a vterm and a slot index for use as a console 
> (console_init) */
> +extern int hvc_instantiate(uint32_t vtermno, int index, struct hv_ops 
> *ops);
> +
> +/* register a vterm for hvc tty operation (module_init or hotplug 
> add) */
> +extern struct hvc_struct * __devinit hvc_alloc(uint32_t vtermno, int 
> irq,
> +						 struct hv_ops *ops);
> +/* remove a vterm from hvc tty operation (modele_exit or hotplug 
> remove) */
> +extern int __devexit hvc_remove(struct hvc_struct *hp);
> +
> +#endif // HVC_CONSOLE_H
> diff -uNr linux-2.6.14-rc5/include/asm-ppc64/hvconsole.h 
> linux-2.6.14-rc5-cbe-fss/include/asm-ppc64/hvconsole.h
> --- linux-2.6.14-rc5/include/asm-ppc64/hvconsole.h	2005-10-20 
> 02:23:05.000000000 -0400
> +++ linux-2.6.14-rc5-cbe-fss/include/asm-ppc64/hvconsole.h	2005-11-14 
> 16:24:02.000000000 -0500
> @@ -22,28 +22,7 @@
>  #ifndef _PPC64_HVCONSOLE_H
>  #define _PPC64_HVCONSOLE_H
>
> -/*
> - * This is the max number of console adapters that can/will be found 
> as
> - * console devices on first stage console init.  Any number beyond 
> this range
> - * can't be used as a console device but is still a valid tty device.
> - */
> -#define MAX_NR_HVC_CONSOLES	16
> -
> -/* implemented by a low level driver */
> -struct hv_ops {
> -	int (*get_chars)(uint32_t vtermno, char *buf, int count);
> -	int (*put_chars)(uint32_t vtermno, const char *buf, int count);
> -};
>  extern int hvc_get_chars(uint32_t vtermno, char *buf, int count);
>  extern int hvc_put_chars(uint32_t vtermno, const char *buf, int 
> count);
>
> -struct hvc_struct;
> -
> -/* Register a vterm and a slot index for use as a console 
> (console_init) */
> -extern int hvc_instantiate(uint32_t vtermno, int index, struct hv_ops 
> *ops);
> -/* register a vterm for hvc tty operation (module_init or hotplug 
> add) */
> -extern struct hvc_struct * __devinit hvc_alloc(uint32_t vtermno, int 
> irq,
> -						 struct hv_ops *ops);
> -/* remove a vterm from hvc tty operation (modele_exit or hotplug 
> remove) */
> -extern int __devexit hvc_remove(struct hvc_struct *hp);
>  #endif /* _PPC64_HVCONSOLE_H */

Did I miss the addition of hvc_console.h to hvc_vio.c (and hvsi)?

I'm ok moving the .h but it does constrain the clients to be in
drivers/char.

milton