From: Tomasz Figa <tfiga@chromium.org>
To: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Cc: Helen Koike <helen.koike@collabora.com>,
Linux Media Mailing List <linux-media@vger.kernel.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Ezequiel Garcia <ezequiel@collabora.com>,
Hans Verkuil <hverkuil@xs4all.nl>,
kernel@collabora.com, Dafna Hirschfeld <dafna3@gmail.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: [PATCH v2 5/9] media: staging: rkisp1: add a pointer to rkisp1_device from struct rkisp1_isp
Date: Wed, 5 Aug 2020 13:59:46 +0000 [thread overview]
Message-ID: <20200805135946.GA1015630@chromium.org> (raw)
In-Reply-To: <74c2563c-25ca-833b-2f72-b6e10e6e3847@collabora.com>
On Sat, Aug 01, 2020 at 06:08:06PM +0200, Dafna Hirschfeld wrote:
>
>
> On 21.07.20 17:32, Tomasz Figa wrote:
> > On Tue, Jul 21, 2020 at 5:30 PM Dafna Hirschfeld
> > <dafna.hirschfeld@collabora.com> wrote:
> > >
> > > Hi,
> > >
> > > On 21.07.20 14:36, Tomasz Figa wrote:
> > > > On Tue, Jul 21, 2020 at 2:26 PM Dafna Hirschfeld
> > > > <dafna.hirschfeld@collabora.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On 20.07.20 21:25, Helen Koike wrote:
> > > > > >
> > > > > >
> > > > > > On 7/18/20 11:59 AM, Dafna Hirschfeld wrote:
> > > > > > > The code in rkisp1-isp.c requires access to struct 'rkisp1_device'
> > > > > > > in several places. It access it using the 'container_of' macro.
> > > > > > > This patch adds a pointer to 'rkisp1_device' in struct 'rkisp1_isp'
> > > > > > > to simplify the access.
> > > > > >
> > > > > > What is wrong with container_of?
> > > > >
> > > > > I remember Laurent suggested it a while ago.
> > > > > I also feel container_of is a bit cumbersome and other entities already have a pointer to rkisp1_device.
> > > > >
> > > >
> > > > Do we even need the rkisp1_isp struct? Could we just pass rkisp1_device instead?
> > >
> > > pass to where ? You mean to the function rkisp1_mipi_csi2_start ?
> >
> > Yes, all around the driver, where rkisp1_isp is passed.
>
> Most of the functions are part of subdevice callback implementation
> where the rkisp1_device is not needed, so I don't the see the point.
Okay, so then I'd just lean towards keeping it as is. container_of is
generally considered more efficient than a pointer, because it doesn't
require a load operation to obtain a reference to the parent struct.
Best regards,
Tomasz
next prev parent reply other threads:[~2020-08-05 13:59 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-18 14:59 [PATCH v2 0/9] media: staging: rkisp1: document rkisp1-common.h Dafna Hirschfeld
2020-07-18 14:59 ` [PATCH v2 1/9] media: staging: rkisp1: remove unused field ctrl_handler from struct rkisp1_device Dafna Hirschfeld
2020-07-18 14:59 ` [PATCH v2 2/9] media: staging: rkisp1: remove unused field alloc_ctx " Dafna Hirschfeld
2020-07-18 14:59 ` [PATCH v2 3/9] media: staging: rkisp1: set pads array of the resizer to size 2 Dafna Hirschfeld
2020-07-18 14:59 ` [PATCH v2 4/9] media: staging: rkisp1: don't define vaddr field in rkisp1_buffer as an array Dafna Hirschfeld
2020-07-20 19:22 ` Helen Koike
2020-07-18 14:59 ` [PATCH v2 5/9] media: staging: rkisp1: add a pointer to rkisp1_device from struct rkisp1_isp Dafna Hirschfeld
2020-07-20 19:25 ` Helen Koike
2020-07-21 12:26 ` Dafna Hirschfeld
2020-07-21 12:36 ` Tomasz Figa
2020-07-21 15:30 ` Dafna Hirschfeld
2020-07-21 15:32 ` Tomasz Figa
2020-08-01 16:08 ` Dafna Hirschfeld
2020-08-05 13:59 ` Tomasz Figa [this message]
2020-07-18 14:59 ` [PATCH v2 6/9] media: staging: rkisp1: unify (un)register functions to have the same parameters Dafna Hirschfeld
2020-07-20 19:26 ` Helen Koike
2020-07-18 14:59 ` [PATCH v2 7/9] media: staging: rkisp1: remove declaration of unimplemented function 'rkisp1_params_isr_handler' Dafna Hirschfeld
2020-07-20 19:26 ` Helen Koike
2020-07-18 14:59 ` [PATCH v2 8/9] media: staging: rkisp1: group declaration of similar functions together Dafna Hirschfeld
2020-07-20 19:29 ` Helen Koike
2020-07-18 14:59 ` [PATCH v2 9/9] media: staging: rkisp1: improve documentation of rkisp1-common.h Dafna Hirschfeld
2020-07-20 19:36 ` Helen Koike
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=20200805135946.GA1015630@chromium.org \
--to=tfiga@chromium.org \
--cc=dafna.hirschfeld@collabora.com \
--cc=dafna3@gmail.com \
--cc=ezequiel@collabora.com \
--cc=helen.koike@collabora.com \
--cc=hverkuil@xs4all.nl \
--cc=kernel@collabora.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mchehab@kernel.org \
--cc=sakari.ailus@linux.intel.com \
/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.