Hello Philippe,
On Fri, Jul 08, 2011 at 04:45:00PM +0200, Philippe Rétornaz wrote:
> > > +config INPUT_MC13783_ON
> > > + tristate "MC13783 ON buttons"
> > > + depends on MFD_MC13783
> > > + help
> > > + Support the ON buttons of MC13783 PMIC as an input device
> > > + reporting power button status.
> > > +
> > > + To compile this driver as a module, choose M here: the module
> > > + will be called mc13783-pwrbutton.
> >
> > Why not use INPUT_MC13783_PWRBUTTON to match the source file (and so
> > module) name.
>
> Ok.
>
> >
> > > +
> > > +
> >
> > one empty line should be enough
>
> Ok.
>
> > > diff --git a/drivers/input/misc/mc13783-pwrbutton.c
> > > b/drivers/input/misc/mc13783-pwrbutton.c new file mode 100644
> > > index 0000000..6aee01c
> > > --- /dev/null
> > > +++ b/drivers/input/misc/mc13783-pwrbutton.c
> > > @@ -0,0 +1,288 @@
> > > +/**
> > > + * mc13783-pwrbutton.c - MC13783 Power Button Input Driver
> > > + *
> > > + * Copyright (C) 2011 Philippe Rétornaz
> > > + *
> > > + * Based on twl4030-pwrbutton driver by:
> > > + * Peter De Schrijver <peter.de-schrijver@xxxxxxxxx>
> > > + * Felipe Balbi <felipe.balbi@xxxxxxxxx>
> > > + *
> > > + * This file is subject to the terms and conditions of the GNU General
> > > + * Public License. See the file "COPYING" in the main directory of this
> > > + * archive for more details.
> >
> > This implies GPL v2 only? So you either need to clearify here or you
> > need to adapt the MODULE_LICENSE line below to read
> >
> > MODULE_LICENSE("GPL v2");
>
> Well, I took exactly the same license than the twl4030-pwrbutton.
> This driver is: MODULE_LICENSE("GPL"), why should I change it ?
So this driver is wrong, too. According to include/linux/module.h, "GPL"
means "GNU Public License v2 or later" and "GPL v2" means "GNU Public
License v2".
> > > --- a/drivers/mfd/mc13xxx-core.c
> > > +++ b/drivers/mfd/mc13xxx-core.c
> > >
> > > @@ -778,6 +778,10 @@ err_revision:
> > > mc13xxx_add_subdevice_pdata(mc13xxx, "%s-led",
> > >
> > > pdata->leds, sizeof(*pdata->leds));
> > >
> > > + if (pdata->flags & MC13XXX_USE_BUTTON)
> >
> > Why not use
> >
> > if (pdata->buttons)
> >
> > to detect if the pwrbutton device should be registered?
>
> To keep consistancy as the others sub-device are registered the same way
> (regulator, led). I can do it as you propose for the button but IMHO it's a
> bit strange to have a MC13XXX_USE_* for all the subdevice except the buttons.
> So should I change ?
It would be a third patch for your commit count to convert regulator and
led, too ;-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
