From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933967AbdEKS2V (ORCPT ); Thu, 11 May 2017 14:28:21 -0400 Received: from mx2.suse.de ([195.135.220.15]:39403 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932464AbdEKS2U (ORCPT ); Thu, 11 May 2017 14:28:20 -0400 Date: Thu, 11 May 2017 20:28:18 +0200 From: "Luis R. Rodriguez" To: "Li, Yi" Cc: "Luis R. Rodriguez" , gregkh@linuxfoundation.org, wagi@monom.org, dwmw2@infradead.org, rafal@milecki.pl, arend.vanspriel@broadcom.com, rjw@rjwysocki.net, atull@opensource.altera.com, moritz.fischer@ettus.com, pmladek@suse.com, johannes.berg@intel.com, emmanuel.grumbach@intel.com, luciano.coelho@intel.com, kvalo@codeaurora.org, luto@kernel.org, torvalds@linux-foundation.org, keescook@chromium.org, takahiro.akashi@linaro.org, dhowells@redhat.com, pjones@redhat.com, hdegoede@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 1/5] firmware: add extensible driver data params Message-ID: <20170511182818.GZ28800@wotan.suse.de> References: <20170330032514.17173-1-mcgrof@kernel.org> <20170502084914.23588-1-mcgrof@kernel.org> <20170502084914.23588-2-mcgrof@kernel.org> <3232277e-e7de-68ca-c415-4477ba68c421@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3232277e-e7de-68ca-c415-4477ba68c421@linux.intel.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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 > > +#include > > +#include > > +#include > > + > > +/* > > + * Driver Data internals > > + * > > + * Copyright (C) 2017 Luis R. Rodriguez > > + * > > + * 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