Re: [Patch 08/13] media: am437x-vpfe: Maintain a reference to the current vpfe_fmt

[Benoit Parrot]

Hans Verkuil <hverkuil@xxxxxxxxx> wrote on Fri [2019-Sep-13 15:34:28 +0200]:
> On 9/13/19 3:32 PM, Benoit Parrot wrote:
> > Hans Verkuil <hverkuil@xxxxxxxxx> wrote on Fri [2019-Sep-13 15:14:45 +0200]:
> >> On 9/9/19 6:27 PM, Benoit Parrot wrote:
> >>> Keep a reference to the currently selected struct vpfe_fmt * object.
> >>> By doing so we rename the current struct v4l2_format * member from
> >>> fmt to v_fmt.
> >>> The added struct vpfe_fmt * reference is set to "const" so we also
> >>> constify all accesses and related helper functions.
> >>
> >> This explains what you do, but not why you do it.
> > 
> > Hmm ok I'll rework this a bit.
> > 
> >>
> >> And I think fmt vs v_fmt is *very* confusing naming.
> > 
> > Well in this case v_fmt stands for "v4l2_fmt" and depending on the function
> > I was using fmt to either mean a vpfe_fmt pointer or a v4l2_format pointer
> > so tried (and apparently failed) to make it clearer which was which.
> 
> Well, v_fmt could refer to either v4l2_format or vpfe_fmt, so that prefix
> doesn't help me :-)
> 
> Since 'fmt' was already defined in struct vpfe_device as v4l2_format, I'd
> just keep that rather than causing code churn by changing it. And come up
> with a better name for when you refer to a vpfe_fmt struct.

Fair enough.

> 
> Regards,
> 
> 	Hans
> 
> > 
> >>
> >> I'd keep fmt as-is, and come up with a different name for the vpfe_fmt
> >> pointer. ref_vpfe_fmt?
> >>
> >> Regards,
> >>
> >> 	Hans
> >>