Hi Hans,
On Thu, May 25, 2023 at 09:00:58PM +0200, Hans de Goede wrote:
> Switch the atomisp-gc0310 driver to v4l2 async device registration.
>
> After this change this driver no longer depends on
> atomisp_gmin_platform and all atomisp-isms are gone.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Changes in v2:
> - Drop v4l2_get_acpi_sensor_info() call in this patch
> - Wait for fwnode graph endpoint so that the bridge's ACPI
> parsing gets a chance to register the GPIO mappings
> before probing the sensor
> - Switch to endpoint matching
> ---
> .../media/atomisp/i2c/atomisp-gc0310.c | 29 ++++++++++++-------
> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 ++
> 2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index 1829ba419e3e..9a11793f34f7 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -29,8 +29,6 @@
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-device.h>
>
> -#include "../include/linux/atomisp_gmin_platform.h"
> -
> #define GC0310_NATIVE_WIDTH 656
> #define GC0310_NATIVE_HEIGHT 496
>
> @@ -85,6 +83,7 @@ struct gc0310_device {
> struct mutex input_lock;
> bool is_streaming;
>
> + struct fwnode_handle *ep_fwnode;
> struct gpio_desc *reset;
> struct gpio_desc *powerdown;
>
> @@ -596,11 +595,11 @@ static void gc0310_remove(struct i2c_client *client)
>
> dev_dbg(&client->dev, "gc0310_remove...\n");
>
> - atomisp_unregister_subdev(sd);
> - v4l2_device_unregister_subdev(sd);
> + v4l2_async_unregister_subdev(sd);
> 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);
> }
>
> @@ -613,19 +612,27 @@ static int gc0310_probe(struct i2c_client *client)
> if (!dev)
> return -ENOMEM;
>
> - ret = v4l2_get_acpi_sensor_info(&client->dev, NULL);
> - if (ret)
> - return ret;
> + /*
> + * 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)
> + return dev_err_probe(&client->dev, -EPROBE_DEFER, "waiting for fwnode graph endpoint\n");
>
> dev->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
> - if (IS_ERR(dev->reset))
> + 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))
> + 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");
> + }
>
> mutex_init(&dev->input_lock);
> v4l2_i2c_subdev_init(&dev->sd, client, &gc0310_ops);
> @@ -645,6 +652,7 @@ 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;
Same for this one: leave as-is --- the v4l2_async_register_subdev()
function will set the device fwnode for this.
>
> ret = gc0310_init_controls(dev);
> if (ret) {
> @@ -658,8 +666,7 @@ static int gc0310_probe(struct i2c_client *client)
> return ret;
> }
This driver should (as well as ov2680) check for the link frequencies. This
is an old sensor so if all users eventually use this via firmware that
lacks this information, there's little benefit for adding the code. But
otherwise this is seen as a bug.
<URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks
The raw cameras should use pixel rate and blanking controls for configuring
the frame interval. This one uses s_frame_interval instead, and it may be
difficult to find the information needed for the pixel rate based API.
Cc Jacopo.
>
> - ret = atomisp_register_sensor_no_gmin(&dev->sd, 1, ATOMISP_INPUT_FORMAT_RAW_8,
> - atomisp_bayer_order_grbg);
> + ret = v4l2_async_register_subdev_sensor(&dev->sd);
> if (ret) {
> gc0310_remove(client);
> return ret;
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> index d7d9cac2c3b8..5268a0d25051 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> @@ -89,6 +89,8 @@ static const guid_t atomisp_dsm_guid =
> * power-management and with v4l2-async probing.
> */
> static const struct atomisp_csi2_sensor_config supported_sensors[] = {
> + /* GalaxyCore GC0310 */
> + { "INT0310", 1 },
> /* Omnivision OV2680 */
> { "OVTI2680", 1 },
> };
--
Kind regards,
Sakari Ailus
