public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Mark Brown <broonie@kernel.org>
Cc: "Samuel Ortiz" <sameo@linux.intel.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Benoît Cousson" <b-cousson@ti.com>,
	"Tony Lindgren" <tony@atomide.com>,
	"Jonathan Cameron" <jic23@cam.ac.uk>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Felipe Balbi" <balbi@ti.com>,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-iio@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap
Date: Mon, 17 Jun 2013 13:41:40 +0200	[thread overview]
Message-ID: <51BEF5F4.4090600@linutronix.de> (raw)
In-Reply-To: <20130614135358.GR1403@sirena.org.uk>

On 06/14/2013 03:53 PM, Mark Brown wrote:
> On Tue, Jun 11, 2013 at 04:34:53PM +0200, Sebastian Andrzej Siewior
> wrote:
> 
> It does give you tracepoints and debugfs.  If it's making things at
> all complicated we need to look at why that is and figure out how
> to fix that since it's probably an issue for other users.

debugfs are tracepoints is our offer? Let me check the price one more
time.

A simply mmio read does right now:
- lock + unlock.
  each time you chase another pointer plus enable/disable interrupts
  plus you have to save flags in another structure.

- _regmap_read()
  We check a few variables and then we go after reg_read and we end up
  in _regmap_raw_read().
  Here we call _regmap_range_lookup() which should return NULL. Next
  thing we invoke map->format.format_reg(). Finally we can call
  map->bus->read() which brings us to regmap_mmio_read().
  At the end we invoke map->format.parse_val().

write looks most likely the same.

A simple register read invokes 5 functions pointers. I am not counting
the function arguments in between and I am also not counting the number
of arguments which are involved and take pointer as well. This is a lot
of stuff that is done for a simple read of an mmio.

I understand that most of this may not be expensive in total if it
comes to SPI or I2C and all the goodies like reg caching and one
interface which deals with SPI and I2C. I also understand that some
chips have a non standard interface and are either BE or LE. We have
similar things on USB where people wired the BUS wrongly either at the
BUS level or at the SoC level so some PowerPC have an in-core ehci
controller but its registers are BE and not LE like it should be. The
solution here was variable check a simple swap() in that case.
I like abstractions but this gone a little too far I think.

This is a lot of for a simple mmio access. In terms of performance: If
I add a trace point to my read and write I have still less code which
is called and it can be disabled. This regmap overhead is always there
chasing pointers.

As I said before: I doubt that you get this regmap access in an one GiB
network driver and in turn remove their trace points to register access.

Please don't get me wrong: It is still nice for slow buses and this ADC
driver isn't anything close to high performance like a 1GiB network
driver but it adds a lot of unwanted overhead which I prefer not to
have.

Sebastian

  reply	other threads:[~2013-06-17 11:41 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 11:30 am335x: TSC & ADC reworking including DT pieces, take 4 Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap Sebastian Andrzej Siewior
2013-06-11 14:23   ` Samuel Ortiz
2013-06-11 14:34     ` Sebastian Andrzej Siewior
2013-06-14 13:53       ` Mark Brown
2013-06-17 11:41         ` Sebastian Andrzej Siewior [this message]
2013-06-17 16:03           ` Mark Brown
2013-07-04  9:02             ` Sebastian Andrzej Siewior
2013-07-04 10:45               ` Mark Brown
2013-07-04 11:15                 ` Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 02/22] mfd & input & iio/ti_am335x_adc: use one structure for ti_tscadc_dev Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 03/22] input/ti_am33x_tsc: Step enable bits made configurable Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, " Sebastian Andrzej Siewior
2013-06-11 14:23   ` Samuel Ortiz
2013-06-11 14:35     ` Sebastian Andrzej Siewior
2013-07-04 11:14   ` Sekhar Nori
2013-07-04 11:33     ` Sebastian Andrzej Siewior
2013-07-04 13:39       ` Sekhar Nori
2013-07-04 13:50         ` Sebastian Andrzej Siewior
2013-07-04 14:27           ` Sekhar Nori
2013-06-11 11:30 ` [PATCH 05/22] input/ti_am33x_tsc: remove unwanted fifo flush Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 06/22] input/ti_am33x_tsc: Add DT support Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 07/22] input/ti_am33x_tsc: remove platform_data support Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 08/22] iio/ti_am335x_adc: Add DT support Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 09/22] iio/ti_am335x_adc: remove platform_data support Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support Sebastian Andrzej Siewior
2013-06-11 14:23   ` Samuel Ortiz
2013-06-11 14:42     ` Sebastian Andrzej Siewior
2013-06-11 15:05       ` Samuel Ortiz
2013-06-11 15:41         ` Sebastian Andrzej Siewior
2013-06-11 18:42           ` Lee Jones
2013-06-11 17:10       ` Lee Jones
2013-06-11 11:30 ` [PATCH 11/22] mfd/ti_am335x_tscadc: remove platform_data support Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 12/22] iio/ti_tscadc: provide datasheet_name and scan_type Sebastian Andrzej Siewior
2013-06-11 11:30 ` [PATCH 13/22] mfd/ti_tscadc: deal with partial activation Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support Sebastian Andrzej Siewior
2013-07-04 13:49   ` Sekhar Nori
2013-07-04 13:51     ` Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 15/22] input & mfd: ti_am335x_tsc remove remaining platform data pieces Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 16/22] mfd & input/ti_am335x_tsc: rename device from tsc to TI-am335x-tsc Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 17/22] mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 18/22] input/ti_am335x_adc: use only FIFO0 and clean up a little Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 19/22] input/ti_am335x_tsc: ACK the HW_PEN irq in ISR Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 20/22] input/ti_am335x_tsc: return IRQ_NONE if there was no IRQ for us Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 21/22] iio/ti_am335x_adc: Allow to specify input line Sebastian Andrzej Siewior
2013-06-11 11:31 ` [PATCH 22/22] iio/ti_am335x_adc: check if we found the value Sebastian Andrzej Siewior
2013-06-11 12:05 ` am335x: TSC & ADC reworking including DT pieces, take 4 Lee Jones
2013-06-11 13:53   ` Lars-Peter Clausen
2013-06-11 14:23 ` Samuel Ortiz
2013-06-11 15:29   ` Sebastian Andrzej Siewior
2013-06-11 16:10     ` Samuel Ortiz
2013-06-11 16:18       ` Sebastian Andrzej Siewior
2013-06-14 13:57     ` Mark Brown
2013-06-11 16:04   ` Dmitry Torokhov
2013-06-11 16:15     ` Samuel Ortiz
2013-06-11 16:27       ` Jonathan Cameron
2013-06-11 17:01         ` Lars-Peter Clausen
2013-06-11 17:55         ` Samuel Ortiz
2013-06-12 13:29           ` Sebastian Andrzej Siewior
2013-06-12 13:50             ` Samuel Ortiz
2013-06-12 14:02               ` Sebastian Andrzej Siewior
2013-06-12 14:41                 ` Samuel Ortiz
2013-06-12 15:00                   ` Sebastian Andrzej Siewior
2013-06-11 18:02   ` Jonathan Cameron

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=51BEF5F4.4090600@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=b-cousson@ti.com \
    --cc=balbi@ti.com \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jic23@cam.ac.uk \
    --cc=lee.jones@linaro.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=sameo@linux.intel.com \
    --cc=tony@atomide.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