On Wed, 2017-03-29 at 20:25 -0700, 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
> o struct driver_data_params - stiches both of the the above together,
> also 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.
>
> This commit should introduces no functional changes (TM).
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
> ---
Looks fine with a few nitpicks.
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 54fc4c42f126..f702566554e1 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
[...]
> @@ -40,6 +41,77 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
> MODULE_DESCRIPTION("Multi purpose firmware loading support");
> MODULE_LICENSE("GPL");
>
> +struct driver_data_priv_params {
> + bool use_fallback;
> + bool use_fallback_uevent;
> + bool no_cache;
> + void *alloc_buf;
> + size_t alloc_buf_size;
> +};
> +
> +struct driver_data_params {
> + const struct firmware *driver_data;
> + const struct driver_data_req_params req_params;
> + struct driver_data_priv_params priv_params;
> +};
> +
> +/*
> + * These are kept to remain backward compatible with old behaviour. Do not
> + * modify them unless you know what you are doing. These are to be used only
> + * by the old API, so:
> + *
> + * Old sync APIs:
> + * o request_firmware(): __DATA_REQ_FIRMWARE()
> + * o request_firmware_direct(): __DATA_REQ_FIRMWARE_DIRECT()
> + * o request_firmware_into_buf(): __DATA_REQ_FIRMWARE_BUF()
> + *
> + * Old async API:
> + * o request_firmware_nowait(): __DATA_REQ_FIRMWARE_NOWAIT()
> + */
> +#define __DATA_REQ_FIRMWARE()
> \
> + .priv_params = { \
> + .use_fallback = !!FW_OPT_FALLBACK, \
use_fallback is a boolean, so you don't need the double negation here.
[...]
> @@ -1332,12 +1433,15 @@ request_firmware_into_buf(const struct firmware
> **firmware_p, const char *name,
> struct device *device, void *buf, size_t size)
> {
> int ret;
> + struct driver_data_params data_params = {
> + __DATA_REQ_FIRMWARE_BUF(buf, size),
> + };
>
> __module_get(THIS_MODULE);
> - ret = _request_firmware(firmware_p, name, device, buf, size,
> - FW_OPT_UEVENT | FW_OPT_FALLBACK |
> - FW_OPT_NOCACHE);
> + ret = _request_firmware(firmware_p, name, &data_params, device);
> module_put(THIS_MODULE);
> +
> +
Double empty-lines here.
> return ret;
> }
> EXPORT_SYMBOL(request_firmware_into_buf);
>
[...]
> diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h
> new file mode 100644
> index 000000000000..c3d3a4129838
> --- /dev/null
> +++ b/include/linux/driver_data.h
> @@ -0,0 +1,88 @@
> +#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/.
> + */
> +
> +/**
> + * enum driver_data_mode - driver data mode of operation
> + *
> + * DRIVER_DATA_SYNC: your call to request driver data is synchronous. We will
> + * look for the driver data file you have requested immediatley.
> + * DRIVER_DATA_ASYNC: your call to request driver data is asynchronous. We
> will
It should be @DRIVER_DATA_SYNC and @DRIVER_DATA_ASYNC here.
--
Cheers,
Luca.
