Cache long enums as local variables to fit under 80 characters. Fix a
comment character limit warning. The goal is to comply with the kernel
style guide. All style issues were identified by checkpatch.
John B. Wyatt IV (2):
staging: gasket: Fix 4 over 80 char warnings
staging: gasket: Fix comment 75 character limit warning
drivers/staging/gasket/apex_driver.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
--
2.25.1
Fix 75 character limit warning in comment reported by checkpatch.
Reported by checkpatch.
Signed-off-by: John B. Wyatt IV <[email protected]>
---
drivers/staging/gasket/apex_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index f48209ec7d24..5ad15f398893 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -50,8 +50,8 @@
#define NUM_NODES 1
/*
- * The total number of entries in the page table. Should match the value read
- * from the register APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_SIZE.
+ * The total number of entries in the page table. Should match the
+ * value read from the register APEX_BAR2_REG_KERNEL_HIB_PAGE_TABLE_SIZE.
*/
#define APEX_PAGE_TABLE_TOTAL_ENTRIES 8192
--
2.25.1
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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));
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 <[email protected]>
> > ---
> > 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
On Wed, Apr 01, 2020 at 10:36:17PM -0700, John B. Wyatt IV wrote:
> Fix 75 character limit warning in comment reported by checkpatch.
comments are not allowed to go beyond 75 columns now? Is that something
new?
The code is fine as-is, sorry.
thanks,
greg k-h
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 <[email protected]>
> ---
> 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