linux-kernel - Re: [PATCH] arm64: defconfig: enable regulator to fix mt8173 regression

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: <03eb0fb9-f2bc-f22d-9c57-2d7d132a30c2@collabora.com>
Date:   Fri, 29 Oct 2021 14:42:51 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Adrian Ratiu <adrian.ratiu@...labora.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>
Cc:     linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kernel@...labora.com, "kernelci.org bot" <bot@...nelci.org>,
        Guillaume Tucker <guillaume.tucker@...labora.com>,
        Enric Balletbo Serra <eballetbo@...il.com>
Subject: Re: [PATCH] arm64: defconfig: enable regulator to fix mt8173
 regression

Il 19/10/21 16:38, AngeloGioacchino Del Regno ha scritto:
> Il 11/10/21 14:53, Adrian Ratiu ha scritto:
>> A regression was introduced on some mediatek boards starting with
>> v5.15-rc1 in commit 109fd20601e2b ("arm64: dts: mediatek: mt8173:
>> Add domain supply for mfg_async") which effectively changed the
>> regulator from the always-on dummy to DA9211 without explicitely
>> enabling it, causing failures like the these caught by KernelCI
>> on Hana Chromebooks [1]:
>>
>> mtk-power-controller 10006000.syscon:power-controller: supply domain not found, 
>> using dummy regulator
>> mtu3 11271000.usb: supply vbus not found, using dummy regulator
>> xhci-mtk 11270000.usb: supply vbus not found, using dummy regulator
>>
>> There might be another bug linking these power domains in the
>> mediatek PM driver, but that is a separate issue wich needs
>> addressing, for now just fix the obvious regression due to the
>> new regulator requirement.
>>
>> [1] https://github.com/kernelci/kernelci-project/issues/66
>> Reported-by: "kernelci.org bot" <bot@...nelci.org>
>> Cc: Guillaume Tucker <guillaume.tucker@...labora.com>
>> Suggested-by: Enric Balletbo Serra <eballetbo@...il.com>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
>> ---
>>   arch/arm64/configs/defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>> index 156d96afbbfc..4901cc1213bb 100644
>> --- a/arch/arm64/configs/defconfig
>> +++ b/arch/arm64/configs/defconfig
>> @@ -616,6 +616,7 @@ CONFIG_REGULATOR_FIXED_VOLTAGE=y
>>   CONFIG_REGULATOR_AXP20X=y
>>   CONFIG_REGULATOR_BD718XX=y
>>   CONFIG_REGULATOR_BD9571MWV=y
>> +CONFIG_REGULATOR_DA9211=y
>>   CONFIG_REGULATOR_FAN53555=y
>>   CONFIG_REGULATOR_GPIO=y
>>   CONFIG_REGULATOR_HI6421V530=y
>>
> 
> Hello,
> 
> I've been able to get a working Hana boot, with USB probed as early as possible, as
> well solving that KernelCI failure (as now also the usb network works fine again).
> 
> My proposal here, for which I have a patch that's almost ready, would be to enable
> this regulator driver as a module instead (since Hana is the only device that's
> using it), avoiding to increase the kernel image size for everyone.
> 
> 
> Before pointing out my solution, let's first point out what's going on:
> 
> In mt8173.dtsi, we have a power-controller node (mediatek,mt8173-power-controller),
> under which all of the SoC's power domain nodes are defined. In this node, we have
> both SCPD_DOMAIN_SUPPLY domains and "regular" ones.
> 
> The difference between SCPD domains and the others is that the first ones require a
> parent regulator, while the latter don't (power is supplied from some .. internal
> supply? - either way, no parent vreg necessary/declared).
> As a note, for now, the only two MediaTek SoCs that have a SCPD supply are MT8173
> and MT8183... and nothing else, as the others, including the newer ones seem to
> have no such supplies (the only newer one upstream is MT8192 and has none).
> 
> 
> My solution was to split the power-controller node in two:
> 1. spm: power-controller@0 - contains all of the non-SCPD power domains
> 2. spm_scpd: power-controller@1 - contains the SCPD power domains.
> 
> This made me able to get a full boot without usb/usb-eth issues while enabling this
> regulator as a module; this also requires us to change the
> mediatek,power-controller.yaml binding to allow multiple instances of that driver,
> which is anyway already permitted by the mtk-pm-domains driver itself.
> 
> 
> Hence, this question comes up: how should we proceed? should we...
> a. enable this regulator driver as module and split the power-controller in two; or
> b. keep this commit enabling this driver built-in and still split the
>     power-controller nodes; or
> c. just enable this driver as built-in and not care about declaring two power
>     controller nodes?
> 
> Can you please give us an advice?
> 
> Thank you,
> - Angelo

After a discussion on this topic, we chose to pursue option B, as enabling this
regulator fixes a very bad regression. Splitting the power-controller nodes does
require a bit of time due to some more research that has to be done on that topic.

Adrian will follow with a v2 of this patch, adding a Fixes tag.

Thanks everyone,
- Angelo