Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

8 messages, 4 authors, 2011-06-22 · open the first message on its own page

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Mark Brown <hidden>
Date: 2011-06-21 17:27:48

On Tue, Jun 21, 2011 at 06:34:48AM -0700, Dmitry Torokhov wrote:
On Jun 21, 2011 3:46 PM, "Mark Brown" [off-list ref]
quoted
On Mon, Jun 20, 2011 at 01:45:12AM -0700, Dmitry Torokhov wrote:
quoted
Like Grant says this really isn't terribly sustainable - it's not just
the device registration you need to sort out, it's also the registration
of the drivers so things actually get bound and handing of any delays in
the process of getting things to appear.
If devices are registered only when they are fully usable then driver
registration does not matter.
Right, but this is something that it's not reasonable to implement in
board code - if nothing else implementing it in board code would mean
we'd got lots of repitition of common patterns.
quoted
It's not trivial to get this
right in the general case and it's not reasonable to expect individual
boards to open code things,
Board code has the ultimate knowledge about connected devices though.
Absolutely, board code or data should provide the information about how
things are wired up.  It's the acting on it bit that's the issue.
quoted
quoted
How about we do not register device until all resources are ready? This
is pretty simple concept - do not create an object until it is usable.
Then
quoted
quoted
nobody needs to bother with -EAGAIN or -ENOTYET or any other similar
garbage.
quoted
As soon as you let the user build drivers modular this goes out of the
window.
Why is that? If device is registered only when it is ready to be bound to
then it does not matter when the driver is registered and whether it is
built into the kernel or as a module.
Originally you were talking about registration ordering - solving the
module load issues also requires dynamic delays and rollbacks when
things get unregisterd, something that goes well beyond simple ordering
of the registrations. 
quoted
All the faff with initcall ordering that we do at the minute is
essentially trying to implement this mechanism.
No, what you are doing is creating devices before they are usable and
postponing the driver registration in hopes that devices will be ready by
that time.
Right, which is controlling the ordering of registration so that things
generally work out OK as described above.

Nobody's arguing that we don't want to solve this in a better way, we're
just saying that actually doing that requires improvements in both core
infrastructure and the data we've got available to the infrastructure so
there's no reasonable solutions that we can deploy which are better than
the initcall ordering stuff we're doing at the minute.

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-06-21 20:54:08

On Tue, Jun 21, 2011 at 06:27:45PM +0100, Mark Brown wrote:
On Tue, Jun 21, 2011 at 06:34:48AM -0700, Dmitry Torokhov wrote:
quoted
On Jun 21, 2011 3:46 PM, "Mark Brown" [off-list ref]
quoted
On Mon, Jun 20, 2011 at 01:45:12AM -0700, Dmitry Torokhov wrote:
quoted
quoted
Like Grant says this really isn't terribly sustainable - it's not just
the device registration you need to sort out, it's also the registration
of the drivers so things actually get bound and handing of any delays in
the process of getting things to appear.
quoted
If devices are registered only when they are fully usable then driver
registration does not matter.
Right, but this is something that it's not reasonable to implement in
board code - if nothing else implementing it in board code would mean
we'd got lots of repitition of common patterns.
I agree here. I just disagree that we should be implementing this in
driver core by having special -EAGAIN handling. Having a common
library-like code (probably tied to device-tree) that handles device
dependencies would be great.
quoted
quoted
It's not trivial to get this
right in the general case and it's not reasonable to expect individual
boards to open code things,
quoted
Board code has the ultimate knowledge about connected devices though.
Absolutely, board code or data should provide the information about how
things are wired up.  It's the acting on it bit that's the issue.
quoted
quoted
quoted
How about we do not register device until all resources are ready? This
is pretty simple concept - do not create an object until it is usable.
Then
quoted
quoted
nobody needs to bother with -EAGAIN or -ENOTYET or any other similar
garbage.
quoted
quoted
As soon as you let the user build drivers modular this goes out of the
window.
quoted
Why is that? If device is registered only when it is ready to be bound to
then it does not matter when the driver is registered and whether it is
built into the kernel or as a module.
Originally you were talking about registration ordering - solving the
module load issues also requires dynamic delays and rollbacks when
things get unregisterd, something that goes well beyond simple ordering
of the registrations. 
I always was only saying that devices should be registered when they are
ready. It is my understanding that normally board code tries to register
all devices; drivers may or may not be compiled as modules. Not that we
could not have devices created by modules...
quoted
quoted
All the faff with initcall ordering that we do at the minute is
essentially trying to implement this mechanism.
quoted
No, what you are doing is creating devices before they are usable and
postponing the driver registration in hopes that devices will be ready by
that time.
Right, which is controlling the ordering of registration so that things
generally work out OK as described above.

Nobody's arguing that we don't want to solve this in a better way, we're
just saying that actually doing that requires improvements in both core
infrastructure and the data we've got available to the infrastructure so
there's no reasonable solutions that we can deploy which are better than
the initcall ordering stuff we're doing at the minute.
Ah, OK, so we basically in agreement here with the exception that I do
not want the band-aid to hit mainline since it takes the heat off people
who need inter-device dependency to actually work.

Can the initcall stuff be kept out of mainline? I'd expect
there exist board-specific trees where such patches could be kept? Or
maybe interested parties could create board-crap tree to store patches
like this one?

Thanks.

-- 
Dmitry

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Mark Brown <hidden>
Date: 2011-06-21 23:02:45

On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
On Tue, Jun 21, 2011 at 06:27:45PM +0100, Mark Brown wrote:
quoted
Right, but this is something that it's not reasonable to implement in
board code - if nothing else implementing it in board code would mean
we'd got lots of repitition of common patterns.
I agree here. I just disagree that we should be implementing this in
driver core by having special -EAGAIN handling. Having a common
library-like code (probably tied to device-tree) that handles device
dependencies would be great.
Ah, that's more OK then.  I'm not entirely sure about the -EAGAIN
proposal but it does seem to have some advantages in terms of
deployment.
Ah, OK, so we basically in agreement here with the exception that I do
not want the band-aid to hit mainline since it takes the heat off people
who need inter-device dependency to actually work.
Can the initcall stuff be kept out of mainline? I'd expect
The init order stuff is in mainline already, you're far too late to the
party here.
there exist board-specific trees where such patches could be kept? Or
maybe interested parties could create board-crap tree to store patches
like this one?
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: David Jander <hidden>
Date: 2011-06-22 06:11:36

On Wed, 22 Jun 2011 00:02:42 +0100
Mark Brown [off-list ref] wrote:
On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
quoted
On Tue, Jun 21, 2011 at 06:27:45PM +0100, Mark Brown wrote:
quoted
quoted
Right, but this is something that it's not reasonable to implement in
board code - if nothing else implementing it in board code would mean
we'd got lots of repitition of common patterns.
quoted
I agree here. I just disagree that we should be implementing this in
driver core by having special -EAGAIN handling. Having a common
library-like code (probably tied to device-tree) that handles device
dependencies would be great.
Ah, that's more OK then.  I'm not entirely sure about the -EAGAIN
proposal but it does seem to have some advantages in terms of
deployment.
quoted
Ah, OK, so we basically in agreement here with the exception that I do
not want the band-aid to hit mainline since it takes the heat off people
who need inter-device dependency to actually work.
quoted
Can the initcall stuff be kept out of mainline? I'd expect
The init order stuff is in mainline already, you're far too late to the
party here.
quoted
there exist board-specific trees where such patches could be kept? Or
maybe interested parties could create board-crap tree to store patches
like this one?
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.
Looks like we all agree then?
Dmitry, would you consider the late_initcall() part of the hack now
(temporarily)?

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-06-22 07:01:02

On Wed, Jun 22, 2011 at 12:02:42AM +0100, Mark Brown wrote:
On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
quoted
On Tue, Jun 21, 2011 at 06:27:45PM +0100, Mark Brown wrote:
quoted
quoted
Right, but this is something that it's not reasonable to implement in
board code - if nothing else implementing it in board code would mean
we'd got lots of repitition of common patterns.
quoted
I agree here. I just disagree that we should be implementing this in
driver core by having special -EAGAIN handling. Having a common
library-like code (probably tied to device-tree) that handles device
dependencies would be great.
Ah, that's more OK then.  I'm not entirely sure about the -EAGAIN
proposal but it does seem to have some advantages in terms of
deployment.
quoted
Ah, OK, so we basically in agreement here with the exception that I do
not want the band-aid to hit mainline since it takes the heat off people
who need inter-device dependency to actually work.
quoted
Can the initcall stuff be kept out of mainline? I'd expect
The init order stuff is in mainline already, you're far too late to the
party here.
For some drivers it might be already in mainline, it does not matter
that we should continue adding more.
quoted
there exist board-specific trees where such patches could be kept? Or
maybe interested parties could create board-crap tree to store patches
like this one?
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.
So you are saying that we should accept such crap directly into
mainline?

Again, it looks like we agree that shuffling initcalls is not proper
solution for this problem nor it is maintainable, so it is exactly the
kind of patches that should be kept in the board trees and out of
mainline.

-- 
Dmitry

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Mark Brown <hidden>
Date: 2011-06-22 11:38:38

On Wed, Jun 22, 2011 at 12:00:52AM -0700, Dmitry Torokhov wrote:
On Wed, Jun 22, 2011 at 12:02:42AM +0100, Mark Brown wrote:
quoted
On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
quoted
quoted
Can the initcall stuff be kept out of mainline? I'd expect
quoted
The init order stuff is in mainline already, you're far too late to the
party here.
For some drivers it might be already in mainline, it does not matter
that we should continue adding more.
It's not just a few drivers, there's entire subsystems that are doing
this.
quoted
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.
So you are saying that we should accept such crap directly into
mainline?
Pretty much, yes.  In code terms it's not really invasive and it doesn't
have any real impact on other systems so it's the sort of thing we can
carry without too much pain.  Pragmatically it's not unreasonable.
Again, it looks like we agree that shuffling initcalls is not proper
solution for this problem nor it is maintainable, so it is exactly the
kind of patches that should be kept in the board trees and out of
mainline.
On the other hand if we're telling people that they can't run their
system usefully from mainline (in some cases we can't even boot) then
we're sending a bad message about the usefulness of mainline and we're
encouraging a space where non-mainline code is acceptable.

The situation here is similar to what we used to have with interrupt
controllers on slow buses - we spent a while working with open coded
non-genirq implementations confined to particular drivers before genirq
was able to support this sort of hardware because there wasn't a clear
route to getting that done in a reasonable timeframe.

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Grant Likely <hidden>
Date: 2011-06-22 14:59:06

On Wed, Jun 22, 2011 at 5:38 AM, Mark Brown
[off-list ref] wrote:
On Wed, Jun 22, 2011 at 12:00:52AM -0700, Dmitry Torokhov wrote:
quoted
On Wed, Jun 22, 2011 at 12:02:42AM +0100, Mark Brown wrote:
quoted
On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
quoted
quoted
quoted
Can the initcall stuff be kept out of mainline? I'd expect
quoted
quoted
The init order stuff is in mainline already, you're far too late to the
party here.
quoted
For some drivers it might be already in mainline, it does not matter
that we should continue adding more.
It's not just a few drivers, there's entire subsystems that are doing
this.
quoted
quoted
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.
quoted
So you are saying that we should accept such crap directly into
mainline?
Pretty much, yes.  In code terms it's not really invasive and it doesn't
have any real impact on other systems so it's the sort of thing we can
carry without too much pain.  Pragmatically it's not unreasonable.
+1

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-06-22 21:43:16

On Wed, Jun 22, 2011 at 08:58:45AM -0600, Grant Likely wrote:
On Wed, Jun 22, 2011 at 5:38 AM, Mark Brown
[off-list ref] wrote:
quoted
On Wed, Jun 22, 2011 at 12:00:52AM -0700, Dmitry Torokhov wrote:
quoted
On Wed, Jun 22, 2011 at 12:02:42AM +0100, Mark Brown wrote:
quoted
On Tue, Jun 21, 2011 at 01:48:05PM -0700, Dmitry Torokhov wrote:
quoted
quoted
quoted
Can the initcall stuff be kept out of mainline? I'd expect
quoted
quoted
The init order stuff is in mainline already, you're far too late to the
party here.
quoted
For some drivers it might be already in mainline, it does not matter
that we should continue adding more.
It's not just a few drivers, there's entire subsystems that are doing
this.
quoted
quoted
Keeping things in board trees is exactly the sort of thing we want to
avoid people doing.  That just means people do all sorts of stuff that
wouldn't be acceptable upstream, either out of ignorance or through
knowing that only their systems have to work with what they're doing,
and just don't bother working upstream at all half the time making life
miserable for pretty much everyone.
quoted
So you are saying that we should accept such crap directly into
mainline?
Pretty much, yes.  In code terms it's not really invasive and it doesn't
have any real impact on other systems so it's the sort of thing we can
carry without too much pain.  Pragmatically it's not unreasonable.
+1
OK, you wore me out. I'll apply the initcall change...

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help