Re: [PATCH v3 2/4] staging: media: atomisp: fix style of open brace
On 26/07/20 3:12 pm, Mauro Carvalho Chehab wrote:
> Em Sun, 26 Jul 2020 14:35:12 +0530
> Rohit K Bharadwaj escreveu:
>
>> this patch fixes style of open brace after functions and if statements
>>
>> Signed-off-by: Rohit K Bharadwaj
>
>> @@ -119,8 +121,9 @@ sh_css_load_blob_info(const char *fw, const struct
>> ia_css_fw_info *bi,
>> blob = (const unsigned char *)fw + bi->blob.offset;
>>
>> /* sanity check */
>> -if (bi->blob.size != bi->blob.text_size + bi->blob.icache_size +
>> bi->blob.data_size + bi->blob.padding_size)
>> -{
>> +if (bi->blob.size !=
>> +bi->blob.text_size + bi->blob.icache_size +
>> +bi->blob.data_size + bi->blob.padding_size) {
>
>
> Please adjust the alignments here. I'm pretty sure checkpatch --strict
> will end complaining with the above, as the first line ends with "=".
>
> I would, instead, code it as:
>
> if (bi->blob.size != bi->blob.text_size + bi->blob.icache_size +
>bi->blob.data_size + bi->blob.padding_size) {
>
>
> Thanks,
> Mauro
>
sure sir, do I have to make a v4 of the patch and send all the patches in this
series again or should I just send this one particular patch again?
Re: [PATCH v3 1/4] staging: media: atomisp: fix style of block comments
On 26/07/20 2:42 pm, Greg KH wrote:
> On Sun, Jul 26, 2020 at 02:35:10PM +0530, Rohit K Bharadwaj wrote:
>> this patch fixes the coding style of block comments.
>>
>> Signed-off-by: Rohit K Bharadwaj
>> ---
>> v3: change patch subject prefix
>> v2: split patch into sequence of patches
>> v1: fix all coding style issues in single patch
>>
>> .../media/atomisp/pci/sh_css_firmware.c | 28 +--
>> 1 file changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
>> b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
>> index d4ab15b6d1ac..2907aead98b7 100644
>> --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
>> +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
>> @@ -51,9 +51,12 @@ struct fw_param {
>>
>> static struct firmware_header *firmware_header;
>>
>> -/* The string STR is a place holder
>> +/*
>> + * The string STR is a place holder
>> * which will be replaced with the actual RELEASE_VERSION
>> - * during package generation. Please do not modify */
>> + * during package generation. Please do not modify
>> + */
>> +
>
> Why the blank line?
>
Sir, I thought that it would improve the readability of the code and would help
easily distinguish comments from the variable declaration part, and also since
there was a blank line between the start of comment and the static struct
firmware_header variable, I thought of putting a blank line after the comment
end as well to maintain symmetry.
[PATCH v3 4/4] staging: media: atomisp: fix line length exceeds
this patch fixes the line length exceeded error from checkpatch.pl
Signed-off-by: Rohit K Bharadwaj
---
v3: change patch subject prefix
v2: split patch into sequence of patches
v1: fix all coding style issues in single patch
drivers/staging/media/atomisp/pci/sh_css_firmware.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index 2208453cbef0..6e8f45e38804 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -243,7 +243,8 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
firmware_header = (struct firmware_header *)fw_data;
file_header = _header->file_header;
binaries = _header->binary_header;
- strscpy(FW_rel_ver_name, file_header->version,
min(sizeof(FW_rel_ver_name), sizeof(file_header->version)));
+ strscpy(FW_rel_ver_name, file_header->version,
+ min(sizeof(FW_rel_ver_name), sizeof(file_header->version)));
ret = sh_css_check_firmware_version(dev, fw_data);
if (ret) {
IA_CSS_ERROR("CSS code version (%s) and firmware version (%s)
mismatch!",
--
2.25.1
[PATCH v3 3/4] staging: media: atomisp: fix trailing statement of if
this patch fixes the error from checkpatch.pl which says that trailing statements after if keyword to be on next line Signed-off-by: Rohit K Bharadwaj --- v3: change patch subject prefix v2: split patch into sequence of patches v1: fix all coding style issues in single patch drivers/staging/media/atomisp/pci/sh_css_firmware.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/sh_css_firmware.c index 988785ab6c95..2208453cbef0 100644 --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c @@ -115,7 +115,8 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi, return -EINVAL; /* Special case: only one binary in fw */ - if (!bi) bi = (const struct ia_css_fw_info *)fw; + if (!bi) + bi = (const struct ia_css_fw_info *)fw; name = fw + bi->blob.prog_name_offset; blob = (const unsigned char *)fw + bi->blob.offset; -- 2.25.1
[PATCH v3 2/4] staging: media: atomisp: fix style of open brace
this patch fixes style of open brace after functions and if statements
Signed-off-by: Rohit K Bharadwaj
---
v3: change patch subject prefix
v2: split patch into sequence of patches
v1: fix all coding style issues in single patch
.../media/atomisp/pci/sh_css_firmware.c | 29 +--
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index 2907aead98b7..988785ab6c95 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -81,7 +81,8 @@ char *sh_css_get_fw_version(void)
/* Setup sp/sp1 binary */
static int
setup_binary(struct ia_css_fw_info *fw, const char *fw_data,
-struct ia_css_fw_info *sh_css_fw, unsigned int binary_id) {
+struct ia_css_fw_info *sh_css_fw, unsigned int binary_id)
+{
const char *blob_data;
if ((!fw) || (!fw_data))
@@ -105,7 +106,8 @@ setup_binary(struct ia_css_fw_info *fw, const char *fw_data,
int
sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi,
struct ia_css_blob_descr *bd,
- unsigned int index) {
+ unsigned int index)
+{
const char *name;
const unsigned char *blob;
@@ -119,8 +121,9 @@ sh_css_load_blob_info(const char *fw, const struct
ia_css_fw_info *bi,
blob = (const unsigned char *)fw + bi->blob.offset;
/* sanity check */
- if (bi->blob.size != bi->blob.text_size + bi->blob.icache_size +
bi->blob.data_size + bi->blob.padding_size)
- {
+ if (bi->blob.size !=
+ bi->blob.text_size + bi->blob.icache_size +
+ bi->blob.data_size + bi->blob.padding_size) {
/* sanity check, note the padding bytes added for section to
DDR alignment */
return -EINVAL;
}
@@ -131,21 +134,18 @@ sh_css_load_blob_info(const char *fw, const struct
ia_css_fw_info *bi,
bd->blob = blob;
bd->header = *bi;
- if (bi->type == ia_css_isp_firmware || bi->type == ia_css_sp_firmware)
- {
+ if (bi->type == ia_css_isp_firmware || bi->type == ia_css_sp_firmware) {
char *namebuffer;
namebuffer = kstrdup(name, GFP_KERNEL);
if (!namebuffer)
return -ENOMEM;
bd->name = fw_minibuffer[index].name = namebuffer;
- } else
- {
+ } else {
bd->name = name;
}
- if (bi->type == ia_css_isp_firmware)
- {
+ if (bi->type == ia_css_isp_firmware) {
size_t paramstruct_size = sizeof(struct ia_css_memory_offsets);
size_t configstruct_size = sizeof(struct
ia_css_config_memory_offsets);
size_t statestruct_size = sizeof(struct
ia_css_state_memory_offsets);
@@ -226,7 +226,8 @@ static const char * const fw_acc_type_name[] = {
int
sh_css_load_firmware(struct device *dev, const char *fw_data,
-unsigned int fw_size) {
+unsigned int fw_size)
+{
unsigned int i;
struct ia_css_fw_info *binaries;
struct sh_css_fw_bi_file_h *file_header;
@@ -260,8 +261,7 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
sh_css_num_binaries = file_header->binary_nr;
/* Only allocate memory for ISP blob info */
- if (sh_css_num_binaries > NUM_OF_SPS)
- {
+ if (sh_css_num_binaries > NUM_OF_SPS) {
sh_css_blob_info = kmalloc(
(sh_css_num_binaries - NUM_OF_SPS) *
sizeof(*sh_css_blob_info), GFP_KERNEL);
@@ -276,8 +276,7 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
if (!fw_minibuffer)
return -ENOMEM;
- for (i = 0; i < sh_css_num_binaries; i++)
- {
+ for (i = 0; i < sh_css_num_binaries; i++) {
struct ia_css_fw_info *bi = [i];
/*
* note: the var below is made static as it is quite large;
--
2.25.1
[PATCH v3 1/4] staging: media: atomisp: fix style of block comments
this patch fixes the coding style of block comments.
Signed-off-by: Rohit K Bharadwaj
---
v3: change patch subject prefix
v2: split patch into sequence of patches
v1: fix all coding style issues in single patch
.../media/atomisp/pci/sh_css_firmware.c | 28 +--
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index d4ab15b6d1ac..2907aead98b7 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -51,9 +51,12 @@ struct fw_param {
static struct firmware_header *firmware_header;
-/* The string STR is a place holder
+/*
+ * The string STR is a place holder
* which will be replaced with the actual RELEASE_VERSION
- * during package generation. Please do not modify */
+ * during package generation. Please do not modify
+ */
+
static const char *isp2400_release_version =
STR(irci_stable_candrpv_0415_20150521_0458);
static const char *isp2401_release_version = STR(irci_ecr -
master_20150911_0724);
@@ -276,10 +279,11 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
for (i = 0; i < sh_css_num_binaries; i++)
{
struct ia_css_fw_info *bi = [i];
- /* note: the var below is made static as it is quite large;
- if it is not static it ends up on the stack which could
- cause issues for drivers
- */
+ /*
+* note: the var below is made static as it is quite large;
+* if it is not static it ends up on the stack which could
+* cause issues for drivers
+*/
static struct ia_css_blob_descr bd;
int err;
@@ -333,7 +337,11 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
return err;
} else {
- /* All subsequent binaries (including bootloaders)
(i>NUM_OF_SPS) are ISP firmware */
+ /*
+* All subsequent binaries
+* (including bootloaders) (i>NUM_OF_SPS)
+* are ISP firmware
+*/
if (i < NUM_OF_SPS)
return -EINVAL;
@@ -374,8 +382,10 @@ ia_css_ptr
sh_css_load_blob(const unsigned char *blob, unsigned int size)
{
ia_css_ptr target_addr = hmm_alloc(size, HMM_BO_PRIVATE, 0, NULL, 0);
- /* this will allocate memory aligned to a DDR word boundary which
- is required for the CSS DMA to read the instructions. */
+ /*
+* this will allocate memory aligned to a DDR word boundary which
+* is required for the CSS DMA to read the instructions.
+*/
assert(blob);
if (target_addr)
--
2.25.1
Re: [PATCH v2 1/4] fix style of block comments
On 26/07/20 2:18 pm, Rohit K Bharadwaj wrote:
> this patch fixes the coding style of block comments.
>
> Signed-off-by: Rohit K Bharadwaj
> ---
> v2: split patch into sequence of patches
> v1: fix all coding style issues in single patch
>
> .../media/atomisp/pci/sh_css_firmware.c | 28 +--
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
> b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
> index d4ab15b6d1ac..2907aead98b7 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
> @@ -51,9 +51,12 @@ struct fw_param {
>
> static struct firmware_header *firmware_header;
>
> -/* The string STR is a place holder
> +/*
> + * The string STR is a place holder
> * which will be replaced with the actual RELEASE_VERSION
> - * during package generation. Please do not modify */
> + * during package generation. Please do not modify
> + */
> +
> static const char *isp2400_release_version =
> STR(irci_stable_candrpv_0415_20150521_0458);
> static const char *isp2401_release_version = STR(irci_ecr -
> master_20150911_0724);
>
> @@ -276,10 +279,11 @@ sh_css_load_firmware(struct device *dev, const char
> *fw_data,
> for (i = 0; i < sh_css_num_binaries; i++)
> {
> struct ia_css_fw_info *bi = [i];
> - /* note: the var below is made static as it is quite large;
> -if it is not static it ends up on the stack which could
> -cause issues for drivers
> - */
> + /*
> + * note: the var below is made static as it is quite large;
> + * if it is not static it ends up on the stack which could
> + * cause issues for drivers
> + */
> static struct ia_css_blob_descr bd;
> int err;
>
> @@ -333,7 +337,11 @@ sh_css_load_firmware(struct device *dev, const char
> *fw_data,
> return err;
>
> } else {
> - /* All subsequent binaries (including bootloaders)
> (i>NUM_OF_SPS) are ISP firmware */
> + /*
> + * All subsequent binaries
> + * (including bootloaders) (i>NUM_OF_SPS)
> + * are ISP firmware
> + */
> if (i < NUM_OF_SPS)
> return -EINVAL;
>
> @@ -374,8 +382,10 @@ ia_css_ptr
> sh_css_load_blob(const unsigned char *blob, unsigned int size)
> {
> ia_css_ptr target_addr = hmm_alloc(size, HMM_BO_PRIVATE, 0, NULL, 0);
> - /* this will allocate memory aligned to a DDR word boundary which
> -is required for the CSS DMA to read the instructions. */
> + /*
> + * this will allocate memory aligned to a DDR word boundary which
> + * is required for the CSS DMA to read the instructions.
> + */
>
> assert(blob);
> if (target_addr)
>
I'm really sorry for the spam, please excuse me, I realized that I didn't
change the subject prefix so will send another version, apologies for any
inconvenience caused.
[PATCH v2 1/4] fix style of block comments
this patch fixes the coding style of block comments.
Signed-off-by: Rohit K Bharadwaj
---
v2: split patch into sequence of patches
v1: fix all coding style issues in single patch
.../media/atomisp/pci/sh_css_firmware.c | 28 +--
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index d4ab15b6d1ac..2907aead98b7 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -51,9 +51,12 @@ struct fw_param {
static struct firmware_header *firmware_header;
-/* The string STR is a place holder
+/*
+ * The string STR is a place holder
* which will be replaced with the actual RELEASE_VERSION
- * during package generation. Please do not modify */
+ * during package generation. Please do not modify
+ */
+
static const char *isp2400_release_version =
STR(irci_stable_candrpv_0415_20150521_0458);
static const char *isp2401_release_version = STR(irci_ecr -
master_20150911_0724);
@@ -276,10 +279,11 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
for (i = 0; i < sh_css_num_binaries; i++)
{
struct ia_css_fw_info *bi = [i];
- /* note: the var below is made static as it is quite large;
- if it is not static it ends up on the stack which could
- cause issues for drivers
- */
+ /*
+* note: the var below is made static as it is quite large;
+* if it is not static it ends up on the stack which could
+* cause issues for drivers
+*/
static struct ia_css_blob_descr bd;
int err;
@@ -333,7 +337,11 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
return err;
} else {
- /* All subsequent binaries (including bootloaders)
(i>NUM_OF_SPS) are ISP firmware */
+ /*
+* All subsequent binaries
+* (including bootloaders) (i>NUM_OF_SPS)
+* are ISP firmware
+*/
if (i < NUM_OF_SPS)
return -EINVAL;
@@ -374,8 +382,10 @@ ia_css_ptr
sh_css_load_blob(const unsigned char *blob, unsigned int size)
{
ia_css_ptr target_addr = hmm_alloc(size, HMM_BO_PRIVATE, 0, NULL, 0);
- /* this will allocate memory aligned to a DDR word boundary which
- is required for the CSS DMA to read the instructions. */
+ /*
+* this will allocate memory aligned to a DDR word boundary which
+* is required for the CSS DMA to read the instructions.
+*/
assert(blob);
if (target_addr)
--
2.25.1
[PATCH] staging: media: atomisp: fix code style issues
this patch fixes the warnings and errors generated after running checkpatch.pl
on pci/sh_css_firmware.c
Signed-off-by: Rohit K Bharadwaj
---
.../media/atomisp/pci/sh_css_firmware.c | 62 +++
1 file changed, 36 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index d4ab15b6d1ac..244c7c7780a3 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -51,9 +51,11 @@ struct fw_param {
static struct firmware_header *firmware_header;
-/* The string STR is a place holder
+/*
+ * The string STR is a place holder
* which will be replaced with the actual RELEASE_VERSION
- * during package generation. Please do not modify */
+ * during package generation. Please do not modify
+ */
static const char *isp2400_release_version =
STR(irci_stable_candrpv_0415_20150521_0458);
static const char *isp2401_release_version = STR(irci_ecr -
master_20150911_0724);
@@ -78,7 +80,8 @@ char *sh_css_get_fw_version(void)
/* Setup sp/sp1 binary */
static int
setup_binary(struct ia_css_fw_info *fw, const char *fw_data,
-struct ia_css_fw_info *sh_css_fw, unsigned int binary_id) {
+struct ia_css_fw_info *sh_css_fw, unsigned int binary_id)
+{
const char *blob_data;
if ((!fw) || (!fw_data))
@@ -102,7 +105,8 @@ setup_binary(struct ia_css_fw_info *fw, const char *fw_data,
int
sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi,
struct ia_css_blob_descr *bd,
- unsigned int index) {
+ unsigned int index)
+{
const char *name;
const unsigned char *blob;
@@ -110,14 +114,16 @@ sh_css_load_blob_info(const char *fw, const struct
ia_css_fw_info *bi,
return -EINVAL;
/* Special case: only one binary in fw */
- if (!bi) bi = (const struct ia_css_fw_info *)fw;
+ if (!bi)
+ bi = (const struct ia_css_fw_info *)fw;
name = fw + bi->blob.prog_name_offset;
blob = (const unsigned char *)fw + bi->blob.offset;
/* sanity check */
- if (bi->blob.size != bi->blob.text_size + bi->blob.icache_size +
bi->blob.data_size + bi->blob.padding_size)
- {
+ if (bi->blob.size !=
+ bi->blob.text_size + bi->blob.icache_size +
+ bi->blob.data_size + bi->blob.padding_size) {
/* sanity check, note the padding bytes added for section to
DDR alignment */
return -EINVAL;
}
@@ -128,21 +134,18 @@ sh_css_load_blob_info(const char *fw, const struct
ia_css_fw_info *bi,
bd->blob = blob;
bd->header = *bi;
- if (bi->type == ia_css_isp_firmware || bi->type == ia_css_sp_firmware)
- {
+ if (bi->type == ia_css_isp_firmware || bi->type == ia_css_sp_firmware) {
char *namebuffer;
namebuffer = kstrdup(name, GFP_KERNEL);
if (!namebuffer)
return -ENOMEM;
bd->name = fw_minibuffer[index].name = namebuffer;
- } else
- {
+ } else {
bd->name = name;
}
- if (bi->type == ia_css_isp_firmware)
- {
+ if (bi->type == ia_css_isp_firmware) {
size_t paramstruct_size = sizeof(struct ia_css_memory_offsets);
size_t configstruct_size = sizeof(struct
ia_css_config_memory_offsets);
size_t statestruct_size = sizeof(struct
ia_css_state_memory_offsets);
@@ -223,7 +226,8 @@ static const char * const fw_acc_type_name[] = {
int
sh_css_load_firmware(struct device *dev, const char *fw_data,
-unsigned int fw_size) {
+unsigned int fw_size)
+{
unsigned int i;
struct ia_css_fw_info *binaries;
struct sh_css_fw_bi_file_h *file_header;
@@ -238,7 +242,8 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
firmware_header = (struct firmware_header *)fw_data;
file_header = _header->file_header;
binaries = _header->binary_header;
- strscpy(FW_rel_ver_name, file_header->version,
min(sizeof(FW_rel_ver_name), sizeof(file_header->version)));
+ strscpy(FW_rel_ver_name, file_header->version,
+ min(sizeof(FW_rel_ver_name), sizeof(file_header->version)));
ret = sh_css_check_firmware_version(dev, fw_data);
if (ret) {
IA_CSS_ERROR("CSS code version (%s) and firmware version (%s)
mismatch!",
@@ -257,8 +262,7 @@ sh_css_load_firmware(struct device *dev, const char
*fw_data,
sh_css_num_binaries = file_header->binary_nr;
/* Only allocate memory for ISP blob info */
- if (sh_css_num_binaries > NUM_OF_SPS)
