linux-kernel - Re: [PATCH v3 2/2] arm64: dts: qcom: Add configuration for PMI8950 peripheral

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: <2656622.mvXUDI8C0e@g550jk>
Date:   Sun, 04 Dec 2022 17:19:05 +0100
From:   Luca Weiss <luca@...tu.xyz>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        Luca Weiss <luca@...tu.xyz>, linux-arm-msm@...r.kernel.org,
        ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
        AngeloGioacchino Del Regno <kholk11@...il.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jonathan Cameron <jic23@...nel.org>
Subject: Re: [PATCH v3 2/2] arm64: dts: qcom: Add configuration for PMI8950 peripheral

On Freitag, 2. Dezember 2022 10:36:58 CET Marijn Suijten wrote:
> On 2022-11-06 20:37:24, Marijn Suijten wrote:
> > On 2022-11-05 00:44:37, Marijn Suijten wrote:
> > > On 2022-11-01 17:18:00, Luca Weiss wrote:
> > > > From: AngeloGioacchino Del Regno <kholk11@...il.com>
> > > > 
> > > > The PMI8950 features integrated peripherals like ADC, GPIO controller,
> > > > MPPs and others.
> > > > 
> > > > [luca@...tu.xyz: remove pm8950, style changes for 2022 standards, add
> > > > wled]
> > > > Signed-off-by: AngeloGioacchino Del Regno <kholk11@...il.com>
> > > > Signed-off-by: Luca Weiss <luca@...tu.xyz>
> > > > ---
> > > > Changes since v2:
> > > > * Pick up patch, and adjust as mentioned above sign-offs
> > > > 
> > > >  arch/arm64/boot/dts/qcom/pmi8950.dtsi | 97
> > > >  +++++++++++++++++++++++++++
> > > >  1 file changed, 97 insertions(+)
> > > >  create mode 100644 arch/arm64/boot/dts/qcom/pmi8950.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/pmi8950.dtsi
> > > > b/arch/arm64/boot/dts/qcom/pmi8950.dtsi new file mode 100644
> > > > index 000000000000..32d27e2187e3
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/qcom/pmi8950.dtsi
> > > > @@ -0,0 +1,97 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +// Copyright (c) 2019, AngeloGioacchino Del Regno <kholk11@...il.com>
> > > > +
> > > > +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> > > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > > +#include <dt-bindings/spmi/spmi.h>
> > > > +
> > > > +&spmi_bus {
> > > > +	pmic@2 {
> > > > +		compatible = "qcom,pmi8950", "qcom,spmi-pmic";
> > > > +		reg = <0x2 SPMI_USID>;
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		pmi8950_vadc: adc@...0 {
> > > > +			compatible = "qcom,spmi-vadc";
> > > > +			reg = <0x3100>;
> > > > +			interrupts = <0x2 0x31 0x0 
IRQ_TYPE_EDGE_RISING>;
> > > > +			#address-cells = <1>;
> > > > +			#size-cells = <0>;
> > > > +			#io-channel-cells = <1>;
> > > > +
> > > > +			adc-chan@0 {
> > > > +				reg = <VADC_USBIN>;
> > > > +				qcom,pre-scaling = <1 4>;
> > > > +				label = "usbin";
> > > 
> > > I've previously sent a patch with labels in the node name instead [1],
> > > what's the preferred way nowadays?
> > > 
> > > [1]:
> > > https://lore.kernel.org/linux-arm-msm/20220926190148.283805-4-marijn.su
> > > ijten@...ainline.org/> 
> > As it turns out that patch relied on the ADC5 driver propagating the DT
> > node name (and label name if set) to IIO, which doesn't happen for the
> > legacy VADC driver used here.  I sent an RFC to that effect, with a
> > large discussion whether or not we should use node names, labels, or
> > rely on hardcoded names in the drivers entirely.  The recent migration
> > to fwnode made the node name include the `@xx` suffix which makes for
> > unpleasant reading in sysfs, so that's at least one reason to have
> > generic node names *and skip node names in these drivers altogether*.
> > 
> >     https://lore.kernel.org/linux-arm-msm/20221106193018.270106-1-marijn.s
> >     uijten@...ainline.org/T/#u> 
> > In short: we may want to hold off changing these patches until a
> > clear-cut decision has been made (but I think your patch here is the
> > right approach in the end: generic node name *with label*, when the
> > label is more clear than the name hardcoded in the driver).
> 
> We came to the conclusion in [1] that using labels is the way to go so
> that the name doesn't get all mangled, then we can opt for generic node
> names here as well.  This does mean I'll have to send a followup for [2]
> and have to revise [3] again to use generic node names and labels.

So the way this patch does it is good or does it need changes?

> 
> - Marijn
> 
> [1]:
> https://lore.kernel.org/linux-arm-msm/20221112162719.0ac87998@jic23-huawei/
> [2]:
> https://lore.kernel.org/linux-arm-msm/20220926190148.283805-4-marijn.suijte
> n@...ainline.org/ [3]:
> https://lore.kernel.org/linux-arm-msm/20221111120156.48040-10-angelogioacch
> ino.delregno@...labora.com/