On Sat, Jun 24, 2017 at 02:39:51PM +0200, Greg KH wrote:
On Sat, Jun 24, 2017 at 02:48:28AM +0200, Luis R. Rodriguez wrote:
> On Fri, Jun 23, 2017 at 04:09:29PM -0700, Linus Torvalds wrote:
> > On Fri, Jun 23, 2017 at 3:43 PM, Luis R. Rodriguez <[email protected]>
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.
>
> Before you, the only person who had expressed disdain here was Greg.
Very few people actually review code, you know that.
Using that logic, then of course "everybody" was *very* fitting ;)
Then again others who actually are working on extending the firmware
API (Yi
Li), or maintaining vendor trees (Vikram), did express their opinions
on the
current codebase and their appreciate for the changes I made, however
this went
selectively unnoticed.
> > 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.
>
> A vendor tree implemented their *own* solution and were willing to maintain
> it despite this likely making it hard to port stable fixes. That I think says
> a lot for a need...
What vendor tree? Where was it shipped?
The msm-3.18 kernel [0], so assuming this goes to mobile devices, this
could
mean millions of devices.
https://source.codeaurora.org/quic/la/kernel/msm-3.18/commit/drivers/base/firmware_class.c?h=msm-3.18&id=7aa7efd3c150840369739893a84bd1d9f9774319
Why was it external and how is it different from your patches?
As is typical with external trees -- it would seem Vikram actually
wrote the
original request_firmware_into_buf() API for the msm tree. It
contained the
fw_desc changes. Stephen Boyd seems to have worked on the actual
upstreaming
effort and he dropped that fw_desc effort from the upstreaming effort.
Vikarm noted he had had a similar internal discussion with Stephen
Stephen Boyd
as I am with you on this thread back when request_firmware_into_buf()
was being
upstreamed [0]. He noted that back then reason for this proposed change
was
that "the number of things being passed around between layers of
functions
inside firmware_class seemed a bit untenable". I will note around that
time I
had proposed a similar change using the fw_desc name, it was only later
that
this renamed to a params postfix as Linus did not like the descriptor
name.
[0]
https://lkml.kernel.org/r/[email protected]
The only difference is that his patch does only modifying the private
members
of the internal API and routines from my patch 1/5, and he kept the
"descriptor" name Linus disliked a while ago. This is precisely why
AKASHI
noted I could split up my patch 1 in more ways in this series to help
*patch
review*.
Was it used because your version has taken so long to be
submitted/reviwed?
Vikram would have a better idea as he is the one who authored it, but
it would
seem this effort was in parallel to my own at that time.
> There are still other requirements and features in the pipeline for which we
> can consider parameters to parse for, rather than adding new API. Case in
> point, do we want *one* API just to disable the firmware cache? Specially
> knowing that another feature in the pipeline later would make use of this as a
> requirement?
Again, I do not care! You can not justify patches today with some
mythical thing in the future that might never even happen.
Some of these features are things actually being discussed for a while,
so to
say they are mythical is not accurate. I can trace back firmware
signing
discussions back to 2015, along with Plumbers in person discussions
where we
seem to have agreed upon a path forward among a few folks who disagreed
on a
technical basis. Linaro has a clear interest so AKASHI picked up that
work now
as I have been busy with general maintainer duties. The fact that Linus
just
suggested an alternative approach to a params approach is new, and yet
to be
reviewe by AKASHI for firmware signing.
Granting the option to make async firmware optional was discussed since
December 2016 by RafaÅ [1]. It was only later during my driver data API
changes
that Hans noted the nvram part was actually *not* optional [2] so this
requirement dropped. *However* as the maintainer I believ ethis
requirement *is
sensible* and would not be surprised if alternative firmware already
exists
where this is what is intended.
