Re: [PATCH v8 2/2] media: i2c: Add driver for AMS-OSRAM Mira220

[Jai Luthra]

Hi Jacopo,

Thank you for the fixes!

Quoting Jacopo Mondi (2026-07-24 21:07:01)
> From: Philippe Baetens <philippebaetens@xxxxxxxxx>
> 
> Add a V4L2 subdev driver for driver for the AMS-OSRAM Mira220 image
> sensor.
> 
> Mira220 is a global shutter image sensor with a resolution of 1600x1400
> pixels.
> 
> The driver implements support for mono and RGB 12, 10 and 8 bits
> formats. The output data-rate per lane is 1500Mbit/s, with a maximum
> frame rate up to 90 fps.
> 
> Signed-off-by: Philippe Baetens <philippebaetens@xxxxxxxxx>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> 

[...]

> +static int mira220_parse_endpoint(struct device *dev, struct mira220 *mira220)
> +{
> +       struct fwnode_handle *endpoint;
> +       struct v4l2_fwnode_endpoint ep_cfg = {
> +               .bus_type = V4L2_MBUS_CSI2_DPHY
> +       };
> +       int ret = 0;
> +
> +       endpoint = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
> +       if (!endpoint) {
> +               dev_err(dev, "Endpoint node not found\n");
> +               return -EINVAL;
> +       }
> +
> +       if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg)) {
> +               ret = -EINVAL;
> +               dev_err(dev, "Failed to parse endpoint\n");
> +               goto error_out;
> +       }
> +
> +       /* Non-continuous mode not implemented. */
> +       if (ep_cfg.bus.mipi_csi2.flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK) {
> +               dev_warn(dev, "clock non-continuous mode not supported\n");
> +               ret = -EINVAL;
> +               goto error_out;
> +       }
> +
> +       if (ep_cfg.nr_of_link_frequencies > ARRAY_SIZE(mira220_link_freqs)) {
> +               ret = -EINVAL;
> +               dev_err(dev, "Unsupported number of link_frequencies: %u\n",
> +                       ep_cfg.nr_of_link_frequencies);
> +               goto error_out;
> +       }
> +
> +       if (ep_cfg.link_frequencies[0] != MIRA220_LINK_FREQ_750M) {

The LLM-bot says link_frequencies[0] might be invalid if DT omitted the
link-frequencies property, and I think that's a valid concern.

Any particular reason for not using the v4l2_link_freq_to_bitmap() helper I
had suggested? It can handles all of these cases and error msgs too.

With the edge case fixed (with or without the helper):

Reviewed-by: Jai Luthra <jai.luthra@xxxxxxxxxxxxxxxx>

Thanks,
    Jai
> +               ret = -EINVAL;
> +               dev_err(dev, "Unsupported link_frequency: %llu\n",
> +                       ep_cfg.link_frequencies[0]);
> +               goto error_out;
> +       }
> +
> +       /* Check the number of MIPI CSI2 data lanes */
> +       if (ep_cfg.bus.mipi_csi2.num_data_lanes != 1 &&
> +           ep_cfg.bus.mipi_csi2.num_data_lanes != 2) {
> +               ret = -EINVAL;
> +               dev_err(dev, "%u data lanes are not supported\n",
> +                       ep_cfg.bus.mipi_csi2.num_data_lanes);
> +               goto error_out;
> +       }
> +
> +       mira220->lanes = ep_cfg.bus.mipi_csi2.num_data_lanes;
> +       mira220->row_length = MIRA220_ROW_LENGTH_MIN * (2 / mira220->lanes);
> +
> +error_out:
> +       v4l2_fwnode_endpoint_free(&ep_cfg);
> +       fwnode_handle_put(endpoint);
> +
> +       return ret;
> +}
> +