2023-03-15 12:23:47

by Menna Mahmoud

[permalink] [raw]
Subject: [PATCH 1/2] staging: vme_user: add space around operators

add a space before and after the operator, for readability.

Reported By checkpatch script:

CHECK: spaces preferred around that '+' (ctx:VxV)
+ image->bus_resource.name = kmalloc(VMENAMSIZ+3, GFP_ATOMIC);
^
CHECK: spaces preferred around that '<<' (ctx:VxV)
+ temp_ctl &= ~(3<<4);
^
CHECK: spaces preferred around that '>>' (ctx:VxV)
+ cbar = (cbar & TSI148_CRCSR_CBAR_M)>>3;
^
CHECK: spaces preferred around that '<<' (ctx:VxV)
+ iowrite32be(cbar<<3, bridge->base + TSI148_CBAR);

Signed-off-by: Menna Mahmoud <[email protected]>
---
drivers/staging/vme_user/vme_tsi148.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vme_user/vme_tsi148.c b/drivers/staging/vme_user/vme_tsi148.c
index 482049cfc664..bfa604043355 100644
--- a/drivers/staging/vme_user/vme_tsi148.c
+++ b/drivers/staging/vme_user/vme_tsi148.c
@@ -737,7 +737,7 @@ static int tsi148_alloc_resource(struct vme_master_resource *image,
return 0;

if (!image->bus_resource.name) {
- image->bus_resource.name = kmalloc(VMENAMSIZ+3, GFP_ATOMIC);
+ image->bus_resource.name = kmalloc(VMENAMSIZ + 3, GFP_ATOMIC);
if (!image->bus_resource.name) {
retval = -ENOMEM;
goto err_name;
@@ -983,7 +983,7 @@ static int tsi148_master_set(struct vme_master_resource *image, int enabled,
goto err_aspace;
}

- temp_ctl &= ~(3<<4);
+ temp_ctl &= ~(3 << 4);
if (cycle & VME_SUPER)
temp_ctl |= TSI148_LCSR_OTAT_SUP;
if (cycle & VME_PROG)
@@ -2187,14 +2187,14 @@ static int tsi148_crcsr_init(struct vme_bridge *tsi148_bridge,

/* Ensure that the CR/CSR is configured at the correct offset */
cbar = ioread32be(bridge->base + TSI148_CBAR);
- cbar = (cbar & TSI148_CRCSR_CBAR_M)>>3;
+ cbar = (cbar & TSI148_CRCSR_CBAR_M) >> 3;

vstat = tsi148_slot_get(tsi148_bridge);

if (cbar != vstat) {
cbar = vstat;
dev_info(tsi148_bridge->parent, "Setting CR/CSR offset\n");
- iowrite32be(cbar<<3, bridge->base + TSI148_CBAR);
+ iowrite32be(cbar << 3, bridge->base + TSI148_CBAR);
}
dev_info(tsi148_bridge->parent, "CR/CSR Offset: %d\n", cbar);

--
2.34.1



2023-03-15 12:23:57

by Menna Mahmoud

[permalink] [raw]
Subject: [PATCH 2/2] staging: vme_user: remove unnecessary blank lines

remove unnecessary blank lines before a close brace
as reported by checkpatch script

CHECK: Blank lines aren't necessary before a close brace '}'
+
+}

CHECK: Blank lines aren't necessary before a close brace '}'
+
+}

CHECK: Blank lines aren't necessary before a close brace '}'
+
+ }

CHECK: Blank lines aren't necessary before a close brace '}'
+
+}

CHECK: Blank lines aren't necessary before a close brace '}'
+
+}

Signed-off-by: Menna Mahmoud <[email protected]>
---
drivers/staging/vme_user/vme_tsi148.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/staging/vme_user/vme_tsi148.c b/drivers/staging/vme_user/vme_tsi148.c
index bfa604043355..2f5eafd50934 100644
--- a/drivers/staging/vme_user/vme_tsi148.c
+++ b/drivers/staging/vme_user/vme_tsi148.c
@@ -1023,7 +1023,6 @@ static int tsi148_master_set(struct vme_master_resource *image, int enabled,
err_res:
err_window:
return retval;
-
}

/*
@@ -1741,7 +1740,6 @@ static int tsi148_dma_list_add(struct vme_dma_list *list,
list);
prev->descriptor.dnlau = cpu_to_be32(address_high);
prev->descriptor.dnlal = cpu_to_be32(address_low);
-
}

return 0;
@@ -1773,7 +1771,6 @@ static int tsi148_dma_busy(struct vme_bridge *tsi148_bridge, int channel)
return 0;
else
return 1;
-
}

/*
@@ -2220,7 +2217,6 @@ static int tsi148_crcsr_init(struct vme_bridge *tsi148_bridge,
}

return 0;
-
}

static void tsi148_crcsr_exit(struct vme_bridge *tsi148_bridge,
@@ -2530,7 +2526,6 @@ static int tsi148_probe(struct pci_dev *pdev, const struct pci_device_id *id)
kfree(tsi148_bridge);
err_struct:
return retval;
-
}

static void tsi148_remove(struct pci_dev *pdev)
--
2.34.1


2023-03-15 13:20:18

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: vme_user: remove unnecessary blank lines

On mercoled? 15 marzo 2023 13:21:33 CET Menna Mahmoud wrote:
> remove unnecessary blank lines before a close brace
> as reported by checkpatch script
>
> CHECK: Blank lines aren't necessary before a close brace '}'
> +
> +}
>
> CHECK: Blank lines aren't necessary before a close brace '}'
> +
> +}
>
> CHECK: Blank lines aren't necessary before a close brace '}'
> +
> + }
>
> CHECK: Blank lines aren't necessary before a close brace '}'
> +
> +}
>
> CHECK: Blank lines aren't necessary before a close brace '}'
> +
> +}

There are no valid reasons about copy-pasting that warning 5 times in a row.
Just say that you get that message from checkpatch.pl in five different sites
in the file you are changing.

What if you had had 20 or 30 occurrences of that same "CHECK" in the same
file?

Thanks,

Fabio

> Signed-off-by: Menna Mahmoud <[email protected]>
> ---
> drivers/staging/vme_user/vme_tsi148.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/staging/vme_user/vme_tsi148.c
> b/drivers/staging/vme_user/vme_tsi148.c index bfa604043355..2f5eafd50934
> 100644
> --- a/drivers/staging/vme_user/vme_tsi148.c
> +++ b/drivers/staging/vme_user/vme_tsi148.c
> @@ -1023,7 +1023,6 @@ static int tsi148_master_set(struct
vme_master_resource
> *image, int enabled, err_res:
> err_window:
> return retval;
> -
> }
>
> /*
> @@ -1741,7 +1740,6 @@ static int tsi148_dma_list_add(struct vme_dma_list
> *list, list);
> prev->descriptor.dnlau = cpu_to_be32(address_high);
> prev->descriptor.dnlal = cpu_to_be32(address_low);
> -
> }
>
> return 0;
> @@ -1773,7 +1771,6 @@ static int tsi148_dma_busy(struct vme_bridge
> *tsi148_bridge, int channel) return 0;
> else
> return 1;
> -
> }
>
> /*
> @@ -2220,7 +2217,6 @@ static int tsi148_crcsr_init(struct vme_bridge
> *tsi148_bridge, }
>
> return 0;
> -
> }
>
> static void tsi148_crcsr_exit(struct vme_bridge *tsi148_bridge,
> @@ -2530,7 +2526,6 @@ static int tsi148_probe(struct pci_dev *pdev, const
> struct pci_device_id *id) kfree(tsi148_bridge);
> err_struct:
> return retval;
> -
> }
>
> static void tsi148_remove(struct pci_dev *pdev)
> --
> 2.34.1





2023-03-15 13:25:18

by Menna Mahmoud

[permalink] [raw]
Subject: Re: [PATCH 2/2] staging: vme_user: remove unnecessary blank lines


On ١٥‏/٣‏/٢٠٢٣ ١٥:١٩, Fabio M. De Francesco wrote:
> On mercoledì 15 marzo 2023 13:21:33 CET Menna Mahmoud wrote:
>> remove unnecessary blank lines before a close brace
>> as reported by checkpatch script
>>
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> +
>> +}
>>
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> +
>> +}
>>
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> +
>> + }
>>
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> +
>> +}
>>
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> +
>> +}
> There are no valid reasons about copy-pasting that warning 5 times in a row.
> Just say that you get that message from checkpatch.pl in five different sites
> in the file you are changing.
>
> What if you had had 20 or 30 occurrences of that same "CHECK" in the same
> file?


yes, got your point. I will edit it.


Thanks,

Menna

>
> Thanks,
>
> Fabio
>
>> Signed-off-by: Menna Mahmoud <[email protected]>
>> ---
>> drivers/staging/vme_user/vme_tsi148.c | 5 -----
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/staging/vme_user/vme_tsi148.c
>> b/drivers/staging/vme_user/vme_tsi148.c index bfa604043355..2f5eafd50934
>> 100644
>> --- a/drivers/staging/vme_user/vme_tsi148.c
>> +++ b/drivers/staging/vme_user/vme_tsi148.c
>> @@ -1023,7 +1023,6 @@ static int tsi148_master_set(struct
> vme_master_resource
>> *image, int enabled, err_res:
>> err_window:
>> return retval;
>> -
>> }
>>
>> /*
>> @@ -1741,7 +1740,6 @@ static int tsi148_dma_list_add(struct vme_dma_list
>> *list, list);
>> prev->descriptor.dnlau = cpu_to_be32(address_high);
>> prev->descriptor.dnlal = cpu_to_be32(address_low);
>> -
>> }
>>
>> return 0;
>> @@ -1773,7 +1771,6 @@ static int tsi148_dma_busy(struct vme_bridge
>> *tsi148_bridge, int channel) return 0;
>> else
>> return 1;
>> -
>> }
>>
>> /*
>> @@ -2220,7 +2217,6 @@ static int tsi148_crcsr_init(struct vme_bridge
>> *tsi148_bridge, }
>>
>> return 0;
>> -
>> }
>>
>> static void tsi148_crcsr_exit(struct vme_bridge *tsi148_bridge,
>> @@ -2530,7 +2526,6 @@ static int tsi148_probe(struct pci_dev *pdev, const
>> struct pci_device_id *id) kfree(tsi148_bridge);
>> err_struct:
>> return retval;
>> -
>> }
>>
>> static void tsi148_remove(struct pci_dev *pdev)
>> --
>> 2.34.1
>
>
>

2023-03-15 18:48:02

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: vme_user: add space around operators

Menna Mahmoud wrote:
> add a space before and after the operator, for readability.
>
> Reported By checkpatch script:
>
> CHECK: spaces preferred around that '+' (ctx:VxV)
> + image->bus_resource.name = kmalloc(VMENAMSIZ+3, GFP_ATOMIC);
> ^
> CHECK: spaces preferred around that '<<' (ctx:VxV)
> + temp_ctl &= ~(3<<4);
> ^
> CHECK: spaces preferred around that '>>' (ctx:VxV)
> + cbar = (cbar & TSI148_CRCSR_CBAR_M)>>3;
> ^
> CHECK: spaces preferred around that '<<' (ctx:VxV)
> + iowrite32be(cbar<<3, bridge->base + TSI148_CBAR);

Similar comment to what Fabio made.

But I'll add some formatting comments.

It looks like you have submitted a 2 patch series which fixes all the
checkpatch errors in the vme_user driver?

Kudos for separating out the 2 patches for the 2 different types of
checkpatch errors! And for submitting a series which fixes the entire
driver!

But you should also include a cover letter for your series.

How are you creating this series? Are you using 'git format-patch'?
'b4'? If not using 'b4' I strongly recommend it.[*]

Both of those tools can help with formatting a cover letter and b4 will
help keep track of multiple versions of the series as you fix things.

Ira

[*] https://git.kernel.org/pub/scm/utils/b4/b4.git

2023-03-15 19:15:11

by Menna Mahmoud

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: vme_user: add space around operators


On ١٥‏/٣‏/٢٠٢٣ ٢٠:٤٧, Ira Weiny wrote:
> Menna Mahmoud wrote:
>> add a space before and after the operator, for readability.
>>
>> Reported By checkpatch script:
>>
>> CHECK: spaces preferred around that '+' (ctx:VxV)
>> + image->bus_resource.name = kmalloc(VMENAMSIZ+3, GFP_ATOMIC);
>> ^
>> CHECK: spaces preferred around that '<<' (ctx:VxV)
>> + temp_ctl &= ~(3<<4);
>> ^
>> CHECK: spaces preferred around that '>>' (ctx:VxV)
>> + cbar = (cbar & TSI148_CRCSR_CBAR_M)>>3;
>> ^
>> CHECK: spaces preferred around that '<<' (ctx:VxV)
>> + iowrite32be(cbar<<3, bridge->base + TSI148_CBAR);
> Similar comment to what Fabio made.
>
> But I'll add some formatting comments.
>
> It looks like you have submitted a 2 patch series which fixes all the
> checkpatch errors in the vme_user driver?

yes, each patch for specific error in multi-position.

>
> Kudos for separating out the 2 patches for the 2 different types of
> checkpatch errors! And for submitting a series which fixes the entire
> driver!

to double check that i understood right, you mean I should create one
patch-set for one error?

Because I have already done that in previous patch but Julia commented
on it that no need to create patch-set

for the same error.


> But you should also include a cover letter for your series.
okay, I will include it.
>
> How are you creating this series? Are you using 'git format-patch'?
> 'b4'? If not using 'b4' I strongly recommend it.[*]

yes, used ' git format-patch` but I will try b4.

> Both of those tools can help with formatting a cover letter and b4 will
> help keep track of multiple versions of the series as you fix things.
>
> Ira
>
> [*] https://git.kernel.org/pub/scm/utils/b4/b4.git

Thanks,

Menna


2023-03-16 00:18:50

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: vme_user: add space around operators

Menna Mahmoud wrote:
>
> On ١٥‏/٣‏/٢٠٢٣ ٢٠:٤٧, Ira Weiny wrote:
> > Menna Mahmoud wrote:
> >> add a space before and after the operator, for readability.
> >>
> >> Reported By checkpatch script:
> >>
> >> CHECK: spaces preferred around that '+' (ctx:VxV)
> >> + image->bus_resource.name = kmalloc(VMENAMSIZ+3, GFP_ATOMIC);
> >> ^
> >> CHECK: spaces preferred around that '<<' (ctx:VxV)
> >> + temp_ctl &= ~(3<<4);
> >> ^
> >> CHECK: spaces preferred around that '>>' (ctx:VxV)
> >> + cbar = (cbar & TSI148_CRCSR_CBAR_M)>>3;
> >> ^
> >> CHECK: spaces preferred around that '<<' (ctx:VxV)
> >> + iowrite32be(cbar<<3, bridge->base + TSI148_CBAR);
> > Similar comment to what Fabio made.
> >
> > But I'll add some formatting comments.
> >
> > It looks like you have submitted a 2 patch series which fixes all the
> > checkpatch errors in the vme_user driver?
>
> yes, each patch for specific error in multi-position.
>
> >
> > Kudos for separating out the 2 patches for the 2 different types of
> > checkpatch errors! And for submitting a series which fixes the entire
> > driver!
>
> to double check that i understood right, you mean I should create one
> patch-set for one error?

No what you did was correct. Each patch should fix one type of error. I
only meant you should include a cover letter which explains the group of
patches as a bundle to fix the checkpatch errors of this driver.

https://kernelnewbies.org/PatchSeries

>
> Because I have already done that in previous patch but Julia commented
> on it that no need to create patch-set
>
> for the same error.
>
>
> > But you should also include a cover letter for your series.
> okay, I will include it.
> >
> > How are you creating this series? Are you using 'git format-patch'?
> > 'b4'? If not using 'b4' I strongly recommend it.[*]
>
> yes, used ' git format-patch` but I will try b4.

While I still recomend using b4 check out the --cover-letter option to
format-patch.

--cover-letter generate a cover letter

Ira