On 01/01/2026 13:50, [email protected] 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 ?
Anyway leaving that aside the bit that's really objectionable and IMO
obvious a bug is val |= readl();
Why or the bit back in ? and then why not check the bit was set on the
read ?
val = readl() is a lot less janky and shouldn't it matter that the bit
we tried to set is actually reflected in the read-back ?
Failure to set the bit would certainly be a problem...
---
bod
