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@xxxxxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > On 21.07.20 14:36, Tomasz Figa wrote: > > > > On Tue, Jul 21, 2020 at 2:26 PM Dafna Hirschfeld > > > > <dafna.hirschfeld@xxxxxxxxxxxxx> 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
