linux-kernel - Re: [PATCH v3 3/9] remoteproc: qcom_q6v5

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1440e47e-2d7b-4d49-97c4-a717fadd3fb6@oss.qualcomm.com>
Date: Fri, 2 Jan 2026 13:00:50 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Bryan O'Donoghue <bod@...nel.org>, barnabas.czeman@...nlining.org
Cc: Bjorn Andersson <andersson@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Stephan Gerhold <stephan@...hold.net>, linux-arm-msm@...r.kernel.org,
        linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

On 1/1/26 9:58 PM, Bryan O'Donoghue wrote:
> On 01/01/2026 13:50, barnabas.czeman@...nlining.org wrote:
>>>> +                for (; i >= 0; i--) {
>>>> +                    val |= BIT(i);
>>>> +                    writel(val, qproc->reg_base + mem_pwr_ctl);
>>>> +                    /*
>>>> +                     * Read back value to ensure the write is done then
>>>> +                     * wait for 1us for both memory peripheral and data
>>>> +                     * array to turn on.
>>>> +                     */
>>>> +                    val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>> +                    udelay(1);
>>> Isn't the logic here inverted ?
>>>
>>> i.e. you've written a thing and ostensibly require a delay for that
>>> thing to take effect, the power to switch on in this case.
>>>
>>> It makes more sense to write, delay and read back rather than write,
>>> readback and delay surely...
>> This is the original reset sequence without modification, i have just
>> moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
> 
> Doesn't make it correct, we fix upstream logic bugs all the time...
> 
> For example a read-back to ensure write completion is only required for posted memory transactions.
> 
> Is this a posted write ?
> 
> Is there an io-fabric in the world which exceeds 1 microsecond to perform a write transaction ?

Writes on arm64 aren't usually observable from the remote endpoint when
you would expect them to, they can be buffered unless there's an explicit
readback right afterwards (which creates a dependency that the processor
will fulfill)

Now I don't like that this driver is going

val |= BIT(i);
writel(val, foo);
// val is "altered" but not really
val |= readl(foo);

I didn't notice we were just doing a readback for the sake of a readback
in the last revision. MDM9607 should most definitely have it too..
Perhaps I should have just read the comment

Konrad