Gentle ping. Could you check this patch? > Thank you for your detailed feedback and suggestions. Based on your > comments, I have a few thoughts and questions to ensure we move forward > effectively. > > Jonathan Cameron wrote: > > On Thu, 12 Dec 2024 09:08:54 +0000 > > "Daisuke Kobayashi (Fujitsu)" <kobayashi.da-06@xxxxxxxxxxx> wrote: > > > > > Jonathan Cameron wrote: > > > > On Tue, 10 Dec 2024 13:08:21 +0900 "Kobayashi,Daisuke" > > > > <kobayashi.da-06@xxxxxxxxxxx> wrote: > > ... snip ... > > > Regarding your proposed file structure: > > > > > > budget0_power > > > budget0_pm_state > > > ... > > > budget1_power > > > budget1_pm_state > > > ... > > > budget2_xxx > > > ... > > > > > > Is this the correct format? > > > > Yes. That is what I had mind. > > > > Thanks, I see. > > > > > > > In my understanding, implementing this would require 256 attributes > > > corresponding to the DataSelectRegister, which seems overly redundant. > > > Additionally, I was concerned about the mechanism of changing the > > DataSelectRegister each time a file is read. > > > > I agree it is quite a large number of files, but there are larger > > sysfs interfaces so I'm not that concerned. Do you have any data on > > how many entries are typically used? > > We could use an is_visible() callback and register all 256, or we > > could do dynamic allocation though that would require walking to find out > what is there. > > > > The implementation should cache the current dataselectregister value > > and only write it if the value needs to change. Sensible userspace > > software would just read all the elements of each budgetX_* before moving > on to the next one. > > > > Unfortunately, I don't have specific data on the typical number of entries used. > However, I estimate that it would be within 10 at most. > Could you please provide examples of large sysfs interfaces and where they are > implemented? I would like to refer to them for implementation. > In pci-sysfs.c, there is an attribute called resourceX_resize, and I believe its > basic concept could be useful. However, the PowerBudget implementation is > more complex because it involves multiple sysfs outputs for each index. > If there are more reference implementations, I could propose a more mature > implementation. > > > > > > > Therefore, I proposed this patch implementation, even though it > > > might not be > > ideal. > > > > > > If you have a suitable implementation to achieve your proposed > > > structure, I would greatly appreciate it if you could share it. > > > > > > Upon further reflection, the issue of needing to change the > > > DataSelectRegister could potentially be avoided by initially > > > retrieving and > > storing the register values for all indices. > > > > Cacheing these is definitely an option to explore. I'm not sure we > > would want to pay that cost at boot, but if not we could do it on > > first touch. The 0 value is reserved anyway so if cached copy is 0, > > read it from the device and fill the cache. > > We would need to read some at boot to figure out which should be > > visible, but that would be logN of max entries at worse, rather than > > all 256. If there are only a few it would make sense to just read them all at > boot. > > > Although the exact number of indices is uncertain, I believe it is sufficiently > small. > Therefore, I plan to adopt a method where all values are read and cached at > boot time. > I intend to create an array in the struct pci_dev and initialize it in the probe > function. > If there are any other suitable places or methods, please let me know. > > > Annoying there isn't a count register for these! > > I also feel that. Thank you. > > > > Jonathan > > > > > > > > > > > > > > Thank you for pointing out the coding style issue, I'll fix it in the next patch. > > > > > > > > > > > > > Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@xxxxxxxxxxx> > ... snip ...
