Re: [PATCH v3 08/11] arm64: dts: qcom: pm6150: Add PM6150/PM6150L PMIC peripherals

[kgunda]

On 2019-10-29 22:08, Stephen Boyd wrote:
> Quoting Rajendra Nayak (2019-10-23 02:02:16)
> > diff --git a/arch/arm64/boot/dts/qcom/pm6150.dtsi 
> > b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> > new file mode 100644
> > index 000000000000..20eb928e5ce3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pm6150.dtsi
> > @@ -0,0 +1,85 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +// Copyright (c) 2019, The Linux Foundation. All rights reserved.
> > +
> > +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> > +#include <dt-bindings/input/linux-event-codes.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/spmi/spmi.h>
> > +#include <dt-bindings/thermal/thermal.h>
> > +
> > +&spmi_bus {
> > +       pm6150_lsid0: pmic@0 {
> > +               compatible = "qcom,pm6150", "qcom,spmi-pmic";
> > +               reg = <0x0 SPMI_USID>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               pm6150_pon: pon@800 {
> > +                       compatible = "qcom,pm8998-pon";
> > +                       reg = <0x800>;
> > +                       mode-bootloader = <0x2>;
> > +                       mode-recovery = <0x1>;
>
> Can this have status = "disabled"? Or is the idea that if the pmic 
> power
> button isn't used it should be disabled in the board dts file?
Yes. The idea is to go with latter option. Disable it in the board dts 
file if the
pmic power button is not used.
> > +
> > +                       pwrkey {
> > +                               compatible = "qcom,pm8941-pwrkey";
> > +                               interrupts = <0x0 0x8 0 
> > IRQ_TYPE_EDGE_BOTH>;
> > +                               debounce = <15625>;
> > +                               bias-pull-up;
> > +                               linux,code = <KEY_POWER>;
> > +                       };
> > +               };
> > +
> > +               pm6150_temp: temp-alarm@2400 {
> > +                       compatible = "qcom,spmi-temp-alarm";
> > +                       reg = <0x2400>;
> > +                       interrupts = <0x0 0x24 0x0 
> > IRQ_TYPE_EDGE_RISING>;
> > +                       io-channels = <&pm6150_adc ADC5_DIE_TEMP>;
> > +                       io-channel-names = "thermal";
> > +                       #thermal-sensor-cells = <0>;
> > +               };
> > +
> > +               pm6150_adc: adc@3100 {
> > +                       compatible = "qcom,spmi-adc5";
> > +                       reg = <0x3100>;
> > +                       interrupts = <0x0 0x31 0x0 
> > IRQ_TYPE_EDGE_RISING>;
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +                       #io-channel-cells = <1>;
> > +
> > +                       adc-chan@ADC5_DIE_TEMP {
> > +                               reg = <ADC5_DIE_TEMP>;
> > +                               label = "die_temp";
> > +                       };
> > +               };
> > +
> > +               pm6150_gpio: gpios@c000 {
> > +                       compatible = "qcom,pm6150-gpio", 
> > "qcom,spmi-gpio";
> > +                       reg = <0xc000 0xa00>;
>
> Drop the size?
Will drop it in next series.
> > +                       gpio-controller;
> > +                       #gpio-cells = <2>;
> > +                       interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc1 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc2 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc3 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc4 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc5 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc6 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc7 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc8 0 IRQ_TYPE_NONE>,
> > +                                    <0 0xc9 0 IRQ_TYPE_NONE>;
>
> Isn't this supposed to go away?
Yes. We can remove them if we want to go with the way done for pm8998.
> > +
> > +                       interrupt-names = "pm6150_gpio1", 
> > "pm6150_gpio2",
> > +                                       "pm6150_gpio3", 
> > "pm6150_gpio4",
> > +                                       "pm6150_gpio5", 
> > "pm6150_gpio6",
> > +                                       "pm6150_gpio7", 
> > "pm6150_gpio8",
> > +                                       "pm6150_gpio9", 
> > "pm6150_gpio10";
>
> And this? And have gpio-ranges and use the irqdomain work. Basically,
> should look like pm8998.
Ok.. We can go ahead with the pm8998 way as well. We will address it in 
next series.
> > +               };
> > +       };
> > +
> > +       pm6150_lsid1: pmic@1 {
> > +               compatible = "qcom,pm6150", "qcom,spmi-pmic";
> > +               reg = <0x1 SPMI_USID>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +       };
> > +};