Re: [PATCH v2] Docs: usb: update struct usb_driver, __init and __exit
From: Philipp Hortmann
Date: Fri Oct 22 2021 - 02:23:58 EST
On 10/21/21 10:00 AM, Greg KH wrote:
On Wed, Oct 20, 2021 at 10:14:46PM +0200, Philipp Hortmann wrote:
update struct usb_driver from usb-skeleton.c.
update __init and __exit functions that are moved from
usb-skeleton.c to common used multi-stage macros.
Signed-off-by: Philipp Hortmann <philipp.g.hortmann@xxxxxxxxx>
---
V1 -> V2: changed :c:func:`usb_register` to usb_register()
changed the :c:func:`usb_deregister` to usb_deregister()
used literal blocks for makro module_usb_driver and added one more
stage of multi-stage macros.
.../driver-api/usb/writing_usb_driver.rst | 70 ++++++++++---------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/Documentation/driver-api/usb/writing_usb_driver.rst b/Documentation/driver-api/usb/writing_usb_driver.rst
index 2176297e5765..12e0481cceae 100644
--- a/Documentation/driver-api/usb/writing_usb_driver.rst
+++ b/Documentation/driver-api/usb/writing_usb_driver.rst
@@ -54,12 +54,15 @@ information is passed to the USB subsystem in the :c:type:`usb_driver`
structure. The skeleton driver declares a :c:type:`usb_driver` as::
static struct usb_driver skel_driver = {
- .name = "skeleton",
- .probe = skel_probe,
- .disconnect = skel_disconnect,
- .fops = &skel_fops,
- .minor = USB_SKEL_MINOR_BASE,
- .id_table = skel_table,
+ .name = "skeleton",
+ .probe = skel_probe,
+ .disconnect = skel_disconnect,
+ .suspend = skel_suspend,
+ .resume = skel_resume,
+ .pre_reset = skel_pre_reset,
+ .post_reset = skel_post_reset,
+ .id_table = skel_table,
+ .supports_autosuspend = 1,
Why remove the tabs? Is that needed here?
You are right will be changed.
};
@@ -81,36 +84,35 @@ this user-space interaction. The skeleton driver needs this kind of
interface, so it provides a minor starting number and a pointer to its
:c:type:`file_operations` functions.
-The USB driver is then registered with a call to :c:func:`usb_register`,
-usually in the driver's init function, as shown here::
-
- static int __init usb_skel_init(void)
- {
- int result;
-
- /* register this driver with the USB subsystem */
- result = usb_register(&skel_driver);
- if (result < 0) {
- err("usb_register failed for the "__FILE__ "driver."
- "Error number %d", result);
- return -1;
- }
-
- return 0;
- }
- module_init(usb_skel_init);
-
+The USB driver is then registered with a call to usb_register()
+which is usually in the driver's init function. Since this functionality
+is usable with many USB drivers, it is hidden behind multi-stage macros.
I don't understand the need for the "multi-stage macros" term here.
I am not a native English speaker so "multi-stage macros" is just not a
fitting wording. May be “staged macros” is better or something else…
And what functionality is referred to here by "this"?
The “this” is replacing the “init function” but when this is unclear I
will change in a later proposal…
+While the first macros are USB specific the later macros are used in different
+subsystems. This removes a lot of boilerplate code.
What later macros? Is that really needed to describe here?
I will improve wording...
I think the above code example should remain, as it is good for learning
and understanding, and maybe just add something that says "Or you can
use the following macro to replace all of the above common code."
I understand the need for keeping the code examples. But I would like to
inform the reader about the macros first.
When the driver is unloaded from the system, it needs to deregister
-itself with the USB subsystem. This is done with the :c:func:`usb_deregister`
-function::
-
- static void __exit usb_skel_exit(void)
- {
- /* deregister this driver with the USB subsystem */
- usb_deregister(&skel_driver);
- }
- module_exit(usb_skel_exit);
+itself with the USB subsystem. This is done with usb_deregister()
+which is also hidden behind multi-stage macros.
+
+The init and exit functions are included in the macro module_usb_driver.
+Find the first three stages of macros below::
+
+ module_usb_driver(skel_driver);
+ |
+ V
+ module_driver(__usb_driver, usb_register, usb_deregister)
+ | \ \
+ V ---------- ----------
+ static int __init __driver##_init(void) \ | |
+ { \ v--------------------------- |
+ return __register(&(__driver) , ##__VA_ARGS__); \ |
+ } \ |
+ module_init(__driver##_init); \ |
+ static void __exit __driver##_exit(void) \ |
+ { \ v------------------------------------------------
+ __unregister(&(__driver) , ##__VA_ARGS__); \
+ } \
+ module_exit(__driver##_exit);
As the one who wrote these macros, I can't really understand the
ascii-art here, so I worry about anyone else :)
Code is just better readable, even when code uses more lines. Will be
changed in next proposal.
Again, do not think trying to show an implementation detail like this is
needed.
The big question for me is for whom is this document written? For the
USB subsystem maintainer that has even written the code by himself? I
guess not, but may be I am wrong. Or for the kernel newbies like me?
Please consider that the changed lines are may be not so much of use for
me anymore as I am in the details.
When I saw the __init and __exit code example first, I was very happy to
see it and then I was searching in the code for it. I did not find
“init” and “exit” and was very frustrated. I want to help others to get
into this example more smoothly.
thanks,
greg k-h
Thanks that you replied at all.
Thanks for your very fast reply.
Kernelnewbie Philipp G. Hortmann