Re: [PATCH 1/2] staging: greybus: vibrator: use proper API for vibrator devices

[Johan Hovold]

On Tue, Jan 05, 2021 at 04:19:02PM +0100, Greg Kroah-Hartman wrote:
> The correct user/kernel api for vibrator devices is the Input rumble
> api, not a random sysfs file like the greybus vibrator driver currently
> uses.
> 
> Add support for the correct input api to the vibrator driver so that it
> hooks up to the kernel and userspace correctly.
> 
> Cc: Johan Hovold 
> Cc: Alex Elder 
> Signed-off-by: Greg Kroah-Hartman 
> ---
>  drivers/staging/greybus/vibrator.c | 59 ++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/staging/greybus/vibrator.c 
> b/drivers/staging/greybus/vibrator.c
> index 0e2b188e5ca3..94110cadb5bd 100644
> --- a/drivers/staging/greybus/vibrator.c
> +++ b/drivers/staging/greybus/vibrator.c
> @@ -13,13 +13,18 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  struct gb_vibrator_device {
>   struct gb_connection*connection;
> + struct input_dev*input;
>   struct device   *dev;
>   int minor;  /* vibrator minor number */
>   struct delayed_work delayed_work;
> + boolrunning;
> + boolon;
> + struct work_struct  play_work;
>  };
>  
>  /* Greybus Vibrator operation types */
> @@ -36,6 +41,7 @@ static int turn_off(struct gb_vibrator_device *vib)
>  
>   gb_pm_runtime_put_autosuspend(bundle);
>  
> + vib->on = false;

You update but never seem to actually use vib->on.

>   return ret;
>  }
>  
> @@ -59,11 +65,48 @@ static int turn_on(struct gb_vibrator_device *vib, u16 
> timeout_ms)
>   return ret;
>   }
>  
> + vib->on = true;
>   schedule_delayed_work(&vib->delayed_work, msecs_to_jiffies(timeout_ms));
>  
>   return 0;
>  }
>  
> +static void gb_vibrator_play_work(struct work_struct *work)
> +{
> + struct gb_vibrator_device *vib =
> + container_of(work, struct gb_vibrator_device, play_work);
> +
> + if (vib->running)

Is this test inverted?

> + turn_off(vib);
> + else
> + turn_on(vib, 100);

Why 100 ms?

Shouldn't it just be left on indefinitely with the new API?

> +}
> +
> +static int gb_vibrator_play_effect(struct input_dev *input, void *data,
> +struct ff_effect *effect)
> +{
> + struct gb_vibrator_device *vib = input_get_drvdata(input);
> + int level;
> +
> + level = effect->u.rumble.strong_magnitude;
> + if (!level)
> + level = effect->u.rumble.weak_magnitude;
> +
> + vib->running = level;
> + schedule_work(&vib->play_work);
> + return 0;
> +}
> +
> +static void gb_vibrator_close(struct input_dev *input)
> +{
> + struct gb_vibrator_device *vib = input_get_drvdata(input);
> +
> + cancel_delayed_work_sync(&vib->delayed_work);
> + cancel_work_sync(&vib->play_work);
> + turn_off(vib);
> + vib->running = false;
> +}
> +
>  static void gb_vibrator_worker(struct work_struct *work)
>  {
>   struct delayed_work *delayed_work = to_delayed_work(work);
> @@ -169,10 +212,26 @@ static int gb_vibrator_probe(struct gb_bundle *bundle,
>  
>   INIT_DELAYED_WORK(&vib->delayed_work, gb_vibrator_worker);
>  
> + INIT_WORK(&vib->play_work, gb_vibrator_play_work);

Hmm. Looks like you forgot to actually allocate the input device...

> + vib->input->name = "greybus-vibrator";
> + vib->input->close = gb_vibrator_close;
> + vib->input->dev.parent = &bundle->dev;
> + vib->input->id.bustype = BUS_HOST;
> +
> + input_set_drvdata(vib->input, vib);
> + input_set_capability(vib->input, EV_FF, FF_RUMBLE);
> +
> + retval = input_ff_create_memless(vib->input, NULL,
> +  gb_vibrator_play_effect);
> + if (retval)
> + goto err_device_remove;
> +
>   gb_pm_runtime_put_autosuspend(bundle);
>  
>   return 0;
>  
> +err_device_remove:
> + device_unregister(vib->dev);

I know you're removing this in the next patch, but as the class device
has been registered you need to cancel the delayed work and turn off the
motor here too (side note: looks like this is done in the wrong order in
remove()).

>  err_ida_remove:
>   ida_simple_remove(&minors, vib->minor);
>  err_connection_disable:

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

Re: [PATCH 1/2] staging: greybus: vibrator: use proper API for vibrator devices

[Johan Hovold]

On Wed, Jan 06, 2021 at 01:04:04PM +0100, Johan Hovold wrote:
> On Tue, Jan 05, 2021 at 04:19:02PM +0100, Greg Kroah-Hartman wrote:
> > The correct user/kernel api for vibrator devices is the Input rumble
> > api, not a random sysfs file like the greybus vibrator driver currently
> > uses.
> > 
> > Add support for the correct input api to the vibrator driver so that it
> > hooks up to the kernel and userspace correctly.
> > 
> > Cc: Johan Hovold 
> > Cc: Alex Elder 
> > Signed-off-by: Greg Kroah-Hartman 
> > ---
> >  drivers/staging/greybus/vibrator.c | 59 ++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/drivers/staging/greybus/vibrator.c 
> > b/drivers/staging/greybus/vibrator.c
> > index 0e2b188e5ca3..94110cadb5bd 100644
> > --- a/drivers/staging/greybus/vibrator.c
> > +++ b/drivers/staging/greybus/vibrator.c
> > @@ -13,13 +13,18 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  struct gb_vibrator_device {
> > struct gb_connection*connection;
> > +   struct input_dev*input;
> > struct device   *dev;
> > int minor;  /* vibrator minor number */
> > struct delayed_work delayed_work;
> > +   boolrunning;
> > +   boolon;
> > +   struct work_struct  play_work;
> >  };
> >  
> >  /* Greybus Vibrator operation types */
> > @@ -36,6 +41,7 @@ static int turn_off(struct gb_vibrator_device *vib)
> >  
> > gb_pm_runtime_put_autosuspend(bundle);
> >  
> > +   vib->on = false;
> 
> You update but never seem to actually use vib->on.
> 
> > return ret;
> >  }
> >  
> > @@ -59,11 +65,48 @@ static int turn_on(struct gb_vibrator_device *vib, u16 
> > timeout_ms)
> > return ret;
> > }
> >  
> > +   vib->on = true;
> > schedule_delayed_work(&vib->delayed_work, msecs_to_jiffies(timeout_ms));
> >  
> > return 0;
> >  }
> >  
> > +static void gb_vibrator_play_work(struct work_struct *work)
> > +{
> > +   struct gb_vibrator_device *vib =
> > +   container_of(work, struct gb_vibrator_device, play_work);
> > +
> > +   if (vib->running)
> 
> Is this test inverted?
> 
> > +   turn_off(vib);
> > +   else
> > +   turn_on(vib, 100);
> 
> Why 100 ms?
> 
> Shouldn't it just be left on indefinitely with the new API?
> 
> > +}
> > +
> > +static int gb_vibrator_play_effect(struct input_dev *input, void *data,
> > +  struct ff_effect *effect)
> > +{
> > +   struct gb_vibrator_device *vib = input_get_drvdata(input);
> > +   int level;
> > +
> > +   level = effect->u.rumble.strong_magnitude;
> > +   if (!level)
> > +   level = effect->u.rumble.weak_magnitude;
> > +
> > +   vib->running = level;
> > +   schedule_work(&vib->play_work);
> > +   return 0;
> > +}
> > +
> > +static void gb_vibrator_close(struct input_dev *input)
> > +{
> > +   struct gb_vibrator_device *vib = input_get_drvdata(input);
> > +
> > +   cancel_delayed_work_sync(&vib->delayed_work);
> > +   cancel_work_sync(&vib->play_work);
> > +   turn_off(vib);
> > +   vib->running = false;
> > +}
> > +
> >  static void gb_vibrator_worker(struct work_struct *work)
> >  {
> > struct delayed_work *delayed_work = to_delayed_work(work);
> > @@ -169,10 +212,26 @@ static int gb_vibrator_probe(struct gb_bundle *bundle,
> >  
> > INIT_DELAYED_WORK(&vib->delayed_work, gb_vibrator_worker);
> >  
> > +   INIT_WORK(&vib->play_work, gb_vibrator_play_work);
> 
> Hmm. Looks like you forgot to actually allocate the input device...
> 
> > +   vib->input->name = "greybus-vibrator";
> > +   vib->input->close = gb_vibrator_close;
> > +   vib->input->dev.parent = &bundle->dev;
> > +   vib->input->id.bustype = BUS_HOST;
> > +
> > +   input_set_drvdata(vib->input, vib);
> > +   input_set_capability(vib->input, EV_FF, FF_RUMBLE);
> > +
> > +   retval = input_ff_create_memless(vib->input, NULL,
> > +gb_vibrator_play_effect);
> > +   if (retval)
> > +   goto err_device_remove;
> > +

Oh, and you forgot to register the input device here too (and deregister
in remove()).

> > gb_pm_runtime_put_autosuspend(bundle);
> >  
> > return 0;
> >  
> > +err_device_remove:
> > +   device_unregister(vib->dev);
> 
> I know you're removing this in the next patch, but as the class device
> has been registered you need to cancel the delayed work and turn off the
> motor here too (side note: looks like this is done in the wrong order in
> remove()).
> 
> >  err_ida_remove:
> > ida_simple_remove(&minors, vib->minor);
> >  err_connection_disable:
 
Johan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH -next] media: zoran: use resource_size

[Zheng Yongjun]

Use resource_size rather than a verbose computation on
the end and start fields.

Signed-off-by: Zheng Yongjun 
---
 drivers/staging/media/zoran/zoran_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/zoran/zoran_driver.c 
b/drivers/staging/media/zoran/zoran_driver.c
index 808196ea5b81..d60b4c73ea80 100644
--- a/drivers/staging/media/zoran/zoran_driver.c
+++ b/drivers/staging/media/zoran/zoran_driver.c
@@ -1020,7 +1020,7 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue 
*vq)
vq->buf_struct_size = sizeof(struct zr_buffer);
vq->ops = &zr_video_qops;
vq->mem_ops = &vb2_dma_contig_memops;
-   vq->gfp_flags = GFP_DMA32,
+   vq->gfp_flags = GFP_DMA32;
vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
vq->min_buffers_needed = 9;
vq->lock = &zr->lock;
-- 
2.22.0

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

[PATCH -next] media: rkvdec: use resource_size

[Zheng Yongjun]

Use resource_size rather than a verbose computation on
the end and start fields.

Signed-off-by: Zheng Yongjun 
---
 drivers/staging/media/rkvdec/rkvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/rkvdec/rkvdec.c 
b/drivers/staging/media/rkvdec/rkvdec.c
index d25c4a37e2af..66572066e7a0 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -130,7 +130,7 @@ static void rkvdec_reset_fmt(struct rkvdec_ctx *ctx, struct 
v4l2_format *f,
memset(f, 0, sizeof(*f));
f->fmt.pix_mp.pixelformat = fourcc;
f->fmt.pix_mp.field = V4L2_FIELD_NONE;
-   f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_REC709,
+   f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_REC709;
f->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
f->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT;
f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
-- 
2.22.0

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

[PATCH -next] media: hantro: use resource_size

[Zheng Yongjun]

Use resource_size rather than a verbose computation on
the end and start fields.

Signed-off-by: Zheng Yongjun 
---
 drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/hantro/hantro_v4l2.c 
b/drivers/staging/media/hantro/hantro_v4l2.c
index b668a82d40ad..e1081c16f56a 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -316,7 +316,7 @@ hantro_reset_fmt(struct v4l2_pix_format_mplane *fmt,
 
fmt->pixelformat = vpu_fmt->fourcc;
fmt->field = V4L2_FIELD_NONE;
-   fmt->colorspace = V4L2_COLORSPACE_JPEG,
+   fmt->colorspace = V4L2_COLORSPACE_JPEG;
fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
-- 
2.22.0

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

[PATCH -next] media: atomisp: use resource_size

[Zheng Yongjun]

Use resource_size rather than a verbose computation on
the end and start fields.

Signed-off-by: Zheng Yongjun 
---
 drivers/staging/media/atomisp/pci/sh_css_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c 
b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 24fc497bd491..4a02948e5612 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -949,7 +949,7 @@ sh_css_set_black_frame(struct ia_css_stream *stream,
 
params = stream->isp_params_configs;
height = raw_black_frame->info.res.height;
-   width = raw_black_frame->info.padded_width,
+   width = raw_black_frame->info.padded_width;
 
ptr = raw_black_frame->data
+ raw_black_frame->planes.raw.offset;
@@ -1443,7 +1443,7 @@ static int sh_css_params_default_morph_table(
IA_CSS_ENTER_PRIVATE("");
 
step = (ISP_VEC_NELEMS / 16) * 128,
-   width = binary->morph_tbl_width,
+   width = binary->morph_tbl_width;
height = binary->morph_tbl_height;
 
tab = ia_css_morph_table_allocate(width, height);
-- 
2.22.0

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

答复: [PATCH -next] media: zoran: use resource_size

[zhengyongjun]

Sorry, this is my fault, all this media related patch commit msg is wrong, I 
will send patch v2, please ignore it.

-邮件原件-
发件人: zhengyongjun 
发送时间: 2021年1月6日 21:17
收件人: cla...@baylibre.com; mche...@kernel.org; 
mjpeg-us...@lists.sourceforge.net; linux-me...@vger.kernel.org; 
de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
抄送: gre...@linuxfoundation.org; zhengyongjun 
主题: [PATCH -next] media: zoran: use resource_size

Use resource_size rather than a verbose computation on the end and start fields.

Signed-off-by: Zheng Yongjun 
---
 drivers/staging/media/zoran/zoran_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/zoran/zoran_driver.c 
b/drivers/staging/media/zoran/zoran_driver.c
index 808196ea5b81..d60b4c73ea80 100644
--- a/drivers/staging/media/zoran/zoran_driver.c
+++ b/drivers/staging/media/zoran/zoran_driver.c
@@ -1020,7 +1020,7 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue 
*vq)
vq->buf_struct_size = sizeof(struct zr_buffer);
vq->ops = &zr_video_qops;
vq->mem_ops = &vb2_dma_contig_memops;
-   vq->gfp_flags = GFP_DMA32,
+   vq->gfp_flags = GFP_DMA32;
vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
vq->min_buffers_needed = 9;
vq->lock = &zr->lock;
--
2.22.0

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

Re: [PATCH -next] media: hantro: use resource_size

[Philipp Zabel]

Hi Zheng,

On Wed, 2021-01-06 at 21:18 +0800, Zheng Yongjun wrote:
> Use resource_size rather than a verbose computation on
> the end and start fields.
> 
> Signed-off-by: Zheng Yongjun 
> ---
>  drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c 
> b/drivers/staging/media/hantro/hantro_v4l2.c
> index b668a82d40ad..e1081c16f56a 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -316,7 +316,7 @@ hantro_reset_fmt(struct v4l2_pix_format_mplane *fmt,
>  
>   fmt->pixelformat = vpu_fmt->fourcc;
>   fmt->field = V4L2_FIELD_NONE;
> - fmt->colorspace = V4L2_COLORSPACE_JPEG,
> + fmt->colorspace = V4L2_COLORSPACE_JPEG;
>   fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>   fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>   fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;

Subject and commit message do not describe the patch.

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

Re: [PATCH -next] media: rkvdec: use resource_size

[Greg KH]

On Wed, Jan 06, 2021 at 09:18:20PM +0800, Zheng Yongjun wrote:
> Use resource_size rather than a verbose computation on
> the end and start fields.
> 
> Signed-off-by: Zheng Yongjun 
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c 
> b/drivers/staging/media/rkvdec/rkvdec.c
> index d25c4a37e2af..66572066e7a0 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -130,7 +130,7 @@ static void rkvdec_reset_fmt(struct rkvdec_ctx *ctx, 
> struct v4l2_format *f,
>   memset(f, 0, sizeof(*f));
>   f->fmt.pix_mp.pixelformat = fourcc;
>   f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> - f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_REC709,
> + f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_REC709;
>   f->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>   f->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT;
>   f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> -- 
> 2.22.0
> 

That is not what this patch does at all :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH -next] media: atomisp: use resource_size

[Greg KH]

On Wed, Jan 06, 2021 at 09:17:37PM +0800, Zheng Yongjun wrote:
> Use resource_size rather than a verbose computation on
> the end and start fields.
> 
> Signed-off-by: Zheng Yongjun 
> ---
>  drivers/staging/media/atomisp/pci/sh_css_params.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c 
> b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index 24fc497bd491..4a02948e5612 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -949,7 +949,7 @@ sh_css_set_black_frame(struct ia_css_stream *stream,
>  
>   params = stream->isp_params_configs;
>   height = raw_black_frame->info.res.height;
> - width = raw_black_frame->info.padded_width,
> + width = raw_black_frame->info.padded_width;
d>  
>   ptr = raw_black_frame->data
>   + raw_black_frame->planes.raw.offset;
> @@ -1443,7 +1443,7 @@ static int sh_css_params_default_morph_table(
>   IA_CSS_ENTER_PRIVATE("");
>  
>   step = (ISP_VEC_NELEMS / 16) * 128,
> - width = binary->morph_tbl_width,
> + width = binary->morph_tbl_width;
>   height = binary->morph_tbl_height;
>  
>   tab = ia_css_morph_table_allocate(width, height);
> -- 
> 2.22.0
> 

Your description does not match what the patch does at all :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH -next] media: zoran: use resource_size

[Dan Carpenter]

On Wed, Jan 06, 2021 at 09:17:02PM +0800, Zheng Yongjun wrote:
> Use resource_size rather than a verbose computation on
> the end and start fields.
> 
> Signed-off-by: Zheng Yongjun 
> ---
>  drivers/staging/media/zoran/zoran_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/zoran/zoran_driver.c 
> b/drivers/staging/media/zoran/zoran_driver.c
> index 808196ea5b81..d60b4c73ea80 100644
> --- a/drivers/staging/media/zoran/zoran_driver.c
> +++ b/drivers/staging/media/zoran/zoran_driver.c
> @@ -1020,7 +1020,7 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue 
> *vq)
>   vq->buf_struct_size = sizeof(struct zr_buffer);
>   vq->ops = &zr_video_qops;
>   vq->mem_ops = &vb2_dma_contig_memops;
> - vq->gfp_flags = GFP_DMA32,
> + vq->gfp_flags = GFP_DMA32;

The commit doesn't match the patch.  Also this driver is in
staging because it's going to be deleted soon so there probably isn't
much point doing cleanups.

If you're looking at the Smatch resource_size() message, then that's a
tricky thing because of the + 1 on part of the "end - start + 1"...
It's sometimes hard to know if we should add the + 1 which resource_size()
does or not.  (That check is ancient and not up to modern standards).

regards,
dan carpenter

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

Re: [PATCH] staging: ION: remove some references to CONFIG_ION

[Greg Kroah-Hartman]

On Wed, Jan 06, 2021 at 03:52:01PM +, Matthias Maennich wrote:
> With commit e722a295cf49 ("staging: ion: remove from the tree"), ION and
> its corresponding config CONFIG_ION is gone. Remove stale references
> from drivers/staging/media/atomisp/pci and from the recommended Android
> kernel config.
> 
> Fixes: e722a295cf49 ("staging: ion: remove from the tree")
> Cc: Greg Kroah-Hartman 
> Cc: Hridya Valsaraju 
> Cc: Rob Herring 
> Cc: linux-me...@vger.kernel.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Matthias Maennich 
> ---
>  .../media/atomisp/pci/atomisp_subdev.c| 20 ---
>  kernel/configs/android-recommended.config |  1 -
>  2 files changed, 21 deletions(-)

Thanks for finding these remnants, I'll go queue this up now.

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

Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name

[Filip Kolev]

On 06-Jan-21 09:51, Greg Kroah-Hartman wrote:

On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote:

There is a debug message using hardcoded function name instead of the
__func__ macro. Replace it.

Report from checkpatch.pl on the file:

WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this 
function's name, in a string
+   dev_dbg(&client->dev, "ov2722_remove...\n");

Signed-off-by: Filip Kolev 
---
  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c 
b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index eecefcd734d0e..21d6bc62d452a 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct ov2722_device *dev = to_ov2722_sensor(sd);
  
-	dev_dbg(&client->dev, "ov2722_remove...\n");

+   dev_dbg(&client->dev, "%s...\n", __func__);


dev_dbg() provides the function name already, and this is just a "trace"
call, and ftrace should be used instead, so the whole line should be
removed entirely.


Thank you for the review!

How do I go about this? Do I amend the patch and re-send as v2 or create 
a new patch entirely?
Newbie here, doing this as part of the Eudyptula challenge, so I very 
much appreciate everyone's patience.




thanks,

greg k-h


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

Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name

[Greg Kroah-Hartman]

On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote:
> 
> 
> On 06-Jan-21 09:51, Greg Kroah-Hartman wrote:
> > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote:
> > > There is a debug message using hardcoded function name instead of the
> > > __func__ macro. Replace it.
> > > 
> > > Report from checkpatch.pl on the file:
> > > 
> > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this 
> > > function's name, in a string
> > > + dev_dbg(&client->dev, "ov2722_remove...\n");
> > > 
> > > Signed-off-by: Filip Kolev 
> > > ---
> > >   drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c 
> > > b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > index eecefcd734d0e..21d6bc62d452a 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client)
> > >   struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > >   struct ov2722_device *dev = to_ov2722_sensor(sd);
> > > - dev_dbg(&client->dev, "ov2722_remove...\n");
> > > + dev_dbg(&client->dev, "%s...\n", __func__);
> > 
> > dev_dbg() provides the function name already, and this is just a "trace"
> > call, and ftrace should be used instead, so the whole line should be
> > removed entirely.
> 
> Thank you for the review!
> 
> How do I go about this? Do I amend the patch and re-send as v2 or create a
> new patch entirely?

New patch entirely please.

thanks,

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

[PATCH] media: atomisp: ov2722: remove unnecessary debug print

[Filip Kolev]

checkpatch.pl emits the following warning:

WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this 
function's name, in a string
+   dev_dbg(&client->dev, "ov2722_remove...\n");

This is just a "trace" call and therefore should be removed entirely;
ftrace should be used instead.

Signed-off-by: Filip Kolev 
---
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c 
b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index eecefcd734d0e..1209492c1826a 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -1175,8 +1175,6 @@ static int ov2722_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct ov2722_device *dev = to_ov2722_sensor(sd);
 
-   dev_dbg(&client->dev, "ov2722_remove...\n");
-
dev->platform_data->csi_cfg(sd, 0);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
v4l2_device_unregister_subdev(sd);
-- 
2.30.0

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

Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name

[Joe Perches]

On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: 
> > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote:
> > > > There is a debug message using hardcoded function name instead of the
> > > > __func__ macro. Replace it.
> > > > 
> > > > Report from checkpatch.pl on the file:
> > > > 
> > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', 
> > > > this function's name, in a string
> > > > +   dev_dbg(&client->dev, "ov2722_remove...\n");
[]
> > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c 
> > > > b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
[]
> > > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client 
> > > > *client)
> > > >     struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > >     struct ov2722_device *dev = to_ov2722_sensor(sd);
> > > > -   dev_dbg(&client->dev, "ov2722_remove...\n");
> > > > +   dev_dbg(&client->dev, "%s...\n", __func__);
> > > 
> > > dev_dbg() provides the function name already, and this is just a "trace"
> > > call, and ftrace should be used instead, so the whole line should be
> > > removed entirely.
> > 
> > Thank you for the review!
> > 
> > How do I go about this? Do I amend the patch and re-send as v2 or create a
> > new patch entirely?
> 
> New patch entirely please.

There are quite a lot of these relatively useless function tracing like
uses in the kernel:

$ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l
1065

Perhaps yet another checkpatch warning would be useful:
---
 scripts/checkpatch.pl | 8 
 1 file changed, 8 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e6857bdfcb2d..46b8ec8fe9e1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5981,6 +5981,14 @@ sub process {
 "Prefer using '\"%s...\", __func__' to using 
'$context_function', this function's name, in a string\n" . $herecurr);
}
 
+# check for unnecessary function tracing like uses
+   if ($rawline =~ 
/^\+\s*$logFunctions\s*\([^"]*"%s[\.\!]*\\n"\s*,\s*__func__\s*\)\s*;\s*$/) {
+   if (WARN("TRACING_LOGGING",
+"Unnecessary ftrace-like logging - prefer 
using ftrace\n" . $herecurr) &&
+   $fix) {
+fix_delete_line($fixlinenr, $rawline);
+   }
+   }
 # check for spaces before a quoted newline
if ($rawline =~ /^.*\".*\s\\n/) {
if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",

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

Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name

[Dan Carpenter]

On Wed, Jan 06, 2021 at 10:25:26AM -0800, Joe Perches wrote:
> On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote:
> > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: 
> > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote:
> > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote:
> > > > > There is a debug message using hardcoded function name instead of the
> > > > > __func__ macro. Replace it.
> > > > > 
> > > > > Report from checkpatch.pl on the file:
> > > > > 
> > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', 
> > > > > this function's name, in a string
> > > > > + dev_dbg(&client->dev, "ov2722_remove...\n");
> []
> > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c 
> > > > > b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> []
> > > > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client 
> > > > > *client)
> > > > >   struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > >   struct ov2722_device *dev = to_ov2722_sensor(sd);
> > > > > - dev_dbg(&client->dev, "ov2722_remove...\n");
> > > > > + dev_dbg(&client->dev, "%s...\n", __func__);
> > > > 
> > > > dev_dbg() provides the function name already, and this is just a "trace"
> > > > call, and ftrace should be used instead, so the whole line should be
> > > > removed entirely.
> > > 
> > > Thank you for the review!
> > > 
> > > How do I go about this? Do I amend the patch and re-send as v2 or create a
> > > new patch entirely?
> > 
> > New patch entirely please.
> 
> There are quite a lot of these relatively useless function tracing like
> uses in the kernel:
> 
> $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l
> 1065

These are printing other stuff besides just the function name.  Maybe
grep for '", __func__\)'?

regards,
dan carpenter

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

Re: [PATCH -next] media: zoran: use resource_size

[LABBE Corentin]

Le Wed, Jan 06, 2021 at 05:51:00PM +0300, Dan Carpenter a écrit :
> On Wed, Jan 06, 2021 at 09:17:02PM +0800, Zheng Yongjun wrote:
> > Use resource_size rather than a verbose computation on
> > the end and start fields.
> > 
> > Signed-off-by: Zheng Yongjun 
> > ---
> >  drivers/staging/media/zoran/zoran_driver.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/zoran/zoran_driver.c 
> > b/drivers/staging/media/zoran/zoran_driver.c
> > index 808196ea5b81..d60b4c73ea80 100644
> > --- a/drivers/staging/media/zoran/zoran_driver.c
> > +++ b/drivers/staging/media/zoran/zoran_driver.c
> > @@ -1020,7 +1020,7 @@ int zoran_queue_init(struct zoran *zr, struct 
> > vb2_queue *vq)
> > vq->buf_struct_size = sizeof(struct zr_buffer);
> > vq->ops = &zr_video_qops;
> > vq->mem_ops = &vb2_dma_contig_memops;
> > -   vq->gfp_flags = GFP_DMA32,
> > +   vq->gfp_flags = GFP_DMA32;
> 
> The commit doesn't match the patch.  Also this driver is in
> staging because it's going to be deleted soon so there probably isn't
> much point doing cleanups.
> 

No, the driver just came back in staging since I fixed the videobuf2 conversion.
One of the reason it is kept in staging is that media maintainer want to test 
it with its own zoran card but covid19 delayed the physical recovery of it.

So the patch need to be resent, please.

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

[PATCH] staging: ION: remove some references to CONFIG_ION

[Matthias Maennich]

With commit e722a295cf49 ("staging: ion: remove from the tree"), ION and
its corresponding config CONFIG_ION is gone. Remove stale references
from drivers/staging/media/atomisp/pci and from the recommended Android
kernel config.

Fixes: e722a295cf49 ("staging: ion: remove from the tree")
Cc: Greg Kroah-Hartman 
Cc: Hridya Valsaraju 
Cc: Rob Herring 
Cc: linux-me...@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Matthias Maennich 
---
 .../media/atomisp/pci/atomisp_subdev.c| 20 ---
 kernel/configs/android-recommended.config |  1 -
 2 files changed, 21 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c 
b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 52b9fb18c87f..b666cb23e5ca 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -1062,26 +1062,6 @@ static const struct v4l2_ctrl_config 
ctrl_select_isp_version = {
.def = 0,
 };
 
-#if 0 /* #ifdef CONFIG_ION */
-/*
- * Control for ISP ion device fd
- *
- * userspace will open ion device and pass the fd to kernel.
- * this fd will be used to map shared fd to buffer.
- */
-/* V4L2_CID_ATOMISP_ION_DEVICE_FD is not defined */
-static const struct v4l2_ctrl_config ctrl_ion_dev_fd = {
-   .ops = &ctrl_ops,
-   .id = V4L2_CID_ATOMISP_ION_DEVICE_FD,
-   .type = V4L2_CTRL_TYPE_INTEGER,
-   .name = "Ion Device Fd",
-   .min = -1,
-   .max = 1024,
-   .step = 1,
-   .def = ION_FD_UNSET
-};
-#endif
-
 static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd,
 struct atomisp_video_pipe *pipe, enum 
v4l2_buf_type buf_type)
 {
diff --git a/kernel/configs/android-recommended.config 
b/kernel/configs/android-recommended.config
index 53d688bdd894..eb0029c9a6a6 100644
--- a/kernel/configs/android-recommended.config
+++ b/kernel/configs/android-recommended.config
@@ -81,7 +81,6 @@ CONFIG_INPUT_JOYSTICK=y
 CONFIG_INPUT_MISC=y
 CONFIG_INPUT_TABLET=y
 CONFIG_INPUT_UINPUT=y
-CONFIG_ION=y
 CONFIG_JOYSTICK_XPAD=y
 CONFIG_JOYSTICK_XPAD_FF=y
 CONFIG_JOYSTICK_XPAD_LEDS=y
-- 
2.29.2.729.g45daf8777d-goog

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

Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name

[Joe Perches]

On Wed, 2021-01-06 at 22:36 +0300, Dan Carpenter wrote:
> On Wed, Jan 06, 2021 at 10:25:26AM -0800, Joe Perches wrote:
> > On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote:
> > > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: 
> > > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote:
> > > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote:
> > > > > > There is a debug message using hardcoded function name instead of 
> > > > > > the
> > > > > > __func__ macro. Replace it.
> > > > > > 
> > > > > > Report from checkpatch.pl on the file:
> > > > > > 
> > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', 
> > > > > > this function's name, in a string
> > > > > > +   dev_dbg(&client->dev, "ov2722_remove...\n");
[]
> > There are quite a lot of these relatively useless function tracing like
> > uses in the kernel:
> > 
> > $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l
> > 1065
> 
> These are printing other stuff besides just the function name.

No, these are printing _only_ the function name.

> Maybe grep for '", __func__\)'?

No, that wouldn't work as there are many many uses like:

printk('%s: some string\n", __func__);


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

Calendario de Cursos Bonificables 2021

[FOESCO]

Buenos días


Os informamos desde FOESCO (Formación Estatal Continua) que nos encontramos 
organizando el Calendario de Cursos Bonificables 2021 para empleados en activo 
o en ERTE.


Rogamos respondáis a este mismo correo electrónico eligiendo una de las 
opciones que a continuación indicamos:

1 - Precisamos información para la PRESENTE convocatoria ENERO 2021

2 - Precisamos información para el mes de .. (Indicar mes)

3 - No precisamos ninguna información BAJA


Quedamos a la espera de vuestra respuesta.


Un cordial saludo.


Alex Pons.
Director FOESCO.

FOESCO Formación Estatal Continua
e-mail:  cur...@foesco.net
Tel.:91 032 37 94
(Horario de 9h a 14h y de 16:30h a 21h de Lunes a Viernes)

FOESCO ofrece formación a empresas y trabajadores en activo a través de cursos 
bonificables por la Fundación Estatal para la Formación en el Empleo (antiguo 
FORCEM) que gestiona las acciones formativas de FORMACIÓN CONTINUA para 
trabajadores y se rige por la ley 30/2015 de 9 de Septiembre.

Antes de imprimir este e-mail piense bien si es necesario hacerlo. Before 
printing this e-mail please think twice if you really need it. FOESCO Tfno: 910 
382 880 Email: cur...@foesco.com. La información transmitida en este mensaje 
está dirigida solamente a las personas o entidades que figuran en el 
encabezamiento y contiene información confidencial, por lo que, si usted lo 
recibiera por error, por favor destrúyalo sin copiarlo, usarlo ni distribuirlo, 
comunicándolo inmediatamente al emisor del mensaje. De conformidad con lo 
dispuesto en el Reglamento Europeo del 2016/679, del 27 de Abril de 2016, 
FOESCO le informa que los datos por usted suministrados serán tratados con las 
medidas de seguridad conformes a la normativa vigente que se requiere. Dichos 
datos serán empleados con fines de gestión. Para el ejercicio de sus derechos 
de transparencia, información, acceso, rectificación, supresión o derecho al 
olvido, limitación del tratamiento , portabilidad de datos y oposición de sus 
datos de carácter personal deberá dirigirse a la dirección del Responsable del 
tratamiento a C/ LAGUNA DEL MARQUESADO Nº10, 28021, MADRID, "PULSANDO AQUI" 
 y "ENVIAR" o a traves de la 
dirección de correo electrónico: ba...@foesco.com 

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