Message-ID: <ee1ebac4-bf18-019a-f770-5cb88703d06b@linaro.org>
Date:   Tue, 14 Mar 2023 12:41:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 3/3] arm64: dts: qcom: Split out SA8155P and use correct
 RPMh power domains
Content-Language: en-US
From:   Konrad Dybcio <konrad.dybcio@linaro.org>
To:     Bjorn Andersson <andersson@kernel.org>
Cc:     linux-arm-msm@vger.kernel.org, agross@kernel.org,
        krzysztof.kozlowski@linaro.org, marijn.suijten@somainline.org,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230214095435.2192153-1-konrad.dybcio@linaro.org>
 <20230214095435.2192153-3-konrad.dybcio@linaro.org>
 <20230314001052.7qvgbwkl73x22oll@ripper>
 <eaf2ca0d-4d90-b68b-3b36-8bb0148cfb95@linaro.org>
In-Reply-To: <eaf2ca0d-4d90-b68b-3b36-8bb0148cfb95@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 14.03.2023 12:36, Konrad Dybcio wrote:
> 
> 
> On 14.03.2023 01:10, Bjorn Andersson wrote:
>> On Tue, Feb 14, 2023 at 10:54:35AM +0100, Konrad Dybcio wrote:
>>> The RPMhPD setup on SA8155P is different compared to SM8150. Correct
>>> it to ensure the platform will not try accessing forbidden/missing
>>> RPMh entries at boot, as a bad vote will hang the machine.
>>>
>>
>> I don't see that this will scale, as soon as someone adds a new device
>> in sm8150.dtsi that has the need to scale a power rail this will be
>> forgotten and we will have a mix of references to the SM8150 and SA8155P
>> value space.
>>
>> That said, I think it's reasonable to avoid duplicating the entire
>> sm8150.dtsi.
> Yeah, this problem has no obvious good solutions and even though it's
> not very elegant, this seems to be the less bad one..
> 
>>
>> How about making the SA8155P_* macros match the SM8150_* macros?
>> That way things will fail gracefully if a device node references a
>> resource not defined for either platform...
> Okay, let's do that
Re-thinking it, it's good that the indices don't match, as this way the
board will (should) refuse to function properly if there's an oversight,
which may have gone unnoticed if they were matching, so this only guards
us against programmer error which is not great :/

Konrad
> 
> Konrad
>>
>> Regards,
>> Bjorn
>>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sa8155p-adp.dts |  2 +-
>>>  arch/arm64/boot/dts/qcom/sa8155p.dtsi    | 51 ++++++++++++++++++++++++
>>>  2 files changed, 52 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/arm64/boot/dts/qcom/sa8155p.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>>> index 459384ec8f23..9454e8e4e517 100644
>>> --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>>> @@ -7,7 +7,7 @@
>>>  
>>>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>>>  #include <dt-bindings/gpio/gpio.h>
>>> -#include "sm8150.dtsi"
>>> +#include "sa8155p.dtsi"
>>>  #include "pmm8155au_1.dtsi"
>>>  #include "pmm8155au_2.dtsi"
>>>  
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8155p.dtsi b/arch/arm64/boot/dts/qcom/sa8155p.dtsi
>>> new file mode 100644
>>> index 000000000000..f2fd7c28764e
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/sa8155p.dtsi
>>> @@ -0,0 +1,51 @@
>>> +// SPDX-License-Identifier: BSD-3-Clause
>>> +/*
>>> + * Copyright (c) 2023, Linaro Limited
>>> + *
>>> + * SA8155P is an automotive variant of SM8150, with some minor changes.
>>> + * Most notably, the RPMhPD setup differs: MMCX and LCX/LMX rails are gone.
>>> + */
>>> +
>>> +#include "sm8150.dtsi"
>>> +
>>> +&dispcc {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&mdss_mdp {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&mdss_dsi0 {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&mdss_dsi1 {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&remoteproc_adsp {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&remoteproc_cdsp {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> +
>>> +&remoteproc_mpss {
>>> +	power-domains = <&rpmhpd SA8155P_CX>,
>>> +			<&rpmhpd SA8155P_MSS>;
>>> +};
>>> +
>>> +&remoteproc_slpi {
>>> +	power-domains = <&rpmhpd SA8155P_CX>,
>>> +			<&rpmhpd SA8155P_MX>;
>>> +};
>>> +
>>> +&rpmhpd {
>>> +	compatible = "qcom,sa8155p-rpmhpd";
>>> +};
>>> +
>>> +&sdhc_2 {
>>> +	power-domains = <&rpmhpd SA8155P_CX>;
>>> +};
>>> -- 
>>> 2.39.1
>>>