| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Feb 2017 11:41:25 +0100
From: Johan Hovold <johan@...nel.org>
To: Gioh Kim <gi-oh.kim@...fitbricks.com>
Cc: johan@...nel.org, gregkh@...uxfoundation.org, elder@...nel.org,
greybus-dev@...ts.linaro.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] staging: greybus: fix "line over 80 characters"
coding style issues
On Thu, Feb 09, 2017 at 05:30:11PM +0100, Gioh Kim wrote:
> This patch fixes only obvious lines.
> There are still more issues.
Please make this commit message self-contained (e.g. mention what you
are doing here and why).
> Signed-off-by: Gioh Kim <gi-oh.kim@...fitbricks.com>
> ---
> drivers/staging/greybus/arche-apb-ctrl.c | 5 +++-
> drivers/staging/greybus/arche-platform.c | 43 +++++++++++++++++++++-----------
> drivers/staging/greybus/audio_codec.c | 5 +++-
> drivers/staging/greybus/bootrom.c | 13 ++++++----
> drivers/staging/greybus/es2.c | 3 ++-
> drivers/staging/greybus/fw-download.c | 6 +++--
> drivers/staging/greybus/gbphy.c | 3 ++-
> drivers/staging/greybus/gpio.c | 3 ++-
> drivers/staging/greybus/svc.c | 26 +++++++++++--------
> drivers/staging/greybus/uart.c | 8 +++---
> drivers/staging/greybus/vibrator.c | 4 ++-
> 11 files changed, 77 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
> index 17fa290..02243b4 100644
> --- a/drivers/staging/greybus/arche-apb-ctrl.c
> +++ b/drivers/staging/greybus/arche-apb-ctrl.c
> @@ -168,7 +168,10 @@ static int standby_boot_seq(struct platform_device *pdev)
> if (apb->init_disabled)
> return 0;
>
> - /* Even if it is in OFF state, then we do not want to change the state */
> + /*
> + * Even if it is in OFF state,
> + * then we do not want to change the state
> + */
Odd comment, but still don't break the line after the comma, break
around 80 cols.
But in this case I think the comment should instead be shortened to:
/* Even if in OFF state we do not want to change the state. */
> if (apb->state == ARCHE_PLATFORM_STATE_STANDBY ||
> apb->state == ARCHE_PLATFORM_STATE_OFF)
> return 0;
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index 338c2d3..aac1145 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -312,9 +312,11 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
> if (arche_pdata->wake_detect_state == WD_STATE_IDLE) {
> arche_pdata->wake_detect_start = jiffies;
> /*
> - * In the begining, when wake/detect goes low (first time), we assume
> - * it is meant for coldboot and set the flag. If wake/detect line stays low
> - * beyond 30msec, then it is coldboot else fallback to standby boot.
> + * In the begining, when wake/detect goes low
> + * (first time), we assume it is meant for coldboot
> + * and set the flag. If wake/detect line stays low
> + * beyond 30msec, then it is coldboot else fallback
> + * to standby boot.
> */
> arche_platform_set_wake_detect_state(arche_pdata,
> WD_STATE_BOOT_INIT);
> @@ -330,7 +332,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
> /*
> * Requires arche_pdata->platform_state_mutex to be held
> */
> -static int arche_platform_coldboot_seq(struct arche_platform_drvdata *arche_pdata)
> +static int
> +arche_platform_coldboot_seq(struct arche_platform_drvdata *arche_pdata)
> {
> int ret;
>
> @@ -364,7 +367,8 @@ static int arche_platform_coldboot_seq(struct arche_platform_drvdata *arche_pdat
> /*
> * Requires arche_pdata->platform_state_mutex to be held
> */
> -static int arche_platform_fw_flashing_seq(struct arche_platform_drvdata *arche_pdata)
> +static int
> +arche_platform_fw_flashing_seq(struct arche_platform_drvdata *arche_pdata)
> {
> int ret;
>
> @@ -398,7 +402,8 @@ static int arche_platform_fw_flashing_seq(struct arche_platform_drvdata *arche_p
> /*
> * Requires arche_pdata->platform_state_mutex to be held
> */
> -static void arche_platform_poweroff_seq(struct arche_platform_drvdata *arche_pdata)
> +static void
> +arche_platform_poweroff_seq(struct arche_platform_drvdata *arche_pdata)
> {
> unsigned long flags;
>
> @@ -561,14 +566,17 @@ static int arche_platform_probe(struct platform_device *pdev)
> struct device_node *np = dev->of_node;
> int ret;
>
> - arche_pdata = devm_kzalloc(&pdev->dev, sizeof(*arche_pdata), GFP_KERNEL);
> + arche_pdata = devm_kzalloc(&pdev->dev, sizeof(*arche_pdata),
> + GFP_KERNEL);
> if (!arche_pdata)
> return -ENOMEM;
>
> /* setup svc reset gpio */
> arche_pdata->is_reset_act_hi = of_property_read_bool(np,
> "svc,reset-active-high");
> - arche_pdata->svc_reset_gpio = of_get_named_gpio(np, "svc,reset-gpio", 0);
> + arche_pdata->svc_reset_gpio = of_get_named_gpio(np,
> + "svc,reset-gpio",
> + 0);
> if (arche_pdata->svc_reset_gpio < 0) {
> dev_err(dev, "failed to get reset-gpio\n");
> return arche_pdata->svc_reset_gpio;
> @@ -610,7 +618,8 @@ static int arche_platform_probe(struct platform_device *pdev)
> dev_err(dev, "failed to get svc clock-req gpio\n");
> return arche_pdata->svc_refclk_req;
> }
> - ret = devm_gpio_request(dev, arche_pdata->svc_refclk_req, "svc-clk-req");
> + ret = devm_gpio_request(dev, arche_pdata->svc_refclk_req,
> + "svc-clk-req");
> if (ret) {
> dev_err(dev, "failed to request svc-clk-req gpio: %d\n", ret);
> return ret;
> @@ -634,13 +643,16 @@ static int arche_platform_probe(struct platform_device *pdev)
> arche_pdata->num_apbs = of_get_child_count(np);
> dev_dbg(dev, "Number of APB's available - %d\n", arche_pdata->num_apbs);
>
> - arche_pdata->wake_detect_gpio = of_get_named_gpio(np, "svc,wake-detect-gpio", 0);
> + arche_pdata->wake_detect_gpio = of_get_named_gpio(np,
> + "svc,wake-detect-gpio",
> + 0);
> if (arche_pdata->wake_detect_gpio < 0) {
> dev_err(dev, "failed to get wake detect gpio\n");
> return arche_pdata->wake_detect_gpio;
> }
>
> - ret = devm_gpio_request(dev, arche_pdata->wake_detect_gpio, "wake detect");
> + ret = devm_gpio_request(dev, arche_pdata->wake_detect_gpio,
> + "wake detect");
> if (ret) {
> dev_err(dev, "Failed requesting wake_detect gpio %d\n",
> arche_pdata->wake_detect_gpio);
> @@ -658,10 +670,11 @@ static int arche_platform_probe(struct platform_device *pdev)
> gpio_to_irq(arche_pdata->wake_detect_gpio);
>
> ret = devm_request_threaded_irq(dev, arche_pdata->wake_detect_irq,
> - arche_platform_wd_irq,
> - arche_platform_wd_irq_thread,
> - IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> - dev_name(dev), arche_pdata);
> + arche_platform_wd_irq,
> + arche_platform_wd_irq_thread,
> + IRQF_TRIGGER_FALLING |
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + dev_name(dev), arche_pdata);
Just leave this unchanged for now.
> if (ret) {
> dev_err(dev, "failed to request wake detect IRQ %d\n", ret);
> return ret;
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index 30941f9..25c8bb4 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -838,7 +838,10 @@ int gbaudio_register_module(struct gbaudio_module_info *module)
> snd_soc_dapm_link_component_dai_widgets(codec->card,
> &codec->dapm);
> #ifdef CONFIG_SND_JACK
> - /* register jack devices for this module from codec->jack_list */
> + /*
> + * register jack devices for this module
> + * from codec->jack_list
> + */
> list_for_each_entry(jack, &codec->jack_list, list) {
> if ((jack == &module->headset_jack)
> || (jack == &module->button_jack))
> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
> index 5f90721..06df0ce 100644
> --- a/drivers/staging/greybus/bootrom.c
> +++ b/drivers/staging/greybus/bootrom.c
> @@ -53,7 +53,8 @@ static void free_firmware(struct gb_bootrom *bootrom)
> static void gb_bootrom_timedout(struct work_struct *work)
> {
> struct delayed_work *dwork = to_delayed_work(work);
> - struct gb_bootrom *bootrom = container_of(dwork, struct gb_bootrom, dwork);
> + struct gb_bootrom *bootrom = container_of(dwork,
> + struct gb_bootrom, dwork);
Leave this unchanged as well.
> struct device *dev = &bootrom->connection->bundle->dev;
> const char *reason;
>
> @@ -187,7 +188,8 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage)
> static int gb_bootrom_firmware_size_request(struct gb_operation *op)
> {
> struct gb_bootrom *bootrom = gb_connection_get_data(op->connection);
> - struct gb_bootrom_firmware_size_request *size_request = op->request->payload;
> + struct gb_bootrom_firmware_size_request *size_request =
> + op->request->payload;
Don't break declarations this way if it can be avoided. Leave unchanged
for now.
> struct gb_bootrom_firmware_size_response *size_response;
> struct device *dev = &op->connection->bundle->dev;
> int ret;
> @@ -220,7 +222,8 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op)
> size_response = op->response->payload;
> size_response->size = cpu_to_le32(bootrom->fw->size);
>
> - dev_dbg(dev, "%s: firmware size %d bytes\n", __func__, size_response->size);
> + dev_dbg(dev, "%s: firmware size %d bytes\n",
> + __func__, size_response->size);
>
> unlock:
> mutex_unlock(&bootrom->mutex);
> @@ -287,8 +290,8 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
> firmware_response = op->response->payload;
> memcpy(firmware_response->data, fw->data + offset, size);
>
> - dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n", offset,
> - size);
> + dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n",
> + offset, size);
>
> unlock:
> mutex_unlock(&bootrom->mutex);
> diff --git a/drivers/staging/greybus/es2.c b/drivers/staging/greybus/es2.c
> index 93afd93..0eba059 100644
> --- a/drivers/staging/greybus/es2.c
> +++ b/drivers/staging/greybus/es2.c
> @@ -1085,7 +1085,8 @@ static void apb_log_get(struct es2_ap_dev *es2, char *buf)
> retval = usb_control_msg(es2->usb_dev,
> usb_rcvctrlpipe(es2->usb_dev, 0),
> GB_APB_REQUEST_LOG,
> - USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
> + USB_DIR_IN | USB_TYPE_VENDOR |
> + USB_RECIP_INTERFACE,
Leave unchanged.
> 0x00, 0x00,
> buf,
> APB1_LOG_MSG_SIZE,
> diff --git a/drivers/staging/greybus/fw-download.c b/drivers/staging/greybus/fw-download.c
> index 2d72468..8a1a413 100644
> --- a/drivers/staging/greybus/fw-download.c
> +++ b/drivers/staging/greybus/fw-download.c
> @@ -130,7 +130,8 @@ static void free_firmware(struct fw_download *fw_download,
> static void fw_request_timedout(struct work_struct *work)
> {
> struct delayed_work *dwork = to_delayed_work(work);
> - struct fw_request *fw_req = container_of(dwork, struct fw_request, dwork);
> + struct fw_request *fw_req = container_of(dwork,
> + struct fw_request, dwork);
Drop this too.
> struct fw_download *fw_download = fw_req->fw_download;
>
> dev_err(fw_download->parent,
> @@ -239,7 +240,8 @@ static int fw_download_find_firmware(struct gb_operation *op)
> tag = (const char *)request->firmware_tag;
>
> /* firmware_tag must be null-terminated */
> - if (strnlen(tag, GB_FIRMWARE_TAG_MAX_SIZE) == GB_FIRMWARE_TAG_MAX_SIZE) {
> + if (strnlen(tag, GB_FIRMWARE_TAG_MAX_SIZE) ==
> + GB_FIRMWARE_TAG_MAX_SIZE) {
Drop.
> dev_err(fw_download->parent,
> "firmware-tag is not null-terminated\n");
> return -EINVAL;
> diff --git a/drivers/staging/greybus/gbphy.c b/drivers/staging/greybus/gbphy.c
> index bcde7c9..64a1eb9 100644
> --- a/drivers/staging/greybus/gbphy.c
> +++ b/drivers/staging/greybus/gbphy.c
> @@ -104,7 +104,8 @@ static int gbphy_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
> }
>
> static const struct gbphy_device_id *
> -gbphy_dev_match_id(struct gbphy_device *gbphy_dev, struct gbphy_driver *gbphy_drv)
> +gbphy_dev_match_id(struct gbphy_device *gbphy_dev,
> + struct gbphy_driver *gbphy_drv)
> {
> const struct gbphy_device_id *id = gbphy_drv->id_table;
>
> diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c
> index 558550c..0eabfe1 100644
> --- a/drivers/staging/greybus/gpio.c
> +++ b/drivers/staging/greybus/gpio.c
> @@ -557,7 +557,8 @@ static void gb_gpio_irqchip_remove(struct gb_gpio_controller *ggc)
> /* Remove all IRQ mappings and delete the domain */
> if (ggc->irqdomain) {
> for (offset = 0; offset < (ggc->line_max + 1); offset++)
> - irq_dispose_mapping(irq_find_mapping(ggc->irqdomain, offset));
> + irq_dispose_mapping(irq_find_mapping(ggc->irqdomain,
> + offset));
> irq_domain_remove(ggc->irqdomain);
> }
>
> diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c
> index be151a6..5549db2 100644
> --- a/drivers/staging/greybus/svc.c
> +++ b/drivers/staging/greybus/svc.c
> @@ -678,7 +678,8 @@ static int gb_svc_version_request(struct gb_operation *op)
> static ssize_t pwr_debugfs_voltage_read(struct file *file, char __user *buf,
> size_t len, loff_t *offset)
> {
> - struct svc_debugfs_pwrmon_rail *pwrmon_rails = file_inode(file)->i_private;
> + struct svc_debugfs_pwrmon_rail *pwrmon_rails =
> + file_inode(file)->i_private;
Drop.
> struct gb_svc *svc = pwrmon_rails->svc;
> int ret, desc;
> u32 value;
> @@ -701,7 +702,8 @@ static ssize_t pwr_debugfs_voltage_read(struct file *file, char __user *buf,
> static ssize_t pwr_debugfs_current_read(struct file *file, char __user *buf,
> size_t len, loff_t *offset)
> {
> - struct svc_debugfs_pwrmon_rail *pwrmon_rails = file_inode(file)->i_private;
> + struct svc_debugfs_pwrmon_rail *pwrmon_rails =
> + file_inode(file)->i_private;
Drop.
> struct gb_svc *svc = pwrmon_rails->svc;
> int ret, desc;
> u32 value;
> @@ -724,7 +726,8 @@ static ssize_t pwr_debugfs_current_read(struct file *file, char __user *buf,
> static ssize_t pwr_debugfs_power_read(struct file *file, char __user *buf,
> size_t len, loff_t *offset)
> {
> - struct svc_debugfs_pwrmon_rail *pwrmon_rails = file_inode(file)->i_private;
> + struct svc_debugfs_pwrmon_rail *pwrmon_rails =
> + file_inode(file)->i_private;
Drop.
> struct gb_svc *svc = pwrmon_rails->svc;
> int ret, desc;
> u32 value;
> @@ -924,14 +927,15 @@ static void gb_svc_process_hello_deferred(struct gb_operation *operation)
> * Power Mode Changes is resolved.
> */
> ret = gb_svc_intf_set_power_mode(svc, svc->ap_intf_id,
> - GB_SVC_UNIPRO_HS_SERIES_A,
> - GB_SVC_UNIPRO_SLOW_AUTO_MODE,
> - 2, 1,
> - GB_SVC_SMALL_AMPLITUDE, GB_SVC_NO_DE_EMPHASIS,
> - GB_SVC_UNIPRO_SLOW_AUTO_MODE,
> - 2, 1,
> - 0, 0,
> - NULL, NULL);
> + GB_SVC_UNIPRO_HS_SERIES_A,
> + GB_SVC_UNIPRO_SLOW_AUTO_MODE,
> + 2, 1,
> + GB_SVC_SMALL_AMPLITUDE,
> + GB_SVC_NO_DE_EMPHASIS,
> + GB_SVC_UNIPRO_SLOW_AUTO_MODE,
> + 2, 1,
> + 0, 0,
> + NULL, NULL);
>
> if (ret)
> dev_warn(&svc->dev,
> diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
> index 6d39f4a..6177b9c 100644
> --- a/drivers/staging/greybus/uart.c
> +++ b/drivers/staging/greybus/uart.c
> @@ -630,8 +630,9 @@ static int get_serial_info(struct gb_tty *gb_tty,
> tmp.xmit_fifo_size = 16;
> tmp.baud_base = 9600;
> tmp.close_delay = gb_tty->port.close_delay / 10;
> - tmp.closing_wait = gb_tty->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
> - ASYNC_CLOSING_WAIT_NONE : gb_tty->port.closing_wait / 10;
> + tmp.closing_wait =
> + gb_tty->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
> + ASYNC_CLOSING_WAIT_NONE : gb_tty->port.closing_wait / 10;
Leave as is for now, this should be rewritten without the ternary
operator.
> if (copy_to_user(info, &tmp, sizeof(tmp)))
> return -EFAULT;
> @@ -1000,7 +1001,8 @@ static int gb_tty_init(void)
> gb_tty_driver->subtype = SERIAL_TYPE_NORMAL;
> gb_tty_driver->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
> gb_tty_driver->init_termios = tty_std_termios;
> - gb_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
> + gb_tty_driver->init_termios.c_cflag = B9600 | CS8 |
> + CREAD | HUPCL | CLOCAL;
Leave as is.
> tty_set_operations(gb_tty_driver, &gb_ops);
>
> retval = tty_register_driver(gb_tty_driver);
> diff --git a/drivers/staging/greybus/vibrator.c b/drivers/staging/greybus/vibrator.c
> index 4ba0e16..77a2365 100644
> --- a/drivers/staging/greybus/vibrator.c
> +++ b/drivers/staging/greybus/vibrator.c
> @@ -70,7 +70,9 @@ static void gb_vibrator_worker(struct work_struct *work)
> {
> struct delayed_work *delayed_work = to_delayed_work(work);
> struct gb_vibrator_device *vib =
> - container_of(delayed_work, struct gb_vibrator_device, delayed_work);
> + container_of(delayed_work,
> + struct gb_vibrator_device,
> + delayed_work);
Split declaration and initialisation here instead.
>
> turn_off(vib);
> }
Johan
Powered by blists - more mailing lists