Hello linux-kernel,
We here at Handhelds.org upgrading our drivers to 2.6.18 and I just
caught a case of find_bus() being undefined during link. Quickly
traced this to
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7e4ef085ea4b00cfc34e854edf448c729de8a0a5
But alas, the commit message is not as good as some others are, and
doesn't mention what should be used instead. So, if find_bus() is
"unused", what should be used instead?
Thank you,
--
Paul mailto:[email protected]
Paul Sokolovsky wrote:
> But alas, the commit message is not as good as some others are, and
> doesn't mention what should be used instead. So, if find_bus() is
> "unused", what should be used instead?
You should probably mention what for?
regards,
--
http://www.fi.muni.cz/~xslaby/ Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8 22A0 32CC 55C3 39D4 7A7E
Hello Jiri,
Monday, November 20, 2006, 1:45:51 AM, you wrote:
> Paul Sokolovsky wrote:
>> But alas, the commit message is not as good as some others are, and
>> doesn't mention what should be used instead. So, if find_bus() is
>> "unused", what should be used instead?
> You should probably mention what for?
Indeed, I'm sorry! Looking at find_bus()'s docstring:
/**
* find_bus - locate bus by name.
* @name: name of bus.
So well, I'd like to know exactly that - what function should be
used instead of find_bus() to locate bus by name.
> regards,
--
Best regards,
Paul mailto:[email protected]
On Mon, Nov 20, 2006 at 12:45:51AM +0100, Jiri Slaby wrote:
> Paul Sokolovsky wrote:
> > But alas, the commit message is not as good as some others are, and
> > doesn't mention what should be used instead. So, if find_bus() is
> > "unused", what should be used instead?
>
> You should probably mention what for?
Exactly. It was removed because no one was using it, and I couldn't
think of a reason why it would be needed.
Also, any reason why your drivers aren't in the mainline kernel yet? It
would have kept something like this from happening a while ago. And, it
will also help out with the recent driver core changes that are being
planned and are starting to show up in the -mm tree. If your stuff is
in the kernel, then I'll do the work for you, otherwise, you all are on
your own :(
thanks,
greg k-h
On Mon, 2006-11-20 at 02:13 +0200, Paul Sokolovsky wrote:
> Hello Jiri,
>
> Monday, November 20, 2006, 1:45:51 AM, you wrote:
>
> > Paul Sokolovsky wrote:
> >> But alas, the commit message is not as good as some others are, and
> >> doesn't mention what should be used instead. So, if find_bus() is
> >> "unused", what should be used instead?
>
> > You should probably mention what for?
>
> Indeed, I'm sorry! Looking at find_bus()'s docstring:
>
> /**
> * find_bus - locate bus by name.
> * @name: name of bus.
>
> So well, I'd like to know exactly that - what function should be
> used instead of find_bus() to locate bus by name.
I think the question more was "what do you need to look up a bus by name
for in the kernel"? Like.. what are you trying to achieve? What module
is this for? (does it have a homepage where people can download the
source etc so that they can give you a more informed answer)....
>
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
Hi Paul,
On 11/19/06, Paul Sokolovsky <[email protected]> wrote:
> Hello linux-kernel,
>
> We here at Handhelds.org upgrading our drivers to 2.6.18 and I just
> caught a case of find_bus() being undefined during link.
I assume you are referring to the ipaq h2200 battery driver that uses
bus = find_bus("w1");
to "find" the one wire bus?
I think the solution would be to use w1_bus_type instead, but for that
it would have to be exported.
Index: linux-2.6/drivers/w1/w1.c
===================================================================
--- linux-2.6.orig/drivers/w1/w1.c 2006-10-28 22:58:11.000000000 +0200
+++ linux-2.6/drivers/w1/w1.c 2006-10-29 02:01:30.000000000 +0200
@@ -194,7 +194,7 @@
static int w1_uevent(struct device *dev, char **envp, int num_envp, char *buffe
r, int buffer_size);
-static struct bus_type w1_bus_type = {
+struct bus_type w1_bus_type = {
.name = "w1",
.match = w1_master_match,
.uevent = w1_uevent,
@@ -978,3 +978,5 @@
module_init(w1_init);
module_exit(w1_fini);
+
+EXPORT_SYMBOL_GPL(w1_bus_type);
regards
Philipp
Hello Greg,
Monday, November 20, 2006, 2:12:12 AM, you wrote:
> On Mon, Nov 20, 2006 at 12:45:51AM +0100, Jiri Slaby wrote:
>> Paul Sokolovsky wrote:
>> > But alas, the commit message is not as good as some others are, and
>> > doesn't mention what should be used instead. So, if find_bus() is
>> > "unused", what should be used instead?
>>
>> You should probably mention what for?
> Exactly. It was removed because no one was using it, and I couldn't
> think of a reason why it would be needed.
So, I assume the answer to my question is "No replacement. Ability
to query bus set in the kernel was just removed, period." That's to
which conclusion I came myself after studying 2.6.18 source and patch
from 2.6.17.
[Uninteresting specific case]
Ok, so the situation is following: we have a kind of multi-layered
driver here. Lowest level is a w1_slave bus driver, talking to a
specific chip and providing low-level API for accessing data in terms
of this chip (or chip class) notions. Above it, we have higher-level
driver which interprets data from the low-level one, converting it to
a standard device-independent form, plus possibly does some other
minor things, like providing feedback indication on these data.
(Forgot to say that this is battery driver.)
So, just in case if some reader of this has quick suggestion of
merging these drivers into one, thanks, but they do different things,
and we want to keep them nicely decoupled. But now issue of how these
drivers talk between themselves raises, and that's exactly the grief
point.
High-level driver used to find w1 bus by name, then enumerate
devices on the bus, to find compatible device on it, then hooks into
that device and its driver.
So, you see the issue: we cannot enumerate devices on w1 bus. (And
yes, w1_bus_type is not exported).
Sure, the source is up:
http://handhelds.org/cgi-bin/cvsweb.cgi/linux/kernel26/drivers/misc/h2200_battery.c?rev=1.20&content-type=text/x-cvsweb-markup
[Trends thoughts]
But note that I don't really ask mainline kernel developers how to
fix this driver - I would actually be ashamed to do so, as I myself a
(newbie) kernel hacker. So, the question stays the same, though I
probably reformulate it a bit stronger now: how it came that ability
to query buses (at all) was removed from 2.6.18?
As it was before, it was clear that LDM consists of multiple layers,
and each layer offers consistent and complete set of operations on it,
like adding new object on this layer, removing, adding child, removing
child, *and* query objects on this level or among childs. I may miss
some accidental gaps in that picture of course, but it still was an
integral, complete design paradigm offering full dynamicity and
introspection.
And suddenly - oops, in 2.6.18 we lose ability to query the highest
level of hierarchy, namely bus set. And on what criterion? "unused". I
would really dream that such core, the most basic APIs are not being
defined in terms of "someone does use it right now". A method to query
objects of core kernel data sets is just integral part of interface to
these datasets, you cannot remove it and not cripple such interface.
Again, it's loss of introspection, and that's not just "cleanup", it's
a paradigm shift.
Suddenly, concrete building of LDM appears to be shaking. And reasonable
question here is: is this a trend? What we will lose next - ability to
query/enumerate devices on bus? How much time it will take until
someone submit patch like "You know, I had a look at this
device vs drivers matching in LDM. You know, I found that in 90% it's
one-to-one mapping. You know, I think that rest 10% are just doing
something wrong. You know, let's don't care about them. Here's a
patch to match compile-time by C symbols. Oh no, wait, I have a patch
in preparation which kills device vs drivers distinction at all,
leading us to ol' good dirty monolithic 'drivers'." And it may be the
case that even designers of LDM will look at the code after all
those previous "cleanups" applied, and would agree that such patch
comes as pretty natural succession.
Thanks for following my mental movie ;-). It's not a pure rhetoric
though, I indeed would like to know the answer - are all those
structures in LDM are big guys' games only? And would it better for small
guys to be as KISS as stupid, err..., possible? ;-)
> Also, any reason why your drivers aren't in the mainline kernel yet? It
> would have kept something like this from happening a while ago. And, it
> will also help out with the recent driver core changes that are being
> planned and are starting to show up in the -mm tree. If your stuff is
> in the kernel, then I'll do the work for you, otherwise, you all are on
> your own :(
One reason is of course because it's not that easy to get something
into mainline. ;-) Another one is that we have great deal of code made by
different people over different periods of time, and we need to do
internal cleanup first. And for example, as new people do it, and
learn the entire codebase, they find that some things are done,
well, so to say, above the ground level. And they start to worry if
that was right in the first place, and would it be now possible to get
this somebody's code into mainline, or its better to keep doing
homework and reapply the KISS paradigm. It is also a bit of ethical
problem, as gentlemen who made such implementations, were apparently
experienced developers to come up with such solutions, but no longer
around/have resources to lobby for such code themselves, and newcomers
would likely get only frustration from trying to do that, especially
if they see there *is* at least an abstract possibility to do it in
some other, more mundane way. And yet they can't readily do that local
paradigm shift, as well, they don't have enough experience to undo
what gurus did previously, plus the whole Linux-on-PDA community is
too short on people to spend time flip-flopping each other's work.
All above has little to do with mainline kernel, of course, and not
its problem in any way. Just another entry into your sociological
survey "Why people don't submit code upstream." ;-)
> thanks,
> greg k-h
Thanks for your following this case,
--
Paul mailto:[email protected]
On Mon, Nov 20, 2006 at 04:13:22PM +0200, Paul Sokolovsky wrote:
>...
> But note that I don't really ask mainline kernel developers how to
> fix this driver - I would actually be ashamed to do so, as I myself a
> (newbie) kernel hacker. So, the question stays the same, though I
> probably reformulate it a bit stronger now: how it came that ability
> to query buses (at all) was removed from 2.6.18?
>
> As it was before, it was clear that LDM consists of multiple layers,
> and each layer offers consistent and complete set of operations on it,
> like adding new object on this layer, removing, adding child, removing
> child, *and* query objects on this level or among childs. I may miss
> some accidental gaps in that picture of course, but it still was an
> integral, complete design paradigm offering full dynamicity and
> introspection.
>
> And suddenly - oops, in 2.6.18 we lose ability to query the highest
> level of hierarchy, namely bus set. And on what criterion? "unused". I
> would really dream that such core, the most basic APIs are not being
> defined in terms of "someone does use it right now". A method to query
> objects of core kernel data sets is just integral part of interface to
> these datasets, you cannot remove it and not cripple such interface.
> Again, it's loss of introspection, and that's not just "cleanup", it's
> a paradigm shift.
>...
As Documentation/stable_api_nonsense.txt explains, there is no stable
kernel API. If you don't want to get the APIs your driver uses
changed/removed you should really try to get it merged into mainline.
The find_bus() case is even more interesting since you are using it in a
driver although it has never been exported to modules. Considering that
you anyway have to patch the kernel for getting your driver running
(since it won't run as a module against an unmodified kernel), you could
simply undo my patch locally until you submit your driver for inclusion
in mainline.
> Paul
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, Nov 20, 2006 at 04:13:22PM +0200, Paul Sokolovsky wrote:
> Hello Greg,
>
> Monday, November 20, 2006, 2:12:12 AM, you wrote:
>
> > On Mon, Nov 20, 2006 at 12:45:51AM +0100, Jiri Slaby wrote:
> >> Paul Sokolovsky wrote:
> >> > But alas, the commit message is not as good as some others are, and
> >> > doesn't mention what should be used instead. So, if find_bus() is
> >> > "unused", what should be used instead?
> >>
> >> You should probably mention what for?
>
> > Exactly. It was removed because no one was using it, and I couldn't
> > think of a reason why it would be needed.
>
> So, I assume the answer to my question is "No replacement. Ability
> to query bus set in the kernel was just removed, period." That's to
> which conclusion I came myself after studying 2.6.18 source and patch
> from 2.6.17.
Yes, it was removed because no one was using it, and no one could think
of any reason why it would be needed at the time.
> [Uninteresting specific case]
>
> Ok, so the situation is following: we have a kind of multi-layered
> driver here. Lowest level is a w1_slave bus driver, talking to a
> specific chip and providing low-level API for accessing data in terms
> of this chip (or chip class) notions. Above it, we have higher-level
> driver which interprets data from the low-level one, converting it to
> a standard device-independent form, plus possibly does some other
> minor things, like providing feedback indication on these data.
> (Forgot to say that this is battery driver.)
>
> So, just in case if some reader of this has quick suggestion of
> merging these drivers into one, thanks, but they do different things,
> and we want to keep them nicely decoupled. But now issue of how these
> drivers talk between themselves raises, and that's exactly the grief
> point.
>
> High-level driver used to find w1 bus by name, then enumerate
> devices on the bus, to find compatible device on it, then hooks into
> that device and its driver.
Ick, why not just export the pointer to the devices?
> So, you see the issue: we cannot enumerate devices on w1 bus. (And
> yes, w1_bus_type is not exported).
>
> Sure, the source is up:
> http://handhelds.org/cgi-bin/cvsweb.cgi/linux/kernel26/drivers/misc/h2200_battery.c?rev=1.20&content-type=text/x-cvsweb-markup
>
>
> [Trends thoughts]
>
> But note that I don't really ask mainline kernel developers how to
> fix this driver - I would actually be ashamed to do so, as I myself a
> (newbie) kernel hacker. So, the question stays the same, though I
> probably reformulate it a bit stronger now: how it came that ability
> to query buses (at all) was removed from 2.6.18?
See above.
> As it was before, it was clear that LDM consists of multiple layers,
> and each layer offers consistent and complete set of operations on it,
> like adding new object on this layer, removing, adding child, removing
> child, *and* query objects on this level or among childs. I may miss
> some accidental gaps in that picture of course, but it still was an
> integral, complete design paradigm offering full dynamicity and
> introspection.
Hm, I've never heard the driver model be called a "complete design
paradigm" in the past. I've heard it called a lot of real nasty things
though :)
> And suddenly - oops, in 2.6.18 we lose ability to query the highest
> level of hierarchy, namely bus set. And on what criterion? "unused".
<snip>
Please go read Documenation/stable_api_nonsense.txt for why the
in-kernel apis always change. It's how Linux works.
> Suddenly, concrete building of LDM appears to be shaking. And reasonable
> question here is: is this a trend? What we will lose next - ability to
> query/enumerate devices on bus? How much time it will take until
> someone submit patch like "You know, I had a look at this
> device vs drivers matching in LDM. You know, I found that in 90% it's
> one-to-one mapping. You know, I think that rest 10% are just doing
> something wrong. You know, let's don't care about them. Here's a
> patch to match compile-time by C symbols. Oh no, wait, I have a patch
> in preparation which kills device vs drivers distinction at all,
> leading us to ol' good dirty monolithic 'drivers'." And it may be the
> case that even designers of LDM will look at the code after all
> those previous "cleanups" applied, and would agree that such patch
> comes as pretty natural succession.
It might just come to that if things evolve that way, who really knows.
> Thanks for following my mental movie ;-). It's not a pure rhetoric
> though, I indeed would like to know the answer - are all those
> structures in LDM are big guys' games only? And would it better for small
> guys to be as KISS as stupid, err..., possible? ;-)
Please, get your code into the main kernel tree so that we can see it
and know how the driver core is being used.
> > Also, any reason why your drivers aren't in the mainline kernel yet? It
> > would have kept something like this from happening a while ago. And, it
> > will also help out with the recent driver core changes that are being
> > planned and are starting to show up in the -mm tree. If your stuff is
> > in the kernel, then I'll do the work for you, otherwise, you all are on
> > your own :(
>
> One reason is of course because it's not that easy to get something
> into mainline. ;-)
Why do you feel this way? Is it a proceedural thing? A technical
thing? A time thing?
We want to make it as easy as possible to get code into the tree, please
submit it and be persistant to get it there.
> Another one is that we have great deal of code made by
> different people over different periods of time, and we need to do
> internal cleanup first.
That's the way the kernel works, nothing new here.
> And for example, as new people do it, and
> learn the entire codebase, they find that some things are done,
> well, so to say, above the ground level. And they start to worry if
> that was right in the first place, and would it be now possible to get
> this somebody's code into mainline, or its better to keep doing
> homework and reapply the KISS paradigm. It is also a bit of ethical
> problem, as gentlemen who made such implementations, were apparently
> experienced developers to come up with such solutions, but no longer
> around/have resources to lobby for such code themselves, and newcomers
> would likely get only frustration from trying to do that, especially
> if they see there *is* at least an abstract possibility to do it in
> some other, more mundane way. And yet they can't readily do that local
> paradigm shift, as well, they don't have enough experience to undo
> what gurus did previously, plus the whole Linux-on-PDA community is
> too short on people to spend time flip-flopping each other's work.
Evolve things over time in the main tree, like the main tree works.
Again, this is nothing different from the rest of the kernel.
> All above has little to do with mainline kernel, of course, and not
> its problem in any way. Just another entry into your sociological
> survey "Why people don't submit code upstream." ;-)
No, I really think you are somehow feeling that once the code is in
mainline it can't be ever changed. That's just not true...
thanks,
greg k-h
Greg KH wrote:
> On Mon, Nov 20, 2006 at 04:13:22PM +0200, Paul Sokolovsky wrote:
> > And suddenly - oops, in 2.6.18 we lose ability to query the highest
> > level of hierarchy, namely bus set. And on what criterion? "unused".
>
> Please go read Documenation/stable_api_nonsense.txt for why the
> in-kernel apis always change. It's how Linux works.
>
> > Suddenly, concrete building of LDM appears to be shaking. And reasonable
> > question here is: is this a trend?
Good question.
> > > Also, any reason why your drivers aren't in the mainline kernel yet?
> >
> > One reason is of course because it's not that easy to get something
> > into mainline. ;-)
>
> Why do you feel this way? Is it a proceedural thing? A technical
> thing? A time thing?
>
> We want to make it as easy as possible to get code into the tree, please
> submit it and be persistant to get it there.
Persistent as in: get down on your knees and say "please, please, pretty
please"?
AFAICT, code is being ignored/rejected for the mere reason that it's not
considered useful; but who's to say what is useful and what's not?
There should probably be some guideline to indicate what kind of code is
acceptable and what's not. You know, something like a constitution.
Otherwise, we'll probably have a situtation like the jungle, where the weak
are silently being killed off.
Is that the kind of evolution you are aiming for?
BTW, no troll, really.
Thanks!
--
Al
> Persistent as in: get down on your knees and say "please, please, pretty
> please"?
no as in "listen to the code review feedback and fix things and then try
again"
>
> AFAICT, code is being ignored/rejected for the mere reason that it's not
> considered useful; but who's to say what is useful and what's not?
no things are being reject because they're not being *used*.
Subtle but important difference ;)
But yes, you need to do a moderate amount of "selling" when submitting
code. For a driver that's easy usually, "this is a driver for device X"
and "sold" if the code is reasonable.
For major subsystem changes... you need to explain why it's worth the
disruption. If you can't explain that... then what good would it be ?
--
if you want to mail me at work (and no Mr Al Boldi, you really don't want to), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
Hello Adrian,
Monday, November 20, 2006, 7:35:50 PM, you wrote:
[]
>> And suddenly - oops, in 2.6.18 we lose ability to query the highest
>> level of hierarchy, namely bus set.
[]
> As Documentation/stable_api_nonsense.txt explains, there is no stable
> kernel API.
Thanks, Adrian, from the first mail I really counted which reply #
(if I would get them at all) will point me to that "bible" of kernel
developers, and just wondered, would it be too ugly to ask for an answer
which wouldn't recourse to that doc.
But now that you mentioned it, let me make you laugh at jaundiced
eye's look on the issue: it appears it's just itch for kernel
developers to make APIs not stable. When some API is stable, because
it's both trivial and complete (like that bus methods which are just
based on generic container object idea - you can't add or take from
that), some artificial criteria are employed, like "unused", to still
find a way to destabilize API ;-).
> If you don't want to get the APIs your driver uses
> changed/removed you should really try to get it merged into mainline.
Would this really be the case? I guess, most people would think that
getting something into mainline is indeed the end of battle. And yet
for others, it may be the case that single driver using some API is
almost just the same as 0 drivers doing that. If 1001 is almost 1000,
why 1 can't be almost 0? Why this would be worse than "Well, noone
uses that call now - let's make noone use it ever, at all."
So that's what I'm trying to argue - let the change which needs to
be done, be based on the high-level ideas of the meaning of the thing
being changed, not on purely quantitative, and thus, subject to
separate interpretation, parameters.
And removing a method from an integral, high-level API set is not the
same as killing static variable in a hardware driver.
> The find_bus() case is even more interesting since you are using it in a
> driver although it has never been exported to modules. Considering that
> you anyway have to patch the kernel for getting your driver running
> (since it won't run as a module against an unmodified kernel), you could
> simply undo my patch locally until you submit your driver for inclusion
> in mainline.
Well, I believe you expected and cared to get such feedback, because
otherwise, you wouldn't use #if 0, but killed unlucky function
completely ;-(. I don't hold breath for those #if 0 to be removed
based on the above right now, but I hope the function at least won't
be removed completely for now, to indeed let whoever may have need for
it still use it somehow (and submit code to mainline, if that is what
actually would take to have the issue resolved).
As for EXPORT's, indeed, mainline kernel lacks quite a few
which are useful. But obviously, I won't argue for adding them now,
out of context - that would be just as random change, as, IMHO, and
sorry, removing find_bus() from use.
>> Paul
> cu
> Adrian
--
Best regards,
Paul mailto:[email protected]
> And removing a method from an integral, high-level API set is not the
> same as killing static variable in a hardware driver.
removing dead code that nobody is using should be applauded though.
If we left all unused code in the kernel binary we'd soon have a 100Mb
vmlinuz file.... and an unmaintainable mess.
Actively removing parts of the kernel code that nobody is using is a
fight against that bloat (and yes the kernel has grown far too big
already, as I'm sure the handheld.org people already know... you
wouldn't want a 2x bigger vmlinuz right?...
Also, if an API or part of an API is unused, it's quite possibly an API
that SHOULDN'T be used, either because it's designed totally shite (like
sleep_on() is :), because the implementation is really horrid or because
it offers a function that nobody actually needs.
All three are grounds for removal to keep the vmlinuz side down in my
book. That isn't change for change's sake (I'm starting to sound like a
Harry Potter book) but that's an attempt to keep the bloat of the
vmlinuz down.
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
On Tue, Nov 21, 2006 at 05:08:46PM +0200, Paul Sokolovsky wrote:
> Hello Adrian,
Hello Paul,
> Monday, November 20, 2006, 7:35:50 PM, you wrote:
>
> []
>
> >> And suddenly - oops, in 2.6.18 we lose ability to query the highest
> >> level of hierarchy, namely bus set.
>
> []
>
> > As Documentation/stable_api_nonsense.txt explains, there is no stable
> > kernel API.
>
> Thanks, Adrian, from the first mail I really counted which reply #
> (if I would get them at all) will point me to that "bible" of kernel
> developers, and just wondered, would it be too ugly to ask for an answer
> which wouldn't recourse to that doc.
>
> But now that you mentioned it, let me make you laugh at jaundiced
> eye's look on the issue: it appears it's just itch for kernel
> developers to make APIs not stable. When some API is stable, because
> it's both trivial and complete (like that bus methods which are just
> based on generic container object idea - you can't add or take from
> that), some artificial criteria are employed, like "unused", to still
> find a way to destabilize API ;-).
if API changes in the kernel break external modules that's not
intentional, but a positive side effect reminding people that they
should submit their modules for inclusion in mainline.
> > If you don't want to get the APIs your driver uses
> > changed/removed you should really try to get it merged into mainline.
>
> Would this really be the case? I guess, most people would think that
> getting something into mainline is indeed the end of battle. And yet
> for others, it may be the case that single driver using some API is
> almost just the same as 0 drivers doing that. If 1001 is almost 1000,
> why 1 can't be almost 0? Why this would be worse than "Well, noone
> uses that call now - let's make noone use it ever, at all."
The rule is roughly:
If you change/remove a kernel API, you have to fix all in-kernel
users.
As an example, the irqreturn_t change in 2.6.19 will require changes to
your driver. It also broke at about 1000 in-kernel drivers, but all of
them have been fixed as part of the change (the commit changed 1079
different files).
> So that's what I'm trying to argue - let the change which needs to
> be done, be based on the high-level ideas of the meaning of the thing
> being changed, not on purely quantitative, and thus, subject to
> separate interpretation, parameters.
>...
Many people are still using kernel 2.4 for new kernel development
because kernel 2.6 images are significantly bigger. Every unused byte
removed from the kernel images makes this space regression smaller.
> > The find_bus() case is even more interesting since you are using it in a
> > driver although it has never been exported to modules. Considering that
> > you anyway have to patch the kernel for getting your driver running
> > (since it won't run as a module against an unmodified kernel), you could
> > simply undo my patch locally until you submit your driver for inclusion
> > in mainline.
>
> Well, I believe you expected and cared to get such feedback, because
> otherwise, you wouldn't use #if 0, but killed unlucky function
> completely ;-(. I don't hold breath for those #if 0 to be removed
> based on the above right now, but I hope the function at least won't
> be removed completely for now, to indeed let whoever may have need for
> it still use it somehow (and submit code to mainline, if that is what
> actually would take to have the issue resolved).
>
> As for EXPORT's, indeed, mainline kernel lacks quite a few
> which are useful. But obviously, I won't argue for adding them now,
> out of context - that would be just as random change, as, IMHO, and
> sorry, removing find_bus() from use.
It's an integral part of the Linux development model that any kind of
in-kernel API changes are always possible.
There's a choice between stable APIs on the one side and speed of
development and smaller kernel images on the other side, and the Linux
kernel prefers the latter.
> Best regards,
> Paul
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
Arjan van de Ven wrote:
> On Mon, 2006-11-20 at 02:13 +0200, Paul Sokolovsky wrote:
>> Hello Jiri,
>>
>> Monday, November 20, 2006, 1:45:51 AM, you wrote:
>>
>>> Paul Sokolovsky wrote:
>>>> But alas, the commit message is not as good as some others are, and
>>>> doesn't mention what should be used instead. So, if find_bus() is
>>>> "unused", what should be used instead?
>>> You should probably mention what for?
>> Indeed, I'm sorry! Looking at find_bus()'s docstring:
>>
>> /**
>> * find_bus - locate bus by name.
>> * @name: name of bus.
>>
>> So well, I'd like to know exactly that - what function should be
>> used instead of find_bus() to locate bus by name.
>
> I think the question more was "what do you need to look up a bus by name
> for in the kernel"? Like.. what are you trying to achieve? What module
> is this for? (does it have a homepage where people can download the
> source etc so that they can give you a more informed answer)....
>
>
Arjan has a great point, and it's been mentioned elsewhere in this thread, too.
Maybe I can help get us there. Since you've already started to explain
(ignoring your non-sequitur on the development process):
----
Ok, so the situation is following: we have a kind of multi-layered
driver here. Lowest level is a w1_slave bus driver, talking to a
specific chip and providing low-level API for accessing data in terms
of this chip (or chip class) notions. Above it, we have higher-level
driver which interprets data from the low-level one, converting it to
a standard device-independent form, plus possibly does some other
minor things, like providing feedback indication on these data.
(Forgot to say that this is battery driver.)
So, just in case if some reader of this has quick suggestion of
merging these drivers into one, thanks, but they do different things,
and we want to keep them nicely decoupled. But now issue of how these
drivers talk between themselves raises, and that's exactly the grief
point.
High-level driver used to find w1 bus by name, then enumerate
devices on the bus, to find compatible device on it, then hooks into
that device and its driver.
So, you see the issue: we cannot enumerate devices on w1 bus. (And
yes, w1_bus_type is not exported).
Sure, the source is up:
http://handhelds.org/cgi-bin/cvsweb.cgi/linux/kernel26/drivers/misc/h2200_battery.c?rev=1.20&content-type=text/x-cvsweb-markup
----
So you have nested drivers. The bottom driver (w1/slaves/w1_ds2760.c) talks to
the hardware, and the top driver (misc/h2200_battery.c) interprets the output.
You're dealing with a Dallas 1-Wire Bus protocol to talk to your battery
management chip, which is a DS2760 High-Precision Li+ Battery Monitor. You're
telling us that h2200_battery uses find_bus() to locate the w1 bus and then the
devices on that bus, so that it can use w1_ds2760 to read the chip. So what is
hanging you up here is that your top-level driver can't find the bus that the
chip is on; once it can, everything should work?
The specific code:
void
h2200_battery_probe_work(void *data)
{
struct bus_type *bus;
/* Get the battery w1 slave device. */
bus = find_bus("w1");
if (bus)
ds2760_dev = bus_find_device(bus, NULL, NULL,
h2200_battery_match_callback);
if (!ds2760_dev) {
/* No DS2760 device found; try again later. */
queue_delayed_work(probe_q, &probe_work, HZ * 5);
return;
}
}
What we need to do is help you find a better way to locate and identify a w1 device.
(cc: E. Polyakov for the w1 expertise)
Matt
On 11/20/06, Paul Sokolovsky <[email protected]> wrote:
>
> [Uninteresting specific case]
>
> Ok, so the situation is following: we have a kind of multi-layered
> driver here. Lowest level is a w1_slave bus driver, talking to a
> specific chip and providing low-level API for accessing data in terms
> of this chip (or chip class) notions. Above it, we have higher-level
> driver which interprets data from the low-level one, converting it to
> a standard device-independent form, plus possibly does some other
> minor things, like providing feedback indication on these data.
> (Forgot to say that this is battery driver.)
>
> So, just in case if some reader of this has quick suggestion of
> merging these drivers into one, thanks, but they do different things,
> and we want to keep them nicely decoupled. But now issue of how these
> drivers talk between themselves raises, and that's exactly the grief
> point.
>
My suggestion would be do define a class for your device independent
data and have low-level driver create a class device (or just a device
if Greg has his way with driver core) and change your high-level
driver to become a class interface.
And then you won't have to rescan w1 bus every 5 seconds for that battery...
--
Dmitry
On Tue, Nov 21, 2006 at 12:29:15PM -0600, Matthew Frost ([email protected]) wrote:
> So you have nested drivers. The bottom driver (w1/slaves/w1_ds2760.c) talks to
> the hardware, and the top driver (misc/h2200_battery.c) interprets the output.
> You're dealing with a Dallas 1-Wire Bus protocol to talk to your battery
> management chip, which is a DS2760 High-Precision Li+ Battery Monitor. You're
> telling us that h2200_battery uses find_bus() to locate the w1 bus and then the
> devices on that bus, so that it can use w1_ds2760 to read the chip. So what is
> hanging you up here is that your top-level driver can't find the bus that the
> chip is on; once it can, everything should work?
>
> The specific code:
>
> void
> h2200_battery_probe_work(void *data)
> {
> struct bus_type *bus;
>
> /* Get the battery w1 slave device. */
> bus = find_bus("w1");
> if (bus)
> ds2760_dev = bus_find_device(bus, NULL, NULL,
> h2200_battery_match_callback);
>
> if (!ds2760_dev) {
> /* No DS2760 device found; try again later. */
> queue_delayed_work(probe_q, &probe_work, HZ * 5);
> return;
> }
> }
>
> What we need to do is help you find a better way to locate and identify a w1 device.
>
> (cc: E. Polyakov for the w1 expertise)
Hello.
If find_bus() will not be resurrected, I can export w1_bus_type or
create special helpers to directly access w1 bus master devices.
But in that case there is no need to have all driver model in w1
subsystem at all...
> Matt
--
Evgeniy Polyakov
Hello Evgeniy,
Tuesday, November 21, 2006, 9:01:50 PM, you wrote:
> On Tue, Nov 21, 2006 at 12:29:15PM -0600, Matthew Frost ([email protected]) wrote:
>> So you have nested drivers.
Matthew, thanks for your help, but it wasn't really my intention to
waste other people's time with fixing our drivers. We are kernel
hackers themselves, and eat our dogfood - with each kernel
release, we have bunch of drivers breaking, and we patiently fix them
(and yes, with recent releases, number of such drivers reduces, and
that makes us really happy with recent 2.6 releases).
But, this particular case made me wonder - was it some issue with
change made in mainline, or there's something wrong with our manner to
write drivers? And we'd like to be updated in the latter case.
[]
>>
>> (cc: E. Polyakov for the w1 expertise)
> Hello.
> If find_bus() will not be resurrected, I can export w1_bus_type or
> create special helpers to directly access w1 bus master devices.
> But in that case there is no need to have all driver model in w1
> subsystem at all...
Thanks for your response, Evgeniy!
Ok, so now it's not just me, it's the maintainer of a bus subsystem
in mainline. There's no wonders, and one uncareful change in core API
will cascade to many other places. Commented out find_bus()? Now need to
make sure all bus types structures are exported. At least. But maybe
maintainers will also wonder what happens here, and shouldn't they
provide adhoc API to query a specific bus? And then indeed, what is
the use of LDM? Where did go idea of having common, bus- and driver-
independent API to do consistently all the stuff one *may* need (not
all feature of which everyone necessarily uses all the time).
P.S.
find_bus()is hardly a threat for kernel binary size and for bringing
more 2.4 users to 2.6:
http://lxr.linux.no/source/drivers/base/bus.c?v=2.6.18#L602
Well, I'd actually hardly ventured into arguing that its removal may
be not exactly right, if it wasn't such an obvious case of crippling
API for no real benefit. But it's not long cry for 2 lines of code,
but for understanding of where kernel goes...
>> Matt
--
Best regards,
Paul mailto:[email protected]
Paul Sokolovsky wrote:
> Hello Evgeniy,
>
> Tuesday, November 21, 2006, 9:01:50 PM, you wrote:
>
>> On Tue, Nov 21, 2006 at 12:29:15PM -0600, Matthew Frost ([email protected]) wrote:
>>> So you have nested drivers.
>
> Matthew, thanks for your help, but it wasn't really my intention to
> waste other people's time with fixing our drivers. We are kernel
> hackers themselves, and eat our dogfood - with each kernel
> release, we have bunch of drivers breaking, and we patiently fix them
> (and yes, with recent releases, number of such drivers reduces, and
> that makes us really happy with recent 2.6 releases).
Alright, you're kernel hackers. But I'd like to take issue with the mindset
that these drivers are your burden, to bear by yourselves. It seems to me that
anyone with an iPaq relies upon this code path, or one like it, so you are
supporting a bunch of users by yourselves, and holding up the burden of
compatibility with the mainline kernel at the same time. Doing twice as much
work, on two moving targets. That's the waste of time.
> But, this particular case made me wonder - was it some issue with
> change made in mainline, or there's something wrong with our manner to
> write drivers? And we'd like to be updated in the latter case.
And here's where the recommendation to get your code into mainline comes in. If
the kernel developers don't know that you're using something, they can't tell
you what it was replaced by, or even that they need to replace it with anything.
"No mainline users of the code" is a reliable baseline for value, because we
can't go looking for out-of-tree users in any convenient way. Therefore nobody
tells you it's going to break until you learn the hard way that it broke, and
holler at the list. Nobody *can* update you. It isn't malice, it's finity. If
your code is in mainline, whoever makes a change to mainline has to care,
because there is a legitimate burden of compatibility assurance, and it isn't
yours alone any longer.
>
> []
>
>>> (cc: E. Polyakov for the w1 expertise)
>
>> Hello.
>
>> If find_bus() will not be resurrected, I can export w1_bus_type or
>> create special helpers to directly access w1 bus master devices.
>> But in that case there is no need to have all driver model in w1
>> subsystem at all...
>
> Thanks for your response, Evgeniy!
>
>
> Ok, so now it's not just me, it's the maintainer of a bus subsystem
> in mainline. There's no wonders, and one uncareful change in core API
> will cascade to many other places. Commented out find_bus()? Now need to
> make sure all bus types structures are exported. At least. But maybe
> maintainers will also wonder what happens here, and shouldn't they
> provide adhoc API to query a specific bus? And then indeed, what is
> the use of LDM? Where did go idea of having common, bus- and driver-
> independent API to do consistently all the stuff one *may* need (not
> all feature of which everyone necessarily uses all the time).
>
I could be wrong, but it doesn't sound like Evgeniy is complaining that this
problem hits him. The other drivers in w1/slaves don't use find_bus(). He's
offering to give you an export to hook into. Though I might also look at
Dmitry's suggestion and rework around driver classes. Which may fit in with
what Evgeniy says about moving the driver model out of w1.
Matt
Hello Greg,
Tuesday, November 21, 2006, 9:54:31 AM, you wrote:
[]
>> So, I assume the answer to my question is "No replacement. Ability
>> to query bus set in the kernel was just removed, period." That's to
>> which conclusion I came myself after studying 2.6.18 source and patch
>> from 2.6.17.
> Yes, it was removed because no one was using it, and no one could think
> of any reason why it would be needed at the time.
>> [Uninteresting specific case]
>>
>> Ok, so the situation is following: we have a kind of multi-layered
>> driver here. Lowest level is a w1_slave bus driver, talking to a
>> specific chip and providing low-level API for accessing data in terms
>> of this chip (or chip class) notions. Above it, we have higher-level
>> driver which interprets data from the low-level one, converting it to
>> a standard device-independent form, plus possibly does some other
>> minor things, like providing feedback indication on these data.
>> (Forgot to say that this is battery driver.)
>>
>> So, just in case if some reader of this has quick suggestion of
>> merging these drivers into one, thanks, but they do different things,
>> and we want to keep them nicely decoupled. But now issue of how these
>> drivers talk between themselves raises, and that's exactly the grief
>> point.
>>
>> High-level driver used to find w1 bus by name, then enumerate
>> devices on the bus, to find compatible device on it, then hooks into
>> that device and its driver.
> Ick, why not just export the pointer to the devices?
That's what I kept in mind as one possible solution to our problem
with that driver. Indeed, if kernel no longer wants us to enumerate
buses, why reduce to half measures - use static reference to
w1_bus_type, but still enumerate devices on it? Let's just statically
tap into the exact low-level device we need.
Oops, but w1 core doesn't provide static struct device for w1 devices.
Devices are defined in terms of w1 bus attributes, and struct device's
are allocated solely on demand. And I know very little about w1 bus &
core, but what I know is enough for me *not* to challenge what w1 core
does: that bus is inherently hot pluggable, plus designed for industrial
automation, so it may be the case that thousands of w1 sensors would
be attached to host, so allocating more or less big struct device's on
demand is what I call "saving memory".
But let me add another para here: in the description of how our
driver works, I didn't wrote we look for *specific* w1 device. We look
for *compatible* w1 device. It's true that at this time we have 1-to-1
mapping, and even true that in our specific case we actually could
live with 1-to-1 mapping for long time more. But with "deprecating"
tendencies towards using introspectives on LDM objects, you rule out
such cases of dynamic binding at all ;-( (And I don't think
noone uses it because it's useless, I think that LDM with its dynamic
features is still too young for people to fully explore its potential
and start use it at its best).
[]
>> As it was before, it was clear that LDM consists of multiple layers,
>> and each layer offers consistent and complete set of operations on it,
>> like adding new object on this layer, removing, adding child, removing
>> child, *and* query objects on this level or among childs. I may miss
>> some accidental gaps in that picture of course, but it still was an
>> integral, complete design paradigm offering full dynamicity and
>> introspection.
> Hm, I've never heard the driver model be called a "complete design
> paradigm" in the past. I've heard it called a lot of real nasty things
> though :)
Well, it's probably still too young for people to take it as
a whole paradigm yet ;-). And I don't share opinion that it is nasty
thing, so skip on that ;-).
[]
[]
> Please, get your code into the main kernel tree so that we can see it
> and know how the driver core is being used.
That's what we want very much too, so we'll start (actually,
continue, of course) on that.
[]
>> One reason is of course because it's not that easy to get something
>> into mainline. ;-)
> Why do you feel this way? Is it a proceedural thing? A technical
> thing? A time thing?
> We want to make it as easy as possible to get code into the tree, please
> submit it and be persistant to get it there.
Well, mere look at kernel release tarball size suggests the kernel
should use conservative change control ;-). And no, I don't think that
people should submit their rough code, and waste kernel maintainers'
time with reviewing unpolished code.
But thanks for encouraging, it is indeed very nice to hear that ease
of submission and support of people doing that is the priority for
kernel developers.
[]
> Evolve things over time in the main tree, like the main tree works.
> Again, this is nothing different from the rest of the kernel.
>> All above has little to do with mainline kernel, of course, and not
>> its problem in any way. Just another entry into your sociological
>> survey "Why people don't submit code upstream." ;-)
> No, I really think you are somehow feeling that once the code is in
> mainline it can't be ever changed. That's just not true...
Well, I don't have that feeling, it's just when it gets into
mainline it's harder to do the changes, so we'd better do trivial
ones, like ensuring consistent symbol naming on our side first. And of
course we don't want to get bounce simply because code style doesn't
comply. And that's the start of list of 10+ "small" things we need
to do before submission.
So, we won't be posting our 8Mb+ patch just tomorrow, and of course
won't expect it to to me merged in a week ;-). But we hope to have
something ready to 2.6.20 patch window (even though we probably don't
have anything too serious to require submission only within patch
window). Probably even start before that, to get criticism sooner, as
patch window is too short for our process.
So, I probably stop wasting your time with this discussion now,
saving it for real things. And I'd like to thank everyone who
responded. I made the following resume out of it:
There's nothing wrong going with LDM, it's still as shiny as it
was. It's just find_bus() has become to be considered harmful, unless
proven otherwise - with the code in mainline. So far, so good.
Thanks!
> thanks,
> greg k-h
--
Best regards,
Paul mailto:[email protected]
Arjan van de Ven wrote:
> > Persistent as in: get down on your knees and say "please, please, pretty
> > please"?
>
> no as in "listen to the code review feedback and fix things and then try
> again"
You can't do much listening if it's being ignored.
> > AFAICT, code is being ignored/rejected for the mere reason that it's not
> > considered useful; but who's to say what is useful and what's not?
>
> no things are being reject because they're not being *used*.
> Subtle but important difference ;)
>
> But yes, you need to do a moderate amount of "selling" when submitting
> code. For a driver that's easy usually, "this is a driver for device X"
> and "sold" if the code is reasonable.
>
> For major subsystem changes... you need to explain why it's worth the
> disruption. If you can't explain that...
So now we have to be linguists? Some people don't have the ability to
explain things easily(wordily); call it a disability, if you like.
> --
> if you want to mail me at work (and no Mr Al Boldi, you really don't want
> to), use arjan (at) linux.intel.com
I was trying to do you a favor; anyway, is something forcing you to put that
line into your sig?
Thanks anyway!
--
Al