On Wed, 2015-06-17 at 08:34 +0000, David Lin wrote: > > Johannes Berg wrote: > > > > On Wed, 2015-06-17 at 08:07 +0000, David Lin wrote: > > > > > > Also, you probably really should have two header files - one for the > > > > firmware structs and one for the driver structs - especially since > > > > you seem to be confusing the two! > > > > > > > As mentioned before, this is current interface with F/W, and this F/W > > > is used by other marvell's drivers, I can't change it. > > > > You're misunderstanding. I'm not asking you to change the interface. I'm > > just > > asking you to make sure you know which part is firmware interface and which > > isn't. Clearly, pointers *cannot* be firmware interface - if there's > > something > > "cookie-like" that you use as pointers you at least need to make sure you > > know > > how long this field is (32 or 64 bits)... > > > Yes, we know this kind of issue. We will enhance it in the future. > > > > > All structures defined in this file are related to F/W commands, it > > > should be better let them be defined in this file. > > > > Given your own confusion on what's firmware API and what isn't, I'm not so > > sure about that ... > > > I am sure all structures defined in this file is related to firmware command.
They may well be related, but that doesn't necessarily mean that the driver side cannot be clearer about what fields of a struct are actually read by the firmware, and what fields are private to the host implementation as a data stash (eg, 'priv'-type stuff). Redoing the structs like Johannes suggested originally would make that crystal-clear... Dan -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
