Hi Greg, Luis,
On 2017-06-17 12:38, Greg KH wrote:
On Tue, Jun 13, 2017 at 09:40:11PM +0200, Luis R. Rodriguez wrote:
On Tue, Jun 13, 2017 at 11:05:48AM +0200, Greg KH wrote:
> On Mon, Jun 05, 2017 at 02:39:33PM -0700, Luis R. Rodriguez wrote:
> > As the firmware API evolves we keep extending functions with more arguments.
> > Stop this nonsense by proving an extensible data structure which can be used
> > to represent both user parameters and private internal parameters.
>
> Let's take a simple C function interface and make it a more complex
> data-driven interface that is impossible to understand and obviously
> understand how it is to be used and works!
The firmware codebase was already complex!
Heh, I'm not arguing with you there :)
What you have to ask yourself really is if this makes it *less
complex* and
helps *clean things up* in a much better way than it was before. Also
does it
allow us to *pave the way for new functionality easily*, without
creating
further mess?
I agree, that's what I'm saying here. I just do not see that happening
with your patch set at all. It's adding more code, a more complex way
to interact with the subsystem, and not making driver writer lives any
easier at all that I can see.
Again, the code is now bigger, does more, with not even any real
benefit
for existing users.
If not, what concrete alternatives do you suggest?
It's working, so leave it alone? :)
So I wanted to note here that I had a similar internal discussion with
Stephen
Boyd when he first upstreamed the request_firmware_into_buf API. I
actually did
change things a bit to pass around a 'desc' structure with all the flags
and parameters in our internal vendor tree [1]. This is a sort of an
ultra-lite
version of what Luis is trying to do - extensibility - but does not
change the API
for driver owners. Existing APIs become wrappers around the new API. My
primary
reason then was the number of things being passed around between layers
of functions
inside firmware_class seemed a bit untenable.
Just like Greg, Stephen also brought up the argument about why the
unnecessary complication to the API without any measurable benefit -
this seemed
a fair argument to me at that time. But here's my point - if Luis agrees
that _this_
patchset is going slightly Mjolnir, perhaps a light internal rework
inside
firmware_class might be more suitable towards the extensibility goal
while keeping
driver API usage as simple as it is today (even if you hate my patch for
various
reasons)?
Thanks,
Vikram
[1] -
https://source.codeaurora.org/quic/la/kernel/msm-3.18/commit/drivers/base/firmware_class.c?h=msm-3.18&id=7aa7efd3c150840369739893a84bd1d9f9774319
