Re: [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option

Saravana Kannan
2020-02-28T03:27:40Z

On Fri, Feb 21, 2020 at 5:40 PM Saravana Kannan <saravanak@google.com> wrote:
>
> fwnode_operations.add_links allows creating device links from
> information provided by firmware.
>
> fwnode_operations.add_links is currently implemented only by
> OF/devicetree code and a specific case of efi. However, there's nothing
> preventing ACPI or other firmware types from implementing it.
>
> The OF implementation is currently controlled by a kernel commandline
> parameter called of_devlink.
>
> Since this feature is generic isn't limited to OF, add a generic
> fw_devlink kernel commandline parameter to control this feature across
> firmware types.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> .../admin-guide/kernel-parameters.txt | 18 +++++++++++++
> drivers/base/core.c | 27 ++++++++++++++++++-
> include/linux/fwnode.h | 2 ++
> 3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index dbc22d684627..29985152b66d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1350,6 +1350,24 @@
> can be changed at run time by the max_graph_depth file
> in the tracefs tracing directory. default: 0 (no limit)
>
> + fw_devlink= [KNL] Create device links between consumer and supplier
> + devices by scanning the firmware to infer the
> + consumer/supplier relationships. This feature is
> + especially useful when drivers are loaded as modules as
> + it ensures proper ordering of tasks like device probing
> + (suppliers first, then consumers), supplier boot state
> + clean up (only after all consumers have probed),
> + suspend/resume & runtime PM (consumers first, then
> + suppliers).
> + Format: { off | permissive | on | rpm }
> + off -- Don't create device links from firmware info.
> + permissive -- Create device links from firmware info
> + but use it only for ordering boot state clean
> + up (sync_state() calls).
> + on -- Create device links from firmware info and use it
> + to enforce probe and suspend/resume ordering.
> + rpm -- Like "on", but also use to order runtime PM.
> +

A bit of bikeshedding myself: I could rename "on" to "enforce" and
"rpm" to "enforce-rpm".

Let me know if any of you have a strong preference on these two options.

-Saravana

> gamecon.map[2|3]=
> [HW,JOY] Multisystem joystick and NES/SNES/PSX pad
> support via parallel port (up to 5 devices per port)
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d32a3aefff32..aeaca8a3aad9 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2345,6 +2345,31 @@ static int device_private_init(struct device *dev)
> return 0;
> }
>
> +u32 fw_devlink_flags;
> +static int __init fw_devlink_setup(char *arg)
> +{
> + if (!arg)
> + return -EINVAL;
> +
> + if (strcmp(arg, "off") == 0) {
> + fw_devlink_flags = 0;
> + } else if (strcmp(arg, "permissive") == 0) {
> + fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY;
> + } else if (strcmp(arg, "on") == 0) {
> + fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER;
> + } else if (strcmp(arg, "rpm") == 0) {
> + fw_devlink_flags = DL_FLAG_AUTOPROBE_CONSUMER |
> + DL_FLAG_PM_RUNTIME;
> + }
> + return 0;
> +}
> +early_param("fw_devlink", fw_devlink_setup);
> +
> +u32 fw_devlink_get_flags(void)
> +{
> + return fw_devlink_flags;
> +}
> +
> /**
> * device_add - add device to device hierarchy.
> * @dev: device.
> @@ -2493,7 +2518,7 @@ int device_add(struct device *dev)
> */
> device_link_add_missing_supplier_links();
>
> - if (fwnode_has_op(dev->fwnode, add_links)) {
> + if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) {
> fw_ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
> if (fw_ret == -ENODEV)
> device_link_wait_for_mandatory_supplier(dev);
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 8feeb94b8acc..e0abafbb17f8 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -170,4 +170,6 @@ struct fwnode_operations {
> } while (false)
> #define get_dev_from_fwnode(fwnode) get_device((fwnode)->dev)
>
> +extern u32 fw_devlink_get_flags(void);
> +
> #endif
> --
> 2.25.0.265.gbab2e86ba0-goog
>