On 19/03/2023 17:38, Jan Jasper de Kroon wrote: >> >>> - The property is required for proper system operation and is not optional >>> in specific device use cases. To be more specific in the case of the >>> PinePhone Original and Pro. The original commit message of the driver >>> implementation in driver/input/touchscreen contained the following: >>> "It consumes quite a bit of power (~40mW) during system sleep, and more >>> when the screen is touched." >>> Because the phone is usually kept in your pocket, so prone to a lot of >>> screen touches, this is highly undesired behavior for the touchscreen in >>> this case. This in my opinion makes it a mandatory property in this >>> situation. >> Why then the touchscree should not be kept in reset for other devices? >> IOW, this should be always used. If you now say "I prefer to keep or not >> keep it in reset for my device" - it's a policy. > Thank you for your question. While it's true that keeping the touchscreen > in reset state during system sleep can reduce power consumption for other > devices, the decision to use this property should be based on the specific > use case and hardware configuration of each device. In the case of the > PinePhone Original and Pro, the touchscreen's power consumption during > system sleep is significant, and the device is often kept in a pocket, so > accidental screen touches can occur frequently, leading to further power > drain. As such, keeping the touchscreen in reset state is necessary for > proper system operation in these specific devices. However, for other > devices with different hardware configurations and use cases, the decision > to use this property should be based on a thorough assessment of the power > consumption and potential impact on system behavior. Why? Even if energy consumption for these other devices is very low, it is still reasonable to keep the touchscreen off during suspend. Why anyone would like otherwise? Wakeup could be the reason, but for this we have property. >> >> >>> - The property is not a user-facing configuration option and is not meant >>> to be changed by the end-user. >> Does not matter. >> >>> - The property, although in separate device specific kernel, and still >>> called 'poweroff-in-suspend' is already in use on specific devices, >>> including the PinePhone Original and PinePhone Pro. >> I could not find such property in the kernel. > I apologize for the confusion, but the current mainline kernel doesn't > include this property. As I stated to support the PinePhone Original and > PinePhone Pro, the community makes use of a forked mainline kernel, with > a lot of out-of-tree patches/commits, mainly maintained by developer > Ondrej Jirman. For the PinePhone Original, the DeviceTree configuration > in the PinePhone DTS gets set in the following commit: > https://github.com/megous/linux/commit/74fc0a5f0527afdccb67ce3be690f0ae18c8eca6 > For the PinePhone Pro it is set in the following commit, at line 466: > https://github.com/megous/linux/commit/471c5f33ba74c3d498ccc1eb69c098623b193926# > The property here is still called "poweroff-in-suspend". Whatever forks are doing is rarely argument for us. Did that property pass DT maintainers review? No. >> >>> However, I understand your concern that Devicetree should not be used for >>> policies. To address this concern, I would like to propose the following >>> changes to the property description: >>> 1. Remove the sentence about reducing power consumption, as this could be >>> considered a policy. >>> 2. Emphasize that the property is a required hardware configuration and >>> not an optional feature on certain devices. >>> 3. Recommend that any changes to the property value should only be made by >>> experienced system administrators and not end-users. >> Please answer - why this should not be enabled always. > One reason why the Touchscreen Controller should not be kept in reset > always is that some devices may have a use case where the touchscreen > needs to remain active even during system sleep, and keeping it in reset > would cause issues with that case. Use case is rather policy... Except wakeup, but for this we have property. > However, in the case of the > battery-powered PinePhone Original and Pro, keeping the touchscreen > controller in reset during system sleep is required for proper system > operation. The question was "why not enabled always". How this is related? > Having the device in your pocket makes unintentional screen > touches almost unavoidable, and this property enabled is necessary for > these devices. In the case of the PinePhone Original and Pro, enabling > this feature we do consider its impact on battery life or in other words > power consumption. You keep repeating the same and email is very long. > But bottomlined, enabling this feature should be evaluated on a > case-by-case basis, taking into consideration the specific device use case > and hardware configurations. Thank you for your feedback. Best regards, Krzysztof
