lkml.org 
[lkml]   [2007]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC 8/13] Char: nozomi, tty cleanup
    Date
    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/

    \
     
     \ /
      Last update: 2007-11-12 19:45    [W:0.024 / U:29.152 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site