> > There's quite a bit of `magic' here [based on the ethtool `magic' > > field] which is retained by the user application responsible for > > interacting with the driver for the purpose of upgrading the nvram > > image. > > This is not how the 'magic' value of the eeprom structure is specified to be > used, > please use it the way it is supposed to be used rather than inventing > semantics > which only apply to your device. > > The entire point of defining a common interface for device driver interface > interaction with the user is exactly so that driver authors don't do things > like this.
While I obviously get what you're writing, I actually think this is the more revealing API option. I.e., the bottom line is that write to this device's nvram is complex in nature and requires interacting with the management firmware. The other immediate option would have been to implement this in driver as simple read/write toward the management firmware, and let the management firmware analyze what it needs to do with the data based on hidden meta-data inside the buffer written by the application. While this might be less 'extending' of the ethtool API, it's a security- in-obscurity kind of API, where a reviewer can't possibly know how to perform read/write from reading the driver code. The third option I see is to completely ditch this, stating that although We obviously CAN set the non-volatile memory we CAN'T do it with the regular API, and to move into doing this via some proprietary API such as debugfs. Thanks, Yuval
