driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: devel@driverdev.osuosl.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 09/11] media: atomisp: partially get rid of one abstraction layer
Date: Tue, 26 May 2020 10:41:11 +0200	[thread overview]
Message-ID: <20200526104111.52a6ef23@coco.lan> (raw)
In-Reply-To: <20200526072605.GJ7618@paasikivi.fi.intel.com>

Em Tue, 26 May 2020 10:26:05 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> Hi Mauro,
> 
> On Mon, May 25, 2020 at 08:56:08AM +0200, Mauro Carvalho Chehab wrote:
> > The very same macros are defined as CSS_foo and IA_CSS_foo.
> > 
> > Remove this abstraction, as it just make things confusing,
> > for no good reason.  
> 
> I think this boils down to which prefix should the uAPI structs of this
> driver use. I'd prefer atomisp_something, ia_css has been used internally,
> and that probably has been there in later firmware versions (vs. just css).
> 
> At this point removing the duplication makes sense though, so I'm not
> proposing changes here.

Yeah, agreed. From CSS PoV, there are several abstraction layers: the "hive"
code, the "css" code (overlayed by a css layer for isp2400 and another one
for isp2401), the "ia_css" layer, the "compat_css20" layer, and the
"ia_css" layer. Finally, the "atomisp" upper layer.

From namespace PoV, IA_CSS_foo is worse than CSS_foo. The best would be 
to use ATOMISP_foo, but there are still too much layers there abstracting
the code. For now, I prefer to keep the name used by the "IA_" layer,
in order to avoid needing to deal with symbol conflicts. Let's get
rid of it when we remove the IA_ abstraction as a hole.

Btw, the vast majority of this driver's source code is due to all those
layers.

We need to be careful to remove them. There are even some duplicated
structures there for the same thing, with some ugly compile-time
checks to identify if the same struct (with different name) still
preserves the same size. For now, I'm refraining touching these, as 
some of those structs could be used by the firmware (check needed).

My plan is to do the renaming once we get rid of those abstractions.

-

Btw, from practical PoV, I suspect that the next layer to be
removed would be the atomisp_compat_css20 one. Several functions
there are just wrapper ones.

Thanks,
Mauro
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

  reply	other threads:[~2020-05-26  8:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25  6:55 [PATCH 00/11] Some fixes and cleanups for atomisp driver Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 01/11] media: atomisp: get rid of hmm_vm.c Mauro Carvalho Chehab
2020-05-27  9:35   ` kbuild test robot
2020-05-25  6:56 ` [PATCH 02/11] media: atomisp: reduce debug printk rate when IRQs are received Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 03/11] media: atomisp: avoid a copy of v4l2_mbus_framefmt at stack Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 04/11] media: atomisp: improve debug messages for set format Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 05/11] media: atomisp: don't flood dmesg with -EAGAIN return codes Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 06/11] media: atomisp: update TODO list Mauro Carvalho Chehab
2020-05-26  7:21   ` Sakari Ailus
2020-05-25  6:56 ` [PATCH 07/11] media: atomisp: get rid of some old broken debug code Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 08/11] media: atomisp: make it use dbg_level to control debug level Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 09/11] media: atomisp: partially get rid of one abstraction layer Mauro Carvalho Chehab
2020-05-26  7:26   ` Sakari Ailus
2020-05-26  8:41     ` Mauro Carvalho Chehab [this message]
2020-05-25  6:56 ` [PATCH 10/11] media: atomisp: drop a cast for a const argument Mauro Carvalho Chehab
2020-05-25  6:56 ` [PATCH 11/11] media: atomisp: fix size of delay_frames array Mauro Carvalho Chehab
2020-05-26  7:35 ` [PATCH 00/11] Some fixes and cleanups for atomisp driver Sakari Ailus

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=20200526104111.52a6ef23@coco.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.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).