Patchwork i2c: sirf: move driver init from module_init to subsys_initcall

login
register
mail settings
Submitter Barry Song
Date May 16, 2013, 2:50 a.m.
Message ID <1368672607-5536-1-git-send-email-Baohua.Song@csr.com>
Download mbox | patch
Permalink /patch/244206/
State Rejected
Headers show

Comments

Barry Song - May 16, 2013, 2:50 a.m.
From: Xiaomeng Hou <Xiaomeng.Hou@csr.com>

if we initilize i2c bus by module_init, there are some devices which want
initialization earlier than i2c and could not do that in time, so move i2c
driver initilization to subsys_initcall and make i2c ready before devices
init.

Signed-off-by: Xiaomeng Hou <Xiaomeng.Hou@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/i2c/busses/i2c-sirf.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
Wolfram Sang - May 16, 2013, 9:38 a.m.
On Thu, May 16, 2013 at 10:50:07AM +0800, Barry Song wrote:
> From: Xiaomeng Hou <Xiaomeng.Hou@csr.com>
> 
> if we initilize i2c bus by module_init, there are some devices which want
> initialization earlier than i2c and could not do that in time, so move i2c
> driver initilization to subsys_initcall and make i2c ready before devices
> init.
> 
> Signed-off-by: Xiaomeng Hou <Xiaomeng.Hou@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

What about deferred probing?

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Barry Song - May 16, 2013, 10:25 a.m.
2013/5/16 Wolfram Sang <wsa@the-dreams.de>:
> On Thu, May 16, 2013 at 10:50:07AM +0800, Barry Song wrote:
>> From: Xiaomeng Hou <Xiaomeng.Hou@csr.com>
>>
>> if we initilize i2c bus by module_init, there are some devices which want
>> initialization earlier than i2c and could not do that in time, so move i2c
>> driver initilization to subsys_initcall and make i2c ready before devices
>> init.
>>
>> Signed-off-by: Xiaomeng Hou <Xiaomeng.Hou@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>
> What about deferred probing?

deferred probing could work but could not work for automative.
what we really want is that devices begin to work as early as
possible. we want i2c clients like lcd, camera begin to show images as
early as possible as an automative requirement.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown - May 25, 2013, 8:10 p.m.
On Thu, May 16, 2013 at 06:25:39PM +0800, Barry Song wrote:
> 2013/5/16 Wolfram Sang <wsa@the-dreams.de>:

> > What about deferred probing?

> deferred probing could work but could not work for automative.
> what we really want is that devices begin to work as early as
> possible. we want i2c clients like lcd, camera begin to show images as
> early as possible as an automative requirement.

Probe deferral should have at most a mimimal impact on the boot time -
it's pretty much just changing the order providing the probe functions
aren't excessively slow to defer.  If the probe functions are taking a
long time to defer that's probably what needs looking at.

If it really is probe deferral itself that's too slow then we ought to
be looking at improving that rather than boding around it.
Barry Song - May 27, 2013, 1:54 a.m.
2013/5/26 Mark Brown <broonie@kernel.org>:
> On Thu, May 16, 2013 at 06:25:39PM +0800, Barry Song wrote:
>> 2013/5/16 Wolfram Sang <wsa@the-dreams.de>:
>
>> > What about deferred probing?
>
>> deferred probing could work but could not work for automative.
>> what we really want is that devices begin to work as early as
>> possible. we want i2c clients like lcd, camera begin to show images as
>> early as possible as an automative requirement.
>
> Probe deferral should have at most a mimimal impact on the boot time -
> it's pretty much just changing the order providing the probe functions
> aren't excessively slow to defer.  If the probe functions are taking a
> long time to defer that's probably what needs looking at.
>
> If it really is probe deferral itself that's too slow then we ought to
> be looking at improving that rather than boding around it.

Mark, the case is not that deferred probing is slow or not. deferred
probing is pretty good.
the case is that we want to i2c and media connected with i2c probed
earlier than other devices.
in auto infotainment devices, we actually do some hacking in kernel
that makes rear view work earlier than other device driver
initialization with a kernel thread which take care of backing-car
policy not only mechanism. that means, we make camera work to see
backview image even earlier than other drivers' initialization.
we don't want media deferred to wait for i2c. we want make some early
jobs ready earlier.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown - May 27, 2013, 12:16 p.m.
On Mon, May 27, 2013 at 09:54:56AM +0800, Barry Song wrote:

> Mark, the case is not that deferred probing is slow or not. deferred
> probing is pretty good.
> the case is that we want to i2c and media connected with i2c probed
> earlier than other devices.
> in auto infotainment devices, we actually do some hacking in kernel
> that makes rear view work earlier than other device driver
> initialization with a kernel thread which take care of backing-car
> policy not only mechanism. that means, we make camera work to see
> backview image even earlier than other drivers' initialization.
> we don't want media deferred to wait for i2c. we want make some early
> jobs ready earlier.

So this change makes no practical difference in mainline and exists to
support out of tree hacks for performance?  It doesn't seem like that
big a patch to carry along with the out of tree stuff...
Barry Song - May 27, 2013, 3:36 p.m.
2013/5/27 Mark Brown <broonie@kernel.org>:
> On Mon, May 27, 2013 at 09:54:56AM +0800, Barry Song wrote:
>
>> Mark, the case is not that deferred probing is slow or not. deferred
>> probing is pretty good.
>> the case is that we want to i2c and media connected with i2c probed
>> earlier than other devices.
>> in auto infotainment devices, we actually do some hacking in kernel
>> that makes rear view work earlier than other device driver
>> initialization with a kernel thread which take care of backing-car
>> policy not only mechanism. that means, we make camera work to see
>> backview image even earlier than other drivers' initialization.
>> we don't want media deferred to wait for i2c. we want make some early
>> jobs ready earlier.
>
> So this change makes no practical difference in mainline and exists to
> support out of tree hacks for performance?  It doesn't seem like that
> big a patch to carry along with the out of tree stuff...

yes. but i don't think we are easy to make those out-of-mainline hacks
 be in mainline. but this patch is both ok to mainline and local tree.
making local tree and mainline same as many as possible decreases our
maintaince efforts totally.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - June 10, 2013, 1:45 p.m.
On Mon, May 27, 2013 at 11:36:14PM +0800, Barry Song wrote:
> 2013/5/27 Mark Brown <broonie@kernel.org>:
> > On Mon, May 27, 2013 at 09:54:56AM +0800, Barry Song wrote:
> >
> >> Mark, the case is not that deferred probing is slow or not. deferred
> >> probing is pretty good.
> >> the case is that we want to i2c and media connected with i2c probed
> >> earlier than other devices.
> >> in auto infotainment devices, we actually do some hacking in kernel
> >> that makes rear view work earlier than other device driver
> >> initialization with a kernel thread which take care of backing-car
> >> policy not only mechanism. that means, we make camera work to see
> >> backview image even earlier than other drivers' initialization.
> >> we don't want media deferred to wait for i2c. we want make some early
> >> jobs ready earlier.
> >
> > So this change makes no practical difference in mainline and exists to
> > support out of tree hacks for performance?  It doesn't seem like that
> > big a patch to carry along with the out of tree stuff...
> 
> yes. but i don't think we are easy to make those out-of-mainline hacks
>  be in mainline. but this patch is both ok to mainline and local tree.
> making local tree and mainline same as many as possible decreases our
> maintaince efforts totally.

I understand that, yet I agree with Mark. The mainline idea is to use
deferred probing to make sure all components come up correctly. In fact,
I wish for patches removing subsys_initcall with deferred probing.
Barry Song - June 11, 2013, 1:14 a.m.
2013/6/10 Wolfram Sang <wsa@the-dreams.de>:
> On Mon, May 27, 2013 at 11:36:14PM +0800, Barry Song wrote:
>> 2013/5/27 Mark Brown <broonie@kernel.org>:
>> > On Mon, May 27, 2013 at 09:54:56AM +0800, Barry Song wrote:
>> >
>> >> Mark, the case is not that deferred probing is slow or not. deferred
>> >> probing is pretty good.
>> >> the case is that we want to i2c and media connected with i2c probed
>> >> earlier than other devices.
>> >> in auto infotainment devices, we actually do some hacking in kernel
>> >> that makes rear view work earlier than other device driver
>> >> initialization with a kernel thread which take care of backing-car
>> >> policy not only mechanism. that means, we make camera work to see
>> >> backview image even earlier than other drivers' initialization.
>> >> we don't want media deferred to wait for i2c. we want make some early
>> >> jobs ready earlier.
>> >
>> > So this change makes no practical difference in mainline and exists to
>> > support out of tree hacks for performance?  It doesn't seem like that
>> > big a patch to carry along with the out of tree stuff...
>>
>> yes. but i don't think we are easy to make those out-of-mainline hacks
>>  be in mainline. but this patch is both ok to mainline and local tree.
>> making local tree and mainline same as many as possible decreases our
>> maintaince efforts totally.
>
> I understand that, yet I agree with Mark. The mainline idea is to use
> deferred probing to make sure all components come up correctly. In fact,
> I wish for patches removing subsys_initcall with deferred probing.

hi Wolfram,

the mainline idea you mentioned is that you don't care about any early
device, which means some devices want to start earlier than others in
some real automative user scenerioes.
but i think another important idea is that mainline codes come from
branches of different vendors and serve final users of those drivers,
but not only make source codes no difference to all environments. the
auto users i2c-sirf serves, here, actually means some differences with
PC/tablet/mobilephone. we don't make codes good-looking by losing
functionality and not close to final users.

anyway, it is up to you to decide. i can definitely maintain the
difference locally.

>

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown - June 11, 2013, 8:48 a.m.
On Tue, Jun 11, 2013 at 09:14:41AM +0800, Barry Song wrote:

> the mainline idea you mentioned is that you don't care about any early
> device, which means some devices want to start earlier than others in
> some real automative user scenerioes.
> but i think another important idea is that mainline codes come from
> branches of different vendors and serve final users of those drivers,
> but not only make source codes no difference to all environments. the
> auto users i2c-sirf serves, here, actually means some differences with
> PC/tablet/mobilephone. we don't make codes good-looking by losing
> functionality and not close to final users.

It's not that people don't care about these users, it's that people
don't care too much about out of tree users.  Part of the goal here is
to convince people working on such applications that they should make
mainline better for everyone so that you don't need external code to
make the kernel useful.
Barry Song - June 11, 2013, 11:13 a.m.
2013/6/11 Mark Brown <broonie@kernel.org>:
> On Tue, Jun 11, 2013 at 09:14:41AM +0800, Barry Song wrote:
>
>> the mainline idea you mentioned is that you don't care about any early
>> device, which means some devices want to start earlier than others in
>> some real automative user scenerioes.
>> but i think another important idea is that mainline codes come from
>> branches of different vendors and serve final users of those drivers,
>> but not only make source codes no difference to all environments. the
>> auto users i2c-sirf serves, here, actually means some differences with
>> PC/tablet/mobilephone. we don't make codes good-looking by losing
>> functionality and not close to final users.
>
> It's not that people don't care about these users, it's that people
> don't care too much about out of tree users.  Part of the goal here is
> to convince people working on such applications that they should make
> mainline better for everyone so that you don't need external code to
> make the kernel useful.

Mark, then this is really confusing me.
for this reason, the target should be making this out-of-tree stuff be
inside the tree. to make i2c-sirf "mainline better and not need
external code", the target should be making this external code not
external but become internal. otherwise, my mainline users will always
need this external code.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown - June 11, 2013, 12:10 p.m.
On Tue, Jun 11, 2013 at 07:13:33PM +0800, Barry Song wrote:
> 2013/6/11 Mark Brown <broonie@kernel.org>:

> > It's not that people don't care about these users, it's that people
> > don't care too much about out of tree users.  Part of the goal here is
> > to convince people working on such applications that they should make
> > mainline better for everyone so that you don't need external code to
> > make the kernel useful.

> Mark, then this is really confusing me.
> for this reason, the target should be making this out-of-tree stuff be
> inside the tree. to make i2c-sirf "mainline better and not need
> external code", the target should be making this external code not
> external but become internal. otherwise, my mainline users will always
> need this external code.

It's not just this one bit of code that they're relying on, this also
gets built on with other things that are also out of tree.  The thing
we need to do is figure out what problem the solution as a whole is
fixing and then make sure that mainline can deal with that.

Patch

diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
index 5a7ad24..e438f48 100644
--- a/drivers/i2c/busses/i2c-sirf.c
+++ b/drivers/i2c/busses/i2c-sirf.c
@@ -454,7 +454,19 @@  static struct platform_driver i2c_sirfsoc_driver = {
 	.probe = i2c_sirfsoc_probe,
 	.remove = i2c_sirfsoc_remove,
 };
-module_platform_driver(i2c_sirfsoc_driver);
+
+static __init int i2c_sirfsoc_init(void)
+{
+	return platform_driver_register(&i2c_sirfsoc_driver);
+}
+
+static void __exit i2c_sirfsoc_exit(void)
+{
+	platform_driver_unregister(&i2c_sirfsoc_driver);
+}
+
+subsys_initcall(i2c_sirfsoc_init);
+module_exit(i2c_sirfsoc_exit);
 
 MODULE_DESCRIPTION("SiRF SoC I2C master controller driver");
 MODULE_AUTHOR("Zhiwu Song <Zhiwu.Song@csr.com>, "