* [PATCH v2 0/3] Staging: octeon-usb fixes for coding style, SPDX and readability. @ 2018-07-26 15:41 Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier Georgios Tsotsos ` (2 more replies) 0 siblings, 3 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 15:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, James Hogan, Aaro Koskinen, Joe Perches, devel, linux-kernel Hello, Ok, this have gone too far, i sent these series so many times wrong... I am sorry its my first patch (and series) and i keep mix-up patch generation an sending to my self. Last time not everybody was cc. Here are three patches which trying to resolve TODO's list requirements number 45 about octeon-usb. There are SPDX licence additions on code, makefile and header files. Checkpatch warnings are resolved,also a notice about CVMX_WAIT_FOR_FIELD32 macro. After Joe Perches's (joe@perches.com) suggestion i broke down the cvmx_usb_poll_channel function in order to improve readability and keeping it under 80 columns. Georgios Tsotsos (3): Staging: octeon-usb: Adding SPDX license identifier Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco. Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). drivers/staging/octeon-usb/Makefile | 1 + drivers/staging/octeon-usb/octeon-hcd.c | 128 +++++++++++++++++++------------- drivers/staging/octeon-usb/octeon-hcd.h | 1 + 3 files changed, 80 insertions(+), 50 deletions(-) -- 2.16.4 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier 2018-07-26 15:41 [PATCH v2 0/3] Staging: octeon-usb fixes for coding style, SPDX and readability Georgios Tsotsos @ 2018-07-26 13:30 ` Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 2 siblings, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 13:30 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Georgios Tsotsos, linux-kernel Adding appropriate SPDX-License-Identifier (GPL-2) that were missing from code, header and make files. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/Makefile | 1 + drivers/staging/octeon-usb/octeon-hcd.c | 1 + drivers/staging/octeon-usb/octeon-hcd.h | 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/staging/octeon-usb/Makefile b/drivers/staging/octeon-usb/Makefile index 5588be395f2a..9873a0130ad5 100644 --- a/drivers/staging/octeon-usb/Makefile +++ b/drivers/staging/octeon-usb/Makefile @@ -1 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 obj-${CONFIG_OCTEON_USB} := octeon-hcd.o diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cded30f145aa..cff5e790b196 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * This file is subject to the terms and conditions of the GNU General Public * License. See the file "COPYING" in the main directory of this archive diff --git a/drivers/staging/octeon-usb/octeon-hcd.h b/drivers/staging/octeon-usb/octeon-hcd.h index 3353aefe662e..769c36cf6614 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.h +++ b/drivers/staging/octeon-usb/octeon-hcd.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Octeon HCD hardware register definitions. * -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 1/3] Staging: octeon-usb: Adding SPDX license identifier 2018-07-26 13:30 ` [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier Georgios Tsotsos @ 2018-07-26 15:41 ` Georgios Tsotsos 0 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 15:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, James Hogan, Aaro Koskinen, Joe Perches, devel, linux-kernel Adding appropriate SPDX-License-Identifier (GPL-2) that were missing from code, header and make files. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/Makefile | 1 + drivers/staging/octeon-usb/octeon-hcd.c | 1 + drivers/staging/octeon-usb/octeon-hcd.h | 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/staging/octeon-usb/Makefile b/drivers/staging/octeon-usb/Makefile index 5588be395f2a..9873a0130ad5 100644 --- a/drivers/staging/octeon-usb/Makefile +++ b/drivers/staging/octeon-usb/Makefile @@ -1 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 obj-${CONFIG_OCTEON_USB} := octeon-hcd.o diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cded30f145aa..cff5e790b196 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * This file is subject to the terms and conditions of the GNU General Public * License. See the file "COPYING" in the main directory of this archive diff --git a/drivers/staging/octeon-usb/octeon-hcd.h b/drivers/staging/octeon-usb/octeon-hcd.h index 3353aefe662e..769c36cf6614 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.h +++ b/drivers/staging/octeon-usb/octeon-hcd.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* * Octeon HCD hardware register definitions. * -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco. 2018-07-26 15:41 [PATCH v2 0/3] Staging: octeon-usb fixes for coding style, SPDX and readability Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier Georgios Tsotsos @ 2018-07-26 13:30 ` Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-27 15:15 ` Greg Kroah-Hartman 2018-07-26 13:30 ` [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 2 siblings, 2 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 13:30 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Georgios Tsotsos, linux-kernel Fixing coding style for CVMX_WAIT_FOR_FIELD32 was confusing. Also encapsulates into parentheses timeout_usec. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 44 +++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cff5e790b196..c8e0ebf1434f 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -378,27 +378,29 @@ struct octeon_hcd { }; /* This macro spins on a register waiting for it to reach a condition. */ -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ - ({int result; \ - do { \ - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ - octeon_get_clock_rate() / 1000000; \ - union _union c; \ - \ - while (1) { \ - c.u32 = cvmx_usb_read_csr32(usb, address); \ - \ - if (cond) { \ - result = 0; \ - break; \ - } else if (cvmx_get_cycle() > done) { \ - result = -1; \ - break; \ - } else \ - __delay(100); \ - } \ - } while (0); \ - result; }) +#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ +({ \ + int result; \ + do { \ + u64 done = cvmx_get_cycle() + (u64)(timeout_usec) * \ + octeon_get_clock_rate() / 1000000; \ + union _union c; \ + \ + while (1) { \ + c.u32 = cvmx_usb_read_csr32(usb, address); \ + \ + if (cond) { \ + result = 0; \ + break; \ + } else if (cvmx_get_cycle() > done) { \ + result = -1; \ + break; \ + } else \ + __delay(100); \ + } \ + } while (0); \ + result; \ +}) /* * This macro logically sets a single field in a CSR. It does the sequence -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco. 2018-07-26 13:30 ` [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco Georgios Tsotsos @ 2018-07-26 15:41 ` Georgios Tsotsos 2018-07-27 15:15 ` Greg Kroah-Hartman 1 sibling, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 15:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, James Hogan, Aaro Koskinen, Joe Perches, devel, linux-kernel Fixing coding style for CVMX_WAIT_FOR_FIELD32 was confusing. Also encapsulates into parentheses timeout_usec. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 44 +++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cff5e790b196..c8e0ebf1434f 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -378,27 +378,29 @@ struct octeon_hcd { }; /* This macro spins on a register waiting for it to reach a condition. */ -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ - ({int result; \ - do { \ - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ - octeon_get_clock_rate() / 1000000; \ - union _union c; \ - \ - while (1) { \ - c.u32 = cvmx_usb_read_csr32(usb, address); \ - \ - if (cond) { \ - result = 0; \ - break; \ - } else if (cvmx_get_cycle() > done) { \ - result = -1; \ - break; \ - } else \ - __delay(100); \ - } \ - } while (0); \ - result; }) +#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ +({ \ + int result; \ + do { \ + u64 done = cvmx_get_cycle() + (u64)(timeout_usec) * \ + octeon_get_clock_rate() / 1000000; \ + union _union c; \ + \ + while (1) { \ + c.u32 = cvmx_usb_read_csr32(usb, address); \ + \ + if (cond) { \ + result = 0; \ + break; \ + } else if (cvmx_get_cycle() > done) { \ + result = -1; \ + break; \ + } else \ + __delay(100); \ + } \ + } while (0); \ + result; \ +}) /* * This macro logically sets a single field in a CSR. It does the sequence -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco. 2018-07-26 13:30 ` [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos @ 2018-07-27 15:15 ` Greg Kroah-Hartman 2018-07-28 15:48 ` Georgios Tsotsos ` (4 more replies) 1 sibling, 5 replies; 28+ messages in thread From: Greg Kroah-Hartman @ 2018-07-27 15:15 UTC (permalink / raw) To: Georgios Tsotsos Cc: devel, Aaro Koskinen, James Hogan, linux-kernel, Joe Perches On Thu, Jul 26, 2018 at 06:41:52PM +0300, Georgios Tsotsos wrote: > Fixing coding style for CVMX_WAIT_FOR_FIELD32 was confusing. Also > encapsulates into parentheses timeout_usec. > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > --- > drivers/staging/octeon-usb/octeon-hcd.c | 44 +++++++++++++++++---------------- > 1 file changed, 23 insertions(+), 21 deletions(-) > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > index cff5e790b196..c8e0ebf1434f 100644 > --- a/drivers/staging/octeon-usb/octeon-hcd.c > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > @@ -378,27 +378,29 @@ struct octeon_hcd { > }; > > /* This macro spins on a register waiting for it to reach a condition. */ > -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ > - ({int result; \ > - do { \ > - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ > - octeon_get_clock_rate() / 1000000; \ > - union _union c; \ > - \ > - while (1) { \ > - c.u32 = cvmx_usb_read_csr32(usb, address); \ > - \ > - if (cond) { \ > - result = 0; \ > - break; \ > - } else if (cvmx_get_cycle() > done) { \ > - result = -1; \ > - break; \ > - } else \ > - __delay(100); \ > - } \ > - } while (0); \ > - result; }) > +#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ > +({ \ > + int result; \ > + do { \ > + u64 done = cvmx_get_cycle() + (u64)(timeout_usec) * \ > + octeon_get_clock_rate() / 1000000; \ > + union _union c; \ > + \ > + while (1) { \ > + c.u32 = cvmx_usb_read_csr32(usb, address); \ > + \ > + if (cond) { \ > + result = 0; \ > + break; \ > + } else if (cvmx_get_cycle() > done) { \ > + result = -1; \ > + break; \ > + } else \ > + __delay(100); \ > + } \ > + } while (0); \ > + result; \ > +}) It's still a mess, why not just make this a function call instead? thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco. 2018-07-27 15:15 ` Greg Kroah-Hartman @ 2018-07-28 15:48 ` Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 0/2] Staging: octeon-usb: Changed CVMX_WAIT_FOR_FIELD32 macro Georgios Tsotsos ` (3 subsequent siblings) 4 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-28 15:48 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Aaro Koskinen, James Hogan, linux-kernel, Joe Perches I will change it into function, as i checked so far i will need to change USB_SET_FIELD32 for the same reason. On Fri, 27 Jul 2018 at 18:15, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Jul 26, 2018 at 06:41:52PM +0300, Georgios Tsotsos wrote: > > Fixing coding style for CVMX_WAIT_FOR_FIELD32 was confusing. Also > > encapsulates into parentheses timeout_usec. > > > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > > --- > > drivers/staging/octeon-usb/octeon-hcd.c | 44 +++++++++++++++++---------------- > > 1 file changed, 23 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > > index cff5e790b196..c8e0ebf1434f 100644 > > --- a/drivers/staging/octeon-usb/octeon-hcd.c > > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > > @@ -378,27 +378,29 @@ struct octeon_hcd { > > }; > > > > /* This macro spins on a register waiting for it to reach a condition. */ > > -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ > > - ({int result; \ > > - do { \ > > - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ > > - octeon_get_clock_rate() / 1000000; \ > > - union _union c; \ > > - \ > > - while (1) { \ > > - c.u32 = cvmx_usb_read_csr32(usb, address); \ > > - \ > > - if (cond) { \ > > - result = 0; \ > > - break; \ > > - } else if (cvmx_get_cycle() > done) { \ > > - result = -1; \ > > - break; \ > > - } else \ > > - __delay(100); \ > > - } \ > > - } while (0); \ > > - result; }) > > +#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ > > +({ \ > > + int result; \ > > + do { \ > > + u64 done = cvmx_get_cycle() + (u64)(timeout_usec) * \ > > + octeon_get_clock_rate() / 1000000; \ > > + union _union c; \ > > + \ > > + while (1) { \ > > + c.u32 = cvmx_usb_read_csr32(usb, address); \ > > + \ > > + if (cond) { \ > > + result = 0; \ > > + break; \ > > + } else if (cvmx_get_cycle() > done) { \ > > + result = -1; \ > > + break; \ > > + } else \ > > + __delay(100); \ > > + } \ > > + } while (0); \ > > + result; \ > > +}) > > It's still a mess, why not just make this a function call instead? > > thanks, > > greg k-h -- Best regards! Georgios Tsotsos Greece-Evia-Chalkida ------------------------------------ Georgios Tsotsos *Greece - Evia - Chalkida* ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 0/2] Staging: octeon-usb: Changed CVMX_WAIT_FOR_FIELD32 macro 2018-07-27 15:15 ` Greg Kroah-Hartman 2018-07-28 15:48 ` Georgios Tsotsos @ 2018-07-29 11:40 ` Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL Georgios Tsotsos ` (2 subsequent siblings) 4 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 11:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Replying to previous message, i changed CVMX_WAIT_FOR_FIELD32 macro to function and altered a multiple calling of CVMX_USBCX_GRSTCTL call to single call and stored to variable. Georgios Tsotsos (2): Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call. drivers/staging/octeon-usb/octeon-hcd.c | 77 +++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 33 deletions(-) -- 2.16.4 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL 2018-07-27 15:15 ` Greg Kroah-Hartman 2018-07-28 15:48 ` Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 0/2] Staging: octeon-usb: Changed CVMX_WAIT_FOR_FIELD32 macro Georgios Tsotsos @ 2018-07-29 11:40 ` Georgios Tsotsos 2018-07-29 12:44 ` Greg Kroah-Hartman 2018-07-29 11:40 ` [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call Georgios Tsotsos 2018-07-29 14:13 ` [PATCH v4 " Georgios Tsotsos 4 siblings, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 11:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple calling a macro. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cff5e790b196..4615133292b5 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -598,6 +598,7 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3; union cvmx_usbcx_gnptxfsiz npsiz; union cvmx_usbcx_hptxfsiz psiz; + u64 address; usbcx_ghwcfg3.u32 = cvmx_usb_read_csr32(usb, CVMX_USBCX_GHWCFG3(usb->index)); @@ -629,17 +630,16 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) psiz.s.ptxfstaddr = 3 * usbcx_ghwcfg3.s.dfifodepth / 4; cvmx_usb_write_csr32(usb, CVMX_USBCX_HPTXFSIZ(usb->index), psiz.u32); + address = CVMX_USBCX_GRSTCTL(usb->index); + /* Flush all FIFOs */ - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, txfnum, 0x10); - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, txfflsh, 1); - CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100); - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, rxfflsh, 1); - CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1); + CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, + c.s.txfflsh == 0, 100); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1); + CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, + c.s.rxfflsh == 0, 100); } /** -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL 2018-07-29 11:40 ` [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL Georgios Tsotsos @ 2018-07-29 12:44 ` Greg Kroah-Hartman 2018-07-29 14:13 ` [PATCH v4 " Georgios Tsotsos 0 siblings, 1 reply; 28+ messages in thread From: Greg Kroah-Hartman @ 2018-07-29 12:44 UTC (permalink / raw) To: Georgios Tsotsos; +Cc: devel, James Hogan, linux-kernel, Aaro Koskinen On Sun, Jul 29, 2018 at 02:40:34PM +0300, Georgios Tsotsos wrote: > Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple > calling a macro. > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > --- > drivers/staging/octeon-usb/octeon-hcd.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) What changed from previous versions? You should always list that below the --- line so that we know what is going on... v4? :) thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL 2018-07-29 12:44 ` Greg Kroah-Hartman @ 2018-07-29 14:13 ` Georgios Tsotsos 0 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 14:13 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Assign to variable the result of CVMX_USBCX_GRSTCTL instead of multiple calling a macro. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- v2: It wasn't exist in this or earlier versions of patch series v3: It seems a logical to avoid multiple calls of CVMX_USBCX_GRSTCTL that will also help cleaning up calls of CVMX_WAIT_FOR_FIELD32 v4: Added patch version text drivers/staging/octeon-usb/octeon-hcd.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index cff5e790b196..4615133292b5 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -598,6 +598,7 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3; union cvmx_usbcx_gnptxfsiz npsiz; union cvmx_usbcx_hptxfsiz psiz; + u64 address; usbcx_ghwcfg3.u32 = cvmx_usb_read_csr32(usb, CVMX_USBCX_GHWCFG3(usb->index)); @@ -629,17 +630,16 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) psiz.s.ptxfstaddr = 3 * usbcx_ghwcfg3.s.dfifodepth / 4; cvmx_usb_write_csr32(usb, CVMX_USBCX_HPTXFSIZ(usb->index), psiz.u32); + address = CVMX_USBCX_GRSTCTL(usb->index); + /* Flush all FIFOs */ - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, txfnum, 0x10); - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, txfflsh, 1); - CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100); - USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, rxfflsh, 1); - CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index), - cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1); + CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, + c.s.txfflsh == 0, 100); + USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1); + CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, + c.s.rxfflsh == 0, 100); } /** -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call. 2018-07-27 15:15 ` Greg Kroah-Hartman ` (2 preceding siblings ...) 2018-07-29 11:40 ` [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL Georgios Tsotsos @ 2018-07-29 11:40 ` Georgios Tsotsos 2018-07-29 19:27 ` Aaro Koskinen 2018-07-29 14:13 ` [PATCH v4 " Georgios Tsotsos 4 siblings, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 11:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Replacing CVMX_WAIT_FOR_FIELD32 macro with equivalent function. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 65 +++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index 4615133292b5..8a7bdf1a9fe6 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -377,29 +377,6 @@ struct octeon_hcd { struct cvmx_usb_tx_fifo nonperiodic; }; -/* This macro spins on a register waiting for it to reach a condition. */ -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ - ({int result; \ - do { \ - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ - octeon_get_clock_rate() / 1000000; \ - union _union c; \ - \ - while (1) { \ - c.u32 = cvmx_usb_read_csr32(usb, address); \ - \ - if (cond) { \ - result = 0; \ - break; \ - } else if (cvmx_get_cycle() > done) { \ - result = -1; \ - break; \ - } else \ - __delay(100); \ - } \ - } while (0); \ - result; }) - /* * This macro logically sets a single field in a CSR. It does the sequence * read, modify, and write @@ -593,6 +570,42 @@ static inline int cvmx_usb_get_data_pid(struct cvmx_usb_pipe *pipe) return 0; /* Data0 */ } +/** + * Loop through register until txfflsh or rxfflsh become zero. + * + * @usb: USB block + * @address: 64bit address to read + * @timeout_usec: Timeout + * @fflsh_type: Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh + * + */ +static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address, + u64 timeout_usec, int fflsh_type) +{ + int result; + u64 done = cvmx_get_cycle() + timeout_usec * + (u64)octeon_get_clock_rate / 1000000; + + union cvmx_usbcx_grstctl c; + + while (1) { + c.u32 = cvmx_usb_read_csr32(usb, address); + if (fflsh_type == 0 && c.s.txfflsh == 0) { + result = 0; + break; + } else if (fflsh_type == 1 && c.s.rxfflsh == 0) { + result = 0; + break; + } else if (cvmx_get_cycle() > done) { + result = -1; + break; + } + + __delay(100); + } + return result; +} + static void cvmx_fifo_setup(struct octeon_hcd *usb) { union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3; @@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) /* Flush all FIFOs */ USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10); USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1); - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, - c.s.txfflsh == 0, 100); + cvmx_wait_for_field32(usb, address, 0, 100); USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1); - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, - c.s.rxfflsh == 0, 100); + cvmx_wait_for_field32(usb, address, 1, 100); } /** -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call. 2018-07-29 11:40 ` [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call Georgios Tsotsos @ 2018-07-29 19:27 ` Aaro Koskinen 0 siblings, 0 replies; 28+ messages in thread From: Aaro Koskinen @ 2018-07-29 19:27 UTC (permalink / raw) To: Georgios Tsotsos; +Cc: Greg Kroah-Hartman, James Hogan, devel, linux-kernel Hi, On Sun, Jul 29, 2018 at 02:40:35PM +0300, Georgios Tsotsos wrote: > +/** > + * Loop through register until txfflsh or rxfflsh become zero. > + * > + * @usb: USB block > + * @address: 64bit address to read > + * @timeout_usec: Timeout > + * @fflsh_type: Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh > + * > + */ > +static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address, > + u64 timeout_usec, int fflsh_type) You should change the name of the function, as it's no longer generic "wait for any field" to reach specified condition. The "address" should be calculated from "usb" inside the function. The timeout is always 100, you could just omit it. Nobody is ever checking the result, so it's also redundant... [...] > +{ > + int result; > + u64 done = cvmx_get_cycle() + timeout_usec * > + (u64)octeon_get_clock_rate / 1000000; > + > + union cvmx_usbcx_grstctl c; > + > + while (1) { > + c.u32 = cvmx_usb_read_csr32(usb, address); > + if (fflsh_type == 0 && c.s.txfflsh == 0) { > + result = 0; > + break; > + } else if (fflsh_type == 1 && c.s.rxfflsh == 0) { > + result = 0; > + break; > + } else if (cvmx_get_cycle() > done) { > + result = -1; > + break; > + } > + > + __delay(100); > + } > + return result; > +} > + > static void cvmx_fifo_setup(struct octeon_hcd *usb) > { > union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3; > @@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) > /* Flush all FIFOs */ > USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10); > USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1); > - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, > - c.s.txfflsh == 0, 100); > + cvmx_wait_for_field32(usb, address, 0, 100); How did you test this patch? The parameters are in wrong order, you are specifying 0 timeout. > USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1); > - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, > - c.s.rxfflsh == 0, 100); > + cvmx_wait_for_field32(usb, address, 1, 100); Also wrong order here. A. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call. 2018-07-27 15:15 ` Greg Kroah-Hartman ` (3 preceding siblings ...) 2018-07-29 11:40 ` [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call Georgios Tsotsos @ 2018-07-29 14:13 ` Georgios Tsotsos 4 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 14:13 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Replacing CVMX_WAIT_FOR_FIELD32 macro with equivalent function. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- v2: Changed CVMX_WAIT_FOR_FIELD32 syntax to avoid checkpatch check notice and tried to make the macro more readable. v3: Changed CVMX_WAIT_FOR_FIELD32 macro to function according as refereed in commit message and suggested by Greg Kroah-Hartman v4: Added patch version text drivers/staging/octeon-usb/octeon-hcd.c | 65 +++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index 4615133292b5..8a7bdf1a9fe6 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -377,29 +377,6 @@ struct octeon_hcd { struct cvmx_usb_tx_fifo nonperiodic; }; -/* This macro spins on a register waiting for it to reach a condition. */ -#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \ - ({int result; \ - do { \ - u64 done = cvmx_get_cycle() + (u64)timeout_usec * \ - octeon_get_clock_rate() / 1000000; \ - union _union c; \ - \ - while (1) { \ - c.u32 = cvmx_usb_read_csr32(usb, address); \ - \ - if (cond) { \ - result = 0; \ - break; \ - } else if (cvmx_get_cycle() > done) { \ - result = -1; \ - break; \ - } else \ - __delay(100); \ - } \ - } while (0); \ - result; }) - /* * This macro logically sets a single field in a CSR. It does the sequence * read, modify, and write @@ -593,6 +570,42 @@ static inline int cvmx_usb_get_data_pid(struct cvmx_usb_pipe *pipe) return 0; /* Data0 */ } +/** + * Loop through register until txfflsh or rxfflsh become zero. + * + * @usb: USB block + * @address: 64bit address to read + * @timeout_usec: Timeout + * @fflsh_type: Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh + * + */ +static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address, + u64 timeout_usec, int fflsh_type) +{ + int result; + u64 done = cvmx_get_cycle() + timeout_usec * + (u64)octeon_get_clock_rate / 1000000; + + union cvmx_usbcx_grstctl c; + + while (1) { + c.u32 = cvmx_usb_read_csr32(usb, address); + if (fflsh_type == 0 && c.s.txfflsh == 0) { + result = 0; + break; + } else if (fflsh_type == 1 && c.s.rxfflsh == 0) { + result = 0; + break; + } else if (cvmx_get_cycle() > done) { + result = -1; + break; + } + + __delay(100); + } + return result; +} + static void cvmx_fifo_setup(struct octeon_hcd *usb) { union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3; @@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb) /* Flush all FIFOs */ USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10); USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1); - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, - c.s.txfflsh == 0, 100); + cvmx_wait_for_field32(usb, address, 0, 100); USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1); - CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl, - c.s.rxfflsh == 0, 100); + cvmx_wait_for_field32(usb, address, 1, 100); } /** -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-26 15:41 [PATCH v2 0/3] Staging: octeon-usb fixes for coding style, SPDX and readability Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco Georgios Tsotsos @ 2018-07-26 13:30 ` Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-26 16:31 ` Joe Perches 2 siblings, 2 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 13:30 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Georgios Tsotsos, linux-kernel In order to make this function more clear a new function created that controls channels halt on no DMA mode. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 83 +++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index c8e0ebf1434f..f9f429d385ce 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -2585,6 +2585,52 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, } } +/** + * Handles channels halt in non DMA mode + * @usbc_hcchar: Host Channel-n Characteristics Register (HCCHAR) + * @usbc_hcint: Host Channel-n Interrupt Register + * @usb: USB device + * @channel: Channel to poll + * + * In non DMA mode the channels don't halt themselves. We need + * to manually disable channels that are left running + * + * Returns: -1 on halt + */ +static int cvmx_usb_dma_halt(union cvmx_usbcx_hccharx usbc_hcchar, + union cvmx_usbcx_hcintx usbc_hcint, + struct octeon_hcd *usb, + int channel) +{ + struct usb_hcd *hcd = octeon_to_hcd(usb); + struct device *dev = hcd->self.controller; + + if (usbc_hcchar.s.chena) { + union cvmx_usbcx_hcintmskx hcintmsk; + /* Disable all interrupts except CHHLTD */ + hcintmsk.u32 = 0; + hcintmsk.s.chhltdmsk = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCINTMSKX(channel, usb->index), + hcintmsk.u32); + usbc_hcchar.s.chdis = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCCHARX(channel, usb->index), + usbc_hcchar.u32); + return -1; + } else if (usbc_hcint.s.xfercompl) { + /* + * Successful IN/OUT with transfer complete. + * Channel halt isn't needed. + */ + } else { + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", + usb->index, channel); + return -1; + } + + return 0; +} /** * Poll a channel for status * @@ -2595,8 +2641,6 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, */ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) { - struct usb_hcd *hcd = octeon_to_hcd(usb); - struct device *dev = hcd->self.controller; union cvmx_usbcx_hcintx usbc_hcint; union cvmx_usbcx_hctsizx usbc_hctsiz; union cvmx_usbcx_hccharx usbc_hcchar; @@ -2627,35 +2671,16 @@ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) usbc_hcchar.u32); return 0; } - - /* - * In non DMA mode the channels don't halt themselves. We need - * to manually disable channels that are left running - */ + /* In case of non DMA mode handle halt */ if (!usbc_hcint.s.chhltd) { - if (usbc_hcchar.s.chena) { - union cvmx_usbcx_hcintmskx hcintmsk; - /* Disable all interrupts except CHHLTD */ - hcintmsk.u32 = 0; - hcintmsk.s.chhltdmsk = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCINTMSKX(channel, usb->index), - hcintmsk.u32); - usbc_hcchar.s.chdis = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCCHARX(channel, usb->index), - usbc_hcchar.u32); - return 0; - } else if (usbc_hcint.s.xfercompl) { - /* - * Successful IN/OUT with transfer complete. - * Channel halt isn't needed. - */ - } else { - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", - usb->index, channel); + int dma_halt_status = 0; + + dma_halt_status = cvmx_usb_dma_halt(usbc_hcchar, + usbc_hcint, + usb, channel); + + if (dma_halt_status < 0) return 0; - } } } else { /* -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-26 13:30 ` [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos @ 2018-07-26 15:41 ` Georgios Tsotsos 2018-07-26 16:31 ` Joe Perches 1 sibling, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 15:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, James Hogan, Aaro Koskinen, Joe Perches, devel, linux-kernel In order to make this function more clear a new function created that controls channels halt on no DMA mode. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 83 +++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index c8e0ebf1434f..f9f429d385ce 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -2585,6 +2585,52 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, } } +/** + * Handles channels halt in non DMA mode + * @usbc_hcchar: Host Channel-n Characteristics Register (HCCHAR) + * @usbc_hcint: Host Channel-n Interrupt Register + * @usb: USB device + * @channel: Channel to poll + * + * In non DMA mode the channels don't halt themselves. We need + * to manually disable channels that are left running + * + * Returns: -1 on halt + */ +static int cvmx_usb_dma_halt(union cvmx_usbcx_hccharx usbc_hcchar, + union cvmx_usbcx_hcintx usbc_hcint, + struct octeon_hcd *usb, + int channel) +{ + struct usb_hcd *hcd = octeon_to_hcd(usb); + struct device *dev = hcd->self.controller; + + if (usbc_hcchar.s.chena) { + union cvmx_usbcx_hcintmskx hcintmsk; + /* Disable all interrupts except CHHLTD */ + hcintmsk.u32 = 0; + hcintmsk.s.chhltdmsk = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCINTMSKX(channel, usb->index), + hcintmsk.u32); + usbc_hcchar.s.chdis = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCCHARX(channel, usb->index), + usbc_hcchar.u32); + return -1; + } else if (usbc_hcint.s.xfercompl) { + /* + * Successful IN/OUT with transfer complete. + * Channel halt isn't needed. + */ + } else { + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", + usb->index, channel); + return -1; + } + + return 0; +} /** * Poll a channel for status * @@ -2595,8 +2641,6 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, */ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) { - struct usb_hcd *hcd = octeon_to_hcd(usb); - struct device *dev = hcd->self.controller; union cvmx_usbcx_hcintx usbc_hcint; union cvmx_usbcx_hctsizx usbc_hctsiz; union cvmx_usbcx_hccharx usbc_hcchar; @@ -2627,35 +2671,16 @@ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) usbc_hcchar.u32); return 0; } - - /* - * In non DMA mode the channels don't halt themselves. We need - * to manually disable channels that are left running - */ + /* In case of non DMA mode handle halt */ if (!usbc_hcint.s.chhltd) { - if (usbc_hcchar.s.chena) { - union cvmx_usbcx_hcintmskx hcintmsk; - /* Disable all interrupts except CHHLTD */ - hcintmsk.u32 = 0; - hcintmsk.s.chhltdmsk = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCINTMSKX(channel, usb->index), - hcintmsk.u32); - usbc_hcchar.s.chdis = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCCHARX(channel, usb->index), - usbc_hcchar.u32); - return 0; - } else if (usbc_hcint.s.xfercompl) { - /* - * Successful IN/OUT with transfer complete. - * Channel halt isn't needed. - */ - } else { - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", - usb->index, channel); + int dma_halt_status = 0; + + dma_halt_status = cvmx_usb_dma_halt(usbc_hcchar, + usbc_hcint, + usb, channel); + + if (dma_halt_status < 0) return 0; - } } } else { /* -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-26 13:30 ` [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos @ 2018-07-26 16:31 ` Joe Perches 2018-07-26 22:08 ` Georgios Tsotsos ` (2 more replies) 1 sibling, 3 replies; 28+ messages in thread From: Joe Perches @ 2018-07-26 16:31 UTC (permalink / raw) To: Georgios Tsotsos, Greg Kroah-Hartman Cc: James Hogan, Aaro Koskinen, devel, linux-kernel On Thu, 2018-07-26 at 18:41 +0300, Georgios Tsotsos wrote: > In order to make this function more clear a new function created that controls > channels halt on no DMA mode. [] > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c [] > @@ -2585,6 +2585,52 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > } > } > > +/** > + * Handles channels halt in non DMA mode > + * @usbc_hcchar: Host Channel-n Characteristics Register (HCCHAR) > + * @usbc_hcint: Host Channel-n Interrupt Register > + * @usb: USB device > + * @channel: Channel to poll > + * > + * In non DMA mode the channels don't halt themselves. We need > + * to manually disable channels that are left running > + * > + * Returns: -1 on halt > + */ > +static int cvmx_usb_dma_halt(union cvmx_usbcx_hccharx usbc_hcchar, > + union cvmx_usbcx_hcintx usbc_hcint, It looks very suspect to pass unions on the stack. Are you sure these aren't used after this function is called? Likely these should be pointers to unions. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-26 16:31 ` Joe Perches @ 2018-07-26 22:08 ` Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 0/1] " Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 1/1] " Georgios Tsotsos 2 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-26 22:08 UTC (permalink / raw) To: Joe Perches Cc: Greg Kroah-Hartman, James Hogan, Aaro Koskinen, devel, linux-kernel Indeed i should probably either use pointer or pass the values, i will do some more testing and update this. Thanks On Thu, 26 Jul 2018 at 19:31, Joe Perches <joe@perches.com> wrote: > > On Thu, 2018-07-26 at 18:41 +0300, Georgios Tsotsos wrote: > > In order to make this function more clear a new function created that controls > > channels halt on no DMA mode. > [] > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > [] > > @@ -2585,6 +2585,52 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > > } > > } > > > > +/** > > + * Handles channels halt in non DMA mode > > + * @usbc_hcchar: Host Channel-n Characteristics Register (HCCHAR) > > + * @usbc_hcint: Host Channel-n Interrupt Register > > + * @usb: USB device > > + * @channel: Channel to poll > > + * > > + * In non DMA mode the channels don't halt themselves. We need > > + * to manually disable channels that are left running > > + * > > + * Returns: -1 on halt > > + */ > > +static int cvmx_usb_dma_halt(union cvmx_usbcx_hccharx usbc_hcchar, > > + union cvmx_usbcx_hcintx usbc_hcint, > > It looks very suspect to pass unions on the stack. > > Are you sure these aren't used after this function > is called? > > Likely these should be pointers to unions. > -- Best regards! Georgios Tsotsos Greece-Evia-Chalkida tsotsos@linux.com skype: tsotsos ------------------------------------ Georgios Tsotsos *Greece - Evia - Chalkida* tsotsos[at]linux.com skype: tsotsos ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 0/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() 2018-07-26 16:31 ` Joe Perches 2018-07-26 22:08 ` Georgios Tsotsos @ 2018-07-29 11:41 ` Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 1/1] " Georgios Tsotsos 2 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 11:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Changed the function in order to use only values instead passing unions i think its better than passing pointers there. Georgios Tsotsos (1): Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). drivers/staging/octeon-usb/octeon-hcd.c | 81 +++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 28 deletions(-) -- 2.16.4 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-26 16:31 ` Joe Perches 2018-07-26 22:08 ` Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 0/1] " Georgios Tsotsos @ 2018-07-29 11:41 ` Georgios Tsotsos 2018-07-29 12:43 ` Greg Kroah-Hartman 2 siblings, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 11:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel In order to make this function more clear a new function created that controls channels halt on no DMA mode. Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- drivers/staging/octeon-usb/octeon-hcd.c | 81 +++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 28 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index 8a7bdf1a9fe6..3f44ac260eff 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); } } +/** + * Handles channels halt in non DMA mode + * @hcchar_chena: + * @hcint_xfercompl: + * @usb: USB device + * @channel: Channel to poll + * + * In non DMA mode the channels don't halt themselves. We need + * to manually disable channels that are left running + * + * Returns: -1 on halt + */ +static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, + struct octeon_hcd *usb, int channel) +{ + struct usb_hcd *hcd = octeon_to_hcd(usb); + struct device *dev = hcd->self.controller; + if (hcchar_chena) { + union cvmx_usbcx_hcintmskx hcintmsk; + union cvmx_usbcx_hccharx usbc_hcchar; + /* Disable all interrupts except CHHLTD */ + hcintmsk.u32 = 0; + hcintmsk.s.chhltdmsk = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCINTMSKX(channel, usb->index), + hcintmsk.u32); + usbc_hcchar.s.chdis = 1; + cvmx_usb_write_csr32(usb, + CVMX_USBCX_HCCHARX(channel, usb->index), + usbc_hcchar.u32); + return -1; + } else if (hcint_xfercompl) { + /* + * Successful IN/OUT with transfer complete. + * Channel halt isn't needed. + */ + } else { + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", + usb->index, channel); + return -1; + } + + return 0; +} /** * Poll a channel for status * @@ -2604,8 +2648,6 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, */ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) { - struct usb_hcd *hcd = octeon_to_hcd(usb); - struct device *dev = hcd->self.controller; union cvmx_usbcx_hcintx usbc_hcint; union cvmx_usbcx_hctsizx usbc_hctsiz; union cvmx_usbcx_hccharx usbc_hcchar; @@ -2637,34 +2679,17 @@ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel) return 0; } - /* - * In non DMA mode the channels don't halt themselves. We need - * to manually disable channels that are left running - */ + /* In case of non DMA mode handle halt */ if (!usbc_hcint.s.chhltd) { - if (usbc_hcchar.s.chena) { - union cvmx_usbcx_hcintmskx hcintmsk; - /* Disable all interrupts except CHHLTD */ - hcintmsk.u32 = 0; - hcintmsk.s.chhltdmsk = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCINTMSKX(channel, usb->index), - hcintmsk.u32); - usbc_hcchar.s.chdis = 1; - cvmx_usb_write_csr32(usb, - CVMX_USBCX_HCCHARX(channel, usb->index), - usbc_hcchar.u32); - return 0; - } else if (usbc_hcint.s.xfercompl) { - /* - * Successful IN/OUT with transfer complete. - * Channel halt isn't needed. - */ - } else { - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", - usb->index, channel); + int dma_halt_status = 0; + u32 xfercompl = usbc_hcint.s.xfercompl; + + dma_halt_status = cvmx_usb_dma_halt(usbc_hcchar.s.chena, + xfercompl, + usb, channel); + + if (dma_halt_status < 0) return 0; - } } } else { /* -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-29 11:41 ` [PATCH v3 1/1] " Georgios Tsotsos @ 2018-07-29 12:43 ` Greg Kroah-Hartman 2018-07-29 14:33 ` [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style Georgios Tsotsos 2018-07-29 14:52 ` [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 0 siblings, 2 replies; 28+ messages in thread From: Greg Kroah-Hartman @ 2018-07-29 12:43 UTC (permalink / raw) To: Georgios Tsotsos; +Cc: devel, James Hogan, linux-kernel, Aaro Koskinen On Sun, Jul 29, 2018 at 02:41:53PM +0300, Georgios Tsotsos wrote: > In order to make this function more clear a new function created that controls > channels halt on no DMA mode. > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > --- > drivers/staging/octeon-usb/octeon-hcd.c | 81 +++++++++++++++++++++------------ > 1 file changed, 53 insertions(+), 28 deletions(-) > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > index 8a7bdf1a9fe6..3f44ac260eff 100644 > --- a/drivers/staging/octeon-usb/octeon-hcd.c > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > @@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); > } > } > +/** Blank line between functions please. Also, as this is not a global function, no need for kerneldoc formatting, but you did it already, so no big deal. > + * Handles channels halt in non DMA mode > + * @hcchar_chena: > + * @hcint_xfercompl: > + * @usb: USB device > + * @channel: Channel to poll > + * > + * In non DMA mode the channels don't halt themselves. We need > + * to manually disable channels that are left running > + * > + * Returns: -1 on halt > + */ > +static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, > + struct octeon_hcd *usb, int channel) > +{ > + struct usb_hcd *hcd = octeon_to_hcd(usb); > + struct device *dev = hcd->self.controller; > > + if (hcchar_chena) { > + union cvmx_usbcx_hcintmskx hcintmsk; > + union cvmx_usbcx_hccharx usbc_hcchar; > + /* Disable all interrupts except CHHLTD */ > + hcintmsk.u32 = 0; > + hcintmsk.s.chhltdmsk = 1; > + cvmx_usb_write_csr32(usb, > + CVMX_USBCX_HCINTMSKX(channel, usb->index), > + hcintmsk.u32); > + usbc_hcchar.s.chdis = 1; > + cvmx_usb_write_csr32(usb, > + CVMX_USBCX_HCCHARX(channel, usb->index), > + usbc_hcchar.u32); > + return -1; Do not make up error values, return -EINVAL or something like that (what ever the real error here is.) > + } else if (hcint_xfercompl) { > + /* > + * Successful IN/OUT with transfer complete. > + * Channel halt isn't needed. > + */ > + } else { > + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", > + usb->index, channel); > + return -1; Same here. thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style 2018-07-29 12:43 ` Greg Kroah-Hartman @ 2018-07-29 14:33 ` Georgios Tsotsos 2018-07-29 20:21 ` Aaro Koskinen 2018-07-29 14:52 ` [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 1 sibling, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 14:33 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Replaced -1 with defined error code EINVAL Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- v2: Apply coding style to function cvmx_usb_poll_channel v3: Break down function cvmx_usb_poll_channel v4: Return defined error code and applying coding style for function calls drivers/staging/octeon-usb/octeon-hcd.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index 3f44ac260eff..edf87d1b3609 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -2590,6 +2590,7 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, } } else { pipe->next_tx_frame += pipe->interval; + cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); } } @@ -2624,16 +2625,16 @@ static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, cvmx_usb_write_csr32(usb, CVMX_USBCX_HCCHARX(channel, usb->index), usbc_hcchar.u32); - return -1; + return -EINVAL; } else if (hcint_xfercompl) { /* * Successful IN/OUT with transfer complete. * Channel halt isn't needed. */ } else { - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", + dev_ err(dev, "USB%d: Channel %d interrupt without halt\n", usb->index, channel); - return -1; + return -EINVAL; } return 0; -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style 2018-07-29 14:33 ` [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style Georgios Tsotsos @ 2018-07-29 20:21 ` Aaro Koskinen 2018-07-29 21:17 ` Georgios Tsotsos 2018-07-29 22:29 ` [PATCH v5] " Georgios Tsotsos 0 siblings, 2 replies; 28+ messages in thread From: Aaro Koskinen @ 2018-07-29 20:21 UTC (permalink / raw) To: Georgios Tsotsos; +Cc: Greg Kroah-Hartman, James Hogan, devel, linux-kernel Hi, On Sun, Jul 29, 2018 at 05:33:38PM +0300, Georgios Tsotsos wrote: > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > index 3f44ac260eff..edf87d1b3609 100644 > --- a/drivers/staging/octeon-usb/octeon-hcd.c > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > @@ -2590,6 +2590,7 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > } > } else { > pipe->next_tx_frame += pipe->interval; > + > cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); Unrelated whitespace change... > } > } > @@ -2624,16 +2625,16 @@ static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, > cvmx_usb_write_csr32(usb, > CVMX_USBCX_HCCHARX(channel, usb->index), > usbc_hcchar.u32); > - return -1; > + return -EINVAL; > } else if (hcint_xfercompl) { > /* > * Successful IN/OUT with transfer complete. > * Channel halt isn't needed. > */ > } else { > - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", > + dev_ err(dev, "USB%d: Channel %d interrupt without halt\n", > usb->index, channel); ...also here. Does this even compile? > - return -1; > + return -EINVAL; > } > > return 0; > -- > 2.16.4 A. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style 2018-07-29 20:21 ` Aaro Koskinen @ 2018-07-29 21:17 ` Georgios Tsotsos 2018-07-29 22:29 ` [PATCH v5] " Georgios Tsotsos 1 sibling, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 21:17 UTC (permalink / raw) To: Aaro Koskinen; +Cc: Greg Kroah-Hartman, James Hogan, devel, linux-kernel Hi, Indeed there was a mix-up with patches i will send correction asap. On Sun, 29 Jul 2018 at 23:21, Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > > Hi, > > On Sun, Jul 29, 2018 at 05:33:38PM +0300, Georgios Tsotsos wrote: > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > > index 3f44ac260eff..edf87d1b3609 100644 > > --- a/drivers/staging/octeon-usb/octeon-hcd.c > > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > > @@ -2590,6 +2590,7 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > > } > > } else { > > pipe->next_tx_frame += pipe->interval; > > + > > cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); > > Unrelated whitespace change... > > > } > > } > > @@ -2624,16 +2625,16 @@ static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, > > cvmx_usb_write_csr32(usb, > > CVMX_USBCX_HCCHARX(channel, usb->index), > > usbc_hcchar.u32); > > - return -1; > > + return -EINVAL; > > } else if (hcint_xfercompl) { > > /* > > * Successful IN/OUT with transfer complete. > > * Channel halt isn't needed. > > */ > > } else { > > - dev_err(dev, "USB%d: Channel %d interrupt without halt\n", > > + dev_ err(dev, "USB%d: Channel %d interrupt without halt\n", > > usb->index, channel); > > ...also here. Does this even compile? > > > - return -1; > > + return -EINVAL; > > } > > > > return 0; > > -- > > 2.16.4 > > A. -- Best regards! Georgios Tsotsos Greece-Evia-Chalkida tsotsos@linux.com skype: tsotsos ------------------------------------ Georgios Tsotsos *Greece - Evia - Chalkida* tsotsos[at]linux.com skype: tsotsos ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5] Staging: octeon-usb: Using defined error codes and applying coding style. 2018-07-29 20:21 ` Aaro Koskinen 2018-07-29 21:17 ` Georgios Tsotsos @ 2018-07-29 22:29 ` Georgios Tsotsos 2018-07-30 8:51 ` Greg Kroah-Hartman 1 sibling, 1 reply; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 22:29 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Georgios Tsotsos, Aaro Koskinen, James Hogan, devel, linux-kernel Replaced -1 with defined error code EINVAL Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> --- v2: Apply coding style to function cvmx_usb_poll_channel v3: Break down function cvmx_usb_poll_channel v4: Return defined error code and applying coding style for function calls v5: Fixing wrong patch applied before with typo on dev_err and applying coding style as suggested on v3 drivers/staging/octeon-usb/octeon-hcd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c index c9fbff93bed4..dd73fd48e12e 100644 --- a/drivers/staging/octeon-usb/octeon-hcd.c +++ b/drivers/staging/octeon-usb/octeon-hcd.c @@ -2587,10 +2587,10 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, } } else { pipe->next_tx_frame += pipe->interval; - cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); } } + /** * Handles channels halt in non DMA mode * @hcchar_chena: @@ -2629,7 +2629,7 @@ static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, * Channel halt isn't needed. */ } else { - dev_ err(dev, "USB%d: Channel %d interrupt without halt\n", + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", usb->index, channel); return -EINVAL; } -- 2.16.4 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v5] Staging: octeon-usb: Using defined error codes and applying coding style. 2018-07-29 22:29 ` [PATCH v5] " Georgios Tsotsos @ 2018-07-30 8:51 ` Greg Kroah-Hartman 2018-07-30 20:18 ` Georgios Tsotsos 0 siblings, 1 reply; 28+ messages in thread From: Greg Kroah-Hartman @ 2018-07-30 8:51 UTC (permalink / raw) To: Georgios Tsotsos; +Cc: devel, James Hogan, linux-kernel, Aaro Koskinen On Mon, Jul 30, 2018 at 01:29:36AM +0300, Georgios Tsotsos wrote: > Replaced -1 with defined error code EINVAL > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > --- > v2: Apply coding style to function cvmx_usb_poll_channel > v3: Break down function cvmx_usb_poll_channel > v4: Return defined error code and applying coding style for function calls > v5: Fixing wrong patch applied before with typo on dev_err and applying coding > style as suggested on v3 > > drivers/staging/octeon-usb/octeon-hcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Ugh, I have no idea anymore what patches to apply from you for this driver, sorry, there are too many flying around. Take a day off, relax, and then resend all of your pending patches as one series, properly numbered, and we can go from there. I've dropped all of your patches from my queue for now. thanks, greg k-h ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5] Staging: octeon-usb: Using defined error codes and applying coding style. 2018-07-30 8:51 ` Greg Kroah-Hartman @ 2018-07-30 20:18 ` Georgios Tsotsos 0 siblings, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-30 20:18 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: devel, James Hogan, linux-kernel, Aaro Koskinen Yes reseeding in a better way seems logical, i even got lost with the patch series. I will return with more consistent series. On Mon, 30 Jul 2018 at 11:51, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Jul 30, 2018 at 01:29:36AM +0300, Georgios Tsotsos wrote: > > Replaced -1 with defined error code EINVAL > > > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > > --- > > v2: Apply coding style to function cvmx_usb_poll_channel > > v3: Break down function cvmx_usb_poll_channel > > v4: Return defined error code and applying coding style for function calls > > v5: Fixing wrong patch applied before with typo on dev_err and applying coding > > style as suggested on v3 > > > > drivers/staging/octeon-usb/octeon-hcd.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Ugh, I have no idea anymore what patches to apply from you for this > driver, sorry, there are too many flying around. > > Take a day off, relax, and then resend all of your pending patches as > one series, properly numbered, and we can go from there. I've dropped > all of your patches from my queue for now. > > thanks, > > greg k-h -- Best regards! Georgios Tsotsos Greece-Evia-Chalkida tsotsos@linux.com skype: tsotsos ------------------------------------ Georgios Tsotsos *Greece - Evia - Chalkida* tsotsos[at]linux.com skype: tsotsos ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel(). 2018-07-29 12:43 ` Greg Kroah-Hartman 2018-07-29 14:33 ` [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style Georgios Tsotsos @ 2018-07-29 14:52 ` Georgios Tsotsos 1 sibling, 0 replies; 28+ messages in thread From: Georgios Tsotsos @ 2018-07-29 14:52 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: devel, James Hogan, linux-kernel, Aaro Koskinen Hello, Regarding your latest comment, i have notice many functions in this module using kerneldoc and not been global, also there are various erroneous situations that functions return not defined error codes. I will try to fix them all and a new patch series for those two issues after this one is ok. (I hope with less versions :)) On Sun, 29 Jul 2018 at 15:43, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Sun, Jul 29, 2018 at 02:41:53PM +0300, Georgios Tsotsos wrote: > > In order to make this function more clear a new function created that controls > > channels halt on no DMA mode. > > > > Signed-off-by: Georgios Tsotsos <tsotsos@gmail.com> > > --- > > drivers/staging/octeon-usb/octeon-hcd.c | 81 +++++++++++++++++++++------------ > > 1 file changed, 53 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c > > index 8a7bdf1a9fe6..3f44ac260eff 100644 > > --- a/drivers/staging/octeon-usb/octeon-hcd.c > > +++ b/drivers/staging/octeon-usb/octeon-hcd.c > > @@ -2593,7 +2593,51 @@ static void cvmx_usb_transfer_isoc(struct octeon_hcd *usb, > > cvmx_usb_complete(usb, pipe, transaction, CVMX_USB_STATUS_OK); > > } > > } > > +/** > > Blank line between functions please. > > Also, as this is not a global function, no need for kerneldoc > formatting, but you did it already, so no big deal. > > > + * Handles channels halt in non DMA mode > > + * @hcchar_chena: > > + * @hcint_xfercompl: > > + * @usb: USB device > > + * @channel: Channel to poll > > + * > > + * In non DMA mode the channels don't halt themselves. We need > > + * to manually disable channels that are left running > > + * > > + * Returns: -1 on halt > > + */ > > +static int cvmx_usb_dma_halt(u32 hcchar_chena, u32 hcint_xfercompl, > > + struct octeon_hcd *usb, int channel) > > +{ > > + struct usb_hcd *hcd = octeon_to_hcd(usb); > > + struct device *dev = hcd->self.controller; > > > > + if (hcchar_chena) { > > + union cvmx_usbcx_hcintmskx hcintmsk; > > + union cvmx_usbcx_hccharx usbc_hcchar; > > + /* Disable all interrupts except CHHLTD */ > > + hcintmsk.u32 = 0; > > + hcintmsk.s.chhltdmsk = 1; > > + cvmx_usb_write_csr32(usb, > > + CVMX_USBCX_HCINTMSKX(channel, usb->index), > > + hcintmsk.u32); > > + usbc_hcchar.s.chdis = 1; > > + cvmx_usb_write_csr32(usb, > > + CVMX_USBCX_HCCHARX(channel, usb->index), > > + usbc_hcchar.u32); > > + return -1; > > Do not make up error values, return -EINVAL or something like that (what > ever the real error here is.) > > > + } else if (hcint_xfercompl) { > > + /* > > + * Successful IN/OUT with transfer complete. > > + * Channel halt isn't needed. > > + */ > > + } else { > > + dev_err(dev, "USB%d: Channel %d interrupt without halt\n", > > + usb->index, channel); > > + return -1; > > Same here. > > thanks, > > greg k-h -- Best regards! Georgios Tsotsos Greece-Evia-Chalkida tsotsos@linux.com skype: tsotsos ------------------------------------ Georgios Tsotsos *Greece - Evia - Chalkida* tsotsos[at]linux.com skype: tsotsos ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2018-07-30 20:18 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-26 15:41 [PATCH v2 0/3] Staging: octeon-usb fixes for coding style, SPDX and readability Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 1/3] Staging: octeon-usb: Adding SPDX license identifier Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 2/3] Staging: octeon-usb: Change coding style of CVMX_WAIT_FOR_FIELD32 marco Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-27 15:15 ` Greg Kroah-Hartman 2018-07-28 15:48 ` Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 0/2] Staging: octeon-usb: Changed CVMX_WAIT_FOR_FIELD32 macro Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 1/2] Staging: octeon-usb: Change multiple calling of CVMX_USBCX_GRSTCTL Georgios Tsotsos 2018-07-29 12:44 ` Greg Kroah-Hartman 2018-07-29 14:13 ` [PATCH v4 " Georgios Tsotsos 2018-07-29 11:40 ` [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call Georgios Tsotsos 2018-07-29 19:27 ` Aaro Koskinen 2018-07-29 14:13 ` [PATCH v4 " Georgios Tsotsos 2018-07-26 13:30 ` [PATCH 3/3] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos 2018-07-26 15:41 ` [PATCH v2 " Georgios Tsotsos 2018-07-26 16:31 ` Joe Perches 2018-07-26 22:08 ` Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 0/1] " Georgios Tsotsos 2018-07-29 11:41 ` [PATCH v3 1/1] " Georgios Tsotsos 2018-07-29 12:43 ` Greg Kroah-Hartman 2018-07-29 14:33 ` [PATCH v4 1/1] Staging: octeon-usb: Using defined error codes and applying coding style Georgios Tsotsos 2018-07-29 20:21 ` Aaro Koskinen 2018-07-29 21:17 ` Georgios Tsotsos 2018-07-29 22:29 ` [PATCH v5] " Georgios Tsotsos 2018-07-30 8:51 ` Greg Kroah-Hartman 2018-07-30 20:18 ` Georgios Tsotsos 2018-07-29 14:52 ` [PATCH v3 1/1] Staging: octeon-usb: Breaks down cvmx_usb_poll_channel() Georgios Tsotsos
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).