Re: [PATCH v4 7/7] arm64: dts: qcom: x1e80100: Add deepest idle state

[Maulik Shah (mkshah)]

On 7/11/2026 11:37 PM, Bjorn Andersson wrote:
> On Tue, Jul 07, 2026 at 02:51:39PM +0530, Maulik Shah wrote:
>> Add deepest idle state as GPIO IRQs can work as wakeup capable interrupts
>> in deepest idle state.
>>
> 
> There's a lot of implied background in this sentence. When you write
> commit messages, please intend for them to be read by someone who don't
> have your background insight to the problem. In fact, if someone asks
> you about this sentence in 5 years would you be able to retell exactly
> why we ended up with this patch?
> 
> Please rewrite this to start with a problem description, then describe
> the user-visible change.

Agree. Added in v5.

> 
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> 
>> Update entry/exit-latency-us to follow DSDT for cluster_cl5 idle state.
> 
> I don't have strong opinions about bundling this part of the change - it
> could be argued that it's a separate change, but I won't force it.
> 
> But as written I think any reasonable language parser would consider
> this to be related to the addition of the deepest idle state, not "While
> we're adding the new state, also adjust the cluster_cl5 latencies with
> values from production configuration."

Added in v5 commit message on why its bundled in same change, 

"The newly added domain_ss3 state has entry/exit latencies of 2500 us.
The shallower cluster_cl5 state had entry/exit latencies of 2200/4000 us
which are higher than the deeper state — an inconsistency that would
confuse the idle governor. Correct cluster_cl5 latencies to 2000 us each
to match production configuration values".

Thanks,
Maulik