diff mbox series

[V2,1/5] arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325

Message ID 1617268396-1837-2-git-send-email-skakit@codeaurora.org
State Superseded
Headers show
Series Add PMIC DT files for sc7280 | expand

Commit Message

Satya Priya April 1, 2021, 9:13 a.m. UTC 
Add temp-alarm and GPIO support for pm7325.

Signed-off-by: satya priya <skakit@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/pm7325.dtsi | 53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi

Comments

Matthias Kaehlcke April 2, 2021, 5:35 p.m. UTC | #1 
On Thu, Apr 01, 2021 at 02:43:12PM +0530, satya priya wrote:

> subject: arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325

nit: maybe just 'arm64: dts: qcom: Add pm7325 support/.dtsi' or similar?

> Add temp-alarm and GPIO support for pm7325.

nit: it's more than that, you are adding the .dtsi for the PMIC itself.

> Signed-off-by: satya priya <skakit@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/pm7325.dtsi | 53 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> new file mode 100644
> index 0000000..1e0848a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2021, The Linux Foundation. All rights reserved.
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/spmi/spmi.h>
> +
> +&spmi_bus {
> +	pm7325: pmic@1 {
> +		compatible = "qcom,pm7325", "qcom,spmi-pmic";

I saw the patches that add the compatible strings for the GPIOs, but
can't find those that add the strings for the PMICs themselves. Could
you provide a link if they have been sent already?

> +		reg = <0x1 SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pm7325_temp_alarm: temp-alarm@a00 {
> +			compatible = "qcom,spmi-temp-alarm";
> +			reg = <0xa00>;
> +			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
> +			#thermal-sensor-cells = <0>;
> +		};
> +
> +		pm7325_gpios: gpios@8800 {
> +			compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio";
> +			reg = <0x8800>;
> +			gpio-controller;
> +			gpio-ranges = <&pm7325_gpios 0 0 10>;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +};
> +
> +&thermal_zones {
> +	pm7325_thermal: pm7325-thermal {
> +		polling-delay-passive = <100>;
> +		polling-delay = <0>;
> +		thermal-sensors = <&pm7325_temp_alarm>;
> +
> +		trips {
> +			pm7325_trip0: trip0 {
> +				temperature = <95000>;
> +				hysteresis = <0>;
> +				type = "passive";
> +			};
> +
> +			pm7325_trip1: trip1 {

nit: the critical trip point is often named <name>-crit. One reason for
this could be that it allows to add other non-critical trip points (in
the .dtsi itself or the <board>.dts), without messing up the
enumeration scheme.

> +				temperature = <115000>;
> +				hysteresis = <0>;
> +				type = "critical";
> +			};
> +		};
> +	};
> +};
Matthias Kaehlcke April 2, 2021, 8:35 p.m. UTC | #2 
On Fri, Apr 02, 2021 at 10:35:54AM -0700, Matthias Kaehlcke wrote:
> On Thu, Apr 01, 2021 at 02:43:12PM +0530, satya priya wrote:
> 
> > subject: arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325
> 
> nit: maybe just 'arm64: dts: qcom: Add pm7325 support/.dtsi' or similar?
> 
> > Add temp-alarm and GPIO support for pm7325.
> 
> nit: it's more than that, you are adding the .dtsi for the PMIC itself.
> 
> > Signed-off-by: satya priya <skakit@codeaurora.org>
> > ---
> >  arch/arm64/boot/dts/qcom/pm7325.dtsi | 53 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > new file mode 100644
> > index 0000000..1e0848a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
> > @@ -0,0 +1,53 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +// Copyright (c) 2021, The Linux Foundation. All rights reserved.
> > +
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/spmi/spmi.h>
> > +
> > +&spmi_bus {
> > +	pm7325: pmic@1 {
> > +		compatible = "qcom,pm7325", "qcom,spmi-pmic";
> 
> I saw the patches that add the compatible strings for the GPIOs, but
> can't find those that add the strings for the PMICs themselves. Could
> you provide a link if they have been sent already?
> 
> > +		reg = <0x1 SPMI_USID>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		pm7325_temp_alarm: temp-alarm@a00 {
> > +			compatible = "qcom,spmi-temp-alarm";
> > +			reg = <0xa00>;
> > +			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
> > +			#thermal-sensor-cells = <0>;
> > +		};
> > +
> > +		pm7325_gpios: gpios@8800 {
> > +			compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio";
> > +			reg = <0x8800>;
> > +			gpio-controller;
> > +			gpio-ranges = <&pm7325_gpios 0 0 10>;

The GPIO enumeration is a bit confusing. The pm7325 has GPIO_01 to
GPIO_10, however IIUC they are mapped such that under Linux
enumeration starts with 0. I guess it makes sense to start with 0 and
it's done consistently for 'qcom,spmi-gpio', but it's something that must
be taken into account when using/configuring those GPIOs.
Satya Priya April 7, 2021, 3:24 p.m. UTC | #3 
Hi Matthias,

On 2021-04-02 23:05, Matthias Kaehlcke wrote:
> On Thu, Apr 01, 2021 at 02:43:12PM +0530, satya priya wrote:

> 

>> subject: arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325

> 

> nit: maybe just 'arm64: dts: qcom: Add pm7325 support/.dtsi' or 

> similar?

> 

>> Add temp-alarm and GPIO support for pm7325.

> 

> nit: it's more than that, you are adding the .dtsi for the PMIC itself.

> 


Okay, will change the commit text.

>> Signed-off-by: satya priya <skakit@codeaurora.org>

>> ---

>>  arch/arm64/boot/dts/qcom/pm7325.dtsi | 53 

>> ++++++++++++++++++++++++++++++++++++

>>  1 file changed, 53 insertions(+)

>>  create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi

>> 

>> diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi 

>> b/arch/arm64/boot/dts/qcom/pm7325.dtsi

>> new file mode 100644

>> index 0000000..1e0848a

>> --- /dev/null

>> +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi

>> @@ -0,0 +1,53 @@

>> +// SPDX-License-Identifier: BSD-3-Clause

>> +// Copyright (c) 2021, The Linux Foundation. All rights reserved.

>> +

>> +#include <dt-bindings/interrupt-controller/irq.h>

>> +#include <dt-bindings/spmi/spmi.h>

>> +

>> +&spmi_bus {

>> +	pm7325: pmic@1 {

>> +		compatible = "qcom,pm7325", "qcom,spmi-pmic";

> 

> I saw the patches that add the compatible strings for the GPIOs, but

> can't find those that add the strings for the PMICs themselves. Could

> you provide a link if they have been sent already?

> 


They are not sent yet, qcom,spmi-pmic.txt file conversion to yaml [1] is 
currently on hold as all the child node bindings needs to be converted 
to yaml first. Once that is done we can add these strings.

[1] https://lore.kernel.org/patchwork/patch/1359552/

>> +		reg = <0x1 SPMI_USID>;

>> +		#address-cells = <1>;

>> +		#size-cells = <0>;

>> +

>> +		pm7325_temp_alarm: temp-alarm@a00 {

>> +			compatible = "qcom,spmi-temp-alarm";

>> +			reg = <0xa00>;

>> +			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;

>> +			#thermal-sensor-cells = <0>;

>> +		};

>> +

>> +		pm7325_gpios: gpios@8800 {

>> +			compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio";

>> +			reg = <0x8800>;

>> +			gpio-controller;

>> +			gpio-ranges = <&pm7325_gpios 0 0 10>;

>> +			#gpio-cells = <2>;

>> +			interrupt-controller;

>> +			#interrupt-cells = <2>;

>> +		};

>> +	};

>> +};

>> +

>> +&thermal_zones {

>> +	pm7325_thermal: pm7325-thermal {

>> +		polling-delay-passive = <100>;

>> +		polling-delay = <0>;

>> +		thermal-sensors = <&pm7325_temp_alarm>;

>> +

>> +		trips {

>> +			pm7325_trip0: trip0 {

>> +				temperature = <95000>;

>> +				hysteresis = <0>;

>> +				type = "passive";

>> +			};

>> +

>> +			pm7325_trip1: trip1 {

> 

> nit: the critical trip point is often named <name>-crit. One reason for

> this could be that it allows to add other non-critical trip points (in

> the .dtsi itself or the <board>.dts), without messing up the

> enumeration scheme.

> 


ok.

>> +				temperature = <115000>;

>> +				hysteresis = <0>;

>> +				type = "critical";

>> +			};

>> +		};

>> +	};

>> +};
Satya Priya April 7, 2021, 3:29 p.m. UTC | #4 
On 2021-04-03 02:05, Matthias Kaehlcke wrote:
> On Fri, Apr 02, 2021 at 10:35:54AM -0700, Matthias Kaehlcke wrote:
>> On Thu, Apr 01, 2021 at 02:43:12PM +0530, satya priya wrote:
>> 
>> > subject: arm64: dts: qcom: pm7325: Add PMIC peripherals for pm7325
>> 
>> nit: maybe just 'arm64: dts: qcom: Add pm7325 support/.dtsi' or 
>> similar?
>> 
>> > Add temp-alarm and GPIO support for pm7325.
>> 
>> nit: it's more than that, you are adding the .dtsi for the PMIC 
>> itself.
>> 
>> > Signed-off-by: satya priya <skakit@codeaurora.org>
>> > ---
>> >  arch/arm64/boot/dts/qcom/pm7325.dtsi | 53 ++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 53 insertions(+)
>> >  create mode 100644 arch/arm64/boot/dts/qcom/pm7325.dtsi
>> >
>> > diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> > new file mode 100644
>> > index 0000000..1e0848a
>> > --- /dev/null
>> > +++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
>> > @@ -0,0 +1,53 @@
>> > +// SPDX-License-Identifier: BSD-3-Clause
>> > +// Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> > +
>> > +#include <dt-bindings/interrupt-controller/irq.h>
>> > +#include <dt-bindings/spmi/spmi.h>
>> > +
>> > +&spmi_bus {
>> > +	pm7325: pmic@1 {
>> > +		compatible = "qcom,pm7325", "qcom,spmi-pmic";
>> 
>> I saw the patches that add the compatible strings for the GPIOs, but
>> can't find those that add the strings for the PMICs themselves. Could
>> you provide a link if they have been sent already?
>> 
>> > +		reg = <0x1 SPMI_USID>;
>> > +		#address-cells = <1>;
>> > +		#size-cells = <0>;
>> > +
>> > +		pm7325_temp_alarm: temp-alarm@a00 {
>> > +			compatible = "qcom,spmi-temp-alarm";
>> > +			reg = <0xa00>;
>> > +			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
>> > +			#thermal-sensor-cells = <0>;
>> > +		};
>> > +
>> > +		pm7325_gpios: gpios@8800 {
>> > +			compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio";
>> > +			reg = <0x8800>;
>> > +			gpio-controller;
>> > +			gpio-ranges = <&pm7325_gpios 0 0 10>;
> 
> The GPIO enumeration is a bit confusing. The pm7325 has GPIO_01 to
> GPIO_10, however IIUC they are mapped such that under Linux
> enumeration starts with 0. I guess it makes sense to start with 0 and
> it's done consistently for 'qcom,spmi-gpio', but it's something that 
> must
> be taken into account when using/configuring those GPIOs.

Sure, will take care of this while configuring the GPIOs.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/pm7325.dtsi b/arch/arm64/boot/dts/qcom/pm7325.dtsi
new file mode 100644
index 0000000..1e0848a
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm7325.dtsi
@@ -0,0 +1,53 @@ 
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2021, The Linux Foundation. All rights reserved.
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/spmi/spmi.h>
+
+&spmi_bus {
+	pm7325: pmic@1 {
+		compatible = "qcom,pm7325", "qcom,spmi-pmic";
+		reg = <0x1 SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pm7325_temp_alarm: temp-alarm@a00 {
+			compatible = "qcom,spmi-temp-alarm";
+			reg = <0xa00>;
+			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+			#thermal-sensor-cells = <0>;
+		};
+
+		pm7325_gpios: gpios@8800 {
+			compatible = "qcom,pm7325-gpio", "qcom,spmi-gpio";
+			reg = <0x8800>;
+			gpio-controller;
+			gpio-ranges = <&pm7325_gpios 0 0 10>;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+};
+
+&thermal_zones {
+	pm7325_thermal: pm7325-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+		thermal-sensors = <&pm7325_temp_alarm>;
+
+		trips {
+			pm7325_trip0: trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			pm7325_trip1: trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "critical";
+			};
+		};
+	};
+};