linux-kernel - Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <728003b9-db27-fdc0-e761-197a02a38c24@linaro.org>
Date:   Thu, 31 Aug 2023 13:54:37 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Luca Weiss <luca.weiss@...rphone.com>
Cc:     cros-qcom-dts-watchers@...omium.org,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH 04/11] arm64: dts: qcom: pm7250b: make SID configurable

On 31/08/2023 13:33, Dmitry Baryshkov wrote:
> On Thu, 31 Aug 2023 at 13:13, Luca Weiss <luca.weiss@...rphone.com> wrote:
>>
>> On Wed Aug 30, 2023 at 12:06 PM CEST, Krzysztof Kozlowski wrote:
>>> On 30/08/2023 11:58, Luca Weiss wrote:
>>>> Like other Qualcomm PMICs the PM7250B can be used on different addresses
>>>> on the SPMI bus. Use similar defines like the PMK8350 to make this
>>>> possible.
>>>>
>>>> Signed-off-by: Luca Weiss <luca.weiss@...rphone.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/pm7250b.dtsi | 23 ++++++++++++++++-------
>>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> index e8540c36bd99..3514de536baa 100644
>>>> --- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
>>>> @@ -7,6 +7,15 @@
>>>>  #include <dt-bindings/interrupt-controller/irq.h>
>>>>  #include <dt-bindings/spmi/spmi.h>
>>>>
>>>> +/* This PMIC can be configured to be at different SIDs */
>>>> +#ifndef PM7250B_SID
>>>> +   #define PM7250B_SID 2
>>>> +#endif
>>>
>>> Why do you send the same patch as v1, without any reference to previous
>>> discussions?
>>>
>>> You got here feedback already.
>>>
>>> https://lore.kernel.org/linux-arm-msm/f52524da-719b-790f-ad2c-0c3f313d9fe9@linaro.org/
>>
>> Hi Krzysztof,
>>
>> I did mention that original patch in the cover letter of this series.
>> I'm definitely aware of the discussion earlier this year there but also
>> tried to get an update lately if there's any update with no response.
> 
> I think the overall consensus was that my proposal is too complicated
> for the DT files.

I proposed to duplicate the entries. Do you keep QUP nodes in DTSI and
customize per address? No.

I definitely do not agree to these ifndef->define. Maybe using just
define would work (so drop ifndef->define), because this makes it
obvious and fail-safe if included in wrong place... except that it is
still not the define we expect. This is not the coding style present in
other DTSes.

The true problem how these SPMI bindings were created. Requiring SID
address in every child is clearly redundant and I think we do not follow
such approach anywhere else.

Best regards,
Krzysztof