* [PATCH 1/3] media: imx274: don't declare events, they are not implemented
@ 2018-11-27 8:34 Luca Ceresoli
2018-11-27 8:34 ` [PATCH 2/3] media: imx274: declare the correct number of controls Luca Ceresoli
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Luca Ceresoli @ 2018-11-27 8:34 UTC (permalink / raw)
To: linux-media
Cc: Luca Ceresoli, Sakari Ailus, Leon Luo, Mauro Carvalho Chehab,
linux-kernel
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(-)
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;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] media: imx274: declare the correct number of controls
2018-11-27 8:34 [PATCH 1/3] media: imx274: don't declare events, they are not implemented Luca Ceresoli
@ 2018-11-27 8:34 ` Luca Ceresoli
2018-11-27 8:34 ` [PATCH 3/3] media: imx274: select REGMAP_I2C Luca Ceresoli
2018-11-27 8:38 ` [PATCH 1/3] media: imx274: don't declare events, they are not implemented Sakari Ailus
2 siblings, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2018-11-27 8:34 UTC (permalink / raw)
To: linux-media
Cc: Luca Ceresoli, Sakari Ailus, Leon Luo, Mauro Carvalho Chehab,
linux-kernel
v4l2_ctrl_handler_init() expects a hint of how many controls this
handler is expected to refer to. Since this number here is always 4,
let's pass exactly 4.
Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
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 78746c51071d..40c717f13eb8 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1904,7 +1904,7 @@ static int imx274_probe(struct i2c_client *client,
imx274_reset(imx274, 1);
/* initialize controls */
- ret = v4l2_ctrl_handler_init(&imx274->ctrls.handler, 2);
+ ret = v4l2_ctrl_handler_init(&imx274->ctrls.handler, 4);
if (ret < 0) {
dev_err(&client->dev,
"%s : ctrl handler init Failed\n", __func__);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] media: imx274: select REGMAP_I2C
2018-11-27 8:34 [PATCH 1/3] media: imx274: don't declare events, they are not implemented Luca Ceresoli
2018-11-27 8:34 ` [PATCH 2/3] media: imx274: declare the correct number of controls Luca Ceresoli
@ 2018-11-27 8:34 ` Luca Ceresoli
2018-11-27 8:38 ` [PATCH 1/3] media: imx274: don't declare events, they are not implemented Sakari Ailus
2 siblings, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2018-11-27 8:34 UTC (permalink / raw)
To: linux-media
Cc: Luca Ceresoli, Sakari Ailus, Leon Luo, Mauro Carvalho Chehab,
linux-kernel
The imx274 driver uses regmap and the build will fail without it.
Fixes:
drivers/media/i2c/imx274.c:142:21: error: variable ‘imx274_regmap_config’ has initializer but incomplete type
static const struct regmap_config imx274_regmap_config = {
^~~~~~~~~~~~~
drivers/media/i2c/imx274.c:1869:19: error: implicit declaration of function ‘devm_regmap_init_i2c’ [-Werror=implicit-function-declaration]
imx274->regmap = devm_regmap_init_i2c(client, &imx274_regmap_config);
^~~~~~~~~~~~~~~~~~~~
and others.
Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
drivers/media/i2c/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 66bbbec487ec..005fc2bd0d05 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -622,6 +622,7 @@ config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
+ select REGMAP_I2C
---help---
This is a V4L2 sensor driver for the Sony IMX274
CMOS image sensor.
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] media: imx274: don't declare events, they are not implemented
2018-11-27 8:34 [PATCH 1/3] media: imx274: don't declare events, they are not implemented Luca Ceresoli
2018-11-27 8:34 ` [PATCH 2/3] media: imx274: declare the correct number of controls Luca Ceresoli
2018-11-27 8:34 ` [PATCH 3/3] media: imx274: select REGMAP_I2C Luca Ceresoli
@ 2018-11-27 8:38 ` Sakari Ailus
2018-11-27 9:09 ` Luca Ceresoli
2 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2018-11-27 8:38 UTC (permalink / raw)
To: Luca Ceresoli; +Cc: linux-media, Leon Luo, Mauro Carvalho Chehab, linux-kernel
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;
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] media: imx274: don't declare events, they are not implemented
2018-11-27 8:38 ` [PATCH 1/3] media: imx274: don't declare events, they are not implemented Sakari Ailus
@ 2018-11-27 9:09 ` Luca Ceresoli
2018-11-27 9:19 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2018-11-27 9:09 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Leon Luo, Mauro Carvalho Chehab, linux-kernel
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?
--
Luca
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] media: imx274: don't declare events, they are not implemented
2018-11-27 9:09 ` Luca Ceresoli
@ 2018-11-27 9:19 ` Sakari Ailus
0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2018-11-27 9:19 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Sakari Ailus, linux-media, Leon Luo, Mauro Carvalho Chehab, linux-kernel
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.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-27 20:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 8:34 [PATCH 1/3] media: imx274: don't declare events, they are not implemented Luca Ceresoli
2018-11-27 8:34 ` [PATCH 2/3] media: imx274: declare the correct number of controls Luca Ceresoli
2018-11-27 8:34 ` [PATCH 3/3] media: imx274: select REGMAP_I2C Luca Ceresoli
2018-11-27 8:38 ` [PATCH 1/3] media: imx274: don't declare events, they are not implemented Sakari Ailus
2018-11-27 9:09 ` Luca Ceresoli
2018-11-27 9:19 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).