lkml.org 
[lkml]   [2017]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 1/3] firmware: add new extensible firmware API - drvdata
    On Thu, Jan 12, 2017 at 07:02:42AM -0800, Luis R. Rodriguez wrote:
    > The firmware API has evolved over the years slowly, as it
    > grows we extend it by adding new routines or at times we extend
    > existing routines with more or less arguments. This doesn't scale
    > well, when new arguments are added to existing routines it means
    > we need to traverse the kernel with a slew of collateral
    > evolutions to adjust old driver users. The firmware API is also
    > now being used for things outside of the scope of what typically
    > would be considered "firmware", an example here is the p54 driver
    > enables users to provide a custom EEPROM through this interface.
    > Another example is optional CPU microcode updates. This list is
    > actually quite endless...
    >
    > There are other subsystems which would like to make use of the
    > APIs for similar things and its clearly not firmware, but have different
    > requirements and criteria which they'd like to be met for the
    > requested file. If different requirements are needed it would
    > again mean adding more arguments and making a slew of collateral
    > evolutions, or adding yet-another-new-API-call (TM).
    >
    > Another sticking point over the current firmware API is that
    > some callers may need the firmware fallback mechanism when its
    > enabled. There are two types of fallback mechanisms and both have
    > shortcomings. This new API accepts the current status quo and
    > ignore the fallback mechanism all together. When and if we add
    > support for it, it will be well though out.
    >
    > This new extensible firmware API enables new extensions to be added by
    > avoiding future unnecessary collateral evolutions as this code /
    > features get added. This new set of APIs leaves the old firmware API
    > as-is, ignores all firmware fallback mechanism, labels the new
    > API to reflect its broad use outside of the scope of firmware: driver
    > data helpers, and builds on top of the original firmware core code.
    > We purposely try to limit the scope of changes in this new API to
    > simply enable a flexible API to start off with.
    >
    > The new extensible "driver data" set of helpers accepts that there
    > really are only two types of requests for accessing driver data:
    >
    > a) synchronous requests
    > b) asynchronous requests
    >
    > Both of these requests may have a different set of requirements which
    > must be met. These requirements can simply be passed as a struct
    > drvdata_req_params to each type of request. This struct can be extended
    > over time to support different requirements as the kernel evolves.
    >
    > Using the new driver data helpers is only necessary if you have
    > requirements outside of what the existing old firmware API accepts
    > or alternatively if you want to ensure to avoid the old firmware
    > fallback mechanism at all times, regardless of what kernel your driver
    > might run in.
    >
    > Developers with new uses should extend the new new struct drvdata_req_params
    > and driver data code to provide support for new features.
    >
    > A *few* simple features added as part of the new set of driver data
    > request APIs, other than making the new API easily extensible for
    > the future:
    >
    > - The firmware fallback mechanism is currenlty always ignored
    > - By default the kernel will free the driver data file for you after
    > your callbacks are called, you however are allowed to request that
    > you wish to keep the driver data file on the descriptor. The new
    > drvdata API is able to free the drvdata file for you by requiring a
    > consumer callback for the driver data file.
    > - You no longer need to declare and use your own completions, you
    > can replace your completions with drvdata_synchronize_request() using
    > the async_cookie set for you by drvdata_file_request_async(). When
    > drvdata_file_request_async() completes you can rest assured all the
    > work for both triggering, and processing the drvdata using any of
    > your callbacks has completed.
    > - Allow both asynchronous and synchronous request to specify that driver data
    > files are optional. With the old APIs we had added one full API call,
    > request_firmware_direct() just for this purpose -- although it should be
    > noted another one of its goal was to also skip the fallback mechanisms.
    > The driver data request APIs allow for you to annotate that a driver
    > data file is optional for both synchronous or asynchronous requests
    > through the same two basic set of APIs.
    > - The driver data request APIs currently match the old synchronous firmware
    > API calls to refcounted firmware_class module, but it should be easy
    > to add support now to enable also refcounting the caller's module
    > should it be be needed. Likewise the driver data request APIs match the
    > old asynchronous firmware API call and refcounts the caller's module.

    I think this changelog novel is longer than the documentation you added
    to the kernel :(

    > --- /dev/null
    > +++ b/Documentation/driver-api/firmware/drvdata.rst
    > @@ -0,0 +1,91 @@
    > +===========
    > +drvdata API

    Here kid, have a few vowels, we have plenty...

    Please spell this out "driver_data", there's no need to shorten it for
    no reason at all except to confuse people / non-native speakers for a
    while before they figure it out.

    > +===========
    > +
    > +As the kernel evolves we keep extending the firmware_class set of APIs
    > +with more or less arguments, this creates a slew of collateral evolutions.

    Why is this sentance here?

    > +The set of users of firmware request APIs has also grown now to include
    > +users which are not looking for "firmware" per se, but instead general
    > +driver data files which for one reason or another has been decided to be
    > +kept oustide of the kernel, and/or to allow dynamic updates. The driver data
    > +request set of APIs addresses rebranding of firmware as generic driver data
    > +files, and provides a way to enable these APIs to easily be extended without
    > +much collateral evolutions.
    > +
    > +Driver data modes of operation
    > +==============================
    > +
    > +There are only two types of modes of operation for system data requests:
    > +
    > + * synchronous - drvdata_request()
    > + * asynchronous - drvdata_request_async()
    > +
    > +Synchronous requests expect requests to be done immediately, asynchronous
    > +requests enable requests to be scheduled for a later time.
    > +
    > +Driver data request parameters
    > +==============================
    > +
    > +Variations of types of driver data requests are specified by a driver data
    > +request parameter data structure. This data structure can grow as with new
    > +fields as requirements grow. The old firmware API provides two synchronous
    > +requests: request_firmware() and request_firmware_direct(), the later allowing
    > +the caller to specify that the "driver data file" is optional. The driver data
    > +request API allows a caller to set the optional nature of the driver data
    > +on the request parameter data structure using the same synchronous API. Since
    > +this requirement is part of the request paramter data structure it also allows
    > +asynchronous requests to specify that the driver data is optional.
    > +
    > +Reference counting and releasing the system data file
    > +=====================================================
    > +
    > +As with the old firmware API both the device and module are bumped with
    > +reference counts during the driver data requests. This prevents removal
    > +of the device and module making the driver data request call until the
    > +driver data request callbacks have completed, either synchronously or
    > +asynchronously.
    > +
    > +The old firmware APIs refcounted the firmware_class module for synchronous
    > +requests, meanwhile asynchronous requests refcounted the caller's module.
    > +The driver data request API currently mimic this behaviour, for synchronous
    > +requests the firmware_class module is refcounted through the use of
    > +dfl_sync_reqs, although if in the future we may later enable use of
    > +also refcounting the caller's module as well. Likewise in the future we
    > +may extend asynchronous calls to refcount the firmware_class module.
    > +
    > +Typical use of the old synchronous firmware APIs consist of the caller
    > +requesting for "driver data", consuming it after a request and finally
    > +freeing it. Typical asynchronous use of the old firmware APIs consist of
    > +the caller requesting for "driver data" and then finally freeing it on
    > +asynchronous callback.
    > +
    > +The driver data request API enables callers to provide a callback for both
    > +synchronous and asynchronous requests and since consumption can be expected
    > +in these callbacks it frees it for you by default after callback handlers
    > +are issued. If you wish to keep the driver data around after your callbacks
    > +you must specify this through the driver data request paramter data structure.
    > +
    > +Async cookies, replacing completions
    > +====================================
    > +
    > +With this new API you do not need to declare and use your own completions, you

    It's not going to be "new" in a year, are you going to go and change the
    documentation here?

    And if you want to provide a "how to convert from firmware to
    driver_data" document, great, but to constantly compare the two seems a
    bit like you are trying too hard. It should stand on it's own without
    needing to do that.

    > +can replace your completions with drvdata_synchronize_request() using the
    > +async_cookie set for you by drvdata_file_request_async(). When
    > +drvdata_file_request_async() completes you can rest assured all the work for
    > +both triggering, and processing the drvdata using any of your callbacks has
    > +completed.
    > +
    > +Fallback mechanisms on the driver data API
    > +==========================================
    > +
    > +The old firmware API provided support for a series of fallback mechanisms. The
    > +new driver data API abandons all current notions of the fallback mechanisms,
    > +it may soon add support for one though.

    Oh come on, is this paragraph really needed at all? "soon"? Hah.


    > +Tracking development enhancements and ideas
    > +===========================================
    > +
    > +To help track ongoing development for firmware_class and related items to
    > +firmware_class refer to the kernel newbies wiki page [0].
    > +
    > +[0] http://kernelnewbies.org/KernelProjects/firmware-class-enhancements
    > diff --git a/Documentation/driver-api/firmware/index.rst b/Documentation/driver-api/firmware/index.rst
    > index 1abe01793031..8d275c4c165b 100644
    > --- a/Documentation/driver-api/firmware/index.rst
    > +++ b/Documentation/driver-api/firmware/index.rst
    > @@ -7,6 +7,7 @@ Linux Firmware API
    > introduction
    > core
    > request_firmware
    > + drvdata
    >
    > .. only:: subproject and html
    >
    > diff --git a/Documentation/driver-api/firmware/introduction.rst b/Documentation/driver-api/firmware/introduction.rst
    > index 211cb44eb972..d7d5ef846ca0 100644
    > --- a/Documentation/driver-api/firmware/introduction.rst
    > +++ b/Documentation/driver-api/firmware/introduction.rst
    > @@ -25,3 +25,14 @@ are already using asynchronous initialization mechanisms which will not
    > stall or delay boot. Even if loading firmware does not take a lot of time
    > processing firmware might, and this can still delay boot or initialization,
    > as such mechanisms such as asynchronous probe can help supplement drivers.
    > +
    > +Two APIs
    > +========
    > +
    > +Two APIs are provided for firmware:
    > +
    > +* request_firmware API - old firmware API
    > +* drvdata API - new flexible API

    "new" isn't "new" in a few months.

    thanks,

    greg k-h

    \
     
     \ /
      Last update: 2017-01-19 12:37    [W:8.784 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site