Re: [PATCH v2 0/3] Drop control handler argument support for v4l2_get_link_freq()

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Thu, Aug 21, 2025 at 04:27:09PM +0300, Laurent Pinchart wrote:
> On Thu, Aug 21, 2025 at 04:24:04PM +0300, Sakari Ailus wrote:
> > On Thu, Aug 21, 2025 at 04:01:25PM +0300, Laurent Pinchart wrote:
> > > On Thu, Aug 21, 2025 at 03:56:20PM +0300, Sakari Ailus wrote:
> > > > On Thu, Aug 21, 2025 at 03:41:48PM +0300, Laurent Pinchart wrote:
> > > > > On Thu, Aug 21, 2025 at 03:12:04PM +0300, Sakari Ailus wrote:
> > > > > > Hi folks,
> > > > > > 
> > > > > > This set removes the workaround from v4l2_get_link_freq() that allowed
> > > > > > calling it on a control handler. The Amlogic c3-mipi-csi2 driver is
> > > > > > converted in the process as well.
> > > > > > 
> > > > > > As a result there's also no definition of v4l2_get_link_freq() without
> > > > > > CONFIG_MEDIA_CONTROLLER but that should be fine now. The Amlogic patch is
> > > > > > untested.
> > > > > 
> > > > > Could you please cherry-pick
> > > > > https://lore.kernel.org/linux-media/20250821000944.27849-2-laurent.pinchart@ideasonboard.com/
> > > > > and include it in this series ?
> > > > 
> > > > If a pad is const, I'd expect the container sub-device would be, too. I'll
> > > > see if anything breaks if media_entity_to_v4l2_subdev() would switch to use
> > > > container_of_const().
> > > 
> > > pad->entity will still not be const, so media_entity_to_v4l2_subdev()
> > > will return a non-const pointer anyway.
> > 
> > It may be technically correct but const suggests the argument (and what is
> > referred from that) is const henceforth, but that's not the case.
> 
> Yes, and that's why I'm adding a const here. The function retrieves the
> link frequency, it should not change the state of the pad, entity or
> subdev. That's the meaning conveyed by the API change. Internally, due
> to the nature of the C language, we go from a const struct media_pad to
> a non-const sturct media_entity. That's fine, it's inside the
> v4l2_get_link_freq() function, and it means the function must simply be
> careful not to break the API contract. It doesn't change the nature of
> the contract.

I'd like to have a third opinion here. Hans?

> 
> > I simply wouldn't change this. 
> 

-- 
Regards,

Sakari Ailus