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] [day] [month] [year] [list]

Message-ID: <af693a39a3ad88a573e2d97b3ab411b3@mainlining.org>
Date: Sat, 03 Jan 2026 08:41:01 +0100
From: barnabas.czeman@...nlining.org
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bryan O'Donoghue <bod@...nel.org>, 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 2026-01-02 13:00, Konrad Dybcio wrote:
> 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..
In this case I should go back to previous inrush current mitigation from 
v2.
> Perhaps I should have just read the comment
> 
> Konrad