On Tue, Jul 10, 2012 at 8:46 AM, Julian Calaby <julian.calaby@xxxxxxxxx> wrote: > Hi Arik, > > On Tue, Jul 10, 2012 at 3:42 PM, Arik Nemtsov <arik@xxxxxxxxxx> wrote: >> 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. > > Ah, does it produce binary chunks of data that are then "parsed" with > this structure? Yea. It produces a bin file that the driver takes using request_firmware(). The driver uses this file as its internal configuration struct. > >> I'm afraid we have bigger issues with this tool right now. Currently >> it can't even parse arrays properly :) > > That's a big problem =) Tell me about it :) -- 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
