On Thu, May 11, 2017 at 01:17:39PM -0500, Li, Yi wrote: > On 5/2/2017 3:49 AM, 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. > > > > We introduce 3 data structures: > > > > o struct driver_data_req_params - used for user specified parameters > > o struct driver_data_priv_params - used for internal use only > > o struct driver_data_params - stiches both of the the above together, > > only for internal use > > > > This starts off by just making the existing APIs use the new data > > structures, it will make subsequent changes easier to review which will > > be adding new flexible APIs. > > > > A side consequences is get to replace all the old internal "firmware > > behavior options" flags with enums we properly document, remove the > > blinding #ifdefs, and compartamentlize the userhelper fallback code > > more appropriately unde CONFIG_FW_LOADER_USER_HELPER_FALLBACK. > > > > This commit should introduces no functional changes (TM). > > > > Signed-off-by: Luis R. Rodriguez <[email protected]> > > --- > > drivers/base/firmware_class.c | 331 > > ++++++++++++++++++++++++++++++++---------- > > include/linux/driver_data.h | 82 +++++++++++ > > 2 files changed, 339 insertions(+), 74 deletions(-) > > create mode 100644 include/linux/driver_data.h > > ... > > diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h > > new file mode 100644 > > index 000000000000..7ce3216a9a99 > > --- /dev/null > > +++ b/include/linux/driver_data.h > > @@ -0,0 +1,82 @@ > > +#ifndef _LINUX_DRIVER_DATA_H > > +#define _LINUX_DRIVER_DATA_H > > + > > +#include <linux/types.h> > > +#include <linux/compiler.h> > > +#include <linux/gfp.h> > > +#include <linux/device.h> > > + > > +/* > > + * Driver Data internals > > + * > > + * Copyright (C) 2017 Luis R. Rodriguez <[email protected]> > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of copyleft-next (version 0.3.1 or later) as published > > + * at http://copyleft-next.org/. > > + */ > > + > > +/** > > + * struct driver_data_async_cbs - callbacks for handling driver data > > requests > > + * @found_cb: callback to be used when the driver data has been found. A > > + * callback is required. If the requested driver data is found it will > > + * passed on the callback, using the context set on @found_ctx. > > + * @found_ctx: preferred context to be used as the second argument to > > + * @found_cb. > > + * > > + * Used for specifying callbacks and contexts used for when asynchronous > > driver > > + * data requests have completed. If no driver data is found the error will > > be > > + * passed on the respective callback. > > + */ > > +struct driver_data_async_cbs { > > + void (*found_cb)(const struct firmware *driver_data, > > + void *context, > > + int error); > > Any reason why the call back function format are not consistent between > async and sync mode. In sync mode, the context is before struct firmware. > in Async mode, it's reversed. > > struct driver_data_sync_cbs { > int __must_check > (*found_cb)(void *context, > const struct firmware *driver_data, > int error); >
Indeed, its just to ensure if the user makes a mistake and uses a sync callback for an async request we get a compile error, all without using typdefs or anything nasty. Luis

