It could happen that we need to control suspend/resume ordering between
devices without obvious consumer/supplier link. For example when touchscreens
and DSI panels share the same reset line, in this case we need to be sure
of pm_runtime operations ordering between those two devices to correctly
perform reset.
DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
I2C client and I2C bus or regulator client and regulator provider) so we need
to describe this in device-tree.
This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
helpers to find and parse 'links-add' property in a device-tree node.
It allows to create and remove links between consumer and suppliers from
device-tree data so consumers will be suspend before their suppliers and resume
after them.
Benjamin Gaignard (3):
of/device: Add of_ functions for device_link_{add,remove}
Input: edt-ft5x06: Document links-add property
Input: goodix: Document links-add property
Yannick Fertré (2):
input: edt-ft5x06 - Call devm_of_device_links_add() to create links
input: goodix - Call devm_of_device_links_add() to create links
.../bindings/input/touchscreen/edt-ft5x06.txt | 2 +
.../bindings/input/touchscreen/goodix.txt | 2 +
drivers/input/touchscreen/edt-ft5x06.c | 2 +
drivers/input/touchscreen/goodix.c | 3 +
drivers/of/device.c | 103 +++++++++++++++++++++
include/linux/of_device.h | 20 ++++
6 files changed, 132 insertions(+)
--
2.15.0
From: Yannick Fertré <[email protected]>
Add a call to devm_of_device_links_add() to create links with suppliers
at probe time.
Signed-off-by: Yannick Fertré <[email protected]>
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/input/touchscreen/edt-ft5x06.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 702bfda7ee77..ac9f7e85efb0 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1167,6 +1167,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
i2c_set_clientdata(client, tsdata);
+ devm_of_device_links_add(&client->dev);
+
irq_flags = irq_get_trigger_type(client->irq);
if (irq_flags == IRQF_TRIGGER_NONE)
irq_flags = IRQF_TRIGGER_FALLING;
--
2.15.0
Explain the purpose of links_add property.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 8cf0b4d38a7e..0447151608c6 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -24,6 +24,8 @@ Optional properties:
- touchscreen-size-x
- touchscreen-size-y
- touchscreen-swapped-x-y
+ - links-add : List of suppliers handles, suppliers will be
+ suspended after goodix device and resume before it.
The touchscreen-* properties are documented in touchscreen.txt in this
directory.
--
2.15.0
Explain the purpose of links_add property.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 870b8c5cce9b..38b84d2b32e6 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -60,6 +60,8 @@ Optional properties:
- touchscreen-inverted-x : See touchscreen.txt
- touchscreen-inverted-y : See touchscreen.txt
- touchscreen-swapped-x-y : See touchscreen.txt
+ - links-add : List of suppliers handles, suppliers will be
+ suspended after touchscreen device and resume before it.
Example:
polytouch: edt-ft5x06@38 {
--
2.15.0
Allows to create and remove links between consumer and suppliers from
device-tree data. Use 'links-add' property from consumer node to setup
a link with a list of suppliers.
Consumers will be suspend before their suppliers and resume after them.
Add devm_of_device_links_add() to automatically remove the links
when the device is unbound from the bus.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/of/device.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_device.h | 20 +++++++++
2 files changed, 123 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 3717f2a20d0d..011ba9bf7642 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -336,3 +336,106 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
+
+/**
+ * of_device_links_add - Create links between consumer and suppliers from
+ * device tree data
+ *
+ * @consumer: consumer device
+ *
+ * Returns 0 on success, < 0 on failure.
+ */
+int of_device_links_add(struct device *consumer)
+{
+ struct device_node *np;
+ struct platform_device *pdev;
+ int i = 0;
+
+ np = of_parse_phandle(consumer->of_node, "links-add", i++);
+ while (np) {
+ pdev = of_find_device_by_node(np);
+ of_node_put(np);
+ if (!pdev)
+ return -EINVAL;
+
+ device_link_add(consumer, &pdev->dev, DL_FLAG_STATELESS);
+ platform_device_put(pdev);
+
+ np = of_parse_phandle(consumer->of_node, "links-add", i++);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_device_links_add);
+
+/**
+ * of_device_links_remove - Remove links between consumer and suppliers from
+ * device tree data
+ *
+ * @consumer: consumer device
+ *
+ * Returns 0 on success, < 0 on failure.
+ */
+int of_device_links_remove(struct device *consumer)
+{
+ struct device_node *np;
+ struct platform_device *pdev;
+ int i = 0;
+
+ np = of_parse_phandle(consumer->of_node, "links-add", i++);
+ while (np) {
+ pdev = of_find_device_by_node(np);
+ of_node_put(np);
+ if (!pdev)
+ return -EINVAL;
+
+ device_link_remove(consumer, &pdev->dev);
+ platform_device_put(pdev);
+
+ np = of_parse_phandle(consumer->of_node, "links-add", i++);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_device_links_remove);
+
+static void devm_of_device_links_remove(struct device *dev, void *res)
+{
+ of_device_links_remove(*(struct device **)res);
+}
+
+/**
+ * devm_of_device_links_add - Create links between consumer and suppliers
+ * from device tree data
+ *
+ * @consumer: consumer device
+ *
+ * Returns 0 on success, < 0 on failure.
+ *
+ * Similar to of_device_links_add(), but will automatically call
+ * of_device_links_remove() when the device is unbound from the bus.
+ */
+int devm_of_device_links_add(struct device *consumer)
+{
+ struct device **ptr;
+ int ret;
+
+ if (!consumer)
+ return -EINVAL;
+
+ ptr = devres_alloc(devm_of_device_links_remove,
+ sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ ret = of_device_links_add(consumer);
+ if (ret < 0) {
+ devres_free(ptr);
+ } else {
+ *ptr = consumer;
+ devres_add(consumer, ptr);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_of_device_links_add);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 8d31e39dd564..ad01db6828e8 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -41,6 +41,11 @@ extern int of_device_request_module(struct device *dev);
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+
+extern int of_device_links_add(struct device *consumer);
+extern int of_device_links_remove(struct device *consumer);
+extern int devm_of_device_links_add(struct device *consumer);
+
static inline void of_device_node_put(struct device *dev)
{
of_node_put(dev->of_node);
@@ -91,6 +96,21 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static int of_device_links_add(struct device *consumer)
+{
+ return 0;
+}
+
+static int of_device_links_remove(struct device *consumer)
+{
+ return 0;
+}
+
+static int devm_of_device_links_add(struct device *consumer)
+{
+ return 0;
+}
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
--
2.15.0
From: Yannick Fertré <[email protected]>
Add a call to devm_of_device_links_add() to create links with suppliers
at probe time.
Signed-off-by: Yannick Fertré <[email protected]>
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/input/touchscreen/goodix.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f57d82220a88..9aefbfa39319 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -30,6 +30,7 @@
#include <linux/slab.h>
#include <linux/acpi.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <asm/unaligned.h>
struct goodix_ts_data;
@@ -812,6 +813,8 @@ static int goodix_ts_probe(struct i2c_client *client,
ts->chip = goodix_get_chip_data(ts->id);
+ devm_of_device_links_add(&client->dev);
+
if (ts->gpiod_int && ts->gpiod_rst) {
/* update device config */
ts->cfg_name = devm_kasprintf(&client->dev, GFP_KERNEL,
--
2.15.0
On 4/24/19 3:06 PM, Rafael J. Wysocki wrote:
> On 4/24/2019 12:19 PM, Benjamin Gaignard wrote:
>> Allows to create and remove links between consumer and suppliers from
>> device-tree data. Use 'links-add' property from consumer node to setup
>> a link with a list of suppliers.
>
> One immediate question about this one is why stateless links are
> better here?
They aren't better, it is just because I wanted to keep one of_ function
for each related device_link_* functions.
>
>> Consumers will be suspend before their suppliers and resume after them.
>>
>> Add devm_of_device_links_add() to automatically remove the links
>> when the device is unbound from the bus.
>
> And this might not be necessary even with managed links.
I guess I could use DL_FLAG_PM_RUNTIME and DL_FLAG_AUTOREMOVE_CONSUMER
flags and just keep
of_device_links_add() but the naming will not be explicit.
>
>> Signed-off-by: Benjamin Gaignard <[email protected]>
>> ---
>> drivers/of/device.c | 103
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/of_device.h | 20 +++++++++
>> 2 files changed, 123 insertions(+)
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 3717f2a20d0d..011ba9bf7642 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -336,3 +336,106 @@ int of_device_uevent_modalias(struct device
>> *dev, struct kobj_uevent_env *env)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
>> +
>> +/**
>> + * of_device_links_add - Create links between consumer and suppliers
>> from
>> + * device tree data
>> + *
>> + * @consumer: consumer device
>> + *
>> + * Returns 0 on success, < 0 on failure.
>> + */
>> +int of_device_links_add(struct device *consumer)
>> +{
>> + struct device_node *np;
>> + struct platform_device *pdev;
>> + int i = 0;
>> +
>> + np = of_parse_phandle(consumer->of_node, "links-add", i++);
>> + while (np) {
>> + pdev = of_find_device_by_node(np);
>> + of_node_put(np);
>> + if (!pdev)
>> + return -EINVAL;
>> +
>> + device_link_add(consumer, &pdev->dev, DL_FLAG_STATELESS);
>> + platform_device_put(pdev);
>> +
>> + np = of_parse_phandle(consumer->of_node, "links-add", i++);
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(of_device_links_add);
>> +
>> +/**
>> + * of_device_links_remove - Remove links between consumer and
>> suppliers from
>> + * device tree data
>> + *
>> + * @consumer: consumer device
>> + *
>> + * Returns 0 on success, < 0 on failure.
>> + */
>> +int of_device_links_remove(struct device *consumer)
>> +{
>> + struct device_node *np;
>> + struct platform_device *pdev;
>> + int i = 0;
>> +
>> + np = of_parse_phandle(consumer->of_node, "links-add", i++);
>> + while (np) {
>> + pdev = of_find_device_by_node(np);
>> + of_node_put(np);
>> + if (!pdev)
>> + return -EINVAL;
>> +
>> + device_link_remove(consumer, &pdev->dev);
>> + platform_device_put(pdev);
>> +
>> + np = of_parse_phandle(consumer->of_node, "links-add", i++);
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(of_device_links_remove);
>> +
>> +static void devm_of_device_links_remove(struct device *dev, void *res)
>> +{
>> + of_device_links_remove(*(struct device **)res);
>> +}
>> +
>> +/**
>> + * devm_of_device_links_add - Create links between consumer and
>> suppliers
>> + * from device tree data
>> + *
>> + * @consumer: consumer device
>> + *
>> + * Returns 0 on success, < 0 on failure.
>> + *
>> + * Similar to of_device_links_add(), but will automatically call
>> + * of_device_links_remove() when the device is unbound from the bus.
>> + */
>> +int devm_of_device_links_add(struct device *consumer)
>> +{
>> + struct device **ptr;
>> + int ret;
>> +
>> + if (!consumer)
>> + return -EINVAL;
>> +
>> + ptr = devres_alloc(devm_of_device_links_remove,
>> + sizeof(*ptr), GFP_KERNEL);
>> + if (!ptr)
>> + return -ENOMEM;
>> +
>> + ret = of_device_links_add(consumer);
>> + if (ret < 0) {
>> + devres_free(ptr);
>> + } else {
>> + *ptr = consumer;
>> + devres_add(consumer, ptr);
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_of_device_links_add);
>> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
>> index 8d31e39dd564..ad01db6828e8 100644
>> --- a/include/linux/of_device.h
>> +++ b/include/linux/of_device.h
>> @@ -41,6 +41,11 @@ extern int of_device_request_module(struct device
>> *dev);
>> extern void of_device_uevent(struct device *dev, struct
>> kobj_uevent_env *env);
>> extern int of_device_uevent_modalias(struct device *dev, struct
>> kobj_uevent_env *env);
>> +
>> +extern int of_device_links_add(struct device *consumer);
>> +extern int of_device_links_remove(struct device *consumer);
>> +extern int devm_of_device_links_add(struct device *consumer);
>> +
>> static inline void of_device_node_put(struct device *dev)
>> {
>> of_node_put(dev->of_node);
>> @@ -91,6 +96,21 @@ static inline int of_device_uevent_modalias(struct
>> device *dev,
>> return -ENODEV;
>> }
>> +static int of_device_links_add(struct device *consumer)
>> +{
>> + return 0;
>> +}
>> +
>> +static int of_device_links_remove(struct device *consumer)
>> +{
>> + return 0;
>> +}
>> +
>> +static int devm_of_device_links_add(struct device *consumer)
>> +{
>> + return 0;
>> +}
>> +
>> static inline void of_device_node_put(struct device *dev) { }
>> static inline const struct of_device_id *__of_match_device(
>
>
Hi Benjamin,
On Wed, Apr 24, 2019 at 12:19:11PM +0200, Benjamin Gaignard wrote:
> From: Yannick Fertré <[email protected]>
>
> Add a call to devm_of_device_links_add() to create links with suppliers
> at probe time.
>
> Signed-off-by: Yannick Fertr? <[email protected]>
> Signed-off-by: Benjamin Gaignard <[email protected]>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 702bfda7ee77..ac9f7e85efb0 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -1167,6 +1167,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, tsdata);
>
> + devm_of_device_links_add(&client->dev);
> +
This seems pretty generic action and I believe it should be done in
generic code, either bus (i2c, spi, etc), or in device core.
Thanks.
--
Dmitry
On 4/25/19 12:52 AM, Dmitry Torokhov wrote:
> Hi Benjamin,
>
> On Wed, Apr 24, 2019 at 12:19:11PM +0200, Benjamin Gaignard wrote:
>> From: Yannick Fertré <[email protected]>
>>
>> Add a call to devm_of_device_links_add() to create links with suppliers
>> at probe time.
>>
>> Signed-off-by: Yannick Fertré <[email protected]>
>> Signed-off-by: Benjamin Gaignard <[email protected]>
>> ---
>> drivers/input/touchscreen/edt-ft5x06.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
>> index 702bfda7ee77..ac9f7e85efb0 100644
>> --- a/drivers/input/touchscreen/edt-ft5x06.c
>> +++ b/drivers/input/touchscreen/edt-ft5x06.c
>> @@ -1167,6 +1167,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>>
>> i2c_set_clientdata(client, tsdata);
>>
>> + devm_of_device_links_add(&client->dev);
>> +
> This seems pretty generic action and I believe it should be done in
> generic code, either bus (i2c, spi, etc), or in device core.
It is done (or could be done) in most of the buses or framework (like
for regulator)
but I think that adding it in device core add a device-tree node parsing
for all the
drivers while only a few of them will really need this feature. That why
I have put the
call here.
Benjamin
>
> Thanks.
>
On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
<[email protected]> wrote:
>
> It could happen that we need to control suspend/resume ordering between
> devices without obvious consumer/supplier link. For example when touchscreens
> and DSI panels share the same reset line, in this case we need to be sure
> of pm_runtime operations ordering between those two devices to correctly
> perform reset.
> DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
> I2C client and I2C bus or regulator client and regulator provider) so we need
> to describe this in device-tree.
Needing to know which touchscreen is attached to a panel could be
important to describe if you have multiple displays.
Doesn't the reset subsystem already have some support for shared
resets? Seems like it could provide clients with struct device or
device_node ptrs to other devices sharing a reset.
>
> This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
> helpers to find and parse 'links-add' property in a device-tree node.
Going to document that property somewhere? :)
I think this is too generic and coupled to Linux. It doesn't have any
information as to what is the dependency or connection nor what the
direction of the dependency is.
I'm not convinced we need to solve this generically vs. defining
something for this specific example.
Rob
On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <[email protected]> wrote:
>
> On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
> <[email protected]> wrote:
> >
> > It could happen that we need to control suspend/resume ordering between
> > devices without obvious consumer/supplier link. For example when touchscreens
> > and DSI panels share the same reset line, in this case we need to be sure
> > of pm_runtime operations ordering between those two devices to correctly
> > perform reset.
> > DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
> > I2C client and I2C bus or regulator client and regulator provider) so we need
> > to describe this in device-tree.
>
> Needing to know which touchscreen is attached to a panel could be
> important to describe if you have multiple displays.
>
> Doesn't the reset subsystem already have some support for shared
> resets? Seems like it could provide clients with struct device or
> device_node ptrs to other devices sharing a reset.
>
> >
> > This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
> > helpers to find and parse 'links-add' property in a device-tree node.
>
> Going to document that property somewhere? :)
>
> I think this is too generic and coupled to Linux. It doesn't have any
> information as to what is the dependency or connection nor what the
> direction of the dependency is.
>
> I'm not convinced we need to solve this generically vs. defining
> something for this specific example.
I am pretty sure there will be more drivers needing complex
dependencies. Doesn't ACPI allow defining relationship between devices
that goes beyond the tree structure?
Thanks.
--
Dmitry
On Thu, Apr 25, 2019 at 2:24 PM Dmitry Torokhov
<[email protected]> wrote:
>
> On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <[email protected]> wrote:
> >
> > On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
> > <[email protected]> wrote:
> > >
> > > It could happen that we need to control suspend/resume ordering between
> > > devices without obvious consumer/supplier link. For example when touchscreens
> > > and DSI panels share the same reset line, in this case we need to be sure
> > > of pm_runtime operations ordering between those two devices to correctly
> > > perform reset.
> > > DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
> > > I2C client and I2C bus or regulator client and regulator provider) so we need
> > > to describe this in device-tree.
> >
> > Needing to know which touchscreen is attached to a panel could be
> > important to describe if you have multiple displays.
> >
> > Doesn't the reset subsystem already have some support for shared
> > resets? Seems like it could provide clients with struct device or
> > device_node ptrs to other devices sharing a reset.
> >
> > >
> > > This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
> > > helpers to find and parse 'links-add' property in a device-tree node.
> >
> > Going to document that property somewhere? :)
> >
> > I think this is too generic and coupled to Linux. It doesn't have any
> > information as to what is the dependency or connection nor what the
> > direction of the dependency is.
> >
> > I'm not convinced we need to solve this generically vs. defining
> > something for this specific example.
>
> I am pretty sure there will be more drivers needing complex
> dependencies. Doesn't ACPI allow defining relationship between devices
> that goes beyond the tree structure?
Can't speak to ACPI, but I assume you where implying ACPI supports
this so DT should too.
Almost every binding we have is defining relationships between
devices. Interrupts, clocks, gpio, pinctrl, etc. all do this. To use a
similar example to the one here, we already define the relationship
between a display and a backlight with a 'backlight' property in the
display node. Why should touchscreen be any different than backlight?
What really concerns me here is folks just add relationships to
'links-add' which are already captured in DT (such as backlight) just
because they'll get it for free and not have to go add support to
handle each specific binding.
Rob
On 4/26/19 1:02 AM, Rob Herring wrote:
> On Thu, Apr 25, 2019 at 2:24 PM Dmitry Torokhov
> <[email protected]> wrote:
>> On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <[email protected]> wrote:
>>> On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
>>> <[email protected]> wrote:
>>>> It could happen that we need to control suspend/resume ordering between
>>>> devices without obvious consumer/supplier link. For example when touchscreens
>>>> and DSI panels share the same reset line, in this case we need to be sure
>>>> of pm_runtime operations ordering between those two devices to correctly
>>>> perform reset.
>>>> DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
>>>> I2C client and I2C bus or regulator client and regulator provider) so we need
>>>> to describe this in device-tree.
>>> Needing to know which touchscreen is attached to a panel could be
>>> important to describe if you have multiple displays.
>>>
>>> Doesn't the reset subsystem already have some support for shared
>>> resets? Seems like it could provide clients with struct device or
>>> device_node ptrs to other devices sharing a reset.
Sharing reset will not help here because we don't want to have two reset
occuring in the same line but only one when the both devices are resumed.
That why we to force suspend/resume ordering and so add a link between the
devices.
>>>
>>>> This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
>>>> helpers to find and parse 'links-add' property in a device-tree node.
>>> Going to document that property somewhere? :)
I have put a description of this property in each device bindings but if
you can tell me
where to put it, I will follow your advice.
>>>
>>> I think this is too generic and coupled to Linux. It doesn't have any
>>> information as to what is the dependency or connection nor what the
>>> direction of the dependency is.
Could something like 'link-suppliers' or 'pm-runtime-dependencies'
are more explicit property name to describe direction, goal, and connection
between consumer and supplier ?
>>>
>>> I'm not convinced we need to solve this generically vs. defining
>>> something for this specific example.
>> I am pretty sure there will be more drivers needing complex
>> dependencies. Doesn't ACPI allow defining relationship between devices
>> that goes beyond the tree structure?
> Can't speak to ACPI, but I assume you where implying ACPI supports
> this so DT should too.
>
> Almost every binding we have is defining relationships between
> devices. Interrupts, clocks, gpio, pinctrl, etc. all do this. To use a
> similar example to the one here, we already define the relationship
> between a display and a backlight with a 'backlight' property in the
> display node. Why should touchscreen be any different than backlight?
It is different because it is only about suspend/resume ordering.
There is no need for a panel to knows about touchscreen unlike
than backlight that it could drive.
I have the same need to order suspend/resume operations between
GPU and display to be sure that GPU is suspend before display and resume
after it.
>
> What really concerns me here is folks just add relationships to
> 'links-add' which are already captured in DT (such as backlight) just
> because they'll get it for free and not have to go add support to
> handle each specific binding.
It won't be for free since I don't want to put it in device core so each
driver
will have to add a call to a function to enable it and will have to
explain why doing it.
Benjamin
>
> Rob