arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi | 56 +++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/qcs615-ride.dts | 1 + arch/arm64/boot/dts/qcom/qcs615.dtsi | 23 +++++++++++++ 3 files changed, 80 insertions(+)
Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher-
als in the PMIC that controlled by SPMI bus. The peripherals include
GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power
key and volume down key are controlled by PMIC PON hardware on QCS615.
Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com>
---
This patch depends on the patch series:
- https://lore.kernel.org/all/20240926-add_initial_support_for_qcs615-v3-0-e37617e91c62@quicinc.com/
---
arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi | 56 +++++++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/qcs615-ride.dts | 1 +
arch/arm64/boot/dts/qcom/qcs615.dtsi | 23 +++++++++++++
3 files changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..d5e98503bb32aa6c75f9a9c613d562c6140ddf14
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/spmi/spmi.h>
+
+&spmi_bus {
+ pmm6155au_0: pmic@0 {
+ compatible = "qcom,spmi-pmic";
+ reg = <0x0 SPMI_USID>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmm6155au_0_pon: pon@800 {
+ compatible = "qcom,pm8998-pon";
+ reg = <0x800>;
+
+ pon_pwrkey: pwrkey {
+ compatible = "qcom,pm8941-pwrkey";
+ interrupts = <0x0 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
+ debounce = <15625>;
+ bias-pull-up;
+ linux,code = <KEY_POWER>;
+ };
+
+ pon_resin: resin {
+ compatible = "qcom,pm8941-resin";
+ interrupts = <0x0 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
+ debounce = <15625>;
+ bias-pull-up;
+ linux,code = <KEY_VOLUMEDOWN>;
+ };
+ };
+
+ pmm6155au_0_rtc: rtc@6100 {
+ compatible = "qcom,pm8941-rtc";
+ reg = <0x6100>, <0x6200>;
+ reg-names = "rtc", "alarm";
+ interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
+ allow-set-time;
+ };
+
+ pmm6155au_0_gpios: gpio@c000 {
+ compatible = "qcom,pm8150-gpio";
+ reg = <0xc000>;
+ gpio-controller;
+ gpio-ranges = <&pmm6155au_0_gpios 0 0 12>;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ };
+};
+
diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
index 4ef969a6af150933c72a7a83374a5a2657eebc1b..b79c22730920e3097425e1d1933e744205b3c18e 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -6,6 +6,7 @@
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include "qcs615.dtsi"
+#include "qcs615-pmic.dtsi"
/ {
model = "Qualcomm Technologies, Inc. QCS615 Ride";
compatible = "qcom,qcs615-ride", "qcom,qcs615";
diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
index ac4c4c751da1fbb28865877555ba317677bc6bd2..9793161db515a2ef1df6465c8d0a04a11e71ffc1 100644
--- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
@@ -517,6 +517,29 @@ sram@c3f0000 {
reg = <0x0 0x0c3f0000 0x0 0x400>;
};
+ spmi_bus: qcom,spmi@c440000 {
+ compatible = "qcom,spmi-pmic-arb";
+ reg = <0x0 0xc440000 0x0 0x1100>,
+ <0x0 0xc600000 0x0 0x2000000>,
+ <0x0 0xe600000 0x0 0x100000>,
+ <0x0 0xe700000 0x0 0xa0000>,
+ <0x0 0xc40a000 0x0 0x26000>;
+ reg-names = "core",
+ "chnls",
+ "obsrvr",
+ "intr",
+ "cnfg";
+ interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "periph_irq";
+ interrupt-controller;
+ #interrupt-cells = <4>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+ cell-index = <0>;
+ qcom,channel = <0>;
+ qcom,ee = <0>;
+ };
+
intc: interrupt-controller@17a00000 {
compatible = "arm,gic-v3";
reg = <0x0 0x17a00000 0x0 0x10000>, /* GICD */
---
base-commit: c83f0b825741bcb9d8a7be67c63f6b9045d30f5a
change-id: 20240929-adds-spmi-pmic-peripherals-for-qcs615-16ee53179a7d
prerequisite-change-id: 20240924-add_initial_support_for_qcs615-a01bb2dd4650:v3
prerequisite-patch-id: 09782474af7eecf1013425fd34f9d2f082fb3616
prerequisite-patch-id: 624720e543d7857e46d3ee49b8cea413772deb4c
prerequisite-patch-id: 04ca722967256efddc402b7bab94136a5174b0b9
prerequisite-patch-id: ab88a42ec69ad90e8509c9c5b7c6bdd595a7f783
prerequisite-patch-id: 918724fafe43acaa4c4b980bfabe36e9c3212cd1
prerequisite-patch-id: 91cb230c6d129ff21c24d124fad9e37a66cb6a22
prerequisite-patch-id: 57afeee80c9aa069ee243f5a5b634702867d20f1
Best regards,
--
Tingguo Cheng <quic_tingguoc@quicinc.com>On Mon, Oct 14, 2024 at 06:08:17PM +0800, Tingguo Cheng wrote:
> Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher-
> als in the PMIC that controlled by SPMI bus. The peripherals include
> GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power
> key and volume down key are controlled by PMIC PON hardware on QCS615.
Why do you need a separate file? Unless there is a good reason such
configuration should be split between the pmic.dtsi and the board file.
BTW, what is the PMIC id for pmm6155au? Is it a real PMIC or a version
of some other PMIC?
>
> Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com>
> ---
> This patch depends on the patch series:
> - https://lore.kernel.org/all/20240926-add_initial_support_for_qcs615-v3-0-e37617e91c62@quicinc.com/
> ---
> arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi | 56 +++++++++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/qcs615-ride.dts | 1 +
> arch/arm64/boot/dts/qcom/qcs615.dtsi | 23 +++++++++++++
> 3 files changed, 80 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..d5e98503bb32aa6c75f9a9c613d562c6140ddf14
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/spmi/spmi.h>
> +
> +&spmi_bus {
> + pmm6155au_0: pmic@0 {
> + compatible = "qcom,spmi-pmic";
> + reg = <0x0 SPMI_USID>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmm6155au_0_pon: pon@800 {
> + compatible = "qcom,pm8998-pon";
> + reg = <0x800>;
> +
> + pon_pwrkey: pwrkey {
> + compatible = "qcom,pm8941-pwrkey";
> + interrupts = <0x0 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
> + debounce = <15625>;
> + bias-pull-up;
> + linux,code = <KEY_POWER>;
> + };
> +
> + pon_resin: resin {
> + compatible = "qcom,pm8941-resin";
> + interrupts = <0x0 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
> + debounce = <15625>;
> + bias-pull-up;
> + linux,code = <KEY_VOLUMEDOWN>;
> + };
> + };
> +
> + pmm6155au_0_rtc: rtc@6100 {
> + compatible = "qcom,pm8941-rtc";
> + reg = <0x6100>, <0x6200>;
> + reg-names = "rtc", "alarm";
> + interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
> + allow-set-time;
> + };
> +
> + pmm6155au_0_gpios: gpio@c000 {
> + compatible = "qcom,pm8150-gpio";
> + reg = <0xc000>;
> + gpio-controller;
> + gpio-ranges = <&pmm6155au_0_gpios 0 0 12>;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> + };
> +};
> +
> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> index 4ef969a6af150933c72a7a83374a5a2657eebc1b..b79c22730920e3097425e1d1933e744205b3c18e 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> @@ -6,6 +6,7 @@
>
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> #include "qcs615.dtsi"
> +#include "qcs615-pmic.dtsi"
> / {
> model = "Qualcomm Technologies, Inc. QCS615 Ride";
> compatible = "qcom,qcs615-ride", "qcom,qcs615";
> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> index ac4c4c751da1fbb28865877555ba317677bc6bd2..9793161db515a2ef1df6465c8d0a04a11e71ffc1 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> @@ -517,6 +517,29 @@ sram@c3f0000 {
> reg = <0x0 0x0c3f0000 0x0 0x400>;
> };
>
> + spmi_bus: qcom,spmi@c440000 {
> + compatible = "qcom,spmi-pmic-arb";
> + reg = <0x0 0xc440000 0x0 0x1100>,
> + <0x0 0xc600000 0x0 0x2000000>,
> + <0x0 0xe600000 0x0 0x100000>,
> + <0x0 0xe700000 0x0 0xa0000>,
> + <0x0 0xc40a000 0x0 0x26000>;
> + reg-names = "core",
> + "chnls",
> + "obsrvr",
> + "intr",
> + "cnfg";
> + interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "periph_irq";
> + interrupt-controller;
> + #interrupt-cells = <4>;
> + #address-cells = <2>;
> + #size-cells = <0>;
> + cell-index = <0>;
> + qcom,channel = <0>;
> + qcom,ee = <0>;
> + };
> +
> intc: interrupt-controller@17a00000 {
> compatible = "arm,gic-v3";
> reg = <0x0 0x17a00000 0x0 0x10000>, /* GICD */
>
> ---
> base-commit: c83f0b825741bcb9d8a7be67c63f6b9045d30f5a
> change-id: 20240929-adds-spmi-pmic-peripherals-for-qcs615-16ee53179a7d
> prerequisite-change-id: 20240924-add_initial_support_for_qcs615-a01bb2dd4650:v3
> prerequisite-patch-id: 09782474af7eecf1013425fd34f9d2f082fb3616
> prerequisite-patch-id: 624720e543d7857e46d3ee49b8cea413772deb4c
> prerequisite-patch-id: 04ca722967256efddc402b7bab94136a5174b0b9
> prerequisite-patch-id: ab88a42ec69ad90e8509c9c5b7c6bdd595a7f783
> prerequisite-patch-id: 918724fafe43acaa4c4b980bfabe36e9c3212cd1
> prerequisite-patch-id: 91cb230c6d129ff21c24d124fad9e37a66cb6a22
> prerequisite-patch-id: 57afeee80c9aa069ee243f5a5b634702867d20f1
>
> Best regards,
> --
> Tingguo Cheng <quic_tingguoc@quicinc.com>
>
--
With best wishes
Dmitry
On 10/14/2024 6:53 PM, Dmitry Baryshkov wrote:
> On Mon, Oct 14, 2024 at 06:08:17PM +0800, Tingguo Cheng wrote:
>> Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher-
>> als in the PMIC that controlled by SPMI bus. The peripherals include
>> GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power
>> key and volume down key are controlled by PMIC PON hardware on QCS615.
>
> Why do you need a separate file? Unless there is a good reason such
> configuration should be split between the pmic.dtsi and the board file.
>
One reason is that I use upstream as
template(x1ee80100,sc8180x,sa8775p...), they all have
platform-pmics.dtsi alongside.
On the other hand, qcs615-pmic.dtsi contains only the PMIC's
peripherals, creating a new file can eliminate duplicate code if there
are any other new boards with pmm6155au as their PMIC(similar to
x1e80100-lenovo-xxx/x1e80100-microsoft-xxxx, where "x1e80100-pmics.dtsi"
has been included).
> BTW, what is the PMIC id for pmm6155au? Is it a real PMIC or a version
> of some other PMIC?
>
pmm6155au is the PMIC id(Silk scree). It's a real PMIC for qcs615.Maybe
I should use the name qcs615-pmics.dtsi instead of qcs6150-pmic.dtsi to
align with other platforms.
>>
>> Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com>
>> ---
>> This patch depends on the patch series:
>> - https://lore.kernel.org/all/20240926-add_initial_support_for_qcs615-v3-0-e37617e91c62@quicinc.com/
>> ---
>> arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi | 56 +++++++++++++++++++++++++++++++
>> arch/arm64/boot/dts/qcom/qcs615-ride.dts | 1 +
>> arch/arm64/boot/dts/qcom/qcs615.dtsi | 23 +++++++++++++
>> 3 files changed, 80 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..d5e98503bb32aa6c75f9a9c613d562c6140ddf14
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/qcs615-pmic.dtsi
>> @@ -0,0 +1,56 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/spmi/spmi.h>
>> +
>> +&spmi_bus {
>> + pmm6155au_0: pmic@0 {
>> + compatible = "qcom,spmi-pmic";
>> + reg = <0x0 SPMI_USID>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + pmm6155au_0_pon: pon@800 {
>> + compatible = "qcom,pm8998-pon";
>> + reg = <0x800>;
>> +
>> + pon_pwrkey: pwrkey {
>> + compatible = "qcom,pm8941-pwrkey";
>> + interrupts = <0x0 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
>> + debounce = <15625>;
>> + bias-pull-up;
>> + linux,code = <KEY_POWER>;
>> + };
>> +
>> + pon_resin: resin {
>> + compatible = "qcom,pm8941-resin";
>> + interrupts = <0x0 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
>> + debounce = <15625>;
>> + bias-pull-up;
>> + linux,code = <KEY_VOLUMEDOWN>;
>> + };
>> + };
>> +
>> + pmm6155au_0_rtc: rtc@6100 {
>> + compatible = "qcom,pm8941-rtc";
>> + reg = <0x6100>, <0x6200>;
>> + reg-names = "rtc", "alarm";
>> + interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
>> + allow-set-time;
>> + };
>> +
>> + pmm6155au_0_gpios: gpio@c000 {
>> + compatible = "qcom,pm8150-gpio";
>> + reg = <0xc000>;
>> + gpio-controller;
>> + gpio-ranges = <&pmm6155au_0_gpios 0 0 12>;
>> + #gpio-cells = <2>;
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> + };
>> +};
>> +
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> index 4ef969a6af150933c72a7a83374a5a2657eebc1b..b79c22730920e3097425e1d1933e744205b3c18e 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> @@ -6,6 +6,7 @@
>>
>> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> #include "qcs615.dtsi"
>> +#include "qcs615-pmic.dtsi"
>> / {
>> model = "Qualcomm Technologies, Inc. QCS615 Ride";
>> compatible = "qcom,qcs615-ride", "qcom,qcs615";
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> index ac4c4c751da1fbb28865877555ba317677bc6bd2..9793161db515a2ef1df6465c8d0a04a11e71ffc1 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> @@ -517,6 +517,29 @@ sram@c3f0000 {
>> reg = <0x0 0x0c3f0000 0x0 0x400>;
>> };
>>
>> + spmi_bus: qcom,spmi@c440000 {
>> + compatible = "qcom,spmi-pmic-arb";
>> + reg = <0x0 0xc440000 0x0 0x1100>,
>> + <0x0 0xc600000 0x0 0x2000000>,
>> + <0x0 0xe600000 0x0 0x100000>,
>> + <0x0 0xe700000 0x0 0xa0000>,
>> + <0x0 0xc40a000 0x0 0x26000>;
>> + reg-names = "core",
>> + "chnls",
>> + "obsrvr",
>> + "intr",
>> + "cnfg";
>> + interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "periph_irq";
>> + interrupt-controller;
>> + #interrupt-cells = <4>;
>> + #address-cells = <2>;
>> + #size-cells = <0>;
>> + cell-index = <0>;
>> + qcom,channel = <0>;
>> + qcom,ee = <0>;
>> + };
>> +
>> intc: interrupt-controller@17a00000 {
>> compatible = "arm,gic-v3";
>> reg = <0x0 0x17a00000 0x0 0x10000>, /* GICD */
>>
>> ---
>> base-commit: c83f0b825741bcb9d8a7be67c63f6b9045d30f5a
>> change-id: 20240929-adds-spmi-pmic-peripherals-for-qcs615-16ee53179a7d
>> prerequisite-change-id: 20240924-add_initial_support_for_qcs615-a01bb2dd4650:v3
>> prerequisite-patch-id: 09782474af7eecf1013425fd34f9d2f082fb3616
>> prerequisite-patch-id: 624720e543d7857e46d3ee49b8cea413772deb4c
>> prerequisite-patch-id: 04ca722967256efddc402b7bab94136a5174b0b9
>> prerequisite-patch-id: ab88a42ec69ad90e8509c9c5b7c6bdd595a7f783
>> prerequisite-patch-id: 918724fafe43acaa4c4b980bfabe36e9c3212cd1
>> prerequisite-patch-id: 91cb230c6d129ff21c24d124fad9e37a66cb6a22
>> prerequisite-patch-id: 57afeee80c9aa069ee243f5a5b634702867d20f1
>>
>> Best regards,
>> --
>> Tingguo Cheng <quic_tingguoc@quicinc.com>
>>
>
--
Thank you & BRs
Tingguo
On Tue, 22 Oct 2024 at 11:26, Tingguo Cheng <quic_tingguoc@quicinc.com> wrote: > > > > On 10/14/2024 6:53 PM, Dmitry Baryshkov wrote: > > On Mon, Oct 14, 2024 at 06:08:17PM +0800, Tingguo Cheng wrote: > >> Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher- > >> als in the PMIC that controlled by SPMI bus. The peripherals include > >> GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power > >> key and volume down key are controlled by PMIC PON hardware on QCS615. > > > > Why do you need a separate file? Unless there is a good reason such > > configuration should be split between the pmic.dtsi and the board file. > > > One reason is that I use upstream as > template(x1ee80100,sc8180x,sa8775p...), they all have > platform-pmics.dtsi alongside. > > On the other hand, qcs615-pmic.dtsi contains only the PMIC's > peripherals, creating a new file can eliminate duplicate code if there > are any other new boards with pmm6155au as their PMIC(similar to > x1e80100-lenovo-xxx/x1e80100-microsoft-xxxx, where "x1e80100-pmics.dtsi" > has been included). And this is pretty unique to those platforms and it exists mostly to facilitate the case when a platform has several instances of the same PMIC. In all other cases (mobile, IoT) usually there is just one instance of the particular PMIC. In such a case pmic definitions go to "pmABCDEF.dtsi" to be reused by the platform and then board DT can include that file and wire it up according to the platform needs. > > BTW, what is the PMIC id for pmm6155au? Is it a real PMIC or a version > > of some other PMIC? > > > pmm6155au is the PMIC id(Silk scree). It's a real PMIC for qcs615.Maybe > I should use the name qcs615-pmics.dtsi instead of qcs6150-pmic.dtsi to > align with other platforms. Is it the same thing as PM6150? Or is it a combo of PM6150 and PM6150L? > >> Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com> -- With best wishes Dmitry
On 10/22/2024 5:38 PM, Dmitry Baryshkov wrote: > On Tue, 22 Oct 2024 at 11:26, Tingguo Cheng <quic_tingguoc@quicinc.com> wrote: >> >> >> >> On 10/14/2024 6:53 PM, Dmitry Baryshkov wrote: >>> On Mon, Oct 14, 2024 at 06:08:17PM +0800, Tingguo Cheng wrote: >>>> Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher- >>>> als in the PMIC that controlled by SPMI bus. The peripherals include >>>> GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power >>>> key and volume down key are controlled by PMIC PON hardware on QCS615. >>> >>> Why do you need a separate file? Unless there is a good reason such >>> configuration should be split between the pmic.dtsi and the board file. >>> >> One reason is that I use upstream as >> template(x1ee80100,sc8180x,sa8775p...), they all have >> platform-pmics.dtsi alongside. >> >> On the other hand, qcs615-pmic.dtsi contains only the PMIC's >> peripherals, creating a new file can eliminate duplicate code if there >> are any other new boards with pmm6155au as their PMIC(similar to >> x1e80100-lenovo-xxx/x1e80100-microsoft-xxxx, where "x1e80100-pmics.dtsi" >> has been included). > > And this is pretty unique to those platforms and it exists mostly to > facilitate the case when a platform has several instances of the same > PMIC. In all other cases (mobile, IoT) usually there is just one > instance of the particular PMIC. In such a case pmic definitions go to > "pmABCDEF.dtsi" to be reused by the platform and then board DT can > include that file and wire it up according to the platform needs. > Yes, in this case, there is only one PMIC. So making a new file does not make sense except the PMIC is totally a new design. >>> BTW, what is the PMIC id for pmm6155au? Is it a real PMIC or a version >>> of some other PMIC? >>> >> pmm6155au is the PMIC id(Silk scree). It's a real PMIC for qcs615.Maybe >> I should use the name qcs615-pmics.dtsi instead of qcs6150-pmic.dtsi to >> align with other platforms. > > Is it the same thing as PM6150? Or is it a combo of PM6150 and PM6150L? > I got your point, I did some more research, it's a variant of pm8150. In this case, I will upload a new version that makes DT to be changed after it includes pm8150.dtsi. >>>> Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com> > -- Thank you & BRs Tingguo
On Wed, Oct 23, 2024 at 06:25:33PM +0800, Tingguo Cheng wrote: > > > On 10/22/2024 5:38 PM, Dmitry Baryshkov wrote: > > On Tue, 22 Oct 2024 at 11:26, Tingguo Cheng <quic_tingguoc@quicinc.com> wrote: > > > > > > > > > > > > On 10/14/2024 6:53 PM, Dmitry Baryshkov wrote: > > > > On Mon, Oct 14, 2024 at 06:08:17PM +0800, Tingguo Cheng wrote: > > > > > Create a new file qcs615-pmic.dtsi to add on-board PMIC and peripher- > > > > > als in the PMIC that controlled by SPMI bus. The peripherals include > > > > > GPIO, RTC, PON, power key and resin-key for QCS615 platform. The power > > > > > key and volume down key are controlled by PMIC PON hardware on QCS615. > > > > > > > > Why do you need a separate file? Unless there is a good reason such > > > > configuration should be split between the pmic.dtsi and the board file. > > > > > > > One reason is that I use upstream as > > > template(x1ee80100,sc8180x,sa8775p...), they all have > > > platform-pmics.dtsi alongside. > > > > > > On the other hand, qcs615-pmic.dtsi contains only the PMIC's > > > peripherals, creating a new file can eliminate duplicate code if there > > > are any other new boards with pmm6155au as their PMIC(similar to > > > x1e80100-lenovo-xxx/x1e80100-microsoft-xxxx, where "x1e80100-pmics.dtsi" > > > has been included). > > > > And this is pretty unique to those platforms and it exists mostly to > > facilitate the case when a platform has several instances of the same > > PMIC. In all other cases (mobile, IoT) usually there is just one > > instance of the particular PMIC. In such a case pmic definitions go to > > "pmABCDEF.dtsi" to be reused by the platform and then board DT can > > include that file and wire it up according to the platform needs. > > > Yes, in this case, there is only one PMIC. So making a new file does not > make sense except the PMIC is totally a new design. > > > > BTW, what is the PMIC id for pmm6155au? Is it a real PMIC or a version > > > > of some other PMIC? > > > > > > > pmm6155au is the PMIC id(Silk scree). It's a real PMIC for qcs615.Maybe > > > I should use the name qcs615-pmics.dtsi instead of qcs6150-pmic.dtsi to > > > align with other platforms. > > > > Is it the same thing as PM6150? Or is it a combo of PM6150 and PM6150L? > > > I got your point, I did some more research, it's a variant of pm8150. In > this case, I will upload a new version that makes DT to be changed after it > includes pm8150.dtsi. That sounds good, thank you. > > > > > Signed-off-by: Tingguo Cheng <quic_tingguoc@quicinc.com> > > > > -- > Thank you & BRs > Tingguo > -- With best wishes Dmitry
On 14/10/2024 12:08, Tingguo Cheng wrote:
> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> index 4ef969a6af150933c72a7a83374a5a2657eebc1b..b79c22730920e3097425e1d1933e744205b3c18e 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> @@ -6,6 +6,7 @@
>
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> #include "qcs615.dtsi"
> +#include "qcs615-pmic.dtsi"
> / {
> model = "Qualcomm Technologies, Inc. QCS615 Ride";
> compatible = "qcom,qcs615-ride", "qcom,qcs615";
> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> index ac4c4c751da1fbb28865877555ba317677bc6bd2..9793161db515a2ef1df6465c8d0a04a11e71ffc1 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> @@ -517,6 +517,29 @@ sram@c3f0000 {
> reg = <0x0 0x0c3f0000 0x0 0x400>;
> };
>
> + spmi_bus: qcom,spmi@c440000 {
Please do not send your downstream code... Don't just copy and paste
that stuff.
Rewrite it from scratch or use the upstream as template. I find it waste
of time to comment or fix the same issue over and over again. The
problem is the way you work - copying and sending downstream at us. This
must stop.
Best regards,
Krzysztof
On 10/14/2024 6:45 PM, Krzysztof Kozlowski wrote:
> On 14/10/2024 12:08, Tingguo Cheng wrote:
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> index 4ef969a6af150933c72a7a83374a5a2657eebc1b..b79c22730920e3097425e1d1933e744205b3c18e 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> @@ -6,6 +6,7 @@
>>
>> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> #include "qcs615.dtsi"
>> +#include "qcs615-pmic.dtsi"
>> / {
>> model = "Qualcomm Technologies, Inc. QCS615 Ride";
>> compatible = "qcom,qcs615-ride", "qcom,qcs615";
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> index ac4c4c751da1fbb28865877555ba317677bc6bd2..9793161db515a2ef1df6465c8d0a04a11e71ffc1 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
>> @@ -517,6 +517,29 @@ sram@c3f0000 {
>> reg = <0x0 0x0c3f0000 0x0 0x400>;
>> };
>>
>> + spmi_bus: qcom,spmi@c440000 {
>
> Please do not send your downstream code... Don't just copy and paste
> that stuff.
>
> Rewrite it from scratch or use the upstream as template. I find it waste
> of time to comment or fix the same issue over and over again. The
> problem is the way you work - copying and sending downstream at us. This
> must stop.
We have realized the problem, and we will be more careful.For the
current patch, I'll upload a new one with fixing
>
> Best regards,
> Krzysztof
>
--
Thank you & BRs
Tingguo
© 2016 - 2024 Red Hat, Inc.