Re: [PATCH] staging: ks7010: fix styling WARNINGs
From: Joe Perches
Date: Tue Aug 01 2017 - 12:23:16 EST
On Tue, 2017-08-01 at 17:11 +0100, Ashish Kalra wrote:
> Signed-off-by: Ashish Kalra <eashishkalra@xxxxxxxxx>
Still many issues with this patch submission:
o no commit message
o no version of patch submission in subject line
And other issues below:
> ---
> drivers/staging/ks7010/ks7010_sdio.c | 47 +++++++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 14 deletions(-)
>
> Trivial style changes. There are still "line over 80 characters"
> checkpatch.pl warnings, but I think they are best left alone as
> breaking these could hurt readability
This message should go above the --- line and the
version information about what changed between
each submission should go below the --- line.
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
[]
> @@ -9,7 +9,6 @@
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> */
> -
Why is this line removed?
> #include <linux/firmware.h>
> #include <linux/mmc/card.h>
> #include <linux/mmc/sdio_func.h>
> @@ -32,19 +31,39 @@
> };
> MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
>
> -#define inc_txqhead(priv) \
> - (priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
> -#define inc_txqtail(priv) \
> - (priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
> -#define cnt_txqbody(priv) \
> - (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE)
> -
> -#define inc_rxqhead(priv) \
> - (priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
> -#define inc_rxqtail(priv) \
> - (priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
> -#define cnt_rxqbody(priv) \
> - (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE)
> +static int inc_txqhead(struct ks_wlan_private *priv)
> +{
> + priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE;
> + return 0;
> +}
> +
> +static int inc_txqtail(struct ks_wlan_private *priv)
> +{
> + priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE;
> + return 0;
> +}
> +
> +static int inc_rxqtail(struct ks_wlan_private *priv)
> +{
> + priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE;
> + return 0;
> +}
> +
> +static int inc_rxqhead(struct ks_wlan_private *priv)
> +{
> + priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE;
> + return 0;
> +}
Why are the functions above not returning void?
> +
> +static int cnt_rxqbody(struct ks_wlan_private *priv)
> +{
> + return (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE);
> +}
> +
> +static int cnt_txqbody(struct ks_wlan_private *priv)
> +{
> + return (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE);
> +}
Why are these 2 functions above not returning unsigned int?