[1/2] media: i2c: ov772x: Add support for BT656 mode
Commit Message
Add support to read the bus-type and enable BT656 mode if needed.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
drivers/media/i2c/ov772x.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
Comments
Hi Lad,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.8-rc6 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lad-Prabhakar/media-i2c-ov772x-Enable-BT656-mode-and-test-pattern-support/20200724-231016
base: git://linuxtv.org/media_tree.git master
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1d09ecf36175f7910ffedd6d497c07b5c74c22fb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/i2c/ov772x.c:1425:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!endpoint) {
^~~~~~~~~
drivers/media/i2c/ov772x.c:1471:9: note: uninitialized use occurs here
return ret;
^~~
drivers/media/i2c/ov772x.c:1425:2: note: remove the 'if' if its condition is always false
if (!endpoint) {
^~~~~~~~~~~~~~~~
drivers/media/i2c/ov772x.c:1363:11: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
1 warning generated.
vim +1425 drivers/media/i2c/ov772x.c
1354
1355 /*
1356 * i2c_driver function
1357 */
1358
1359 static int ov772x_probe(struct i2c_client *client)
1360 {
1361 struct fwnode_handle *endpoint;
1362 struct ov772x_priv *priv;
1363 int ret;
1364 static const struct regmap_config ov772x_regmap_config = {
1365 .reg_bits = 8,
1366 .val_bits = 8,
1367 .max_register = DSPAUTO,
1368 };
1369
1370 if (!client->dev.of_node && !client->dev.platform_data) {
1371 dev_err(&client->dev,
1372 "Missing ov772x platform data for non-DT device\n");
1373 return -EINVAL;
1374 }
1375
1376 priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
1377 if (!priv)
1378 return -ENOMEM;
1379
1380 priv->regmap = devm_regmap_init_sccb(client, &ov772x_regmap_config);
1381 if (IS_ERR(priv->regmap)) {
1382 dev_err(&client->dev, "Failed to allocate register map\n");
1383 return PTR_ERR(priv->regmap);
1384 }
1385
1386 priv->info = client->dev.platform_data;
1387 mutex_init(&priv->lock);
1388
1389 v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops);
1390 priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
1391 V4L2_SUBDEV_FL_HAS_EVENTS;
1392 v4l2_ctrl_handler_init(&priv->hdl, 3);
1393 /* Use our mutex for the controls */
1394 priv->hdl.lock = &priv->lock;
1395 priv->vflip_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1396 V4L2_CID_VFLIP, 0, 1, 1, 0);
1397 priv->hflip_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1398 V4L2_CID_HFLIP, 0, 1, 1, 0);
1399 priv->band_filter_ctrl = v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
1400 V4L2_CID_BAND_STOP_FILTER,
1401 0, 256, 1, 0);
1402 priv->subdev.ctrl_handler = &priv->hdl;
1403 if (priv->hdl.error) {
1404 ret = priv->hdl.error;
1405 goto error_mutex_destroy;
1406 }
1407
1408 priv->clk = clk_get(&client->dev, NULL);
1409 if (IS_ERR(priv->clk)) {
1410 dev_err(&client->dev, "Unable to get xclk clock\n");
1411 ret = PTR_ERR(priv->clk);
1412 goto error_ctrl_free;
1413 }
1414
1415 priv->pwdn_gpio = gpiod_get_optional(&client->dev, "powerdown",
1416 GPIOD_OUT_LOW);
1417 if (IS_ERR(priv->pwdn_gpio)) {
1418 dev_info(&client->dev, "Unable to get GPIO \"powerdown\"");
1419 ret = PTR_ERR(priv->pwdn_gpio);
1420 goto error_clk_put;
1421 }
1422
1423 endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
1424 NULL);
> 1425 if (!endpoint) {
1426 dev_err(&client->dev, "endpoint node not found\n");
1427 goto error_clk_put;
1428 }
1429
1430 ret = v4l2_fwnode_endpoint_parse(endpoint, &priv->ep);
1431 fwnode_handle_put(endpoint);
1432 if (ret) {
1433 dev_err(&client->dev, "Could not parse endpoint\n");
1434 goto error_clk_put;
1435 }
1436
1437 ret = ov772x_video_probe(priv);
1438 if (ret < 0)
1439 goto error_gpio_put;
1440
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Prabhakar,
On Fri, Jul 24, 2020 at 04:08:15PM +0100, Lad Prabhakar wrote:
> Add support to read the bus-type and enable BT656 mode if needed.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
The DT bindings should also be changed.
The default should be parallel, I guess, since the type hasn't been
documented. Parsing should be also updated so the driver can set meaningful
defaults for the flags --- this btw. also applies to the corresponding
ov5640 patch.
Hi Sakari,
Thank you for the review.
On Fri, Jul 31, 2020 at 5:03 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jul 24, 2020 at 04:08:15PM +0100, Lad Prabhakar wrote:
> > Add support to read the bus-type and enable BT656 mode if needed.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> The DT bindings should also be changed.
>
I'll get this done in V2.
> The default should be parallel, I guess, since the type hasn't been
> documented. Parsing should be also updated so the driver can set meaningful
> defaults for the flags --- this btw. also applies to the corresponding
> ov5640 patch.
>
Agreed, will do.
Cheers,
Prabhakar
> --
> Sakari Ailus
@@ -31,6 +31,7 @@
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
#include <media/v4l2-image-sizes.h>
#include <media/v4l2-subdev.h>
@@ -434,6 +435,7 @@ struct ov772x_priv {
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_pad pad;
#endif
+ struct v4l2_fwnode_endpoint ep;
};
/*
@@ -585,7 +587,6 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
enable ? 0 : SOFT_SLEEP_MODE);
if (ret)
goto done;
-
if (enable) {
dev_dbg(&client->dev, "format %d, win %s\n",
priv->cfmt->code, priv->win->name);
@@ -1104,7 +1105,10 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
/* COM7: Sensor resolution and output format control. */
- ret = regmap_write(priv->regmap, COM7, win->com7_bit | cfmt->com7);
+ val = win->com7_bit | cfmt->com7;
+ if (priv->ep.bus_type == V4L2_MBUS_BT656)
+ val |= ITU656_ON_OFF;
+ ret = regmap_write(priv->regmap, COM7, val);
if (ret < 0)
goto ov772x_set_fmt_error;
@@ -1354,6 +1358,7 @@ static const struct v4l2_subdev_ops ov772x_subdev_ops = {
static int ov772x_probe(struct i2c_client *client)
{
+ struct fwnode_handle *endpoint;
struct ov772x_priv *priv;
int ret;
static const struct regmap_config ov772x_regmap_config = {
@@ -1415,6 +1420,20 @@ static int ov772x_probe(struct i2c_client *client)
goto error_clk_put;
}
+ endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
+ NULL);
+ if (!endpoint) {
+ dev_err(&client->dev, "endpoint node not found\n");
+ goto error_clk_put;
+ }
+
+ ret = v4l2_fwnode_endpoint_parse(endpoint, &priv->ep);
+ fwnode_handle_put(endpoint);
+ if (ret) {
+ dev_err(&client->dev, "Could not parse endpoint\n");
+ goto error_clk_put;
+ }
+
ret = ov772x_video_probe(priv);
if (ret < 0)
goto error_gpio_put;