Re: [Outreachy kernel] [RESEND PATCH] staging: emxx_udc: Ending line with argument
From: Matthew Wilcox
Date: Tue Apr 06 2021 - 15:56:47 EST
On Tue, Apr 06, 2021 at 08:34:09PM +0100, Beatriz Martins de Carvalho wrote:
> Cleans up check of "Lines should not end with a '('"
> with argument present in next line in file emxx_udc.c
I appreciate that you've removed the checkpatch warning, but this is
still harder to read than the original used to be.
> - _nbu2ss_writel(
> - &preg->EP_REGS[ep->epnum - 1].EP_WRITE,
> - p_buf_32->dw);
> + _nbu2ss_writel(&preg->EP_REGS[ep->epnum - 1].EP_WRITE,
> + p_buf_32->dw);
> - length = _nbu2ss_readl(
> - &ep->udc->p_regs->EP_REGS[ep->epnum - 1].EP_LEN_DCNT);
> + length = _nbu2ss_readl(&ep->udc->p_regs->EP_REGS[ep->epnum - 1].EP_LEN_DCNT);
> - regdata = _nbu2ss_readl(
> - &preg->EP_REGS[ep->epnum - 1].EP_STATUS);
> + regdata = _nbu2ss_readl(&preg->EP_REGS[ep->epnum - 1].EP_STATUS);
The real problem with this driver is that their abstraction layer is
wrong. For example:
/* Interrupt Status */
status = _nbu2ss_readl(&udc->p_regs->EP_REGS[num].EP_STATUS);
/* Interrupt Clear */
_nbu2ss_writel(&udc->p_regs->EP_REGS[num].EP_STATUS, ~status);
If instead this were:
status = nbu2ss_read_ep_status(udc, num);
nbu2ss_write_ep_status(udc, num, ~status);
that would be a lot shorter and clearer. Cleanups along these lines
would be a lot more useful, and would fix the 80 column warning.