[PATCH] media: gc0310: Stop setting v4l2_subdev.fwnode to the endpoint fwnode

[PATCH] media: gc0310: Stop setting v4l2_subdev.fwnode to the endpoint fwnode

[Hans de Goede]

Endpoint matching is now handled by the v4l2-core and drivers no longer
should set v4l2_subdev.fwnode to the endpoint fwnode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/i2c/atomisp-gc0310.c        | 20 +++++++++----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 9a11793f34f7..c438accb0472 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -83,7 +83,6 @@ struct gc0310_device {
 	struct mutex input_lock;
 	bool is_streaming;
 
-	struct fwnode_handle *ep_fwnode;
 	struct gpio_desc *reset;
 	struct gpio_desc *powerdown;
 
@@ -599,37 +598,37 @@ static void gc0310_remove(struct i2c_client *client)
 	media_entity_cleanup(&dev->sd.entity);
 	v4l2_ctrl_handler_free(&dev->ctrls.handler);
 	mutex_destroy(&dev->input_lock);
-	fwnode_handle_put(dev->ep_fwnode);
 	pm_runtime_disable(&client->dev);
 }
 
 static int gc0310_probe(struct i2c_client *client)
 {
+	struct fwnode_handle *ep_fwnode;
 	struct gc0310_device *dev;
 	int ret;
 
-	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
-	if (!dev)
-		return -ENOMEM;
-
 	/*
 	 * Sometimes the fwnode graph is initialized by the bridge driver.
 	 * Bridge drivers doing this may also add GPIO mappings, wait for this.
 	 */
-	dev->ep_fwnode = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
-	if (!dev->ep_fwnode)
+	ep_fwnode = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
+	if (!ep_fwnode)
 		return dev_err_probe(&client->dev, -EPROBE_DEFER, "waiting for fwnode graph endpoint\n");
 
+	fwnode_handle_put(ep_fwnode);
+
+	dev = devm_kzalloc(&client->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
 	dev->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(dev->reset)) {
-		fwnode_handle_put(dev->ep_fwnode);
 		return dev_err_probe(&client->dev, PTR_ERR(dev->reset),
 				     "getting reset GPIO\n");
 	}
 
 	dev->powerdown = devm_gpiod_get(&client->dev, "powerdown", GPIOD_OUT_HIGH);
 	if (IS_ERR(dev->powerdown)) {
-		fwnode_handle_put(dev->ep_fwnode);
 		return dev_err_probe(&client->dev, PTR_ERR(dev->powerdown),
 				     "getting powerdown GPIO\n");
 	}
@@ -652,7 +651,6 @@ static int gc0310_probe(struct i2c_client *client)
 	dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	dev->pad.flags = MEDIA_PAD_FL_SOURCE;
 	dev->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
-	dev->sd.fwnode = dev->ep_fwnode;
 
 	ret = gc0310_init_controls(dev);
 	if (ret) {
-- 
2.41.0

Re: [PATCH] media: gc0310: Stop setting v4l2_subdev.fwnode to the endpoint fwnode

[Andy Shevchenko]

On Sat, Aug 12, 2023 at 4:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Endpoint matching is now handled by the v4l2-core and drivers no longer
> should set v4l2_subdev.fwnode to the endpoint fwnode.

...

>         dev->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
>         if (IS_ERR(dev->reset)) {
> -               fwnode_handle_put(dev->ep_fwnode);
>                 return dev_err_probe(&client->dev, PTR_ERR(dev->reset),
>                                      "getting reset GPIO\n");
>         }

Now {} can be dropped.

...

>         dev->powerdown = devm_gpiod_get(&client->dev, "powerdown", GPIOD_OUT_HIGH);
>         if (IS_ERR(dev->powerdown)) {
> -               fwnode_handle_put(dev->ep_fwnode);
>                 return dev_err_probe(&client->dev, PTR_ERR(dev->powerdown),
>                                      "getting powerdown GPIO\n");
>         }

Ditto.

However, no strong opinion here as it is more style thingy. FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko