[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20111101121851.7d393277@bob.linux.org.uk>
Date: Tue, 1 Nov 2011 12:18:51 +0000
From: Alan Cox <alan@...ux.intel.com>
To: Paul Schilling <paul.s.schilling@...il.com>
Cc: Ben Dooks <ben-linux@...ff.org>,
Kukjin Kim <kgene.kim@...sung.com>,
Russell King <linux@....linux.org.uk>,
Greg Kroah-Hartman <gregkh@...e.de>,
Boojin Kim <boojin.kim@...sung.com>,
Nicolas Pitre <nicolas.pitre@...aro.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-serial@...r.kernel.org
Subject: Re: [PATCH 02/14] ARM : SAMSUNG : Add RS485 support.
> +config SAMSUNG_HAS_RS485
> + bool "Enable RS485 support for Samsung"
> + depends on SERIAL_SAMSUNG && (MACH_CONDOR2440 ||
> MACH_CONDOR2416 || MACH_MINI2440)
> + default y if (MACH_CONDOR2440 || MACH_CONDOR2416)
> + default n if (MACH_MINI2440)
There really needs to be more ARM thinking about doing this sort of
stuff at runtime. If you can detect the board type at runtime then you
need to be moving towards kicking board specifics out of the depends,
and treating it as an "include support for this, if the board has it"
so you can move towards less build time only configuration.
> +/* FIXME */
> + #if 0
> + if (last_state != en) {
> +
So this doesn't look ready to submit either...
> + if ((utrstat & S3C2410_UTRSTAT_TXE) ? 1 : 0) {
> + if (cfg->gpio_transmit_en > -1) {
> +
> gpio_set_value(cfg->gpio_transmit_en, 0);
> + }
Lots of excess brackets (see CodingStyle) - removing a few of the
excess ones would make it easier to read later.
The later bits become a real mess of ifdefs - much not your fault, the
combination of your ifdefs and existing lack of design push it beyond
ugly and really want addressing.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists