Re: [PATCH v5 2/2] media: i2c: mira016: Add driver for Mira016

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Sakari

On Thu, Oct 01, 2026 at 09:46:08AM +0300, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Sep 30, 2026 at 12:51:20PM +0200, Jacopo Mondi wrote:
> > Add driver for the ams OSRAM Mira016 sensor.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > +static int mira016_parse_endpoint(struct device *dev, struct mira016 *mira016)
> > +{
> > +	struct fwnode_handle *endpoint __free(fwnode_handle) = NULL;
> > +	struct v4l2_fwnode_endpoint ep_cfg = {
> > +		.bus_type = V4L2_MBUS_CSI2_DPHY
> > +	};
> > +	int ret;
> > +
> > +	endpoint = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
> > +	if (!endpoint)
> > +		return -ENODEV;
> > +
> > +	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &ep_cfg);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * Link frequencies: the driver supports a single link frequency,
> > +	 * no need to check bitmap after this call.
> > +	 */
> > +	ret = v4l2_link_freq_to_bitmap(dev, ep_cfg.link_frequencies,
> > +				       ep_cfg.nr_of_link_frequencies,
> > +				       mira016_link_freqs,
> > +				       ARRAY_SIZE(mira016_link_freqs),
> > +				       &mira016->link_freq_bitmap);
> > +	if (ret) {
> > +		v4l2_fwnode_endpoint_free(&ep_cfg);
>
> I recall commenting about this at in least three occasions earlier.

And, again, I have replied twice to your comment without receiving a
response: https://lore.kernel.org/linux-media/arPJ_-yKVMXE-Gav@zed/

I'll repeat here anyway: do not mix cleanups and gotos. In this case
it's harmless, but why contradict the usage notes to avoid typing out
v4l2_fwnode_endpoint_free() 2 times ?

>
> Also applies to the other driver.
>
> > +		return ret;
> > +	}
> > +
> > +	/* TODO: Implement D-PHY configuration to support continuous clock. */
> > +	if (!(ep_cfg.bus.mipi_csi2.flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
> > +		dev_err(dev, "Continuous clock is not supported\n");
> > +		v4l2_fwnode_endpoint_free(&ep_cfg);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mira016->bus_config = ep_cfg.bus.mipi_csi2.flags;
> > +
> > +	v4l2_fwnode_endpoint_free(&ep_cfg);
> > +
> > +	return 0;
> > +}
>
> --
> Regards,
>
> Sakari Ailus