Messages in this thread | | | Date | Thu, 19 Jan 2017 12:36:52 +0100 | From | Greg KH <> | Subject | Re: [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
| |