Messages in this thread | | | Date | Fri, 28 Nov 2014 13:54:57 +0200 | From | Mika Westerberg <> | Subject | Re: [PATCH 1/3] platform: x86: dell-rbtn: Dell Airplane Mode Switch driver |
| |
On Fri, Nov 28, 2014 at 12:45:55PM +0100, Pali Rohár wrote: > Hello, > > I will fix all those style problems and add some comments. > > On Friday 28 November 2014 12:33:28 Mika Westerberg wrote: > > > + if (ACPI_FAILURE(status)) > > > + return; > > > + > > > + rfkill_set_states(rfkill, !output, !output); > > > > You can also write it like: > > > > if (ACPI_SUCCESS(status)) > > rfkill_set_states(rfkill, !output, !output); > > > > which looks better to me at least. > > > > In whole module I'm using this style: > > f1(); > if (f1_failed) > return; > f2() > if (f2_failed) > return; > > So I would like not to change it for one function.
Fair enough.
> > > +} > > > + > > > +static int rbtn_set_block(void *data, bool blocked) > > > +{ > > > + /* FIXME: setting soft rfkill cause problems, so disable > > > it for now */ + return -EINVAL; > > > +} > > > + > > > +struct rfkill_ops rbtn_ops = { > > > > static? const? > > > > Yes, static const should be used. > > > > > > +/*** module functions ***/ > > > + > > > +static int __init rbtn_init(void) > > > +{ > > > + return acpi_bus_register_driver(&rbtn_driver); > > > +} > > > + > > > +static void __exit rbtn_exit(void) > > > +{ > > > + acpi_bus_unregister_driver(&rbtn_driver); > > > +} > > > + > > > +module_init(rbtn_init); > > > +module_exit(rbtn_exit); > > > > module_acpi_driver()? > > > > No, see PATCH 2/3.
Yeah, I noticed it after I wrote the comment.
| |