All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: Cezary Gapinski <gapalinux@gmail.com>
Cc: gregkh@linuxfoundation.org, wsa@the-dreams.de,
	karniksayli1995@gmail.com, shiva@exdev.nl, bhumirks@gmail.com,
	mattkilgore12@gmail.com, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging/ks7010: Fix type assignment for struct hostif_hdr
Date: Sun, 30 Apr 2017 18:00:44 +1000	[thread overview]
Message-ID: <20170430080044.GC8204@eros> (raw)
In-Reply-To: <1493488470-13487-1-git-send-email-gapalinux@gmail.com>

On Sat, Apr 29, 2017 at 07:54:30PM +0200, Cezary Gapinski wrote:
> Sparse spits out a warnings about __le16 and unsigned short assignment.
> Change the type of size and event members of struct hostif_hdr
> to __le16 and correct conversion to the proper cpu type.

I believe that this patch is correct. I am of the opinion though that
endian changes to this driver require prior testing on hardware. There
are some anomalies that I have not figured out yet (for example the
eap header appears to be in network byte order).

I may, however, very well be wrong. It may be perfectly safe to merge
this as is.

Good luck,
Tobin.

> Signed-off-by: Cezary Gapinski <gapalinux@gmail.com>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 10 ++++++----
>  drivers/staging/ks7010/ks_hostif.h   |  4 ++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index ec11799..e3a134d 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -269,7 +269,8 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer,
>  	hdr = (struct hostif_hdr *)buffer;
>  
>  	DPRINTK(4, "size=%d\n", hdr->size);
> -	if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) {
> +	if (le16_to_cpu(hdr->event) < HIF_DATA_REQ ||
> +	    le16_to_cpu(hdr->event) > HIF_REQ_MAX) {
>  		DPRINTK(1, "unknown event=%04X\n", hdr->event);
>  		return 0;
>  	}
> @@ -327,13 +328,14 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size,
>  
>  	hdr = (struct hostif_hdr *)p;
>  
> -	if (hdr->event < HIF_DATA_REQ || HIF_REQ_MAX < hdr->event) {
> +	if (le16_to_cpu(hdr->event) < HIF_DATA_REQ ||
> +	    le16_to_cpu(hdr->event) > HIF_REQ_MAX) {
>  		DPRINTK(1, "unknown event=%04X\n", hdr->event);
>  		return 0;
>  	}
>  
>  	/* add event to hostt buffer */
> -	priv->hostt.buff[priv->hostt.qtail] = hdr->event;
> +	priv->hostt.buff[priv->hostt.qtail] = le16_to_cpu(hdr->event);
>  	priv->hostt.qtail = (priv->hostt.qtail + 1) % SME_EVENT_BUFF_SIZE;
>  
>  	DPRINTK(4, "event=%04X\n", hdr->event);
> @@ -403,7 +405,7 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
>  
>  	hdr = (struct hostif_hdr *)&rx_buffer->data[0];
>  	rx_buffer->size = le16_to_cpu(hdr->size) + sizeof(hdr->size);
> -	event = hdr->event;
> +	event = le16_to_cpu(hdr->event);
>  	inc_rxqtail(priv);
>  
>  	ret = ks7010_sdio_writeb(priv, READ_STATUS, REG_STATUS_IDLE);
> diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h
> index d773432..7e4d1aa 100644
> --- a/drivers/staging/ks7010/ks_hostif.h
> +++ b/drivers/staging/ks7010/ks_hostif.h
> @@ -62,8 +62,8 @@
>   */
>  
>  struct hostif_hdr {
> -	u16 size;
> -	u16 event;
> +	__le16 size;
> +	__le16 event;
>  } __packed;
>  
>  struct hostif_data_request_t {
> -- 
> 2.7.4
> 

  reply	other threads:[~2017-04-30  8:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-29 17:54 [PATCH] staging/ks7010: Fix type assignment for struct hostif_hdr Cezary Gapinski
2017-04-30  8:00 ` Tobin C. Harding [this message]
2017-05-01  9:59 ` Tobin C. Harding
2017-05-01 14:11   ` gapalinux
2017-05-01 21:31     ` Tobin C. Harding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170430080044.GC8204@eros \
    --to=me@tobin.cc \
    --cc=bhumirks@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gapalinux@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=karniksayli1995@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mattkilgore12@gmail.com \
    --cc=shiva@exdev.nl \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.