Hi Nuno,
On Wed, 06 Mar 2024 15:50:44 +0100 Nuno Sá noname.nuno@gmail.com wrote:
...
That makes sense but then the only thing I still don't fully get is why we have a separate devlink_class_init() initcall for registering the devlink class (which can also fail)...
Well, I haven't added it. :-)
What I take from the above is that we should fail the driver model if one of it's fundamental components fails so I would say we should merge devlink_class_init() with device_init() otherwise it's a bit confusing (at least to me) and gives the idea that it's ok for the driver model to exist without the links (unless I'm missing some other reason for the devlink init function).
+1
Feel free to send a patch along these lines, chances are that it will be popular. ;-)
I was actually thinking about that but I think I encountered the reason why we have it like this... devices_init() is called from driver_init() and there we have:
...
devices_init(); buses_init(); classes_init();
...
So classes are initialized after devices which means we can't really do class_register(&devlink_class) from devices_init(). Unless, of course, we re- order things in driver_init() but that would be a questionable change at the very least.
So, while I agree with what you've said, I'm still not sure if mixing devlink stuff between devices_init() and devlink_class_init() is the best thing to do given that we already have the case where devlink_class_init() can fail while the driver model is up.
So why don't you make devlink_class_init() do a BUG() on failure instead of returning an error? IMO crashing early is better than crashing later or otherwise failing in a subtle way due to a missed dependency.
Well, I do agree with that... Maybe that's something that Herve can sneak in this patch? Otherwise, I can later (after this one is applied) send a patch for it.
Well, I don't thing that this have to be part of this current series. It is an other topic and should be handled out of this current series.
Hervé