Re: [PATCH 2/2] staging: rtl8192u: Corrects 'Avoid CamelCase' for variables

From: Joe Perches
Date: Wed Mar 18 2020 - 13:59:04 EST


On Wed, 2020-03-18 at 14:31 -0300, Camylla Cantanheide wrote:
> Dear Dan Carpenter and Joe Perches,
>
> Thank you very much for the suggestions, I found the evaluation of both
> very significant. Now, I have another perspective on variables.
>
> I solved the problem for the *setKey *function, however, when executing the
> checkpatch, more *Checks* of the same type appeared.
>
> Should I send the second version of the patch, only to the *setKey*
> function or do I resolve all *Checks* for the entire file?

A single patch refactoring the function would be fine.

Also perhaps a name change as a separate patch later as
setKey is a relatively generic name and not obviously
specific to the rtl8192u..

Perhaps a first refactoring patch like the below
then a renaming one.

(untested)
---
drivers/staging/rtl8192u/r8192U_core.c | 50 ++++++++++++++++------------------
1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index 89dd1fb..0c0cb08 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -4880,7 +4880,7 @@ void EnableHWSecurityConfig8192(struct net_device *dev)
void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
u8 *MacAddr, u8 DefaultKey, u32 *KeyContent)
{
- u32 TargetCommand = 0;
+ u32 TargetCommand = CAM_CONTENT_COUNT * EntryNo | BIT(31) | BIT(16);
u32 TargetContent = 0;
u16 usConfig = 0;
u8 i;
@@ -4897,32 +4897,28 @@ void setKey(struct net_device *dev, u8 EntryNo, u8 KeyIndex, u16 KeyType,
else
usConfig |= BIT(15) | (KeyType << 2) | KeyIndex;

- for (i = 0; i < CAM_CONTENT_COUNT; i++) {
- TargetCommand = i + CAM_CONTENT_COUNT * EntryNo;
- TargetCommand |= BIT(31) | BIT(16);
-
- if (i == 0) { /* MAC|Config */
- TargetContent = (u32)(*(MacAddr + 0)) << 16 |
- (u32)(*(MacAddr + 1)) << 24 |
- (u32)usConfig;
-
- write_nic_dword(dev, WCAMI, TargetContent);
- write_nic_dword(dev, RWCAM, TargetCommand);
- } else if (i == 1) { /* MAC */
- TargetContent = (u32)(*(MacAddr + 2)) |
- (u32)(*(MacAddr + 3)) << 8 |
- (u32)(*(MacAddr + 4)) << 16 |
- (u32)(*(MacAddr + 5)) << 24;
- write_nic_dword(dev, WCAMI, TargetContent);
- write_nic_dword(dev, RWCAM, TargetCommand);
- } else {
- /* Key Material */
- if (KeyContent) {
- write_nic_dword(dev, WCAMI,
- *(KeyContent + i - 2));
- write_nic_dword(dev, RWCAM, TargetCommand);
- }
- }
+ TargetContent = MacAddr[0] << 16 |
+ MacAddr[0] << 24 |
+ (u32)usConfig;
+
+ write_nic_dword(dev, WCAMI, TargetContent);
+ write_nic_dword(dev, RWCAM, TargetCommand++);
+
+ /* MAC */
+ TargetContent = MacAddr[2] |
+ MacAddr[3] << 8 |
+ MacAddr[4] << 16 |
+ MacAddr[5] << 24;
+ write_nic_dword(dev, WCAMI, TargetContent);
+ write_nic_dword(dev, RWCAM, TargetCommand++);
+
+ /* Key Material */
+ if (!KeyContent)
+ return;
+
+ for (i = 2; i < CAM_CONTENT_COUNT; i++) {
+ write_nic_dword(dev, WCAMI, *KeyContent++);
+ write_nic_dword(dev, RWCAM, TargetCommand++);
}
}