On 21/02/2024 12:40, Sakari Ailus wrote:
> Hi Hans,
>
> On Wed, Feb 21, 2024 at 12:19:21PM +0100, Hans Verkuil wrote:
>> On 21/02/2024 11:53, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Mon, Feb 05, 2024 at 04:02:24PM +0100, Hans Verkuil wrote:
>>>> On 20/12/2023 11:37, Sakari Ailus wrote:
>>>>> Release all the resources when the media device is related, moving away
>>>
>>> s/related/released/
>>>
>>>>> form the struct v4l2_device used for that purpose.
>>>>
>>>> form -> from
>>>
>>> Yes.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>> drivers/media/test-drivers/vimc/vimc-core.c | 15 +++++++++------
>>>>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
>>>>> index af127476e920..3e59f8c256c7 100644
>>>>> --- a/drivers/media/test-drivers/vimc/vimc-core.c
>>>>> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
>>>>> @@ -264,13 +264,12 @@ static int vimc_add_subdevs(struct vimc_device *vimc)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> -static void vimc_v4l2_dev_release(struct v4l2_device *v4l2_dev)
>>>>> +static void vimc_mdev_release(struct media_device *mdev)
>>>>> {
>>>>> struct vimc_device *vimc =
>>>>> - container_of(v4l2_dev, struct vimc_device, v4l2_dev);
>>>>> + container_of_const(mdev, struct vimc_device, mdev);
>>>>
>>>> Why this change?
>>>
>>> I changed the line already. There's no reason to continue using
>>> container_of() instead of container_of_const() that takes const-ness into
>>> account, too.
>>
>> But neither vimc nor mdev can be const anyway, that would make no sense
>> here.
>
> Neither is const, true. Yet container_of_const() is preferred over
Says who?
It makes sense in generic defines that use it, e.g.:
drivers/base/firmware_loader/sysfs.h:#define to_fw_sysfs(__dev) container_of_const(__dev, struct fw_sysfs, dev)
That way it can handle both const and non-const __dev pointers.
In cases where this doesn't come into play I think there is no need to
make code changes. Perhaps when writing new code it might make sense to
use it, but changing it in existing code, esp. as part of a patch that
deals with something else entirely, seems just unnecessary churn.
I won't block this, but I recommend just dropping this change in this patch.
Regards,
Hans
> container_of(), due to the fact that it does take const-ness into account.
> container_of() should really be avoided.
>
> I'll add this to the commit message as Laurent suggested.
>
