drivers/staging/r8188eu/core/rtw_cmd.c | 145 +++++++---------- drivers/staging/r8188eu/core/rtw_mlme.c | 179 +++++++++------------ drivers/staging/r8188eu/include/rtw_mlme.h | 4 +- 3 files changed, 135 insertions(+), 193 deletions(-)
Drop some unnecessary wrappers and update all the references accordingly. Tested on Comfast CF-WU810N RTL8188EUS wireless adapter. v1 -> v2: Drop the wrapper functions with underscores prefixed. Vihas Makwana (7): staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd drivers/staging/r8188eu/core/rtw_cmd.c | 145 +++++++---------- drivers/staging/r8188eu/core/rtw_mlme.c | 179 +++++++++------------ drivers/staging/r8188eu/include/rtw_mlme.h | 4 +- 3 files changed, 135 insertions(+), 193 deletions(-) -- 2.30.2
On Mon, Apr 11, 2022 at 03:51:29PM +0530, Vihas Makwana wrote: > Drop some unnecessary wrappers and update all the references > accordingly. > Tested on Comfast CF-WU810N RTL8188EUS wireless adapter. > > v1 -> v2: > Drop the wrapper functions with underscores prefixed. > > Vihas Makwana (7): > staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv > staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv > staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network > staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd > Thanks! Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> regards, dan carpenter
On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote: > Drop some unnecessary wrappers and update all the references > accordingly. > Tested on Comfast CF-WU810N RTL8188EUS wireless adapter. > > v1 -> v2: > Drop the wrapper functions with underscores prefixed. > > Vihas Makwana (7): > staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv > staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv > staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv > staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network > staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd > > drivers/staging/r8188eu/core/rtw_cmd.c | 145 +++++++---------- > drivers/staging/r8188eu/core/rtw_mlme.c | 179 +++++++++------------ > drivers/staging/r8188eu/include/rtw_mlme.h | 4 +- > 3 files changed, 135 insertions(+), 193 deletions(-) > > -- > 2.30.2 > Formally, you are removing the wrapped functions (or helpers, if you prefer) by moving their code into the wrappers. To say that you are removing the wrappers is not correct. However, I'm sure this kind of formal mistake won't prevent your patches to be accepted. I wanted to point this out to you in case you decide to submit similar patches in the future and perhaps want to describe your changes a little more accurately. Thanks, Fabio M. De Francesco
On Mon, Apr 11, 2022 at 10:34:44PM +0200, Fabio M. De Francesco wrote: > On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote: > > Drop some unnecessary wrappers and update all the references > > accordingly. > > Tested on Comfast CF-WU810N RTL8188EUS wireless adapter. > > > > v1 -> v2: > > Drop the wrapper functions with underscores prefixed. > > > > Vihas Makwana (7): > > staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > > staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv > > staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv > > staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv > > staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv > > staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network > > staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd > > > > drivers/staging/r8188eu/core/rtw_cmd.c | 145 +++++++---------- > > drivers/staging/r8188eu/core/rtw_mlme.c | 179 +++++++++------------ > > drivers/staging/r8188eu/include/rtw_mlme.h | 4 +- > > 3 files changed, 135 insertions(+), 193 deletions(-) > > > > -- > > 2.30.2 > > > Formally, you are removing the wrapped functions (or helpers, if you > prefer) by moving their code into the wrappers. To say that you are > removing the wrappers is not correct. I once had someone make me re-write a commit message four time just as a kind of bullying and then at the end he was like, "You said NULL dereference instead of NULL pointer dereference so I had to re-write the commit message and I added some comment to the kernel git log explaining how you suck." So these days I have made it a rule that if you're going to complain about commit messages then you have to write your own for people to cut and paste. Otherwise people are like, "You're too stupid to read my mind. LOL. Do it again." But in this case the commit message is fine. The key things with a commit message are: 1) What's the motivation 2) What's the effect for the user 3) Are all the surprising aspects are explained. Do I have enough information to review it quickly. Removing wrappers is the motivation. No need to explain that further. No effects for the user. There were no surprising bits. It's fine. regards, dan carpenter
On martedì 12 aprile 2022 07:06:30 CEST Dan Carpenter wrote: > On Mon, Apr 11, 2022 at 10:34:44PM +0200, Fabio M. De Francesco wrote: > > On lunedì 11 aprile 2022 12:21:29 CEST Vihas Makwana wrote: > > > Drop some unnecessary wrappers and update all the references > > > accordingly. > > > Tested on Comfast CF-WU810N RTL8188EUS wireless adapter. > > > > > > v1 -> v2: > > > Drop the wrapper functions with underscores prefixed. > > > > > > Vihas Makwana (7): > > > staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > > > staging: r8188eu: drop unnecessary wrapper _rtw_init_cmd_priv > > > staging: r8188eu: drop unnecessary wrapper _rtw_init_evt_priv > > > staging: r8188eu: drop unnecessary wrapper _rtw_init_mlme_priv > > > staging: r8188eu: drop unnecessary wrapper _rtw_free_mlme_priv > > > staging: r8188eu: drop unnecessary wrapper _rtw_alloc_network > > > staging: r8188eu: drop unnecessary wrapper _rtw_dequeue_cmd > > > > > > drivers/staging/r8188eu/core/rtw_cmd.c | 145 +++++++---------- > > > drivers/staging/r8188eu/core/rtw_mlme.c | 179 +++++++++------------ > > > drivers/staging/r8188eu/include/rtw_mlme.h | 4 +- > > > 3 files changed, 135 insertions(+), 193 deletions(-) > > > > > > -- > > > 2.30.2 > > > > > Formally, you are removing the wrapped functions (or helpers, if you > > prefer) by moving their code into the wrappers. To say that you are > > removing the wrappers is not correct. > > I once had someone make me re-write a commit message four time just as a > kind of bullying and then at the end he was like, "You said NULL > dereference instead of NULL pointer dereference so I had to re-write the > commit message and I added some comment to the kernel git log explaining > how you suck." > So these days I have made it a rule that if you're going > to complain about commit messages then you have to write your own for > people to cut and paste. Otherwise people are like, "You're too stupid > to read my mind. LOL. Do it again." I'm assuming you don't believe that I attempted some kind of bullying similar to what you had to face when you sent the above-mentioned patch. Nevertheless, I was expecting some sort of reaction from you but I think that you are misunderstanding what my intentions were. I didn't suggest a re-write of the commit messages. I just pointed out that those messages are formally inaccurate but that these kinds of small formal inaccuracies won't ever prevent Vihas' patches from being accepted as they are. It's only that, IMO, if people start to take care of being formally correct in drivers/staging, even when they do simple changes, they train their minds to improve their abilities to communicate in the future when they eventually decide to submit much more complex patches in other subsystems. > But in this case the commit message is fine. The key things with a > commit message are: > > 1) What's the motivation > 2) What's the effect for the user > 3) Are all the surprising aspects are explained. Do I have enough > information to review it quickly. > > Removing wrappers is the motivation. No need to explain that further. > No effects for the user. > There were no surprising bits. > > It's fine. I agree with you: "it's fine". I respect your opinion and your pragmatism: after all, here, everything is pretty clear, so why bother? However, writing accurate and formally correct commit messages takes the same amount of time of writing inaccurate messages. So, why not do better next time? Again, I don't suggest to do so now. Not for these patches, because it would only be a loss of precious time. Thanks, Fabio M. De Francesco > > regards, > dan carpenter > >
On Tue, Apr 12, 2022 at 11:53:42AM +0200, Fabio M. De Francesco wrote: > I didn't suggest a re-write of the commit messages. Yes. That's the problem. If you want to complain about a commit message then you *should* suggest how you would re-write it. Otherwise how are we supposed to read your mind? > I just pointed out that those messages are formally inaccurate but > that these kinds of small formal inaccuracies I'm so puzzled but what you mean here. There were no "small formal inaccuracies". The patch removed unnecessary wrappers exactly as described. Anyway, I haven't seen your proposed commit message so I can't say. regards, dan carpenter
On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote: > On Tue, Apr 12, 2022 at 11:53:42AM +0200, Fabio M. De Francesco wrote: > > I didn't suggest a re-write of the commit messages. > > Yes. That's the problem. If you want to complain about a commit > message then you *should* suggest how you would re-write it. Otherwise > how are we supposed to read your mind? Ah, sorry. I had missed that you were asking for a re-write (literally). Here it is a sort of diff on Vihas' patch: - [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv + [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv() - Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to - rtw_free_cmd_priv. + Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and + move its code to the wrapper rtw_free_cmd_priv(). > > I just pointed out that those messages are formally inaccurate but > > that these kinds of small formal inaccuracies > > I'm so puzzled but what you mean here. There were no "small formal > inaccuracies". The patch removed unnecessary wrappers exactly as > described. The patch removed unnecessary helpers. > Anyway, I haven't seen your proposed commit message so I > can't say. > > regards, > dan carpenter Thanks, Fabio M. De Francesco
On Tue, Apr 12, 2022 at 06:08:24PM +0200, Fabio M. De Francesco wrote: > On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote: > > On Tue, Apr 12, 2022 at 11:53:42AM +0200, Fabio M. De Francesco wrote: > > > I didn't suggest a re-write of the commit messages. > > > > Yes. That's the problem. If you want to complain about a commit > > message then you *should* suggest how you would re-write it. Otherwise > > how are we supposed to read your mind? > > Ah, sorry. I had missed that you were asking for a re-write (literally). > > Here it is a sort of diff on Vihas' patch: > > - [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > + [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv() > > - Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to > - rtw_free_cmd_priv. > + Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and > + move its code to the wrapper rtw_free_cmd_priv(). > I kind of feel like the original is better. Why would we want to remove helper functions? Helper functions are ok. What we don't like are pointless wrapper functions. The whole motivation for the patch is to remove wrapper functions and that's what it does. regards, dan carpenter
Did this patch series get lost? just pinging :) On Wed, Apr 13, 2022 at 11:13 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Tue, Apr 12, 2022 at 06:08:24PM +0200, Fabio M. De Francesco wrote: > > On marted? 12 aprile 2022 17:15:29 CEST Dan Carpenter wrote: > > > On Tue, Apr 12, 2022 at 11:53:42AM +0200, Fabio M. De Francesco wrote: > > > > I didn't suggest a re-write of the commit messages. > > > > > > Yes. That's the problem. If you want to complain about a commit > > > message then you *should* suggest how you would re-write it. Otherwise > > > how are we supposed to read your mind? > > > > Ah, sorry. I had missed that you were asking for a re-write (literally). > > > > Here it is a sort of diff on Vihas' patch: > > > > - [PATCH v2 1/7] staging: r8188eu: drop unnecessary wrapper _rtw_free_cmd_priv > > + [PATCH v2 1/7] staging: r8188eu: drop unnecessary helper _rtw_free_cmd_priv() > > > > - Drop unnecessary wrapper _rtw_free_cmd_priv and move its logic to > > - rtw_free_cmd_priv. > > + Drop unnecessary helper (wrapped function) _rtw_free_cmd_priv() and > > + move its code to the wrapper rtw_free_cmd_priv(). > > > > I kind of feel like the original is better. Why would we want to remove > helper functions? Helper functions are ok. What we don't like are > pointless wrapper functions. The whole motivation for the patch is to > remove wrapper functions and that's what it does. > > regards, > dan carpenter > -- Thanks, Vihas
On Fri, Apr 22, 2022 at 12:26:21AM +0530, Vihas Makwana wrote: > Did this patch series get lost? What series? (hint, do not top post). My queue of staging patches was empty as of a few days ago (it's filled up since then), so if they were not applied, please resend. thanks, greg k-h
On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote: > On Fri, Apr 22, 2022 at 12:26:21AM +0530, Vihas Makwana wrote: > > Did this patch series get lost? > > What series? (hint, do not top post). > > My queue of staging patches was empty as of a few days ago (it's filled > up since then), so if they were not applied, please resend. Are you also talking about re-sending patches for media/atomisp? I submitted three patches for that driver between April 8-12. They have already been reviewed and tested by Ira Weiny and Hans de Goede. However, if I remember correctly, they should take a different path and it should be up to Mauro C. Chehab to apply them. Do you want them to be sent again? Thanks, Fabio M. De Francesco
On Fri, Apr 22, 2022 at 11:21:21AM +0200, Fabio M. De Francesco wrote: > On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote: > > On Fri, Apr 22, 2022 at 12:26:21AM +0530, Vihas Makwana wrote: > > > Did this patch series get lost? > > > > What series? (hint, do not top post). > > > > My queue of staging patches was empty as of a few days ago (it's filled > > up since then), so if they were not applied, please resend. > > Are you also talking about re-sending patches for media/atomisp? Heh. No. No one was discussing your patches or anything to do with media. These are for staging: r8188eu. Vihas, please put the subsystem prefix in the PATCH 0/7 email next time. regards, dan carpenter
On venerdì 22 aprile 2022 11:29:15 CEST Dan Carpenter wrote: > On Fri, Apr 22, 2022 at 11:21:21AM +0200, Fabio M. De Francesco wrote: > > On venerdì 22 aprile 2022 08:10:23 CEST Greg Kroah-Hartman wrote: > > > On Fri, Apr 22, 2022 at 12:26:21AM +0530, Vihas Makwana wrote: > > > > Did this patch series get lost? > > > > > > What series? (hint, do not top post). > > > > > > My queue of staging patches was empty as of a few days ago (it's filled > > > up since then), so if they were not applied, please resend. > > > > Are you also talking about re-sending patches for media/atomisp? > > Heh. No. No one was discussing your patches or anything to do with > media. These are for staging: r8188eu. Dan, got it now :) I was confused by Greg's words talking about "My queue of staging..." . Now you confirm that media/atomisp has never been in his queue. Thanks, Fabio M. De Francesco > Vihas, please put the subsystem prefix in the PATCH 0/7 email next time. > > regards, > dan carpenter > >
On Fri, Apr 22, 2022 at 12:26:21AM +0530, Vihas Makwana wrote: > Did this patch series get lost? > just pinging :) > Wait for at least two weeks (not counting merge windows) before asking for feedback. regards, dan carpenter
© 2016 - 2026 Red Hat, Inc.