Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 21 Sep 2022 12:49:13 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     jia-wei.chang@mediatek.com,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     rafael@kernel.org, lgirdwood@gmail.com, broonie@kernel.org,
        matthias.bgg@gmail.com, andrew-sh.cheng@mediatek.com,
        rex-bc.chen@mediatek.com, nfraprado@collabora.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] cpufreq: mediatek: Fix KP and lockups on proc/sram
 regulators error
Message-ID: <20220921071913.p7kwsjnnuad2jgvk@vireshk-i7>
References: <20220909093724.40078-1-angelogioacchino.delregno@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220909093724.40078-1-angelogioacchino.delregno@collabora.com>
Precedence: bulk

Jia, do you want to reply to this thread as the Fixes patch was added
by you ?

On 09-09-22, 11:37, AngeloGioacchino Del Regno wrote:
> Function regulator_get_optional() returns a negative error number on
> any kind of regulator_get() failure: failing to check for that in the
> teardown path will lead to a kernel panic due to a call to function
> regulator_disable().

I don't see how this can happen. The code does check if the regulators
are enabled earlier or not.

> Besides that, the "proc" regulator does actually provide power to the
> CPU cluster(s): disabling it will produce a lockup on at least some
> SoCs, such as MT8173.

We are just dropping the count that we increased earlier, how will
that disable the regulator which was already enabled ?

> That consideration is also valid for the "sram" regulator, providing
> power to the CPU caches instead, present on some other SoCs, such as
> MT8183, MT8186 (and others).
> 
> Resolve both situations and by simply removing the entire faulty
> branches responsible for disabling the aforementioned regulators if
> enabled, keeping in mind that these are enabled (and left enabled)
> by the bootloader before booting the kernel.

This looks fishy, we just keep on increasing the ref count of the
regulator but never take it down.

-- 
viresh