Re: [PATCH 1/2] staging: gasket: Fix 4 over 80 char warnings
On Wed, Apr 01, 2020 at 10:36:16PM -0700, John B. Wyatt IV wrote:
> Fix 4 over 80 char warnings by caching long enum values into local
> variables.
>
> All enums are only used once inside each function (and once inside
> the entire file).
>
> Reported by checkpatch.
>
> Signed-off-by: John B. Wyatt IV
> ---
> drivers/staging/gasket/apex_driver.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/gasket/apex_driver.c
> b/drivers/staging/gasket/apex_driver.c
> index 46199c8ca441..f48209ec7d24 100644
> --- a/drivers/staging/gasket/apex_driver.c
> +++ b/drivers/staging/gasket/apex_driver.c
> @@ -253,6 +253,8 @@ static int apex_get_status(struct gasket_dev *gasket_dev)
> /* Enter GCB reset state. */
> static int apex_enter_reset(struct gasket_dev *gasket_dev)
> {
> + int idle_gen_reg = APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER;
> +
Sorry, but I also hate these where we have a one time use temporary
variable to get around the 80 character rule. Generally, avoid pointless
indirection. The original code is better.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: gasket: Fix 4 over 80 char warnings
On Wed, Apr 01, 2020 at 10:54:17PM -0700, Joe Perches wrote:
> On Wed, 2020-04-01 at 22:36 -0700, John B. Wyatt IV wrote:
> > Fix 4 over 80 char warnings by caching long enum values into local
> > variables.
> >
> > All enums are only used once inside each function (and once inside
> > the entire file).
> >
> > Reported by checkpatch.
> >
> > Signed-off-by: John B. Wyatt IV
> > ---
> > drivers/staging/gasket/apex_driver.c | 13 +
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/gasket/apex_driver.c
> > b/drivers/staging/gasket/apex_driver.c
> > index 46199c8ca441..f48209ec7d24 100644
> > --- a/drivers/staging/gasket/apex_driver.c
> > +++ b/drivers/staging/gasket/apex_driver.c
> > @@ -253,6 +253,8 @@ static int apex_get_status(struct gasket_dev
> > *gasket_dev)
> > /* Enter GCB reset state. */
> > static int apex_enter_reset(struct gasket_dev *gasket_dev)
> > {
> > + int idle_gen_reg = APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER;
> > +
>
> This indirection only makes the code more difficult to understand.
I agree, this patch does not improve the readability of the driver at
all, which should always be the primary goal of any coding style
cleanup.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: gasket: Fix 4 over 80 char warnings
On Wed, 2020-04-01 at 22:36 -0700, John B. Wyatt IV wrote:
> Fix 4 over 80 char warnings by caching long enum values into local
> variables.
>
> All enums are only used once inside each function (and once inside
> the entire file).
>
> Reported by checkpatch.
>
> Signed-off-by: John B. Wyatt IV
> ---
> drivers/staging/gasket/apex_driver.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/gasket/apex_driver.c
> b/drivers/staging/gasket/apex_driver.c
> index 46199c8ca441..f48209ec7d24 100644
> --- a/drivers/staging/gasket/apex_driver.c
> +++ b/drivers/staging/gasket/apex_driver.c
> @@ -253,6 +253,8 @@ static int apex_get_status(struct gasket_dev *gasket_dev)
> /* Enter GCB reset state. */
> static int apex_enter_reset(struct gasket_dev *gasket_dev)
> {
> + int idle_gen_reg = APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER;
> +
This indirection only makes the code more difficult to understand.
> if (bypass_top_level)
> return 0;
>
> @@ -263,7 +265,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev)
>*- Enable GCB idle
>*/
> gasket_read_modify_write_64(gasket_dev, APEX_BAR_INDEX,
> -
> APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER,
> + idle_gen_reg,
> 0x0, 1, 32);
>
> /*- Initiate DMA pause */
> @@ -395,11 +397,12 @@ static int apex_device_cleanup(struct gasket_dev
> *gasket_dev)
> u64 scalar_error;
> u64 hib_error;
> int ret = 0;
> + int status = APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS;
>
> hib_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
> APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
> scalar_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
> -
> APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
> + status);
>
> dev_dbg(gasket_dev->dev,
> "%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
> @@ -584,6 +587,8 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
> ulong page_table_ready, msix_table_ready;
> int retries = 0;
> struct gasket_dev *gasket_dev;
> + int page_table_init = APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT;
> + int msix_table_init = APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT;
>
> ret = pci_enable_device(pci_dev);
> if (ret) {
> @@ -606,10 +611,10 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
> while (retries < APEX_RESET_RETRY) {
> page_table_ready =
> gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
> -
> APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
> +page_table_init);
> msix_table_ready =
> gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
> -
> APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
> +msix_table_init);
> if (page_table_ready && msix_table_ready)
> break;
> schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: gasket: Fix 4 over 80 char warnings
Fix 4 over 80 char warnings by caching long enum values into local
variables.
All enums are only used once inside each function (and once inside
the entire file).
Reported by checkpatch.
Signed-off-by: John B. Wyatt IV
---
drivers/staging/gasket/apex_driver.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c
b/drivers/staging/gasket/apex_driver.c
index 46199c8ca441..f48209ec7d24 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -253,6 +253,8 @@ static int apex_get_status(struct gasket_dev *gasket_dev)
/* Enter GCB reset state. */
static int apex_enter_reset(struct gasket_dev *gasket_dev)
{
+ int idle_gen_reg = APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER;
+
if (bypass_top_level)
return 0;
@@ -263,7 +265,7 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev)
*- Enable GCB idle
*/
gasket_read_modify_write_64(gasket_dev, APEX_BAR_INDEX,
-
APEX_BAR2_REG_IDLEGENERATOR_IDLEGEN_IDLEREGISTER,
+ idle_gen_reg,
0x0, 1, 32);
/*- Initiate DMA pause */
@@ -395,11 +397,12 @@ static int apex_device_cleanup(struct gasket_dev
*gasket_dev)
u64 scalar_error;
u64 hib_error;
int ret = 0;
+ int status = APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS;
hib_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_USER_HIB_ERROR_STATUS);
scalar_error = gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
-
APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
+ status);
dev_dbg(gasket_dev->dev,
"%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
@@ -584,6 +587,8 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
ulong page_table_ready, msix_table_ready;
int retries = 0;
struct gasket_dev *gasket_dev;
+ int page_table_init = APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT;
+ int msix_table_init = APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT;
ret = pci_enable_device(pci_dev);
if (ret) {
@@ -606,10 +611,10 @@ static int apex_pci_probe(struct pci_dev *pci_dev,
while (retries < APEX_RESET_RETRY) {
page_table_ready =
gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
-
APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_INIT);
+ page_table_init);
msix_table_ready =
gasket_dev_read_64(gasket_dev, APEX_BAR_INDEX,
-
APEX_BAR2_REG_KERNEL_HIB_MSIX_TABLE_INIT);
+ msix_table_init);
if (page_table_ready && msix_table_ready)
break;
schedule_timeout(msecs_to_jiffies(APEX_RESET_DELAY));
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
