Re: [RFC 8/13] Char: nozomi, tty cleanup

[Frank Seidel <fseidel@suse.de>]

On Samstag 10 November 2007 00:48:11, you (Jiri Slaby) wrote:
> nozomi, tty cleanup
> - init and deinit tty driver at module load/unload. When the OS (user)
>   loads the driver the hardware usually is ready to driver.
> - merge (unify) MAX_PORT, NTTY_TTY_MINORS into NOZOMI_MAX_PORTS
> - remove struct nozomi_devices, it was used only as list entries

This one took a little bit longer to review and test in depth, but
i finally made it.
I really find this a really good rework, i did only a few little
very minor changes (see comments below).
Please take my comments with a big pinch of salt (as i really am very
passionate to become a kernel hacker but still lack too much knowledge
and experience in many areas).

> ...
> @@ -172,9 +172,6 @@ static int debug;
>  ...
> -#define NTTY_TTY_MINORS		MAX_PORT
> -#define NTTY_TTY_MAXMINORS	256
>  ...
> -#define MAX_PORT		4
> -#define NOZOMI_MAX_PORTS	5
> +#define NOZOMI_MAX_PORTS	4
> +#define NOZOMI_MAX_CARDS	(256 / NOZOMI_MAX_PORTS)
> +#define NOZOMI_MAX_MINORS	(NOZOMI_MAX_PORTS * NOZOMI_MAX_CARDS)

I don't see the benefit in indirectly defining the maxminors in maxcards
and count back then. Perhaps this is cleaner/better but i just don't
understand it (now). So, i just did a additional
#define NOZOMI_MAX_CARDS (NTTY_TTY_MAXMINORS / NTTY_TTY_MINORS)
which seamed more straightforward to me and didn't break anything (as the
change to NOZOMI_MAX_PORTS currently would).
But to at least merge it a bit i removed the NTTY_TTY_MINORS alias for
MAX_PORT (and of course replaced usages with MAX_PORT).

>  ...
> -		for (i = PORT_MDM; i < MAX_PORT; i++) {
> +		for (i = PORT_MDM; i < NOZOMI_MAX_PORTS; i++) {

Those changes e.g. aren't needed when just adding the one define as i
suggested above.
All other comments vanished after i studied them better :-)

I'll repost the complete patch in a few moments (in CC to Greg again).

Thanks so much for this,
Frank
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/