Re: [PATCH v3 2/2] arm64: dts: qcom: Add configuration for PMI8950 peripheral
From: Luca Weiss
Date: Sun Dec 04 2022 - 11:19:12 EST
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@xxxxxxxxx>
> > > >
> > > > The PMI8950 features integrated peripherals like ADC, GPIO controller,
> > > > MPPs and others.
> > > >
> > > > [luca@xxxxxxxxx: remove pm8950, style changes for 2022 standards, add
> > > > wled]
> > > > Signed-off-by: AngeloGioacchino Del Regno <kholk11@xxxxxxxxx>
> > > > Signed-off-by: Luca Weiss <luca@xxxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxxx>
> > > > +
> > > > +#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@3100 {
> > > > + 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@xxxxxxxxxxxxxx/>
> > 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@xxxxxxxxxxxxxx/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@xxxxxxxxxxxxxx/ [3]:
> https://lore.kernel.org/linux-arm-msm/20221111120156.48040-10-angelogioacch
> ino.delregno@xxxxxxxxxxxxx/