| Message ID | 1644994755-12975-2-git-send-email-quic_linyyuan@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: xhci: fix of some code/comment | expand |
Hello! On 2/16/22 9:59 AM, Linyu Yuan wrote: > The for loop to find page size bit can be replaced with ffs(). > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > drivers/usb/host/xhci-mem.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 0e31206..3cbc7f2 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -2395,12 +2395,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > page_size = readl(&xhci->op_regs->page_size); > xhci_dbg_trace(xhci, trace_xhci_dbg_init, > "Supported page size register = 0x%x", page_size); > - for (i = 0; i < 16; i++) { > - if ((0x1 & page_size) != 0) > - break; > - page_size = page_size >> 1; > - } > - if (i < 16) > + if ((i = ffs(page_size)) < 16) Always run your patches thru scripts/checkpatch.pl -- in this case it will complain of an assignment in the *if* expression... [...] MNR, Sergey
Thanks, that's correct, from my view, one line is good, What's your suggestion ? two lines ? -----Original Message----- From: Sergei Shtylyov <sergei.shtylyov@gmail.com> Sent: Wednesday, February 16, 2022 5:42 PM To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com>; Mathias Nyman <mathias.nyman@intel.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-usb@vger.kernel.org; Jack Pham (QUIC) <quic_jackp@quicinc.com> Subject: Re: [PATCH 1/5] usb: host: xhci: use ffs() in xhci_mem_init() Hello! On 2/16/22 9:59 AM, Linyu Yuan wrote: > The for loop to find page size bit can be replaced with ffs(). > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > drivers/usb/host/xhci-mem.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 0e31206..3cbc7f2 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -2395,12 +2395,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) > page_size = readl(&xhci->op_regs->page_size); > xhci_dbg_trace(xhci, trace_xhci_dbg_init, > "Supported page size register = 0x%x", page_size); > - for (i = 0; i < 16; i++) { > - if ((0x1 & page_size) != 0) > - break; > - page_size = page_size >> 1; > - } > - if (i < 16) > + if ((i = ffs(page_size)) < 16) Always run your patches thru scripts/checkpatch.pl -- in this case it will complain of an assignment in the *if* expression... [...] MNR, Sergey
On 2/16/22 12:47 PM, Linyu Yuan (QUIC) wrote: > that's correct, from my view, one line is good, > > What's your suggestion ? two lines ? Yes, and it is not just my suggestion -- it's the kernel coding style. [...] MBR, Sergey
> From: Sergei Shtylyov <sergei.shtylyov@gmail.com> > Sent: Wednesday, February 16, 2022 5:53 PM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com>; Mathias Nyman > <mathias.nyman@intel.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org> > Cc: linux-usb@vger.kernel.org; Jack Pham (QUIC) <quic_jackp@quicinc.com> > Subject: Re: [PATCH 1/5] usb: host: xhci: use ffs() in xhci_mem_init() > > On 2/16/22 12:47 PM, Linyu Yuan (QUIC) wrote: > > > that's correct, from my view, one line is good, > > > > What's your suggestion ? two lines ? > > Yes, and it is not just my suggestion -- it's the kernel coding style. I think in linux, there are many code like this, arch/m68k/sun3x/dvma.c: if((pmd = pmd_alloc(&init_mm, pud, vaddr)) == NULL) { ... Two lines here is not good. > > [...] > > MBR, Sergey
On 2/16/22 12:57 PM, Linyu Yuan (QUIC) wrote: [...] >>> that's correct, from my view, one line is good, >>> >>> What's your suggestion ? two lines ? >> >> Yes, and it is not just my suggestion -- it's the kernel coding style. > I think in linux, there are many code like this, > > arch/m68k/sun3x/dvma.c: if((pmd = pmd_alloc(&init_mm, pud, vaddr)) == NULL) { > ... > > Two lines here is not good. Why?! 8-) >> [...] MBR, Sergey
On Wed, Feb 16, 2022 at 09:57:04AM +0000, Linyu Yuan (QUIC) wrote: > > From: Sergei Shtylyov <sergei.shtylyov@gmail.com> > > Sent: Wednesday, February 16, 2022 5:53 PM > > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com>; Mathias Nyman > > <mathias.nyman@intel.com>; Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> > > Cc: linux-usb@vger.kernel.org; Jack Pham (QUIC) <quic_jackp@quicinc.com> > > Subject: Re: [PATCH 1/5] usb: host: xhci: use ffs() in xhci_mem_init() > > > > On 2/16/22 12:47 PM, Linyu Yuan (QUIC) wrote: > > > > > that's correct, from my view, one line is good, > > > > > > What's your suggestion ? two lines ? > > > > Yes, and it is not just my suggestion -- it's the kernel coding style. > I think in linux, there are many code like this, > > arch/m68k/sun3x/dvma.c: if((pmd = pmd_alloc(&init_mm, pud, vaddr)) == NULL) { > ... That file pre-dates the use of the kernel coding style. Please look at modern code (i.e. written in the last 15 years.) thanks, greg k-h
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 0e31206..3cbc7f2 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2395,12 +2395,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) page_size = readl(&xhci->op_regs->page_size); xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Supported page size register = 0x%x", page_size); - for (i = 0; i < 16; i++) { - if ((0x1 & page_size) != 0) - break; - page_size = page_size >> 1; - } - if (i < 16) + if ((i = ffs(page_size)) < 16) xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Supported page size of %iK", (1 << (i+12)) / 1024); else
The for loop to find page size bit can be replaced with ffs(). Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- drivers/usb/host/xhci-mem.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)