From: Maxime Ripard <maxime@cerno.tech>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Thomas Zimmermann <tzimmermann@suse.de>,
Emma Anholt <emma@anholt.net>,
Dave Stevenson <dave.stevenson@raspberrypi.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 1/9] drm/vc4: Switch to container_of_const
Date: Tue, 21 Feb 2023 12:38:34 +0100 [thread overview]
Message-ID: <20230221113834.i3nitxp4soev6cks@houat> (raw)
In-Reply-To: <c70e40fe-6834-2382-ec89-28714a67fd1f@xs4all.nl>
Hi Hans,
On Sat, Feb 18, 2023 at 11:45:04AM +0100, Hans Verkuil wrote:
> > diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> > index 86d629e45307..d0a00ed42cb0 100644
> > --- a/drivers/gpu/drm/vc4/vc4_bo.c
> > +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> > @@ -609,7 +609,7 @@ static void vc4_free_object(struct drm_gem_object *gem_bo)
> > static void vc4_bo_cache_time_work(struct work_struct *work)
> > {
> > struct vc4_dev *vc4 =
> > - container_of(work, struct vc4_dev, bo_cache.time_work);
> > + container_of_const(work, struct vc4_dev, bo_cache.time_work);
>
> ...I think this is misleading. It's definitely not const, so switching to
> container_of_const suggests that there is some 'constness' involved, which
> isn't the case. I'd leave this just as 'container_of'. This also reduces the
> size of the patch, since this is done in quite a few places.
The name threw me off too, but it's supposed to keep the argument
pointer constness, not always take and return a const pointer. I still
believe that it's beneficial since, if the work pointer was ever to
change constness, we would have that additional check.
Maxime
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Emma Anholt <emma@anholt.net>, David Airlie <airlied@gmail.com>,
Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Dave Stevenson <dave.stevenson@raspberrypi.com>,
Thomas Zimmermann <tzimmermann@suse.de>
Subject: Re: [PATCH v2 1/9] drm/vc4: Switch to container_of_const
Date: Tue, 21 Feb 2023 12:38:34 +0100 [thread overview]
Message-ID: <20230221113834.i3nitxp4soev6cks@houat> (raw)
In-Reply-To: <c70e40fe-6834-2382-ec89-28714a67fd1f@xs4all.nl>
Hi Hans,
On Sat, Feb 18, 2023 at 11:45:04AM +0100, Hans Verkuil wrote:
> > diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> > index 86d629e45307..d0a00ed42cb0 100644
> > --- a/drivers/gpu/drm/vc4/vc4_bo.c
> > +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> > @@ -609,7 +609,7 @@ static void vc4_free_object(struct drm_gem_object *gem_bo)
> > static void vc4_bo_cache_time_work(struct work_struct *work)
> > {
> > struct vc4_dev *vc4 =
> > - container_of(work, struct vc4_dev, bo_cache.time_work);
> > + container_of_const(work, struct vc4_dev, bo_cache.time_work);
>
> ...I think this is misleading. It's definitely not const, so switching to
> container_of_const suggests that there is some 'constness' involved, which
> isn't the case. I'd leave this just as 'container_of'. This also reduces the
> size of the patch, since this is done in quite a few places.
The name threw me off too, but it's supposed to keep the argument
pointer constness, not always take and return a const pointer. I still
believe that it's beneficial since, if the work pointer was ever to
change constness, we would have that additional check.
Maxime
next prev parent reply other threads:[~2023-02-21 11:38 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 13:46 [PATCH v2 0/9] drm/vc4: hdmi: Broadcast RGB, BT601, BT2020 Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 1/9] drm/vc4: Switch to container_of_const Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-02-18 10:45 ` Hans Verkuil
2023-02-18 10:45 ` Hans Verkuil
2023-02-21 11:38 ` Maxime Ripard [this message]
2023-02-21 11:38 ` Maxime Ripard
2023-02-21 14:18 ` Hans Verkuil
2023-02-21 14:18 ` Hans Verkuil
2023-01-26 13:46 ` [PATCH v2 2/9] drm/vc4: hdmi: Update all the planes if the TV margins are changed Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 3/9] drm/vc4: hdmi: Add Broadcast RGB property to allow override of RGB range Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-02-18 11:33 ` Hans Verkuil
2023-02-18 11:33 ` Hans Verkuil
2023-02-20 15:23 ` Dave Stevenson
2023-02-20 15:23 ` Dave Stevenson
2023-02-21 11:27 ` Hans Verkuil
2023-02-21 11:27 ` Hans Verkuil
2023-02-21 11:38 ` Sebastian Wick
2023-02-21 11:38 ` Sebastian Wick
2023-01-26 13:46 ` [PATCH v2 4/9] drm/vc4: hdmi: Rename full range helper Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 5/9] drm/vc4: hdmi: Swap CSC matrix channels for YUV444 Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 6/9] drm/vc4: hdmi: Rework the CSC matrices organization Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 7/9] drm/vc4: hdmi: Add a function to retrieve the CSC matrix Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 8/9] drm/vc4: hdmi: Add BT.601 Support Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
2023-01-26 13:46 ` [PATCH v2 9/9] drm/vc4: hdmi: Add BT.2020 Support Maxime Ripard
2023-01-26 13:46 ` Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230221113834.i3nitxp4soev6cks@houat \
--to=maxime@cerno.tech \
--cc=dave.stevenson@raspberrypi.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=emma@anholt.net \
--cc=hverkuil@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.