On 3.01.2024 11:40, Johan Hovold wrote: > On Tue, Jan 02, 2024 at 06:03:36PM +0100, Konrad Dybcio wrote: >> On 2.01.2024 11:17, Johan Hovold wrote: >>> On Sat, Dec 30, 2023 at 02:16:18AM +0100, Konrad Dybcio wrote: >>>> On 29.12.2023 16:29, Johan Hovold wrote: >>>>> On Fri, Dec 29, 2023 at 04:01:27PM +0100, Konrad Dybcio wrote: >>>>>> On 29.12.2023 15:04, Johan Hovold wrote: >>>>>>> On Wed, Dec 27, 2023 at 11:17:19PM +0100, Konrad Dybcio wrote: >>>>>>>> At least on SC8280XP, if the PCIe reset is asserted, the corresponding >>>>>>>> AUX_CLK will be stuck at 'off'. >>>>>>> >>>>>>> No, this path is exercised on every boot without the aux clock ever >>>>>>> being stuck at off. So something is clearly missing in this description. >>>>> >>>>>> That's likely because the hardware has been initialized and not cleanly >>>>>> shut down by your bootloader. When you reset it, or your bootloader >>>>>> wasn't so kind, you need to start initialization from scratch. >>>>> >>>>> What does that even mean? I'm telling you that this reset is asserted on >>>>> each boot, on all sc8280xp platforms I have access to, and never have I >>>>> seen the aux clk stuck at off. >>>>> >>>>> So clearly your claim above is too broad and the commit message is >>>>> incorrect or incomplete. > >>> We're clearly talking past each other. When I'm saying reset is asserted >>> on each boot, I'm referring to reset being asserted in >>> qcom_pcie_init_2_7_0(), whereas you appear to be referring to whether >>> the reset has been left asserted by the bootloader when the driver >>> probes. >> >> OK, "boot" meant "booting the device" to me, not the PCIe controller. > > Still not getting across to you apparently. > > Again, the code in question is exercised on every boot and not once have > I seen a stuck clock due to reset being asserted *in* > qcom_pcie_init_2_7_0(). > > Now that's not what you were trying to describe as you were thinking of > reset having been left asserted *before* the driver probes (or before > qcom_pcie_init_2_7_0() is called). > > See? Do you understand now what I was trying to say and why my > misinterpretation of your terse commit message lead me to claim that it > was clearly false? No, my response was an acknowledgement of having understood you. Maybe it's a direct translation of some Polish idiom that's not obvious to others, but I definitely tried to say that "we were talking about different things, I had been previously thinking of something else, but now we're on the same page". > >>> I understand what you meant to say now, but I think you should rephrase: >>> >>> At least on SC8280XP, if the PCIe reset is asserted, the >>> corresponding AUX_CLK will be stuck at 'off'. >>> >>> because as it stands, it sounds like the problem happens when the driver >>> asserts reset. >> >> Does this sound good? >> >> "At least on SC8280XP, trying to enable the AUX_CLK associated with >> a PCIe host fails if the corresponding PCIe reset is asserted." > > Yes, but you need to also say something about how this would happen, for > example, your hypothetical bootloader leaving it asserted and your actual > motivation for this change which is that it appears to be needed after > suspend. > > A commit message should be clear and self-contained and not force > reviewers to have to try to interpret what it means and guess what the > motivation for the change really is. Got it Konrad
