| Message ID | 20190424101913.1534-1-benjamin.gaignard@st.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add of_ functions for device_link_add() | expand |
On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard <benjamin.gaignard@st.com> 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 <robh+dt@kernel.org> wrote: > > On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard > <benjamin.gaignard@st.com> 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.
On Thu, Apr 25, 2019 at 2:24 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard > > <benjamin.gaignard@st.com> 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 > <dmitry.torokhov@gmail.com> wrote: >> On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <robh+dt@kernel.org> wrote: >>> On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard >>> <benjamin.gaignard@st.com> 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
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(+)