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: <20221106193722.j64xrhitdencrjxy@SoMainline.org>
Date:   Sun, 6 Nov 2022 20:37:22 +0100
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     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
Subject: Re: [PATCH v3 2/2] arm64: dts: qcom: Add configuration for PMI8950
 peripheral

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.suijten@somainline.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.suijten@somainline.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).

- Marijn