[1/3] media: imx274: don't declare events, they are not implemented

Message ID 20181127083445.27737-1-luca@lucaceresoli.net (mailing list archive)
State Rejected, archived
Delegated to: Sakari Ailus
Headers

Commit Message

Luca Ceresoli Nov. 27, 2018, 8:34 a.m. UTC
  The V4L2_SUBDEV_FL_HAS_EVENTS flag should not be set, event are just
not implemented.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/media/i2c/imx274.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Sakari Ailus Nov. 27, 2018, 8:38 a.m. UTC | #1
Hi Luca,

On Tue, Nov 27, 2018 at 09:34:43AM +0100, Luca Ceresoli wrote:
> The V4L2_SUBDEV_FL_HAS_EVENTS flag should not be set, event are just
> not implemented.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

The driver supports controls, and so control events can be subscribed and
received by the user. Therefore I don't see a reason to remove the flag.

> ---
>  drivers/media/i2c/imx274.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
> index 95a0e7d9851a..78746c51071d 100644
> --- a/drivers/media/i2c/imx274.c
> +++ b/drivers/media/i2c/imx274.c
> @@ -1878,7 +1878,7 @@ static int imx274_probe(struct i2c_client *client,
>  	imx274->client = client;
>  	sd = &imx274->sd;
>  	v4l2_i2c_subdev_init(sd, client, &imx274_subdev_ops);
> -	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	/* initialize subdev media pad */
>  	imx274->pad.flags = MEDIA_PAD_FL_SOURCE;
  
Luca Ceresoli Nov. 27, 2018, 9:09 a.m. UTC | #2
Hi Sakari,

On 27/11/18 09:38, Sakari Ailus wrote:
> Hi Luca,
> 
> On Tue, Nov 27, 2018 at 09:34:43AM +0100, Luca Ceresoli wrote:
>> The V4L2_SUBDEV_FL_HAS_EVENTS flag should not be set, event are just
>> not implemented.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> The driver supports controls, and so control events can be subscribed and
> received by the user. Therefore I don't see a reason to remove the flag.

Thanks, good to know.

Would it be worth adding a note where V4L2_SUBDEV_FL_HAS_EVENTS is
#defined, to make this clear?
  
Sakari Ailus Nov. 27, 2018, 9:19 a.m. UTC | #3
On Tue, Nov 27, 2018 at 10:09:08AM +0100, Luca Ceresoli wrote:
> Hi Sakari,
> 
> On 27/11/18 09:38, Sakari Ailus wrote:
> > Hi Luca,
> > 
> > On Tue, Nov 27, 2018 at 09:34:43AM +0100, Luca Ceresoli wrote:
> >> The V4L2_SUBDEV_FL_HAS_EVENTS flag should not be set, event are just
> >> not implemented.
> >>
> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> > 
> > The driver supports controls, and so control events can be subscribed and
> > received by the user. Therefore I don't see a reason to remove the flag.

I further missed the driver does not set the event (un)subscription
callbacks; the event support is actually not functional as a result. :-\

It's trivial to do that, see e.g. the imx319 driver.

> 
> Thanks, good to know.
> 
> Would it be worth adding a note where V4L2_SUBDEV_FL_HAS_EVENTS is
> #defined, to make this clear?

Could you send a patch? A few words should be enough, no need for a too
elaborate description I guess.
  

Patch

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 95a0e7d9851a..78746c51071d 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1878,7 +1878,7 @@  static int imx274_probe(struct i2c_client *client,
 	imx274->client = client;
 	sd = &imx274->sd;
 	v4l2_i2c_subdev_init(sd, client, &imx274_subdev_ops);
-	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
+	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	/* initialize subdev media pad */
 	imx274->pad.flags = MEDIA_PAD_FL_SOURCE;