Hi Hans,
On Thu, Jun 27, 2024 at 08:53:22AM +0200, Hans Verkuil wrote:
> On 10/06/2024 12:05, Sakari Ailus wrote:
> > This reverts commit 6f0dd24a084a ("[media] media: fix media devnode
> > ioctl/syscall and unregister race"). The commit was part of an original
> > patchset to avoid crashes when an unregistering device is in use.
>
> Reverting all these old commits and then adding back some of the code that
> was removed is IMHO a bad idea.
>
> I took patches 1-8 and just folded them all together, and the end result
> was *much* easier to review. And the resulting patch can be cleaned up a
> bit as there are some unnecessary changes included (e.g. a cec change).
>
> With all the reverts and then reinstating code I also have little confidence
> in the quality of the code if you have to do a git bisect later and you
> end up in the middle of patches 1-8: it is far better to just do a single
> patch. Effectively it is embedding devnode in media_device, so just do
> that.
The reverts shouldn't really need a review as we're just reverting to an
earlier state of affairs in MC codebase. As you probably noticed, over the
first 8 patches there is little more than reverts while the rest are fixes
(as well as some preparation for what follows). So from review point of
view I do prefer the current state of the first 8 patches.
I'd like to have Laurent's opinion on this, too.
--
Kind regards,
Sakari Ailus
