linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  reply	other threads:[~2020-08-05 19:53 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).