| Message ID | 4947abeebc2cecca43137675519a2fb48577d3e5.1618859059.git.drv@mailo.com |
|---|---|
| State | New |
| Headers | show |
| Series | staging: media: atomisp: code cleanup fixes | expand |
Hi Deepak, On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote: > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a > warning. It is better to use pr_info() which comes with an > inbuilt KERN_INFO level. > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > index b572551f1a0d..a0f3c5c32f94 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd, > return -EINVAL; > } > > - printk("%s: before gc0310_write_reg_array %s\n", __func__, > + pr_info("%s: before gc0310_write_reg_array %s\n", __func__, > gc0310_res[dev->fmt_idx].desc); in a driver is best recommended to replace a raw printk call with a dev_*() log. Check few lines above here to see how it is done. > ret = startup(sd); > if (ret) { > -- > 2.25.1 > > thank you, fabio
On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote: > Hi Deepak, > > On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote: > > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a > > warning. It is better to use pr_info() which comes with an > > inbuilt KERN_INFO level. > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > index b572551f1a0d..a0f3c5c32f94 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd, > > return -EINVAL; > > } > > > > - printk("%s: before gc0310_write_reg_array %s\n", __func__, > > + pr_info("%s: before gc0310_write_reg_array %s\n", __func__, > > gc0310_res[dev->fmt_idx].desc); > > in a driver is best recommended to replace a raw printk call > with a dev_*() log. Check few lines above here to see > how it is done. Hello Fabio, thank you for the feedback. Should I resubmit this patch with additional printk() / pr_info() replacement by dev_*()? OR send in a new patch with this change? Just want to make sure I am not including more than one change type in the patch. deepak. > > > > ret = startup(sd); > > if (ret) { > > -- > > 2.25.1 > > > > > > > thank you, > > fabio
On Tue, Apr 20, 2021 at 10:43:53PM +0530, Deepak R Varma wrote: > On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote: > > Hi Deepak, > > > > On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote: > > > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a > > > warning. It is better to use pr_info() which comes with an > > > inbuilt KERN_INFO level. > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > --- > > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > index b572551f1a0d..a0f3c5c32f94 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd, > > > return -EINVAL; > > > } > > > > > > - printk("%s: before gc0310_write_reg_array %s\n", __func__, > > > + pr_info("%s: before gc0310_write_reg_array %s\n", __func__, > > > gc0310_res[dev->fmt_idx].desc); > > > > in a driver is best recommended to replace a raw printk call > > with a dev_*() log. Check few lines above here to see > > how it is done. > > Hello Fabio, > thank you for the feedback. Should I resubmit this patch with additional > printk() / pr_info() replacement by dev_*()? OR send in a new patch with > this change? Just want to make sure I am not including more than one > change type in the patch. > > deepak. > Hi Deepak, what I would do is to just resend this patch with replacement by dev_*(), which is just one change. Then if you want to replace all other raw printk/pr_*() occurrences is matter of another separate patch/series. > > > > > > > ret = startup(sd); > > > if (ret) { > > > -- > > > 2.25.1 > > > > > > > > > > > > thank you, > > > > fabio thank you, fabio
On Wed, Apr 21, 2021 at 09:22:17AM +0200, Fabio Aiuto wrote: > On Tue, Apr 20, 2021 at 10:43:53PM +0530, Deepak R Varma wrote: > > On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote: > > > Hi Deepak, > > > > > > On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote: > > > > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a > > > > warning. It is better to use pr_info() which comes with an > > > > inbuilt KERN_INFO level. > > > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > > --- > > > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > > index b572551f1a0d..a0f3c5c32f94 100644 > > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd, > > > > return -EINVAL; > > > > } > > > > > > > > - printk("%s: before gc0310_write_reg_array %s\n", __func__, > > > > + pr_info("%s: before gc0310_write_reg_array %s\n", __func__, > > > > gc0310_res[dev->fmt_idx].desc); > > > > > > in a driver is best recommended to replace a raw printk call > > > with a dev_*() log. Check few lines above here to see > > > how it is done. > > > > Hello Fabio, > > thank you for the feedback. Should I resubmit this patch with additional > > printk() / pr_info() replacement by dev_*()? OR send in a new patch with > > this change? Just want to make sure I am not including more than one > > change type in the patch. > > > > deepak. > > > > Hi Deepak, > > what I would do is to just resend this patch with replacement by dev_*(), > which is just one change. Then if you want to replace all other > raw printk/pr_*() occurrences is matter of another separate > patch/series. > Hi Fabio, That sounds good. Thank you for your comments. I will resubmit the patch set according to the feedbacks received. Have a good day. deepak. > > > > > > > > > > ret = startup(sd); > > > > if (ret) { > > > > -- > > > > 2.25.1 > > > > > > > > > > > > > > > > > thank you, > > > > > > fabio > > thank you, > > fabio
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c index b572551f1a0d..a0f3c5c32f94 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd, return -EINVAL; } - printk("%s: before gc0310_write_reg_array %s\n", __func__, + pr_info("%s: before gc0310_write_reg_array %s\n", __func__, gc0310_res[dev->fmt_idx].desc); ret = startup(sd); if (ret) {
printk() without KERN_<LEVEL> facility is flagged by checkpatch as a warning. It is better to use pr_info() which comes with an inbuilt KERN_INFO level. Signed-off-by: Deepak R Varma <drv@mailo.com> --- drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)