All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eugeniu Rosca <erosca@de.adit-jv.com>
To: unlisted-recipients:; (no To-header on input)
Cc: <linux-i2c@vger.kernel.org>, <linux-renesas-soc@vger.kernel.org>,
	Eugeniu Rosca <erosca@de.adit-jv.com>
Subject: Re: [PATCH] i2c: rcar: initialize earlier using subsys_initcall()
Date: Wed, 4 Apr 2018 23:20:18 +0200	[thread overview]
Message-ID: <20180404212018.GA29865@vmlxhi-102.adit-jv.com> (raw)
In-Reply-To: <20180403170829.iaieac5wk4hkleme@ninjato>

Hello Wolfram, Vladimir,

Thanks for your precious inputs.

I think you outlined two topics in your comments (based on the
description submitted with the patch). One (primary?) is related to
async probing and one (secondary, but still interesting) is related to
the minor (~7ms -> ~1ms) startup improvement of the rcar-i2c driver.

I did a little bit of research to understand what's behind the startup
improvement (hoping it is simpler to explain) and I think I reached some
conclusions. I used vanilla v4.16 kernel/defconfig/dtb and the
h3-es20-salvator-X target. The only configuration change was to enable
CONFIG_DEBUG_DRIVER and to increase CONFIG_LOG_BUF_SHIFT from 17 to 22
to avoid overwriting the printk buffer. Kernel was boot-ed "quiet"-ly.

With this HW/SW setup, I am able to confirm (based on 5 dmesg logs with
and w/o the submitted patch) that:

[1] W/o the submitted patch, rcar_i2c_driver_init takes around 7092 us
[2] With the submitted patch, rcar_i2c_driver_init takes around 1241 us

Comparing the <CONTENTS> marked below in the two cases:
   calling  rcar_i2c_driver_init
   <CONTENTS>
   initcall rcar_i2c_driver_init+0x0/0x20 returned 0 after X us

I notice that w/o the proposed patch, <CONTENTS> is consistently more
rich, containing output related to additional three drivers:
* cs2000-cp
* rcar-dmac
* ohci-platform

These additional lines cumulatively consume around 5-6 ms, which
is exactly the difference between [1] and [2] seen above.

To explain why these lines are present in [1], but not in [2], I checked
the order of initialization of the mentioned drivers. Here is what I see:

[11] W/o the submitted patch (in the order of execution):
Initcall                Time (us)
cs2000_driver_init         30
rcar_dmac_driver_init    8107
ohci_platform_init     178199
rcar_i2c_driver_init     7092
                 SUM   193428

[22] With the submitted patch (in the order of execution):
Initcall                Time (us)
rcar_i2c_driver_init     1241
cs2000_driver_init       5667
rcar_dmac_driver_init    8160
ohci_platform_init     178110
                SUM    193178

So, the idea is that the startup improvement of rcar_i2c_driver_init()
comes at the cost of a slower cs2000_driver_init(). In the end,
there is no benefit and this doesn't work as an argument why the patch
should go upstream.

With regard to the primary argument, that the proposed patch
reduces/minimizes the boot time variance when switching certain drivers
like rcar-du from synchronous to asynchronous probing, I can clearly
sense this in my measurements, but I'm afraid I won't be able to provide
a simple and comprehensive explanation why this happens, since we
enter the territory of concurrent/parallel driver initialization.

If evidence in the form of measurements showing improved behavior
is not enough for the patch to be accepted, then we have no choice but
to keep the patch out-of-tree as a "workaround", hoping that we'll be able
to come up with a better/cleaner solution in future.

If you ever stumble upon a mailing list discussing how to "stabilize"
the boot time as a consequence of using asynchronous probing, then
please drop a link and this will be appreciated.

Many thanks,
Eugeniu.

  reply	other threads:[~2018-04-04 21:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-16 10:58 [PATCH] i2c: rcar: initialize earlier using subsys_initcall() Vladimir Zapolskiy
2018-03-28 14:40 ` Eugeniu Rosca
2018-04-03 15:55 ` Wolfram Sang
2018-04-03 16:40   ` Vladimir Zapolskiy
2018-04-03 17:08     ` Wolfram Sang
2018-04-04 21:20       ` Eugeniu Rosca [this message]
2018-04-04 21:27       ` Eugeniu Rosca
2018-04-08  7:47         ` Wolfram Sang

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=20180404212018.GA29865@vmlxhi-102.adit-jv.com \
    --to=erosca@de.adit-jv.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    /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.