[RFC PATCH 00/12] platform/x86: intel_cht_int33fe: Real DisplayPort reference

[RFC PATCH 00/12] platform/x86: intel_cht_int33fe: Real DisplayPort reference

[Heikki Krogerus]

Hi Hans,

This is RFC, so not CCing everybody who should be CCd yet.

I spend a bit of time on thinking how could we use real fwnode
references instead of that string you proposed in order to hook up the
USB Type-C connector with the DisplayPort connector. This is what I
came up with. I was planning on introducing the references to software
nodes in any case like I mentioned before.

I'm not yet adding any sub-nodes for the DP alt mode. I'm just
assigning references to all the relevant components (nodes) for the
USB Type-C connector node. I think that the reference to the
DisplayPort should be assigned to the USB Type-C connector and not
to the alt mode sub-node.

Using these you should be able to add that notifier, and in it just
get the reference to the correct DisplayPort node from the USB Type-C
connector, and then you can simply walk through the drm_connectors and
match using the fwnode.


thanks,


Heikki Krogerus (12):
  software node: Prevent potential NULL Pointer Dereference
  software node: Increment parent node's ref count
  software node: Add support for references
  software node: Implement .get_reference_args fwnode operation
  ACPI / property: Don't limit named child node matching to data nodes
  device connection: Find connections also by checking the references
  platform/x86: intel_cht_int33fe: Provide software node for all
    components
  platform/x86: intel_cht_int33fe: Provide fwnode for the USB connector
  platform/x86: intel_cht_int33fe: Link with external dependencies using
    fwnodes
  platform/x86: intel_cht_int33fe: Replacing the old connections with
    references
  drm: Add fwnode member to the struct drm_connector
  drm/i915: Associate the ACPI connector nodes with connector entries

 drivers/acpi/property.c                  |  26 +-
 drivers/base/devcon.c                    |  28 ++
 drivers/base/swnode.c                    | 165 ++++++++++-
 drivers/gpu/drm/drm_sysfs.c              |  49 +++-
 drivers/gpu/drm/i915/intel_display.c     |  41 +++
 drivers/platform/x86/intel_cht_int33fe.c | 349 ++++++++++++++++++++---
 include/drm/drm_connector.h              |   2 +
 include/linux/property.h                 |   8 +
 8 files changed, 599 insertions(+), 69 deletions(-)

-- 
2.20.1

[PATCH 01/12] software node: Prevent potential NULL Pointer Dereference

[Heikki Krogerus]

Software nodes are not forced to have device properties.
Adding check to property_entries_dup() to make it possible
to create software nodes that don't have any properties.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 1fad9291f6aa..3a50a8edd3d3 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -383,6 +383,9 @@ property_entries_dup(const struct property_entry *properties)
 	int i, n = 0;
 	int ret;
 
+	if (!properties)
+		return NULL;
+
 	while (properties[n].name)
 		n++;
 
-- 
2.20.1

[01/12] software node: Prevent potential NULL Pointer Dereference

[Heikki Krogerus]

Software nodes are not forced to have device properties.
Adding check to property_entries_dup() to make it possible
to create software nodes that don't have any properties.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 1fad9291f6aa..3a50a8edd3d3 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -383,6 +383,9 @@ property_entries_dup(const struct property_entry *properties)
 	int i, n = 0;
 	int ret;
 
+	if (!properties)
+		return NULL;
+
 	while (properties[n].name)
 		n++;
 

[PATCH 02/12] software node: Increment parent node's ref count

[Heikki Krogerus]

If the parent node is removed before its children, release()
may be called first for the parent instead of the last child
when the child's ref count reaches zero. Since the nodes
release their parent's resources in the release callback,
that would cause NULL pointer dereference to happen. To
prevent that from ever happening, incrementing the parent
node's reference count when creating a new child node and
decrementing it when the child node is "released".

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 3a50a8edd3d3..8d4485d8d0f8 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -560,6 +560,7 @@ static void software_node_release(struct kobject *kobj)
 	if (swnode->parent) {
 		ida_simple_remove(&swnode->parent->child_ids, swnode->id);
 		list_del(&swnode->entry);
+		kobject_put(&swnode->parent->kobj);
 	} else {
 		ida_simple_remove(&swnode_root_ids, swnode->id);
 	}
@@ -610,8 +611,10 @@ fwnode_create_software_node(const struct property_entry *properties,
 	INIT_LIST_HEAD(&swnode->children);
 	swnode->parent = p;
 
-	if (p)
+	if (p) {
+		kobject_get(&p->kobj);
 		list_add_tail(&swnode->entry, &p->children);
+	}
 
 	ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
 				   p ? &p->kobj : NULL, "node%d", swnode->id);
-- 
2.20.1

[02/12] software node: Increment parent node's ref count

[Heikki Krogerus]

If the parent node is removed before its children, release()
may be called first for the parent instead of the last child
when the child's ref count reaches zero. Since the nodes
release their parent's resources in the release callback,
that would cause NULL pointer dereference to happen. To
prevent that from ever happening, incrementing the parent
node's reference count when creating a new child node and
decrementing it when the child node is "released".

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 3a50a8edd3d3..8d4485d8d0f8 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -560,6 +560,7 @@ static void software_node_release(struct kobject *kobj)
 	if (swnode->parent) {
 		ida_simple_remove(&swnode->parent->child_ids, swnode->id);
 		list_del(&swnode->entry);
+		kobject_put(&swnode->parent->kobj);
 	} else {
 		ida_simple_remove(&swnode_root_ids, swnode->id);
 	}
@@ -610,8 +611,10 @@ fwnode_create_software_node(const struct property_entry *properties,
 	INIT_LIST_HEAD(&swnode->children);
 	swnode->parent = p;
 
-	if (p)
+	if (p) {
+		kobject_get(&p->kobj);
 		list_add_tail(&swnode->entry, &p->children);
+	}
 
 	ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
 				   p ? &p->kobj : NULL, "node%d", swnode->id);

[PATCH 03/12] software node: Add support for references

[Heikki Krogerus]

Introducing functions that can be used for adding and
removing references to other software nodes. The goal is to
support fwnode_property_get_reference_args() also with
software nodes, however, get_reference_args fwnode operation
callback is not yet implemented in this commit for the
software nodes. This commit will only add support for
reference addition/removal.

The next example shows how to add references to GPIOs using the
format described in Documentation/acpi/gpio-properties.txt:

/* Array with two GPIOs */
static struct fwnode_reference_args gpioargs[] __initdata = {
	{
		.nargs = 3,
		.args[0]= 19, /* index */
		.args[1]= 0,  /* pin */
		.args[2]= 0,  /* active_low */
	},
	{
		.nargs = 3,
		.args[0]= 20, /* index */
		.args[1]= 0,  /* pin */
		.args[2]= 0,  /* active_low */
	},
	{ }
};

static int myinit(void)
{
	struct software_node_reference *ref;
	struct fwnode_handle *gpio_node;
	struct fwnode_handle *my_node;

	/* Creating the nodes */
	gpio_node = fwnode_create_software_node(gpio_props, NULL);
	...
	my_node = fwnode_create_software_node(my_props, NULL);
	...

	/* gpio_node is associated with a GPIO/Pin controller in this example */
	...

	/* Assigning the actual node references */
	gpioargs[0].fwnode = gpio_node;
	gpioargs[1].fwnode = gpio_node;

	/* my_node will now have a named ("gpios") reference to the two GPIOs */
	ref = fwnode_create_software_node_reference(my_node, "gpios", gpioargs);
	...
	return 0;
}

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c    | 101 +++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |   8 ++++
 2 files changed, 109 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 8d4485d8d0f8..591f290d25be 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -11,10 +11,19 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
+struct software_node_reference {
+	struct list_head list;
+	const char *name;
+	int nrefs;
+	struct fwnode_reference_args *args;
+};
+
 struct software_node {
 	int id;
 	struct kobject kobj;
 	struct fwnode_handle fwnode;
+	struct list_head references;
+	struct mutex lock; /* node lock */
 
 	/* hierarchy */
 	struct ida child_ids;
@@ -606,9 +615,11 @@ fwnode_create_software_node(const struct property_entry *properties,
 	swnode->kobj.kset = swnode_kset;
 	swnode->fwnode.ops = &software_node_ops;
 
+	mutex_init(&swnode->lock);
 	ida_init(&swnode->child_ids);
 	INIT_LIST_HEAD(&swnode->entry);
 	INIT_LIST_HEAD(&swnode->children);
+	INIT_LIST_HEAD(&swnode->references);
 	swnode->parent = p;
 
 	if (p) {
@@ -641,10 +652,100 @@ void fwnode_remove_software_node(struct fwnode_handle *fwnode)
 	if (!swnode)
 		return;
 
+	mutex_lock(&swnode->lock);
+	WARN(!list_empty(&swnode->references),
+	     "\"%s\" has still references", kobject_name(&swnode->kobj));
+	mutex_unlock(&swnode->lock);
+
 	kobject_put(&swnode->kobj);
 }
 EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
 
+/**
+ * fwnode_create_software_node_reference - Create named reference description
+ * @fwnode: The software node to have the references
+ * @name: Name given to reference description
+ * @args: Zero terminated array of software node references with arguments
+ *
+ * Associates software nodes listed in @args with @fwnode. The association is
+ * named @name. The reference count is incremented for the nodes in @args.
+ *
+ * Returns pointer to software node reference description on success, or ERR_PTR
+ * on failure.
+ */
+struct software_node_reference *
+fwnode_create_software_node_reference(const struct fwnode_handle *fwnode,
+				      const char *name,
+				      const struct fwnode_reference_args *args)
+{
+	struct software_node *swnode = to_software_node(fwnode);
+	struct software_node_reference *ref;
+	int n;
+
+	if (!swnode)
+		return ERR_PTR(-EINVAL);
+
+	for (n = 0; args[n].fwnode; n++)
+		if (!is_software_node(args[n].fwnode))
+			return ERR_PTR(-EINVAL);
+
+	ref = kzalloc(sizeof(*ref), GFP_KERNEL);
+	if (!ref)
+		return ERR_PTR(-ENOMEM);
+
+	ref->nrefs = n;
+
+	ref->name = kstrdup(name, GFP_KERNEL);
+	if (!ref->name) {
+		kfree(ref);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ref->args = kcalloc(ref->nrefs, sizeof(*ref->args), GFP_KERNEL);
+	if (!ref->args) {
+		kfree(ref->name);
+		kfree(ref);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	for (n = 0; n < ref->nrefs; n++) {
+		ref->args[n] = args[n];
+		software_node_get(ref->args[n].fwnode);
+	}
+
+	mutex_lock(&swnode->lock);
+	list_add_tail(&ref->list, &swnode->references);
+	mutex_unlock(&swnode->lock);
+
+	return ref;
+}
+EXPORT_SYMBOL_GPL(fwnode_create_software_node_reference);
+
+/**
+ * fwnode_remove_software_node_reference - Remove named reference description
+ * @ref: Software node reference description
+ *
+ * Remove named reference @ref. Decrements the software node reference count of
+ * each node in @ref, and removes the association that was created in
+ * fwnode_create_software_node_reference().
+ */
+void fwnode_remove_software_node_reference(struct software_node_reference *ref)
+{
+	int n;
+
+	if (IS_ERR_OR_NULL(ref))
+		return;
+
+	for (n = 0; n < ref->nrefs; n++)
+		kobject_put(&to_software_node(ref->args[n].fwnode)->kobj);
+
+	list_del(&ref->list);
+	kfree(ref->args);
+	kfree(ref->name);
+	kfree(ref);
+}
+EXPORT_SYMBOL_GPL(fwnode_remove_software_node_reference);
+
 int software_node_notify(struct device *dev, unsigned long action)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(dev);
diff --git a/include/linux/property.h b/include/linux/property.h
index 65d3420dd5d1..40e12ca43556 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -314,6 +314,8 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 /* -------------------------------------------------------------------------- */
 /* Software fwnode support - when HW description is incomplete or missing */
 
+struct sofware_node_reference;
+
 bool is_software_node(const struct fwnode_handle *fwnode);
 
 int software_node_notify(struct device *dev, unsigned long action);
@@ -323,4 +325,10 @@ fwnode_create_software_node(const struct property_entry *properties,
 			    const struct fwnode_handle *parent);
 void fwnode_remove_software_node(struct fwnode_handle *fwnode);
 
+struct software_node_reference *
+fwnode_create_software_node_reference(const struct fwnode_handle *fwnode,
+				      const char *name,
+				      const struct fwnode_reference_args *args);
+void fwnode_remove_software_node_reference(struct software_node_reference *ref);
+
 #endif /* _LINUX_PROPERTY_H_ */
-- 
2.20.1

[03/12] software node: Add support for references

[Heikki Krogerus]

Introducing functions that can be used for adding and
removing references to other software nodes. The goal is to
support fwnode_property_get_reference_args() also with
software nodes, however, get_reference_args fwnode operation
callback is not yet implemented in this commit for the
software nodes. This commit will only add support for
reference addition/removal.

The next example shows how to add references to GPIOs using the
format described in Documentation/acpi/gpio-properties.txt:

/* Array with two GPIOs */
static struct fwnode_reference_args gpioargs[] __initdata = {
	{
		.nargs = 3,
		.args[0]= 19, /* index */
		.args[1]= 0,  /* pin */
		.args[2]= 0,  /* active_low */
	},
	{
		.nargs = 3,
		.args[0]= 20, /* index */
		.args[1]= 0,  /* pin */
		.args[2]= 0,  /* active_low */
	},
	{ }
};

static int myinit(void)
{
	struct software_node_reference *ref;
	struct fwnode_handle *gpio_node;
	struct fwnode_handle *my_node;

	/* Creating the nodes */
	gpio_node = fwnode_create_software_node(gpio_props, NULL);
	...
	my_node = fwnode_create_software_node(my_props, NULL);
	...

	/* gpio_node is associated with a GPIO/Pin controller in this example */
	...

	/* Assigning the actual node references */
	gpioargs[0].fwnode = gpio_node;
	gpioargs[1].fwnode = gpio_node;

	/* my_node will now have a named ("gpios") reference to the two GPIOs */
	ref = fwnode_create_software_node_reference(my_node, "gpios", gpioargs);
	...
	return 0;
}

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c    | 101 +++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |   8 ++++
 2 files changed, 109 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 8d4485d8d0f8..591f290d25be 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -11,10 +11,19 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
+struct software_node_reference {
+	struct list_head list;
+	const char *name;
+	int nrefs;
+	struct fwnode_reference_args *args;
+};
+
 struct software_node {
 	int id;
 	struct kobject kobj;
 	struct fwnode_handle fwnode;
+	struct list_head references;
+	struct mutex lock; /* node lock */
 
 	/* hierarchy */
 	struct ida child_ids;
@@ -606,9 +615,11 @@ fwnode_create_software_node(const struct property_entry *properties,
 	swnode->kobj.kset = swnode_kset;
 	swnode->fwnode.ops = &software_node_ops;
 
+	mutex_init(&swnode->lock);
 	ida_init(&swnode->child_ids);
 	INIT_LIST_HEAD(&swnode->entry);
 	INIT_LIST_HEAD(&swnode->children);
+	INIT_LIST_HEAD(&swnode->references);
 	swnode->parent = p;
 
 	if (p) {
@@ -641,10 +652,100 @@ void fwnode_remove_software_node(struct fwnode_handle *fwnode)
 	if (!swnode)
 		return;
 
+	mutex_lock(&swnode->lock);
+	WARN(!list_empty(&swnode->references),
+	     "\"%s\" has still references", kobject_name(&swnode->kobj));
+	mutex_unlock(&swnode->lock);
+
 	kobject_put(&swnode->kobj);
 }
 EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
 
+/**
+ * fwnode_create_software_node_reference - Create named reference description
+ * @fwnode: The software node to have the references
+ * @name: Name given to reference description
+ * @args: Zero terminated array of software node references with arguments
+ *
+ * Associates software nodes listed in @args with @fwnode. The association is
+ * named @name. The reference count is incremented for the nodes in @args.
+ *
+ * Returns pointer to software node reference description on success, or ERR_PTR
+ * on failure.
+ */
+struct software_node_reference *
+fwnode_create_software_node_reference(const struct fwnode_handle *fwnode,
+				      const char *name,
+				      const struct fwnode_reference_args *args)
+{
+	struct software_node *swnode = to_software_node(fwnode);
+	struct software_node_reference *ref;
+	int n;
+
+	if (!swnode)
+		return ERR_PTR(-EINVAL);
+
+	for (n = 0; args[n].fwnode; n++)
+		if (!is_software_node(args[n].fwnode))
+			return ERR_PTR(-EINVAL);
+
+	ref = kzalloc(sizeof(*ref), GFP_KERNEL);
+	if (!ref)
+		return ERR_PTR(-ENOMEM);
+
+	ref->nrefs = n;
+
+	ref->name = kstrdup(name, GFP_KERNEL);
+	if (!ref->name) {
+		kfree(ref);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ref->args = kcalloc(ref->nrefs, sizeof(*ref->args), GFP_KERNEL);
+	if (!ref->args) {
+		kfree(ref->name);
+		kfree(ref);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	for (n = 0; n < ref->nrefs; n++) {
+		ref->args[n] = args[n];
+		software_node_get(ref->args[n].fwnode);
+	}
+
+	mutex_lock(&swnode->lock);
+	list_add_tail(&ref->list, &swnode->references);
+	mutex_unlock(&swnode->lock);
+
+	return ref;
+}
+EXPORT_SYMBOL_GPL(fwnode_create_software_node_reference);
+
+/**
+ * fwnode_remove_software_node_reference - Remove named reference description
+ * @ref: Software node reference description
+ *
+ * Remove named reference @ref. Decrements the software node reference count of
+ * each node in @ref, and removes the association that was created in
+ * fwnode_create_software_node_reference().
+ */
+void fwnode_remove_software_node_reference(struct software_node_reference *ref)
+{
+	int n;
+
+	if (IS_ERR_OR_NULL(ref))
+		return;
+
+	for (n = 0; n < ref->nrefs; n++)
+		kobject_put(&to_software_node(ref->args[n].fwnode)->kobj);
+
+	list_del(&ref->list);
+	kfree(ref->args);
+	kfree(ref->name);
+	kfree(ref);
+}
+EXPORT_SYMBOL_GPL(fwnode_remove_software_node_reference);
+
 int software_node_notify(struct device *dev, unsigned long action)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(dev);
diff --git a/include/linux/property.h b/include/linux/property.h
index 65d3420dd5d1..40e12ca43556 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -314,6 +314,8 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 /* -------------------------------------------------------------------------- */
 /* Software fwnode support - when HW description is incomplete or missing */
 
+struct sofware_node_reference;
+
 bool is_software_node(const struct fwnode_handle *fwnode);
 
 int software_node_notify(struct device *dev, unsigned long action);
@@ -323,4 +325,10 @@ fwnode_create_software_node(const struct property_entry *properties,
 			    const struct fwnode_handle *parent);
 void fwnode_remove_software_node(struct fwnode_handle *fwnode);
 
+struct software_node_reference *
+fwnode_create_software_node_reference(const struct fwnode_handle *fwnode,
+				      const char *name,
+				      const struct fwnode_reference_args *args);
+void fwnode_remove_software_node_reference(struct software_node_reference *ref);
+
 #endif /* _LINUX_PROPERTY_H_ */

[PATCH 04/12] software node: Implement .get_reference_args fwnode operation

[Heikki Krogerus]

This makes it possible to support drivers that use
fwnode_property_get_reference_args() function.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 591f290d25be..b1320a55351d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -534,6 +534,61 @@ software_node_get_named_child_node(const struct fwnode_handle *fwnode,
 	return NULL;
 }
 
+static int
+software_node_get_reference_args(const struct fwnode_handle *fwnode,
+				 const char *propname, const char *nargs_prop,
+				 unsigned int nargs, unsigned int index,
+				 struct fwnode_reference_args *args)
+{
+	struct software_node *swnode = to_software_node(fwnode);
+	struct software_node_reference *ref;
+	const struct property_entry *prop;
+	int ret = -ENOENT;
+	int i;
+
+	mutex_lock(&swnode->lock);
+
+	if (!swnode || list_empty(&swnode->references))
+		goto err_unlock;
+
+	if (nargs_prop) {
+		prop = property_entry_get(swnode->properties, nargs_prop);
+		if (!prop) {
+			ret = -EINVAL;
+			goto err_unlock;
+		}
+
+		nargs = prop->value.u32_data;
+	}
+
+	if (nargs > NR_FWNODE_REFERENCE_ARGS) {
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+
+	list_for_each_entry(ref, &swnode->references, list) {
+		if (strcmp(ref->name, propname))
+			continue;
+
+		if (index > (ref->nrefs - 1))
+			break;
+
+		args->nargs = nargs;
+		args->fwnode = software_node_get(ref->args[index].fwnode);
+
+		for (i = 0; i < nargs; i++)
+			args->args[i] = ref->args[index].args[i];
+
+		ret = 0;
+		break;
+	}
+
+err_unlock:
+	mutex_unlock(&swnode->lock);
+
+	return ret;
+}
+
 static const struct fwnode_operations software_node_ops = {
 	.get = software_node_get,
 	.put = software_node_put,
@@ -543,6 +598,7 @@ static const struct fwnode_operations software_node_ops = {
 	.get_parent = software_node_get_parent,
 	.get_next_child_node = software_node_get_next_child,
 	.get_named_child_node = software_node_get_named_child_node,
+	.get_reference_args = software_node_get_reference_args,
 };
 
 /* -------------------------------------------------------------------------- */
-- 
2.20.1

[04/12] software node: Implement .get_reference_args fwnode operation

[Heikki Krogerus]

This makes it possible to support drivers that use
fwnode_property_get_reference_args() function.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 591f290d25be..b1320a55351d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -534,6 +534,61 @@ software_node_get_named_child_node(const struct fwnode_handle *fwnode,
 	return NULL;
 }
 
+static int
+software_node_get_reference_args(const struct fwnode_handle *fwnode,
+				 const char *propname, const char *nargs_prop,
+				 unsigned int nargs, unsigned int index,
+				 struct fwnode_reference_args *args)
+{
+	struct software_node *swnode = to_software_node(fwnode);
+	struct software_node_reference *ref;
+	const struct property_entry *prop;
+	int ret = -ENOENT;
+	int i;
+
+	mutex_lock(&swnode->lock);
+
+	if (!swnode || list_empty(&swnode->references))
+		goto err_unlock;
+
+	if (nargs_prop) {
+		prop = property_entry_get(swnode->properties, nargs_prop);
+		if (!prop) {
+			ret = -EINVAL;
+			goto err_unlock;
+		}
+
+		nargs = prop->value.u32_data;
+	}
+
+	if (nargs > NR_FWNODE_REFERENCE_ARGS) {
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+
+	list_for_each_entry(ref, &swnode->references, list) {
+		if (strcmp(ref->name, propname))
+			continue;
+
+		if (index > (ref->nrefs - 1))
+			break;
+
+		args->nargs = nargs;
+		args->fwnode = software_node_get(ref->args[index].fwnode);
+
+		for (i = 0; i < nargs; i++)
+			args->args[i] = ref->args[index].args[i];
+
+		ret = 0;
+		break;
+	}
+
+err_unlock:
+	mutex_unlock(&swnode->lock);
+
+	return ret;
+}
+
 static const struct fwnode_operations software_node_ops = {
 	.get = software_node_get,
 	.put = software_node_put,
@@ -543,6 +598,7 @@ static const struct fwnode_operations software_node_ops = {
 	.get_parent = software_node_get_parent,
 	.get_next_child_node = software_node_get_next_child,
 	.get_named_child_node = software_node_get_named_child_node,
+	.get_reference_args = software_node_get_reference_args,
 };
 
 /* -------------------------------------------------------------------------- */

[PATCH 05/12] ACPI / property: Don't limit named child node matching to data nodes

[Heikki Krogerus]

There is no reason why we should limit the use of
fwnode_get_named_child_node() to data nodes only.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/acpi/property.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 77abe0ec4043..c3fb52c387a6 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -602,15 +602,29 @@ static struct fwnode_handle *
 acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 				 const char *childname)
 {
+	char name[ACPI_PATH_SEGMENT_LENGTH];
 	struct fwnode_handle *child;
+	struct acpi_buffer path;
+	acpi_status status;
 
-	/*
-	 * Find first matching named child node of this fwnode.
-	 * For ACPI this will be a data only sub-node.
-	 */
-	fwnode_for_each_child_node(fwnode, child)
-		if (acpi_data_node_match(child, childname))
+	path.length = sizeof(name);
+	path.pointer = name;
+
+	fwnode_for_each_child_node(fwnode, child) {
+		if (is_acpi_data_node(child)) {
+			if (acpi_data_node_match(child, childname))
+				return child;
+			continue;
+		}
+
+		status = acpi_get_name(ACPI_HANDLE_FWNODE(child),
+				       ACPI_SINGLE_NAME, &path);
+		if (ACPI_FAILURE(status))
+			break;
+
+		if (!strncmp(name, childname, ACPI_NAME_SIZE))
 			return child;
+	}
 
 	return NULL;
 }
-- 
2.20.1

[05/12] ACPI / property: Don't limit named child node matching to data nodes

[Heikki Krogerus]

There is no reason why we should limit the use of
fwnode_get_named_child_node() to data nodes only.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/acpi/property.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 77abe0ec4043..c3fb52c387a6 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -602,15 +602,29 @@ static struct fwnode_handle *
 acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 				 const char *childname)
 {
+	char name[ACPI_PATH_SEGMENT_LENGTH];
 	struct fwnode_handle *child;
+	struct acpi_buffer path;
+	acpi_status status;
 
-	/*
-	 * Find first matching named child node of this fwnode.
-	 * For ACPI this will be a data only sub-node.
-	 */
-	fwnode_for_each_child_node(fwnode, child)
-		if (acpi_data_node_match(child, childname))
+	path.length = sizeof(name);
+	path.pointer = name;
+
+	fwnode_for_each_child_node(fwnode, child) {
+		if (is_acpi_data_node(child)) {
+			if (acpi_data_node_match(child, childname))
+				return child;
+			continue;
+		}
+
+		status = acpi_get_name(ACPI_HANDLE_FWNODE(child),
+				       ACPI_SINGLE_NAME, &path);
+		if (ACPI_FAILURE(status))
+			break;
+
+		if (!strncmp(name, childname, ACPI_NAME_SIZE))
 			return child;
+	}
 
 	return NULL;
 }

[PATCH 06/12] device connection: Find connections also by checking the references

[Heikki Krogerus]

We can also use this API to find named references that the
device nodes have by using fwnode_property_get_reference_args()
function.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/devcon.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 04db9ae235e4..4cdf95532b63 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -38,6 +38,30 @@ fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
 	return NULL;
 }
 
+static void *
+fwnode_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
+		    void *data, devcon_match_fn_t match)
+{
+	struct device_connection con = { };
+	struct fwnode_reference_args args;
+	void *ret;
+	int i;
+
+	for (i = 0; ; i++) {
+		if (fwnode_property_get_reference_args(fwnode, con_id, NULL, 0,
+						       i, &args))
+			break;
+
+		con.fwnode = args.fwnode;
+		ret = match(&con, -1, data);
+		fwnode_handle_put(args.fwnode);
+		if (ret)
+			return ret;
+	}
+
+	return NULL;
+}
+
 /**
  * device_connection_find_match - Find physical connection to a device
  * @dev: Device with the connection
@@ -65,6 +89,10 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
 		ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
 		if (ret)
 			return ret;
+
+		ret = fwnode_devcon_match(fwnode, con_id, data, match);
+		if (ret)
+			return ret;
 	}
 
 	mutex_lock(&devcon_lock);
-- 
2.20.1

[06/12] device connection: Find connections also by checking the references

[Heikki Krogerus]

We can also use this API to find named references that the
device nodes have by using fwnode_property_get_reference_args()
function.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/devcon.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 04db9ae235e4..4cdf95532b63 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -38,6 +38,30 @@ fwnode_graph_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
 	return NULL;
 }
 
+static void *
+fwnode_devcon_match(struct fwnode_handle *fwnode, const char *con_id,
+		    void *data, devcon_match_fn_t match)
+{
+	struct device_connection con = { };
+	struct fwnode_reference_args args;
+	void *ret;
+	int i;
+
+	for (i = 0; ; i++) {
+		if (fwnode_property_get_reference_args(fwnode, con_id, NULL, 0,
+						       i, &args))
+			break;
+
+		con.fwnode = args.fwnode;
+		ret = match(&con, -1, data);
+		fwnode_handle_put(args.fwnode);
+		if (ret)
+			return ret;
+	}
+
+	return NULL;
+}
+
 /**
  * device_connection_find_match - Find physical connection to a device
  * @dev: Device with the connection
@@ -65,6 +89,10 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
 		ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
 		if (ret)
 			return ret;
+
+		ret = fwnode_devcon_match(fwnode, con_id, data, match);
+		if (ret)
+			return ret;
 	}
 
 	mutex_lock(&devcon_lock);

[PATCH 07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Heikki Krogerus]

Software nodes provide two features that we will need later.
1) Software nodes can have references to other software nodes.
2) Software nodes can exist before a device entry is created.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
 1 file changed, 92 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 6fa3cced6f8e..26444d31945b 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,15 +24,25 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 
 #define EXPECTED_PTYPE		4
 
+enum {
+	INT33FE_NODE_FUSB302,
+	INT33FE_NODE_MAX17047,
+	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_MAX,
+};
+
 struct cht_int33fe_data {
 	struct i2c_client *max17047;
 	struct i2c_client *fusb302;
 	struct i2c_client *pi3usb30532;
 	/* Contain a list-head must be per device */
 	struct device_connection connections[4];
+
+	struct fwnode_handle *node[INT33FE_NODE_MAX];
 };
 
 /*
@@ -63,14 +73,6 @@ static int cht_int33fe_check_for_max17047(struct device *dev, void *data)
 	return 1;
 }
 
-static struct i2c_client *cht_int33fe_find_max17047(void)
-{
-	struct i2c_client *max17047 = NULL;
-
-	i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
-	return max17047;
-}
-
 static const char * const max17047_suppliers[] = { "bq24190-charger" };
 
 static const struct property_entry max17047_props[] = {
@@ -78,6 +80,36 @@ static const struct property_entry max17047_props[] = {
 	{ }
 };
 
+static int
+cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_MAX17047];
+	struct i2c_client *max17047 = NULL;
+	struct i2c_board_info board_info;
+	int ret;
+
+	i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
+	if (max17047) {
+		/* Pre-existing i2c-client for the max17047, add device-props */
+		max17047->dev.fwnode->secondary = fwnode;
+		/* And re-probe to get the new device-props applied. */
+		ret = device_reprobe(&max17047->dev);
+		if (ret)
+			dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
+		return 0;
+	}
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
+	board_info.dev_name = "max17047";
+	board_info.fwnode = fwnode;
+	data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
+	if (IS_ERR(data->max17047))
+		return PTR_ERR(data->max17047);
+
+	return 0;
+}
+
 static const struct property_entry fusb302_props[] = {
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
 	PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
@@ -86,12 +118,50 @@ static const struct property_entry fusb302_props[] = {
 	{ }
 };
 
+static const struct property_entry *props[] = {
+	[INT33FE_NODE_FUSB302]		= fusb302_props,
+	[INT33FE_NODE_MAX17047]		= NULL,
+	[INT33FE_NODE_PI3USB30532]	= NULL,
+};
+
+static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
+{
+	int i;
+
+	for (i = 0; i < INT33FE_NODE_MAX; i++) {
+		fwnode_remove_software_node(data->node[i]);
+		data->node[i] = NULL;
+	}
+}
+
+static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode;
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(props); i++) {
+		fwnode = fwnode_create_software_node(props[i], NULL);
+		if (IS_ERR(fwnode)) {
+			ret = PTR_ERR(fwnode);
+			goto err_remove_nodes;
+		}
+		data->node[i] = fwnode;
+	}
+
+	return 0;
+
+err_remove_nodes:
+	cht_int33fe_remove_nodes(data);
+
+	return ret;
+}
+
 static int cht_int33fe_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct i2c_board_info board_info;
 	struct cht_int33fe_data *data;
-	struct i2c_client *max17047;
 	struct regulator *regulator;
 	unsigned long long ptyp;
 	acpi_status status;
@@ -151,26 +221,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	ret = cht_int33fe_add_nodes(data);
+	if (ret)
+		return ret;
+
 	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
-	max17047 = cht_int33fe_find_max17047();
-	if (max17047) {
-		/* Pre-existing i2c-client for the max17047, add device-props */
-		ret = device_add_properties(&max17047->dev, max17047_props);
-		if (ret)
-			return ret;
-		/* And re-probe to get the new device-props applied. */
-		ret = device_reprobe(&max17047->dev);
-		if (ret)
-			dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
-	} else {
-		memset(&board_info, 0, sizeof(board_info));
-		strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
-		board_info.dev_name = "max17047";
-		board_info.properties = max17047_props;
-		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
-		if (IS_ERR(data->max17047))
-			return PTR_ERR(data->max17047);
-	}
+	ret = cht_int33fe_max17047(dev, data);
+	if (ret)
+		goto out_remove_nodes;
 
 	data->connections[0].endpoint[0] = "port0";
 	data->connections[0].endpoint[1] = "i2c-pi3usb30532";
@@ -187,7 +245,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	memset(&board_info, 0, sizeof(board_info));
 	strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
 	board_info.dev_name = "fusb302";
-	board_info.properties = fusb302_props;
+	board_info.fwnode = data->node[INT33FE_NODE_FUSB302];
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
@@ -198,6 +256,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 
 	memset(&board_info, 0, sizeof(board_info));
 	board_info.dev_name = "pi3usb30532";
+	board_info.fwnode = data->node[INT33FE_NODE_PI3USB30532];
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
@@ -214,9 +273,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 
 out_unregister_max17047:
+	device_connections_remove(data->connections);
 	i2c_unregister_device(data->max17047);
 
-	device_connections_remove(data->connections);
+out_remove_nodes:
+	cht_int33fe_remove_nodes(data);
 
 	return ret;
 }
@@ -230,6 +291,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
 	i2c_unregister_device(data->max17047);
 
 	device_connections_remove(data->connections);
+	cht_int33fe_remove_nodes(data);
 
 	return 0;
 }
-- 
2.20.1

[07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Heikki Krogerus]

Software nodes provide two features that we will need later.
1) Software nodes can have references to other software nodes.
2) Software nodes can exist before a device entry is created.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
 1 file changed, 92 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 6fa3cced6f8e..26444d31945b 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,15 +24,25 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/pci.h>
 
 #define EXPECTED_PTYPE		4
 
+enum {
+	INT33FE_NODE_FUSB302,
+	INT33FE_NODE_MAX17047,
+	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_MAX,
+};
+
 struct cht_int33fe_data {
 	struct i2c_client *max17047;
 	struct i2c_client *fusb302;
 	struct i2c_client *pi3usb30532;
 	/* Contain a list-head must be per device */
 	struct device_connection connections[4];
+
+	struct fwnode_handle *node[INT33FE_NODE_MAX];
 };
 
 /*
@@ -63,14 +73,6 @@ static int cht_int33fe_check_for_max17047(struct device *dev, void *data)
 	return 1;
 }
 
-static struct i2c_client *cht_int33fe_find_max17047(void)
-{
-	struct i2c_client *max17047 = NULL;
-
-	i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
-	return max17047;
-}
-
 static const char * const max17047_suppliers[] = { "bq24190-charger" };
 
 static const struct property_entry max17047_props[] = {
@@ -78,6 +80,36 @@ static const struct property_entry max17047_props[] = {
 	{ }
 };
 
+static int
+cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_MAX17047];
+	struct i2c_client *max17047 = NULL;
+	struct i2c_board_info board_info;
+	int ret;
+
+	i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
+	if (max17047) {
+		/* Pre-existing i2c-client for the max17047, add device-props */
+		max17047->dev.fwnode->secondary = fwnode;
+		/* And re-probe to get the new device-props applied. */
+		ret = device_reprobe(&max17047->dev);
+		if (ret)
+			dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
+		return 0;
+	}
+
+	memset(&board_info, 0, sizeof(board_info));
+	strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
+	board_info.dev_name = "max17047";
+	board_info.fwnode = fwnode;
+	data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
+	if (IS_ERR(data->max17047))
+		return PTR_ERR(data->max17047);
+
+	return 0;
+}
+
 static const struct property_entry fusb302_props[] = {
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
 	PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
@@ -86,12 +118,50 @@ static const struct property_entry fusb302_props[] = {
 	{ }
 };
 
+static const struct property_entry *props[] = {
+	[INT33FE_NODE_FUSB302]		= fusb302_props,
+	[INT33FE_NODE_MAX17047]		= NULL,
+	[INT33FE_NODE_PI3USB30532]	= NULL,
+};
+
+static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
+{
+	int i;
+
+	for (i = 0; i < INT33FE_NODE_MAX; i++) {
+		fwnode_remove_software_node(data->node[i]);
+		data->node[i] = NULL;
+	}
+}
+
+static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode;
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(props); i++) {
+		fwnode = fwnode_create_software_node(props[i], NULL);
+		if (IS_ERR(fwnode)) {
+			ret = PTR_ERR(fwnode);
+			goto err_remove_nodes;
+		}
+		data->node[i] = fwnode;
+	}
+
+	return 0;
+
+err_remove_nodes:
+	cht_int33fe_remove_nodes(data);
+
+	return ret;
+}
+
 static int cht_int33fe_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct i2c_board_info board_info;
 	struct cht_int33fe_data *data;
-	struct i2c_client *max17047;
 	struct regulator *regulator;
 	unsigned long long ptyp;
 	acpi_status status;
@@ -151,26 +221,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 
+	ret = cht_int33fe_add_nodes(data);
+	if (ret)
+		return ret;
+
 	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
-	max17047 = cht_int33fe_find_max17047();
-	if (max17047) {
-		/* Pre-existing i2c-client for the max17047, add device-props */
-		ret = device_add_properties(&max17047->dev, max17047_props);
-		if (ret)
-			return ret;
-		/* And re-probe to get the new device-props applied. */
-		ret = device_reprobe(&max17047->dev);
-		if (ret)
-			dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
-	} else {
-		memset(&board_info, 0, sizeof(board_info));
-		strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
-		board_info.dev_name = "max17047";
-		board_info.properties = max17047_props;
-		data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
-		if (IS_ERR(data->max17047))
-			return PTR_ERR(data->max17047);
-	}
+	ret = cht_int33fe_max17047(dev, data);
+	if (ret)
+		goto out_remove_nodes;
 
 	data->connections[0].endpoint[0] = "port0";
 	data->connections[0].endpoint[1] = "i2c-pi3usb30532";
@@ -187,7 +245,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	memset(&board_info, 0, sizeof(board_info));
 	strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
 	board_info.dev_name = "fusb302";
-	board_info.properties = fusb302_props;
+	board_info.fwnode = data->node[INT33FE_NODE_FUSB302];
 	board_info.irq = fusb302_irq;
 
 	data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
@@ -198,6 +256,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 
 	memset(&board_info, 0, sizeof(board_info));
 	board_info.dev_name = "pi3usb30532";
+	board_info.fwnode = data->node[INT33FE_NODE_PI3USB30532];
 	strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
 
 	data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
@@ -214,9 +273,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 
 out_unregister_max17047:
+	device_connections_remove(data->connections);
 	i2c_unregister_device(data->max17047);
 
-	device_connections_remove(data->connections);
+out_remove_nodes:
+	cht_int33fe_remove_nodes(data);
 
 	return ret;
 }
@@ -230,6 +291,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
 	i2c_unregister_device(data->max17047);
 
 	device_connections_remove(data->connections);
+	cht_int33fe_remove_nodes(data);
 
 	return 0;
 }

Re: [PATCH 07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Software nodes provide two features that we will need later.
> 1) Software nodes can have references to other software nodes.
> 2) Software nodes can exist before a device entry is created.
>

I don't see any problems with code (*), though I hope Hans will have
chance to test this.
One nit below.

(*) Means all patches touching PDx86 in this series

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
>  1 file changed, 92 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> index 6fa3cced6f8e..26444d31945b 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -24,15 +24,25 @@
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>

> +#include <linux/pci.h>

Preserve the ordering, please.

>
>  #define EXPECTED_PTYPE         4
>
> +enum {
> +       INT33FE_NODE_FUSB302,
> +       INT33FE_NODE_MAX17047,
> +       INT33FE_NODE_PI3USB30532,
> +       INT33FE_NODE_MAX,
> +};
> +
>  struct cht_int33fe_data {
>         struct i2c_client *max17047;
>         struct i2c_client *fusb302;
>         struct i2c_client *pi3usb30532;
>         /* Contain a list-head must be per device */
>         struct device_connection connections[4];
> +
> +       struct fwnode_handle *node[INT33FE_NODE_MAX];
>  };
>
>  /*
> @@ -63,14 +73,6 @@ static int cht_int33fe_check_for_max17047(struct device *dev, void *data)
>         return 1;
>  }
>
> -static struct i2c_client *cht_int33fe_find_max17047(void)
> -{
> -       struct i2c_client *max17047 = NULL;
> -
> -       i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
> -       return max17047;
> -}
> -
>  static const char * const max17047_suppliers[] = { "bq24190-charger" };
>
>  static const struct property_entry max17047_props[] = {
> @@ -78,6 +80,36 @@ static const struct property_entry max17047_props[] = {
>         { }
>  };
>
> +static int
> +cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
> +{
> +       struct fwnode_handle *fwnode = data->node[INT33FE_NODE_MAX17047];
> +       struct i2c_client *max17047 = NULL;
> +       struct i2c_board_info board_info;
> +       int ret;
> +
> +       i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
> +       if (max17047) {
> +               /* Pre-existing i2c-client for the max17047, add device-props */
> +               max17047->dev.fwnode->secondary = fwnode;
> +               /* And re-probe to get the new device-props applied. */
> +               ret = device_reprobe(&max17047->dev);
> +               if (ret)
> +                       dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
> +               return 0;
> +       }
> +
> +       memset(&board_info, 0, sizeof(board_info));
> +       strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> +       board_info.dev_name = "max17047";
> +       board_info.fwnode = fwnode;
> +       data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
> +       if (IS_ERR(data->max17047))
> +               return PTR_ERR(data->max17047);
> +
> +       return 0;
> +}
> +
>  static const struct property_entry fusb302_props[] = {
>         PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
>         PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
> @@ -86,12 +118,50 @@ static const struct property_entry fusb302_props[] = {
>         { }
>  };
>
> +static const struct property_entry *props[] = {
> +       [INT33FE_NODE_FUSB302]          = fusb302_props,
> +       [INT33FE_NODE_MAX17047]         = NULL,
> +       [INT33FE_NODE_PI3USB30532]      = NULL,
> +};
> +
> +static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
> +{
> +       int i;
> +
> +       for (i = 0; i < INT33FE_NODE_MAX; i++) {
> +               fwnode_remove_software_node(data->node[i]);
> +               data->node[i] = NULL;
> +       }
> +}
> +
> +static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
> +{
> +       struct fwnode_handle *fwnode;
> +       int ret;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(props); i++) {
> +               fwnode = fwnode_create_software_node(props[i], NULL);
> +               if (IS_ERR(fwnode)) {
> +                       ret = PTR_ERR(fwnode);
> +                       goto err_remove_nodes;
> +               }
> +               data->node[i] = fwnode;
> +       }
> +
> +       return 0;
> +
> +err_remove_nodes:
> +       cht_int33fe_remove_nodes(data);
> +
> +       return ret;
> +}
> +
>  static int cht_int33fe_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct i2c_board_info board_info;
>         struct cht_int33fe_data *data;
> -       struct i2c_client *max17047;
>         struct regulator *regulator;
>         unsigned long long ptyp;
>         acpi_status status;
> @@ -151,26 +221,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         if (!data)
>                 return -ENOMEM;
>
> +       ret = cht_int33fe_add_nodes(data);
> +       if (ret)
> +               return ret;
> +
>         /* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
> -       max17047 = cht_int33fe_find_max17047();
> -       if (max17047) {
> -               /* Pre-existing i2c-client for the max17047, add device-props */
> -               ret = device_add_properties(&max17047->dev, max17047_props);
> -               if (ret)
> -                       return ret;
> -               /* And re-probe to get the new device-props applied. */
> -               ret = device_reprobe(&max17047->dev);
> -               if (ret)
> -                       dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
> -       } else {
> -               memset(&board_info, 0, sizeof(board_info));
> -               strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> -               board_info.dev_name = "max17047";
> -               board_info.properties = max17047_props;
> -               data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
> -               if (IS_ERR(data->max17047))
> -                       return PTR_ERR(data->max17047);
> -       }
> +       ret = cht_int33fe_max17047(dev, data);
> +       if (ret)
> +               goto out_remove_nodes;
>
>         data->connections[0].endpoint[0] = "port0";
>         data->connections[0].endpoint[1] = "i2c-pi3usb30532";
> @@ -187,7 +245,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         memset(&board_info, 0, sizeof(board_info));
>         strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
>         board_info.dev_name = "fusb302";
> -       board_info.properties = fusb302_props;
> +       board_info.fwnode = data->node[INT33FE_NODE_FUSB302];
>         board_info.irq = fusb302_irq;
>
>         data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
> @@ -198,6 +256,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>
>         memset(&board_info, 0, sizeof(board_info));
>         board_info.dev_name = "pi3usb30532";
> +       board_info.fwnode = data->node[INT33FE_NODE_PI3USB30532];
>         strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
>
>         data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
> @@ -214,9 +273,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         i2c_unregister_device(data->fusb302);
>
>  out_unregister_max17047:
> +       device_connections_remove(data->connections);
>         i2c_unregister_device(data->max17047);
>
> -       device_connections_remove(data->connections);
> +out_remove_nodes:
> +       cht_int33fe_remove_nodes(data);
>
>         return ret;
>  }
> @@ -230,6 +291,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
>         i2c_unregister_device(data->max17047);
>
>         device_connections_remove(data->connections);
> +       cht_int33fe_remove_nodes(data);
>
>         return 0;
>  }
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko

[07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Software nodes provide two features that we will need later.
> 1) Software nodes can have references to other software nodes.
> 2) Software nodes can exist before a device entry is created.
>

I don't see any problems with code (*), though I hope Hans will have
chance to test this.
One nit below.

(*) Means all patches touching PDx86 in this series

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
>  1 file changed, 92 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> index 6fa3cced6f8e..26444d31945b 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -24,15 +24,25 @@
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>

> +#include <linux/pci.h>

Preserve the ordering, please.

>
>  #define EXPECTED_PTYPE         4
>
> +enum {
> +       INT33FE_NODE_FUSB302,
> +       INT33FE_NODE_MAX17047,
> +       INT33FE_NODE_PI3USB30532,
> +       INT33FE_NODE_MAX,
> +};
> +
>  struct cht_int33fe_data {
>         struct i2c_client *max17047;
>         struct i2c_client *fusb302;
>         struct i2c_client *pi3usb30532;
>         /* Contain a list-head must be per device */
>         struct device_connection connections[4];
> +
> +       struct fwnode_handle *node[INT33FE_NODE_MAX];
>  };
>
>  /*
> @@ -63,14 +73,6 @@ static int cht_int33fe_check_for_max17047(struct device *dev, void *data)
>         return 1;
>  }
>
> -static struct i2c_client *cht_int33fe_find_max17047(void)
> -{
> -       struct i2c_client *max17047 = NULL;
> -
> -       i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
> -       return max17047;
> -}
> -
>  static const char * const max17047_suppliers[] = { "bq24190-charger" };
>
>  static const struct property_entry max17047_props[] = {
> @@ -78,6 +80,36 @@ static const struct property_entry max17047_props[] = {
>         { }
>  };
>
> +static int
> +cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
> +{
> +       struct fwnode_handle *fwnode = data->node[INT33FE_NODE_MAX17047];
> +       struct i2c_client *max17047 = NULL;
> +       struct i2c_board_info board_info;
> +       int ret;
> +
> +       i2c_for_each_dev(&max17047, cht_int33fe_check_for_max17047);
> +       if (max17047) {
> +               /* Pre-existing i2c-client for the max17047, add device-props */
> +               max17047->dev.fwnode->secondary = fwnode;
> +               /* And re-probe to get the new device-props applied. */
> +               ret = device_reprobe(&max17047->dev);
> +               if (ret)
> +                       dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
> +               return 0;
> +       }
> +
> +       memset(&board_info, 0, sizeof(board_info));
> +       strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> +       board_info.dev_name = "max17047";
> +       board_info.fwnode = fwnode;
> +       data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
> +       if (IS_ERR(data->max17047))
> +               return PTR_ERR(data->max17047);
> +
> +       return 0;
> +}
> +
>  static const struct property_entry fusb302_props[] = {
>         PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
>         PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
> @@ -86,12 +118,50 @@ static const struct property_entry fusb302_props[] = {
>         { }
>  };
>
> +static const struct property_entry *props[] = {
> +       [INT33FE_NODE_FUSB302]          = fusb302_props,
> +       [INT33FE_NODE_MAX17047]         = NULL,
> +       [INT33FE_NODE_PI3USB30532]      = NULL,
> +};
> +
> +static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
> +{
> +       int i;
> +
> +       for (i = 0; i < INT33FE_NODE_MAX; i++) {
> +               fwnode_remove_software_node(data->node[i]);
> +               data->node[i] = NULL;
> +       }
> +}
> +
> +static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
> +{
> +       struct fwnode_handle *fwnode;
> +       int ret;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(props); i++) {
> +               fwnode = fwnode_create_software_node(props[i], NULL);
> +               if (IS_ERR(fwnode)) {
> +                       ret = PTR_ERR(fwnode);
> +                       goto err_remove_nodes;
> +               }
> +               data->node[i] = fwnode;
> +       }
> +
> +       return 0;
> +
> +err_remove_nodes:
> +       cht_int33fe_remove_nodes(data);
> +
> +       return ret;
> +}
> +
>  static int cht_int33fe_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct i2c_board_info board_info;
>         struct cht_int33fe_data *data;
> -       struct i2c_client *max17047;
>         struct regulator *regulator;
>         unsigned long long ptyp;
>         acpi_status status;
> @@ -151,26 +221,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         if (!data)
>                 return -ENOMEM;
>
> +       ret = cht_int33fe_add_nodes(data);
> +       if (ret)
> +               return ret;
> +
>         /* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
> -       max17047 = cht_int33fe_find_max17047();
> -       if (max17047) {
> -               /* Pre-existing i2c-client for the max17047, add device-props */
> -               ret = device_add_properties(&max17047->dev, max17047_props);
> -               if (ret)
> -                       return ret;
> -               /* And re-probe to get the new device-props applied. */
> -               ret = device_reprobe(&max17047->dev);
> -               if (ret)
> -                       dev_warn(dev, "Reprobing max17047 error: %d\n", ret);
> -       } else {
> -               memset(&board_info, 0, sizeof(board_info));
> -               strlcpy(board_info.type, "max17047", I2C_NAME_SIZE);
> -               board_info.dev_name = "max17047";
> -               board_info.properties = max17047_props;
> -               data->max17047 = i2c_acpi_new_device(dev, 1, &board_info);
> -               if (IS_ERR(data->max17047))
> -                       return PTR_ERR(data->max17047);
> -       }
> +       ret = cht_int33fe_max17047(dev, data);
> +       if (ret)
> +               goto out_remove_nodes;
>
>         data->connections[0].endpoint[0] = "port0";
>         data->connections[0].endpoint[1] = "i2c-pi3usb30532";
> @@ -187,7 +245,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         memset(&board_info, 0, sizeof(board_info));
>         strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
>         board_info.dev_name = "fusb302";
> -       board_info.properties = fusb302_props;
> +       board_info.fwnode = data->node[INT33FE_NODE_FUSB302];
>         board_info.irq = fusb302_irq;
>
>         data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info);
> @@ -198,6 +256,7 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>
>         memset(&board_info, 0, sizeof(board_info));
>         board_info.dev_name = "pi3usb30532";
> +       board_info.fwnode = data->node[INT33FE_NODE_PI3USB30532];
>         strlcpy(board_info.type, "pi3usb30532", I2C_NAME_SIZE);
>
>         data->pi3usb30532 = i2c_acpi_new_device(dev, 3, &board_info);
> @@ -214,9 +273,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
>         i2c_unregister_device(data->fusb302);
>
>  out_unregister_max17047:
> +       device_connections_remove(data->connections);
>         i2c_unregister_device(data->max17047);
>
> -       device_connections_remove(data->connections);
> +out_remove_nodes:
> +       cht_int33fe_remove_nodes(data);
>
>         return ret;
>  }
> @@ -230,6 +291,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
>         i2c_unregister_device(data->max17047);
>
>         device_connections_remove(data->connections);
> +       cht_int33fe_remove_nodes(data);
>
>         return 0;
>  }
> --
> 2.20.1
>

Re: [PATCH 07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Heikki Krogerus]

On Sun, Mar 17, 2019 at 10:36:03PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Software nodes provide two features that we will need later.
> > 1) Software nodes can have references to other software nodes.
> > 2) Software nodes can exist before a device entry is created.
> >
> 
> I don't see any problems with code (*), though I hope Hans will have
> chance to test this.
> One nit below.
> 
> (*) Means all patches touching PDx86 in this series
> 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
> >  1 file changed, 92 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> > index 6fa3cced6f8e..26444d31945b 100644
> > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > @@ -24,15 +24,25 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> 
> > +#include <linux/pci.h>
> 
> Preserve the ordering, please.

OK.

thanks,

-- 
heikki

[07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Heikki Krogerus]

On Sun, Mar 17, 2019 at 10:36:03PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Software nodes provide two features that we will need later.
> > 1) Software nodes can have references to other software nodes.
> > 2) Software nodes can exist before a device entry is created.
> >
> 
> I don't see any problems with code (*), though I hope Hans will have
> chance to test this.
> One nit below.
> 
> (*) Means all patches touching PDx86 in this series
> 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/platform/x86/intel_cht_int33fe.c | 122 +++++++++++++++++------
> >  1 file changed, 92 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
> > index 6fa3cced6f8e..26444d31945b 100644
> > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > @@ -24,15 +24,25 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> 
> > +#include <linux/pci.h>
> 
> Preserve the ordering, please.

OK.

thanks,

Re: [PATCH 07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Andy Shevchenko]

On Sun, Mar 17, 2019 at 10:36 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:

> I don't see any problems with code (*), though I hope Hans will have
> chance to test this.

> (*) Means all patches touching PDx86 in this series

To be clear, I implied here Rb or Ab tag on your choice.

-- 
With Best Regards,
Andy Shevchenko

[07/12] platform/x86: intel_cht_int33fe: Provide software node for all components

[Andy Shevchenko]

On Sun, Mar 17, 2019 at 10:36 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 15, 2019 at 6:58 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:

> I don't see any problems with code (*), though I hope Hans will have
> chance to test this.

> (*) Means all patches touching PDx86 in this series

To be clear, I implied here Rb or Ab tag on your choice.

[PATCH 08/12] platform/x86: intel_cht_int33fe: Provide fwnode for the USB connector

[Heikki Krogerus]

In ACPI, and now also in DT, the USB connectors usually have
their own device nodes. In case of USB Type-C, those
connector (port) nodes are child nodes of the controller or
PHY device, in our case the fusb302. The software fwnodes
allow us to create a similar child node for fusb302 that
represents the connector also on Intel CHT.

This makes it possible replace the fusb302 specific device
properties which were deprecated with the common USB
connector properties that tcpm.c is able to use directly.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 37 ++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 26444d31945b..44860c748bf5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -25,6 +25,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/usb/pd.h>
 
 #define EXPECTED_PTYPE		4
 
@@ -32,6 +33,7 @@ enum {
 	INT33FE_NODE_FUSB302,
 	INT33FE_NODE_MAX17047,
 	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_USB_CONNECTOR,
 	INT33FE_NODE_MAX,
 };
 
@@ -112,9 +114,29 @@ cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
 
 static const struct property_entry fusb302_props[] = {
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   3000000),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 36000000),
+	{ }
+};
+
+#define PDO_FIXED_FLAGS \
+	(PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP | PDO_FIXED_USB_COMM)
+
+static const u32 src_pdo[] = {
+	PDO_FIXED(5000, 1500, PDO_FIXED_FLAGS),
+};
+
+static const u32 snk_pdo[] = {
+	PDO_FIXED(5000, 400, PDO_FIXED_FLAGS),
+	PDO_VAR(5000, 12000, 3000),
+};
+
+static const struct property_entry usb_connector_props[] = {
+	PROPERTY_ENTRY_STRING("name", "connector"),
+	PROPERTY_ENTRY_STRING("data-role", "dual"),
+	PROPERTY_ENTRY_STRING("power-role", "dual"),
+	PROPERTY_ENTRY_STRING("try-power-role", "sink"),
+	PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo),
+	PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo),
+	PROPERTY_ENTRY_U32("op-sink-microwatt", 36000000),
 	{ }
 };
 
@@ -149,6 +171,15 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 		data->node[i] = fwnode;
 	}
 
+	/* Node for the USB connector (FUSB302 is the parent) */
+	fwnode = fwnode_create_software_node(usb_connector_props,
+					     data->node[INT33FE_NODE_FUSB302]);
+	if (IS_ERR(fwnode)) {
+		ret = PTR_ERR(fwnode);
+		goto err_remove_nodes;
+	}
+	data->node[INT33FE_NODE_USB_CONNECTOR] = fwnode;
+
 	return 0;
 
 err_remove_nodes:
-- 
2.20.1

[08/12] platform/x86: intel_cht_int33fe: Provide fwnode for the USB connector

[Heikki Krogerus]

In ACPI, and now also in DT, the USB connectors usually have
their own device nodes. In case of USB Type-C, those
connector (port) nodes are child nodes of the controller or
PHY device, in our case the fusb302. The software fwnodes
allow us to create a similar child node for fusb302 that
represents the connector also on Intel CHT.

This makes it possible replace the fusb302 specific device
properties which were deprecated with the common USB
connector properties that tcpm.c is able to use directly.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 37 ++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 26444d31945b..44860c748bf5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -25,6 +25,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/usb/pd.h>
 
 #define EXPECTED_PTYPE		4
 
@@ -32,6 +33,7 @@ enum {
 	INT33FE_NODE_FUSB302,
 	INT33FE_NODE_MAX17047,
 	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_USB_CONNECTOR,
 	INT33FE_NODE_MAX,
 };
 
@@ -112,9 +114,29 @@ cht_int33fe_max17047(struct device *dev, struct cht_int33fe_data *data)
 
 static const struct property_entry fusb302_props[] = {
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 12000000),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   3000000),
-	PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 36000000),
+	{ }
+};
+
+#define PDO_FIXED_FLAGS \
+	(PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP | PDO_FIXED_USB_COMM)
+
+static const u32 src_pdo[] = {
+	PDO_FIXED(5000, 1500, PDO_FIXED_FLAGS),
+};
+
+static const u32 snk_pdo[] = {
+	PDO_FIXED(5000, 400, PDO_FIXED_FLAGS),
+	PDO_VAR(5000, 12000, 3000),
+};
+
+static const struct property_entry usb_connector_props[] = {
+	PROPERTY_ENTRY_STRING("name", "connector"),
+	PROPERTY_ENTRY_STRING("data-role", "dual"),
+	PROPERTY_ENTRY_STRING("power-role", "dual"),
+	PROPERTY_ENTRY_STRING("try-power-role", "sink"),
+	PROPERTY_ENTRY_U32_ARRAY("source-pdos", src_pdo),
+	PROPERTY_ENTRY_U32_ARRAY("sink-pdos", snk_pdo),
+	PROPERTY_ENTRY_U32("op-sink-microwatt", 36000000),
 	{ }
 };
 
@@ -149,6 +171,15 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 		data->node[i] = fwnode;
 	}
 
+	/* Node for the USB connector (FUSB302 is the parent) */
+	fwnode = fwnode_create_software_node(usb_connector_props,
+					     data->node[INT33FE_NODE_FUSB302]);
+	if (IS_ERR(fwnode)) {
+		ret = PTR_ERR(fwnode);
+		goto err_remove_nodes;
+	}
+	data->node[INT33FE_NODE_USB_CONNECTOR] = fwnode;
+
 	return 0;
 
 err_remove_nodes:

[PATCH 09/12] platform/x86: intel_cht_int33fe: Link with external dependencies using fwnodes

[Heikki Krogerus]

Supplying also external devices - the DisplayPort connector
and the USB role switch - software fwnodes. After this the
driver has access to all the components tied to the USB
Type-C connector and can start creating software node
references to actually associate them with the USB Type-C
connector device.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 102 +++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 44860c748bf5..aa71b97a22e9 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -33,6 +33,8 @@ enum {
 	INT33FE_NODE_FUSB302,
 	INT33FE_NODE_MAX17047,
 	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_DISPLAYPORT,
+	INT33FE_NODE_ROLE_SWITCH,
 	INT33FE_NODE_USB_CONNECTOR,
 	INT33FE_NODE_MAX,
 };
@@ -44,6 +46,7 @@ struct cht_int33fe_data {
 	/* Contain a list-head must be per device */
 	struct device_connection connections[4];
 
+	struct fwnode_handle *dp;
 	struct fwnode_handle *node[INT33FE_NODE_MAX];
 };
 
@@ -144,8 +147,81 @@ static const struct property_entry *props[] = {
 	[INT33FE_NODE_FUSB302]		= fusb302_props,
 	[INT33FE_NODE_MAX17047]		= NULL,
 	[INT33FE_NODE_PI3USB30532]	= NULL,
+	[INT33FE_NODE_DISPLAYPORT]	= NULL,
+	[INT33FE_NODE_ROLE_SWITCH]	= NULL,
 };
 
+static const char *dp_acpi_node_name = "DD02";
+static const char *mux_platfrom_dev_name = "intel_xhci_usb_sw";
+static const char *mux_name = "intel_xhci_usb_sw-role-switch";
+
+static int cht_int33fe_setup_dp(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_DISPLAYPORT];
+	struct pci_dev *pdev;
+
+	/* First let's find the GPU PCI device */
+	pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, NULL);
+	if (!pdev || (pdev->vendor != PCI_VENDOR_ID_INTEL))
+		return -ENODEV;
+
+	/* Then the DP child device node */
+	data->dp = device_get_named_child_node(&pdev->dev, dp_acpi_node_name);
+	pci_dev_put(pdev);
+	if (!data->dp)
+		return -ENODEV;
+
+	fwnode->secondary = ERR_PTR(-ENODEV);
+	data->dp->secondary = fwnode;
+
+	return 0;
+}
+
+static int role_switch_match(struct device *dev, void *name)
+{
+	return !strcmp(dev_name(dev), name);
+}
+
+static int cht_int33fe_setup_mux(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_ROLE_SWITCH];
+	struct pci_dev *pdev;
+	struct device *dev;
+	struct device *p;
+
+	/* First let's find xHCI PCI device */
+	pdev = pci_get_class(PCI_CLASS_SERIAL_USB_XHCI, NULL);
+	if (!pdev || (pdev->vendor != PCI_VENDOR_ID_INTEL))
+		return -ENODEV;
+
+	/* Then the child platform device */
+	p = device_find_child(&pdev->dev, (void *)mux_platfrom_dev_name,
+			      role_switch_match);
+	pci_dev_put(pdev);
+	if (!p)
+		return -EPROBE_DEFER;
+
+	/* Finally the mux device */
+	dev = device_find_child(p, (void *)mux_name, role_switch_match);
+	put_device(p);
+	if (!dev)
+		return -EPROBE_DEFER;
+
+	/* If there already is a node for the mux, using that one. */
+	if (dev->fwnode) {
+		fwnode_handle_get(dev->fwnode);
+		fwnode_remove_software_node(fwnode);
+		data->node[INT33FE_NODE_ROLE_SWITCH] = dev->fwnode;
+	} else {
+		/* The node can be tied to the lifetime of the device. */
+		dev->fwnode = fwnode_handle_get(dev->fwnode);
+	}
+
+	put_device(dev);
+
+	return 0;
+}
+
 static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
 {
 	int i;
@@ -154,6 +230,12 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
 		fwnode_remove_software_node(data->node[i]);
 		data->node[i] = NULL;
 	}
+
+	if (data->dp) {
+		data->dp->secondary = NULL;
+		fwnode_handle_put(data->dp);
+		data->dp = NULL;
+	}
 }
 
 static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
@@ -180,6 +262,26 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 	}
 	data->node[INT33FE_NODE_USB_CONNECTOR] = fwnode;
 
+	/* The devices that are not created in this driver need extra steps. */
+
+	/*
+	 * There is no ACPI device node for the USB role mux, so we need to find
+	 * the mux device and assign our node directly to it. That means we
+	 * depend on the mux driver. This function will return -PROBE_DEFER
+	 * until the mux device is registered.
+	 */
+	ret = cht_int33fe_setup_mux(data);
+	if (ret)
+		goto err_remove_nodes;
+
+	/*
+	 * The DP connector does have ACPI device node. In this case we can just
+	 * find that ACPI node and assing our node as the secondary node to it.
+	 */
+	ret = cht_int33fe_setup_dp(data);
+	if (ret)
+		goto err_remove_nodes;
+
 	return 0;
 
 err_remove_nodes:
-- 
2.20.1

[09/12] platform/x86: intel_cht_int33fe: Link with external dependencies using fwnodes

[Heikki Krogerus]

Supplying also external devices - the DisplayPort connector
and the USB role switch - software fwnodes. After this the
driver has access to all the components tied to the USB
Type-C connector and can start creating software node
references to actually associate them with the USB Type-C
connector device.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 102 +++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 44860c748bf5..aa71b97a22e9 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -33,6 +33,8 @@ enum {
 	INT33FE_NODE_FUSB302,
 	INT33FE_NODE_MAX17047,
 	INT33FE_NODE_PI3USB30532,
+	INT33FE_NODE_DISPLAYPORT,
+	INT33FE_NODE_ROLE_SWITCH,
 	INT33FE_NODE_USB_CONNECTOR,
 	INT33FE_NODE_MAX,
 };
@@ -44,6 +46,7 @@ struct cht_int33fe_data {
 	/* Contain a list-head must be per device */
 	struct device_connection connections[4];
 
+	struct fwnode_handle *dp;
 	struct fwnode_handle *node[INT33FE_NODE_MAX];
 };
 
@@ -144,8 +147,81 @@ static const struct property_entry *props[] = {
 	[INT33FE_NODE_FUSB302]		= fusb302_props,
 	[INT33FE_NODE_MAX17047]		= NULL,
 	[INT33FE_NODE_PI3USB30532]	= NULL,
+	[INT33FE_NODE_DISPLAYPORT]	= NULL,
+	[INT33FE_NODE_ROLE_SWITCH]	= NULL,
 };
 
+static const char *dp_acpi_node_name = "DD02";
+static const char *mux_platfrom_dev_name = "intel_xhci_usb_sw";
+static const char *mux_name = "intel_xhci_usb_sw-role-switch";
+
+static int cht_int33fe_setup_dp(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_DISPLAYPORT];
+	struct pci_dev *pdev;
+
+	/* First let's find the GPU PCI device */
+	pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, NULL);
+	if (!pdev || (pdev->vendor != PCI_VENDOR_ID_INTEL))
+		return -ENODEV;
+
+	/* Then the DP child device node */
+	data->dp = device_get_named_child_node(&pdev->dev, dp_acpi_node_name);
+	pci_dev_put(pdev);
+	if (!data->dp)
+		return -ENODEV;
+
+	fwnode->secondary = ERR_PTR(-ENODEV);
+	data->dp->secondary = fwnode;
+
+	return 0;
+}
+
+static int role_switch_match(struct device *dev, void *name)
+{
+	return !strcmp(dev_name(dev), name);
+}
+
+static int cht_int33fe_setup_mux(struct cht_int33fe_data *data)
+{
+	struct fwnode_handle *fwnode = data->node[INT33FE_NODE_ROLE_SWITCH];
+	struct pci_dev *pdev;
+	struct device *dev;
+	struct device *p;
+
+	/* First let's find xHCI PCI device */
+	pdev = pci_get_class(PCI_CLASS_SERIAL_USB_XHCI, NULL);
+	if (!pdev || (pdev->vendor != PCI_VENDOR_ID_INTEL))
+		return -ENODEV;
+
+	/* Then the child platform device */
+	p = device_find_child(&pdev->dev, (void *)mux_platfrom_dev_name,
+			      role_switch_match);
+	pci_dev_put(pdev);
+	if (!p)
+		return -EPROBE_DEFER;
+
+	/* Finally the mux device */
+	dev = device_find_child(p, (void *)mux_name, role_switch_match);
+	put_device(p);
+	if (!dev)
+		return -EPROBE_DEFER;
+
+	/* If there already is a node for the mux, using that one. */
+	if (dev->fwnode) {
+		fwnode_handle_get(dev->fwnode);
+		fwnode_remove_software_node(fwnode);
+		data->node[INT33FE_NODE_ROLE_SWITCH] = dev->fwnode;
+	} else {
+		/* The node can be tied to the lifetime of the device. */
+		dev->fwnode = fwnode_handle_get(dev->fwnode);
+	}
+
+	put_device(dev);
+
+	return 0;
+}
+
 static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
 {
 	int i;
@@ -154,6 +230,12 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)
 		fwnode_remove_software_node(data->node[i]);
 		data->node[i] = NULL;
 	}
+
+	if (data->dp) {
+		data->dp->secondary = NULL;
+		fwnode_handle_put(data->dp);
+		data->dp = NULL;
+	}
 }
 
 static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
@@ -180,6 +262,26 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 	}
 	data->node[INT33FE_NODE_USB_CONNECTOR] = fwnode;
 
+	/* The devices that are not created in this driver need extra steps. */
+
+	/*
+	 * There is no ACPI device node for the USB role mux, so we need to find
+	 * the mux device and assign our node directly to it. That means we
+	 * depend on the mux driver. This function will return -PROBE_DEFER
+	 * until the mux device is registered.
+	 */
+	ret = cht_int33fe_setup_mux(data);
+	if (ret)
+		goto err_remove_nodes;
+
+	/*
+	 * The DP connector does have ACPI device node. In this case we can just
+	 * find that ACPI node and assing our node as the secondary node to it.
+	 */
+	ret = cht_int33fe_setup_dp(data);
+	if (ret)
+		goto err_remove_nodes;
+
 	return 0;
 
 err_remove_nodes:

[PATCH 10/12] platform/x86: intel_cht_int33fe: Replacing the old connections with references

[Heikki Krogerus]

Now that the software nodes support references, and the
device connection API support parsing fwnode references,
replacing the old connection descriptions with software node
references. Relying on device names when matching the
connection would not have been possible to link the USB
Type-C connector and the DisplayPort connector together, but
with real references it's not problem.

The DisplayPort ACPI node is dag up, and the drivers own
software node for the DisplayPort is set as the secondary
node for it. The USB Type-C connector refers the software
node, but it is now tied to the ACPI node, and therefore any
device entry (struct drm_connector in practice) that the
node combo is assigned to.

The USB role switch device does not have ACPI node, so we
have to wait for the device to appear. Then we can simply
assign our software node for the to the device.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 94 +++++++++++++++++++-----
 1 file changed, 77 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index aa71b97a22e9..5cdb7fa33f2b 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -39,15 +39,22 @@ enum {
 	INT33FE_NODE_MAX,
 };
 
+enum {
+	INT33FE_REF_ORIENTATION,
+	INT33FE_REF_MODE_MUX,
+	INT33FE_REF_DISPLAYPORT,
+	INT33FE_REF_ROLE_SWITCH,
+	INT33FE_REF_MAX,
+};
+
 struct cht_int33fe_data {
 	struct i2c_client *max17047;
 	struct i2c_client *fusb302;
 	struct i2c_client *pi3usb30532;
-	/* Contain a list-head must be per device */
-	struct device_connection connections[4];
 
 	struct fwnode_handle *dp;
 	struct fwnode_handle *node[INT33FE_NODE_MAX];
+	struct software_node_reference *ref[INT33FE_REF_MAX];
 };
 
 /*
@@ -290,6 +297,65 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 	return ret;
 }
 
+static void cht_int33fe_remove_references(struct cht_int33fe_data *data)
+{
+	int i;
+
+	for (i = 0; i < INT33FE_REF_MAX; i++)
+		fwnode_remove_software_node_reference(data->ref[i]);
+}
+
+static int cht_int33fe_add_references(struct cht_int33fe_data *data)
+{
+	struct fwnode_reference_args args[2] = { };
+	struct software_node_reference *ref;
+	struct fwnode_handle *con;
+
+	con = data->node[INT33FE_NODE_USB_CONNECTOR];
+
+	/* USB Type-C muxes */
+	args[0].fwnode = data->node[INT33FE_NODE_PI3USB30532];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "orientation-switch",
+						    args);
+	if (IS_ERR(ref))
+		return PTR_ERR(ref);
+	data->ref[INT33FE_REF_ORIENTATION] = ref;
+
+	ref = fwnode_create_software_node_reference(con, "mode-switch",
+						    args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_MODE_MUX] = ref;
+
+	/* USB role switch */
+	args[0].fwnode = data->node[INT33FE_NODE_ROLE_SWITCH];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "usb-role-switch", args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_ROLE_SWITCH] = ref;
+
+	/* DisplayPort */
+	args[0].fwnode = data->node[INT33FE_NODE_DISPLAYPORT];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "displayport", args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_DISPLAYPORT] = ref;
+
+	return 0;
+}
+
 static int cht_int33fe_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -358,22 +424,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
-	ret = cht_int33fe_max17047(dev, data);
+	ret = cht_int33fe_add_references(data);
 	if (ret)
 		goto out_remove_nodes;
 
-	data->connections[0].endpoint[0] = "port0";
-	data->connections[0].endpoint[1] = "i2c-pi3usb30532";
-	data->connections[0].id = "orientation-switch";
-	data->connections[1].endpoint[0] = "port0";
-	data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-	data->connections[1].id = "mode-switch";
-	data->connections[2].endpoint[0] = "i2c-fusb302";
-	data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
-	data->connections[2].id = "usb-role-switch";
-
-	device_connections_add(data->connections);
+	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
+	ret = cht_int33fe_max17047(dev, data);
+	if (ret)
+		goto out_remove_references;
 
 	memset(&board_info, 0, sizeof(board_info));
 	strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -406,9 +464,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 
 out_unregister_max17047:
-	device_connections_remove(data->connections);
 	i2c_unregister_device(data->max17047);
 
+out_remove_references:
+	cht_int33fe_remove_references(data);
+
 out_remove_nodes:
 	cht_int33fe_remove_nodes(data);
 
@@ -423,7 +483,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 	i2c_unregister_device(data->max17047);
 
-	device_connections_remove(data->connections);
+	cht_int33fe_remove_references(data);
 	cht_int33fe_remove_nodes(data);
 
 	return 0;
-- 
2.20.1

[10/12] platform/x86: intel_cht_int33fe: Replacing the old connections with references

[Heikki Krogerus]

Now that the software nodes support references, and the
device connection API support parsing fwnode references,
replacing the old connection descriptions with software node
references. Relying on device names when matching the
connection would not have been possible to link the USB
Type-C connector and the DisplayPort connector together, but
with real references it's not problem.

The DisplayPort ACPI node is dag up, and the drivers own
software node for the DisplayPort is set as the secondary
node for it. The USB Type-C connector refers the software
node, but it is now tied to the ACPI node, and therefore any
device entry (struct drm_connector in practice) that the
node combo is assigned to.

The USB role switch device does not have ACPI node, so we
have to wait for the device to appear. Then we can simply
assign our software node for the to the device.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/x86/intel_cht_int33fe.c | 94 +++++++++++++++++++-----
 1 file changed, 77 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index aa71b97a22e9..5cdb7fa33f2b 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -39,15 +39,22 @@ enum {
 	INT33FE_NODE_MAX,
 };
 
+enum {
+	INT33FE_REF_ORIENTATION,
+	INT33FE_REF_MODE_MUX,
+	INT33FE_REF_DISPLAYPORT,
+	INT33FE_REF_ROLE_SWITCH,
+	INT33FE_REF_MAX,
+};
+
 struct cht_int33fe_data {
 	struct i2c_client *max17047;
 	struct i2c_client *fusb302;
 	struct i2c_client *pi3usb30532;
-	/* Contain a list-head must be per device */
-	struct device_connection connections[4];
 
 	struct fwnode_handle *dp;
 	struct fwnode_handle *node[INT33FE_NODE_MAX];
+	struct software_node_reference *ref[INT33FE_REF_MAX];
 };
 
 /*
@@ -290,6 +297,65 @@ static int cht_int33fe_add_nodes(struct cht_int33fe_data *data)
 	return ret;
 }
 
+static void cht_int33fe_remove_references(struct cht_int33fe_data *data)
+{
+	int i;
+
+	for (i = 0; i < INT33FE_REF_MAX; i++)
+		fwnode_remove_software_node_reference(data->ref[i]);
+}
+
+static int cht_int33fe_add_references(struct cht_int33fe_data *data)
+{
+	struct fwnode_reference_args args[2] = { };
+	struct software_node_reference *ref;
+	struct fwnode_handle *con;
+
+	con = data->node[INT33FE_NODE_USB_CONNECTOR];
+
+	/* USB Type-C muxes */
+	args[0].fwnode = data->node[INT33FE_NODE_PI3USB30532];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "orientation-switch",
+						    args);
+	if (IS_ERR(ref))
+		return PTR_ERR(ref);
+	data->ref[INT33FE_REF_ORIENTATION] = ref;
+
+	ref = fwnode_create_software_node_reference(con, "mode-switch",
+						    args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_MODE_MUX] = ref;
+
+	/* USB role switch */
+	args[0].fwnode = data->node[INT33FE_NODE_ROLE_SWITCH];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "usb-role-switch", args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_ROLE_SWITCH] = ref;
+
+	/* DisplayPort */
+	args[0].fwnode = data->node[INT33FE_NODE_DISPLAYPORT];
+	args[1].fwnode = NULL;
+
+	ref = fwnode_create_software_node_reference(con, "displayport", args);
+	if (IS_ERR(ref)) {
+		cht_int33fe_remove_references(data);
+		return PTR_ERR(ref);
+	}
+	data->ref[INT33FE_REF_DISPLAYPORT] = ref;
+
+	return 0;
+}
+
 static int cht_int33fe_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -358,22 +424,14 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
-	ret = cht_int33fe_max17047(dev, data);
+	ret = cht_int33fe_add_references(data);
 	if (ret)
 		goto out_remove_nodes;
 
-	data->connections[0].endpoint[0] = "port0";
-	data->connections[0].endpoint[1] = "i2c-pi3usb30532";
-	data->connections[0].id = "orientation-switch";
-	data->connections[1].endpoint[0] = "port0";
-	data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-	data->connections[1].id = "mode-switch";
-	data->connections[2].endpoint[0] = "i2c-fusb302";
-	data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
-	data->connections[2].id = "usb-role-switch";
-
-	device_connections_add(data->connections);
+	/* Work around BIOS bug, see comment on cht_int33fe_find_max17047 */
+	ret = cht_int33fe_max17047(dev, data);
+	if (ret)
+		goto out_remove_references;
 
 	memset(&board_info, 0, sizeof(board_info));
 	strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -406,9 +464,11 @@ static int cht_int33fe_probe(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 
 out_unregister_max17047:
-	device_connections_remove(data->connections);
 	i2c_unregister_device(data->max17047);
 
+out_remove_references:
+	cht_int33fe_remove_references(data);
+
 out_remove_nodes:
 	cht_int33fe_remove_nodes(data);
 
@@ -423,7 +483,7 @@ static int cht_int33fe_remove(struct platform_device *pdev)
 	i2c_unregister_device(data->fusb302);
 	i2c_unregister_device(data->max17047);
 
-	device_connections_remove(data->connections);
+	cht_int33fe_remove_references(data);
 	cht_int33fe_remove_nodes(data);
 
 	return 0;

[PATCH 11/12] drm: Add fwnode member to the struct drm_connector

[Heikki Krogerus]

If the system firmware supplies nodes that represent the connectors,
they need to be associated with the connector entries.

ACPI tables at least always supply a device node for every connector
on integrated video hardware - this is explained in ACPI
specification's ch. "Appendix B Video Extension". Many drivers appear
to already deal with those connector firmware nodes "indirectly"
in order to support custom Operation Regions, _DSM (device specific
method) and access other resources the nodes supply for the
connectors.

This commit will only add the fwnode member. It's first up to the
drivers to assign and take advantage of it. For convenience, the nodes
are assigned to the device entries that are used for exposing the
connectors to the user space via sysfs. That makes it possible to see
the link between the connector and its node also from user space. In
case of ACPI, the connector's sysfs directory will have a symlink
named "firmware_node" pointing to the node object directory in sysfs
if a node is associated with the connector.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 49 +++++++++++++++++++++++++------------
 include/drm/drm_connector.h |  2 ++
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ecb7b33002bb..7667939ef1c0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -264,26 +264,50 @@ static const struct attribute_group *connector_dev_groups[] = {
 	NULL
 };
 
+static void drm_sysfs_release(struct device *dev)
+{
+	kfree(dev);
+}
+
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int ret;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
-	DRM_DEBUG("adding \"%s\" to sysfs\n",
-		  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->parent = dev->primary->kdev;
+	kdev->fwnode = connector->fwnode;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_release;
+	dev_set_drvdata(kdev, connector);
+
+	ret = dev_set_name(kdev, "card%d-%s", dev->primary->index,
+			   connector->name);
+	if (ret) {
+		kfree(kdev);
+		return ret;
+	}
+
+	DRM_DEBUG("adding \"%s\" to sysfs\n", connector->name);
+
+	ret = device_add(kdev);
+	if (ret) {
+		DRM_ERROR("failed to register connector device: %d\n", ret);
+		put_device(kdev);
+		return ret;
 	}
 
+	connector->kdev = kdev;
+
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
@@ -330,11 +354,6 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
-static void drm_sysfs_release(struct device *dev)
-{
-	kfree(dev);
-}
-
 struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
 {
 	const char *minor_str;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8fe22abb1e10..e30b7743e019 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -851,6 +851,8 @@ struct drm_connector {
 	struct device *kdev;
 	/** @attr: sysfs attributes */
 	struct device_attribute *attr;
+	/** @fwnode: associated device node supplied by platform firmware */
+	struct fwnode_handle *fwnode;
 
 	/**
 	 * @head:
-- 
2.20.1

[11/12] drm: Add fwnode member to the struct drm_connector

[Heikki Krogerus]

If the system firmware supplies nodes that represent the connectors,
they need to be associated with the connector entries.

ACPI tables at least always supply a device node for every connector
on integrated video hardware - this is explained in ACPI
specification's ch. "Appendix B Video Extension". Many drivers appear
to already deal with those connector firmware nodes "indirectly"
in order to support custom Operation Regions, _DSM (device specific
method) and access other resources the nodes supply for the
connectors.

This commit will only add the fwnode member. It's first up to the
drivers to assign and take advantage of it. For convenience, the nodes
are assigned to the device entries that are used for exposing the
connectors to the user space via sysfs. That makes it possible to see
the link between the connector and its node also from user space. In
case of ACPI, the connector's sysfs directory will have a symlink
named "firmware_node" pointing to the node object directory in sysfs
if a node is associated with the connector.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/drm_sysfs.c | 49 +++++++++++++++++++++++++------------
 include/drm/drm_connector.h |  2 ++
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ecb7b33002bb..7667939ef1c0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -264,26 +264,50 @@ static const struct attribute_group *connector_dev_groups[] = {
 	NULL
 };
 
+static void drm_sysfs_release(struct device *dev)
+{
+	kfree(dev);
+}
+
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int ret;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
-	DRM_DEBUG("adding \"%s\" to sysfs\n",
-		  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->parent = dev->primary->kdev;
+	kdev->fwnode = connector->fwnode;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_release;
+	dev_set_drvdata(kdev, connector);
+
+	ret = dev_set_name(kdev, "card%d-%s", dev->primary->index,
+			   connector->name);
+	if (ret) {
+		kfree(kdev);
+		return ret;
+	}
+
+	DRM_DEBUG("adding \"%s\" to sysfs\n", connector->name);
+
+	ret = device_add(kdev);
+	if (ret) {
+		DRM_ERROR("failed to register connector device: %d\n", ret);
+		put_device(kdev);
+		return ret;
 	}
 
+	connector->kdev = kdev;
+
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
@@ -330,11 +354,6 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
-static void drm_sysfs_release(struct device *dev)
-{
-	kfree(dev);
-}
-
 struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
 {
 	const char *minor_str;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8fe22abb1e10..e30b7743e019 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -851,6 +851,8 @@ struct drm_connector {
 	struct device *kdev;
 	/** @attr: sysfs attributes */
 	struct device_attribute *attr;
+	/** @fwnode: associated device node supplied by platform firmware */
+	struct fwnode_handle *fwnode;
 
 	/**
 	 * @head:

[PATCH 12/12] drm/i915: Associate the ACPI connector nodes with connector entries

[Heikki Krogerus]

On Intel platforms we know that the ACPI connector device
node order will follow the order the driver (i915) decides.
The decision is made using the Intel OpRegion
intel_opregion.c), though, the driver does not know that the
values it supplies are used for associating a device node
for the connectors, and assigning address for them.

In reality it means we end up violating ACPI specification
(we supply dynamic information to objects that are defined
static, for example _ADR), however, it makes associating the
connector nodes with the connector entries straightforward
(one-on-one mapping).

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ccb616351bba..512dc7ad0604 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15304,6 +15304,46 @@ static int intel_initial_commit(struct drm_device *dev)
 	return ret;
 }
 
+/* NOTE: The connector order must be final before this is called. */
+static void intel_assign_connector_fwnodes(struct drm_device *dev)
+{
+	struct drm_connector_list_iter conn_iter;
+	struct device *kdev = &dev->pdev->dev;
+	struct fwnode_handle *fwnode = NULL;
+	struct drm_connector *connector;
+	struct acpi_device *adev;
+
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		/* Always getting the next, even when the last was not used. */
+		fwnode = device_get_next_child_node(kdev, fwnode);
+		if (!fwnode)
+			break;
+
+		switch (connector->connector_type) {
+		case DRM_MODE_CONNECTOR_LVDS:
+		case DRM_MODE_CONNECTOR_eDP:
+		case DRM_MODE_CONNECTOR_DSI:
+			/*
+			 * Integrated displays use a specific address 0x1f on
+			 * most Intel platforms, but not all of them.
+			 */
+			adev = acpi_find_child_device(ACPI_COMPANION(kdev),
+						      0x1f, 0);
+			if (adev) {
+				connector->fwnode = acpi_fwnode_handle(adev);
+				break;
+			}
+			/* fallthrough */
+		default:
+			connector->fwnode = fwnode;
+			break;
+		}
+
+	}
+	drm_connector_list_iter_end(&conn_iter);
+}
+
 int intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -15406,6 +15446,7 @@ int intel_modeset_init(struct drm_device *dev)
 
 	drm_modeset_lock_all(dev);
 	intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
+	intel_assign_connector_fwnodes(dev);
 	drm_modeset_unlock_all(dev);
 
 	for_each_intel_crtc(dev, crtc) {
-- 
2.20.1

[12/12] drm/i915: Associate the ACPI connector nodes with connector entries

[Heikki Krogerus]

On Intel platforms we know that the ACPI connector device
node order will follow the order the driver (i915) decides.
The decision is made using the Intel OpRegion
intel_opregion.c), though, the driver does not know that the
values it supplies are used for associating a device node
for the connectors, and assigning address for them.

In reality it means we end up violating ACPI specification
(we supply dynamic information to objects that are defined
static, for example _ADR), however, it makes associating the
connector nodes with the connector entries straightforward
(one-on-one mapping).

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ccb616351bba..512dc7ad0604 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15304,6 +15304,46 @@ static int intel_initial_commit(struct drm_device *dev)
 	return ret;
 }
 
+/* NOTE: The connector order must be final before this is called. */
+static void intel_assign_connector_fwnodes(struct drm_device *dev)
+{
+	struct drm_connector_list_iter conn_iter;
+	struct device *kdev = &dev->pdev->dev;
+	struct fwnode_handle *fwnode = NULL;
+	struct drm_connector *connector;
+	struct acpi_device *adev;
+
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		/* Always getting the next, even when the last was not used. */
+		fwnode = device_get_next_child_node(kdev, fwnode);
+		if (!fwnode)
+			break;
+
+		switch (connector->connector_type) {
+		case DRM_MODE_CONNECTOR_LVDS:
+		case DRM_MODE_CONNECTOR_eDP:
+		case DRM_MODE_CONNECTOR_DSI:
+			/*
+			 * Integrated displays use a specific address 0x1f on
+			 * most Intel platforms, but not all of them.
+			 */
+			adev = acpi_find_child_device(ACPI_COMPANION(kdev),
+						      0x1f, 0);
+			if (adev) {
+				connector->fwnode = acpi_fwnode_handle(adev);
+				break;
+			}
+			/* fallthrough */
+		default:
+			connector->fwnode = fwnode;
+			break;
+		}
+
+	}
+	drm_connector_list_iter_end(&conn_iter);
+}
+
 int intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -15406,6 +15446,7 @@ int intel_modeset_init(struct drm_device *dev)
 
 	drm_modeset_lock_all(dev);
 	intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
+	intel_assign_connector_fwnodes(dev);
 	drm_modeset_unlock_all(dev);
 
 	for_each_intel_crtc(dev, crtc) {