Re: [PATCH 21/57] media: atomisp: Remove atomisp_gmin_find_subdev()

[Andy Shevchenko <andy@kernel.org>]

On Mon, Jan 23, 2023 at 01:51:29PM +0100, Hans de Goede wrote:
> atomisp_gmin_find_subdev() can be used to lookup a subdev
> given its i2c-adapter + i2c-client-address.
> 
> But the only caller of it reads this from the intel_v4l2_subdev_table
> struct and that same struct already contains a pointer to the v4l2_subdev.
> 
> So this function is not necessary, drop it and modify its only caller
> to directly take the subdev from the intel_v4l2_subdev_table struct.
> 
> Also drop struct intel_v4l2_subdev_i2c_board_info since that now no
> longer is used.

Cool!

Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../include/linux/atomisp_gmin_platform.h     |  2 -
>  .../atomisp/include/linux/atomisp_platform.h  |  6 ---
>  .../media/atomisp/pci/atomisp_gmin_platform.c | 27 ----------
>  .../staging/media/atomisp/pci/atomisp_v4l2.c  | 54 +++----------------
>  4 files changed, 7 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/include/linux/atomisp_gmin_platform.h b/drivers/staging/media/atomisp/include/linux/atomisp_gmin_platform.h
> index 5463d11d4295..64bd54835c32 100644
> --- a/drivers/staging/media/atomisp/include/linux/atomisp_gmin_platform.h
> +++ b/drivers/staging/media/atomisp/include/linux/atomisp_gmin_platform.h
> @@ -21,8 +21,6 @@
>  int atomisp_register_i2c_module(struct v4l2_subdev *subdev,
>  				struct camera_sensor_platform_data *plat_data,
>  				enum intel_v4l2_subdev_type type);
> -struct v4l2_subdev *atomisp_gmin_find_subdev(struct i2c_adapter *adapter,
> -	struct i2c_board_info *board_info);
>  int atomisp_gmin_remove_subdev(struct v4l2_subdev *sd);
>  int gmin_get_var_int(struct device *dev, bool is_gmin,
>  		     const char *var, int def);
> diff --git a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> index 559a497975c5..82973aa0e1eb 100644
> --- a/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> +++ b/drivers/staging/media/atomisp/include/linux/atomisp_platform.h
> @@ -125,13 +125,7 @@ struct intel_v4l2_subdev_id {
>  	enum atomisp_camera_port    port;
>  };
>  
> -struct intel_v4l2_subdev_i2c_board_info {
> -	struct i2c_board_info board_info;
> -	int i2c_adapter_id;
> -};
> -
>  struct intel_v4l2_subdev_table {
> -	struct intel_v4l2_subdev_i2c_board_info v4l2_subdev;
>  	enum intel_v4l2_subdev_type type;
>  	enum atomisp_camera_port port;
>  	struct v4l2_subdev *subdev;
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 6116d3c62315..234088711f29 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -147,7 +147,6 @@ int atomisp_register_i2c_module(struct v4l2_subdev *subdev,
>  				enum intel_v4l2_subdev_type type)
>  {
>  	int i;
> -	struct i2c_board_info *bi;
>  	struct gmin_subdev *gs;
>  	struct i2c_client *client = v4l2_get_subdevdata(subdev);
>  	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> @@ -181,36 +180,10 @@ int atomisp_register_i2c_module(struct v4l2_subdev *subdev,
>  	pdata.subdevs[i].type = type;
>  	pdata.subdevs[i].port = gs->csi_port;
>  	pdata.subdevs[i].subdev = subdev;
> -	pdata.subdevs[i].v4l2_subdev.i2c_adapter_id = client->adapter->nr;
> -
> -	/* Convert i2c_client to i2c_board_info */
> -	bi = &pdata.subdevs[i].v4l2_subdev.board_info;
> -	memcpy(bi->type, client->name, I2C_NAME_SIZE);
> -	bi->flags = client->flags;
> -	bi->addr = client->addr;
> -	bi->irq = client->irq;
> -	bi->platform_data = plat_data;
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(atomisp_register_i2c_module);
>  
> -struct v4l2_subdev *atomisp_gmin_find_subdev(struct i2c_adapter *adapter,
> -	struct i2c_board_info *board_info)
> -{
> -	int i;
> -
> -	for (i = 0; i < MAX_SUBDEVS && pdata.subdevs[i].type; i++) {
> -		struct intel_v4l2_subdev_table *sd = &pdata.subdevs[i];
> -
> -		if (sd->v4l2_subdev.i2c_adapter_id == adapter->nr &&
> -		    sd->v4l2_subdev.board_info.addr == board_info->addr)
> -			return sd->subdev;
> -	}
> -	return NULL;
> -}
> -EXPORT_SYMBOL_GPL(atomisp_gmin_find_subdev);
> -
>  int atomisp_gmin_remove_subdev(struct v4l2_subdev *sd)
>  {
>  	int i, j;
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 2a949d3dc5d1..ba628f7cf385 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -937,45 +937,9 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
>  	/* FIXME: should, instead, use I2C probe */
>  
>  	for (subdevs = pdata->subdevs; subdevs->type; ++subdevs) {
> -		struct v4l2_subdev *subdev;
> -		struct i2c_board_info *board_info =
> -			    &subdevs->v4l2_subdev.board_info;
> -		struct i2c_adapter *adapter =
> -		    i2c_get_adapter(subdevs->v4l2_subdev.i2c_adapter_id);
> -
> -		dev_info(isp->dev, "Probing Subdev %s\n", board_info->type);
> -
> -		if (!adapter) {
> -			dev_err(isp->dev,
> -				"Failed to find i2c adapter for subdev %s\n",
> -				board_info->type);
> -			break;
> -		}
> -
> -		/* In G-Min, the sensor devices will already be probed
> -		 * (via ACPI) and registered, do not create new
> -		 * ones */
> -		subdev = atomisp_gmin_find_subdev(adapter, board_info);
> -		if (!subdev) {
> -			dev_warn(isp->dev, "Subdev %s not found\n",
> -				 board_info->type);
> -			continue;
> -		}
> -		ret = v4l2_device_register_subdev(&isp->v4l2_dev, subdev);
> -		if (ret) {
> -			dev_warn(isp->dev, "Subdev %s detection fail\n",
> -				 board_info->type);
> +		ret = v4l2_device_register_subdev(&isp->v4l2_dev, subdevs->subdev);
> +		if (ret)
>  			continue;
> -		}
> -
> -		if (!subdev) {
> -			dev_warn(isp->dev, "Subdev %s detection fail\n",
> -				 board_info->type);
> -			continue;
> -		}
> -
> -		dev_info(isp->dev, "Subdev %s successfully register\n",
> -			 board_info->type);
>  
>  		switch (subdevs->type) {
>  		case RAW_CAMERA:
> @@ -992,7 +956,7 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
>  
>  			isp->inputs[isp->input_cnt].type = subdevs->type;
>  			isp->inputs[isp->input_cnt].port = subdevs->port;
> -			isp->inputs[isp->input_cnt].camera = subdev;
> +			isp->inputs[isp->input_cnt].camera = subdevs->subdev;
>  			isp->inputs[isp->input_cnt].sensor_index = 0;
>  			/*
>  			 * initialize the subdev frame size, then next we can
> @@ -1004,22 +968,18 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
>  			break;
>  		case CAMERA_MOTOR:
>  			if (isp->motor) {
> -				dev_warn(isp->dev,
> -					 "too many atomisp motors, ignored %s\n",
> -					 board_info->type);
> +				dev_warn(isp->dev, "too many atomisp motors\n");
>  				continue;
>  			}
> -			isp->motor = subdev;
> +			isp->motor = subdevs->subdev;
>  			break;
>  		case LED_FLASH:
>  		case XENON_FLASH:
>  			if (isp->flash) {
> -				dev_warn(isp->dev,
> -					 "too many atomisp flash devices, ignored %s\n",
> -					 board_info->type);
> +				dev_warn(isp->dev, "too many atomisp flash devices\n");
>  				continue;
>  			}
> -			isp->flash = subdev;
> +			isp->flash = subdevs->subdev;
>  			break;
>  		default:
>  			dev_dbg(isp->dev, "unknown subdev probed\n");
> -- 
> 2.39.0
> 

-- 
With Best Regards,
Andy Shevchenko