Re: [PATCH RESEND 0/5] staging: media: atomisp: code formatting changes

[Deepak R Varma]

On Fri, May 21, 2021 at 12:30:41AM +0530, Deepak R Varma wrote:
> This patch set overall improves the code organisation and readability of
> the files of atomisp drivers. There are several complaints reported by
> checkpatch including ERROR and WARNING types on the files under atomisp/pci
> directory.
> 
> The changes are proposed on a per file basis since there are many
> issues to be addressed in each individual file. The patches are built
> on the media_tree/for-v5.14-out1 tree/branch.

Hi All,
I have only received one comment on one of the patch of this patch set. I
have not seen any comment or ack on the other patches.

Will you review and share feedback on the patches please.

Thank you,
deepak.

> 
> 
> Deepak R Varma (5):
>   staging: media: atomisp: code formatting changes atomisp_csi2.c
>   staging: media: atomisp: code formatting changes sh_css_mipi.c
>   staging: media: atomisp: code formatting changes sh_css_params.c
>   staging: media: atomisp: code formatting changes sh_css_sp.c
>   staging: media: atomisp: code formatting changes sh_css_version.c
> 
>  .../staging/media/atomisp/pci/atomisp_csi2.c  |  72 +-
>  .../staging/media/atomisp/pci/sh_css_mipi.c   | 170 ++--
>  .../staging/media/atomisp/pci/sh_css_params.c | 929 +-
>  drivers/staging/media/atomisp/pci/sh_css_sp.c | 471 -
>  .../media/atomisp/pci/sh_css_version.c|   4 +-
>  5 files changed, 754 insertions(+), 892 deletions(-)
> 
> -- 
> 2.30.2
> 


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/5] staging: media: atomisp: code formatting changes sh_css_params.c

[Deepak R Varma]

On Fri, May 21, 2021 at 01:52:22PM +0300, Dan Carpenter wrote:
> On Sat, May 01, 2021 at 12:16:07PM +0530, Deepak R Varma wrote:
> > @@ -1562,8 +1544,10 @@ ia_css_isp_3a_statistics_map_allocate(
> > base_ptr = me->data_ptr;
> >  
> > me->size = isp_stats->size;
> > -   /* GCC complains when we assign a char * to a void *, so these
> > -* casts are necessary unfortunately. */
> > +   /*
> > +* GCC complains when we assign a char * to a void *, so these
> > +* casts are necessary unfortunately.
> > +*/
> 
> Not related to your patch, but assigning a char pointer to a void
> pointer is fine and GCC will not complain.  The problem is that
> me->dmem_stats is not a void pointer.  Someone should delete that
> nonsense comment.

I agree. Well caught. I will remove these comments and send v2. Waiting for 
feedback
on other patches of this series.

deepak.

> 
> > me->dmem_stats= (void *)base_ptr;
> > me->vmem_stats_hi = (void *)(base_ptr + isp_stats->dmem_size);
> > me->vmem_stats_lo = (void *)(base_ptr + isp_stats->dmem_size +
> 
> regards,
> dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: media: atomisp: pci: reposition braces as per coding style

[Deepak R Varma]

On Fri, Apr 30, 2021 at 10:48:12PM +0530, Deepak R Varma wrote:
> On Fri, Apr 30, 2021 at 08:12:41PM +0300, Sakari Ailus wrote:
> > Hi Deepak,
> > 
> > On Fri, Apr 30, 2021 at 10:34:37PM +0530, Deepak R Varma wrote:
> > > On Fri, Apr 30, 2021 at 07:33:27PM +0300, Sakari Ailus wrote:
> > > > Hi Deepak,
> > > > 
> > > > If you're touching all these lines, I might do a little more. Please see
> > > > the comments below.
> > > > 
> > > Hello Sakari,
> > > I can definitely include other changes, but then it will be many different
> > > types of changes into a single patch. Will that be okay?
> > > 
> > > I was planning to address one issue per patch as I think the volume of
> > > change is going to be high.  I mentioned that in the notes section of the 
> > > patch
> > > message.
> > 
> > I think I'd split the patch into smaller chunks if the result becomes too
> > big but I don't think it's necessary yet.
> > 
> > Splitting different kinds of simple cleanups into several patches takes
> > longer time to review when they're touching the same piece of code. As the
> > chunks in these patches have virtually no dependencies to other chunks,
> > it's fine to do several kinds of cleanups at once.
> 
> Okay, sure. That sounds good to me. I will include other related
> improvements in the same area and send split patch set accordingly. I will
> include this patch in the patch set.

Hello Sakari and all,
I have sent in the changes as suggested in a patch set with 5 individual
patches. I will wait for your review and feedback before I work on other
files.

Thank you,
deepak.

> 
> Thank you for the guidance.
> deepak.
> 
> > 
> > -- 
> > Kind regards,
> > 
> > Sakari Ailus


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/5] staging: media: atomisp: code formatting changes sh_css_version.c

[Deepak R Varma]

Trivial code reformatting changes done according to the coding
style guidelines. These code changes overall improve code readability
and also address chackpatch complaints.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/media/atomisp/pci/sh_css_version.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_version.c 
b/drivers/staging/media/atomisp/pci/sh_css_version.c
index fa6de61e4995..f038c61a8499 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_version.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_version.c
@@ -20,8 +20,8 @@
 #include "ia_css_err.h"
 #include "sh_css_firmware.h"
 
-int
-ia_css_get_version(char *version, int max_size) {
+int ia_css_get_version(char *version, int max_size)
+{
char *css_version;
 
if (!IS_ISP2401)
-- 
2.30.2



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/5] staging: media: atomisp: code formatting changes sh_css_sp.c

[Deepak R Varma]

Several trivial code reformatting changes done according to the coding
style guidelines. These code changes overall improve code organisation
and readability and also address many chackpatch error, warning and
check complaints.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/media/atomisp/pci/sh_css_sp.c | 471 --
 1 file changed, 203 insertions(+), 268 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c 
b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index 02f5a73b4096..b2859dcf9ac6 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -91,14 +91,12 @@ static void
 initialize_stage_frames(struct ia_css_frames_sp *frames);
 
 /* This data is stored every frame */
-void
-store_sp_group_data(void)
+void store_sp_group_data(void)
 {
per_frame_data.sp_group_addr = sh_css_store_sp_group_to_ddr();
 }
 
-static void
-copy_isp_stage_to_sp_stage(void)
+static void copy_isp_stage_to_sp_stage(void)
 {
/* [WW07.5]type casting will cause potential issues */
sh_css_sp_stage.num_stripes = (uint8_t)
@@ -109,7 +107,8 @@ copy_isp_stage_to_sp_stage(void)
sh_css_isp_stage.binary_info.iterator.row_stripes_overlap_lines;
sh_css_sp_stage.top_cropping = (uint16_t)
   
sh_css_isp_stage.binary_info.pipeline.top_cropping;
-   /* moved to sh_css_sp_init_stage
+   /*
+* moved to sh_css_sp_init_stage
   sh_css_sp_stage.enable.vf_output =
   sh_css_isp_stage.binary_info.enable.vf_veceven ||
   sh_css_isp_stage.binary_info.num_output_pins > 1;
@@ -118,9 +117,8 @@ copy_isp_stage_to_sp_stage(void)
sh_css_sp_stage.enable.s3a = sh_css_isp_stage.binary_info.enable.s3a;
 }
 
-void
-store_sp_stage_data(enum ia_css_pipe_id id, unsigned int pipe_num,
-   unsigned int stage)
+void store_sp_stage_data(enum ia_css_pipe_id id, unsigned int pipe_num,
+unsigned int stage)
 {
unsigned int thread_id;
 
@@ -136,8 +134,7 @@ store_sp_stage_data(enum ia_css_pipe_id id, unsigned int 
pipe_num,
sh_css_sp_stage.program_input_circuit = false;
 }
 
-static void
-store_sp_per_frame_data(const struct ia_css_fw_info *fw)
+static void store_sp_per_frame_data(const struct ia_css_fw_info *fw)
 {
unsigned int HIVE_ADDR_sp_per_frame_data = 0;
 
@@ -154,16 +151,13 @@ store_sp_per_frame_data(const struct ia_css_fw_info *fw)
return;
}
 
-   sp_dmem_store(SP0_ID,
- (unsigned int)sp_address_of(sp_per_frame_data),
- _frame_data,
- sizeof(per_frame_data));
+   sp_dmem_store(SP0_ID, (unsigned int)sp_address_of(sp_per_frame_data),
+ _frame_data, sizeof(per_frame_data));
 }
 
-static void
-sh_css_store_sp_per_frame_data(enum ia_css_pipe_id pipe_id,
-  unsigned int pipe_num,
-  const struct ia_css_fw_info *sp_fw)
+static void sh_css_store_sp_per_frame_data(enum ia_css_pipe_id pipe_id,
+  unsigned int pipe_num,
+  const struct ia_css_fw_info *sp_fw)
 {
if (!sp_fw)
sp_fw = _css_sp_fw;
@@ -175,8 +169,7 @@ sh_css_store_sp_per_frame_data(enum ia_css_pipe_id pipe_id,
 
 #if SP_DEBUG != SP_DEBUG_NONE
 
-void
-sh_css_sp_get_debug_state(struct sh_css_sp_debug_state *state)
+void sh_css_sp_get_debug_state(struct sh_css_sp_debug_state *state)
 {
const struct ia_css_fw_info *fw = _css_sp_fw;
unsigned int HIVE_ADDR_sp_output = fw->info.sp.output;
@@ -193,10 +186,9 @@ sh_css_sp_get_debug_state(struct sh_css_sp_debug_state 
*state)
 
 #endif
 
-void
-sh_css_sp_start_binary_copy(unsigned int pipe_num,
-   struct ia_css_frame *out_frame,
-   unsigned int two_ppc)
+void sh_css_sp_start_binary_copy(unsigned int pipe_num,
+struct ia_css_frame *out_frame,
+unsigned int two_ppc)
 {
enum ia_css_pipe_id pipe_id;
unsigned int thread_id;
@@ -284,8 +276,10 @@ sh_css_sp_start_raw_copy(struct ia_css_frame *out_frame,
pipe->copy.raw.max_input_width = max_input_width;
pipe->num_stages = 1;
pipe->pipe_id = pipe_id;
-   /* TODO: next indicates from which queues parameters need to be
-sampled, needs checking/improvement */
+   /*
+* TODO: next indicates from which queues parameters need to be
+* sampled, needs checking/improvement
+*/
if (pipe_conf_override == SH_CSS_PIPE_CONFIG_OVRD_NO_OVRD)
pipe->pipe_config =
(SH_CSS_PIPE_CONFIG_SAMPLE_PARAMS << thread_id);
@@ -321,10 +315,10 @@ sh_css_sp_start_raw_copy(struct ia_css_frame *out_frame,
ia_css_debug_pipe_graph_du

[PATCH 3/5] staging: media: atomisp: code formatting changes sh_css_params.c

[Deepak R Varma]

Several trivial code reformatting changes done according to the coding
style guidelines. These code changes overall improve code organisation
and readability and also address many chackpatch error, warning and
check complaints.

Signed-off-by: Deepak R Varma 
---
 .../staging/media/atomisp/pci/sh_css_params.c | 929 +-
 1 file changed, 438 insertions(+), 491 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c 
b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 644e14575987..cbb593580dc8 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -120,8 +120,10 @@
(SH_CSS_MORPH_TABLE_ELEM_BYTES * (binary)->morph_tbl_aligned_width * \
 (binary)->morph_tbl_height)
 
-/* We keep a second copy of the ptr struct for the SP to access.
-   Again, this would not be necessary on the chip. */
+/*
+ * We keep a second copy of the ptr struct for the SP to access.
+ * Again, this would not be necessary on the chip.
+ */
 static ia_css_ptr sp_ddr_ptrs;
 
 /* sp group address on DDR */
@@ -667,7 +669,6 @@ static const struct ia_css_dz_config default_dz_config = {
HRT_GDC_N,
HRT_GDC_N,
{
-   \
{0, 0}, \
{0, 0}, \
}
@@ -690,27 +691,23 @@ struct ia_css_isp_skc_dvs_statistics {
ia_css_ptr p_data;
 };
 
-static int
-ref_sh_css_ddr_address_map(
-struct sh_css_ddr_address_map *map,
-struct sh_css_ddr_address_map *out);
+static int ref_sh_css_ddr_address_map(struct sh_css_ddr_address_map *map,
+ struct sh_css_ddr_address_map *out);
 
-static int
-write_ia_css_isp_parameter_set_info_to_ddr(
-struct ia_css_isp_parameter_set_info *me,
-ia_css_ptr *out);
+static int write_ia_css_isp_parameter_set_info_to_ddr(
+   struct ia_css_isp_parameter_set_info *me,
+   ia_css_ptr *out);
 
 static int
 free_ia_css_isp_parameter_set_info(ia_css_ptr ptr);
 
-static int
-sh_css_params_write_to_ddr_internal(
-struct ia_css_pipe *pipe,
-unsigned int pipe_id,
-struct ia_css_isp_parameters *params,
-const struct ia_css_pipeline_stage *stage,
-struct sh_css_ddr_address_map *ddr_map,
-struct sh_css_ddr_address_map_size *ddr_map_size);
+static int sh_css_params_write_to_ddr_internal(
+   struct ia_css_pipe *pipe,
+   unsigned int pipe_id,
+   struct ia_css_isp_parameters *params,
+   const struct ia_css_pipeline_stage *stage,
+   struct sh_css_ddr_address_map *ddr_map,
+   struct sh_css_ddr_address_map_size *ddr_map_size);
 
 static int
 sh_css_create_isp_params(struct ia_css_stream *stream,
@@ -729,34 +726,30 @@ sh_css_init_isp_params_from_config(struct ia_css_pipe 
*pipe,
   struct ia_css_pipe *pipe_in);
 
 static int
-sh_css_set_global_isp_config_on_pipe(
-struct ia_css_pipe *curr_pipe,
-const struct ia_css_isp_config *config,
-struct ia_css_pipe *pipe);
+sh_css_set_global_isp_config_on_pipe(struct ia_css_pipe *curr_pipe,
+const struct ia_css_isp_config *config,
+struct ia_css_pipe *pipe);
 
 #if defined(SH_CSS_ENABLE_PER_FRAME_PARAMS)
-static int
-sh_css_set_per_frame_isp_config_on_pipe(
-struct ia_css_stream *stream,
-const struct ia_css_isp_config *config,
-struct ia_css_pipe *pipe);
+static int sh_css_set_per_frame_isp_config_on_pipe(
+   struct ia_css_stream *stream,
+   const struct ia_css_isp_config *config,
+   struct ia_css_pipe *pipe);
 #endif
 
-static int
-sh_css_update_uds_and_crop_info_based_on_zoom_region(
-const struct ia_css_binary_info *info,
-const struct ia_css_frame_info *in_frame_info,
-const struct ia_css_frame_info *out_frame_info,
-const struct ia_css_resolution *dvs_env,
-const struct ia_css_dz_config *zoom,
-const struct ia_css_vector *motion_vector,
-struct sh_css_uds_info *uds,   /* out */
-struct sh_css_crop_pos *sp_out_crop_pos,   /* out */
-struct ia_css_resolution pipe_in_res,
-bool enable_zoom);
-
-ia_css_ptr
-sh_css_params_ddr_address_map(void)
+static int sh_css_update_uds_and_crop_info_based_on_zoom_region(
+   const struct ia_css_binary_info *info,
+   const struct ia_css_frame_info *in_frame_info,
+   const struct ia_css_frame_info *out_frame_info,
+   const struct ia_css_resolution *dvs_env,
+   const struct ia_css_dz_config *zoom,
+   const struct ia_css_vector *motion_vector,
+   struct sh_css_uds_info *uds,/* out */
+   struct sh_css_crop_pos *sp_out_crop_pos,/* out */
+   struct ia_css_resolution pipe_in_res,
+   bool enable_zoom);
+
+ia_css_ptr sh_css_params_ddr_address_ma

[PATCH 2/5] staging: media: atomisp: code formatting changes sh_css_mipi.c

[Deepak R Varma]

Several trivial code reformatting changes done according to the coding
style guidelines. These code changes overall improve code organisation
and readability and also address many chackpatch error, warning and
check complaints.

Signed-off-by: Deepak R Varma 
---
 .../staging/media/atomisp/pci/sh_css_mipi.c   | 170 --
 1 file changed, 76 insertions(+), 94 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c 
b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index 3f34cc81be87..1ae6c07d4199 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -37,7 +37,7 @@ ref_count_mipi_allocation[N_CSI_PORTS]; /* Initialized in 
mipi_init */
  * Check if a source port or TPG/PRBS ID is valid
  */
 static bool ia_css_mipi_is_source_port_valid(struct ia_css_pipe *pipe,
-   unsigned int *pport)
+unsigned int *pport)
 {
bool ret = true;
unsigned int port = 0;
@@ -74,7 +74,8 @@ static bool ia_css_mipi_is_source_port_valid(struct 
ia_css_pipe *pipe,
return ret;
 }
 
-/* Assumptions:
+/*
+ * Assumptions:
  * - A line is multiple of 4 bytes = 1 word.
  * - Each frame has SOF and EOF (each 1 word).
  * - Each line has format header and optionally SOL and EOL (each 1 word).
@@ -91,7 +92,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
 const enum atomisp_input_format format,
 const bool hasSOLandEOL,
 const unsigned int embedded_data_size_words,
-unsigned int *size_mem_words) {
+unsigned int *size_mem_words)
+{
int err = 0;
 
unsigned int bits_per_pixel = 0;
@@ -108,7 +110,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
unsigned int width_padded = width;
 
 #if defined(ISP2401)
-   /* The changes will be reverted as soon as RAW
+   /*
+* The changes will be reverted as soon as RAW
 * Buffers are deployed by the 2401 Input System
 * in the non-continuous use scenario.
 */
@@ -118,8 +121,7 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
IA_CSS_ENTER("padded_width=%d, height=%d, format=%d, hasSOLandEOL=%d, 
embedded_data_size_words=%d\n",
 width_padded, height, format, hasSOLandEOL, 
embedded_data_size_words);
 
-   switch (format)
-   {
+   switch (format) {
case ATOMISP_INPUT_FORMAT_RAW_6:/* 4p, 3B, 24bits */
bits_per_pixel = 6;
break;
@@ -134,7 +136,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
case ATOMISP_INPUT_FORMAT_YUV420_10:/* odd 4p, 5B, 40bits, 
even 4p, 10B, 80bits */
case ATOMISP_INPUT_FORMAT_RAW_10:   /* 4p, 5B, 40bits */
 #if !defined(HAS_NO_PACKED_RAW_PIXELS)
-   /* The changes will be reverted as soon as RAW
+   /*
+* The changes will be reverted as soon as RAW
 * Buffers are deployed by the 2401 Input System
 * in the non-continuous use scenario.
 */
@@ -176,33 +179,29 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
odd_line_bytes = (width_padded * bits_per_pixel + 7) >> 3; /* ceil ( 
bits per line / 8) */
 
/* Even lines for YUV420 formats are double in bits_per_pixel. */
-   if (format == ATOMISP_INPUT_FORMAT_YUV420_8
-   || format == ATOMISP_INPUT_FORMAT_YUV420_10
-   || format == ATOMISP_INPUT_FORMAT_YUV420_16)
-   {
-   even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >>
-   3; /* ceil ( bits per line / 8) */
-   } else
-   {
+   if (format == ATOMISP_INPUT_FORMAT_YUV420_8  ||
+   format == ATOMISP_INPUT_FORMAT_YUV420_10 ||
+   format == ATOMISP_INPUT_FORMAT_YUV420_16)
+   even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >> 3; 
/* ceil ( bits per line / 8) */
+   else
even_line_bytes = odd_line_bytes;
-   }
 
/*  a frame represented in memory:  ()- optional; data - payload words.
-   *  addr 0   1   2   3   4   5   6   
7:
-   *  firstSOF (SOL)   PACK_H  datadatadatadata
data
-   *   datadatadatadatadatadatadata
data
-   *   ...
-   *   datadata0   0   0   0   0   0
-   *  second   (EOL)   (SOL)   PACK_H  datadatadatadata
data
-   *   datadatadatadatadatadatadata
data
-   *   ...
-   *   datadata0   0   0   0   0   0
- 

[PATCH 1/5] staging: media: atomisp: code formatting changes atomisp_csi2.c

[Deepak R Varma]

Several trivial code reformatting changes done according to the coding
style guidelines. These changes improves code organisation and readability
and also 4 address many chackpatch error, warning and check complaints.

Signed-off-by: Deepak R Varma 
---
 .../staging/media/atomisp/pci/atomisp_csi2.c  | 72 +--
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c 
b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
index 060b8765ae96..f33a08b2564f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
@@ -22,14 +22,12 @@
 #include "atomisp_internal.h"
 #include "atomisp-regs.h"
 
-static struct v4l2_mbus_framefmt *__csi2_get_format(struct
-   atomisp_mipi_csi2_device
-   * csi2,
-   struct
-   v4l2_subdev_pad_config *cfg,
-   enum
-   v4l2_subdev_format_whence
-   which, unsigned int pad) {
+static struct
+v4l2_mbus_framefmt *__csi2_get_format(struct atomisp_mipi_csi2_device *csi2,
+ struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which,
+ unsigned int pad)
+{
if (which == V4L2_SUBDEV_FORMAT_TRY)
return v4l2_subdev_get_try_format(>subdev, cfg, pad);
else
@@ -42,7 +40,7 @@ static struct v4l2_mbus_framefmt *__csi2_get_format(struct
  * @fh : V4L2 subdev file handle
  * @code   : pointer to v4l2_subdev_pad_mbus_code_enum structure
  * return -EINVAL or zero on success
-*/
+ */
 static int csi2_enum_mbus_code(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_mbus_code_enum *code)
@@ -68,7 +66,7 @@ static int csi2_enum_mbus_code(struct v4l2_subdev *sd,
  * @pad: pad num
  * @fmt: pointer to v4l2 format structure
  * return -EINVAL or zero on success
-*/
+ */
 static int csi2_get_format(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_format *fmt)
@@ -101,12 +99,12 @@ int atomisp_csi2_set_ffmt(struct v4l2_subdev *sd,
else
actual_ffmt->code = atomisp_in_fmt_conv[0].code;
 
-   actual_ffmt->width = clamp_t(
-u32, ffmt->width, ATOM_ISP_MIN_WIDTH,
-ATOM_ISP_MAX_WIDTH);
-   actual_ffmt->height = clamp_t(
- u32, ffmt->height, 
ATOM_ISP_MIN_HEIGHT,
- ATOM_ISP_MAX_HEIGHT);
+   actual_ffmt->width = clamp_t(u32, ffmt->width,
+ATOM_ISP_MIN_WIDTH,
+ATOM_ISP_MAX_WIDTH);
+   actual_ffmt->height = clamp_t(u32, ffmt->height,
+ ATOM_ISP_MIN_HEIGHT,
+ ATOM_ISP_MAX_HEIGHT);
 
tmp_ffmt = *ffmt = *actual_ffmt;
 
@@ -127,7 +125,7 @@ int atomisp_csi2_set_ffmt(struct v4l2_subdev *sd,
  * @pad: pad num
  * @fmt: pointer to v4l2 format structure
  * return -EINVAL or zero on success
-*/
+ */
 static int csi2_set_format(struct v4l2_subdev *sd,
   struct v4l2_subdev_pad_config *cfg,
   struct v4l2_subdev_format *fmt)
@@ -142,7 +140,7 @@ static int csi2_set_format(struct v4l2_subdev *sd,
  * @enable: Enable/disable stream (1/0)
  *
  * Return 0 on success or a negative error code otherwise.
-*/
+ */
 static int csi2_set_stream(struct v4l2_subdev *sd, int enable)
 {
return 0;
@@ -179,7 +177,7 @@ static const struct v4l2_subdev_ops csi2_ops = {
  * @remote : Pointer to remote pad array
  * @flags  : Link flags
  * return -EINVAL or zero on success
-*/
+ */
 static int csi2_link_setup(struct media_entity *entity,
   const struct media_pad *local,
   const struct media_pad *remote, u32 flags)
@@ -217,10 +215,10 @@ static const struct media_entity_operations 
csi2_media_ops = {
 };
 
 /*
-* ispcsi2_init_entities - Initialize subdev and media entity.
-* @csi2: Pointer to ispcsi2 structure.
-* return -ENOMEM or zero on success
-*/
+ * ispcsi2_init_entities - Initialize subdev and media entity.
+ * @csi2: Pointer to ispcsi2 structure.
+ * return -ENOMEM or zero on success
+ */
 static int mipi_csi2_init_entities(struct atomisp_mipi_csi2_device *csi2,
   int port)
 {
@@ -244,9 +242,8 @@ static int mipi_csi2_init_entities(struct 
atomisp_mipi_csi2_device *csi2,
if (ret < 0)
return ret;
 
-   csi2->formats[CSI2_PAD_SINK].code =
-   csi2->formats[CSI2_PAD_SOURCE].code =
-   atomisp_in_

[PATCH 0/5] staging: media: atomisp: code formatting changes

[Deepak R Varma]

This patch set overall improves the code organisation and readability of
the files of atomisp drivers. There are several complaints reported by
checkpatch including ERROR and WARNING types on the files under atomisp/pci
directory.

The changes are proposed on a per file basis since there are many
issues to be addressed in each individual file. The patches are built
on the media_tree/for-v5.14-out1 tree/branch. 

Deepak R Varma (5):
  staging: media: atomisp: code formatting changes atomisp_csi2.c
  staging: media: atomisp: code formatting changes sh_css_mipi.c
  staging: media: atomisp: code formatting changes sh_css_params.c
  staging: media: atomisp: code formatting changes sh_css_sp.c
  staging: media: atomisp: code formatting changes sh_css_version.c

 .../staging/media/atomisp/pci/atomisp_csi2.c  |  72 +-
 .../staging/media/atomisp/pci/sh_css_mipi.c   | 170 ++--
 .../staging/media/atomisp/pci/sh_css_params.c | 929 +-
 drivers/staging/media/atomisp/pci/sh_css_sp.c | 471 -
 .../media/atomisp/pci/sh_css_version.c|   4 +-
 5 files changed, 754 insertions(+), 892 deletions(-)

-- 
2.30.2



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: media: atomisp: pci: reposition braces as per coding style

[Deepak R Varma]

On Fri, Apr 30, 2021 at 08:12:41PM +0300, Sakari Ailus wrote:
> Hi Deepak,
> 
> On Fri, Apr 30, 2021 at 10:34:37PM +0530, Deepak R Varma wrote:
> > On Fri, Apr 30, 2021 at 07:33:27PM +0300, Sakari Ailus wrote:
> > > Hi Deepak,
> > > 
> > > If you're touching all these lines, I might do a little more. Please see
> > > the comments below.
> > > 
> > Hello Sakari,
> > I can definitely include other changes, but then it will be many different
> > types of changes into a single patch. Will that be okay?
> > 
> > I was planning to address one issue per patch as I think the volume of
> > change is going to be high.  I mentioned that in the notes section of the 
> > patch
> > message.
> 
> I think I'd split the patch into smaller chunks if the result becomes too
> big but I don't think it's necessary yet.
> 
> Splitting different kinds of simple cleanups into several patches takes
> longer time to review when they're touching the same piece of code. As the
> chunks in these patches have virtually no dependencies to other chunks,
> it's fine to do several kinds of cleanups at once.

Okay, sure. That sounds good to me. I will include other related
improvements in the same area and send split patch set accordingly. I will
include this patch in the patch set.

Thank you for the guidance.
deepak.

> 
> -- 
> Kind regards,
> 
> Sakari Ailus


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: media: atomisp: pci: reposition braces as per coding style

[Deepak R Varma]

On Fri, Apr 30, 2021 at 07:33:27PM +0300, Sakari Ailus wrote:
> Hi Deepak,
> 
> If you're touching all these lines, I might do a little more. Please see
> the comments below.
> 
Hello Sakari,
I can definitely include other changes, but then it will be many different
types of changes into a single patch. Will that be okay?

I was planning to address one issue per patch as I think the volume of
change is going to be high.  I mentioned that in the notes section of the patch
message.

Let me know if you still suggest I combine those.

Thank you,
deepak.



> On Fri, Apr 30, 2021 at 09:10:12PM +0530, Deepak R Varma wrote:
> > Misplaced braces makes it difficult to follow the code easily. This also
> > goes against the code style guidelines. This resolved following checkpatch
> > complaints:
> > 
> > ERROR: open brace '{' following function definitions go on the next line
> > ERROR: that open brace { should be on the previous line
> > 
> > Signed-off-by: Deepak R Varma 
> > ---
> > 
> > Please Note: There are several other checkpatch triggered warnings and
> > errors that should be addressed in separate patches.
> > 
> > 
> >  .../staging/media/atomisp/pci/atomisp_csi2.c  |   3 +-
> >  .../staging/media/atomisp/pci/sh_css_mipi.c   |  69 +++
> >  .../staging/media/atomisp/pci/sh_css_params.c | 171 --
> >  drivers/staging/media/atomisp/pci/sh_css_sp.c | 108 +--
> >  .../media/atomisp/pci/sh_css_version.c|   3 +-
> >  5 files changed, 157 insertions(+), 197 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c 
> > b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
> > index 060b8765ae96..200f16994f3a 100644
> > --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c
> > +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
> > @@ -29,7 +29,8 @@ static struct v4l2_mbus_framefmt *__csi2_get_format(struct
> > v4l2_subdev_pad_config *cfg,
> > enum
> > v4l2_subdev_format_whence
> 
> You could have more arguments on the same line up to 80 characters per
> line.
> 
> > -   which, unsigned int pad) {
> > +   which, unsigned int pad)
> > +{
> > if (which == V4L2_SUBDEV_FORMAT_TRY)
> > return v4l2_subdev_get_try_format(>subdev, cfg, pad);
> > else
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c 
> > b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > index 3f34cc81be87..75489f7d75ee 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > @@ -91,7 +91,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
> >  const enum atomisp_input_format format,
> >  const bool hasSOLandEOL,
> >  const unsigned int embedded_data_size_words,
> > -unsigned int *size_mem_words) {
> > +unsigned int *size_mem_words)
> > +{
> > int err = 0;
> >  
> > unsigned int bits_per_pixel = 0;
> > @@ -118,8 +119,7 @@ ia_css_mipi_frame_calculate_size(const unsigned int 
> > width,
> > IA_CSS_ENTER("padded_width=%d, height=%d, format=%d, hasSOLandEOL=%d, 
> > embedded_data_size_words=%d\n",
> >  width_padded, height, format, hasSOLandEOL, 
> > embedded_data_size_words);
> >  
> > -   switch (format)
> > -   {
> > +   switch (format) {
> > case ATOMISP_INPUT_FORMAT_RAW_6:/* 4p, 3B, 24bits */
> > bits_per_pixel = 6;
> > break;
> > @@ -178,12 +178,10 @@ ia_css_mipi_frame_calculate_size(const unsigned int 
> > width,
> > /* Even lines for YUV420 formats are double in bits_per_pixel. */
> > if (format == ATOMISP_INPUT_FORMAT_YUV420_8
> > || format == ATOMISP_INPUT_FORMAT_YUV420_10
> > -   || format == ATOMISP_INPUT_FORMAT_YUV420_16)
> > -   {
> > +   || format == ATOMISP_INPUT_FORMAT_YUV420_16) {
> > even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >>
> > 3; /* ceil ( bits per line / 8) */
> > -   } else
> > -   {
> > +   } else {
> > even_line_bytes = odd_line_bytes;
> > }
> >  
> > @@ -236,7 +234,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int 
> > width,
> >  #if !defined(ISP2401)
> >  int
> >  ia_css_mipi_frame_enable_check_on_size(const enum mipi_port_id port,
> > - 

[PATCH] staging: media: atomisp: pci: reposition braces as per coding style

[Deepak R Varma]

Misplaced braces makes it difficult to follow the code easily. This also
goes against the code style guidelines. This resolved following checkpatch
complaints:

ERROR: open brace '{' following function definitions go on the next line
ERROR: that open brace { should be on the previous line

Signed-off-by: Deepak R Varma 
---

Please Note: There are several other checkpatch triggered warnings and
errors that should be addressed in separate patches.


 .../staging/media/atomisp/pci/atomisp_csi2.c  |   3 +-
 .../staging/media/atomisp/pci/sh_css_mipi.c   |  69 +++
 .../staging/media/atomisp/pci/sh_css_params.c | 171 --
 drivers/staging/media/atomisp/pci/sh_css_sp.c | 108 +--
 .../media/atomisp/pci/sh_css_version.c|   3 +-
 5 files changed, 157 insertions(+), 197 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c 
b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
index 060b8765ae96..200f16994f3a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
@@ -29,7 +29,8 @@ static struct v4l2_mbus_framefmt *__csi2_get_format(struct
v4l2_subdev_pad_config *cfg,
enum
v4l2_subdev_format_whence
-   which, unsigned int pad) {
+   which, unsigned int pad)
+{
if (which == V4L2_SUBDEV_FORMAT_TRY)
return v4l2_subdev_get_try_format(>subdev, cfg, pad);
else
diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c 
b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index 3f34cc81be87..75489f7d75ee 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -91,7 +91,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
 const enum atomisp_input_format format,
 const bool hasSOLandEOL,
 const unsigned int embedded_data_size_words,
-unsigned int *size_mem_words) {
+unsigned int *size_mem_words)
+{
int err = 0;
 
unsigned int bits_per_pixel = 0;
@@ -118,8 +119,7 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
IA_CSS_ENTER("padded_width=%d, height=%d, format=%d, hasSOLandEOL=%d, 
embedded_data_size_words=%d\n",
 width_padded, height, format, hasSOLandEOL, 
embedded_data_size_words);
 
-   switch (format)
-   {
+   switch (format) {
case ATOMISP_INPUT_FORMAT_RAW_6:/* 4p, 3B, 24bits */
bits_per_pixel = 6;
break;
@@ -178,12 +178,10 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
/* Even lines for YUV420 formats are double in bits_per_pixel. */
if (format == ATOMISP_INPUT_FORMAT_YUV420_8
|| format == ATOMISP_INPUT_FORMAT_YUV420_10
-   || format == ATOMISP_INPUT_FORMAT_YUV420_16)
-   {
+   || format == ATOMISP_INPUT_FORMAT_YUV420_16) {
even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >>
3; /* ceil ( bits per line / 8) */
-   } else
-   {
+   } else {
even_line_bytes = odd_line_bytes;
}
 
@@ -236,7 +234,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
 #if !defined(ISP2401)
 int
 ia_css_mipi_frame_enable_check_on_size(const enum mipi_port_id port,
-  const unsigned int   size_mem_words) 
{
+  const unsigned int   size_mem_words)
+{
u32 idx;
 
int err = -EBUSY;
@@ -246,11 +245,9 @@ ia_css_mipi_frame_enable_check_on_size(const enum 
mipi_port_id port,
 
for (idx = 0; idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT &&
 my_css.mipi_sizes_for_check[port][idx] != 0;
-idx++)   /* do nothing */
-   {
+idx++) { /* do nothing */
}
-   if (idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT)
-   {
+   if (idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT) {
my_css.mipi_sizes_for_check[port][idx] = size_mem_words;
err = 0;
}
@@ -271,7 +268,8 @@ mipi_init(void)
 int
 calculate_mipi_buff_size(
 struct ia_css_stream_config *stream_cfg,
-unsigned int *size_mem_words) {
+unsigned int *size_mem_words)
+{
 #if !defined(ISP2401)
int err = -EINVAL;
(void)stream_cfg;
@@ -346,12 +344,10 @@ calculate_mipi_buff_size(
 
/* Even lines for YUV420 formats are double in bits_per_pixel. */
if (format == ATOMISP_INPUT_FORMAT_YUV420_8
-   || format == ATOMISP_INPUT_FORMAT_YUV420_10)
-   {
+   || format == ATOMISP_INPUT_FORMAT_YUV420_10) {
even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >>
3; /* c

Re: [PATCH v2 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

On Mon, Oct 26, 2020 at 06:57:17AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Oct 26, 2020 at 09:34:53AM +0530, Deepak R Varma wrote:
> > On Wed, Oct 21, 2020 at 01:01:07PM +0530, Deepak R Varma wrote:
> > 
> > Hello,
> > Requesting a review / ack of this patch.
> 
> As I said in my previous email, to the outreachy list, if I have not
> applied something, it needs to be resent, if you still think it is
> needed.

Thank you Greg. I have just resent the patch set with changes suggested by 
Matthew.

Deepak.

> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/2] staging: kpc2000: kpc_dma: rename show function per convention

[Deepak R Varma]

Rename show_engine_regs show function to engine_regs_show as per the
convention followed. The show function macro DEVICE_ATTR also replaced
by DEVICE_ATTR_RO. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v2:
   - None for this patch.
Changes since v1:
   - Replace DEVICE_ATTR by DEVICE_ATTR_RO as suggested by Greg.

 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c 
b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index e1dac89ca6a2..175fe8b0d055 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -50,7 +50,7 @@ static void kpc_dma_del_device(struct kpc_dma_device *ldev)
 }
 
 /**  SysFS Attributes **/
-static ssize_t  show_engine_regs(struct device *dev, struct device_attribute 
*attr, char *buf)
+static ssize_t  engine_regs_show(struct device *dev, struct device_attribute 
*attr, char *buf)
 {
struct kpc_dma_device *ldev;
struct platform_device *pldev = to_platform_device(dev);
@@ -80,7 +80,7 @@ static ssize_t  show_engine_regs(struct device *dev, struct 
device_attribute *at
ldev->desc_completed
);
 }
-static DEVICE_ATTR(engine_regs, 0444, show_engine_regs, NULL);
+static DEVICE_ATTR_RO(engine_regs);
 
 static const struct attribute *ndd_attr_list[] = {
_attr_engine_regs.attr,
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

Reformat lines that exceed 100 column in length. Issue reported by
checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v2:
   - Avoid too much line breaks for a single instruction.
   - change suggested by Matthew Wilcox.
Changes since v1:
   - No change in this patch.
   - Patch 2/2 has a change.

 drivers/staging/kpc2000/kpc_dma/dma.c | 21 ++
 drivers/staging/kpc2000/kpc_dma/fileops.c | 28 +--
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  9 --
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c 
b/drivers/staging/kpc2000/kpc_dma/dma.c
index 452a3f7c835d..e169ac609ba4 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -16,7 +16,8 @@ irqreturn_t  ndd_irq_handler(int irq, void *dev_id)
 {
struct kpc_dma_device *ldev = (struct kpc_dma_device *)dev_id;
 
-   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) || 
(ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
+   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) ||
+   (ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
schedule_work(>irq_work);
 
return IRQ_HANDLED;
@@ -39,7 +40,8 @@ void  ndd_irq_worker(struct work_struct *ws)
cur = eng->desc_completed;
do {
cur = cur->Next;
-   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n", cur, cur->acd);
+   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n",
+   cur, cur->acd);
BUG_ON(cur == eng->desc_next); // Ordering failure.
 
if (cur->DescControlFlags & DMA_DESC_CTL_SOP) {
@@ -56,7 +58,8 @@ void  ndd_irq_worker(struct work_struct *ws)
 
if (cur->DescControlFlags & DMA_DESC_CTL_EOP) {
if (cur->acd)
-   transfer_complete_cb(cur->acd, 
eng->accumulated_bytes, eng->accumulated_flags | ACD_FLAG_DONE);
+   transfer_complete_cb(cur->acd, 
eng->accumulated_bytes,
+eng->accumulated_flags | 
ACD_FLAG_DONE);
}
 
eng->desc_completed = cur;
@@ -103,7 +106,9 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
eng->dir = DMA_TO_DEVICE;
 
eng->desc_pool_cnt = desc_cnt;
-   eng->desc_pool = dma_pool_create("KPC DMA Descriptors", 
>pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 4096);
+   eng->desc_pool = dma_pool_create("KPC DMA Descriptors", 
>pldev->dev,
+sizeof(struct kpc_dma_descriptor),
+DMA_DESC_ALIGNMENT, 4096);
 
eng->desc_pool_first = dma_pool_alloc(eng->desc_pool, GFP_KERNEL | 
GFP_DMA, _handle);
if (!eng->desc_pool_first) {
@@ -141,7 +146,8 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
INIT_WORK(>irq_work, ndd_irq_worker);
 
// Grab IRQ line
-   rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED, 
KP_DRIVER_NAME_DMA_CONTROLLER, eng);
+   rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED,
+KP_DRIVER_NAME_DMA_CONTROLLER, eng);
if (rv) {
dev_err(>pldev->dev, "%s: failed to request_irq: %d\n", 
__func__, rv);
return rv;
@@ -195,7 +201,10 @@ void  stop_dma_engine(struct kpc_dma_device *eng)
}
 
// Clear any persistent bits just to make sure there is no residue from 
the reset
-   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE 
| ENG_CTL_DESC_ALIGN_ERR | ENG_CTL_DESC_FETCH_ERR | ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END | ENG_CTL_DMA_WAITING_PERSIST), 0);
+   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE |
+   ENG_CTL_DESC_ALIGN_ERR | 
ENG_CTL_DESC_FETCH_ERR |
+   ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END |
+   ENG_CTL_DMA_WAITING_PERSIST), 0);
 
// Reset performance counters
 
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index e1c7c04f16fe..10dcd6646b01 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -76,7 +76,8 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 
// Lock the user buffer pages in memory, and hold on to the page 
pointers (for the sglist)
mmap_read_lock(current->mm);  /*  get memory map semaphore */
-   rv = pin_user_pages(iov_base, acd->

Re: [Outreachy kernel] Re: [PATCH v2 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

On Mon, Oct 26, 2020 at 04:16:52AM +, Matthew Wilcox wrote:
> On Mon, Oct 26, 2020 at 09:34:53AM +0530, Deepak R Varma wrote:
> > > - dev_dbg(>pldev->dev, "Handling completed descriptor %p 
> > > (acd = %p)\n", cur, cur->acd);
> > > + dev_dbg(>pldev->dev, "Handling completed descriptor %p 
> > > (acd = %p)\n",
> > > + cur,
> > > + cur->acd);
> 
> Why do you put 'cur' and 'cur->acd' on different lines?

Hello,
I was thinking it makes it more readable. However, I understand your
comment and will rearrange the changes accordingly. Thank you for
pointing it out.

> 
> > > - rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED, 
> > > KP_DRIVER_NAME_DMA_CONTROLLER, eng);
> > > + rv = request_irq(eng->irq,
> > > +  ndd_irq_handler,
> > > +  IRQF_SHARED,
> > > +  KP_DRIVER_NAME_DMA_CONTROLLER,
> > > +  eng);
> 
> Likewise.  I'd do:
> 
>   rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED,
>   KP_DRIVER_NAME_DMA_CONTROLLER, eng);
> 

Yes, will correct all such occurrences and resend another version.

Thank you,
Deepak.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 2/2] staging: kpc2000: kpc_dma: rename show function per convention

[Deepak R Varma]

On Wed, Oct 21, 2020 at 01:05:25PM +0530, Deepak R Varma wrote:

Hello,
Requesting a review / ack for this patch.

Thank you,
Deepak.

> Rename show_engine_regs show function to engine_regs_show as per the
> convention followed. The show function macro DEVICE_ATTR is replaced
> by DEVICE_ATTR_RO. Issue reported by checkpatch script.
> 
> Signed-off-by: Deepak R Varma 
> ---
> Changes since v1:
>- Replace DEVICE_ATTR by DEVICE_ATTR_RO as suggested by Greg.
> 
>  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c 
> b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> index 7698e5ef2a7c..aa9f96793e59 100644
> --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> @@ -50,7 +50,7 @@ static void kpc_dma_del_device(struct kpc_dma_device *ldev)
>  }
>  
>  /**  SysFS Attributes **/
> -static ssize_t  show_engine_regs(struct device *dev, struct device_attribute 
> *attr, char *buf)
> +static ssize_t  engine_regs_show(struct device *dev, struct device_attribute 
> *attr, char *buf)
>  {
>   struct kpc_dma_device *ldev;
>   struct platform_device *pldev = to_platform_device(dev);
> @@ -80,7 +80,7 @@ static ssize_t  show_engine_regs(struct device *dev, struct 
> device_attribute *at
>   ldev->desc_completed
>   );
>  }
> -static DEVICE_ATTR(engine_regs, 0444, show_engine_regs, NULL);
> +static DEVICE_ATTR_RO(engine_regs);
>  
>  static const struct attribute *ndd_attr_list[] = {
>   _attr_engine_regs.attr,
> -- 
> 2.25.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

On Wed, Oct 21, 2020 at 01:01:07PM +0530, Deepak R Varma wrote:

Hello,
Requesting a review / ack of this patch.

Thank you,
Deepak.

> Reformat lines that exceed 100 column in length. Issue reported by
> checkpatch script.
> 
> Signed-off-by: Deepak R Varma 
> ---
> Changes since v1:
>- No change in this patch.
>- Patch 2/2 has a change.
> 
>  drivers/staging/kpc2000/kpc_dma/dma.c | 27 +---
>  drivers/staging/kpc2000/kpc_dma/fileops.c | 44 +++
>  .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  9 ++--
>  3 files changed, 63 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c 
> b/drivers/staging/kpc2000/kpc_dma/dma.c
> index 452a3f7c835d..b8d8294aa4c3 100644
> --- a/drivers/staging/kpc2000/kpc_dma/dma.c
> +++ b/drivers/staging/kpc2000/kpc_dma/dma.c
> @@ -16,7 +16,8 @@ irqreturn_t  ndd_irq_handler(int irq, void *dev_id)
>  {
>   struct kpc_dma_device *ldev = (struct kpc_dma_device *)dev_id;
>  
> - if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) || 
> (ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
> + if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) ||
> + (ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
>   schedule_work(>irq_work);
>  
>   return IRQ_HANDLED;
> @@ -39,7 +40,9 @@ void  ndd_irq_worker(struct work_struct *ws)
>   cur = eng->desc_completed;
>   do {
>   cur = cur->Next;
> - dev_dbg(>pldev->dev, "Handling completed descriptor %p 
> (acd = %p)\n", cur, cur->acd);
> + dev_dbg(>pldev->dev, "Handling completed descriptor %p 
> (acd = %p)\n",
> + cur,
> + cur->acd);
>   BUG_ON(cur == eng->desc_next); // Ordering failure.
>  
>   if (cur->DescControlFlags & DMA_DESC_CTL_SOP) {
> @@ -56,7 +59,9 @@ void  ndd_irq_worker(struct work_struct *ws)
>  
>   if (cur->DescControlFlags & DMA_DESC_CTL_EOP) {
>   if (cur->acd)
> - transfer_complete_cb(cur->acd, 
> eng->accumulated_bytes, eng->accumulated_flags | ACD_FLAG_DONE);
> + transfer_complete_cb(cur->acd,
> +  eng->accumulated_bytes,
> +  eng->accumulated_flags | 
> ACD_FLAG_DONE);
>   }
>  
>   eng->desc_completed = cur;
> @@ -103,7 +108,10 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
> desc_cnt)
>   eng->dir = DMA_TO_DEVICE;
>  
>   eng->desc_pool_cnt = desc_cnt;
> - eng->desc_pool = dma_pool_create("KPC DMA Descriptors", 
> >pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 
> 4096);
> + eng->desc_pool = dma_pool_create("KPC DMA Descriptors",
> +  >pldev->dev,
> +  sizeof(struct kpc_dma_descriptor),
> +  DMA_DESC_ALIGNMENT, 4096);
>  
>   eng->desc_pool_first = dma_pool_alloc(eng->desc_pool, GFP_KERNEL | 
> GFP_DMA, _handle);
>   if (!eng->desc_pool_first) {
> @@ -141,7 +149,11 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
> desc_cnt)
>   INIT_WORK(>irq_work, ndd_irq_worker);
>  
>   // Grab IRQ line
> - rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED, 
> KP_DRIVER_NAME_DMA_CONTROLLER, eng);
> + rv = request_irq(eng->irq,
> +  ndd_irq_handler,
> +  IRQF_SHARED,
> +  KP_DRIVER_NAME_DMA_CONTROLLER,
> +  eng);
>   if (rv) {
>   dev_err(>pldev->dev, "%s: failed to request_irq: %d\n", 
> __func__, rv);
>   return rv;
> @@ -195,7 +207,10 @@ void  stop_dma_engine(struct kpc_dma_device *eng)
>   }
>  
>   // Clear any persistent bits just to make sure there is no residue from 
> the reset
> - SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE 
> | ENG_CTL_DESC_ALIGN_ERR | ENG_CTL_DESC_FETCH_ERR | ENG_CTL_SW_ABORT_ERR | 
> ENG_CTL_DESC_CHAIN_END | ENG_CTL_DMA_WAITING_PERSIST), 0);
> + SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE |
> + ENG_CTL_DESC_ALIGN_ERR | 
> ENG_CTL_DESC_FETCH_ERR |
> + ENG_CTL_SW_ABORT_ERR | 
> ENG_CTL_DESC_CHAIN_END |
> + EN

Re: [Outreachy kernel] Clean up query: greybus/audio_manager_module.c

[Deepak R Varma]

On Thu, Oct 22, 2020 at 07:29:31AM +0200, Greg KH wrote:
> On Thu, Oct 22, 2020 at 09:07:01AM +0530, Deepak R Varma wrote:
> > Hello,
> > I am reviewing the file: drivers/staging/greybus/audio_manager_module.c
> > and have found that there are several gb_audio_module_*_show functions
> > that accept "struct gb_audio_manager_module_attribute * " as a function
> > parameter. However, this parameter is not used and should not be
> > necessary. Would you suggest cleaning up such functions.
> 
> Try removing it and see why it is really needed to be there :)

Thank you Greg. I tried that and realised that the internal show
function signature definition will mismatch. It will not be a straight
forward change for the clean up sake.

I got my answer. Thanks again.
Deepak.

> 
> good luck!
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] Clean up query: greybus/audio_manager_module.c

[Deepak R Varma]

On Thu, Oct 22, 2020 at 07:29:31AM +0200, Greg KH wrote:
> On Thu, Oct 22, 2020 at 09:07:01AM +0530, Deepak R Varma wrote:
> > Hello,
> > I am reviewing the file: drivers/staging/greybus/audio_manager_module.c
> > and have found that there are several gb_audio_module_*_show functions
> > that accept "struct gb_audio_manager_module_attribute * " as a function
> > parameter. However, this parameter is not used and should not be
> > necessary. Would you suggest cleaning up such functions.
> 
> Try removing it and see why it is really needed to be there :)

Sounds exciting. I will definitely do that.

> 
> good luck!

Thank you.
Deepak.

> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: greybus: audio: code indentation and formatting changes

[Deepak R Varma]

Correct code indentation and realignment as per the coding style
guidelines. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/greybus/audio_codec.c | 109 +++---
 .../staging/greybus/audio_manager_module.c|   3 +-
 2 files changed, 40 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/greybus/audio_codec.c 
b/drivers/staging/greybus/audio_codec.c
index 494aa823e998..17a39ed63769 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -71,15 +71,13 @@ static int gbaudio_module_enable_tx(struct 
gbaudio_codec_info *codec,
i2s_port = 0;   /* fixed for now */
cportid = data->connection->hd_cport_id;
ret = gb_audio_apbridgea_register_cport(data->connection,
-   i2s_port, cportid,
-   AUDIO_APBRIDGEA_DIRECTION_TX);
+   i2s_port, cportid,
+   
AUDIO_APBRIDGEA_DIRECTION_TX);
if (ret) {
-   dev_err_ratelimited(module->dev,
-   "reg_cport failed:%d\n", ret);
+   dev_err_ratelimited(module->dev, "reg_cport 
failed:%d\n", ret);
return ret;
}
-   data->state[SNDRV_PCM_STREAM_PLAYBACK] =
-   GBAUDIO_CODEC_STARTUP;
+   data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_STARTUP;
dev_dbg(module->dev, "Dynamic Register %d DAI\n", cportid);
}
 
@@ -93,12 +91,10 @@ static int gbaudio_module_enable_tx(struct 
gbaudio_codec_info *codec,
ret = gb_audio_gb_set_pcm(module->mgmt_connection, data_cport,
  format, rate, channels, sig_bits);
if (ret) {
-   dev_err_ratelimited(module->dev, "set_pcm failed:%d\n",
-   ret);
+   dev_err_ratelimited(module->dev, "set_pcm failed:%d\n", 
ret);
return ret;
}
-   data->state[SNDRV_PCM_STREAM_PLAYBACK] =
-   GBAUDIO_CODEC_HWPARAMS;
+   data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_HWPARAMS;
dev_dbg(module->dev, "Dynamic hw_params %d DAI\n", data_cport);
}
 
@@ -113,15 +109,13 @@ static int gbaudio_module_enable_tx(struct 
gbaudio_codec_info *codec,
ret);
return ret;
}
-   ret = gb_audio_gb_activate_tx(module->mgmt_connection,
- data_cport);
+   ret = gb_audio_gb_activate_tx(module->mgmt_connection, 
data_cport);
if (ret) {
dev_err_ratelimited(module->dev,
"activate_tx failed:%d\n", ret);
return ret;
}
-   data->state[SNDRV_PCM_STREAM_PLAYBACK] =
-   GBAUDIO_CODEC_PREPARE;
+   data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_PREPARE;
dev_dbg(module->dev, "Dynamic prepare %d DAI\n", data_cport);
}
 
@@ -153,25 +147,22 @@ static int gbaudio_module_disable_tx(struct 
gbaudio_module_info *module, int id)
return ret;
}
dev_dbg(module->dev, "Dynamic deactivate %d DAI\n", data_cport);
-   data->state[SNDRV_PCM_STREAM_PLAYBACK] =
-   GBAUDIO_CODEC_HWPARAMS;
+   data->state[SNDRV_PCM_STREAM_PLAYBACK] = GBAUDIO_CODEC_HWPARAMS;
}
 
if (module_state > GBAUDIO_CODEC_SHUTDOWN) {
i2s_port = 0;   /* fixed for now */
cportid = data->connection->hd_cport_id;
ret = gb_audio_apbridgea_unregister_cport(data->connection,
-   i2s_port, cportid,
-   AUDIO_APBRIDGEA_DIRECTION_TX);
+ i2s_port, cportid,
+ 
AUDIO_APBRIDGEA_DIRECTION_TX);
if (ret) {
dev_err_ratelimited(module->dev,
-   "unregister_cport failed:%d\n",
-   ret);
+   "unregister_cport failed:%d\n", 
ret);
return ret;
}

Clean up query: greybus/audio_manager_module.c

[Deepak R Varma]

Hello,
I am reviewing the file: drivers/staging/greybus/audio_manager_module.c
and have found that there are several gb_audio_module_*_show functions
that accept "struct gb_audio_manager_module_attribute * " as a function
parameter. However, this parameter is not used and should not be
necessary. Would you suggest cleaning up such functions.

Thank you.
Deepak.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/3] staging: kpc2000: re-indent code for better readability

[Deepak R Varma]

Re-indent code as per the coding style guidelines. The changes improve
code readability. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v2:
   - None.
Changes since v1:
   - Separate specific checkpatch issues into individual patches.
   - Updated patch subject and description to be specific to the issue
 being fixed.
   - Introduced patch 3/3.
   - Suggested by Vaishali T.

 drivers/staging/kpc2000/kpc2000/core.c| 3 ++-
 drivers/staging/kpc2000/kpc2000/dma_common_defs.h | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c 
b/drivers/staging/kpc2000/kpc2000/core.c
index 358d7b2f4ad1..6462a3059fb0 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -124,6 +124,7 @@ static ssize_t cpld_reconfigure(struct device *dev,
writeq(wr_val, pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
return count;
 }
+
 static DEVICE_ATTR(cpld_reconfigure, 0220, NULL, cpld_reconfigure);
 
 static ssize_t irq_mask_reg_show(struct device *dev,
@@ -367,7 +368,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
 
pcard->dma_bar_base = ioremap(dma_bar_phys_addr,
- dma_bar_phys_len);
+ dma_bar_phys_len);
if (!pcard->dma_bar_base) {
dev_err(>pdev->dev,
"probe: DMA_BAR could not remap memory to virtual 
space\n");
diff --git a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h 
b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
index 21450e3d408f..8bc78be3c259 100644
--- a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
+++ b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
@@ -6,8 +6,7 @@
 #define KPC_DMA_S2C_BASE_OFFSET 0x
 #define KPC_DMA_C2S_BASE_OFFSET 0x2000
 #define KPC_DMA_ENGINE_SIZE 0x0100
-#define  ENGINE_CAP_PRESENT_MASK0x1
-
+#define ENGINE_CAP_PRESENT_MASK 0x1
 
 #define KPC_DMA_CARD_IRQ_ENABLE (1 << 0)
 #define KPC_DMA_CARD_IRQ_ACTIVE (1 << 1)
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/3] staging: kpc2000: rearrange lines exceeding 100 columns

[Deepak R Varma]

Reformat lines that exceed 100 column in length. Issue reported by
checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v1:
   - Separate specific checkpatch issues into individual patches.
   - Updated patch subject and description to be specific to the issue
 being fixed.
   - Introduced patch 3/3.
   - Suggested by Vaishali T.

 drivers/staging/kpc2000/kpc2000/cell_probe.c | 71 +++-
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c 
b/drivers/staging/kpc2000/kpc2000/cell_probe.c
index 738122afc2ae..e7e963d62699 100644
--- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
+++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
@@ -30,9 +30,12 @@
  *
  */
 
-#define KPC_OLD_DMA_CH_NUM(present, channel)   ((present) ? (0x8 | ((channel) 
& 0x7)) : 0)
-#define KPC_OLD_S2C_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, 
cte.s2c_dma_channel_num)
-#define KPC_OLD_C2S_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, 
cte.c2s_dma_channel_num)
+#define KPC_OLD_DMA_CH_NUM(present, channel)   \
+   ((present) ? (0x8 | ((channel) & 0x7)) : 0)
+#define KPC_OLD_S2C_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, cte.s2c_dma_channel_num)
+#define KPC_OLD_C2S_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, cte.c2s_dma_channel_num)
 
 #define KP_CORE_ID_INVALID  0
 #define KP_CORE_ID_I2C  3
@@ -67,7 +70,8 @@ void  parse_core_table_entry_v0(struct core_table_entry *cte, 
const u64 read_val
 static
 void dbg_cte(struct kp2000_device *pcard, struct core_table_entry *cte)
 {
-   dev_dbg(>pdev->dev, "CTE: type:%3d  offset:%3d (%3d)  length:%3d 
(%3d)  s2c:%d  c2s:%d  irq_count:%d  base_irq:%d\n",
+   dev_dbg(>pdev->dev,
+   "CTE: type:%3d  offset:%3d (%3d)  length:%3d (%3d)  s2c:%d  
c2s:%d  irq_count:%d  base_irq:%d\n",
cte->type,
cte->offset,
cte->offset / 4096,
@@ -107,7 +111,14 @@ static int probe_core_basic(unsigned int core_num, struct 
kp2000_device *pcard,
.ddna  = pcard->ddna,
};
 
-   dev_dbg(>pdev->dev, "Found Basic core: type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found Basic core: type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
cell.platform_data = _pdata;
cell.pdata_size = sizeof(struct kpc_core_device_platdata);
@@ -290,7 +301,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
struct kpc_uio_device *kudev;
int rv;
 
-   dev_dbg(>pdev->dev, "Found UIO core:   type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found UIO core:   type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
kudev = kzalloc(sizeof(*kudev), GFP_KERNEL);
if (!kudev)
@@ -315,10 +333,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
 
kudev->uioinfo.mem[0].name = "uiomap";
kudev->uioinfo.mem[0].addr = pci_resource_start(pcard->pdev, REG_BAR) + 
cte.offset;
-   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1); // Round up to nearest PAGE_SIZE boundary
+
+   // Round up to nearest PAGE_SIZE boundary
+   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1);
kudev->uioinfo.mem[0].memtype = UIO_MEM_PHYS;
 
-   kudev->dev = device_create(kpc_uio_class, >pdev->dev, MKDEV(0, 
0), kudev, "%s.%d.%d.%d", kudev->uioinfo.name, pcard->card_num, cte.type, 
kudev->core_num);
+   kudev->dev = device_create(kpc_uio_class,
+  >pdev->dev, MKDEV(0, 0), kudev, 
"%s.%d.%d.%d",
+  kudev->uioinfo.name, pcard->card_num, 
cte.type, kudev->core_num);
if (IS_ERR(kudev->dev)) {
dev_err(>pdev->dev, "%s: device_create failed!\n",
 

[PATCH v3 3/3] staging: kpc2000: Use BIT macro instead of bit masking

[Deepak R Varma]

Replace bit masking by the BIT macro. This resolves the checkpatch issue
"CHECK: Prefer using the BIT macro"

Signed-off-by: Deepak R Varma 
---
Changes since v2:
   - Update patch description as suggested by Julia L.
Changes since v1:
   - Separate specific checkpatch issues into individual patches.
   - Updated patch subject and description to be specific to the issue
 being fixed.
   - Introduced patch 3/3.
   - Suggested by Vaishali T.

 drivers/staging/kpc2000/kpc2000/dma_common_defs.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h 
b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
index 8bc78be3c259..613c4898f65e 100644
--- a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
+++ b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
@@ -8,13 +8,13 @@
 #define KPC_DMA_ENGINE_SIZE 0x0100
 #define ENGINE_CAP_PRESENT_MASK 0x1
 
-#define KPC_DMA_CARD_IRQ_ENABLE (1 << 0)
-#define KPC_DMA_CARD_IRQ_ACTIVE (1 << 1)
-#define KPC_DMA_CARD_IRQ_PENDING(1 << 2)
-#define KPC_DMA_CARD_IRQ_MSI(1 << 3)
-#define KPC_DMA_CARD_USER_INTERRUPT_MODE(1 << 4)
-#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  (1 << 5)
-#define KPC_DMA_CARD_IRQ_MSIX_MODE  (1 << 6)
+#define KPC_DMA_CARD_IRQ_ENABLE BIT(0)
+#define KPC_DMA_CARD_IRQ_ACTIVE BIT(1)
+#define KPC_DMA_CARD_IRQ_PENDINGBIT(2)
+#define KPC_DMA_CARD_IRQ_MSIBIT(3)
+#define KPC_DMA_CARD_USER_INTERRUPT_MODEBIT(4)
+#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  BIT(5)
+#define KPC_DMA_CARD_IRQ_MSIX_MODE  BIT(6)
 #define KPC_DMA_CARD_MAX_PAYLOAD_SIZE_MASK  0x0700
 #define KPC_DMA_CARD_MAX_READ_REQUEST_SIZE_MASK 0x7000
 #define KPC_DMA_CARD_S2C_INTERRUPT_STATUS_MASK  0x00FF
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/3] staging: kpc2000: rearrange lines exceeding 100 columns

[Deepak R Varma]

Reformat lines that exceed 100 column in length. Issue reported by
checkpatch script.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/kpc2000/kpc2000/cell_probe.c | 71 +++-
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c 
b/drivers/staging/kpc2000/kpc2000/cell_probe.c
index 738122afc2ae..e7e963d62699 100644
--- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
+++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
@@ -30,9 +30,12 @@
  *
  */
 
-#define KPC_OLD_DMA_CH_NUM(present, channel)   ((present) ? (0x8 | ((channel) 
& 0x7)) : 0)
-#define KPC_OLD_S2C_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, 
cte.s2c_dma_channel_num)
-#define KPC_OLD_C2S_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, 
cte.c2s_dma_channel_num)
+#define KPC_OLD_DMA_CH_NUM(present, channel)   \
+   ((present) ? (0x8 | ((channel) & 0x7)) : 0)
+#define KPC_OLD_S2C_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, cte.s2c_dma_channel_num)
+#define KPC_OLD_C2S_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, cte.c2s_dma_channel_num)
 
 #define KP_CORE_ID_INVALID  0
 #define KP_CORE_ID_I2C  3
@@ -67,7 +70,8 @@ void  parse_core_table_entry_v0(struct core_table_entry *cte, 
const u64 read_val
 static
 void dbg_cte(struct kp2000_device *pcard, struct core_table_entry *cte)
 {
-   dev_dbg(>pdev->dev, "CTE: type:%3d  offset:%3d (%3d)  length:%3d 
(%3d)  s2c:%d  c2s:%d  irq_count:%d  base_irq:%d\n",
+   dev_dbg(>pdev->dev,
+   "CTE: type:%3d  offset:%3d (%3d)  length:%3d (%3d)  s2c:%d  
c2s:%d  irq_count:%d  base_irq:%d\n",
cte->type,
cte->offset,
cte->offset / 4096,
@@ -107,7 +111,14 @@ static int probe_core_basic(unsigned int core_num, struct 
kp2000_device *pcard,
.ddna  = pcard->ddna,
};
 
-   dev_dbg(>pdev->dev, "Found Basic core: type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found Basic core: type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
cell.platform_data = _pdata;
cell.pdata_size = sizeof(struct kpc_core_device_platdata);
@@ -290,7 +301,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
struct kpc_uio_device *kudev;
int rv;
 
-   dev_dbg(>pdev->dev, "Found UIO core:   type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found UIO core:   type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
kudev = kzalloc(sizeof(*kudev), GFP_KERNEL);
if (!kudev)
@@ -315,10 +333,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
 
kudev->uioinfo.mem[0].name = "uiomap";
kudev->uioinfo.mem[0].addr = pci_resource_start(pcard->pdev, REG_BAR) + 
cte.offset;
-   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1); // Round up to nearest PAGE_SIZE boundary
+
+   // Round up to nearest PAGE_SIZE boundary
+   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1);
kudev->uioinfo.mem[0].memtype = UIO_MEM_PHYS;
 
-   kudev->dev = device_create(kpc_uio_class, >pdev->dev, MKDEV(0, 
0), kudev, "%s.%d.%d.%d", kudev->uioinfo.name, pcard->card_num, cte.type, 
kudev->core_num);
+   kudev->dev = device_create(kpc_uio_class,
+  >pdev->dev, MKDEV(0, 0), kudev, 
"%s.%d.%d.%d",
+  kudev->uioinfo.name, pcard->card_num, 
cte.type, kudev->core_num);
if (IS_ERR(kudev->dev)) {
dev_err(>pdev->dev, "%s: device_create failed!\n",
__func__);
@@ -341,7 +363,9 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
return 0;
 }
 
-static int  create_dma_engine_core(struct kp2000_device *pcard, size_t 
engine_regs

[PATCH v2 2/2] staging: kpc2000: kpc_dma: rename show function per convention

[Deepak R Varma]

Rename show_engine_regs show function to engine_regs_show as per the
convention followed. The show function macro DEVICE_ATTR is replaced
by DEVICE_ATTR_RO. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v1:
   - Replace DEVICE_ATTR by DEVICE_ATTR_RO as suggested by Greg.

 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c 
b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index 7698e5ef2a7c..aa9f96793e59 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -50,7 +50,7 @@ static void kpc_dma_del_device(struct kpc_dma_device *ldev)
 }
 
 /**  SysFS Attributes **/
-static ssize_t  show_engine_regs(struct device *dev, struct device_attribute 
*attr, char *buf)
+static ssize_t  engine_regs_show(struct device *dev, struct device_attribute 
*attr, char *buf)
 {
struct kpc_dma_device *ldev;
struct platform_device *pldev = to_platform_device(dev);
@@ -80,7 +80,7 @@ static ssize_t  show_engine_regs(struct device *dev, struct 
device_attribute *at
ldev->desc_completed
);
 }
-static DEVICE_ATTR(engine_regs, 0444, show_engine_regs, NULL);
+static DEVICE_ATTR_RO(engine_regs);
 
 static const struct attribute *ndd_attr_list[] = {
_attr_engine_regs.attr,
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/3] staging: kpc2000: Use BIT macro instead of bit masking

[Deepak R Varma]

Replace bit masking by BIT macro. This resolves checkpatch issue
"CHECK: Prefer using the BIT macro"

Signed-off-by: Deepak R Varma 
---
Changes since v1:
   - Separate specific checkpatch issues into individual patches.
   - Introduced patch 3/3.
   - Suggested by Vaishali T.

 drivers/staging/kpc2000/kpc2000/dma_common_defs.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h 
b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
index 8bc78be3c259..613c4898f65e 100644
--- a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
+++ b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
@@ -8,13 +8,13 @@
 #define KPC_DMA_ENGINE_SIZE 0x0100
 #define ENGINE_CAP_PRESENT_MASK 0x1
 
-#define KPC_DMA_CARD_IRQ_ENABLE (1 << 0)
-#define KPC_DMA_CARD_IRQ_ACTIVE (1 << 1)
-#define KPC_DMA_CARD_IRQ_PENDING(1 << 2)
-#define KPC_DMA_CARD_IRQ_MSI(1 << 3)
-#define KPC_DMA_CARD_USER_INTERRUPT_MODE(1 << 4)
-#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  (1 << 5)
-#define KPC_DMA_CARD_IRQ_MSIX_MODE  (1 << 6)
+#define KPC_DMA_CARD_IRQ_ENABLE BIT(0)
+#define KPC_DMA_CARD_IRQ_ACTIVE BIT(1)
+#define KPC_DMA_CARD_IRQ_PENDINGBIT(2)
+#define KPC_DMA_CARD_IRQ_MSIBIT(3)
+#define KPC_DMA_CARD_USER_INTERRUPT_MODEBIT(4)
+#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  BIT(5)
+#define KPC_DMA_CARD_IRQ_MSIX_MODE  BIT(6)
 #define KPC_DMA_CARD_MAX_PAYLOAD_SIZE_MASK  0x0700
 #define KPC_DMA_CARD_MAX_READ_REQUEST_SIZE_MASK 0x7000
 #define KPC_DMA_CARD_S2C_INTERRUPT_STATUS_MASK  0x00FF
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

Reformat lines that exceed 100 column in length. Issue reported by
checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v1:
   - No change in this patch.
   - Patch 2/2 has a change.

 drivers/staging/kpc2000/kpc_dma/dma.c | 27 +---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 44 +++
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  9 ++--
 3 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c 
b/drivers/staging/kpc2000/kpc_dma/dma.c
index 452a3f7c835d..b8d8294aa4c3 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -16,7 +16,8 @@ irqreturn_t  ndd_irq_handler(int irq, void *dev_id)
 {
struct kpc_dma_device *ldev = (struct kpc_dma_device *)dev_id;
 
-   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) || 
(ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
+   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) ||
+   (ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
schedule_work(>irq_work);
 
return IRQ_HANDLED;
@@ -39,7 +40,9 @@ void  ndd_irq_worker(struct work_struct *ws)
cur = eng->desc_completed;
do {
cur = cur->Next;
-   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n", cur, cur->acd);
+   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n",
+   cur,
+   cur->acd);
BUG_ON(cur == eng->desc_next); // Ordering failure.
 
if (cur->DescControlFlags & DMA_DESC_CTL_SOP) {
@@ -56,7 +59,9 @@ void  ndd_irq_worker(struct work_struct *ws)
 
if (cur->DescControlFlags & DMA_DESC_CTL_EOP) {
if (cur->acd)
-   transfer_complete_cb(cur->acd, 
eng->accumulated_bytes, eng->accumulated_flags | ACD_FLAG_DONE);
+   transfer_complete_cb(cur->acd,
+eng->accumulated_bytes,
+eng->accumulated_flags | 
ACD_FLAG_DONE);
}
 
eng->desc_completed = cur;
@@ -103,7 +108,10 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
eng->dir = DMA_TO_DEVICE;
 
eng->desc_pool_cnt = desc_cnt;
-   eng->desc_pool = dma_pool_create("KPC DMA Descriptors", 
>pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 4096);
+   eng->desc_pool = dma_pool_create("KPC DMA Descriptors",
+>pldev->dev,
+sizeof(struct kpc_dma_descriptor),
+DMA_DESC_ALIGNMENT, 4096);
 
eng->desc_pool_first = dma_pool_alloc(eng->desc_pool, GFP_KERNEL | 
GFP_DMA, _handle);
if (!eng->desc_pool_first) {
@@ -141,7 +149,11 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
INIT_WORK(>irq_work, ndd_irq_worker);
 
// Grab IRQ line
-   rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED, 
KP_DRIVER_NAME_DMA_CONTROLLER, eng);
+   rv = request_irq(eng->irq,
+ndd_irq_handler,
+IRQF_SHARED,
+KP_DRIVER_NAME_DMA_CONTROLLER,
+eng);
if (rv) {
dev_err(>pldev->dev, "%s: failed to request_irq: %d\n", 
__func__, rv);
return rv;
@@ -195,7 +207,10 @@ void  stop_dma_engine(struct kpc_dma_device *eng)
}
 
// Clear any persistent bits just to make sure there is no residue from 
the reset
-   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE 
| ENG_CTL_DESC_ALIGN_ERR | ENG_CTL_DESC_FETCH_ERR | ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END | ENG_CTL_DMA_WAITING_PERSIST), 0);
+   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE |
+   ENG_CTL_DESC_ALIGN_ERR | 
ENG_CTL_DESC_FETCH_ERR |
+   ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END |
+   ENG_CTL_DMA_WAITING_PERSIST), 0);
 
// Reset performance counters
 
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index e1c7c04f16fe..b929987844ff 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -76,7 +76,11 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 
// Lock the user buffer pages in memory, and hold on to the page 
pointers (for the sglist)
mmap_read_lock(current->mm);  /*  get memory ma

[PATCH v2 2/3] staging: kpc2000: re-indent code for better readability

[Deepak R Varma]

Re-indent code as per the coding style guidelines. The changes improve
code readability. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
Changes since v1:
  - Separate specific checkpatch issues into individual patches.
  - Update patch subject and description to specific issue being fixed.
  - Introduced patch 3/3.
  - Suggested by Vaishali T.

 drivers/staging/kpc2000/kpc2000/core.c| 3 ++-
 drivers/staging/kpc2000/kpc2000/dma_common_defs.h | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c 
b/drivers/staging/kpc2000/kpc2000/core.c
index 358d7b2f4ad1..6462a3059fb0 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -124,6 +124,7 @@ static ssize_t cpld_reconfigure(struct device *dev,
writeq(wr_val, pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
return count;
 }
+
 static DEVICE_ATTR(cpld_reconfigure, 0220, NULL, cpld_reconfigure);
 
 static ssize_t irq_mask_reg_show(struct device *dev,
@@ -367,7 +368,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
 
pcard->dma_bar_base = ioremap(dma_bar_phys_addr,
- dma_bar_phys_len);
+ dma_bar_phys_len);
if (!pcard->dma_bar_base) {
dev_err(>pdev->dev,
"probe: DMA_BAR could not remap memory to virtual 
space\n");
diff --git a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h 
b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
index 21450e3d408f..8bc78be3c259 100644
--- a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
+++ b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
@@ -6,8 +6,7 @@
 #define KPC_DMA_S2C_BASE_OFFSET 0x
 #define KPC_DMA_C2S_BASE_OFFSET 0x2000
 #define KPC_DMA_ENGINE_SIZE 0x0100
-#define  ENGINE_CAP_PRESENT_MASK0x1
-
+#define ENGINE_CAP_PRESENT_MASK 0x1
 
 #define KPC_DMA_CARD_IRQ_ENABLE (1 << 0)
 #define KPC_DMA_CARD_IRQ_ACTIVE (1 << 1)
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: kpc2000: kpc_dma: rename show function per convention

[Deepak R Varma]

On Wed, Oct 21, 2020 at 07:50:31AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 21, 2020 at 10:40:21AM +0530, Deepak R Varma wrote:
> > Rename show_engine_regs to engine_regs_show as per the convention
> > followed. Issue reported by checkpatch script.
> > 
> > Signed-off-by: Deepak R Varma 
> > ---
> >  drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c 
> > b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > index 7698e5ef2a7c..b6d1afbd452d 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
> > @@ -50,7 +50,7 @@ static void kpc_dma_del_device(struct kpc_dma_device 
> > *ldev)
> >  }
> >  
> >  /**  SysFS Attributes **/
> > -static ssize_t  show_engine_regs(struct device *dev, struct 
> > device_attribute *attr, char *buf)
> > +static ssize_t  engine_regs_show(struct device *dev, struct 
> > device_attribute *attr, char *buf)
> >  {
> > struct kpc_dma_device *ldev;
> > struct platform_device *pldev = to_platform_device(dev);
> > @@ -80,7 +80,7 @@ static ssize_t  show_engine_regs(struct device *dev, 
> > struct device_attribute *at
> > ldev->desc_completed
> > );
> >  }
> > -static DEVICE_ATTR(engine_regs, 0444, show_engine_regs, NULL);
> > +static DEVICE_ATTR(engine_regs, 0444, engine_regs_show, NULL);
> 
> Shouldn't this just be using a DEVICE_ATTR_RO() macro instead?  Make
> that change and the name will be fixed up at the same time.
> 

Thank you for the feedback. I will review what the mentioned macro does
and how it can be implemented. Will send a revised patch with the change
suggested.

> And did checkpatch really complain about this?  What was the actual
> message it produced?

Yes, the WARNING message from checkpatch was:

WARNING: Consider renaming function(s) 'show_engine_regs' to 'engine_regs_show' 

+#82: FILE: drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c:82:
 
+}


> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH 1/2] staging: kpc2000: resolve various code style issues

[Deepak R Varma]

On Wed, Oct 21, 2020 at 10:11:52AM +0530, Vaishali Thakkar wrote:
> On Wed, Oct 21, 2020 at 8:33 AM Deepak R Varma  wrote:
> >
> > Multiple issues reported by checkpatch script around lines exceeding 100
> > columns, indentation of function parameters, extra blank lines. These
> > code formatting changes improves the code readability.
> 
> Please send separate patches while fixing different checkpatch
> warnings. It's a good idea to fix different warnings together if
> it's for the same block of code but otherwise your patch should
> handle one similar change at a time.
> 
Okay. Will change my old commit and resend the patch set.

Thank you,
Deepak.

> > Signed-off-by: Deepak R Varma 
> > ---
> >  drivers/staging/kpc2000/kpc2000/cell_probe.c  | 71 ++-
> >  drivers/staging/kpc2000/kpc2000/core.c|  3 +-
> >  .../staging/kpc2000/kpc2000/dma_common_defs.h |  3 +-
> >  3 files changed, 58 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c 
> > b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> > index 738122afc2ae..e7e963d62699 100644
> > --- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> > +++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> > @@ -30,9 +30,12 @@
> >   *
> >   */
> >
> > -#define KPC_OLD_DMA_CH_NUM(present, channel)   ((present) ? (0x8 | 
> > ((channel) & 0x7)) : 0)
> > -#define KPC_OLD_S2C_DMA_CH_NUM(cte)   
> > KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, cte.s2c_dma_channel_num)
> > -#define KPC_OLD_C2S_DMA_CH_NUM(cte)   
> > KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, cte.c2s_dma_channel_num)
> > +#define KPC_OLD_DMA_CH_NUM(present, channel)   \
> > +   ((present) ? (0x8 | ((channel) & 0x7)) : 0)
> > +#define KPC_OLD_S2C_DMA_CH_NUM(cte)   \
> > +   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, 
> > cte.s2c_dma_channel_num)
> > +#define KPC_OLD_C2S_DMA_CH_NUM(cte)   \
> > +   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, 
> > cte.c2s_dma_channel_num)
> >
> >  #define KP_CORE_ID_INVALID  0
> >  #define KP_CORE_ID_I2C  3
> > @@ -67,7 +70,8 @@ void  parse_core_table_entry_v0(struct core_table_entry 
> > *cte, const u64 read_val
> >  static
> >  void dbg_cte(struct kp2000_device *pcard, struct core_table_entry *cte)
> >  {
> > -   dev_dbg(>pdev->dev, "CTE: type:%3d  offset:%3d (%3d)  
> > length:%3d (%3d)  s2c:%d  c2s:%d  irq_count:%d  base_irq:%d\n",
> > +   dev_dbg(>pdev->dev,
> > +   "CTE: type:%3d  offset:%3d (%3d)  length:%3d (%3d)  s2c:%d  
> > c2s:%d  irq_count:%d  base_irq:%d\n",
> > cte->type,
> > cte->offset,
> > cte->offset / 4096,
> > @@ -107,7 +111,14 @@ static int probe_core_basic(unsigned int core_num, 
> > struct kp2000_device *pcard,
> > .ddna  = pcard->ddna,
> > };
> >
> > -   dev_dbg(>pdev->dev, "Found Basic core: type = %02d  dma = 
> > %02x / %02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
> > KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
> > cte.length, cte.length / 8);
> > +   dev_dbg(>pdev->dev,
> > +   "Found Basic core: type = %02d  dma = %02x / %02x  offset = 
> > 0x%x  length = 0x%x (%d regs)\n",
> > +   cte.type,
> > +   KPC_OLD_S2C_DMA_CH_NUM(cte),
> > +   KPC_OLD_C2S_DMA_CH_NUM(cte),
> > +   cte.offset,
> > +   cte.length,
> > +   cte.length / 8);
> >
> > cell.platform_data = _pdata;
> > cell.pdata_size = sizeof(struct kpc_core_device_platdata);
> > @@ -290,7 +301,14 @@ static int probe_core_uio(unsigned int core_num, 
> > struct kp2000_device *pcard,
> > struct kpc_uio_device *kudev;
> > int rv;
> >
> > -   dev_dbg(>pdev->dev, "Found UIO core:   type = %02d  dma = 
> > %02x / %02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
> > KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
> > cte.length, cte.length / 8);
> > +   dev_dbg(>pdev->dev,
> > +   "Found UIO core:   type = %02d  dma = %02x / %02x  offset = 
> > 0x%x  length = 0x%x (%d regs)\n",
> > +   cte.type,
> > +   KPC_OLD_S2C_DMA_CH_NUM(cte),
> > +   KPC_OLD_C2S_DMA_CH_NUM(cte),
> > +   cte.offset,
> >

[PATCH 2/2] staging: kpc2000: kpc_dma: rename show function per convention

[Deepak R Varma]

Rename show_engine_regs to engine_regs_show as per the convention
followed. Issue reported by checkpatch script.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c 
b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
index 7698e5ef2a7c..b6d1afbd452d 100644
--- a/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
+++ b/drivers/staging/kpc2000/kpc_dma/kpc_dma_driver.c
@@ -50,7 +50,7 @@ static void kpc_dma_del_device(struct kpc_dma_device *ldev)
 }
 
 /**  SysFS Attributes **/
-static ssize_t  show_engine_regs(struct device *dev, struct device_attribute 
*attr, char *buf)
+static ssize_t  engine_regs_show(struct device *dev, struct device_attribute 
*attr, char *buf)
 {
struct kpc_dma_device *ldev;
struct platform_device *pldev = to_platform_device(dev);
@@ -80,7 +80,7 @@ static ssize_t  show_engine_regs(struct device *dev, struct 
device_attribute *at
ldev->desc_completed
);
 }
-static DEVICE_ATTR(engine_regs, 0444, show_engine_regs, NULL);
+static DEVICE_ATTR(engine_regs, 0444, engine_regs_show, NULL);
 
 static const struct attribute *ndd_attr_list[] = {
_attr_engine_regs.attr,
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: kpc2000: kpc_dma: rearrange lines exceeding 100 columns

[Deepak R Varma]

Reformat lines that exceed 100 column in length. Issue reported by
checkpatch script.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/kpc2000/kpc_dma/dma.c | 27 +---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 44 +++
 .../staging/kpc2000/kpc_dma/kpc_dma_driver.c  |  9 ++--
 3 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/dma.c 
b/drivers/staging/kpc2000/kpc_dma/dma.c
index 452a3f7c835d..b8d8294aa4c3 100644
--- a/drivers/staging/kpc2000/kpc_dma/dma.c
+++ b/drivers/staging/kpc2000/kpc_dma/dma.c
@@ -16,7 +16,8 @@ irqreturn_t  ndd_irq_handler(int irq, void *dev_id)
 {
struct kpc_dma_device *ldev = (struct kpc_dma_device *)dev_id;
 
-   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) || 
(ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
+   if ((GetEngineControl(ldev) & ENG_CTL_IRQ_ACTIVE) ||
+   (ldev->desc_completed->MyDMAAddr != GetEngineCompletePtr(ldev)))
schedule_work(>irq_work);
 
return IRQ_HANDLED;
@@ -39,7 +40,9 @@ void  ndd_irq_worker(struct work_struct *ws)
cur = eng->desc_completed;
do {
cur = cur->Next;
-   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n", cur, cur->acd);
+   dev_dbg(>pldev->dev, "Handling completed descriptor %p 
(acd = %p)\n",
+   cur,
+   cur->acd);
BUG_ON(cur == eng->desc_next); // Ordering failure.
 
if (cur->DescControlFlags & DMA_DESC_CTL_SOP) {
@@ -56,7 +59,9 @@ void  ndd_irq_worker(struct work_struct *ws)
 
if (cur->DescControlFlags & DMA_DESC_CTL_EOP) {
if (cur->acd)
-   transfer_complete_cb(cur->acd, 
eng->accumulated_bytes, eng->accumulated_flags | ACD_FLAG_DONE);
+   transfer_complete_cb(cur->acd,
+eng->accumulated_bytes,
+eng->accumulated_flags | 
ACD_FLAG_DONE);
}
 
eng->desc_completed = cur;
@@ -103,7 +108,10 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
eng->dir = DMA_TO_DEVICE;
 
eng->desc_pool_cnt = desc_cnt;
-   eng->desc_pool = dma_pool_create("KPC DMA Descriptors", 
>pldev->dev, sizeof(struct kpc_dma_descriptor), DMA_DESC_ALIGNMENT, 4096);
+   eng->desc_pool = dma_pool_create("KPC DMA Descriptors",
+>pldev->dev,
+sizeof(struct kpc_dma_descriptor),
+DMA_DESC_ALIGNMENT, 4096);
 
eng->desc_pool_first = dma_pool_alloc(eng->desc_pool, GFP_KERNEL | 
GFP_DMA, _handle);
if (!eng->desc_pool_first) {
@@ -141,7 +149,11 @@ int  setup_dma_engine(struct kpc_dma_device *eng, u32 
desc_cnt)
INIT_WORK(>irq_work, ndd_irq_worker);
 
// Grab IRQ line
-   rv = request_irq(eng->irq, ndd_irq_handler, IRQF_SHARED, 
KP_DRIVER_NAME_DMA_CONTROLLER, eng);
+   rv = request_irq(eng->irq,
+ndd_irq_handler,
+IRQF_SHARED,
+KP_DRIVER_NAME_DMA_CONTROLLER,
+eng);
if (rv) {
dev_err(>pldev->dev, "%s: failed to request_irq: %d\n", 
__func__, rv);
return rv;
@@ -195,7 +207,10 @@ void  stop_dma_engine(struct kpc_dma_device *eng)
}
 
// Clear any persistent bits just to make sure there is no residue from 
the reset
-   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE 
| ENG_CTL_DESC_ALIGN_ERR | ENG_CTL_DESC_FETCH_ERR | ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END | ENG_CTL_DMA_WAITING_PERSIST), 0);
+   SetClearEngineControl(eng, (ENG_CTL_IRQ_ACTIVE | ENG_CTL_DESC_COMPLETE |
+   ENG_CTL_DESC_ALIGN_ERR | 
ENG_CTL_DESC_FETCH_ERR |
+   ENG_CTL_SW_ABORT_ERR | 
ENG_CTL_DESC_CHAIN_END |
+   ENG_CTL_DMA_WAITING_PERSIST), 0);
 
// Reset performance counters
 
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index e1c7c04f16fe..b929987844ff 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -76,7 +76,11 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
 
// Lock the user buffer pages in memory, and hold on to the page 
pointers (for the sglist)
mmap_read_lock(current->mm);  /*  get memory map semaphore */
-   rv = pin_user_pages(iov_base, acd->page

[PATCH 2/2] staging: kpc2000: Use BIT macro instead of bit masking

[Deepak R Varma]

Replace bit masking by BIT macro. This resolves checkpatch issue
"CHECK: Prefer using the BIT macro"

Signed-off-by: Deepak R Varma 
---
 drivers/staging/kpc2000/kpc2000/dma_common_defs.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h 
b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
index 8bc78be3c259..613c4898f65e 100644
--- a/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
+++ b/drivers/staging/kpc2000/kpc2000/dma_common_defs.h
@@ -8,13 +8,13 @@
 #define KPC_DMA_ENGINE_SIZE 0x0100
 #define ENGINE_CAP_PRESENT_MASK 0x1
 
-#define KPC_DMA_CARD_IRQ_ENABLE (1 << 0)
-#define KPC_DMA_CARD_IRQ_ACTIVE (1 << 1)
-#define KPC_DMA_CARD_IRQ_PENDING(1 << 2)
-#define KPC_DMA_CARD_IRQ_MSI(1 << 3)
-#define KPC_DMA_CARD_USER_INTERRUPT_MODE(1 << 4)
-#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  (1 << 5)
-#define KPC_DMA_CARD_IRQ_MSIX_MODE  (1 << 6)
+#define KPC_DMA_CARD_IRQ_ENABLE BIT(0)
+#define KPC_DMA_CARD_IRQ_ACTIVE BIT(1)
+#define KPC_DMA_CARD_IRQ_PENDINGBIT(2)
+#define KPC_DMA_CARD_IRQ_MSIBIT(3)
+#define KPC_DMA_CARD_USER_INTERRUPT_MODEBIT(4)
+#define KPC_DMA_CARD_USER_INTERRUPT_ACTIVE  BIT(5)
+#define KPC_DMA_CARD_IRQ_MSIX_MODE  BIT(6)
 #define KPC_DMA_CARD_MAX_PAYLOAD_SIZE_MASK  0x0700
 #define KPC_DMA_CARD_MAX_READ_REQUEST_SIZE_MASK 0x7000
 #define KPC_DMA_CARD_S2C_INTERRUPT_STATUS_MASK  0x00FF
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: kpc2000: resolve various code style issues

[Deepak R Varma]

Multiple issues reported by checkpatch script around lines exceeding 100
columns, indentation of function parameters, extra blank lines. These
code formatting changes improves the code readability.

Signed-off-by: Deepak R Varma 
---
 drivers/staging/kpc2000/kpc2000/cell_probe.c  | 71 ++-
 drivers/staging/kpc2000/kpc2000/core.c|  3 +-
 .../staging/kpc2000/kpc2000/dma_common_defs.h |  3 +-
 3 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c 
b/drivers/staging/kpc2000/kpc2000/cell_probe.c
index 738122afc2ae..e7e963d62699 100644
--- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
+++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
@@ -30,9 +30,12 @@
  *
  */
 
-#define KPC_OLD_DMA_CH_NUM(present, channel)   ((present) ? (0x8 | ((channel) 
& 0x7)) : 0)
-#define KPC_OLD_S2C_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, 
cte.s2c_dma_channel_num)
-#define KPC_OLD_C2S_DMA_CH_NUM(cte)   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, 
cte.c2s_dma_channel_num)
+#define KPC_OLD_DMA_CH_NUM(present, channel)   \
+   ((present) ? (0x8 | ((channel) & 0x7)) : 0)
+#define KPC_OLD_S2C_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.s2c_dma_present, cte.s2c_dma_channel_num)
+#define KPC_OLD_C2S_DMA_CH_NUM(cte)   \
+   KPC_OLD_DMA_CH_NUM(cte.c2s_dma_present, cte.c2s_dma_channel_num)
 
 #define KP_CORE_ID_INVALID  0
 #define KP_CORE_ID_I2C  3
@@ -67,7 +70,8 @@ void  parse_core_table_entry_v0(struct core_table_entry *cte, 
const u64 read_val
 static
 void dbg_cte(struct kp2000_device *pcard, struct core_table_entry *cte)
 {
-   dev_dbg(>pdev->dev, "CTE: type:%3d  offset:%3d (%3d)  length:%3d 
(%3d)  s2c:%d  c2s:%d  irq_count:%d  base_irq:%d\n",
+   dev_dbg(>pdev->dev,
+   "CTE: type:%3d  offset:%3d (%3d)  length:%3d (%3d)  s2c:%d  
c2s:%d  irq_count:%d  base_irq:%d\n",
cte->type,
cte->offset,
cte->offset / 4096,
@@ -107,7 +111,14 @@ static int probe_core_basic(unsigned int core_num, struct 
kp2000_device *pcard,
.ddna  = pcard->ddna,
};
 
-   dev_dbg(>pdev->dev, "Found Basic core: type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found Basic core: type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
cell.platform_data = _pdata;
cell.pdata_size = sizeof(struct kpc_core_device_platdata);
@@ -290,7 +301,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
struct kpc_uio_device *kudev;
int rv;
 
-   dev_dbg(>pdev->dev, "Found UIO core:   type = %02d  dma = %02x / 
%02x  offset = 0x%x  length = 0x%x (%d regs)\n", cte.type, 
KPC_OLD_S2C_DMA_CH_NUM(cte), KPC_OLD_C2S_DMA_CH_NUM(cte), cte.offset, 
cte.length, cte.length / 8);
+   dev_dbg(>pdev->dev,
+   "Found UIO core:   type = %02d  dma = %02x / %02x  offset = 
0x%x  length = 0x%x (%d regs)\n",
+   cte.type,
+   KPC_OLD_S2C_DMA_CH_NUM(cte),
+   KPC_OLD_C2S_DMA_CH_NUM(cte),
+   cte.offset,
+   cte.length,
+   cte.length / 8);
 
kudev = kzalloc(sizeof(*kudev), GFP_KERNEL);
if (!kudev)
@@ -315,10 +333,14 @@ static int probe_core_uio(unsigned int core_num, struct 
kp2000_device *pcard,
 
kudev->uioinfo.mem[0].name = "uiomap";
kudev->uioinfo.mem[0].addr = pci_resource_start(pcard->pdev, REG_BAR) + 
cte.offset;
-   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1); // Round up to nearest PAGE_SIZE boundary
+
+   // Round up to nearest PAGE_SIZE boundary
+   kudev->uioinfo.mem[0].size = (cte.length + PAGE_SIZE - 1) & ~(PAGE_SIZE 
- 1);
kudev->uioinfo.mem[0].memtype = UIO_MEM_PHYS;
 
-   kudev->dev = device_create(kpc_uio_class, >pdev->dev, MKDEV(0, 
0), kudev, "%s.%d.%d.%d", kudev->uioinfo.name, pcard->card_num, cte.type, 
kudev->core_num);
+   kudev->dev = device_create(kpc_uio_class,
+  >pdev->dev, MKDEV(0, 0), kudev, 
"%s.%d.%d.%d",
+  kudev->uioinfo.name, pcard->card_num, 
cte.type, kudev->core_num);
if (IS_ERR(kudev->dev)) {
dev_err(>pdev->dev, "%s: device_create failed!\n",
 

[PATCH v3] staging: comedi: tests: Simplify conditional evaluation

[Deepak R Varma]

Boolean comparison of the condition inside unittest function is
unnecessary and can be simplified by directly using the condition
outcome for evaluation. Issue reported by :
scripts/coccinelle/misc/boolinit.cocci

Signed-off-by: Deepak R Varma 
---
Changes since v2:
   - Update patch subject to include tests tag as well to be more
 informative of the scope of change. Suggested by Ian Abbott.
Changes since v1:
   - Corrected wrongly inverted tests. Feedback from Ian Abbott.
   - Note:
   This patch was sent earlier as part of a patch set containing 2
   patches. The second patch of the patch set is dropped. Hence
   sending this standalone patch as v2 version. This is based on the
   feedback from Ian Abbott and Julia L.

 drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c 
b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index eaefaf596a37..4061b3b5f8e9 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
const struct ni_route_tables *T = _tables;
 
init_pci_fake();
-   unittest(route_register_is_valid(4, O(4), T) == false,
+   unittest(!route_register_is_valid(4, O(4), T),
 "check for bad source 4-->4\n");
-   unittest(route_register_is_valid(0, O(1), T) == true,
+   unittest(route_register_is_valid(0, O(1), T),
 "find first source\n");
-   unittest(route_register_is_valid(4, O(6), T) == true,
+   unittest(route_register_is_valid(4, O(6), T),
 "find middle source\n");
-   unittest(route_register_is_valid(9, O(8), T) == true,
+   unittest(route_register_is_valid(9, O(8), T),
 "find last source");
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: comedi: Simplify conditional evaluation

[Deepak R Varma]

Boolean comparison of the condition inside unittest function is
unnecessary and can be simplified by directly using the condition
outcome for evaluation. Issue reported by :
scripts/coccinelle/misc/boolinit.cocci

Signed-off-by: Deepak R Varma 
---

Changes since v1:
   - Corrected wrongly inverted tests. Feedback from Ian Abbott.
   - Note:
   This patch was sent earlier as part of a patch set containing 2
   patches. The second patch of the patch set is dropped. Hence
   sending this standalone patch as v2 version. This is based on the
   feedback from Ian Abbott and Julia L.

 drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c 
b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index eaefaf596a37..4061b3b5f8e9 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
const struct ni_route_tables *T = _tables;
 
init_pci_fake();
-   unittest(route_register_is_valid(4, O(4), T) == false,
+   unittest(!route_register_is_valid(4, O(4), T),
 "check for bad source 4-->4\n");
-   unittest(route_register_is_valid(0, O(1), T) == true,
+   unittest(route_register_is_valid(0, O(1), T),
 "find first source\n");
-   unittest(route_register_is_valid(4, O(6), T) == true,
+   unittest(route_register_is_valid(4, O(6), T),
 "find middle source\n");
-   unittest(route_register_is_valid(9, O(8), T) == true,
+   unittest(route_register_is_valid(9, O(8), T),
 "find last source");
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability

[Deepak R Varma]

On Mon, Oct 19, 2020 at 12:34:15PM +0100, Ian Abbott wrote:
> On 19/10/2020 11:57, Deepak R Varma wrote:
> > On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
> > > 
> > > 
> > > On Mon, 19 Oct 2020, Ian Abbott wrote:
> > > 
> > > > On 18/10/2020 20:49, Deepak R Varma wrote:
> > > > > Instructions split on multiple lines can be combined on a single line
> > > > > for improved readability of the code.
> > > > > 
> > > > > Signed-off-by: Deepak R Varma 
> > > > > ---
> > > > >.../staging/comedi/drivers/tests/ni_routes_test.c| 12 
> > > > > 
> > > > >1 file changed, 4 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > index 7db83cf5e4aa..a3b1be623861 100644
> > > > > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > > > @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
> > > > >   const struct ni_route_tables *T = _tables;
> > > > >   init_pci_fake();
> > > > > - unittest(!route_register_is_valid(4, O(4), T),
> > > > > -  "check for bad source 4-->4\n");
> > > > > - unittest(!route_register_is_valid(0, O(1), T),
> > > > > -  "find first source\n");
> > > > > - unittest(!route_register_is_valid(4, O(6), T),
> > > > > -  "find middle source\n");
> > > > > - unittest(!route_register_is_valid(9, O(8), T),
> > > > > -  "find last source");
> > > > > + unittest(!route_register_is_valid(4, O(4), T), "check for bad 
> > > > > source
> > > > > 4-->4\n");
> > > > > + unittest(!route_register_is_valid(0, O(1), T), "find first 
> > > > > source\n");
> > > > > + unittest(!route_register_is_valid(4, O(6), T), "find middle
> > > > > source\n");
> > > > > + unittest(!route_register_is_valid(9, O(8), T), "find last 
> > > > > source");
> > > > >}
> > > > >  void test_ni_check_trigger_arg(void)
> > > > > 
> > > > 
> > > > Is it worth breaking the 80-column limit for this?
> > > 
> > > Deepak,
> > > 
> > > It was much nicer before.
> > > 
> > > It can be awkward to break eg a + operation at the 80 character limit.
> > > But function argument stand by themselves.
> > > 
> > > julia
> > > 
> > 
> > Hi Julia and Ian,
> > I wanted to take advantage of the relaxation of 80 column limit to 100
> > columns and hence proposed combining the lines. Are you saying this is
> > allowed only in certain cases?
> > 
> > Please confirm and I will handle it accordingly.
> 
> Hi Deepak,
> 
> 80 columns is still the preferred limit.  I think the relaxation is mostly
> to avoid the need to split sub-expressions across lines in really ugly ways
> to keep within the 80 columns at the expense of readability.
> 

Thank you Ian. That sounds good. I will just send the corrected patch 1
and will scrap patch 2.

Can I just send a standalone patch as v2 instead of a patch set of
single patch?

Deepak.

> -- 
> -=( Ian Abbott  || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: comedi: combine split lines for improved readability

[Deepak R Varma]

On Mon, Oct 19, 2020 at 12:41:14PM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 19 Oct 2020, Ian Abbott wrote:
> 
> > On 18/10/2020 20:49, Deepak R Varma wrote:
> > > Instructions split on multiple lines can be combined on a single line
> > > for improved readability of the code.
> > >
> > > Signed-off-by: Deepak R Varma 
> > > ---
> > >   .../staging/comedi/drivers/tests/ni_routes_test.c| 12 
> > >   1 file changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > index 7db83cf5e4aa..a3b1be623861 100644
> > > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > > @@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
> > >   const struct ni_route_tables *T = _tables;
> > >   init_pci_fake();
> > > - unittest(!route_register_is_valid(4, O(4), T),
> > > -  "check for bad source 4-->4\n");
> > > - unittest(!route_register_is_valid(0, O(1), T),
> > > -  "find first source\n");
> > > - unittest(!route_register_is_valid(4, O(6), T),
> > > -  "find middle source\n");
> > > - unittest(!route_register_is_valid(9, O(8), T),
> > > -  "find last source");
> > > + unittest(!route_register_is_valid(4, O(4), T), "check for bad source
> > > 4-->4\n");
> > > + unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
> > > + unittest(!route_register_is_valid(4, O(6), T), "find middle
> > > source\n");
> > > + unittest(!route_register_is_valid(9, O(8), T), "find last source");
> > >   }
> > > void test_ni_check_trigger_arg(void)
> > >
> >
> > Is it worth breaking the 80-column limit for this?
> 
> Deepak,
> 
> It was much nicer before.
> 
> It can be awkward to break eg a + operation at the 80 character limit.
> But function argument stand by themselves.
> 
> julia
> 

Hi Julia and Ian,
I wanted to take advantage of the relaxation of 80 column limit to 100
columns and hence proposed combining the lines. Are you saying this is
allowed only in certain cases?

Please confirm and I will handle it accordingly.

Thank you,
Deepak.

> >
> > --
> > -=( Ian Abbott  || MEV Ltd. is a company  )=-
> > -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> > -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> > -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/outreachy-kernel/f81a537c-c0fb-5133-52a3-825128814435%40mev.co.uk.
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: comedi: Simplify conditional evaluation

[Deepak R Varma]

On Mon, Oct 19, 2020 at 11:17:38AM +0100, Ian Abbott wrote:
> On 18/10/2020 20:48, Deepak R Varma wrote:
> > Boolean comparison of the condition inside unittest function is
> > unnecessary and can be simplified by directly using the condition
> > outcome for evaluation. Issue reported by :
> > scripts/coccinelle/misc/boolinit.cocci
> > 
> > Signed-off-by: Deepak R Varma 
> > ---
> >   drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c 
> > b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > index eaefaf596a37..7db83cf5e4aa 100644
> > --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
> > @@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
> > const struct ni_route_tables *T = _tables;
> > init_pci_fake();
> > -   unittest(route_register_is_valid(4, O(4), T) == false,
> > +   unittest(!route_register_is_valid(4, O(4), T),
> >  "check for bad source 4-->4\n");
> > -   unittest(route_register_is_valid(0, O(1), T) == true,
> > +   unittest(!route_register_is_valid(0, O(1), T),
> >  "find first source\n");
> > -   unittest(route_register_is_valid(4, O(6), T) == true,
> > +   unittest(!route_register_is_valid(4, O(6), T),
> >  "find middle source\n");
> > -   unittest(route_register_is_valid(9, O(8), T) == true,
> > +   unittest(!route_register_is_valid(9, O(8), T),
> >  "find last source");
> >   }
> 
> NAK.
> 
> It looks like you have inadvertently inverted some of the tests.

Hi Ian,
Thank you for catching that. I am correcting it and will send a v2
shortly.

Deepak.

> 
> -- 
> -=( Ian Abbott  || MEV Ltd. is a company  )=-
> -=( registered in England & Wales.  Regd. number: 02862268.  )=-
> -=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
> -=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: comedi: combine split lines for improved readability

[Deepak R Varma]

Instructions split on multiple lines can be combined on a single line
for improved readability of the code.

Signed-off-by: Deepak R Varma 
---
 .../staging/comedi/drivers/tests/ni_routes_test.c| 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c 
b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index 7db83cf5e4aa..a3b1be623861 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,14 +499,10 @@ void test_route_register_is_valid(void)
const struct ni_route_tables *T = _tables;
 
init_pci_fake();
-   unittest(!route_register_is_valid(4, O(4), T),
-"check for bad source 4-->4\n");
-   unittest(!route_register_is_valid(0, O(1), T),
-"find first source\n");
-   unittest(!route_register_is_valid(4, O(6), T),
-"find middle source\n");
-   unittest(!route_register_is_valid(9, O(8), T),
-"find last source");
+   unittest(!route_register_is_valid(4, O(4), T), "check for bad source 
4-->4\n");
+   unittest(!route_register_is_valid(0, O(1), T), "find first source\n");
+   unittest(!route_register_is_valid(4, O(6), T), "find middle source\n");
+   unittest(!route_register_is_valid(9, O(8), T), "find last source");
 }
 
 void test_ni_check_trigger_arg(void)
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: comedi: Simplify conditional evaluation

[Deepak R Varma]

Boolean comparison of the condition inside unittest function is
unnecessary and can be simplified by directly using the condition
outcome for evaluation. Issue reported by :
scripts/coccinelle/misc/boolinit.cocci

Signed-off-by: Deepak R Varma 
---
 drivers/staging/comedi/drivers/tests/ni_routes_test.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c 
b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
index eaefaf596a37..7db83cf5e4aa 100644
--- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c
+++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c
@@ -499,13 +499,13 @@ void test_route_register_is_valid(void)
const struct ni_route_tables *T = _tables;
 
init_pci_fake();
-   unittest(route_register_is_valid(4, O(4), T) == false,
+   unittest(!route_register_is_valid(4, O(4), T),
 "check for bad source 4-->4\n");
-   unittest(route_register_is_valid(0, O(1), T) == true,
+   unittest(!route_register_is_valid(0, O(1), T),
 "find first source\n");
-   unittest(route_register_is_valid(4, O(6), T) == true,
+   unittest(!route_register_is_valid(4, O(6), T),
 "find middle source\n");
-   unittest(route_register_is_valid(9, O(8), T) == true,
+   unittest(!route_register_is_valid(9, O(8), T),
 "find last source");
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel