Messages in this thread | | | Date | Fri, 01 May 2009 20:21:15 +0200 | From | Wolfgang Grandegger <> | Subject | Re: [PATCH 4/8] can: Driver for the SJA1000 CAN controller |
| |
I'm now back fixing the open issues of this patch series...
Jonathan Corbet wrote: [snip] >> +/* >> + * SJA1000 private data structure >> + */ >> +struct sja1000_priv { >> + struct can_priv can; /* must be the first member! */ > > AHA! I knew it! > > This kind of pointer trickery is fragile and dangerous, please don't do > it. Much better would be something like: > > dev->priv = &dev_specific_priv->can; > > Then the higher layers know they have a proper struct can_priv pointer. > Then you can use container_of() at this level to get the outer structure > pointer. Much more robust and in line with normal kernel coding idiom.
Unfortunately, the "struct net_device" does not have a "priv" field. Using
struct can_priv *can = netdev_priv(dev); struct sja1000_priv *sja1000 = candev_priv(dev);
instead is awkward. That's what we had at first. Any other more elegant solution in mind?
Wolfgang.
| |