[PATCH 0/2] staging: vt6655: Fix name and return value of CARDbGetCurrentTSF

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/2] staging: vt6655: Fix name and return value of CARDbGetCurrentTSF
@ 2022-05-01 16:36 Philipp Hortmann
  2022-05-01 16:37 ` [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF Philipp Hortmann
  2022-05-01 16:37 ` [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf Philipp Hortmann
  0 siblings, 2 replies; 5+ messages in thread
From: Philipp Hortmann @ 2022-05-01 16:36 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman, linux-staging, linux-kernel

Fix name of the function that uses CamelCase which
is not accepted by checkpatch.pl

Replace unused return value.

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

Philipp Hortmann (2):
  staging: vt6655: Rename function CARDbGetCurrentTSF
  staging: vt6655: Replace unused return value of card_get_current_tsf

 drivers/staging/vt6655/card.c        | 22 ++++++++++------------
 drivers/staging/vt6655/card.h        |  2 +-
 drivers/staging/vt6655/device_main.c |  2 +-
 3 files changed, 12 insertions(+), 14 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF
  2022-05-01 16:36 [PATCH 0/2] staging: vt6655: Fix name and return value of CARDbGetCurrentTSF Philipp Hortmann
@ 2022-05-01 16:37 ` Philipp Hortmann
  2022-05-01 16:50   ` Greg Kroah-Hartman
  2022-05-01 16:37 ` [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf Philipp Hortmann
  1 sibling, 1 reply; 5+ messages in thread
From: Philipp Hortmann @ 2022-05-01 16:37 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman, linux-staging, linux-kernel

The name of the function uses CamelCase which
is not accepted by checkpatch.pl

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
 drivers/staging/vt6655/card.c        | 10 +++++-----
 drivers/staging/vt6655/card.h        |  2 +-
 drivers/staging/vt6655/device_main.c |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
index ec6fd185d3fd..d1dfd96e13b7 100644
--- a/drivers/staging/vt6655/card.c
+++ b/drivers/staging/vt6655/card.c
@@ -11,7 +11,7 @@
  *      CARDbAddBasicRate - Add to BasicRateSet
  *      CARDbIsOFDMinBasicRate - Check if any OFDM rate is in BasicRateSet
  *      CARDqGetTSFOffset - Calculate TSFOffset
- *      CARDbGetCurrentTSF - Read Current NIC TSF counter
+ *      card_get_current_tsf - Read Current NIC TSF counter
  *      CARDqGetNextTBTT - Calculate Next Beacon TSF counter
  *      CARDvSetFirstNextTBTT - Set NIC Beacon time
  *      CARDvUpdateNextTBTT - Sync. NIC Beacon time
@@ -288,7 +288,7 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
 	u64 local_tsf;
 	u64 qwTSFOffset = 0;
 
-	CARDbGetCurrentTSF(priv, &local_tsf);
+	card_get_current_tsf(priv, &local_tsf);
 
 	if (qwBSSTimestamp != local_tsf) {
 		qwTSFOffset = CARDqGetTSFOffset(byRxRate, qwBSSTimestamp,
@@ -322,7 +322,7 @@ bool CARDbSetBeaconPeriod(struct vnt_private *priv,
 {
 	u64 qwNextTBTT = 0;
 
-	CARDbGetCurrentTSF(priv, &qwNextTBTT); /* Get Local TSF counter */
+	card_get_current_tsf(priv, &qwNextTBTT); /* Get Local TSF counter */
 
 	qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
 
@@ -739,7 +739,7 @@ u64 CARDqGetTSFOffset(unsigned char byRxRate, u64 qwTSF1, u64 qwTSF2)
  *
  * Return Value: true if success; otherwise false
  */
-bool CARDbGetCurrentTSF(struct vnt_private *priv, u64 *pqwCurrTSF)
+bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF)
 {
 	void __iomem *iobase = priv->port_offset;
 	unsigned short ww;
@@ -811,7 +811,7 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
 	void __iomem *iobase = priv->port_offset;
 	u64 qwNextTBTT = 0;
 
-	CARDbGetCurrentTSF(priv, &qwNextTBTT); /* Get Local TSF counter */
+	card_get_current_tsf(priv, &qwNextTBTT); /* Get Local TSF counter */
 
 	qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
 	/* Set NextTBTT */
diff --git a/drivers/staging/vt6655/card.h b/drivers/staging/vt6655/card.h
index 09e7f3f1cbed..55fe039065db 100644
--- a/drivers/staging/vt6655/card.h
+++ b/drivers/staging/vt6655/card.h
@@ -46,7 +46,7 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
 			   unsigned short wBeaconInterval);
 void CARDvUpdateNextTBTT(struct vnt_private *priv, u64 qwTSF,
 			 unsigned short wBeaconInterval);
-bool CARDbGetCurrentTSF(struct vnt_private *priv, u64 *pqwCurrTSF);
+bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF);
 u64 CARDqGetNextTBTT(u64 qwTSF, unsigned short wBeaconInterval);
 u64 CARDqGetTSFOffset(unsigned char byRxRate, u64 qwTSF1, u64 qwTSF2);
 unsigned char CARDbyGetPktType(struct vnt_private *priv);
diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 12fd825c23fe..393430757700 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -1603,7 +1603,7 @@ static u64 vnt_get_tsf(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	struct vnt_private *priv = hw->priv;
 	u64 tsf;
 
-	CARDbGetCurrentTSF(priv, &tsf);
+	card_get_current_tsf(priv, &tsf);
 
 	return tsf;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF
  2022-05-01 16:37 ` [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF Philipp Hortmann
@ 2022-05-01 16:50   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-05-01 16:50 UTC (permalink / raw)
  To: Philipp Hortmann; +Cc: Forest Bond, linux-staging, linux-kernel

On Sun, May 01, 2022 at 06:37:05PM +0200, Philipp Hortmann wrote:
> The name of the function uses CamelCase which
> is not accepted by checkpatch.pl
> 
> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
> ---
>  drivers/staging/vt6655/card.c        | 10 +++++-----
>  drivers/staging/vt6655/card.h        |  2 +-
>  drivers/staging/vt6655/device_main.c |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
> index ec6fd185d3fd..d1dfd96e13b7 100644
> --- a/drivers/staging/vt6655/card.c
> +++ b/drivers/staging/vt6655/card.c
> @@ -11,7 +11,7 @@
>   *      CARDbAddBasicRate - Add to BasicRateSet
>   *      CARDbIsOFDMinBasicRate - Check if any OFDM rate is in BasicRateSet
>   *      CARDqGetTSFOffset - Calculate TSFOffset
> - *      CARDbGetCurrentTSF - Read Current NIC TSF counter
> + *      card_get_current_tsf - Read Current NIC TSF counter
>   *      CARDqGetNextTBTT - Calculate Next Beacon TSF counter
>   *      CARDvSetFirstNextTBTT - Set NIC Beacon time
>   *      CARDvUpdateNextTBTT - Sync. NIC Beacon time
> @@ -288,7 +288,7 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
>  	u64 local_tsf;
>  	u64 qwTSFOffset = 0;
>  
> -	CARDbGetCurrentTSF(priv, &local_tsf);
> +	card_get_current_tsf(priv, &local_tsf);

Why is the value being passed as a reference and the error value (if
any), is not checked at all?

Shouldn't this just return the value as it obviously must always work?
(retorical question, something is wrong here...)

And this is a horrible global function name, can you prefix it with the
driver name to give us a chance to keep the global namespace sane?
vt6655_get_current_tsf() would be better.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf
  2022-05-01 16:36 [PATCH 0/2] staging: vt6655: Fix name and return value of CARDbGetCurrentTSF Philipp Hortmann
  2022-05-01 16:37 ` [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF Philipp Hortmann
@ 2022-05-01 16:37 ` Philipp Hortmann
  2022-05-01 16:53   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 5+ messages in thread
From: Philipp Hortmann @ 2022-05-01 16:37 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman, linux-staging, linux-kernel

Replace unused return value with u64 to increase readability,
reduce address and dereference operators and omit pqwCurrTSF that
uses CamelCase which is not accepted by checkpatch.pl

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
---
Diffs for testing:
        u64 qwNextTBTT;

        qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
+       dev_info(&priv->pcid->dev, "CARDbSetBeaconPeriod 0x%016llx", qwNextTBTT);
        qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);

        /* set HW beacon interval */
@@ -810,7 +810,7 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
        u64 qwNextTBTT;

        qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
+       dev_info(&priv->pcid->dev, "CARDvSetFirstNextTBTT 0x%016llx", qwNextTBTT);
        qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
        /* Set NextTBTT */
        VNSvOutPortD(iobase + MAC_REG_NEXTTBTT, (u32)qwNextTBTT);
Log:
vt6655 0000:01:05.0: CARDbSetBeaconPeriod  0x00 00 00 01 4a 89 52 c4
vt6655 0000:01:05.0: CARDvSetFirstNextTBTT 0x00 00 00 01 4a 89 52 dc
---
 drivers/staging/vt6655/card.c        | 20 +++++++++-----------
 drivers/staging/vt6655/card.h        |  2 +-
 drivers/staging/vt6655/device_main.c |  2 +-
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
index d1dfd96e13b7..50d70ebff83a 100644
--- a/drivers/staging/vt6655/card.c
+++ b/drivers/staging/vt6655/card.c
@@ -288,7 +288,7 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
 	u64 local_tsf;
 	u64 qwTSFOffset = 0;
 
-	card_get_current_tsf(priv, &local_tsf);
+	local_tsf = card_get_current_tsf(priv);
 
 	if (qwBSSTimestamp != local_tsf) {
 		qwTSFOffset = CARDqGetTSFOffset(byRxRate, qwBSSTimestamp,
@@ -320,9 +320,9 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
 bool CARDbSetBeaconPeriod(struct vnt_private *priv,
 			  unsigned short wBeaconInterval)
 {
-	u64 qwNextTBTT = 0;
+	u64 qwNextTBTT;
 
-	card_get_current_tsf(priv, &qwNextTBTT); /* Get Local TSF counter */
+	qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
 
 	qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
 
@@ -739,7 +739,7 @@ u64 CARDqGetTSFOffset(unsigned char byRxRate, u64 qwTSF1, u64 qwTSF2)
  *
  * Return Value: true if success; otherwise false
  */
-bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF)
+u64 card_get_current_tsf(struct vnt_private *priv)
 {
 	void __iomem *iobase = priv->port_offset;
 	unsigned short ww;
@@ -753,16 +753,14 @@ bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF)
 			break;
 	}
 	if (ww == W_MAX_TIMEOUT)
-		return false;
+		return 0;
 	low = ioread32(iobase + MAC_REG_TSFCNTR);
 	high = ioread32(iobase + MAC_REG_TSFCNTR + 4);
 #ifdef __BIG_ENDIAN
-	*pqwCurrTSF = high + ((u64)low << 32);
+	return high + ((u64)low << 32);
 #else
-	*pqwCurrTSF = low + ((u64)high << 32);
+	return low + ((u64)high << 32);
 #endif
-
-	return true;
 }
 
 /*
@@ -809,9 +807,9 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
 			   unsigned short wBeaconInterval)
 {
 	void __iomem *iobase = priv->port_offset;
-	u64 qwNextTBTT = 0;
+	u64 qwNextTBTT;
 
-	card_get_current_tsf(priv, &qwNextTBTT); /* Get Local TSF counter */
+	qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
 
 	qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
 	/* Set NextTBTT */
diff --git a/drivers/staging/vt6655/card.h b/drivers/staging/vt6655/card.h
index 55fe039065db..7830a055b855 100644
--- a/drivers/staging/vt6655/card.h
+++ b/drivers/staging/vt6655/card.h
@@ -46,7 +46,7 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
 			   unsigned short wBeaconInterval);
 void CARDvUpdateNextTBTT(struct vnt_private *priv, u64 qwTSF,
 			 unsigned short wBeaconInterval);
-bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF);
+u64 card_get_current_tsf(struct vnt_private *priv);
 u64 CARDqGetNextTBTT(u64 qwTSF, unsigned short wBeaconInterval);
 u64 CARDqGetTSFOffset(unsigned char byRxRate, u64 qwTSF1, u64 qwTSF2);
 unsigned char CARDbyGetPktType(struct vnt_private *priv);
diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 393430757700..6989d8d04f2f 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -1603,7 +1603,7 @@ static u64 vnt_get_tsf(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	struct vnt_private *priv = hw->priv;
 	u64 tsf;
 
-	card_get_current_tsf(priv, &tsf);
+	tsf = card_get_current_tsf(priv);
 
 	return tsf;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf
  2022-05-01 16:37 ` [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf Philipp Hortmann
@ 2022-05-01 16:53   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-05-01 16:53 UTC (permalink / raw)
  To: Philipp Hortmann; +Cc: Forest Bond, linux-staging, linux-kernel

On Sun, May 01, 2022 at 06:37:11PM +0200, Philipp Hortmann wrote:
> Replace unused return value with u64 to increase readability,
> reduce address and dereference operators and omit pqwCurrTSF that
> uses CamelCase which is not accepted by checkpatch.pl
> 
> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
> ---
> Diffs for testing:
>         u64 qwNextTBTT;
> 
>         qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
> +       dev_info(&priv->pcid->dev, "CARDbSetBeaconPeriod 0x%016llx", qwNextTBTT);

Don't put a diff in a diff please, git _should_ handle it, but I know
other tools that will choke on it.

>         qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
> 
>         /* set HW beacon interval */
> @@ -810,7 +810,7 @@ void CARDvSetFirstNextTBTT(struct vnt_private *priv,
>         u64 qwNextTBTT;
> 
>         qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
> +       dev_info(&priv->pcid->dev, "CARDvSetFirstNextTBTT 0x%016llx", qwNextTBTT);
>         qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
>         /* Set NextTBTT */
>         VNSvOutPortD(iobase + MAC_REG_NEXTTBTT, (u32)qwNextTBTT);
> Log:
> vt6655 0000:01:05.0: CARDbSetBeaconPeriod  0x00 00 00 01 4a 89 52 c4
> vt6655 0000:01:05.0: CARDvSetFirstNextTBTT 0x00 00 00 01 4a 89 52 dc
> ---
>  drivers/staging/vt6655/card.c        | 20 +++++++++-----------
>  drivers/staging/vt6655/card.h        |  2 +-
>  drivers/staging/vt6655/device_main.c |  2 +-
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
> index d1dfd96e13b7..50d70ebff83a 100644
> --- a/drivers/staging/vt6655/card.c
> +++ b/drivers/staging/vt6655/card.c
> @@ -288,7 +288,7 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
>  	u64 local_tsf;
>  	u64 qwTSFOffset = 0;
>  
> -	card_get_current_tsf(priv, &local_tsf);
> +	local_tsf = card_get_current_tsf(priv);

{sigh}  I should read all patches in the series before complaining about
previous ones, sorry about that.

Looks much better, except for the function name.



>  
>  	if (qwBSSTimestamp != local_tsf) {
>  		qwTSFOffset = CARDqGetTSFOffset(byRxRate, qwBSSTimestamp,
> @@ -320,9 +320,9 @@ bool CARDbUpdateTSF(struct vnt_private *priv, unsigned char byRxRate,
>  bool CARDbSetBeaconPeriod(struct vnt_private *priv,
>  			  unsigned short wBeaconInterval)
>  {
> -	u64 qwNextTBTT = 0;
> +	u64 qwNextTBTT;
>  
> -	card_get_current_tsf(priv, &qwNextTBTT); /* Get Local TSF counter */
> +	qwNextTBTT = card_get_current_tsf(priv); /* Get Local TSF counter */
>  
>  	qwNextTBTT = CARDqGetNextTBTT(qwNextTBTT, wBeaconInterval);
>  
> @@ -739,7 +739,7 @@ u64 CARDqGetTSFOffset(unsigned char byRxRate, u64 qwTSF1, u64 qwTSF2)
>   *
>   * Return Value: true if success; otherwise false
>   */
> -bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF)
> +u64 card_get_current_tsf(struct vnt_private *priv)
>  {
>  	void __iomem *iobase = priv->port_offset;
>  	unsigned short ww;
> @@ -753,16 +753,14 @@ bool card_get_current_tsf(struct vnt_private *priv, u64 *pqwCurrTSF)
>  			break;
>  	}
>  	if (ww == W_MAX_TIMEOUT)
> -		return false;
> +		return 0;
>  	low = ioread32(iobase + MAC_REG_TSFCNTR);
>  	high = ioread32(iobase + MAC_REG_TSFCNTR + 4);
>  #ifdef __BIG_ENDIAN

Note, this #ifdef really should never be in kernel code if you are doing
things properly.  There are functions to handle this correctly...

Things get "fun" when you have devices in one endian running on busses
of other endian.  Hopefully this driver never has to deal with that, but
for many others we do, so please stick with the normal functions to
handle this whenever possible.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-05-01 16:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-01 16:36 [PATCH 0/2] staging: vt6655: Fix name and return value of CARDbGetCurrentTSF Philipp Hortmann
2022-05-01 16:37 ` [PATCH 1/2] staging: vt6655: Rename function CARDbGetCurrentTSF Philipp Hortmann
2022-05-01 16:50   ` Greg Kroah-Hartman
2022-05-01 16:37 ` [PATCH 2/2] staging: vt6655: Replace unused return value of card_get_current_tsf Philipp Hortmann
2022-05-01 16:53   ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.