Messages in this thread | | | From | Pali Rohár <> | Subject | Re: [PATCH 1/3] platform: x86: dell-rbtn: Dell Airplane Mode Switch driver | Date | Fri, 28 Nov 2014 12:45:55 +0100 |
| |
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.
> > +} > > + > > +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.
-- Pali Rohár pali.rohar@gmail.com [unhandled content-type:application/pgp-signature] | |