lkml.org 
[lkml]   [2014]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCHv3 10/14] HSI: Introduce driver for SSI Protocol
    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 <carlos.chinea@nokia.com>
    > > Signed-off-by: Sebastian Reichel <sre@kernel.org>
    >
    > > +#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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2014-04-25 21:21    [W:4.822 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site