2022-09-11 10:48:44

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 00/12] staging: vt6655: Cleanup and rename functions of mac.h

Rename functions and local variables to avoid CamelCase which is not
accepted by checkpatch.pl. Remove unnecessary declaration of functions
and make functions static when possible. Change declaration of local
variables to shorten code and remove unnecessary line breaks.

Tested with vt6655 on mini PCI Module
Transferred this patch over wlan connection of vt6655

Philipp Hortmann (12):
staging: vt6655: Cleanup and rename function MACvSetLoopbackMode
staging: vt6655: Cleanup and rename function MACvSaveContext
staging: vt6655: Cleanup and rename function MACvRestoreContext
staging: vt6655: Cleanup and rename function MACbSafeSoftwareReset
staging: vt6655: Rename function MACbSafeRxOff
staging: vt6655: Rename function MACbSafeTxOff
staging: vt6655: Rename function MACbSafeStop
staging: vt6655: Rename function MACvSetCurrRx0DescAddr
staging: vt6655: Rename function MACvSetCurrRx1DescAddr
staging: vt6655: Cleanup and rename function MACvSetCurrTXDescAddr
staging: vt6655: Rename function MACvSetCurrTx0DescAddrEx
staging: vt6655: Rename function MACvSetCurrAC0DescAddrEx

drivers/staging/vt6655/card.c | 8 ++--
drivers/staging/vt6655/mac.c | 85 ++++++++++++++++-------------------
drivers/staging/vt6655/mac.h | 22 ++-------
3 files changed, 46 insertions(+), 69 deletions(-)

--
2.37.3


2022-09-11 11:06:45

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 11/12] staging: vt6655: Rename function MACvSetCurrTx0DescAddrEx

Rename function MACvSetCurrTx0DescAddrEx to vt6655_mac_set_curr_tx_0_...
to avoid CamelCase which is not accepted by checkpatch.pl. Remove
unnecessary declaration of function and make function static. Remove
unnecessary line break.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/mac.c | 5 ++---
drivers/staging/vt6655/mac.h | 2 --
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index 0ff98468b2e0..2fbee8508f0e 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -595,8 +595,7 @@ void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_
* Return Value: none
*
*/
-void MACvSetCurrTx0DescAddrEx(struct vnt_private *priv,
- u32 curr_desc_addr)
+static void vt6655_mac_set_curr_tx_0_desc_addr_ex(struct vnt_private *priv, u32 curr_desc_addr)
{
void __iomem *io_base = priv->port_offset;
unsigned short ww;
@@ -658,7 +657,7 @@ void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32
if (tx_type == TYPE_AC0DMA)
MACvSetCurrAC0DescAddrEx(priv, curr_desc_addr);
else if (tx_type == TYPE_TXDMA0)
- MACvSetCurrTx0DescAddrEx(priv, curr_desc_addr);
+ vt6655_mac_set_curr_tx_0_desc_addr_ex(priv, curr_desc_addr);
}

/*
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 0224f710d603..3fbb891ac57c 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -559,8 +559,6 @@ void MACvInitialize(struct vnt_private *priv);
void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32 curr_desc_addr);
-void MACvSetCurrTx0DescAddrEx(struct vnt_private *priv,
- u32 curr_desc_addr);
void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
u32 curr_desc_addr);
void MACvSetCurrSyncDescAddrEx(struct vnt_private *priv,
--
2.37.3

2022-09-11 11:08:13

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 06/12] staging: vt6655: Rename function MACbSafeTxOff

Rename function MACbSafeTxOff to vt6655_mac_safe_tx_off to avoid
CamelCase which is not accepted by checkpatch.pl. Remove unnecessary
declaration of function and make function static.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/mac.c | 8 ++++----
drivers/staging/vt6655/mac.h | 1 -
2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index e0216d320235..24851fe53683 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -18,7 +18,7 @@
* vt6655_mac_restore_context - Restore Context of MAC Registers
* MACbSoftwareReset - Software Reset MAC
* vt6655_mac_safe_rx_off - Turn Off MAC Rx
- * MACbSafeTxOff - Turn Off MAC Tx
+ * vt6655_mac_safe_tx_off - Turn Off MAC Tx
* MACbSafeStop - Stop MAC function
* MACbShutdown - Shut down MAC
* MACvInitialize - Initialize MAC
@@ -376,7 +376,7 @@ static bool vt6655_mac_safe_rx_off(struct vnt_private *priv)
* Return Value: true if success; otherwise false
*
*/
-bool MACbSafeTxOff(struct vnt_private *priv)
+static bool vt6655_mac_safe_tx_off(struct vnt_private *priv)
{
void __iomem *io_base = priv->port_offset;
unsigned short ww;
@@ -443,8 +443,8 @@ bool MACbSafeStop(struct vnt_private *priv)
vt6655_mac_save_soft_reset(priv);
return false;
}
- if (!MACbSafeTxOff(priv)) {
- pr_debug(" MACbSafeTxOff == false)\n");
+ if (!vt6655_mac_safe_tx_off(priv)) {
+ pr_debug(" vt6655_mac_safe_tx_off == false)\n");
vt6655_mac_save_soft_reset(priv);
return false;
}
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index e6ff860c1bfa..12b4f8937d14 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -554,7 +554,6 @@ void vt6655_mac_set_short_retry_limit(struct vnt_private *priv, unsigned char re
void MACvSetLongRetryLimit(struct vnt_private *priv, unsigned char byRetryLimit);

bool MACbSoftwareReset(struct vnt_private *priv);
-bool MACbSafeTxOff(struct vnt_private *priv);
bool MACbSafeStop(struct vnt_private *priv);
bool MACbShutdown(struct vnt_private *priv);
void MACvInitialize(struct vnt_private *priv);
--
2.37.3

2022-09-11 11:09:57

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 10/12] staging: vt6655: Cleanup and rename function MACvSetCurrTXDescAddr

Rename function MACvSetCurrTXDescAddr to vt6655_mac_set_curr_tx_desc_addr
and iTxType to tx_type to avoid CamelCase which is not accepted by
checkpatch.pl. Remove unnecessary line break.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/card.c | 4 ++--
drivers/staging/vt6655/mac.c | 7 +++----
drivers/staging/vt6655/mac.h | 3 +--
3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
index d137b4b45e3b..c680925b9c92 100644
--- a/drivers/staging/vt6655/card.c
+++ b/drivers/staging/vt6655/card.c
@@ -409,9 +409,9 @@ void CARDvSafeResetTx(struct vnt_private *priv)
}

/* set MAC TD pointer */
- MACvSetCurrTXDescAddr(TYPE_TXDMA0, priv, priv->td0_pool_dma);
+ vt6655_mac_set_curr_tx_desc_addr(TYPE_TXDMA0, priv, priv->td0_pool_dma);

- MACvSetCurrTXDescAddr(TYPE_AC0DMA, priv, priv->td1_pool_dma);
+ vt6655_mac_set_curr_tx_desc_addr(TYPE_AC0DMA, priv, priv->td1_pool_dma);

/* set MAC Beacon TX pointer */
iowrite32((u32)priv->tx_beacon_dma, priv->port_offset + MAC_REG_BCNDMAPTR);
diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index d6614be79e39..0ff98468b2e0 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -653,12 +653,11 @@ void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
iowrite8(DMACTL_RUN, io_base + MAC_REG_AC0DMACTL);
}

-void MACvSetCurrTXDescAddr(int iTxType, struct vnt_private *priv,
- u32 curr_desc_addr)
+void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32 curr_desc_addr)
{
- if (iTxType == TYPE_AC0DMA)
+ if (tx_type == TYPE_AC0DMA)
MACvSetCurrAC0DescAddrEx(priv, curr_desc_addr);
- else if (iTxType == TYPE_TXDMA0)
+ else if (tx_type == TYPE_TXDMA0)
MACvSetCurrTx0DescAddrEx(priv, curr_desc_addr);
}

diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index fff9dc72e2c0..0224f710d603 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -558,8 +558,7 @@ bool MACbShutdown(struct vnt_private *priv);
void MACvInitialize(struct vnt_private *priv);
void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
-void MACvSetCurrTXDescAddr(int iTxType, struct vnt_private *priv,
- u32 curr_desc_addr);
+void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32 curr_desc_addr);
void MACvSetCurrTx0DescAddrEx(struct vnt_private *priv,
u32 curr_desc_addr);
void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
--
2.37.3

2022-09-11 11:10:00

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 08/12] staging: vt6655: Rename function MACvSetCurrRx0DescAddr

Rename function MACvSetCurrRx0DescAddr to vt6655_mac_set_curr_rx_0_desc...
to avoid CamelCase which is not accepted by checkpatch.pl. Remove
unnecessary line break.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/card.c | 2 +-
drivers/staging/vt6655/mac.c | 2 +-
drivers/staging/vt6655/mac.h | 3 +--
3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
index 1b2ba6793ead..dc39b3668c77 100644
--- a/drivers/staging/vt6655/card.c
+++ b/drivers/staging/vt6655/card.c
@@ -458,7 +458,7 @@ void CARDvSafeResetRx(struct vnt_private *priv)
iowrite32(RX_PERPKT, priv->port_offset + MAC_REG_RXDMACTL0);
iowrite32(RX_PERPKT, priv->port_offset + MAC_REG_RXDMACTL1);
/* set MAC RD pointer */
- MACvSetCurrRx0DescAddr(priv, priv->rd0_pool_dma);
+ vt6655_mac_set_curr_rx_0_desc_addr(priv, priv->rd0_pool_dma);

MACvSetCurrRx1DescAddr(priv, priv->rd1_pool_dma);
}
diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index e1f639787316..e88536705d23 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -527,7 +527,7 @@ void MACvInitialize(struct vnt_private *priv)
* Return Value: none
*
*/
-void MACvSetCurrRx0DescAddr(struct vnt_private *priv, u32 curr_desc_addr)
+void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr)
{
void __iomem *io_base = priv->port_offset;
unsigned short ww;
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index c6147a4f563e..b092e59a5b98 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -556,8 +556,7 @@ void MACvSetLongRetryLimit(struct vnt_private *priv, unsigned char byRetryLimit)
bool MACbSoftwareReset(struct vnt_private *priv);
bool MACbShutdown(struct vnt_private *priv);
void MACvInitialize(struct vnt_private *priv);
-void MACvSetCurrRx0DescAddr(struct vnt_private *priv,
- u32 curr_desc_addr);
+void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void MACvSetCurrRx1DescAddr(struct vnt_private *priv,
u32 curr_desc_addr);
void MACvSetCurrTXDescAddr(int iTxType, struct vnt_private *priv,
--
2.37.3

2022-09-11 11:13:14

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 02/12] staging: vt6655: Cleanup and rename function MACvSaveContext

Rename function MACvSaveContext to vt6655_mac_save_context to avoid
CamelCase which is not accepted by checkpatch.pl. Remove unnecessary
declaration of function and make function static. Change declaration of
cxt_buf to shorten code.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/mac.c | 6 +++---
drivers/staging/vt6655/mac.h | 1 -
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index 567bc38ecfa9..092b1fffcfa1 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -14,7 +14,7 @@
* vt6655_mac_set_short_retry_limit - Set 802.11 Short Retry limit
* MACvSetLongRetryLimit - Set 802.11 Long Retry limit
* vt6655_mac_set_loopback_mode - Set MAC Loopback Mode
- * MACvSaveContext - Save Context of MAC Registers
+ * vt6655_mac_save_context - Save Context of MAC Registers
* MACvRestoreContext - Restore Context of MAC Registers
* MACbSoftwareReset - Software Reset MAC
* MACbSafeRxOff - Turn Off MAC Rx
@@ -181,7 +181,7 @@ static void vt6655_mac_set_loopback_mode(struct vnt_private *priv, u8 loopback_m
* Return Value: none
*
*/
-void MACvSaveContext(struct vnt_private *priv, unsigned char *cxt_buf)
+static void vt6655_mac_save_context(struct vnt_private *priv, u8 *cxt_buf)
{
void __iomem *io_base = priv->port_offset;

@@ -303,7 +303,7 @@ bool MACbSafeSoftwareReset(struct vnt_private *priv)
* reset, then restore register's value
*/
/* save MAC context */
- MACvSaveContext(priv, abyTmpRegData);
+ vt6655_mac_save_context(priv, abyTmpRegData);
/* do reset */
bRetVal = MACbSoftwareReset(priv);
/* restore MAC context, except CR0 */
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index f7d00a251677..1752905d7df0 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -553,7 +553,6 @@ void vt6655_mac_set_short_retry_limit(struct vnt_private *priv, unsigned char re

void MACvSetLongRetryLimit(struct vnt_private *priv, unsigned char byRetryLimit);

-void MACvSaveContext(struct vnt_private *priv, unsigned char *cxt_buf);
void MACvRestoreContext(struct vnt_private *priv, unsigned char *cxt_buf);

bool MACbSoftwareReset(struct vnt_private *priv);
--
2.37.3

2022-09-11 11:13:15

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 09/12] staging: vt6655: Rename function MACvSetCurrRx1DescAddr

Rename function MACvSetCurrRx1DescAddr to vt6655_mac_set_curr_rx_1_desc...
to avoid CamelCase which is not accepted by checkpatch.pl. Remove
unnecessary line break.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/card.c | 2 +-
drivers/staging/vt6655/mac.c | 2 +-
drivers/staging/vt6655/mac.h | 3 +--
3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
index dc39b3668c77..d137b4b45e3b 100644
--- a/drivers/staging/vt6655/card.c
+++ b/drivers/staging/vt6655/card.c
@@ -460,7 +460,7 @@ void CARDvSafeResetRx(struct vnt_private *priv)
/* set MAC RD pointer */
vt6655_mac_set_curr_rx_0_desc_addr(priv, priv->rd0_pool_dma);

- MACvSetCurrRx1DescAddr(priv, priv->rd1_pool_dma);
+ vt6655_mac_set_curr_rx_1_desc_addr(priv, priv->rd1_pool_dma);
}

/*
diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index e88536705d23..d6614be79e39 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -561,7 +561,7 @@ void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_
* Return Value: none
*
*/
-void MACvSetCurrRx1DescAddr(struct vnt_private *priv, u32 curr_desc_addr)
+void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_addr)
{
void __iomem *io_base = priv->port_offset;
unsigned short ww;
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index b092e59a5b98..fff9dc72e2c0 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -557,8 +557,7 @@ bool MACbSoftwareReset(struct vnt_private *priv);
bool MACbShutdown(struct vnt_private *priv);
void MACvInitialize(struct vnt_private *priv);
void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
-void MACvSetCurrRx1DescAddr(struct vnt_private *priv,
- u32 curr_desc_addr);
+void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void MACvSetCurrTXDescAddr(int iTxType, struct vnt_private *priv,
u32 curr_desc_addr);
void MACvSetCurrTx0DescAddrEx(struct vnt_private *priv,
--
2.37.3

2022-09-11 11:13:15

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 12/12] staging: vt6655: Rename function MACvSetCurrAC0DescAddrEx

Rename function MACvSetCurrAC0DescAddrEx to vt6655_mac_set_curr_ac_0...
to avoid CamelCase which is not accepted by checkpatch.pl. Remove
unnecessary declaration of function and make function static. Remove
unnecessary line break.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/mac.c | 5 ++---
drivers/staging/vt6655/mac.h | 2 --
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index 2fbee8508f0e..b4ebc7d31961 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -630,8 +630,7 @@ static void vt6655_mac_set_curr_tx_0_desc_addr_ex(struct vnt_private *priv, u32
*
*/
/* TxDMA1 = AC0DMA */
-void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
- u32 curr_desc_addr)
+static void vt6655_mac_set_curr_ac_0_desc_addr_ex(struct vnt_private *priv, u32 curr_desc_addr)
{
void __iomem *io_base = priv->port_offset;
unsigned short ww;
@@ -655,7 +654,7 @@ void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32 curr_desc_addr)
{
if (tx_type == TYPE_AC0DMA)
- MACvSetCurrAC0DescAddrEx(priv, curr_desc_addr);
+ vt6655_mac_set_curr_ac_0_desc_addr_ex(priv, curr_desc_addr);
else if (tx_type == TYPE_TXDMA0)
vt6655_mac_set_curr_tx_0_desc_addr_ex(priv, curr_desc_addr);
}
diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 3fbb891ac57c..acf931c3f5fd 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -559,8 +559,6 @@ void MACvInitialize(struct vnt_private *priv);
void vt6655_mac_set_curr_rx_0_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void vt6655_mac_set_curr_rx_1_desc_addr(struct vnt_private *priv, u32 curr_desc_addr);
void vt6655_mac_set_curr_tx_desc_addr(int tx_type, struct vnt_private *priv, u32 curr_desc_addr);
-void MACvSetCurrAC0DescAddrEx(struct vnt_private *priv,
- u32 curr_desc_addr);
void MACvSetCurrSyncDescAddrEx(struct vnt_private *priv,
u32 curr_desc_addr);
void MACvSetCurrATIMDescAddrEx(struct vnt_private *priv,
--
2.37.3

2022-09-11 11:25:54

by Philipp Hortmann

[permalink] [raw]
Subject: [PATCH 04/12] staging: vt6655: Cleanup and rename function MACbSafeSoftwareReset

Rename function MACbSafeSoftwareReset to vt6655_mac_save_soft_reset and
abyTmpRegData to tmp_reg_data to avoid CamelCase which is not accepted by
checkpatch.pl. Remove return value bRetVal as it is unused by the calling
functions. Remove unnecessary declaration of function and make function
static. Change declaration of tmp_reg_data to shorten code.

Signed-off-by: Philipp Hortmann <[email protected]>
---
drivers/staging/vt6655/mac.c | 17 +++++++----------
drivers/staging/vt6655/mac.h | 1 -
2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index b1aa5fbe4430..f292a34c23dd 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -293,23 +293,20 @@ bool MACbSoftwareReset(struct vnt_private *priv)
* Return Value: true if success; otherwise false
*
*/
-bool MACbSafeSoftwareReset(struct vnt_private *priv)
+static void vt6655_mac_save_soft_reset(struct vnt_private *priv)
{
- unsigned char abyTmpRegData[MAC_MAX_CONTEXT_SIZE_PAGE0 + MAC_MAX_CONTEXT_SIZE_PAGE1];
- bool bRetVal;
+ u8 tmp_reg_data[MAC_MAX_CONTEXT_SIZE_PAGE0 + MAC_MAX_CONTEXT_SIZE_PAGE1];

/* PATCH....
* save some important register's value, then do
* reset, then restore register's value
*/
/* save MAC context */
- vt6655_mac_save_context(priv, abyTmpRegData);
+ vt6655_mac_save_context(priv, tmp_reg_data);
/* do reset */
- bRetVal = MACbSoftwareReset(priv);
+ MACbSoftwareReset(priv);
/* restore MAC context, except CR0 */
- vt6655_mac_restore_context(priv, abyTmpRegData);
-
- return bRetVal;
+ vt6655_mac_restore_context(priv, tmp_reg_data);
}

/*
@@ -443,12 +440,12 @@ bool MACbSafeStop(struct vnt_private *priv)

if (!MACbSafeRxOff(priv)) {
pr_debug(" MACbSafeRxOff == false)\n");
- MACbSafeSoftwareReset(priv);
+ vt6655_mac_save_soft_reset(priv);
return false;
}
if (!MACbSafeTxOff(priv)) {
pr_debug(" MACbSafeTxOff == false)\n");
- MACbSafeSoftwareReset(priv);
+ vt6655_mac_save_soft_reset(priv);
return false;
}

diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
index 25247b0bf039..5dd8644749ec 100644
--- a/drivers/staging/vt6655/mac.h
+++ b/drivers/staging/vt6655/mac.h
@@ -554,7 +554,6 @@ void vt6655_mac_set_short_retry_limit(struct vnt_private *priv, unsigned char re
void MACvSetLongRetryLimit(struct vnt_private *priv, unsigned char byRetryLimit);

bool MACbSoftwareReset(struct vnt_private *priv);
-bool MACbSafeSoftwareReset(struct vnt_private *priv);
bool MACbSafeRxOff(struct vnt_private *priv);
bool MACbSafeTxOff(struct vnt_private *priv);
bool MACbSafeStop(struct vnt_private *priv);
--
2.37.3

2022-09-13 10:23:10

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 04/12] staging: vt6655: Cleanup and rename function MACbSafeSoftwareReset

On Sun, Sep 11, 2022 at 12:46:04PM +0200, Philipp Hortmann wrote:
> Rename function MACbSafeSoftwareReset to vt6655_mac_save_soft_reset and
> abyTmpRegData to tmp_reg_data to avoid CamelCase which is not accepted by
> checkpatch.pl. Remove return value bRetVal as it is unused by the calling
> functions.

Please don't mix this kind of stuff into a patch like this.

> Remove unnecessary declaration of function and make function
> static. Change declaration of tmp_reg_data to shorten code.

When I'm reviewing rename patches I have a script where I call
`rename_rev.pl -a` and it just parses the patch and outputs:

RENAMES:
abyTmpRegData => tmp_reg_data
MACbSafeSoftwareReset => vt6655_mac_save_soft_reset

I don't invest much time in thinking about the new names. The review
takes me about 5 seconds.

Then once you start marking the functions as static then that's slightly
a headache because now I have to look at that part by hand. But
whatever, you can kind of sell that as one thing.

But then when you mix ignoring the error codes in as well it's a big
headache. At first I thought it was because MACbSoftwareReset() always
returns true, so I pulled up that code and that's not true. So then I
am annoyed. And I pull up the commit message to see what's going on
and sure enough that change is burried in the middle of the paragraph.

Don't do that. Just do the renames. Then change all the functions in
one file to static at once. Then make the function void in a separate
patch.

The other thing is that making functions static is not at all
controversial. So long as it builds we will always accept those
patches.

Renaming variables is not super controversial. Sometimes people quarrel
about the new name. For example, I don't like a tmp variable which has
a long name like "tmp_reg_data". Just call it either "tmp" or
"reg_data". Not both. Don't write an essay.

But making functions void is sort of controversial... It's probably the
right thing in this context. But what I'm saying is don't mix things
which different controversial levels, because it means that someone is
going to complain. I would have ignored the "tmp_reg_data" thing
except now I'm already complaining about stuff I may as well mention it.

regards,
dan carpenter

2022-09-13 19:09:27

by Philipp Hortmann

[permalink] [raw]
Subject: Re: [PATCH 04/12] staging: vt6655: Cleanup and rename function MACbSafeSoftwareReset

On 9/13/22 12:12, Dan Carpenter wrote:
> On Sun, Sep 11, 2022 at 12:46:04PM +0200, Philipp Hortmann wrote:
>> Rename function MACbSafeSoftwareReset to vt6655_mac_save_soft_reset and
>> abyTmpRegData to tmp_reg_data to avoid CamelCase which is not accepted by
>> checkpatch.pl. Remove return value bRetVal as it is unused by the calling
>> functions.
> Please don't mix this kind of stuff into a patch like this.
>

In the past Greg let me know that I used to many patches for a change in
the same lines of code and that it would be easier for him to review
when less patches do the same.

As you wrote I am changing to many things at once. Sorry for breaking
your automatism.

I will consider your hints for the next patches.

Thanks

Bye Philipp

2022-09-14 15:08:34

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 04/12] staging: vt6655: Cleanup and rename function MACbSafeSoftwareReset

On Tue, Sep 13, 2022 at 08:26:04PM +0200, Philipp Hortmann wrote:
> On 9/13/22 12:12, Dan Carpenter wrote:
> > On Sun, Sep 11, 2022 at 12:46:04PM +0200, Philipp Hortmann wrote:
> > > Rename function MACbSafeSoftwareReset to vt6655_mac_save_soft_reset and
> > > abyTmpRegData to tmp_reg_data to avoid CamelCase which is not accepted by
> > > checkpatch.pl. Remove return value bRetVal as it is unused by the calling
> > > functions.
> > Please don't mix this kind of stuff into a patch like this.
> >
>
> In the past Greg let me know that I used to many patches for a change in the
> same lines of code and that it would be easier for him to review when less
> patches do the same.
>

It's some kind of an art to write patches that are easy to review...

regards,
dan carpenter