Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Mime-Version: 1.0
Subject: Re: [PATCH v2 4/7] thermal: exynos: simplify regulator
 (de)initialization
Reply-To: m.majewski2@samsung.com
Sender: Mateusz Majewski <m.majewski2@samsung.com>
From:   Mateusz Majewski <m.majewski2@samsung.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "linux-samsung-soc@vger.kernel.org" 
        <linux-samsung-soc@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Amit Kucheria <amitk@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        ALIM AKHTAR <alim.akhtar@samsung.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <bf9a07ff-5628-05ab-2362-a917d3d38313@linaro.org>
Message-ID: <20230926110239eucms1p2b539245c5b10591def4cd15f14896ad6@eucms1p2>
Date:   Tue, 26 Sep 2023 13:02:39 +0200
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <bf9a07ff-5628-05ab-2362-a917d3d38313@linaro.org>
        <20230911133435.14061-1-m.majewski2@samsung.com>
        <20230911133435.14061-5-m.majewski2@samsung.com>
        <CGME20230911133616eucas1p10c5eeb0f0240dde975ccc5935cb5c311@eucms1p2>
Precedence: bulk

Hi,

> This is not equivalent. If regulator is provided and enable fails, the
> old code is nicely returning error. Now, it will print misleading
> message - failed to get regulator - and continue.
>
> While this simplifies the code, it ignores important running condition -
> having regulator enabled.

Would doing this be correct?

ret = devm_regulator_get_enable_optional(&pdev->dev, "vtmu");
switch (ret) {
case 0:
case -ENODEV:
	break;
case -EPROBE_DEFER:
	return -EPROBE_DEFER;
default:
	dev_err(&pdev->dev, "Failed to get enabled regulator: %d\n",
		ret);
	return ret;
}

I understand that we would get -ENODEV in case of the regulator being
unavailable, which we would ignore (this is the "equivalent" of
devm_regulator_get_optional failing in the original code). And in case
of enable failing, we would get some other error, which we would handle.

Thanks for being patient with me by the way, hopefully I will learn quickly :)

Best regards,
Mateusz