Netdev List
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@osdl.org>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	netdev@vger.kernel.org, linux1394-devel@lists.sourceforge.net
Subject: Re: [PATCH] eth1394: endian fixes
Date: Sat, 27 May 2006 09:40:19 -0700	[thread overview]
Message-ID: <20060527094019.4d451093@localhost.localdomain> (raw)
In-Reply-To: <44782474.9050304@s5r6.in-berlin.de>

On Sat, 27 May 2006 12:05:40 +0200
Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> Alexey Dobriyan wrote on 2006-05-16:
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ...
> > @@ -935,7 +935,7 @@ static inline u16 ether1394_parse_encap(
> >  		*(u32*)arp_ptr = arp1394->sip;	/* move sender IP addr */
> >  		arp_ptr += arp->ar_pln;		/* skip over sender IP addr */
> >  
> > -		if (arp->ar_op == 1)
> > +		if (arp->ar_op == htons(1))
> >  			/* just set ARP req target unique ID to 0 */
> >  			*((u64*)arp_ptr) = 0;
> >  		else
> 
> I suggest __constant_htons(ARPOP_REQUEST).

No. That is only for initializers and switch cases. Otherwise, it
adds unnecessary verbosity.  The htons() does the right thing automatically.

> 
> > @@ -1395,7 +1395,7 @@ static inline void ether1394_arp_to_1394
> >  /* We need to encapsulate the standard header with our own. We use the
> >   * ethernet header's proto for our own. */
> >  static inline unsigned int ether1394_encapsulate_prep(unsigned int max_payload,
> > -						      int proto,
> > +						      __be16 proto,
> >  						      union eth1394_hdr *hdr,
> >  						      u16 dg_size, u16 dgl)
> >  {
> > @@ -1626,7 +1626,7 @@ static int ether1394_tx (struct sk_buff 
> >  	gfp_t kmflags = in_interrupt() ? GFP_ATOMIC : GFP_KERNEL;
> >  	struct eth1394hdr *eth;
> >  	struct eth1394_priv *priv = netdev_priv(dev);
> > -	int proto;
> > +	__be16 proto;
> >  	unsigned long flags;
> >  	nodeid_t dest_node;
> >  	eth1394_tx_type tx_type;
> 
> Alexey, I am afraid none of the current 1394 developers is actively 
> using eth1394. Did you have the chance to test your patch on different 
> platforms and with different peer OSs/platforms?
> 
> Also, did someone of the netdev guys already forward the patch (since we 
> 1394 people are slow and eth1394 is marked unmaintained)? If not, I 
> could send it along with a few other 1394 patches to akpm soon.
> -- 
> Stefan Richter
> -=====-=-==- -=-= ==-==
> http://arcgraph.de/sr/
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2006-05-27 16:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-19 13:25 [PATCH] eth1394: endian fixes Alexey Dobriyan
2006-05-27 10:05 ` Stefan Richter
2006-05-27 16:12   ` Alexey Dobriyan
2006-06-10 16:46     ` Stefan Richter
2006-05-27 16:40   ` Stephen Hemminger [this message]
2006-06-10 21:44     ` [PATCH 2.6.17-rc6-mm2] eth1394: replace __constant_htons by htons Stefan Richter

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=20060527094019.4d451093@localhost.localdomain \
    --to=shemminger@osdl.org \
    --cc=adobriyan@gmail.com \
    --cc=linux1394-devel@lists.sourceforge.net \
    --cc=netdev@vger.kernel.org \
    --cc=stefanr@s5r6.in-berlin.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