Linux wireless drivers development
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: Wolfram Sang <wsa@the-dreams.de>
Cc: devel@linuxdriverproject.org, linux-wireless@vger.kernel.org
Subject: Re: ks7010 endianness question
Date: Wed, 19 Apr 2017 14:02:58 +1000	[thread overview]
Message-ID: <20170419040258.GB17058@eros> (raw)
In-Reply-To: <20170419033446.GA17058@eros>

On Wed, Apr 19, 2017 at 01:34:46PM +1000, Tobin C. Harding wrote:
> Hi Wolfram,
> 
> May I please ask you with an ks7010 driver endianness question?
> 
> Comments on the hostif_hdr data structure (ks_hostif.h) state that the
> target uses little endian byte order.
> 
> /*
>  * HOST-MAC I/F data structure
>  * Byte alignmet Little Endian
>  */
> 
> struct hostif_hdr {
> 	u16 size;
> 	u16 event;
> } __packed;
> 
> On the rx data path this header is unpacked using get_WORD()
> 
> void hostif_receive(struct ks_wlan_private *priv, unsigned char *p,
> 		    unsigned int size)
> {
> 	DPRINTK(4, "\n");
> 
> 	devio_rec_ind(priv, p, size);
> 
> 	priv->rxp = p;
> 	priv->rx_size = size;
> 
> 	if (get_WORD(priv) == priv->rx_size) {	/* length check !! */
> 		hostif_event_check(priv);	/* event check */
> 	}
> }
> 
> get_WORD() inverts the byte order
> 
> static inline u16 get_WORD(struct ks_wlan_private *priv)
> {
> 	u16 data;
> 
> 	data = (get_BYTE(priv) & 0xff);
> 	data |= ((get_BYTE(priv) << 8) & 0xff00);
> 	return data;
> }
> 
> Am I missing something? It seems that this code will only work if the
> host and the target have differing endianness. It seems unlikely that
> the driver was tested solely on a big-endian machine, is the comment
> wrong - is the target actually big-endian?

Further investigation suggests that the target is actually using
network byte order (implying that the comment is wrong).

static void hostif_data_indication(struct ks_wlan_private *priv)
{
        ...
        
	auth_type = get_WORD(priv);	/* AuthType */
	get_WORD(priv);	                /* Reserve Area */

	eth_hdr = (struct ether_hdr *)(priv->rxp);
	eth_proto = ntohs(eth_hdr->h_proto);

        ...


I think we can call this question resolved. Remove the comment and
change the hostif_hdr description to

struct hostif_hdr {
	__be16 size;
 	__be16 event;
} __packed;

Are you happy with this?

thanks,
Tobin.

  reply	other threads:[~2017-04-19  4:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-19  3:34 ks7010 endianness question Tobin C. Harding
2017-04-19  4:02 ` Tobin C. Harding [this message]
2017-04-19  6:56   ` Wolfram Sang

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=20170419040258.GB17058@eros \
    --to=me@tobin.cc \
    --cc=devel@linuxdriverproject.org \
    --cc=linux-wireless@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox