Re: [PATCH] pci: hotplug: ibmphp: Fix 'line over 80 characters' Warning

[Andy Shevchenko]

On Sun, Jun 2, 2019 at 7:25 PM Emanuel Bennici
<benniciemanuel78@xxxxxxxxx> wrote:
>
> Fix checkpatch.pl 'line over 80 characters' Warning in ibmphp_ebda.c and
> ibmphp_hpc.c

I'm not sure it's needed.
And actually some of the changes are the regressions in order of
readability ratio.

> +                               debug("dev->device = %x, "
> +                                     "dev->subsystem_device = %x\n",

Don't split string literals like this.

> -       if ((pslot == NULL)
> -           || ((pstatus == NULL) && (cmd != READ_ALLSTAT) && (cmd != READ_BUSSTATUS))) {
> +       if ((pslot == NULL) || ((pstatus == NULL) && (cmd != READ_ALLSTAT) &&
> +                               (cmd != READ_BUSSTATUS))) {

No, no, this breaks readability a lot.
In the former it's clearly shown the logic between conditionals, while
in the latter it's a mess.

> +                                       err("%s - Error ctrl_read failed\n",
> +                                               __func__);

Really, no advantage on this line even for 80 characters.

> +                       err("%s - Exit Error:invalid bus, rc[%d]\n",
> +                               __func__, rc);

Same here.

> -               debug("%s - ctlr id[%x] physical[%lx] logical[%lx] i2c[%x]\n", __func__,
> -               ctlr_ptr->ctlr_id, (ulong) (ctlr_ptr->u.wpeg_ctlr.wpegbbar), (ulong) wpg_bbar,
> -               ctlr_ptr->u.wpeg_ctlr.i2c_addr);
> +               debug("%s - ctlr id[%x] physical[%lx] logical[%lx] i2c[%x]\n",
> +                       __func__, ctlr_ptr->ctlr_id,
> +                       (ulong) (ctlr_ptr->u.wpeg_ctlr.wpegbbar),
> +                       (ulong) wpg_bbar, ctlr_ptr->u.wpeg_ctlr.i2c_addr);

Here better to fix explicit casts (perhaps it means wrong specifiers
being used).

> +               if ((poldslot->status & 0x20) &&
> +                   (SLOT_CONNECT(poldslot->status) == HPC_SLOT_CONNECTED)
> +                   && (SLOT_PRESENT(poldslot->status)))

Here you are not consistent in a way where to put logical operation.

> +                               memcpy((void *) &myslot, (void *) pslot,
> +                                      sizeof(struct slot));

Why spaces after ) here?

> +                               debug("%s - call process_changeinstatus for"
> +                                     "slot[%d]\n", __func__, i);

Do not split string literals like this.

-- 
With Best Regards,
Andy Shevchenko