diff mbox series

[v4,6/7] media: i2c: ov9282: Set v4l2 subdev name according to sensor model

Message ID 20220728130237.3396663-7-alexander.stein@ew.tq-group.com
State New
Headers show
Series OV9281 support | expand

Commit Message

Alexander Stein July 28, 2022, 1:02 p.m. UTC 
To distinguish ov9281 & ov9282 the name has to be explicitly set.
Provide a fixed string using platform data.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v4:
* Replaced v4l2_i2c_subdev_set_name with device_get_match_data and added
  platform data containing the sensor name

 drivers/media/i2c/ov9282.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Alexander Stein July 29, 2022, 8:23 a.m. UTC | #1 
Am Donnerstag, 28. Juli 2022, 23:10:07 CEST schrieb kernel test robot:
> Hi Alexander,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on media-tree/master]
> [also build test WARNING on linus/master v5.19-rc8 next-20220728]
> [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#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/Alexander-Stein/OV9281-suppo
> rt/20220728-210448 base:   git://linuxtv.org/media_tree.git master
> config: arm-randconfig-r022-20220728
> (https://download.01.org/0day-ci/archive/20220729/202207290518.1D7MVS65-lkp
> @intel.com/config) compiler: clang version 15.0.0
> (https://github.com/llvm/llvm-project
> 8dfaecc4c24494337933aff9d9166486ca0949f1) 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 arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         #
> https://github.com/intel-lab-lkp/linux/commit/ee28006553d4d23f600b0076ef606
> 6710519f156 git remote add linux-review
> https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review
> Alexander-Stein/OV9281-support/20220728-210448 git checkout
> ee28006553d4d23f600b0076ef6066710519f156
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
> O=build_dir ARCH=arm SHELL=/bin/bash drivers/media/i2c/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> >> drivers/media/i2c/ov9282.c:1054:10: warning: variable 'ret' is
> >> uninitialized when used here [-Wuninitialized]
>                    return ret;
>                           ^~~
>    drivers/media/i2c/ov9282.c:1041:9: note: initialize the variable 'ret' to
> silence this warning int ret;
>                   ^
>                    = 0
>    1 warning generated.
> 
> 
> vim +/ret +1054 drivers/media/i2c/ov9282.c
> 
>   1030
>   1031	/**
>   1032	 * ov9282_probe() - I2C client device binding
>   1033	 * @client: pointer to i2c client device
>   1034	 *
>   1035	 * Return: 0 if successful, error code otherwise.
>   1036	 */
>   1037	static int ov9282_probe(struct i2c_client *client)
>   1038	{
>   1039		struct ov9282 *ov9282;
>   1040		const char *sensor_name;
>   1041		int ret;
>   1042
>   1043		ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), 
GFP_KERNEL);
>   1044		if (!ov9282)
>   1045			return -ENOMEM;
>   1046
>   1047		ov9282->dev = &client->dev;
>   1048
>   1049		/* Initialize subdev */
>   1050		v4l2_i2c_subdev_init(&ov9282->sd, client, 
&ov9282_subdev_ops);
>   1051		sensor_name = device_get_match_data(ov9282->dev);
>   1052		if (!sensor_name) {
>   1053			dev_err(ov9282->dev, "Sensor name is 
missing");
> 
> > 1054			return ret;
> 
>   1055		}
>   1056		v4l2_i2c_subdev_set_name(&ov9282->sd, client, 
sensor_name, NULL);
>   1057
>   1058		ret = ov9282_parse_hw_config(ov9282);
>   1059		if (ret) {
>   1060			dev_err(ov9282->dev, "HW configuration is not 
supported");
>   1061			return ret;
>   1062		}
>   1063
>   1064		ret = ov9282_get_regulators(ov9282);
>   1065		if (ret) {
>   1066			dev_err(&client->dev, "Failed to get power 
regulators\n");
>   1067			return ret;
>   1068		}
>   1069
>   1070		mutex_init(&ov9282->mutex);
>   1071
>   1072		ret = ov9282_power_on(ov9282->dev);
>   1073		if (ret) {
>   1074			dev_err(ov9282->dev, "failed to power-on the 
sensor");
>   1075			goto error_mutex_destroy;
>   1076		}
>   1077
>   1078		/* Check module identity */
>   1079		ret = ov9282_detect(ov9282);
>   1080		if (ret) {
>   1081			dev_err(ov9282->dev, "failed to find sensor: 
%d", ret);
>   1082			goto error_power_off;
>   1083		}
>   1084
>   1085		/* Set default mode to max resolution */
>   1086		ov9282->cur_mode = &supported_mode;
>   1087		ov9282->vblank = ov9282->cur_mode->vblank;
>   1088
>   1089		ret = ov9282_init_controls(ov9282);
>   1090		if (ret) {
>   1091			dev_err(ov9282->dev, "failed to init 
controls: %d", ret);
>   1092			goto error_power_off;
>   1093		}
>   1094
>   1095		/* Initialize subdev */
>   1096		ov9282->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>   1097		ov9282->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>   1098
>   1099		/* Initialize source pad */
>   1100		ov9282->pad.flags = MEDIA_PAD_FL_SOURCE;
>   1101		ret = media_entity_pads_init(&ov9282->sd.entity, 1, 
&ov9282->pad);
>   1102		if (ret) {
>   1103			dev_err(ov9282->dev, "failed to init entity 
pads: %d", ret);
>   1104			goto error_handler_free;
>   1105		}
>   1106
>   1107		ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
>   1108		if (ret < 0) {
>   1109			dev_err(ov9282->dev,
>   1110				"failed to register async subdev: 
%d", ret);
>   1111			goto error_media_entity;
>   1112		}
>   1113
>   1114		pm_runtime_set_active(ov9282->dev);
>   1115		pm_runtime_enable(ov9282->dev);
>   1116		pm_runtime_idle(ov9282->dev);
>   1117
>   1118		return 0;
>   1119
>   1120	error_media_entity:
>   1121		media_entity_cleanup(&ov9282->sd.entity);
>   1122	error_handler_free:
>   1123		v4l2_ctrl_handler_free(ov9282->sd.ctrl_handler);
>   1124	error_power_off:
>   1125		ov9282_power_off(ov9282->dev);
>   1126	error_mutex_destroy:
>   1127		mutex_destroy(&ov9282->mutex);
>   1128
>   1129		return ret;
>   1130	}
>   1131

Meh, I'll come up with a fixed once discussion about the additional compatible 
has settled. This will also include the missing member documentation in patch 
5

Best regards,
Alexander
Sakari Ailus Aug. 1, 2022, 12:16 p.m. UTC | #2 
Hi Alexander,

On Fri, Jul 29, 2022 at 10:23:48AM +0200, Alexander Stein wrote:
> Am Donnerstag, 28. Juli 2022, 23:10:07 CEST schrieb kernel test robot:
> > Hi Alexander,
> > 
> > Thank you for the patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on media-tree/master]
> > [also build test WARNING on linus/master v5.19-rc8 next-20220728]
> > [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#_base_tree_information]
> > 
> > url:   
> > https://github.com/intel-lab-lkp/linux/commits/Alexander-Stein/OV9281-suppo
> > rt/20220728-210448 base:   git://linuxtv.org/media_tree.git master
> > config: arm-randconfig-r022-20220728
> > (https://download.01.org/0day-ci/archive/20220729/202207290518.1D7MVS65-lkp
> > @intel.com/config) compiler: clang version 15.0.0
> > (https://github.com/llvm/llvm-project
> > 8dfaecc4c24494337933aff9d9166486ca0949f1) 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 arm cross compiling tool for clang build
> >         # apt-get install binutils-arm-linux-gnueabi
> >         #
> > https://github.com/intel-lab-lkp/linux/commit/ee28006553d4d23f600b0076ef606
> > 6710519f156 git remote add linux-review
> > https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review
> > Alexander-Stein/OV9281-support/20220728-210448 git checkout
> > ee28006553d4d23f600b0076ef6066710519f156
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
> > O=build_dir ARCH=arm SHELL=/bin/bash drivers/media/i2c/
> > 
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > >> drivers/media/i2c/ov9282.c:1054:10: warning: variable 'ret' is
> > >> uninitialized when used here [-Wuninitialized]
> >                    return ret;
> >                           ^~~
> >    drivers/media/i2c/ov9282.c:1041:9: note: initialize the variable 'ret' to
> > silence this warning int ret;
> >                   ^
> >                    = 0
> >    1 warning generated.
> > 
> > 
> > vim +/ret +1054 drivers/media/i2c/ov9282.c
> > 
> >   1030
> >   1031	/**
> >   1032	 * ov9282_probe() - I2C client device binding
> >   1033	 * @client: pointer to i2c client device
> >   1034	 *
> >   1035	 * Return: 0 if successful, error code otherwise.
> >   1036	 */
> >   1037	static int ov9282_probe(struct i2c_client *client)
> >   1038	{
> >   1039		struct ov9282 *ov9282;
> >   1040		const char *sensor_name;
> >   1041		int ret;
> >   1042
> >   1043		ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), 
> GFP_KERNEL);
> >   1044		if (!ov9282)
> >   1045			return -ENOMEM;
> >   1046
> >   1047		ov9282->dev = &client->dev;
> >   1048
> >   1049		/* Initialize subdev */
> >   1050		v4l2_i2c_subdev_init(&ov9282->sd, client, 
> &ov9282_subdev_ops);
> >   1051		sensor_name = device_get_match_data(ov9282->dev);
> >   1052		if (!sensor_name) {
> >   1053			dev_err(ov9282->dev, "Sensor name is 
> missing");
> > 
> > > 1054			return ret;
> > 
> >   1055		}
> >   1056		v4l2_i2c_subdev_set_name(&ov9282->sd, client, 
> sensor_name, NULL);
> >   1057
> >   1058		ret = ov9282_parse_hw_config(ov9282);
> >   1059		if (ret) {
> >   1060			dev_err(ov9282->dev, "HW configuration is not 
> supported");
> >   1061			return ret;
> >   1062		}
> >   1063
> >   1064		ret = ov9282_get_regulators(ov9282);
> >   1065		if (ret) {
> >   1066			dev_err(&client->dev, "Failed to get power 
> regulators\n");
> >   1067			return ret;
> >   1068		}
> >   1069
> >   1070		mutex_init(&ov9282->mutex);
> >   1071
> >   1072		ret = ov9282_power_on(ov9282->dev);
> >   1073		if (ret) {
> >   1074			dev_err(ov9282->dev, "failed to power-on the 
> sensor");
> >   1075			goto error_mutex_destroy;
> >   1076		}
> >   1077
> >   1078		/* Check module identity */
> >   1079		ret = ov9282_detect(ov9282);
> >   1080		if (ret) {
> >   1081			dev_err(ov9282->dev, "failed to find sensor: 
> %d", ret);
> >   1082			goto error_power_off;
> >   1083		}
> >   1084
> >   1085		/* Set default mode to max resolution */
> >   1086		ov9282->cur_mode = &supported_mode;
> >   1087		ov9282->vblank = ov9282->cur_mode->vblank;
> >   1088
> >   1089		ret = ov9282_init_controls(ov9282);
> >   1090		if (ret) {
> >   1091			dev_err(ov9282->dev, "failed to init 
> controls: %d", ret);
> >   1092			goto error_power_off;
> >   1093		}
> >   1094
> >   1095		/* Initialize subdev */
> >   1096		ov9282->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >   1097		ov9282->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> >   1098
> >   1099		/* Initialize source pad */
> >   1100		ov9282->pad.flags = MEDIA_PAD_FL_SOURCE;
> >   1101		ret = media_entity_pads_init(&ov9282->sd.entity, 1, 
> &ov9282->pad);
> >   1102		if (ret) {
> >   1103			dev_err(ov9282->dev, "failed to init entity 
> pads: %d", ret);
> >   1104			goto error_handler_free;
> >   1105		}
> >   1106
> >   1107		ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
> >   1108		if (ret < 0) {
> >   1109			dev_err(ov9282->dev,
> >   1110				"failed to register async subdev: 
> %d", ret);
> >   1111			goto error_media_entity;
> >   1112		}
> >   1113
> >   1114		pm_runtime_set_active(ov9282->dev);
> >   1115		pm_runtime_enable(ov9282->dev);
> >   1116		pm_runtime_idle(ov9282->dev);
> >   1117
> >   1118		return 0;
> >   1119
> >   1120	error_media_entity:
> >   1121		media_entity_cleanup(&ov9282->sd.entity);
> >   1122	error_handler_free:
> >   1123		v4l2_ctrl_handler_free(ov9282->sd.ctrl_handler);
> >   1124	error_power_off:
> >   1125		ov9282_power_off(ov9282->dev);
> >   1126	error_mutex_destroy:
> >   1127		mutex_destroy(&ov9282->mutex);
> >   1128
> >   1129		return ret;
> >   1130	}
> >   1131
> 
> Meh, I'll come up with a fixed once discussion about the additional compatible 
> has settled. This will also include the missing member documentation in patch 
> 5

I think you could simply pass device_get_match_data() return value as the
sensor name. The driver is in direct control of the string so I don't think
you need error handling here.
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 352dbe21a902..f79bdfa821e8 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -1037,6 +1037,7 @@  static int ov9282_get_regulators(struct ov9282 *ov9282)
 static int ov9282_probe(struct i2c_client *client)
 {
 	struct ov9282 *ov9282;
+	const char *sensor_name;
 	int ret;
 
 	ov9282 = devm_kzalloc(&client->dev, sizeof(*ov9282), GFP_KERNEL);
@@ -1047,6 +1048,12 @@  static int ov9282_probe(struct i2c_client *client)
 
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
+	sensor_name = device_get_match_data(ov9282->dev);
+	if (!sensor_name) {
+		dev_err(ov9282->dev, "Sensor name is missing");
+		return ret;
+	}
+	v4l2_i2c_subdev_set_name(&ov9282->sd, client, sensor_name, NULL);
 
 	ret = ov9282_parse_hw_config(ov9282);
 	if (ret) {
@@ -1152,8 +1159,8 @@  static const struct dev_pm_ops ov9282_pm_ops = {
 };
 
 static const struct of_device_id ov9282_of_match[] = {
-	{ .compatible = "ovti,ov9281" },
-	{ .compatible = "ovti,ov9282" },
+	{ .compatible = "ovti,ov9281", .data = "ov9281" },
+	{ .compatible = "ovti,ov9282", .data = "ov9282" },
 	{ }
 };