Re: [PATCH] staging: greybuis: fix line over 80 characters style warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Nov 27, 2016 at 3:43 AM, Andrea Ghittino <aghittino@xxxxxxxxx> wrote:
> Fixes greybus "line over 80 characters" style warnings
> found by checkpatch.pl tool
>

It is always trade-off between readability vs 80 character restriction.
I give preference and precedence to readability, so I kept it as is
during development.

I have marked the changes with "+1", wherever I feel they should be
accepted, and others probably not needed due to readability issue.


> Signed-off-by: Andrea Ghittino <aghittino at gmail.com>
> ---
>
> diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
> index 3fda0cd..755120a 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
> +        */

+1

>         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..594fe01 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -291,15 +291,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>                 if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
>                         if (time_before(jiffies,
>                                         arche_pdata->wake_detect_start +
> -                                       msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
> -                               arche_platform_set_wake_detect_state(arche_pdata,
> -                                                                    WD_STATE_IDLE);
> +                                       msecs_to_jiffies(
> +                                               WD_COLDBOOT_PULSE_WIDTH_MS))) {
> +                               arche_platform_set_wake_detect_state(
> +                                                               arche_pdata,
> +                                                               WD_STATE_IDLE);

This I feel that, lets keep original code, as it impacts the readability.

>                         } else {
> -                               /* Check we are not in middle of irq thread already */
> +                               /*
> +                                * Check we are not in middle
> +                                * of irq thread already
> +                                */

+1

>                                 if (arche_pdata->wake_detect_state !=
>                                                 WD_STATE_COLDBOOT_START) {
> -                                       arche_platform_set_wake_detect_state(arche_pdata,
> -                                                                            WD_STATE_COLDBOOT_TRIG);
> +                                       arche_platform_set_wake_detect_state(
> +                                                                  arche_pdata,
> +                                                      WD_STATE_COLDBOOT_TRIG);

Probably keep the original code here as well.

>                                         spin_unlock_irqrestore(
>                                                 &arche_pdata->wake_lock,
>                                                 flags);
> @@ -312,12 +318,14 @@ 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);
> +                                                           WD_STATE_BOOT_INIT);


+1

>                 }
>         }
>
> @@ -330,7 +338,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 +373,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 +408,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 +572,18 @@ 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);

Align it to open parenthesis.
+1

>         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 +625,9 @@ 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 +651,17 @@ 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 +679,10 @@ 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);
>         if (ret) {
>                 dev_err(dev, "failed to request wake detect IRQ %d\n", ret);
>                 return ret;
> @@ -780,12 +801,18 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
>                         arche_platform_resume);
>
>  static const struct of_device_id arche_platform_of_match[] = {
> -       { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
> +       {
> +               /* Use PID/VID of SVC device */
> +               .compatible = "google,arche-platform",
> +       },

+1

>         { },
>  };
>
>  static const struct of_device_id arche_combined_id[] = {
> -       { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC device */
> +       {
> +               /* Use PID/VID of SVC device */
> +               .compatible = "google,arche-platform",
> +       },
>         { .compatible = "usbffff,2", },

+1

>         { },
>  };
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index f8862c6..e8010c8 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -831,7 +831,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
> +                */
+1

>                 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..d26d122 100644
> --- a/drivers/staging/greybus/bootrom.c
> +++ b/drivers/staging/greybus/bootrom.c
> @@ -53,7 +53,9 @@ 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);
>         struct device *dev = &bootrom->connection->bundle->dev;
>         const char *reason;
>
> @@ -187,7 +189,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;
>         struct gb_bootrom_firmware_size_response *size_response;
>         struct device *dev = &op->connection->bundle->dev;
>         int ret;
> @@ -220,7 +223,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,7 +291,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,
> +       dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n",
> +               offset,
>                 size);
>
>  unlock:
> _______________________________________________
> devel mailing list
> devel@xxxxxxxxxxxxxxxxxxxxxx
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux