Thank you Kurt! Den lör 25 jan. 2025 kl 16:06 skrev Kurt Borja <kuurtb@xxxxxxxxx>: > > Now I understand the original problem better. I didn't consider this > possibility when designing the callback. > > While this is a fine solution I believe Thomas' EOPNOTSUPP solution is > the way to go. I think positive err value would be the safest but you > should wait for the advice of someone with more experience. > > Aside from that I really like how the whole platform profile sections > works now. Good design choices :) > > ~ Kurt > > > <snip> Regarding using this positive error code internally within the module, I thought about maybe adding a comment to galaxybook_probe() before all of the inits which describe this a bit -- do you all think this will be helpful or is it clear enough / does not matter and can be skipped? I also realized that maybe it is worth to describe that a specific sequence is needed for doing these "enable feature" + init calls to the ACPI methods otherwise some devices were reported as starting to reject the payloads if the sequence was not followed. Based on these two then I have drafted a comment sort of like this to put in galaxybook_probe() before the init() calls: /* * Features must be enabled and initialized in the following order to * avoid failures seen on certain devices: * - GB_SASB_POWER_MANAGEMENT (including performance mode) * - GB_SASB_KBD_BACKLIGHT * - GB_SASB_BLOCK_RECORDING (as part of fw_attrs init) * * The init function for features which are not supported on all devices * will return EOPNOTSUPP (positive to differentiate it from upstream * error codes) if the feature is not working and should be ignored. */ Does adding something like this seem like it would help make everything more clear (especially thinking when new refactoring comes by other maintainers in X months/years/decades, it would probably help them to know these subtleties, right?)? If this comment (you all are welcome to suggest wording tweaks as well, of course!) plus the few other small tweaks make sense then I can prep this to send as a new version. But I am holding a bit in hopes that the 6.14 stuff gets merged to pdx86 for-next so that I can go ahead with implementing Thomas's new power supply extension interface at the same time. Because there are multiple variations to these devices, and there were some small issues that users with other devices found, I was thinking/hoping once all looks good for all reviewers, including implementing the power supply extension, that this could be merged in to for-next and then I can ask a few people with other supported devices to test this revamped (and in some ways completely refactored) driver directly from the branch so that we can try to catch any other issues that I did not see on my device before it is proposed as a candidate for mainline -- does that sound reasonable? Thanks again! Best regards, Joshua
