On Mon, Jul 8, 2024 at 10:55 AM Krzysztof Wilczyński <kw@xxxxxxxxx> wrote: > > Hello, > > > > Any input from a PCI maintainer here? > > Something that I am curious about: can we make this a single file with a > bitmask inside that denotes what DOE features are enabled? Would this be > approach be even feasible here? In theory there can be any vendor ID (16-bits but not 0xFFFF) and any feature (8-bits). So there is a huge possibility of values here. > > Thoughts? Or is it too late to think about this now? It's just too many possible options to use a bitmask. I guess we could use a feature bit mask per vendor if people feel strongly > > > > There are basically two approaches. > > > > > > 1. We can have a pci_doe_sysfs_init() function that is called where > > > we dynamically add the entries, like in v12 > > > 2. We can go down the dev->groups and device_add() path, like this > > > patch and discussed at > > > https://lore.kernel.org/all/20231019165829.GA1381099@bhelgaas/ > > > > > > For the second we will have to create a global pci_doe_sysfs_group > > > that contains all possible DOE entries on the system and then have the > > > show functions determine if they should be displayed for that device. > > > > > > Everytime we call pci_doe_init() we can check for any missing entries > > > in pci_doe_sysfs_group.attrs and then realloc > > > pci_doe_sysfs_group.attrs to add them. > > > Untested, but that should work > > > even for hot-plugged devices. pci_doe_sysfs_group.attrs would just > > > grow forever though as I don't think we have an easy way to deallocate > > > anything as we aren't sure if we are the only entry. > > > > I think this needs to be per device, not global and you'll have to manually > > do the group visibility magic rather than using the macros. > > Lukas proposes a very interesting feature of kernfs recently per: > > https://lore.kernel.org/linux-pci/16490618cbde91b5aac04873c39c8fb7666ff686.1719771133.git.lukas@xxxxxxxxx > > Would this help with DOE features? That was the previous approach used here: https://lore.kernel.org/linux-pci/20240626045926.680380-3-alistair.francis@xxxxxxx/ Bjorn wanted to try and avoid using a function pci_doe_sysfs_init() [1], which is what I tried here. It sounds like the v12 approach is the way to go then. I'll send a v14 based on v12 with the comments addressed 1: https://lore.kernel.org/all/20231019165829.GA1381099@bhelgaas/ Alistair > > Krzysztof
