On Sat, Apr 19, 2014 at 09:49:36PM +0200, Pavel Machek wrote: > Hi! > > > This adds a driver for the SSI McSAAB protocol as used in > > the Nokia N900. > > > > Signed-off-by: Carlos Chinea <[email protected]> > > Signed-off-by: Sebastian Reichel <[email protected]> > > > +#define SSIP_MIN_PN_HDR 6 /* FIXME: Revisit */ > > +#define SSIP_WDTOUT 2000 /* FIXME: has to be 500 msecs> > > */ > > Can they be fixed now, or do they have to wait?
I would prefer to wait. The timing above works and having it in the git history doesn't hurt. > > +#ifdef __LITTLE_ENDIAN > > + ((u16 *)skb->data)[2] = swab16(((u16 *)skb->data)[2]); > > + dev_dbg(&dev->dev, "RX length fixed (%04x -> %u)\n", > > + ((u16 *)skb->data)[2], ntohs(((u16 *)skb->data)[2])); > > +#endif > > Uh. Can this be replaced by > ((u16 *)skb->data)[2] = htons(((u16 *)skb->data)[2]); > > (without the ifdef?) Yes. > > + /* > > + * Modem sends Phonet messages over SSI with its own endianess... > > + * Assume that modem has the same endianess as we do. > > + */ > > + if (skb_cow_head(skb, 0)) > > + goto drop; > > +#ifdef __LITTLE_ENDIAN > > + ((u16 *)skb->data)[2] = swab16(((u16 *)skb->data)[2]); > > +#endif > > Here, too? Fixed in PATCHv4. > But it looks like the comment does not match the code, because we > swap. The swap only converts the package length field from/to network byte order. I think the comment still applies for the other actual message. I added a comment for the length field and kept the other comment as is for PATCHv4. -- Sebastian
signature.asc
Description: Digital signature
