From: Beatriz Martins de Carvalho <martinsdecarvalhobeatriz@gmail.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
outreachy-kernel@googlegroups.com, gregkh@linuxfoundation.org
Subject: Re: [Outreachy kernel] [RESEND PATCH] staging: emxx_udc: Ending line with argument
Date: Tue, 6 Apr 2021 21:12:33 +0100 [thread overview]
Message-ID: <07ae6975-d4a0-dfec-8030-7ddbd90eedd5@gmail.com> (raw)
In-Reply-To: <20210406195621.GU2531743@casper.infradead.org>
Em 06/04/21 20:56, Matthew Wilcox escreveu:
> 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.
Thank you for your review.
>> - _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.
I can see what you mean and makes sense. Thank you.
Beatriz Martins de Carvalho
prev parent reply other threads:[~2021-04-06 20:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-06 19:34 [RESEND PATCH] staging: emxx_udc: Ending line with argument Beatriz Martins de Carvalho
2021-04-06 19:36 ` Greg KH
2021-04-06 20:00 ` Beatriz Martins de Carvalho
2021-04-07 5:37 ` Greg KH
[not found] ` <4d7ebe9c-8ebc-2fc9-10ed-6756ab42d5d7@gmail.com>
2021-04-07 8:34 ` Greg KH
2021-04-07 9:46 ` Beatriz Martins de Carvalho
2021-04-07 9:54 ` Greg KH
2021-04-06 19:56 ` [Outreachy kernel] " Matthew Wilcox
2021-04-06 20:12 ` Beatriz Martins de Carvalho [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=07ae6975-d4a0-dfec-8030-7ddbd90eedd5@gmail.com \
--to=martinsdecarvalhobeatriz@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy-kernel@googlegroups.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).