Re: [PATCH v9 1/5] firmware: add extensible driver data params

Linus Torvalds
2017-06-23T23:09:32Z

On Fri, Jun 23, 2017 at 3:43 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>
> Ah, yes! Here is what I believe seems to be the *crux* issue of these patch
> series and I'm happy we have finally landed on it. Yes, indeed the new API
> proposed here provides more flexibility, and it does so by embracing a
> "data driven" API Vs the traditional procedural APIs we have seen for
> *the firmware API*.

This has been going on forever. Everybody hates your data-driven one.
It's too indirect, it adds all those nasty "descriptors" of what to
do, and it doesn't match what the current model does at all.

Things like that may be ok as an internal implementation, but even
there it's questionable if it then means a big disconnect between what
people actually use (the normal functional model) and the
implementation.

The thing is, it's much better to just have functions that load the
firmware data. Have them named that way ("load_firmware()"), and act
that way ("just load the damn firmware file") instead of having odd
descriptors that describe what is goign to be done and some state for
it, and then get passed around.

Don't add this kind of crazy abstraction complexity.

If somebody wants to veryify a signature on a firmware file, they
should *NOT* fill in a descriptor that says "check signature when
loading". Thats' complete BS.

They should just do "load_firmware()" and then "check_signature()" or whatever.

Would such a "load firmware file, then check signature" take a few
lines (with error handling)? Yes.

But it is a *simple* interface. It doesn't have some stupid "struct
driver_data_params" that needs to be filled in with random details (or
has magic macros in a header file that fill in default values). It's
_straightforward_.

Linus