Re: CVS commit: src/sys/dev/ic

[Christian Biere <christianbiere%gmx.de@localhost>]

Izumi Tsutsui wrote:
> christianbiere%gmx.de@localhost wrote:
> 
> > Don't you need at least __attribute__((__packed__)) for struct rtk_desc too?
> 
> Maybe it might be needed on machines which have 8 byte alignment
> restriction but currently I don't think there is such restriction
> even on 64 bit machines.

A similar real-life example would be

struct example {
  uint8_t a;
  uint8_t b;
};

On ARM (at least with GCC) sizeof (struct example) won't be 2.

> >  0: uint32_t rtk_cmdstat
> >  4: pad1
> >  8: uint32_t rtk_vlanctl
> > 12: pad2
> > 16: uint32_t rtk_bufaddr_lo
> > 20: pad3
> > 24: uint32_t rtk_bufaddr_hi
> > 28: pad4
> 
> The offsets of descriptor members are specified by hardware design
> of bus-master, so you can't pad it.

I'm not sure how you mean that. Since this struct rtx_desc is not
used in many places and you need endian-conversion anyway, I would
suggest using accessor functions instead of this struct. For example,
something like this:

static inline uint32_t
rtk_read_vlanctl(const char *data)
{
  uint32_t vlanctl;
  memcpy(&vlanctl, data + 4, 4);
  return le32toh(vlanctl);
}

or simpler:

static inline uint32_t
rtk_read_vlanctl(const char *data)
{
  return peek_le32(data + 4);
}

etc.

-- 
Christian