Re: [PATCH v2 3/5] toshiba_acpi: Cleanup blank lines after comment blocks

From: Darren Hart
Date: Tue May 05 2015 - 17:24:24 EST

On Mon, May 04, 2015 at 11:15:56AM -0600, Azael Avalos wrote:
> This patch removes some blank lines after comment blocks, capitalizes
> some comments first words and adds a few comments to the beggining
> of some functions.
>
> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
> ---
> drivers/platform/x86/toshiba_acpi.c | 31 +++++++++++++++++--------------
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 19efb05..dca3dd2 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -78,10 +78,9 @@ MODULE_LICENSE("GPL");
> * SCI stands for "System Configuration Interface" which aim is to
> * conceal differences in hardware between different models.
> */
> -
> #define TCI_WORDS 6
>
> -/* operations */
> +/* Operations */

I don't object - but just for the record. I care about Sentence formatting when
using sentences and multiline comments, not so much for single word or one line
phrases.

Be consistent within the file, that matters most.

> #define HCI_SET 0xff00
> #define HCI_GET 0xfe00
> #define SCI_OPEN 0xf100
> @@ -89,7 +88,7 @@ MODULE_LICENSE("GPL");
> #define SCI_GET 0xf300
> #define SCI_SET 0xf400
>
> -/* return codes */
> +/* Return codes */
> #define TOS_SUCCESS 0x0000
> #define TOS_OPEN_CLOSE_OK 0x0044
> #define TOS_FAILURE 0x1000
> @@ -104,7 +103,7 @@ MODULE_LICENSE("GPL");
> #define TOS_NOT_INITIALIZED 0x8d50
> #define TOS_NOT_INSTALLED 0x8e00
>
> -/* registers */
> +/* Registers */
> #define HCI_FAN 0x0004
> #define HCI_TR_BACKLIGHT 0x0005
> #define HCI_SYSTEM_EVENT 0x0016
> @@ -126,7 +125,7 @@ MODULE_LICENSE("GPL");
> #define SCI_TOUCHPAD 0x050e
> #define SCI_KBD_FUNCTION_KEYS 0x0522
>
> -/* field definitions */
> +/* Field definitions */
> #define HCI_ACCEL_MASK 0x7fff
> #define HCI_HOTKEY_DISABLE 0x0b
> #define HCI_HOTKEY_ENABLE 0x09
> @@ -272,7 +271,6 @@ static const struct dmi_system_id toshiba_vendor_backlight_dmi[] = {
> /*
> * Utility
> */
> -

I'd keep the space, just as you will keep a space between _set_bit and functions
that follow it.

> static inline void _set_bit(u32 *word, u32 mask, int value)
> {
> *word = (*word & ~mask) | (mask * value);
> @@ -281,7 +279,6 @@ static inline void _set_bit(u32 *word, u32 mask, int value)
> /*
> * ACPI interface wrappers
> */
> -
> static int write_acpi_int(const char *methodName, int val)
> {
> acpi_status status;
> @@ -331,7 +328,6 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
> * In addition to the ACPI status, the HCI system returns a result which
> * may be useful (such as "not supported").
> */
> -
> static u32 hci_write(struct toshiba_acpi_dev *dev, u32 reg, u32 in1)
> {
> u32 in[TCI_WORDS] = { HCI_SET, reg, in1, 0, 0, 0 };
> @@ -358,7 +354,6 @@ static u32 hci_read(struct toshiba_acpi_dev *dev, u32 reg, u32 *out1)
> /*
> * Common sci tasks
> */
> -
> static int sci_open(struct toshiba_acpi_dev *dev)
> {
> u32 in[TCI_WORDS] = { SCI_OPEN, 0, 0, 0, 0, 0 };
> @@ -1183,6 +1178,7 @@ static int toshiba_hotkey_event_type_get(struct toshiba_acpi_dev *dev,
> return 0;
> }
>
> +/* Transflective Backlight */
> static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, bool *enabled)
> {
> u32 hci_result;
> @@ -1204,6 +1200,7 @@ static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, bool enable)
>
> static struct proc_dir_entry *toshiba_proc_dir /*= 0*/;
>
> +/* LCD Brightness */
> static int __get_lcd_brightness(struct toshiba_acpi_dev *dev)
> {
> u32 hci_result;
> @@ -1322,6 +1319,7 @@ static const struct file_operations lcd_proc_fops = {
> .write = lcd_proc_write,
> };
>
> +/* Video-Out */
> static int get_video_status(struct toshiba_acpi_dev *dev, u32 *status)
> {
> u32 hci_result;
> @@ -1411,7 +1409,8 @@ static ssize_t video_proc_write(struct file *file, const char __user *buf,
> _set_bit(&new_video_out, HCI_VIDEO_OUT_TV, tv_out);
> /*
> * To avoid unnecessary video disruption, only write the new
> - * video setting if something changed. */
> + * video setting if something changed.
> + */
> if (new_video_out != video_out)
> ret = write_acpi_int(METHOD_VIDEO_OUT, new_video_out);
> }
> @@ -1428,6 +1427,7 @@ static const struct file_operations video_proc_fops = {
> .write = video_proc_write,
> };
>
> +/* Fan status */
> static int get_fan_status(struct toshiba_acpi_dev *dev, u32 *status)
> {
> u32 hci_result;
> @@ -1583,7 +1583,6 @@ static const struct file_operations version_proc_fops = {
> /*
> * Proc and module init
> */
> -
> #define PROC_TOSHIBA "toshiba"
>
> static void create_toshiba_proc_entries(struct toshiba_acpi_dev *dev)
> @@ -2324,9 +2323,7 @@ static void toshiba_acpi_hotkey_work(struct work_struct *work)
> pr_err("ACPI NTFY method execution failed\n");
> }
>
> -/*
> - * Returns hotkey scancode, or < 0 on failure.
> - */
> +/* Returns hotkey scancode, or < 0 on failure */
> static int toshiba_acpi_query_hotkey(struct toshiba_acpi_dev *dev)
> {
> unsigned long long value;
> @@ -2488,6 +2485,9 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
> return error;
> }
>
> +/*
> + * Backlight
> + */
> static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> {
> struct backlight_properties props;
> @@ -2552,6 +2552,9 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> return 0;
> }
>
> +/*
> + * ACPI driver functions
> + */
> static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
> {
> struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
> --
> 2.3.6
>
>

--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/