Re: [RFC PATCH 0/5] device property: Introducing software nodes
Hi Heikki, On Wed, Oct 17, 2018 at 04:36:12PM +0300, Heikki Krogerus wrote: > @@ -189,7 +210,7 @@ static int cht_int33fe_probe(struct i2c_client *client) > 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->fusb302_node; > board_info.irq = fusb302_irq; > > data->fusb302 = i2c_acpi_new_device(dev, 2, &board_info); OK, this totally explains it. I forgot how we can supply the properties in board info for I2C/SPI devices and they would get attached to a device later, and this obviously does not work if one wants to attach a sub-tree. Thank you for providing this example. -- Dmitry
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 05:35:50PM +0300, Andy Shevchenko wrote: > On Fri, Oct 12, 2018 at 2:41 PM Heikki Krogerus > wrote: > > > > Hi guys, > > > > To continue the discussion started by Dmitry [1], this is my proposal > > that I mentioned in my last mail. In short, the idea is that instead > > of trying to extend the support for the currently used struct > > property_set, I'm proposing that we introduce a completely new, > > independent type of fwnode, and replace the struct property_set with > > it. I'm calling the type "software node" here. > > > > The reason for a complete separation of the software nodes from the > > generic property handling code is the need to be able to create the > > nodes independently from the devices that they are bind to. > > > > The way this works is that every node that is created will have a > > kobject registered. That will take care the ref counting for us, and > > also allow us to for example display the properties in sysfs. > > > > There are a few more details in patch 3/5 about the software nodes in > > the commit message. > > > > [1] https://lkml.org/lkml/2018/9/17/1067 > > In private discussion I brought a concern that we exposed properties > as a part of ABI, but at the same time we have not strict rules which > might lead to ambiguous reading, e.g. there is no type exported and > thus no possibility to tell what kind of property it is. > > Examples: > 1. 0x1 and 0x1 ??? are they of the same type? > 2. 0x1 ??? is it an array or single value? > 3. 0x12345678 ??? is it string or hex? > 4. 25 ??? is it hex or decimal? This is mostly a note to self, but also to let everybody know: After thinking about this a bit more I realised that the user space has to know what the property it is reading contains. An array of integers can actually be a string in reality, just like a string may contain an integer value(s). String or string array could describe a data structure, or even supply the values for one. In reality the type of the property, or the fact that it's an array or not, do not help at all to determine the content of the property. So the user space has to know what a property returns if it wants to use it, and once the user space knows that, the user space will also know the type and other details about the property, including knowing is it an array or not. Based on that, I'm against any kind of grouping or naming of the properties, was it based on the type of the property or is it an array or not, or supplying any details about the properties in any way to the user space. That would only complicate the life for the user space, as the grouping or naming, or supplying the details about the properties in any way, does not provide any information that the user space does not already have. The details about the properties would just be a sort of a useless noise for the user space that just has to be filtered out. Therefore I'm going to continue to propose that we expose the properties exactly the way I'm doing now: we'll have the "properties" directory that contains an attribute file for every property named with the names of the properties, and nothing else. The output formats we can still debate about, but Andy had already good proposals for that. I'm still not planning to include the property exposure at the first stage. Well add it after the initial support is in. Thanks, -- heikki
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 10:32:44AM -0700, Dmitry Torokhov wrote: > Hi Heikki, > > On Fri, Oct 12, 2018 at 02:39:29PM +0300, Heikki Krogerus wrote: > > Hi guys, > > > > To continue the discussion started by Dmitry [1], this is my proposal > > that I mentioned in my last mail. In short, the idea is that instead > > of trying to extend the support for the currently used struct > > property_set, I'm proposing that we introduce a completely new, > > independent type of fwnode, and replace the struct property_set with > > it. I'm calling the type "software node" here. > > > > The reason for a complete separation of the software nodes from the > > generic property handling code is the need to be able to create the > > nodes independently from the devices that they are bind to. > > It would be great it you would provide an example of creating these > sowftware property sets separately from devices. How do you tie device > and its properties if they are not created together? The properties are bind to the software nodes and not the devices. The software nodes can then be bind to the devices when they are created. This is actually exactly the same behaviour that we had with the property_sets, the only difference being that we can bind the software node to the device at a later stage. > For OF we have compatibles and phandles for references, ACPI has > HIDs and CIDs and notion of references as well. What do we use here, > especially when software node is created in one subsystem (let's say > drivers/platform/x86), but device is created somewhere else? Reference usually means a handle to a node that is outside of the direct child-parent relationship (hierarchy) for the caller device node. That I do not support at this point. We support the node hierarchy which allows us to "refer" the child and parent nodes, and that is all that we need at this stage. Support for references is in my plans. I will need that later. But we'll do that as the next step. > Another issue that is not clear to me: looking at the USB connector it > seems you want to have references to fwnodes. If by references you mean here access to the nodes outside of the hierarchy, then you've misunderstood. I do not expect that with the USB connectors. > How do you resolve them when there are nodes of different class. > I.e. how do you express software fwnode referencing ACPI or DT node > when you are supplementing ACPI or DT description of a system with > these custom/secondary nodes? So references are not supported as I said, but I don't know if we can or even want to support references to different types of fwnodes. I'm not even sure we would ever need to refer an other type of fwnode from software node. I mean, we should always be able to place a secondary software node to both fwnodes. In any case, this topic is outside the scope of this series. And in case this was not clear, with the hierarchy, different types of fwnode nodes are not supported. It means that software node can only have software node parent and children. > What about the other direction? I.e. can I have a DT system with USB > connector and augment USB set up with static nodes? Not only > basic/scalar properties, but links as well? > > As I said, having and example of using this new code to achieve your > goal with regard to USB connector would be awesome and clear a lot of my > questions. OK. I used the attached code to test these on Intel Cherry Trail board. I'm creating two software nodes there: one for the FUSB302 controller, and one (a child of the FUSB302 node) for the connector. The fusb302 node is assigned to the i2c client that is registered in that driver, but the child node is left waiting. The child node has a property called "name" with value "connector". The fusb302 driver already requests a handle to the child node named "connector" in its probe function which it assigns to the port device that it registers. With the attached patch it will get the child node also on CherryTrail boards, no changes to the Type-C drivers needed. Here is the file listing that we see in sysfs (node3 is for FUSB302): % find /sys/kernel/software_nodes/ | grep -v properties ... /sys/kernel/software_nodes/node3 /sys/kernel/software_nodes/node3/node0 /sys/kernel/software_nodes/node3/node0/port0 /sys/kernel/software_nodes/node3/i2c-fusb302 ... The node for the FUSB302 controller: % ls -l /sys/kernel/software_nodes/node3/ total 0 lrwxrwxrwx1 root root 0 Oct 17 12:12 i2c-fusb302 -> ../../../devices/pci:00/808622C1:00/i2c-0/i2c-fusb302 drwxr-xr-x3 root root 0 Oct 17 12:03 node0 drwxr-xr-x2 root root 0 Oct 17 12:12 properties The node for the connector (child of node3): % ls -l /sys/kernel/software_nodes/node3/node0/ total 0 lrwxrwxrwx1 root root 0 Oct 17 12:12 port0 -> ../.
Re: [RFC PATCH 0/5] device property: Introducing software nodes
Hi Heikki, On Fri, Oct 12, 2018 at 02:39:29PM +0300, Heikki Krogerus wrote: > Hi guys, > > To continue the discussion started by Dmitry [1], this is my proposal > that I mentioned in my last mail. In short, the idea is that instead > of trying to extend the support for the currently used struct > property_set, I'm proposing that we introduce a completely new, > independent type of fwnode, and replace the struct property_set with > it. I'm calling the type "software node" here. > > The reason for a complete separation of the software nodes from the > generic property handling code is the need to be able to create the > nodes independently from the devices that they are bind to. It would be great it you would provide an example of creating these sowftware property sets separately from devices. How do you tie device and its properties if they are not created together? For OF we have compatibles and phandles for references, ACPI has HIDs and CIDs and notion of references as well. What do we use here, especially when software node is created in one subsystem (let's say drivers/platform/x86), but device is created somewhere else? Another issue that is not clear to me: looking at the USB connector it seems you want to have references to fwnodes. How do you resolve them when there are nodes of different class. I.e. how do you express software fwnode referencing ACPI or DT node when you are supplementing ACPI or DT description of a system with these custom/secondary nodes? What about the other direction? I.e. can I have a DT system with USB connector and augment USB set up with static nodes? Not only basic/scalar properties, but links as well? As I said, having and example of using this new code to achieve your goal with regard to USB connector would be awesome and clear a lot of my questions. Thanks! -- Dmitry
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 05:35:50PM +0300, Andy Shevchenko wrote: > On Fri, Oct 12, 2018 at 2:41 PM Heikki Krogerus > wrote: > > > > Hi guys, > > > > To continue the discussion started by Dmitry [1], this is my proposal > > that I mentioned in my last mail. In short, the idea is that instead > > of trying to extend the support for the currently used struct > > property_set, I'm proposing that we introduce a completely new, > > independent type of fwnode, and replace the struct property_set with > > it. I'm calling the type "software node" here. > > > > The reason for a complete separation of the software nodes from the > > generic property handling code is the need to be able to create the > > nodes independently from the devices that they are bind to. > > > > The way this works is that every node that is created will have a > > kobject registered. That will take care the ref counting for us, and > > also allow us to for example display the properties in sysfs. > > > > There are a few more details in patch 3/5 about the software nodes in > > the commit message. > > > > [1] https://lkml.org/lkml/2018/9/17/1067 > > In private discussion I brought a concern that we exposed properties > as a part of ABI, but at the same time we have not strict rules which > might lead to ambiguous reading, e.g. there is no type exported and > thus no possibility to tell what kind of property it is. > > Examples: > 1. 0x1 and 0x1 ??? are they of the same type? > 2. 0x1 ??? is it an array or single value? > 3. 0x12345678 ??? is it string or hex? > 4. 25 ??? is it hex or decimal? > > Until these will not be solved, better to not to expose properties to > userspace. I agree. I'll drop that part from my final version. Thanks Andy, -- heikki
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Fri, Oct 12, 2018 at 2:41 PM Heikki Krogerus wrote: > > Hi guys, > > To continue the discussion started by Dmitry [1], this is my proposal > that I mentioned in my last mail. In short, the idea is that instead > of trying to extend the support for the currently used struct > property_set, I'm proposing that we introduce a completely new, > independent type of fwnode, and replace the struct property_set with > it. I'm calling the type "software node" here. > > The reason for a complete separation of the software nodes from the > generic property handling code is the need to be able to create the > nodes independently from the devices that they are bind to. > > The way this works is that every node that is created will have a > kobject registered. That will take care the ref counting for us, and > also allow us to for example display the properties in sysfs. > > There are a few more details in patch 3/5 about the software nodes in > the commit message. > > [1] https://lkml.org/lkml/2018/9/17/1067 In private discussion I brought a concern that we exposed properties as a part of ABI, but at the same time we have not strict rules which might lead to ambiguous reading, e.g. there is no type exported and thus no possibility to tell what kind of property it is. Examples: 1. 0x1 and 0x1 — are they of the same type? 2. 0x1 — is it an array or single value? 3. 0x12345678 — is it string or hex? 4. 25 — is it hex or decimal? Until these will not be solved, better to not to expose properties to userspace. > > -- > heikki > > > Heikki Krogerus (5): > drivers core: Prepare support for multiple platform notifications > ACPI / glue: Add acpi_platform_notify() function > drivers: base: Introducing software nodes to the firmware node > framework > device property: Move device_add_properties() to swnode.c > device property: Remove struct property_set > > .../ABI/testing/sysfs-devices-software_node | 27 + > drivers/acpi/bus.c| 1 - > drivers/acpi/glue.c | 21 +- > drivers/acpi/internal.h | 1 - > drivers/base/Makefile | 2 +- > drivers/base/core.c | 32 +- > drivers/base/property.c | 529 +--- > drivers/base/swnode.c | 812 ++ > include/linux/acpi.h | 10 + > include/linux/property.h | 12 + > 10 files changed, 929 insertions(+), 518 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-devices-software_node > create mode 100644 drivers/base/swnode.c > > -- > 2.19.1 > -- With Best Regards, Andy Shevchenko
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 10:40 AM Heikki Krogerus wrote: > > On Tue, Oct 16, 2018 at 09:36:33AM +0200, Rafael J. Wysocki wrote: > > On Tue, Oct 16, 2018 at 9:35 AM Linus Walleij > > wrote: > > > > > > On Fri, Oct 12, 2018 at 1:39 PM Heikki Krogerus > > > wrote: > > > > > > > To continue the discussion started by Dmitry [1], this is my proposal > > > > that I mentioned in my last mail. In short, the idea is that instead > > > > of trying to extend the support for the currently used struct > > > > property_set, I'm proposing that we introduce a completely new, > > > > independent type of fwnode, and replace the struct property_set with > > > > it. I'm calling the type "software node" here. > > > > > > I'm a big fan of this approach. > > > Acked-by: Linus Walleij > > > for all patches. > > > > > > I don't know who can finally review and merge this though, > > > I guess Rafael? > > > > Yes, that would be me. :-) > > > > I no one speaks up against them, I'll pick them up. > > Let me send a final version of these. > > I need to add one more patch to the series where I remove an extra > device_remove_properties() call from platform_device_del(). > > It's unnecessary in any case as device_del() calls > device_remove_properties() for every device, but as the properties are > removed there before the device is removed, we're unable to deduct > the final ref count in the "remove" platform notification since our > node is no longer bind to the device. OK, I'll wait for an update, then. Thanks, Rafael
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 09:36:33AM +0200, Rafael J. Wysocki wrote: > On Tue, Oct 16, 2018 at 9:35 AM Linus Walleij > wrote: > > > > On Fri, Oct 12, 2018 at 1:39 PM Heikki Krogerus > > wrote: > > > > > To continue the discussion started by Dmitry [1], this is my proposal > > > that I mentioned in my last mail. In short, the idea is that instead > > > of trying to extend the support for the currently used struct > > > property_set, I'm proposing that we introduce a completely new, > > > independent type of fwnode, and replace the struct property_set with > > > it. I'm calling the type "software node" here. > > > > I'm a big fan of this approach. > > Acked-by: Linus Walleij > > for all patches. > > > > I don't know who can finally review and merge this though, > > I guess Rafael? > > Yes, that would be me. :-) > > I no one speaks up against them, I'll pick them up. Let me send a final version of these. I need to add one more patch to the series where I remove an extra device_remove_properties() call from platform_device_del(). It's unnecessary in any case as device_del() calls device_remove_properties() for every device, but as the properties are removed there before the device is removed, we're unable to deduct the final ref count in the "remove" platform notification since our node is no longer bind to the device. Thanks, -- heikki
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Tue, Oct 16, 2018 at 9:35 AM Linus Walleij wrote: > > On Fri, Oct 12, 2018 at 1:39 PM Heikki Krogerus > wrote: > > > To continue the discussion started by Dmitry [1], this is my proposal > > that I mentioned in my last mail. In short, the idea is that instead > > of trying to extend the support for the currently used struct > > property_set, I'm proposing that we introduce a completely new, > > independent type of fwnode, and replace the struct property_set with > > it. I'm calling the type "software node" here. > > I'm a big fan of this approach. > Acked-by: Linus Walleij > for all patches. > > I don't know who can finally review and merge this though, > I guess Rafael? Yes, that would be me. :-) I no one speaks up against them, I'll pick them up. Cheers, Rafael
Re: [RFC PATCH 0/5] device property: Introducing software nodes
On Fri, Oct 12, 2018 at 1:39 PM Heikki Krogerus wrote: > To continue the discussion started by Dmitry [1], this is my proposal > that I mentioned in my last mail. In short, the idea is that instead > of trying to extend the support for the currently used struct > property_set, I'm proposing that we introduce a completely new, > independent type of fwnode, and replace the struct property_set with > it. I'm calling the type "software node" here. I'm a big fan of this approach. Acked-by: Linus Walleij for all patches. I don't know who can finally review and merge this though, I guess Rafael? Yours, Linus Walleij
[RFC PATCH 0/5] device property: Introducing software nodes
Hi guys, To continue the discussion started by Dmitry [1], this is my proposal that I mentioned in my last mail. In short, the idea is that instead of trying to extend the support for the currently used struct property_set, I'm proposing that we introduce a completely new, independent type of fwnode, and replace the struct property_set with it. I'm calling the type "software node" here. The reason for a complete separation of the software nodes from the generic property handling code is the need to be able to create the nodes independently from the devices that they are bind to. The way this works is that every node that is created will have a kobject registered. That will take care the ref counting for us, and also allow us to for example display the properties in sysfs. There are a few more details in patch 3/5 about the software nodes in the commit message. [1] https://lkml.org/lkml/2018/9/17/1067 -- heikki Heikki Krogerus (5): drivers core: Prepare support for multiple platform notifications ACPI / glue: Add acpi_platform_notify() function drivers: base: Introducing software nodes to the firmware node framework device property: Move device_add_properties() to swnode.c device property: Remove struct property_set .../ABI/testing/sysfs-devices-software_node | 27 + drivers/acpi/bus.c| 1 - drivers/acpi/glue.c | 21 +- drivers/acpi/internal.h | 1 - drivers/base/Makefile | 2 +- drivers/base/core.c | 32 +- drivers/base/property.c | 529 +--- drivers/base/swnode.c | 812 ++ include/linux/acpi.h | 10 + include/linux/property.h | 12 + 10 files changed, 929 insertions(+), 518 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-devices-software_node create mode 100644 drivers/base/swnode.c -- 2.19.1
