Messages in this thread | | | Subject | Re: [PATCH 5/8] minnowboard-gpio: Export MinnowBoard expansion GPIO | From | Andy Shevchenko <> | Date | Wed, 26 Jun 2013 10:55:28 +0300 |
| |
On Tue, 2013-06-25 at 18:53 -0700, Darren Hart wrote: > Request and export the user-configurable GPIO lines to sysfs. This provides a > label readable in /debugfs/gpio and a simple interface for experimenting with > GPIO on the MinnowBoard. > > This is separate from the minnowboard driver to provide users with the > flexibility to write kernel drivers for their own devices using these GPIO > lines.
Few comments below.
> --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -35,6 +35,24 @@ config MINNOWBOARD > > If you have a MinnowBoard, say Y or M here. > > +if MINNOWBOARD > +config MINNOWBOARD_GPIO > + tristate "MinnowBoard Expansion GPIO" > + depends on MINNOWBOARD > + default n
Like you already had been told you don't need to have default n.
> --- /dev/null > +++ b/drivers/platform/x86/minnowboard-gpio.c
> +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/gpio.h> > +#include <linux/gpio_keys.h> > +#include <linux/input.h> > +#include <linux/minnowboard.h>
+ empty line here?
> +#include "minnowboard-gpio.h"
> +static int __init minnow_gpio_module_init(void) > +{ > + int err; > + > + err = -ENODEV; > + if (!minnow_detect()) > + goto out; > + > +#ifdef MODULE > +#ifdef CONFIG_MINNOWBOARD_MODULE
And less ifdefs with IS_MODULE().
> + if (request_module("minnowboard")) > + goto out; > +#endif > +#endif > + > + /* Auxillary Expansion GPIOs */ > + if (!minnow_lvds_detect()) { > + pr_debug("LVDS_DETECT not asserted, configuring Aux GPIO lines\n"); > + err = gpio_request_array(expansion_aux_gpios, > + ARRAY_SIZE(expansion_aux_gpios)); > + if (err) { > + pr_err("Failed to request expansion aux GPIO lines\n"); > + goto out; > + } > + } else { > + pr_debug("LVDS_DETECT asserted, ignoring aux GPIO lines\n"); > + } > + > + /* Expansion GPIOs */ > + err = gpio_request_array(expansion_gpios, ARRAY_SIZE(expansion_gpios)); > + if (err) { > + pr_err("Failed to request expansion GPIO lines\n"); > + if (minnow_lvds_detect()) > + gpio_free_array(expansion_aux_gpios, > + ARRAY_SIZE(expansion_aux_gpios)); > + goto out; > + } > + > + out: > + return err;
Are you planning to add something else to 'out' path? Otherwise I think it will look better if you do return instead of [useless] gotos.
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |