drivers/usb/gadget/udc/dummy_hcd.c | 52 ++++++++++++------------------ 1 file changed, 21 insertions(+), 31 deletions(-)
This patch series focuses on addressing various coding style issues in the dummy_hcd USB gadget driver. The changes includes adding relevant comments, improving readability, and ensuring consistency with kernel coding conventions. Clint George (6): usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444) usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned' usb: gadget: dummy_hcd: document ISO endpoint allocation pattern usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr usb: gadget: dummy_hcd: remove unnecessary parentheses usb: gadget: dummy_hcd: move function braces drivers/usb/gadget/udc/dummy_hcd.c | 52 ++++++++++++------------------ 1 file changed, 21 insertions(+), 31 deletions(-) --- Testing: - Ran compiler testing with no new warnings detected. - Ensured the module builds and inserts cleanly without issues. - Used Static Analysis tools to confirm no new issues were introduced. Please review the changes and let me know if any modifications or further testing of the module is required. As part of my LKMP mentorship i have to complete 5 patches as a criteria for graduation and thus have focused on working on such beginner-friendly patches so that not only do i get the required number of patches but also get familiar with the process of kernel developement. Thus, while this patch series doesn't address the max_stream value exceeding problem that triggers the BUG() API, i will take some time to dig deeper and truly understand the problem and fix it and not just paper-over the problem. Again, i am very grateful for your feedback greg and alan to guide a beginner like me. Thanks, Clint -- 2.43.0
On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote: > As part of my LKMP mentorship i have to complete 5 patches as a criteria > for graduation and thus have focused on working on such > beginner-friendly patches so that not only do i get the required number > of patches but also get familiar with the process of kernel > developement. The LKMP internship should be done in drivers/staging/ as generally coding style cleanups are NOT accepted in other parts of the kernel, unless you get approval from the maintainer ahead of time. Does the maintainer of this driver want this to be used for the intern project? thanks, greg k-h
On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote: > On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote: > > As part of my LKMP mentorship i have to complete 5 patches as a criteria > > for graduation and thus have focused on working on such > > beginner-friendly patches so that not only do i get the required number > > of patches but also get familiar with the process of kernel > > developement. > > The LKMP internship should be done in drivers/staging/ as generally > coding style cleanups are NOT accepted in other parts of the kernel, > unless you get approval from the maintainer ahead of time. > > Does the maintainer of this driver want this to be used for the intern > project? In fact, Clint's changes are small and inoffensive enough, I wouldn't mind having them applied to dummy-hcd. However, Greg is perfectly right that this kind of stylistic update is not something that should be submitted for most parts of the kernel. It just bulks up the Git history with essentially meaningless cruft, making it all that much harder to see the changes that really matter. That's part of the reason for the suggestion that interns and beginners should confine their efforts to drivers/staging. Also, remember that trivial changes like this are fine for learning the procedure of submitting kernel patches, but the effects they have on the kernel itself are minimal. A patch that actually fixes a bug or adds a functional enhancement would be a different story. Alan Stern
On 12/2/25 10:53, Alan Stern wrote: > On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote: >> On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote: >>> As part of my LKMP mentorship i have to complete 5 patches as a criteria >>> for graduation and thus have focused on working on such >>> beginner-friendly patches so that not only do i get the required number >>> of patches but also get familiar with the process of kernel >>> developement. >> >> The LKMP internship should be done in drivers/staging/ as generally >> coding style cleanups are NOT accepted in other parts of the kernel, >> unless you get approval from the maintainer ahead of time. >> >> Does the maintainer of this driver want this to be used for the intern >> project? > > In fact, Clint's changes are small and inoffensive enough, I wouldn't > mind having them applied to dummy-hcd. > > However, Greg is perfectly right that this kind of stylistic update is > not something that should be submitted for most parts of the kernel. It > just bulks up the Git history with essentially meaningless cruft, making > it all that much harder to see the changes that really matter. That's > part of the reason for the suggestion that interns and beginners should > confine their efforts to drivers/staging. > > Also, remember that trivial changes like this are fine for learning the > procedure of submitting kernel patches, but the effects they have on the > kernel itself are minimal. A patch that actually fixes a bug or adds a > functional enhancement would be a different story. > > Alan Stern Understood. David Hunter
On Tue, 2 Dec 2025, Alan Stern wrote:
> On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
> > On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
> > > As part of my LKMP mentorship i have to complete 5 patches as a criteria
> > > for graduation and thus have focused on working on such
> > > beginner-friendly patches so that not only do i get the required number
> > > of patches but also get familiar with the process of kernel
> > > developement.
> >
> > The LKMP internship should be done in drivers/staging/ as generally
> > coding style cleanups are NOT accepted in other parts of the kernel,
> > unless you get approval from the maintainer ahead of time.
> >
> > Does the maintainer of this driver want this to be used for the intern
> > project?
>
> In fact, Clint's changes are small and inoffensive enough, I wouldn't
> mind having them applied to dummy-hcd.
>
> However, Greg is perfectly right that this kind of stylistic update is
> not something that should be submitted for most parts of the kernel. It
> just bulks up the Git history with essentially meaningless cruft, making
> it all that much harder to see the changes that really matter. That's
> part of the reason for the suggestion that interns and beginners should
> confine their efforts to drivers/staging.
>
> Also, remember that trivial changes like this are fine for learning the
> procedure of submitting kernel patches, but the effects they have on the
> kernel itself are minimal. A patch that actually fixes a bug or adds a
> functional enhancement would be a different story.
FWIW, many years ago, I threw together a collection of kernel
"cleanup" scripts that scanned the kernel source for various possible
"improvements". Earlier this year, I posted a number of suggestions
for cleanup work to the kernel janitors mailing, and slapped together
a wiki page that described some obvious cleanups:
https://crashcourse.ca/doku/doku.php?id=linux_kernel_cleanup
As the first example on that page, look for code that insists on
manually calculating the length of an array when the appropriate macro
has been defined in include/linux/array_size.h for years. There's
still some of that lying around that could be tidied up; for example,
scan the drivers/ directory (do not make fun of my hacky solutions):
$ grep -Er "sizeof ?\(?([^\)]+)\)? ?/ ?sizeof ?\(?.*\1.*" drivers
drivers/hid/bpf/progs/hid_bpf_helpers.h:#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
drivers/media/common/tveeprom.c: (i < sizeof(array) / sizeof(char *) ? array[i] : "unknown")
drivers/net/ethernet/mellanox/mlx4/qp.c: for (k = MLX4_QP_TABLE_ZONE_RSS + 1; k < sizeof(*bitmap)/sizeof((*bitmap)[0]);
drivers/net/ethernet/intel/i40e/i40e_adminq.h: if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
drivers/net/ethernet/intel/iavf/iavf_adminq.h: if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
drivers/gpu/drm/xe/xe_guc_hxg_helpers.h:#define hxg_sizeof(T) (sizeof(T) / sizeof(u32) + BUILD_BUG_ON_ZERO(sizeof(T) % sizeof(u32)))
drivers/gpu/drm/nouveau/nvif/fifo.c: a->m.count = sizeof(a->v) / sizeof(a->v.runlists);
drivers/gpu/drm/amd/display/dc/mpc/dcn30/dcn30_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/mpc/dcn20/dcn20_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix) / sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dce110/dce110_opp_csc_v.c: int arr_size = sizeof(input_csc_matrix)/sizeof(struct input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c: table_size = sizeof(qp_table_##mode##_##bpc##bpc_##max)/sizeof(*qp_table_##mode##_##bpc##bpc_##max); \
drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c: for (k = 0; k < sizeof(wb_arb_params->cli_watermark)/sizeof(wb_arb_params->cli_watermark[0]); k++) {
drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
There are other ideas on that wiki page for someone who wanted
something simple to start with that aren't just aesthetic, they
would actually improve the quality and readability of the code.
rday
On 12/2/25 11:28, Robert P. J. Day wrote:
> On Tue, 2 Dec 2025, Alan Stern wrote:
>
>> On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
>>> On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
>>>> As part of my LKMP mentorship i have to complete 5 patches as a criteria
>>>> for graduation and thus have focused on working on such
>>>> beginner-friendly patches so that not only do i get the required number
>>>> of patches but also get familiar with the process of kernel
>>>> developement.
>>>
>>> The LKMP internship should be done in drivers/staging/ as generally
>>> coding style cleanups are NOT accepted in other parts of the kernel,
>>> unless you get approval from the maintainer ahead of time.
>>>>>> Does the maintainer of this driver want this to be used for the intern
>>> project?
>>
>> In fact, Clint's changes are small and inoffensive enough, I wouldn't
>> mind having them applied to dummy-hcd.
>>
>> However, Greg is perfectly right that this kind of stylistic update is
>> not something that should be submitted for most parts of the kernel. It
>> just bulks up the Git history with essentially meaningless cruft, making
>> it all that much harder to see the changes that really matter. That's
>> part of the reason for the suggestion that interns and beginners should
>> confine their efforts to drivers/staging.
>>
>> Also, remember that trivial changes like this are fine for learning the
>> procedure of submitting kernel patches, but the effects they have on the
>> kernel itself are minimal. A patch that actually fixes a bug or adds a
>> functional enhancement would be a different story.
>
> FWIW, many years ago, I threw together a collection of kernel
> "cleanup" scripts that scanned the kernel source for various possible
> "improvements". Earlier this year, I posted a number of suggestions
> for cleanup work to the kernel janitors mailing, and slapped together
> a wiki page that described some obvious cleanups:
>
> https://crashcourse.ca/doku/doku.php?id=linux_kernel_cleanup
>
> As the first example on that page, look for code that insists on
> manually calculating the length of an array when the appropriate macro
> has been defined in include/linux/array_size.h for years. There's
> still some of that lying around that could be tidied up; for example,
> scan the drivers/ directory (do not make fun of my hacky solutions):
>
> $ grep -Er "sizeof ?\(?([^\)]+)\)? ?/ ?sizeof ?\(?.*\1.*" drivers
> drivers/hid/bpf/progs/hid_bpf_helpers.h:#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> drivers/media/common/tveeprom.c: (i < sizeof(array) / sizeof(char *) ? array[i] : "unknown")
> drivers/net/ethernet/mellanox/mlx4/qp.c: for (k = MLX4_QP_TABLE_ZONE_RSS + 1; k < sizeof(*bitmap)/sizeof((*bitmap)[0]);
> drivers/net/ethernet/intel/i40e/i40e_adminq.h: if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
> drivers/net/ethernet/intel/iavf/iavf_adminq.h: if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
> drivers/gpu/drm/xe/xe_guc_hxg_helpers.h:#define hxg_sizeof(T) (sizeof(T) / sizeof(u32) + BUILD_BUG_ON_ZERO(sizeof(T) % sizeof(u32)))
> drivers/gpu/drm/nouveau/nvif/fifo.c: a->m.count = sizeof(a->v) / sizeof(a->v.runlists);
> drivers/gpu/drm/amd/display/dc/mpc/dcn30/dcn30_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/mpc/dcn20/dcn20_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c: int arr_size = sizeof(dpp_input_csc_matrix) / sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dce110/dce110_opp_csc_v.c: int arr_size = sizeof(input_csc_matrix)/sizeof(struct input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c: table_size = sizeof(qp_table_##mode##_##bpc##bpc_##max)/sizeof(*qp_table_##mode##_##bpc##bpc_##max); \
> drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c: for (k = 0; k < sizeof(wb_arb_params->cli_watermark)/sizeof(wb_arb_params->cli_watermark[0]); k++) {
> drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
>
> There are other ideas on that wiki page for someone who wanted
> something simple to start with that aren't just aesthetic, they
> would actually improve the quality and readability of the code.
>
> rday
Hey Rday,
Wow, this is interesting. I did something similar when I was a mentee in
the LKMP. I am a co-mentor now for the program. Would you say that
maintainers regard the patches that would come from fixing these style
issues as fundamentally different than the ones Clint attempted to fix?
Also, are your scripts open for anyone to use, including future mentees?
Greg, if mentees fixed the ones that Rday's script cause, would you
still prefer that those be confined to mainly drivers/staging?
Thanks,
David Hunter
On 12/1/25 15:37, Clint George wrote: > This patch series focuses on addressing various coding style issues in > the dummy_hcd USB gadget driver. The changes includes adding relevant > comments, improving readability, and ensuring consistency with kernel > coding conventions. > > Clint George (6): > usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444) > usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned' > usb: gadget: dummy_hcd: document ISO endpoint allocation pattern > usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr > usb: gadget: dummy_hcd: remove unnecessary parentheses > usb: gadget: dummy_hcd: move function braces > > drivers/usb/gadget/udc/dummy_hcd.c | 52 ++++++++++++------------------ > 1 file changed, 21 insertions(+), 31 deletions(-) > > --- > > Testing: > - Ran compiler testing with no new warnings detected. > - Ensured the module builds and inserts cleanly without issues. > - Used Static Analysis tools to confirm no new issues were introduced. > > Please review the changes and let me know if any modifications > or further testing of the module is required. > > As part of my LKMP mentorship i have to complete 5 patches as a criteria > for graduation and thus have focused on working on such > beginner-friendly patches so that not only do i get the required number > of patches but also get familiar with the process of kernel > developement. Thus, while this patch series doesn't address the max_stream value > exceeding problem that triggers the BUG() API, i will take some time to > dig deeper and truly understand the problem and fix it and not just > paper-over the problem. > > Again, i am very grateful for your feedback greg and alan to guide a > beginner like me. > > Thanks, > Clint > Hey Clint, In general, when submitting version 2 of a patch series, most maintainers will prefer that you send it as a new patch series and not as a reply. I usually recommend putting the links to the respective previous versions in the change log. Thanks, David Hunter
© 2016 - 2026 Red Hat, Inc.