Hey Julian, On Tue, Jul 10, 2012 at 7:32 AM, Julian Calaby <julian.calaby@xxxxxxxxx> wrote: > Hi Arik, > > On Mon, Jul 9, 2012 at 6:14 PM, Arik Nemtsov <arik@xxxxxxxxxx> wrote: >> On Mon, Jul 9, 2012 at 11:03 AM, Johannes Berg >> <johannes@xxxxxxxxxxxxxxxx> wrote: >>> On Sun, 2012-07-08 at 17:08 +0300, Arik Nemtsov wrote: >>> >>>> > kzalloc (which is pointless -- use kmemdup) alignment is sufficient, >>>> > then most likely you could just put "__aligned(4)" or something on the >>>> > conf.phy struct member and not allocate memory at all? >>>> >>>> Yea we need 4 bytes alignment. The kzalloc is a sort of convention in >>>> the driver (see acx.c), but probably here we can use kmemdup. >>>> We can't use fancy __aligned(4) notations most likely, since a >>>> usermode tool also parses these header files, and it's not too smart >>>> :) >>> >>> This seems like a rather bad excuse for making the runtime code more >>> complex and less performant ... I'll let you sort it out with John >>> though, I wouldn't let you get away with this though if I maintained the >>> tree ;-) >>> >>> FWIW, I think your actual problem is that you mark wl18xx_priv_conf as >>> packed unnecessarily. Otherwise it might actually get alignment, at >>> least on ARM I hear. >> >> It's not an excuse, we really have a usermode tool (called wlconf) >> preparing binary conf files. And it usually runs on a different arch >> (x86), so we mark everything packed to avoid errors. >> I'm pretty sure __aligned(4) would screw up the parser there. It has >> to build an internal representation of all the struct, and fill it out >> using an ini file. > > Does this tool parse the compiled code or the actual source files? > Because if it parses the source files, surely you could adjust the > parser so it doesn't trip over a potential __aligned(4) notation. The tool goes over source files, but it's not just a matter of skipping the token - it has to actually align the next element to 4, so it needs to start keeping track of alignment etc. I'm afraid we have bigger issues with this tool right now. Currently it can't even parse arrays properly :) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
