linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/5] Add fw_devlink kernel commandline option
@ 2020-02-22  1:40 Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 1/5] driver core: Reevaluate dev->links.need_for_probe as suppliers are added Saravana Kannan
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Saravana Kannan, kernel-team, linux-doc, linux-kernel, linux-efi,
	devicetree, linux-acpi

1/5 isn't really related to adding fw_devlink, but I'm just rolling it
into this series.

The rest of the series is about adding fw_devlink kernel commandline
that can be set to off|permissive|on|rpm which are in increasing order
of enforcement by device links.

2/5 is the most "interesting" part.

The hope is that once this series merges, we'll consider enabling
fw_devlink=permissive by default and at a later point in time ideally
"on".

Thanks,
Saravana

Saravana Kannan (5):
  driver core: Reevaluate dev->links.need_for_probe as suppliers are
    added
  driver core: Add fw_devlink kernel commandline option
  efi/arm: Start using fw_devlink_get_flags()
  of: property: Start using fw_devlink_get_flags()
  of: property: Delete of_devlink kernel commandline option

 .../admin-guide/kernel-parameters.txt         | 24 +++++++++----
 drivers/base/core.c                           | 35 +++++++++++++++++--
 drivers/firmware/efi/arm-init.c               |  2 +-
 drivers/of/property.c                         |  8 +----
 include/linux/fwnode.h                        |  2 ++
 5 files changed, 54 insertions(+), 17 deletions(-)

-- 
2.25.0.265.gbab2e86ba0-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/5] driver core: Reevaluate dev->links.need_for_probe as suppliers are added
  2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
@ 2020-02-22  1:40 ` Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option Saravana Kannan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown,
	Saravana Kannan
  Cc: kernel-team, linux-doc, linux-kernel, linux-efi, devicetree, linux-acpi

A previous patch 03324507e66c ("driver core: Allow
fwnode_operations.add_links to differentiate errors") forgot to update
all call sites to fwnode_operations.add_links. This patch fixes that.

Legend:
-> Denotes RHS is an optional/potential supplier for LHS
=> Denotes RHS is a mandatory supplier for LHS

Example:

Device A => Device X
Device A -> Device Y

Before this patch:
1. Device A is added.
2. Device A is marked as waiting for mandatory suppliers
3. Device X is added
4. Device A is left marked as waiting for mandatory suppliers

Step 4 is wrong since all mandatory suppliers of Device A have been
added.

After this patch:
1. Device A is added.
2. Device A is marked as waiting for mandatory suppliers
3. Device X is added
4. Device A is no longer considered as waiting for mandatory suppliers

This is the correct behavior.

Fixes: 03324507e66c ("driver core: Allow fwnode_operations.add_links to differentiate errors")
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index dbb0f9130f42..d32a3aefff32 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -523,9 +523,13 @@ static void device_link_add_missing_supplier_links(void)
 
 	mutex_lock(&wfs_lock);
 	list_for_each_entry_safe(dev, tmp, &wait_for_suppliers,
-				 links.needs_suppliers)
-		if (!fwnode_call_int_op(dev->fwnode, add_links, dev))
+				 links.needs_suppliers) {
+		int ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
+		if (!ret)
 			list_del_init(&dev->links.needs_suppliers);
+		else if (ret != -ENODEV)
+			dev->links.need_for_probe = false;
+	}
 	mutex_unlock(&wfs_lock);
 }
 
-- 
2.25.0.265.gbab2e86ba0-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option
  2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 1/5] driver core: Reevaluate dev->links.need_for_probe as suppliers are added Saravana Kannan
@ 2020-02-22  1:40 ` Saravana Kannan
  2020-02-28  3:27   ` Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags() Saravana Kannan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Saravana Kannan, kernel-team, linux-doc, linux-kernel, linux-efi,
	devicetree, linux-acpi

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.
+
 	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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags()
  2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 1/5] driver core: Reevaluate dev->links.need_for_probe as suppliers are added Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option Saravana Kannan
@ 2020-02-22  1:40 ` Saravana Kannan
  2020-02-22  7:37   ` Ard Biesheuvel
  2020-02-22  1:40 ` [PATCH v1 4/5] of: property: " Saravana Kannan
  2020-02-22  1:40 ` [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option Saravana Kannan
  4 siblings, 1 reply; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Saravana Kannan, kernel-team, linux-doc, linux-kernel, linux-efi,
	devicetree, linux-acpi

The fw_devlink_get_flags() provides the right flags to use when creating
mandatory device links derived from information provided by the
firmware. So, use that.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/firmware/efi/arm-init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index d99f5b0c8a09..6703bedfa9e1 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -349,7 +349,7 @@ static int efifb_add_links(const struct fwnode_handle *fwnode,
 	 * If this fails, retrying this function at a later point won't
 	 * change anything. So, don't return an error after this.
 	 */
-	if (!device_link_add(dev, sup_dev, 0))
+	if (!device_link_add(dev, sup_dev, fw_devlink_get_flags()))
 		dev_warn(dev, "device_link_add() failed\n");
 
 	put_device(sup_dev);
-- 
2.25.0.265.gbab2e86ba0-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 4/5] of: property: Start using fw_devlink_get_flags()
  2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
                   ` (2 preceding siblings ...)
  2020-02-22  1:40 ` [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags() Saravana Kannan
@ 2020-02-22  1:40 ` Saravana Kannan
  2020-02-26 21:40   ` Rob Herring
  2020-02-22  1:40 ` [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option Saravana Kannan
  4 siblings, 1 reply; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Saravana Kannan, kernel-team, linux-doc, linux-kernel, linux-efi,
	devicetree, linux-acpi

The fw_devlink_get_flags() provides the right flags to use when creating
mandatory device links derived from information provided by the
firmware. So, use that.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index e851c57a15b0..15fc9315f1a7 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1262,7 +1262,7 @@ static int of_link_property(struct device *dev, struct device_node *con_np,
 	u32 dl_flags;
 
 	if (dev->of_node == con_np)
-		dl_flags = DL_FLAG_AUTOPROBE_CONSUMER;
+		dl_flags = fw_devlink_get_flags();
 	else
 		dl_flags = DL_FLAG_SYNC_STATE_ONLY;
 
-- 
2.25.0.265.gbab2e86ba0-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option
  2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
                   ` (3 preceding siblings ...)
  2020-02-22  1:40 ` [PATCH v1 4/5] of: property: " Saravana Kannan
@ 2020-02-22  1:40 ` Saravana Kannan
  2020-02-26 21:41   ` Rob Herring
  2020-03-31 12:20   ` Geert Uytterhoeven
  4 siblings, 2 replies; 12+ messages in thread
From: Saravana Kannan @ 2020-02-22  1:40 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Saravana Kannan, kernel-team, linux-doc, linux-kernel, linux-efi,
	devicetree, linux-acpi

With the addition of fw_devlink kernel commandline option, of_devlink is
redundant and not useful anymore. So, delete it.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 6 ------
 drivers/of/property.c                           | 6 ------
 2 files changed, 12 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 29985152b66d..6692b2aa6140 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3299,12 +3299,6 @@
 			This can be set from sysctl after boot.
 			See Documentation/admin-guide/sysctl/vm.rst for details.
 
-	of_devlink	[OF, KNL] Create device links between consumer and
-			supplier devices by scanning the devictree to infer the
-			consumer/supplier relationships.  A consumer device
-			will not be probed until all the supplier devices have
-			probed successfully.
-
 	ohci1394_dma=early	[HW] enable debugging via the ohci1394 driver.
 			See Documentation/debugging-via-ohci1394.txt for more
 			info.
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 15fc9315f1a7..f104f15b57fb 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev,
 	return ret;
 }
 
-static bool of_devlink;
-core_param(of_devlink, of_devlink, bool, 0);
-
 static int of_fwnode_add_links(const struct fwnode_handle *fwnode,
 			       struct device *dev)
 {
-	if (!of_devlink)
-		return 0;
-
 	if (unlikely(!is_of_node(fwnode)))
 		return 0;
 
-- 
2.25.0.265.gbab2e86ba0-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags()
  2020-02-22  1:40 ` [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags() Saravana Kannan
@ 2020-02-22  7:37   ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2020-02-22  7:37 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Rob Herring, Frank Rowand, Len Brown, Android Kernel Team,
	Linux Doc Mailing List, Linux Kernel Mailing List, linux-efi,
	Devicetree List, ACPI Devel Maling List

On Sat, 22 Feb 2020 at 02:40, Saravana Kannan <saravanak@google.com> wrote:
>
> The fw_devlink_get_flags() provides the right flags to use when creating
> mandatory device links derived from information provided by the
> firmware. So, use that.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---

Acked-by: Ard Biesheuvel <ardb@kernel.org>

>  drivers/firmware/efi/arm-init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index d99f5b0c8a09..6703bedfa9e1 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -349,7 +349,7 @@ static int efifb_add_links(const struct fwnode_handle *fwnode,
>          * If this fails, retrying this function at a later point won't
>          * change anything. So, don't return an error after this.
>          */
> -       if (!device_link_add(dev, sup_dev, 0))
> +       if (!device_link_add(dev, sup_dev, fw_devlink_get_flags()))
>                 dev_warn(dev, "device_link_add() failed\n");
>
>         put_device(sup_dev);
> --
> 2.25.0.265.gbab2e86ba0-goog
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 4/5] of: property: Start using fw_devlink_get_flags()
  2020-02-22  1:40 ` [PATCH v1 4/5] of: property: " Saravana Kannan
@ 2020-02-26 21:40   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2020-02-26 21:40 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Saravana Kannan, kernel-team, linux-doc,
	linux-kernel, linux-efi, devicetree, linux-acpi

On Fri, 21 Feb 2020 17:40:37 -0800, Saravana Kannan wrote:
> The fw_devlink_get_flags() provides the right flags to use when creating
> mandatory device links derived from information provided by the
> firmware. So, use that.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/of/property.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option
  2020-02-22  1:40 ` [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option Saravana Kannan
@ 2020-02-26 21:41   ` Rob Herring
  2020-03-31 12:20   ` Geert Uytterhoeven
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2020-02-26 21:41 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Saravana Kannan, kernel-team, linux-doc,
	linux-kernel, linux-efi, devicetree, linux-acpi

On Fri, 21 Feb 2020 17:40:38 -0800, Saravana Kannan wrote:
> With the addition of fw_devlink kernel commandline option, of_devlink is
> redundant and not useful anymore. So, delete it.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 6 ------
>  drivers/of/property.c                           | 6 ------
>  2 files changed, 12 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option
  2020-02-22  1:40 ` [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option Saravana Kannan
@ 2020-02-28  3:27   ` Saravana Kannan
  0 siblings, 0 replies; 12+ messages in thread
From: Saravana Kannan @ 2020-02-28  3:27 UTC (permalink / raw)
  To: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown
  Cc: Android Kernel Team, Linux Doc Mailing List, LKML, linux-efi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ACPI Devel Maling List

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option
  2020-02-22  1:40 ` [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option Saravana Kannan
  2020-02-26 21:41   ` Rob Herring
@ 2020-03-31 12:20   ` Geert Uytterhoeven
  2020-03-31 17:10     ` Saravana Kannan
  1 sibling, 1 reply; 12+ messages in thread
From: Geert Uytterhoeven @ 2020-03-31 12:20 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown,
	Android Kernel Team, open list:DOCUMENTATION,
	Linux Kernel Mailing List, linux-efi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ACPI Devel Maling List

Hi Saravana,

On Sat, Feb 22, 2020 at 2:41 AM Saravana Kannan <saravanak@google.com> wrote:
> With the addition of fw_devlink kernel commandline option, of_devlink is
> redundant and not useful anymore. So, delete it.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Thanks for your patch!

This is now commit e94f62b7140fa3da ("of: property: Delete of_devlink
kernel commandline option") upstream.

> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3299,12 +3299,6 @@
>                         This can be set from sysctl after boot.
>                         See Documentation/admin-guide/sysctl/vm.rst for details.
>
> -       of_devlink      [OF, KNL] Create device links between consumer and
> -                       supplier devices by scanning the devictree to infer the
> -                       consumer/supplier relationships.  A consumer device
> -                       will not be probed until all the supplier devices have
> -                       probed successfully.
> -
>         ohci1394_dma=early      [HW] enable debugging via the ohci1394 driver.
>                         See Documentation/debugging-via-ohci1394.txt for more
>                         info.

While I agree with the thunk above...

> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 15fc9315f1a7..f104f15b57fb 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev,
>         return ret;
>  }
>
> -static bool of_devlink;
> -core_param(of_devlink, of_devlink, bool, 0);
> -
>  static int of_fwnode_add_links(const struct fwnode_handle *fwnode,
>                                struct device *dev)
>  {
> -       if (!of_devlink)
> -               return 0;
> -
>         if (unlikely(!is_of_node(fwnode)))
>                 return 0;

... I have some reservations about removing the actual code.
The "of_devlink" kernel parameter was supported in v5.5 and v5.6, so
removing its support may silently break some setups.

Is this likely to happen?
Do we need a compatibility fallback that warns to user to update his kernel
command line?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option
  2020-03-31 12:20   ` Geert Uytterhoeven
@ 2020-03-31 17:10     ` Saravana Kannan
  0 siblings, 0 replies; 12+ messages in thread
From: Saravana Kannan @ 2020-03-31 17:10 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jonathan Corbet, Greg Kroah-Hartman, Rafael J. Wysocki,
	Ard Biesheuvel, Rob Herring, Frank Rowand, Len Brown,
	Android Kernel Team, open list:DOCUMENTATION,
	Linux Kernel Mailing List, linux-efi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	ACPI Devel Maling List

On Tue, Mar 31, 2020 at 5:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Saravana,
>
> On Sat, Feb 22, 2020 at 2:41 AM Saravana Kannan <saravanak@google.com> wrote:
> > With the addition of fw_devlink kernel commandline option, of_devlink is
> > redundant and not useful anymore. So, delete it.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
>
> Thanks for your patch!
>
> This is now commit e94f62b7140fa3da ("of: property: Delete of_devlink
> kernel commandline option") upstream.
>
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -3299,12 +3299,6 @@
> >                         This can be set from sysctl after boot.
> >                         See Documentation/admin-guide/sysctl/vm.rst for details.
> >
> > -       of_devlink      [OF, KNL] Create device links between consumer and
> > -                       supplier devices by scanning the devictree to infer the
> > -                       consumer/supplier relationships.  A consumer device
> > -                       will not be probed until all the supplier devices have
> > -                       probed successfully.
> > -
> >         ohci1394_dma=early      [HW] enable debugging via the ohci1394 driver.
> >                         See Documentation/debugging-via-ohci1394.txt for more
> >                         info.
>
> While I agree with the thunk above...
>
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > index 15fc9315f1a7..f104f15b57fb 100644
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1299,15 +1299,9 @@ static int of_link_to_suppliers(struct device *dev,
> >         return ret;
> >  }
> >
> > -static bool of_devlink;
> > -core_param(of_devlink, of_devlink, bool, 0);
> > -
> >  static int of_fwnode_add_links(const struct fwnode_handle *fwnode,
> >                                struct device *dev)
> >  {
> > -       if (!of_devlink)
> > -               return 0;
> > -
> >         if (unlikely(!is_of_node(fwnode)))
> >                 return 0;

Hi Geert,

> ... I have some reservations about removing the actual code.
> The "of_devlink" kernel parameter was supported in v5.5 and v5.6, so
> removing its support may silently break some setups.
>
> Is this likely to happen?

As much as I'd love to see people start using this, I doubt of_devlink
has been significantly adopted outside of Android yet (I'm working on
making that easier :)). I'd be happy to be proven wrong though :)

of_devlink/fw_devlink is mainly useful when module loading is causing
ordering and clean up issues. So if deletion of of_devlink breaks
anything, I'd expect it to break in obvious ways and not silently.

> Do we need a compatibility fallback that warns to user to update his kernel
> command line?

I don't think we need to, but I'm not strongly against your suggestion
either. Let us know what you think.

-Saravana

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-31 17:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-22  1:40 [PATCH v1 0/5] Add fw_devlink kernel commandline option Saravana Kannan
2020-02-22  1:40 ` [PATCH v1 1/5] driver core: Reevaluate dev->links.need_for_probe as suppliers are added Saravana Kannan
2020-02-22  1:40 ` [PATCH v1 2/5] driver core: Add fw_devlink kernel commandline option Saravana Kannan
2020-02-28  3:27   ` Saravana Kannan
2020-02-22  1:40 ` [PATCH v1 3/5] efi/arm: Start using fw_devlink_get_flags() Saravana Kannan
2020-02-22  7:37   ` Ard Biesheuvel
2020-02-22  1:40 ` [PATCH v1 4/5] of: property: " Saravana Kannan
2020-02-26 21:40   ` Rob Herring
2020-02-22  1:40 ` [PATCH v1 5/5] of: property: Delete of_devlink kernel commandline option Saravana Kannan
2020-02-26 21:41   ` Rob Herring
2020-03-31 12:20   ` Geert Uytterhoeven
2020-03-31 17:10     ` Saravana Kannan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).