Re: [PATCH 4/9] regulator: mt6358: Drop *_SSHUB regulators

AngeloGioacchino Del Regno
2023-06-14T07:39:36Z

Il 12/06/23 06:45, Chen-Yu Tsai ha scritto:
> On Fri, Jun 9, 2023 at 5:03 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 09/06/23 10:30, Chen-Yu Tsai ha scritto:
>>> The *_SSHUB regulators are actually alternate configuration interfaces
>>> for their non *_SSHUB counterparts. They are not separate regulator
>>> outputs. These registers are intended for the companion processor to
>>> use to configure the power rails while the main processor is sleeping.
>>> They are not intended for the main operating system to use.
>>>
>>> Since they are not real outputs they shouldn't be modeled separately.
>>> Remove them. Luckily no device tree actually uses them.
>>>
>>
>> I'm not sure that MT6358/6366 are used only on Chromebook SoCs, and that this SSHUB
>> mechanism (companion processor) is the same across all firmwares.
>
> AFAICT from Internet sources there's also the MT6771 and MT6781, which
> are used on some phones.
>
> But what part are you concerned about? The upstream regulator driver does
> not actually have any code to switch to/from normal operation and SSHUB
> mode.
>
> In a downstream kernel I found that the SSHUB mode is only used if SCP is
> doing DVFS [1]. In that same kernel, the regulator driver [2] doesn't even
> list the *_SSHUB versions. So if SCP DVFS is used, the regulator driver
> has no idea what's going on, and can't interfere either, which I think is
> actually a good thing. Only one side should have complete control of one
> output.
>

Ok, I'm sold! :-P

Jokes apart, thanks for clarifying. At this point, I agree with you in that this
is safe to do, so:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

P.S.: Sorry for the late reply and thank you for the links to that old
downstream kernel.

Cheers,
Angelo

> ChenYu
>
> [1] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/scp/mt6771/scp_dvfs.c
> [2] https://github.com/nokia-dev/android_kernel_nokia_mt6771/blob/android-9.0/drivers/misc/mediatek/pmic/mt6358/v1/regulator_codegen.c
>
>> I'd like someone from MediaTek to confirm that this is valid for both Chromebook
>> and Smartphone firmwares.
>>
>> Regards,
>> Angelo
>>
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>> ---
>>> drivers/regulator/mt6358-regulator.c | 14 --------------
>>> include/linux/regulator/mt6358-regulator.h | 4 ----
>>> 2 files changed, 18 deletions(-)
>>>
>>> diff --git a/drivers/regulator/mt6358-regulator.c b/drivers/regulator/mt6358-regulator.c
>>> index faf6b0757019..946a251a8b3a 100644
>>> --- a/drivers/regulator/mt6358-regulator.c
>>> +++ b/drivers/regulator/mt6358-regulator.c
>>> @@ -505,9 +505,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
>>> MT6358_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
>>> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
>>> MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> - MT6358_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
>>> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
>>> - MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> MT6358_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
>>> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
>>> MT6358_VPA_ANA_CON0, 3),
>>> @@ -583,10 +580,6 @@ static struct mt6358_regulator_info mt6358_regulators[] = {
>>> MT6358_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON2, 0x7f),
>>> - MT6358_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
>>> - 1293750, 6250, buck_volt_range1,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
>>> MT6358_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON3, 0x7f),
>>> @@ -603,9 +596,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
>>> MT6366_BUCK("buck_vcore", VCORE, 500000, 1293750, 6250,
>>> buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_DBG0, 0x7f,
>>> MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> - MT6366_BUCK("buck_vcore_sshub", VCORE_SSHUB, 500000, 1293750, 6250,
>>> - buck_volt_range1, 0x7f, MT6358_BUCK_VCORE_SSHUB_ELR0, 0x7f,
>>> - MT6358_VCORE_VGPU_ANA_CON0, 1),
>>> MT6366_BUCK("buck_vpa", VPA, 500000, 3650000, 50000,
>>> buck_volt_range3, 0x3f, MT6358_BUCK_VPA_DBG0, 0x3f,
>>> MT6358_VPA_ANA_CON0, 3),
>>> @@ -670,10 +660,6 @@ static struct mt6358_regulator_info mt6366_regulators[] = {
>>> MT6366_LDO1("ldo_vsram_others", VSRAM_OTHERS, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_OTHERS_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON2, 0x7f),
>>> - MT6366_LDO1("ldo_vsram_others_sshub", VSRAM_OTHERS_SSHUB, 500000,
>>> - 1293750, 6250, buck_volt_range1,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f,
>>> - MT6358_LDO_VSRAM_OTHERS_SSHUB_CON1, 0x7f),
>>> MT6366_LDO1("ldo_vsram_gpu", VSRAM_GPU, 500000, 1293750, 6250,
>>> buck_volt_range1, MT6358_LDO_VSRAM_GPU_DBG0, 0x7f00,
>>> MT6358_LDO_VSRAM_CON3, 0x7f),
>>> diff --git a/include/linux/regulator/mt6358-regulator.h b/include/linux/regulator/mt6358-regulator.h
>>> index a4307cd9edd6..c71a6a9fce7a 100644
>>> --- a/include/linux/regulator/mt6358-regulator.h
>>> +++ b/include/linux/regulator/mt6358-regulator.h
>>> @@ -47,8 +47,6 @@ enum {
>>> MT6358_ID_VLDO28,
>>> MT6358_ID_VAUD28,
>>> MT6358_ID_VSIM2,
>>> - MT6358_ID_VCORE_SSHUB,
>>> - MT6358_ID_VSRAM_OTHERS_SSHUB,
>>> MT6358_ID_RG_MAX,
>>> };
>>>
>>> @@ -88,8 +86,6 @@ enum {
>>> MT6366_ID_VMC,
>>> MT6366_ID_VAUD28,
>>> MT6366_ID_VSIM2,
>>> - MT6366_ID_VCORE_SSHUB,
>>> - MT6366_ID_VSRAM_OTHERS_SSHUB,
>>> MT6366_ID_RG_MAX,
>>> };
>>>
>>