Messages in this thread | | | Date | Fri, 26 Dec 2008 19:38:54 +0100 | From | Alessandro Zummo <> | Subject | Re: [PATCH] AMD Geode CS553X GPIO driver |
| |
On Fri, 26 Dec 2008 10:24:07 -0800 David Brownell <david-b@pacbell.net> wrote:
> > It uses the gpio framework and the gpio api as defined in > > arch/x86/kernel/geode_32.c > > Eventually I'd hope to see those geode_32.c calls just vanish. > > In fact, the "normal" way to package these GPIOs would be to > always provide them through the standard API, in arch/... code, > with no Kconfig option. Any reason you shouldn't do that in > this patch?
I didn't want to mess with something I did not wrote. Maybe a two steps approach can convince people to move on ;) (you remember what happened with people holding their old rtc code tight :) )
> > +comment "Other GPIO expanders:" > > This counts as "memory mapped" I'd say. Doesn't need > a new category, even if this does need to live outside > the relevant arch/... files.
ok
> > > + > > +config GPIO_CS553X > > + tristate "AMD CS5535/CS5536 Geode Companion Devices" > > + depends on MGEODE_LX && !CS5535_GPIO > > What's this CS5535_GPIO stuff? And why should it affect > whether this can be configured? (It's not in mainline...)
drivers/char/cs5535_gpio.c (mainline)
> > +struct cs553x_gpio_platform_data { > > + > > + unsigned gpio_base; /* number of the first GPIO */ > > + > > + resource_size_t io_base; > > Platform devices should use platform_get_resource() and > friends instead of passing resources through platform data.
ack.
> > ... other than those points, this seems like a simple and > straightforward GPIO driver. Typical of what arch/* holds > in such cases. ;)
:)
will change it a little bit and resubmit
--
Best regards,
Alessandro Zummo, Tower Technologies - Torino, Italy
http://www.towertech.it
| |