[PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[Dmitry Baryshkov]

The sc8280xp platform uses its own copy of PMIC declarations. This can
easily end up with the issues that are fixed in the main PMIC include
file, but are not fixed for sc8280xp (and vice versa). For example
commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON
compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the
PON device, while sc8280xp-pmic.dtsi still has the incorrect
"qcom,pm8998-pon".

Another example is pm8280_2_temp_alarm device, which uses interrupts
tied to SID 2, while having SID 3. This can be easily left unnoticed.

Employ a small amount of C preprocessor magic to make
sc8280xp-pmics.dtsi use standard PMIC include files

Also apply the same approach to sa8540p-pmics/pm8150.

Jonathan Cameron Acked merging the dt-bindigns patch together with the
rest of the patches to simplify merge process.

Dmitry Baryshkov (22):
  arm64: dts: qcom: pm8350: fix thermal zone node name
  arm64: dts: qcom: pm8350b: fix thermal zone node name
  arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device
  arm64: dts: qcom: sc8280xp-pmics: correct interrupt routing for
    pm8280_2_temp_alarm
  dt-bindings: iio: qcom,spmi-adc7-pmk8350.h: include sid into defines
  arm64: dts: qcom: pmk8350: rename pon label
  arm64: dts: qcom: pmk8350: port sdam_6 device from sc8280xp-pmics
  arm64: dts: qcom: pmk8350: rename PMK8350_SID to PMIC_SID
  arm64: dts: qcom: pmk8350: allow overriding the label
  arm64: dts: qcom: pmk8350: use interrupts-extended for IRQ
    specification
  arm64: dts: qcom: sc8280xp*: use pmk8350.dtsi
  arm64: dts: qcom: pm8350: allow overriding SID and label
  arm64: dts: qcom: pm8350: use interrupts-extended for IRQ
    specification
  arm64: dts: qcom: sc8280xp*: use pm8350.dtsi
  arm64: dts: qcom: pm8350c: move thermal zone declaration to the top
  arm64: dts: qcom: pm8350c: allow overriding SID and label
  arm64: dts: qcom: pm8350c: use interrupts-extended for IRQ
    specification
  arm64: dts: qcom: sc8280xp*: use pm8350c.dtsi
  arm64: dts: qcom: sc8280xp*: use pmr735a.dtsi
  arm64: dts: qcom: pm8150: convert to use dynamic SID/LABEL
  arch: arm64: dts: qcom: pm8150: support SID greater that 9
  arm64: dts: qcom sa8540p-pmics: switch to pm8150.dtsi

 .../bindings/iio/adc/qcom,spmi-vadc.yaml      |   2 +-
 .../bindings/thermal/qcom-spmi-adc-tm5.yaml   |   4 +-
 arch/arm64/boot/dts/qcom/pm8150.dtsi          |  53 +++--
 arch/arm64/boot/dts/qcom/pm8350.dtsi          |  33 ++-
 arch/arm64/boot/dts/qcom/pm8350b.dtsi         |   6 +-
 arch/arm64/boot/dts/qcom/pm8350c.dtsi         |  73 +++---
 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  23 ++
 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  26 +++
 arch/arm64/boot/dts/qcom/pmk8350.dtsi         |  51 ++--
 arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi   |  96 ++------
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      |   2 +-
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    |   2 +-
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts     |   4 +-
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |   8 +-
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi  | 221 ++----------------
 .../qcom/sm6375-sony-xperia-murray-pdx225.dts |   7 +-
 .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |   8 +-
 arch/arm64/boot/dts/qcom/sm8350-mtp.dts       |   8 +-
 .../dts/qcom/sm8350-sony-xperia-sagami.dtsi   |   8 +-
 .../dts/qcom/sm8450-sony-xperia-nagara.dtsi   |   4 +-
 .../dt-bindings/iio/qcom,spmi-adc7-pmk8350.h  |  52 ++---
 21 files changed, 279 insertions(+), 412 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi

-- 
2.30.2

[PATCH v2 01/22] arm64: dts: qcom: pm8350: fix thermal zone node name

[Dmitry Baryshkov]

Correct the thermal zone node names to remove the clash with
pm8350c.dtsi. Remove unused labels.

Fixes: 7a79b95f4288 ("arm64: dts: qcom: pm8350: add temp sensor and thermal zone config")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350.dtsi b/arch/arm64/boot/dts/qcom/pm8350.dtsi
index 2dfeb99300d7..9fb963df4b41 100644
--- a/arch/arm64/boot/dts/qcom/pm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350.dtsi
@@ -8,19 +8,19 @@
 
 / {
 	thermal-zones {
-		pm8350_thermal: pm8350c-thermal {
+		pm8350-thermal {
 			polling-delay-passive = <100>;
 			polling-delay = <0>;
 			thermal-sensors = <&pm8350_temp_alarm>;
 
 			trips {
-				pm8350_trip0: trip0 {
+				trip0 {
 					temperature = <95000>;
 					hysteresis = <0>;
 					type = "passive";
 				};
 
-				pm8350_crit: pm8350c-crit {
+				crit {
 					temperature = <115000>;
 					hysteresis = <0>;
 					type = "critical";
-- 
2.30.2

Re: [PATCH v2 01/22] arm64: dts: qcom: pm8350: fix thermal zone node name

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> Correct the thermal zone node names to remove the clash with
> pm8350c.dtsi. Remove unused labels.
> 
> Fixes: 7a79b95f4288 ("arm64: dts: qcom: pm8350: add temp sensor and thermal zone config")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/pm8350.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8350.dtsi b/arch/arm64/boot/dts/qcom/pm8350.dtsi
> index 2dfeb99300d7..9fb963df4b41 100644
> --- a/arch/arm64/boot/dts/qcom/pm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8350.dtsi
> @@ -8,19 +8,19 @@
>  
>  / {
>  	thermal-zones {
> -		pm8350_thermal: pm8350c-thermal {
> +		pm8350-thermal {
>  			polling-delay-passive = <100>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&pm8350_temp_alarm>;
>  
>  			trips {
> -				pm8350_trip0: trip0 {
> +				trip0 {
>  					temperature = <95000>;
>  					hysteresis = <0>;
>  					type = "passive";
>  				};
>  
> -				pm8350_crit: pm8350c-crit {
> +				crit {
>  					temperature = <115000>;
>  					hysteresis = <0>;
>  					type = "critical";

[PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Dmitry Baryshkov]

Correct the thermal zone node name to remove the clash with
pm8350c.dtsi. Remove unused labels.

Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350b.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350b.dtsi b/arch/arm64/boot/dts/qcom/pm8350b.dtsi
index f1c7bd9d079c..95e971b80ccc 100644
--- a/arch/arm64/boot/dts/qcom/pm8350b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350b.dtsi
@@ -8,19 +8,19 @@
 
 / {
 	thermal-zones {
-		pm8350b_thermal: pm8350c-thermal {
+		pm8350b-thermal {
 			polling-delay-passive = <100>;
 			polling-delay = <0>;
 			thermal-sensors = <&pm8350b_temp_alarm>;
 
 			trips {
-				pm8350b_trip0: trip0 {
+				trip0 {
 					temperature = <95000>;
 					hysteresis = <0>;
 					type = "passive";
 				};
 
-				pm8350b_crit: pm8350c-crit {
+				crit {
 					temperature = <115000>;
 					hysteresis = <0>;
 					type = "critical";
-- 
2.30.2

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Krzysztof Kozlowski]

On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> Correct the thermal zone node name to remove the clash with
> pm8350c.dtsi. Remove unused labels.
> 
> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")

Please describe the observable bug from that commit being fixed here.
Any future clash, which did not exist that time, is not a bug. It's future.

Naming changes here are more a matter of style, because the old names
were correct according to our coding guidelines, just not precise (c
instead of b). But node names anyway are not important from the point of
view fixes and adding such tag will cause a needless backport.



Best regards,
Krzysztof

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Dmitry Baryshkov]

On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>> Correct the thermal zone node name to remove the clash with
>> pm8350c.dtsi. Remove unused labels.
>>
>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
> 
> Please describe the observable bug from that commit being fixed here.
> Any future clash, which did not exist that time, is not a bug. It's future.
> 
> Naming changes here are more a matter of style, because the old names
> were correct according to our coding guidelines, just not precise (c
> instead of b). But node names anyway are not important from the point of
> view fixes and adding such tag will cause a needless backport.

It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result 
in one thermal zone overriding another one.

-- 
With best wishes
Dmitry

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Krzysztof Kozlowski]

On 02/04/2023 13:02, Dmitry Baryshkov wrote:
> On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>> Correct the thermal zone node name to remove the clash with
>>> pm8350c.dtsi. Remove unused labels.
>>>
>>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
>>
>> Please describe the observable bug from that commit being fixed here.
>> Any future clash, which did not exist that time, is not a bug. It's future.
>>
>> Naming changes here are more a matter of style, because the old names
>> were correct according to our coding guidelines, just not precise (c
>> instead of b). But node names anyway are not important from the point of
>> view fixes and adding such tag will cause a needless backport.
> 
> It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result 
> in one thermal zone overriding another one.

I don't understand. You used future tense "will", but we talk about
past. So where is the bug in commit 5c1399299d9d?

Best regards,
Krzysztof

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Dmitry Baryshkov]

On 03/04/2023 12:09, Krzysztof Kozlowski wrote:
> On 02/04/2023 13:02, Dmitry Baryshkov wrote:
>> On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
>>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>>> Correct the thermal zone node name to remove the clash with
>>>> pm8350c.dtsi. Remove unused labels.
>>>>
>>>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
>>>
>>> Please describe the observable bug from that commit being fixed here.
>>> Any future clash, which did not exist that time, is not a bug. It's future.
>>>
>>> Naming changes here are more a matter of style, because the old names
>>> were correct according to our coding guidelines, just not precise (c
>>> instead of b). But node names anyway are not important from the point of
>>> view fixes and adding such tag will cause a needless backport.
>>
>> It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result
>> in one thermal zone overriding another one.
> 
> I don't understand. You used future tense "will", but we talk about
> past. So where is the bug in commit 5c1399299d9d?

At that time there already existed sm8350-mtp which included both of 
dtsi files.

-- 
With best wishes
Dmitry

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Konrad Dybcio]

On 3.04.2023 11:50, Dmitry Baryshkov wrote:
> On 03/04/2023 12:09, Krzysztof Kozlowski wrote:
>> On 02/04/2023 13:02, Dmitry Baryshkov wrote:
>>> On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
>>>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>>>> Correct the thermal zone node name to remove the clash with
>>>>> pm8350c.dtsi. Remove unused labels.
>>>>>
>>>>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
>>>>
>>>> Please describe the observable bug from that commit being fixed here.
>>>> Any future clash, which did not exist that time, is not a bug. It's future.
>>>>
>>>> Naming changes here are more a matter of style, because the old names
>>>> were correct according to our coding guidelines, just not precise (c
>>>> instead of b). But node names anyway are not important from the point of
>>>> view fixes and adding such tag will cause a needless backport.
>>>
>>> It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result
>>> in one thermal zone overriding another one.
>>
>> I don't understand. You used future tense "will", but we talk about
>> past. So where is the bug in commit 5c1399299d9d?
> 
> At that time there already existed sm8350-mtp which included both of dtsi files.
> 
pm8350[bc] both have a node named /pm8350c-thermal, which means one will
get overriden with the other. Since we sort the includes alphabetically,
the one in pm8350b.dtsi will never be taken into account and hence the
temp alarm will never be associated to any thermal zone.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v2 02/22] arm64: dts: qcom: pm8350b: fix thermal zone node name

[Krzysztof Kozlowski]

On 03/04/2023 12:00, Konrad Dybcio wrote:
> 
> 
> On 3.04.2023 11:50, Dmitry Baryshkov wrote:
>> On 03/04/2023 12:09, Krzysztof Kozlowski wrote:
>>> On 02/04/2023 13:02, Dmitry Baryshkov wrote:
>>>> On 02/04/2023 13:34, Krzysztof Kozlowski wrote:
>>>>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>>>>> Correct the thermal zone node name to remove the clash with
>>>>>> pm8350c.dtsi. Remove unused labels.
>>>>>>
>>>>>> Fixes: 5c1399299d9d ("arm64: dts: qcom: pm8350b: add temp sensor and thermal zone config")
>>>>>
>>>>> Please describe the observable bug from that commit being fixed here.
>>>>> Any future clash, which did not exist that time, is not a bug. It's future.
>>>>>
>>>>> Naming changes here are more a matter of style, because the old names
>>>>> were correct according to our coding guidelines, just not precise (c
>>>>> instead of b). But node names anyway are not important from the point of
>>>>> view fixes and adding such tag will cause a needless backport.
>>>>
>>>> It is needed. Including both pm8350c.dtsi and pm8350b.dtsi will result
>>>> in one thermal zone overriding another one.
>>>
>>> I don't understand. You used future tense "will", but we talk about
>>> past. So where is the bug in commit 5c1399299d9d?
>>
>> At that time there already existed sm8350-mtp which included both of dtsi files.
>>
> pm8350[bc] both have a node named /pm8350c-thermal, which means one will
> get overriden with the other. Since we sort the includes alphabetically,
> the one in pm8350b.dtsi will never be taken into account and hence the
> temp alarm will never be associated to any thermal zone.

OK, if I understand correctly, this overwrite of thermal node was
already happening at commit 5c1399299d9d. It looks good then.

Best regards,
Krzysztof

[PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Dmitry Baryshkov]

Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
string and add RBS region to the PON device.

Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index c35e7f6bd657..a3c7369f9594 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -59,8 +59,8 @@ pmk8280: pmic@0 {
 		#size-cells = <0>;
 
 		pmk8280_pon: pon@1300 {
-			compatible = "qcom,pm8998-pon";
-			reg = <0x1300>;
+			compatible = "qcom,pmk8350-pon";
+			reg = <0x1300>, <0x800>;
 
 			pmk8280_pon_pwrkey: pwrkey {
 				compatible = "qcom,pmk8350-pwrkey";
-- 
2.30.2

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Krzysztof Kozlowski]

On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
> correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
> pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
> string and add RBS region to the PON device.
> 
> Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")

There is no compatible qcom,pmk8350-pon documented at ccd3517faf18, so
backporting it there is incorrect. qcom,pmk8350-pon is neither in v5.19
nor in v6.0.


Best regards,
Krzysztof

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Dmitry Baryshkov]

On Sun, 2 Apr 2023 at 12:42, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> > Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
> > correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
> > pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
> > string and add RBS region to the PON device.
> >
> > Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
>
> There is no compatible qcom,pmk8350-pon documented at ccd3517faf18, so
> backporting it there is incorrect. qcom,pmk8350-pon is neither in v5.19
> nor in v6.0.

Well, according to Documentation/process/submitting-patches.rst, Fixes
tag is about noting that there was an issue fixed in the commit. The
mentioned commit has an issue, as the device should have a second
region. I did not intend to have this patch backported (no Cc stable).
If I were, I could have also added a Cc stable # 5.19.x 03fccdc76dce.


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Krzysztof Kozlowski]

On 02/04/2023 12:25, Dmitry Baryshkov wrote:
> On Sun, 2 Apr 2023 at 12:42, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>> Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
>>> correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
>>> pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
>>> string and add RBS region to the PON device.
>>>
>>> Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
>>
>> There is no compatible qcom,pmk8350-pon documented at ccd3517faf18, so
>> backporting it there is incorrect. qcom,pmk8350-pon is neither in v5.19
>> nor in v6.0.
> 
> Well, according to Documentation/process/submitting-patches.rst, Fixes
> tag is about noting that there was an issue fixed in the commit. The
> mentioned commit has an issue, as the device should have a second

Depends. If device was working in some limited way with old compatible
and one region, there is nothing to fix maybe. It was just incomplete.

If second region is needed for the work, then only that commit should be
marked as fix. Changing compatible is not a fix of that submission
because at the time, the compatible was correct. That time in Git
history, the "qcom,pmk8350-pon" was not correct.

> region. I did not intend to have this patch backported (no Cc stable).
> If I were, I could have also added a Cc stable # 5.19.x 03fccdc76dce.

AUTOSEL will backport it anyway, if you do not mention otherwise.

Best regards,
Krzysztof

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Dmitry Baryshkov]

On 02/04/2023 13:32, Krzysztof Kozlowski wrote:
> On 02/04/2023 12:25, Dmitry Baryshkov wrote:
>> On Sun, 2 Apr 2023 at 12:42, Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>>> Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
>>>> correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
>>>> pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
>>>> string and add RBS region to the PON device.
>>>>
>>>> Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
>>>
>>> There is no compatible qcom,pmk8350-pon documented at ccd3517faf18, so
>>> backporting it there is incorrect. qcom,pmk8350-pon is neither in v5.19
>>> nor in v6.0.
>>
>> Well, according to Documentation/process/submitting-patches.rst, Fixes
>> tag is about noting that there was an issue fixed in the commit. The
>> mentioned commit has an issue, as the device should have a second
> 
> Depends. If device was working in some limited way with old compatible
> and one region, there is nothing to fix maybe. It was just incomplete.
> 
> If second region is needed for the work, then only that commit should be
> marked as fix. Changing compatible is not a fix of that submission
> because at the time, the compatible was correct. That time in Git
> history, the "qcom,pmk8350-pon" was not correct.

Ack, so we have to backport the schema too.

> 
>> region. I did not intend to have this patch backported (no Cc stable).
>> If I were, I could have also added a Cc stable # 5.19.x 03fccdc76dce.
> 
> AUTOSEL will backport it anyway, if you do not mention otherwise.

Is there a way to influence AUTOSEL to make it also pick up another commit?

> 
> Best regards,
> Krzysztof
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Krzysztof Kozlowski]

On 02/04/2023 13:03, Dmitry Baryshkov wrote:
> 
>>
>>> region. I did not intend to have this patch backported (no Cc stable).
>>> If I were, I could have also added a Cc stable # 5.19.x 03fccdc76dce.
>>
>> AUTOSEL will backport it anyway, if you do not mention otherwise.
> 
> Is there a way to influence AUTOSEL to make it also pick up another commit?

Only via usual stable kernel patch rules/syntax.

Best regards,
Krzysztof

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Konrad Dybcio]

On 2.04.2023 13:12, Krzysztof Kozlowski wrote:
> On 02/04/2023 13:03, Dmitry Baryshkov wrote:
>>
>>>
>>>> region. I did not intend to have this patch backported (no Cc stable).
>>>> If I were, I could have also added a Cc stable # 5.19.x 03fccdc76dce.
>>>
>>> AUTOSEL will backport it anyway, if you do not mention otherwise.
>>
>> Is there a way to influence AUTOSEL to make it also pick up another commit?
> 
> Only via usual stable kernel patch rules/syntax.

Additionally, some patches submitted via Option 1 may have additional patch prerequisites which can be cherry-picked. This can be specified in the following format in the sign-off area:

Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle
Cc: <stable@vger.kernel.org> # 3.3.x: 1b9508f: sched: Rate-limit newidle
Cc: <stable@vger.kernel.org> # 3.3.x: fd21073: sched: Fix affinity logic
Cc: <stable@vger.kernel.org> # 3.3.x
Signed-off-by: Ingo Molnar <mingo@elte.hu>

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html


Konrad

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 03/22] arm64: dts: qcom: sc8280xp-pmics: use pmk8350 specifics for pon device

[Johan Hovold]

On Sun, Apr 02, 2023 at 01:07:51AM +0300, Dmitry Baryshkov wrote:
> Following the commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the
> correct PON compatible") and commit f46ef374e0dc ("arm64: dts: qcom:
> pmk8350: Specify PBS register for PON") use "qcom,pmk8350-pon" compat
> string and add RBS region to the PON device.
> 
> Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

I already sent a fix for this last Monday:

	https://lore.kernel.org/all/20230327122948.4323-1-johan+linaro@kernel.org/

> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> index c35e7f6bd657..a3c7369f9594 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> @@ -59,8 +59,8 @@ pmk8280: pmic@0 {
>  		#size-cells = <0>;
>  
>  		pmk8280_pon: pon@1300 {
> -			compatible = "qcom,pm8998-pon";
> -			reg = <0x1300>;
> +			compatible = "qcom,pmk8350-pon";
> +			reg = <0x1300>, <0x800>;

This is missing 'reg-names'.

>  
>  			pmk8280_pon_pwrkey: pwrkey {
>  				compatible = "qcom,pmk8350-pwrkey";

Johan

[PATCH v2 04/22] arm64: dts: qcom: sc8280xp-pmics: correct interrupt routing for pm8280_2_temp_alarm

[Dmitry Baryshkov]

The PMIC pm8280_2 has SID equal to 3, thus it interrupts-extended should
use 3 as the first argument value. Fix the interrupts-extended value for
pm8280_2_temp_alarm device node.

Fixes: 6c82f40ec94e ("arm64: dts: qcom: sc8280xp-pmics: Add temp alarm for PM8280_{1/2} PMICs")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index a3c7369f9594..254337345e45 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -176,7 +176,7 @@ pmc8280_2: pmic@3 {
 		pm8280_2_temp_alarm: temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
-			interrupts-extended = <&spmi_bus 0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts-extended = <&spmi_bus 0x3 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
 			#thermal-sensor-cells = <0>;
 		};
 
-- 
2.30.2

Re: [PATCH v2 04/22] arm64: dts: qcom: sc8280xp-pmics: correct interrupt routing for pm8280_2_temp_alarm

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> The PMIC pm8280_2 has SID equal to 3, thus it interrupts-extended should
> use 3 as the first argument value. Fix the interrupts-extended value for
> pm8280_2_temp_alarm device node.
> 
> Fixes: 6c82f40ec94e ("arm64: dts: qcom: sc8280xp-pmics: Add temp alarm for PM8280_{1/2} PMICs")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> index a3c7369f9594..254337345e45 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> @@ -176,7 +176,7 @@ pmc8280_2: pmic@3 {
>  		pm8280_2_temp_alarm: temp-alarm@a00 {
>  			compatible = "qcom,spmi-temp-alarm";
>  			reg = <0xa00>;
> -			interrupts-extended = <&spmi_bus 0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
> +			interrupts-extended = <&spmi_bus 0x3 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
>  			#thermal-sensor-cells = <0>;
>  		};
>  

[PATCH v2 05/22] dt-bindings: iio: qcom,spmi-adc7-pmk8350.h: include sid into defines

[Dmitry Baryshkov]

pmk8350 can take different addresses on SPMI bus. Rather than having a
default SID, follow the pm8350's example and make the sid explicit when
specifying ADC channels.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/iio/adc/qcom,spmi-vadc.yaml      |  2 +-
 .../bindings/thermal/qcom-spmi-adc-tm5.yaml   |  4 +-
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi      |  2 +-
 arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi    |  2 +-
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |  4 +-
 .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |  2 +-
 .../dt-bindings/iio/qcom,spmi-adc7-pmk8350.h  | 52 +++++++++----------
 7 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
index bd6e0d6f6e0c..df317901e7d0 100644
--- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml
@@ -293,7 +293,7 @@ examples:
 
             /* Other properties are omitted */
             xo-therm@44 {
-                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>;
                 qcom,ratiometric;
                 qcom,hw-settle-time = <200>;
             };
diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
index 52ec18cf1eda..ff07d27775dc 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
@@ -218,7 +218,7 @@ examples:
 
             /* Other properties are omitted */
             xo-therm@44 {
-                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>;
                 qcom,ratiometric;
                 qcom,hw-settle-time = <200>;
             };
@@ -240,7 +240,7 @@ examples:
 
             pmk8350-xo-therm@0 {
                 reg = <0>;
-                io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU>;
+                io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU(0)>;
                 qcom,decimation = <340>;
                 qcom,ratiometric;
                 qcom,hw-settle-time-us = <200>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 5dc9bee28e7f..14c9bdaa46ed 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -433,7 +433,7 @@ &pcie1_phy {
 
 &pmk8350_vadc {
 	pmk8350-die-temp@3 {
-		reg = <PMK8350_ADC7_DIE_TEMP>;
+		reg = <PMK8350_ADC7_DIE_TEMP(0)>;
 		label = "pmk8350_die_temp";
 		qcom,pre-scaling = <1 1>;
 	};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
index cb0cc2ba2fa3..e3919e074ebd 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
@@ -389,7 +389,7 @@ &pm8350c_pwm {
 
 &pmk8350_vadc {
 	pmk8350-die-temp@3 {
-		reg = <PMK8350_ADC7_DIE_TEMP>;
+		reg = <PMK8350_ADC7_DIE_TEMP(0)>;
 		label = "pmk8350_die_temp";
 		qcom,pre-scaling = <1 1>;
 	};
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 46c7fdafb840..590400985055 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -824,13 +824,13 @@ &pmk8280_vadc {
 	status = "okay";
 
 	pmic-die-temp@3 {
-		reg = <PMK8350_ADC7_DIE_TEMP>;
+		reg = <PMK8350_ADC7_DIE_TEMP(0)>;
 		qcom,pre-scaling = <1 1>;
 		label = "pmk8350_die_temp";
 	};
 
 	xo-therm@44 {
-		reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+		reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>;
 		qcom,hw-settle-time = <200>;
 		qcom,ratiometric;
 		label = "pmk8350_xo_therm";
diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
index 7ae6aba5d2ec..af6cf4fbddc7 100644
--- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
+++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
@@ -516,7 +516,7 @@ &pmk8350_rtc {
 
 &pmk8350_vadc {
 	adc-chan@644 {
-		reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+		reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>;
 		qcom,ratiometric;
 		qcom,hw-settle-time = <200>;
 		qcom,pre-scaling = <1 1>;
diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
index 6c296870e95b..ca85a2d69453 100644
--- a/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
+++ b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h
@@ -6,41 +6,37 @@
 #ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H
 #define _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H
 
-#ifndef PMK8350_SID
-#define PMK8350_SID					0
-#endif
-
 /* ADC channels for PMK8350_ADC for PMIC7 */
-#define PMK8350_ADC7_REF_GND			(PMK8350_SID << 8 | 0x0)
-#define PMK8350_ADC7_1P25VREF			(PMK8350_SID << 8 | 0x01)
-#define PMK8350_ADC7_VREF_VADC			(PMK8350_SID << 8 | 0x02)
-#define PMK8350_ADC7_DIE_TEMP			(PMK8350_SID << 8 | 0x03)
+#define PMK8350_ADC7_REF_GND(sid)			((sid) << 8 | 0x0)
+#define PMK8350_ADC7_1P25VREF(sid)			((sid) << 8 | 0x01)
+#define PMK8350_ADC7_VREF_VADC(sid)			((sid) << 8 | 0x02)
+#define PMK8350_ADC7_DIE_TEMP(sid)			((sid) << 8 | 0x03)
 
-#define PMK8350_ADC7_AMUX_THM1			(PMK8350_SID << 8 | 0x04)
-#define PMK8350_ADC7_AMUX_THM2			(PMK8350_SID << 8 | 0x05)
-#define PMK8350_ADC7_AMUX_THM3			(PMK8350_SID << 8 | 0x06)
-#define PMK8350_ADC7_AMUX_THM4			(PMK8350_SID << 8 | 0x07)
-#define PMK8350_ADC7_AMUX_THM5			(PMK8350_SID << 8 | 0x08)
+#define PMK8350_ADC7_AMUX_THM1(sid)			((sid) << 8 | 0x04)
+#define PMK8350_ADC7_AMUX_THM2(sid)			((sid) << 8 | 0x05)
+#define PMK8350_ADC7_AMUX_THM3(sid)			((sid) << 8 | 0x06)
+#define PMK8350_ADC7_AMUX_THM4(sid)			((sid) << 8 | 0x07)
+#define PMK8350_ADC7_AMUX_THM5(sid)			((sid) << 8 | 0x08)
 
 /* 30k pull-up1 */
-#define PMK8350_ADC7_AMUX_THM1_30K_PU		(PMK8350_SID << 8 | 0x24)
-#define PMK8350_ADC7_AMUX_THM2_30K_PU		(PMK8350_SID << 8 | 0x25)
-#define PMK8350_ADC7_AMUX_THM3_30K_PU		(PMK8350_SID << 8 | 0x26)
-#define PMK8350_ADC7_AMUX_THM4_30K_PU		(PMK8350_SID << 8 | 0x27)
-#define PMK8350_ADC7_AMUX_THM5_30K_PU		(PMK8350_SID << 8 | 0x28)
+#define PMK8350_ADC7_AMUX_THM1_30K_PU(sid)		((sid) << 8 | 0x24)
+#define PMK8350_ADC7_AMUX_THM2_30K_PU(sid)		((sid) << 8 | 0x25)
+#define PMK8350_ADC7_AMUX_THM3_30K_PU(sid)		((sid) << 8 | 0x26)
+#define PMK8350_ADC7_AMUX_THM4_30K_PU(sid)		((sid) << 8 | 0x27)
+#define PMK8350_ADC7_AMUX_THM5_30K_PU(sid)		((sid) << 8 | 0x28)
 
 /* 100k pull-up2 */
-#define PMK8350_ADC7_AMUX_THM1_100K_PU		(PMK8350_SID << 8 | 0x44)
-#define PMK8350_ADC7_AMUX_THM2_100K_PU		(PMK8350_SID << 8 | 0x45)
-#define PMK8350_ADC7_AMUX_THM3_100K_PU		(PMK8350_SID << 8 | 0x46)
-#define PMK8350_ADC7_AMUX_THM4_100K_PU		(PMK8350_SID << 8 | 0x47)
-#define PMK8350_ADC7_AMUX_THM5_100K_PU		(PMK8350_SID << 8 | 0x48)
+#define PMK8350_ADC7_AMUX_THM1_100K_PU(sid)		((sid) << 8 | 0x44)
+#define PMK8350_ADC7_AMUX_THM2_100K_PU(sid)		((sid) << 8 | 0x45)
+#define PMK8350_ADC7_AMUX_THM3_100K_PU(sid)		((sid) << 8 | 0x46)
+#define PMK8350_ADC7_AMUX_THM4_100K_PU(sid)		((sid) << 8 | 0x47)
+#define PMK8350_ADC7_AMUX_THM5_100K_PU(sid)		((sid) << 8 | 0x48)
 
 /* 400k pull-up3 */
-#define PMK8350_ADC7_AMUX_THM1_400K_PU		(PMK8350_SID << 8 | 0x64)
-#define PMK8350_ADC7_AMUX_THM2_400K_PU		(PMK8350_SID << 8 | 0x65)
-#define PMK8350_ADC7_AMUX_THM3_400K_PU		(PMK8350_SID << 8 | 0x66)
-#define PMK8350_ADC7_AMUX_THM4_400K_PU		(PMK8350_SID << 8 | 0x67)
-#define PMK8350_ADC7_AMUX_THM5_400K_PU		(PMK8350_SID << 8 | 0x68)
+#define PMK8350_ADC7_AMUX_THM1_400K_PU(sid)		((sid) << 8 | 0x64)
+#define PMK8350_ADC7_AMUX_THM2_400K_PU(sid)		((sid) << 8 | 0x65)
+#define PMK8350_ADC7_AMUX_THM3_400K_PU(sid)		((sid) << 8 | 0x66)
+#define PMK8350_ADC7_AMUX_THM4_400K_PU(sid)		((sid) << 8 | 0x67)
+#define PMK8350_ADC7_AMUX_THM5_400K_PU(sid)		((sid) << 8 | 0x68)
 
 #endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H */
-- 
2.30.2

[PATCH v2 06/22] arm64: dts: qcom: pmk8350: rename pon label

[Dmitry Baryshkov]

To reduce a possibility of the conflicts, rename pmk8350's pon_pwrkey
and pon_resin to contain the PMIC name too.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pmk8350.dtsi                   | 4 ++--
 arch/arm64/boot/dts/qcom/sm8350-mtp.dts                 | 8 ++++----
 arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi | 8 ++++----
 arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi | 4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index f26fb7d32faf..455ffffb5f5c 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -25,14 +25,14 @@ pmk8350_pon: pon@1300 {
 			reg = <0x1300>, <0x800>;
 			reg-names = "hlos", "pbs";
 
-			pon_pwrkey: pwrkey {
+			pmk8350_pon_pwrkey: pwrkey {
 				compatible = "qcom,pmk8350-pwrkey";
 				interrupts = <PMK8350_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
 				linux,code = <KEY_POWER>;
 				status = "disabled";
 			};
 
-			pon_resin: resin {
+			pmk8350_pon_resin: resin {
 				compatible = "qcom,pmk8350-resin";
 				interrupts = <PMK8350_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
 				status = "disabled";
diff --git a/arch/arm64/boot/dts/qcom/sm8350-mtp.dts b/arch/arm64/boot/dts/qcom/sm8350-mtp.dts
index d21d2aacf201..152601832cbc 100644
--- a/arch/arm64/boot/dts/qcom/sm8350-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8350-mtp.dts
@@ -285,17 +285,17 @@ &mpss {
 	firmware-name = "qcom/sm8350/modem.mbn";
 };
 
-&pmk8350_rtc {
+&pmk8350_pon_pwrkey {
 	status = "okay";
 };
 
-&pon_pwrkey {
+&pmk8350_pon_resin {
 	status = "okay";
+	linux,code = <KEY_VOLUMEDOWN>;
 };
 
-&pon_resin {
+&pmk8350_rtc {
 	status = "okay";
-	linux,code = <KEY_VOLUMEDOWN>;
 };
 
 &qupv3_id_0 {
diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
index 89382ad73133..e28f49e31b9f 100644
--- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
@@ -618,17 +618,17 @@ vol_down_n: vol-down-n-state {
 	};
 };
 
-&pmk8350_rtc {
+&pmk8350_pon_pwrkey {
 	status = "okay";
 };
 
-&pon_pwrkey {
+&pmk8350_pon_resin {
 	status = "okay";
+	linux,code = <KEY_VOLUMEUP>;
 };
 
-&pon_resin {
+&pmk8350_rtc {
 	status = "okay";
-	linux,code = <KEY_VOLUMEUP>;
 };
 
 &qupv3_id_0 {
diff --git a/arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi b/arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi
index 67538b5a557e..99e9b776b93d 100644
--- a/arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi
@@ -684,11 +684,11 @@ &pmk8350_gpios {
 			  "PMK8350_OPTION";
 };
 
-&pon_pwrkey {
+&pmk8350_pon_pwrkey {
 	status = "okay";
 };
 
-&pon_resin {
+&pmk8350_pon_resin {
 	linux,code = <KEY_VOLUMEUP>;
 	status = "okay";
 };
-- 
2.30.2

[PATCH v2 07/22] arm64: dts: qcom: pmk8350: port sdam_6 device from sc8280xp-pmics

[Dmitry Baryshkov]

sc8280xp-pmics.dtsi contains a copy of pmk8350 with one extra device,
nvram (sdam). Port its definition from sc8280xp-pmics.dtsi into main
pmk8350.dtsi file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pmk8350.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index 455ffffb5f5c..26e40dbfc173 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -66,6 +66,15 @@ pmk8350_rtc: rtc@6100 {
 			status = "disabled";
 		};
 
+		pmk8350_sdam_6: nvram@8500 {
+			compatible = "qcom,spmi-sdam";
+			reg = <0x8500>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 0x8500 0x100>;
+			status = "disabled";
+		};
+
 		pmk8350_gpios: gpio@b000 {
 			compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio";
 			reg = <0xb000>;
-- 
2.30.2

Re: [PATCH v2 07/22] arm64: dts: qcom: pmk8350: port sdam_6 device from sc8280xp-pmics

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> sc8280xp-pmics.dtsi contains a copy of pmk8350 with one extra device,
> nvram (sdam). Port its definition from sc8280xp-pmics.dtsi into main
> pmk8350.dtsi file.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Downstream uses the name `pmk8350_sdam_22` for this reg and
sdam_1 corresponds to 0x7000, with 0x100 increments. Was the
sc8280xp definition wrong wrt this, or do they differ?

Konrad
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> index 455ffffb5f5c..26e40dbfc173 100644
> --- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> @@ -66,6 +66,15 @@ pmk8350_rtc: rtc@6100 {
>  			status = "disabled";
>  		};
>  
> +		pmk8350_sdam_6: nvram@8500 {
> +			compatible = "qcom,spmi-sdam";
> +			reg = <0x8500>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 0x8500 0x100>;
> +			status = "disabled";
> +		};
> +
>  		pmk8350_gpios: gpio@b000 {
>  			compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio";
>  			reg = <0xb000>;

[PATCH v2 08/22] arm64: dts: qcom: pmk8350: rename PMK8350_SID to PMIC_SID

[Dmitry Baryshkov]

Several next patches are going to introduce flexibility into several
other PMICs in order to support variable SID and node labels. This will
result in close duplication of several similar preprocessor
constructions. Having a per-PMIC defines would be error prone and can
result in hard-to-notices mistakes. Thus rename PMK8350_SID to a generic
PMIC_SID.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 20 ++++++++++---------
 .../qcom/sm6375-sony-xperia-murray-pdx225.dts |  7 ++++---
 .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |  6 +++---
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index 26e40dbfc173..02e7518ab57b 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -9,14 +9,14 @@
 #include <dt-bindings/spmi/spmi.h>
 
 /* (Sadly) this PMIC can be configured to be at different SIDs */
-#ifndef PMK8350_SID
-	#define PMK8350_SID 0
+#ifndef PMIC_SID
+	#define PMIC_SID 0
 #endif
 
 &spmi_bus {
-	pmk8350: pmic@PMK8350_SID {
+	pmk8350: pmic@PMIC_SID {
 		compatible = "qcom,pmk8350", "qcom,spmi-pmic";
-		reg = <PMK8350_SID SPMI_USID>;
+		reg = <PMIC_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
@@ -27,14 +27,14 @@ pmk8350_pon: pon@1300 {
 
 			pmk8350_pon_pwrkey: pwrkey {
 				compatible = "qcom,pmk8350-pwrkey";
-				interrupts = <PMK8350_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
 				linux,code = <KEY_POWER>;
 				status = "disabled";
 			};
 
 			pmk8350_pon_resin: resin {
 				compatible = "qcom,pmk8350-resin";
-				interrupts = <PMK8350_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
 				status = "disabled";
 			};
 		};
@@ -44,14 +44,14 @@ pmk8350_vadc: adc@3100 {
 			reg = <0x3100>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-			interrupts = <PMK8350_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 			#io-channel-cells = <1>;
 		};
 
 		pmk8350_adc_tm: adc-tm@3400 {
 			compatible = "qcom,adc-tm7";
 			reg = <0x3400>;
-			interrupts = <PMK8350_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#thermal-sensor-cells = <1>;
@@ -62,7 +62,7 @@ pmk8350_rtc: rtc@6100 {
 			compatible = "qcom,pmk8350-rtc";
 			reg = <0x6100>, <0x6200>;
 			reg-names = "rtc", "alarm";
-			interrupts = <PMK8350_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
 			status = "disabled";
 		};
 
@@ -86,3 +86,5 @@ pmk8350_gpios: gpio@b000 {
 		};
 	};
 };
+
+#undef PMIC_SID
diff --git a/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts b/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
index b691c3834b6b..863fc1f4b0b6 100644
--- a/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
+++ b/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
@@ -5,13 +5,14 @@
 
 /dts-v1/;
 
-/* PMK8350 is configured to use SID6 instead of 0 */
-#define PMK8350_SID 6
-
 #include <dt-bindings/gpio/gpio.h>
 #include "sm6375.dtsi"
 #include "pm6125.dtsi"
+
+/* PMK8350 is configured to use SID6 instead of 0 */
+#define PMIC_SID 6
 #include "pmk8350.dtsi"
+
 #include "pmr735a.dtsi"
 
 /* PM6125 PON is used and we can't have duplicate labels */
diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
index af6cf4fbddc7..965b37245007 100644
--- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
+++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
@@ -5,9 +5,6 @@
 
 /dts-v1/;
 
-/* PMK8350 (in reality a PMK8003) is configured to use SID6 instead of 0 */
-#define PMK8350_SID 6
-
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
 #include <dt-bindings/input/input.h>
@@ -18,6 +15,9 @@
 #include "pm6150l.dtsi"
 #include "pm6350.dtsi"
 #include "pm7250b.dtsi"
+
+/* PMK8350 (in reality a PMK8003) is configured to use SID6 instead of 0 */
+#define PMIC_SID 6
 #include "pmk8350.dtsi"
 
 / {
-- 
2.30.2

Re: [PATCH v2 08/22] arm64: dts: qcom: pmk8350: rename PMK8350_SID to PMIC_SID

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> Several next patches are going to introduce flexibility into several
> other PMICs in order to support variable SID and node labels. This will
> result in close duplication of several similar preprocessor
> constructions. Having a 
drop the 'a'

per-PMIC defines would be error prone and can
> result in hard-to-notices
hard-to-notice

 mistakes. Thus rename PMK8350_SID to a generic
> PMIC_SID.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 20 ++++++++++---------
>  .../qcom/sm6375-sony-xperia-murray-pdx225.dts |  7 ++++---
>  .../boot/dts/qcom/sm7225-fairphone-fp4.dts    |  6 +++---
>  3 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> index 26e40dbfc173..02e7518ab57b 100644
> --- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> @@ -9,14 +9,14 @@
>  #include <dt-bindings/spmi/spmi.h>
>  
>  /* (Sadly) this PMIC can be configured to be at different SIDs */
> -#ifndef PMK8350_SID
> -	#define PMK8350_SID 0
> +#ifndef PMIC_SID
> +	#define PMIC_SID 0
>  #endif
>  
>  &spmi_bus {
> -	pmk8350: pmic@PMK8350_SID {
> +	pmk8350: pmic@PMIC_SID {
>  		compatible = "qcom,pmk8350", "qcom,spmi-pmic";
> -		reg = <PMK8350_SID SPMI_USID>;
> +		reg = <PMIC_SID SPMI_USID>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> @@ -27,14 +27,14 @@ pmk8350_pon: pon@1300 {
>  
>  			pmk8350_pon_pwrkey: pwrkey {
>  				compatible = "qcom,pmk8350-pwrkey";
> -				interrupts = <PMK8350_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
>  				linux,code = <KEY_POWER>;
>  				status = "disabled";
>  			};
>  
>  			pmk8350_pon_resin: resin {
>  				compatible = "qcom,pmk8350-resin";
> -				interrupts = <PMK8350_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
>  				status = "disabled";
>  			};
>  		};
> @@ -44,14 +44,14 @@ pmk8350_vadc: adc@3100 {
>  			reg = <0x3100>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -			interrupts = <PMK8350_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#io-channel-cells = <1>;
>  		};
>  
>  		pmk8350_adc_tm: adc-tm@3400 {
>  			compatible = "qcom,adc-tm7";
>  			reg = <0x3400>;
> -			interrupts = <PMK8350_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			#thermal-sensor-cells = <1>;
> @@ -62,7 +62,7 @@ pmk8350_rtc: rtc@6100 {
>  			compatible = "qcom,pmk8350-rtc";
>  			reg = <0x6100>, <0x6200>;
>  			reg-names = "rtc", "alarm";
> -			interrupts = <PMK8350_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
> +			interrupts = <PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
>  			status = "disabled";
>  		};
>  
> @@ -86,3 +86,5 @@ pmk8350_gpios: gpio@b000 {
>  		};
>  	};
>  };
> +
> +#undef PMIC_SID
> diff --git a/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts b/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
> index b691c3834b6b..863fc1f4b0b6 100644
> --- a/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
> +++ b/arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts
> @@ -5,13 +5,14 @@
>  
>  /dts-v1/;
>  
> -/* PMK8350 is configured to use SID6 instead of 0 */
> -#define PMK8350_SID 6
> -
>  #include <dt-bindings/gpio/gpio.h>
>  #include "sm6375.dtsi"
>  #include "pm6125.dtsi"
> +
> +/* PMK8350 is configured to use SID6 instead of 0 */
> +#define PMIC_SID 6
>  #include "pmk8350.dtsi"
> +
>  #include "pmr735a.dtsi"
>  
>  /* PM6125 PON is used and we can't have duplicate labels */
> diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> index af6cf4fbddc7..965b37245007 100644
> --- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> +++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
> @@ -5,9 +5,6 @@
>  
>  /dts-v1/;
>  
> -/* PMK8350 (in reality a PMK8003) is configured to use SID6 instead of 0 */
> -#define PMK8350_SID 6
> -
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
>  #include <dt-bindings/input/input.h>
> @@ -18,6 +15,9 @@
>  #include "pm6150l.dtsi"
>  #include "pm6350.dtsi"
>  #include "pm7250b.dtsi"
> +
> +/* PMK8350 (in reality a PMK8003) is configured to use SID6 instead of 0 */
> +#define PMIC_SID 6
>  #include "pmk8350.dtsi"
>  
>  / {

[PATCH v2 09/22] arm64: dts: qcom: pmk8350: allow overriding the label

[Dmitry Baryshkov]

sc8280xp-pmics.dtsi incorporates a copy of pmk8350, but the dts files
use labels following the markings found in the schematics. Allow
overriding the labels in pmk8350.dtsi.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 13 +++++++++
 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 14 ++++++++++
 arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 28 +++++++++++--------
 3 files changed, 44 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi

diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
new file mode 100644
index 000000000000..1c81269f0783
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+/* cleanly revert the effects pmic-dyn-header.dtsi */
+
+#undef LABEL
+#undef _LABEL
+#undef __LABEL
+
+#undef PMIC_SID
+#undef PMIC_LABEL
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
new file mode 100644
index 000000000000..75f0448568bd
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+/*
+ * Preprocessor symbols to assist in describing PMICs which have configurable
+ * SID. All effects of this header (or its parameters) must be reverted in
+ * pmic-dyn-footer.dtsi.
+ */
+
+#define LABEL(name) _LABEL(PMIC_LABEL, name)
+#define _LABEL(pmic, name) __LABEL(pmic, name)
+#define __LABEL(pmic, name) pmic ## _ ## name
diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index 02e7518ab57b..f26ff3daf119 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -13,33 +13,39 @@
 	#define PMIC_SID 0
 #endif
 
+#ifndef PMIC_LABEL
+#define PMIC_LABEL pmk8350
+#endif
+
+#include "pmic-dyn-header.dtsi"
+
 &spmi_bus {
-	pmk8350: pmic@PMIC_SID {
+	PMIC_LABEL: pmic@PMIC_SID {
 		compatible = "qcom,pmk8350", "qcom,spmi-pmic";
 		reg = <PMIC_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		pmk8350_pon: pon@1300 {
+		LABEL(pon): pon@1300 {
 			compatible = "qcom,pmk8350-pon";
 			reg = <0x1300>, <0x800>;
 			reg-names = "hlos", "pbs";
 
-			pmk8350_pon_pwrkey: pwrkey {
+			LABEL(pon_pwrkey): pwrkey {
 				compatible = "qcom,pmk8350-pwrkey";
 				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
 				linux,code = <KEY_POWER>;
 				status = "disabled";
 			};
 
-			pmk8350_pon_resin: resin {
+			LABEL(pon_resin): resin {
 				compatible = "qcom,pmk8350-resin";
 				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
 				status = "disabled";
 			};
 		};
 
-		pmk8350_vadc: adc@3100 {
+		LABEL(vadc): adc@3100 {
 			compatible = "qcom,spmi-adc7";
 			reg = <0x3100>;
 			#address-cells = <1>;
@@ -48,7 +54,7 @@ pmk8350_vadc: adc@3100 {
 			#io-channel-cells = <1>;
 		};
 
-		pmk8350_adc_tm: adc-tm@3400 {
+		LABEL(adc_tm): adc-tm@3400 {
 			compatible = "qcom,adc-tm7";
 			reg = <0x3400>;
 			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
@@ -58,7 +64,7 @@ pmk8350_adc_tm: adc-tm@3400 {
 			status = "disabled";
 		};
 
-		pmk8350_rtc: rtc@6100 {
+		LABEL(rtc): rtc@6100 {
 			compatible = "qcom,pmk8350-rtc";
 			reg = <0x6100>, <0x6200>;
 			reg-names = "rtc", "alarm";
@@ -66,7 +72,7 @@ pmk8350_rtc: rtc@6100 {
 			status = "disabled";
 		};
 
-		pmk8350_sdam_6: nvram@8500 {
+		LABEL(sdam_6): nvram@8500 {
 			compatible = "qcom,spmi-sdam";
 			reg = <0x8500>;
 			#address-cells = <1>;
@@ -75,11 +81,11 @@ pmk8350_sdam_6: nvram@8500 {
 			status = "disabled";
 		};
 
-		pmk8350_gpios: gpio@b000 {
+		LABEL(gpios): gpio@b000 {
 			compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio";
 			reg = <0xb000>;
 			gpio-controller;
-			gpio-ranges = <&pmk8350_gpios 0 0 4>;
+			gpio-ranges = <&LABEL(gpios) 0 0 4>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
@@ -87,4 +93,4 @@ pmk8350_gpios: gpio@b000 {
 	};
 };
 
-#undef PMIC_SID
+#include "pmic-dyn-footer.dtsi"
-- 
2.30.2

Re: [PATCH v2 09/22] arm64: dts: qcom: pmk8350: allow overriding the label

[Krzysztof Kozlowski]

On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> sc8280xp-pmics.dtsi incorporates a copy of pmk8350, but the dts files
> use labels following the markings found in the schematics. Allow
> overriding the labels in pmk8350.dtsi.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 13 +++++++++
>  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 14 ++++++++++
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 28 +++++++++++--------
>  3 files changed, 44 insertions(+), 11 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> new file mode 100644
> index 000000000000..1c81269f0783
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +/* cleanly revert the effects pmic-dyn-header.dtsi */
> +
> +#undef LABEL
> +#undef _LABEL
> +#undef __LABEL

That makes the code less readable.

> +
> +#undef PMIC_SID
> +#undef PMIC_LABEL

And it keeps growing and confusing.

> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> new file mode 100644
> index 000000000000..75f0448568bd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +/*
> + * Preprocessor symbols to assist in describing PMICs which have configurable
> + * SID. All effects of this header (or its parameters) must be reverted in
> + * pmic-dyn-footer.dtsi.
> + */
> +
> +#define LABEL(name) _LABEL(PMIC_LABEL, name)
> +#define _LABEL(pmic, name) __LABEL(pmic, name)
> +#define __LABEL(pmic, name) pmic ## _ ## name

No, defines in DTS should be simple. This makes it ungrepable.



Best regards,
Krzysztof

Re: [PATCH v2 09/22] arm64: dts: qcom: pmk8350: allow overriding the label

[Dmitry Baryshkov]

On Sun, 2 Apr 2023 at 12:44, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> > sc8280xp-pmics.dtsi incorporates a copy of pmk8350, but the dts files
> > use labels following the markings found in the schematics. Allow
> > overriding the labels in pmk8350.dtsi.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 13 +++++++++
> >  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 14 ++++++++++
> >  arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 28 +++++++++++--------
> >  3 files changed, 44 insertions(+), 11 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> >  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> > new file mode 100644
> > index 000000000000..1c81269f0783
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +/* cleanly revert the effects pmic-dyn-header.dtsi */
> > +
> > +#undef LABEL
> > +#undef _LABEL
> > +#undef __LABEL
>
> That makes the code less readable.
>
> > +
> > +#undef PMIC_SID
> > +#undef PMIC_LABEL
>
> And it keeps growing and confusing.
>
> > diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> > new file mode 100644
> > index 000000000000..75f0448568bd
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +/*
> > + * Preprocessor symbols to assist in describing PMICs which have configurable
> > + * SID. All effects of this header (or its parameters) must be reverted in
> > + * pmic-dyn-footer.dtsi.
> > + */
> > +
> > +#define LABEL(name) _LABEL(PMIC_LABEL, name)
> > +#define _LABEL(pmic, name) __LABEL(pmic, name)
> > +#define __LABEL(pmic, name) pmic ## _ ## name
>
> No, defines in DTS should be simple. This makes it ungrepable.

I see. I still think that we should remove this duplication. Would you
prefer for me to define all pmk8350.dtsi labels individually? What
kind of approach could you possibly propose?


-- 
With best wishes
Dmitry

Re: [PATCH v2 09/22] arm64: dts: qcom: pmk8350: allow overriding the label

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> sc8280xp-pmics.dtsi incorporates a copy of pmk8350, but the dts files
> use labels following the markings found in the schematics. Allow
> overriding the labels in pmk8350.dtsi.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 13 +++++++++
>  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 14 ++++++++++
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 28 +++++++++++--------
>  3 files changed, 44 insertions(+), 11 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> new file mode 100644
> index 000000000000..1c81269f0783
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +/* cleanly revert the effects pmic-dyn-header.dtsi */
> +
> +#undef LABEL
> +#undef _LABEL
> +#undef __LABEL
> +
> +#undef PMIC_SID
> +#undef PMIC_LABEL
> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> new file mode 100644
> index 000000000000..75f0448568bd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +/*
> + * Preprocessor symbols to assist in describing PMICs which have configurable
> + * SID. All effects of this header (or its parameters) must be reverted in
> + * pmic-dyn-footer.dtsi.
> + */
> +
> +#define LABEL(name) _LABEL(PMIC_LABEL, name)
> +#define _LABEL(pmic, name) __LABEL(pmic, name)
> +#define __LABEL(pmic, name) pmic ## _ ## name
#define PMIC_LABEL pmk8350
#define LABEL(name) PMIC_LABEL##_##name

?

Konrad
> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> index 02e7518ab57b..f26ff3daf119 100644
> --- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> @@ -13,33 +13,39 @@
>  	#define PMIC_SID 0
>  #endif
>  
> +#ifndef PMIC_LABEL
> +#define PMIC_LABEL pmk8350
> +#endif
> +
> +#include "pmic-dyn-header.dtsi"
> +
>  &spmi_bus {
> -	pmk8350: pmic@PMIC_SID {
> +	PMIC_LABEL: pmic@PMIC_SID {
>  		compatible = "qcom,pmk8350", "qcom,spmi-pmic";
>  		reg = <PMIC_SID SPMI_USID>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -		pmk8350_pon: pon@1300 {
> +		LABEL(pon): pon@1300 {
>  			compatible = "qcom,pmk8350-pon";
>  			reg = <0x1300>, <0x800>;
>  			reg-names = "hlos", "pbs";
>  
> -			pmk8350_pon_pwrkey: pwrkey {
> +			LABEL(pon_pwrkey): pwrkey {
>  				compatible = "qcom,pmk8350-pwrkey";
>  				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
>  				linux,code = <KEY_POWER>;
>  				status = "disabled";
>  			};
>  
> -			pmk8350_pon_resin: resin {
> +			LABEL(pon_resin): resin {
>  				compatible = "qcom,pmk8350-resin";
>  				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
>  				status = "disabled";
>  			};
>  		};
>  
> -		pmk8350_vadc: adc@3100 {
> +		LABEL(vadc): adc@3100 {
>  			compatible = "qcom,spmi-adc7";
>  			reg = <0x3100>;
>  			#address-cells = <1>;
> @@ -48,7 +54,7 @@ pmk8350_vadc: adc@3100 {
>  			#io-channel-cells = <1>;
>  		};
>  
> -		pmk8350_adc_tm: adc-tm@3400 {
> +		LABEL(adc_tm): adc-tm@3400 {
>  			compatible = "qcom,adc-tm7";
>  			reg = <0x3400>;
>  			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> @@ -58,7 +64,7 @@ pmk8350_adc_tm: adc-tm@3400 {
>  			status = "disabled";
>  		};
>  
> -		pmk8350_rtc: rtc@6100 {
> +		LABEL(rtc): rtc@6100 {
>  			compatible = "qcom,pmk8350-rtc";
>  			reg = <0x6100>, <0x6200>;
>  			reg-names = "rtc", "alarm";
> @@ -66,7 +72,7 @@ pmk8350_rtc: rtc@6100 {
>  			status = "disabled";
>  		};
>  
> -		pmk8350_sdam_6: nvram@8500 {
> +		LABEL(sdam_6): nvram@8500 {
>  			compatible = "qcom,spmi-sdam";
>  			reg = <0x8500>;
>  			#address-cells = <1>;
> @@ -75,11 +81,11 @@ pmk8350_sdam_6: nvram@8500 {
>  			status = "disabled";
>  		};
>  
> -		pmk8350_gpios: gpio@b000 {
> +		LABEL(gpios): gpio@b000 {
>  			compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio";
>  			reg = <0xb000>;
>  			gpio-controller;
> -			gpio-ranges = <&pmk8350_gpios 0 0 4>;
> +			gpio-ranges = <&LABEL(gpios) 0 0 4>;
>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
> @@ -87,4 +93,4 @@ pmk8350_gpios: gpio@b000 {
>  	};
>  };
>  
> -#undef PMIC_SID
> +#include "pmic-dyn-footer.dtsi"

Re: [PATCH v2 09/22] arm64: dts: qcom: pmk8350: allow overriding the label

[Dmitry Baryshkov]

On Mon, 3 Apr 2023 at 13:14, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> > sc8280xp-pmics.dtsi incorporates a copy of pmk8350, but the dts files
> > use labels following the markings found in the schematics. Allow
> > overriding the labels in pmk8350.dtsi.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 13 +++++++++
> >  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 14 ++++++++++
> >  arch/arm64/boot/dts/qcom/pmk8350.dtsi         | 28 +++++++++++--------
> >  3 files changed, 44 insertions(+), 11 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> >  create mode 100644 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> > new file mode 100644
> > index 000000000000..1c81269f0783
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +/* cleanly revert the effects pmic-dyn-header.dtsi */
> > +
> > +#undef LABEL
> > +#undef _LABEL
> > +#undef __LABEL
> > +
> > +#undef PMIC_SID
> > +#undef PMIC_LABEL
> > diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> > new file mode 100644
> > index 000000000000..75f0448568bd
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +/*
> > + * Preprocessor symbols to assist in describing PMICs which have configurable
> > + * SID. All effects of this header (or its parameters) must be reverted in
> > + * pmic-dyn-footer.dtsi.
> > + */
> > +
> > +#define LABEL(name) _LABEL(PMIC_LABEL, name)
> > +#define _LABEL(pmic, name) __LABEL(pmic, name)
> > +#define __LABEL(pmic, name) pmic ## _ ## name
> #define PMIC_LABEL pmk8350
> #define LABEL(name) PMIC_LABEL##_##name

This doesn't work. Using LABEL(test) results in PMIC_LABEL_test.

>
> ?
>
> Konrad
> > diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> > index 02e7518ab57b..f26ff3daf119 100644
> > --- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> > @@ -13,33 +13,39 @@
> >       #define PMIC_SID 0
> >  #endif
> >
> > +#ifndef PMIC_LABEL
> > +#define PMIC_LABEL pmk8350
> > +#endif
> > +
> > +#include "pmic-dyn-header.dtsi"
> > +
> >  &spmi_bus {
> > -     pmk8350: pmic@PMIC_SID {
> > +     PMIC_LABEL: pmic@PMIC_SID {
> >               compatible = "qcom,pmk8350", "qcom,spmi-pmic";
> >               reg = <PMIC_SID SPMI_USID>;
> >               #address-cells = <1>;
> >               #size-cells = <0>;
> >
> > -             pmk8350_pon: pon@1300 {
> > +             LABEL(pon): pon@1300 {
> >                       compatible = "qcom,pmk8350-pon";
> >                       reg = <0x1300>, <0x800>;
> >                       reg-names = "hlos", "pbs";
> >
> > -                     pmk8350_pon_pwrkey: pwrkey {
> > +                     LABEL(pon_pwrkey): pwrkey {
> >                               compatible = "qcom,pmk8350-pwrkey";
> >                               interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
> >                               linux,code = <KEY_POWER>;
> >                               status = "disabled";
> >                       };
> >
> > -                     pmk8350_pon_resin: resin {
> > +                     LABEL(pon_resin): resin {
> >                               compatible = "qcom,pmk8350-resin";
> >                               interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
> >                               status = "disabled";
> >                       };
> >               };
> >
> > -             pmk8350_vadc: adc@3100 {
> > +             LABEL(vadc): adc@3100 {
> >                       compatible = "qcom,spmi-adc7";
> >                       reg = <0x3100>;
> >                       #address-cells = <1>;
> > @@ -48,7 +54,7 @@ pmk8350_vadc: adc@3100 {
> >                       #io-channel-cells = <1>;
> >               };
> >
> > -             pmk8350_adc_tm: adc-tm@3400 {
> > +             LABEL(adc_tm): adc-tm@3400 {
> >                       compatible = "qcom,adc-tm7";
> >                       reg = <0x3400>;
> >                       interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> > @@ -58,7 +64,7 @@ pmk8350_adc_tm: adc-tm@3400 {
> >                       status = "disabled";
> >               };
> >
> > -             pmk8350_rtc: rtc@6100 {
> > +             LABEL(rtc): rtc@6100 {
> >                       compatible = "qcom,pmk8350-rtc";
> >                       reg = <0x6100>, <0x6200>;
> >                       reg-names = "rtc", "alarm";
> > @@ -66,7 +72,7 @@ pmk8350_rtc: rtc@6100 {
> >                       status = "disabled";
> >               };
> >
> > -             pmk8350_sdam_6: nvram@8500 {
> > +             LABEL(sdam_6): nvram@8500 {
> >                       compatible = "qcom,spmi-sdam";
> >                       reg = <0x8500>;
> >                       #address-cells = <1>;
> > @@ -75,11 +81,11 @@ pmk8350_sdam_6: nvram@8500 {
> >                       status = "disabled";
> >               };
> >
> > -             pmk8350_gpios: gpio@b000 {
> > +             LABEL(gpios): gpio@b000 {
> >                       compatible = "qcom,pmk8350-gpio", "qcom,spmi-gpio";
> >                       reg = <0xb000>;
> >                       gpio-controller;
> > -                     gpio-ranges = <&pmk8350_gpios 0 0 4>;
> > +                     gpio-ranges = <&LABEL(gpios) 0 0 4>;
> >                       #gpio-cells = <2>;
> >                       interrupt-controller;
> >                       #interrupt-cells = <2>;
> > @@ -87,4 +93,4 @@ pmk8350_gpios: gpio@b000 {
> >       };
> >  };
> >
> > -#undef PMIC_SID
> > +#include "pmic-dyn-footer.dtsi"



-- 
With best wishes
Dmitry

[PATCH v2 10/22] arm64: dts: qcom: pmk8350: use interrupts-extended for IRQ specification

[Dmitry Baryshkov]

As stated in the commit 2d5cab9232ba ("arm64: dts: qcom: sc8280xp-pmics:
Specify interrupt parent explicitly"), we should not use bare interrupts
for our PMIC devices. Instead interrupts-extended should be used. Change
pmk8350.dtsi to use interrupts-extended propery.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pmk8350.dtsi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
index f26ff3daf119..87b8e4269c60 100644
--- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
@@ -33,14 +33,14 @@ LABEL(pon): pon@1300 {
 
 			LABEL(pon_pwrkey): pwrkey {
 				compatible = "qcom,pmk8350-pwrkey";
-				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
+				interrupts-extended = <&spmi_bus PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
 				linux,code = <KEY_POWER>;
 				status = "disabled";
 			};
 
 			LABEL(pon_resin): resin {
 				compatible = "qcom,pmk8350-resin";
-				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
+				interrupts-extended = <&spmi_bus PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
 				status = "disabled";
 			};
 		};
@@ -50,14 +50,14 @@ LABEL(vadc): adc@3100 {
 			reg = <0x3100>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts-extended = <&spmi_bus PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 			#io-channel-cells = <1>;
 		};
 
 		LABEL(adc_tm): adc-tm@3400 {
 			compatible = "qcom,adc-tm7";
 			reg = <0x3400>;
-			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts-extended = <&spmi_bus PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#thermal-sensor-cells = <1>;
@@ -68,7 +68,7 @@ LABEL(rtc): rtc@6100 {
 			compatible = "qcom,pmk8350-rtc";
 			reg = <0x6100>, <0x6200>;
 			reg-names = "rtc", "alarm";
-			interrupts = <PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupts-extended = <&spmi_bus PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
 			status = "disabled";
 		};
 
-- 
2.30.2

Re: [PATCH v2 10/22] arm64: dts: qcom: pmk8350: use interrupts-extended for IRQ specification

[Konrad Dybcio]

On 2.04.2023 00:07, Dmitry Baryshkov wrote:
> As stated in the commit 2d5cab9232ba ("arm64: dts: qcom: sc8280xp-pmics:
> Specify interrupt parent explicitly"), we should not use bare interrupts
> for our PMIC devices. Instead interrupts-extended should be used. Change
> pmk8350.dtsi to use interrupts-extended propery.
propery/property

It's not about using the interrupts-extended itself, as it's functionally
identical if it points to the same interrupt parent. The main point of this
is to designate the SPMI host as the interrupt provider for these nodes,
as that's what the DT spec expects.

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
For the change:

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/pmk8350.dtsi | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pmk8350.dtsi b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> index f26ff3daf119..87b8e4269c60 100644
> --- a/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmk8350.dtsi
> @@ -33,14 +33,14 @@ LABEL(pon): pon@1300 {
>  
>  			LABEL(pon_pwrkey): pwrkey {
>  				compatible = "qcom,pmk8350-pwrkey";
> -				interrupts = <PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts-extended = <&spmi_bus PMIC_SID 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
>  				linux,code = <KEY_POWER>;
>  				status = "disabled";
>  			};
>  
>  			LABEL(pon_resin): resin {
>  				compatible = "qcom,pmk8350-resin";
> -				interrupts = <PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts-extended = <&spmi_bus PMIC_SID 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
>  				status = "disabled";
>  			};
>  		};
> @@ -50,14 +50,14 @@ LABEL(vadc): adc@3100 {
>  			reg = <0x3100>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts-extended = <&spmi_bus PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#io-channel-cells = <1>;
>  		};
>  
>  		LABEL(adc_tm): adc-tm@3400 {
>  			compatible = "qcom,adc-tm7";
>  			reg = <0x3400>;
> -			interrupts = <PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> +			interrupts-extended = <&spmi_bus PMIC_SID 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			#thermal-sensor-cells = <1>;
> @@ -68,7 +68,7 @@ LABEL(rtc): rtc@6100 {
>  			compatible = "qcom,pmk8350-rtc";
>  			reg = <0x6100>, <0x6200>;
>  			reg-names = "rtc", "alarm";
> -			interrupts = <PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
> +			interrupts-extended = <&spmi_bus PMIC_SID 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
>  			status = "disabled";
>  		};
>  

[PATCH v2 11/22] arm64: dts: qcom: sc8280xp*: use pmk8350.dtsi

[Dmitry Baryshkov]

Employ PMIC_LABEL and switch sc8280xp-pmics to use pmk8350.dtsi in order
to reduce duplication and possible discrepancies.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 67 ++------------------
 1 file changed, 4 insertions(+), 63 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 254337345e45..6c183b6c9207 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -51,70 +51,11 @@ trip1 {
 	};
 };
 
-&spmi_bus {
-	pmk8280: pmic@0 {
-		compatible = "qcom,pmk8350", "qcom,spmi-pmic";
-		reg = <0x0 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmk8280_pon: pon@1300 {
-			compatible = "qcom,pmk8350-pon";
-			reg = <0x1300>, <0x800>;
-
-			pmk8280_pon_pwrkey: pwrkey {
-				compatible = "qcom,pmk8350-pwrkey";
-				interrupts-extended = <&spmi_bus 0x0 0x13 0x7 IRQ_TYPE_EDGE_BOTH>;
-				linux,code = <KEY_POWER>;
-				status = "disabled";
-			};
-
-			pmk8280_pon_resin: resin {
-				compatible = "qcom,pmk8350-resin";
-				interrupts-extended = <&spmi_bus 0x0 0x13 0x6 IRQ_TYPE_EDGE_BOTH>;
-				status = "disabled";
-			};
-		};
-
-		pmk8280_vadc: adc@3100 {
-			compatible = "qcom,spmi-adc7";
-			reg = <0x3100>;
-			interrupts-extended = <&spmi_bus 0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
-			#address-cells = <1>;
-			#size-cells = <0>;
-			#io-channel-cells = <1>;
-			status = "disabled";
-		};
-
-		pmk8280_adc_tm: adc-tm@3400 {
-			compatible = "qcom,spmi-adc-tm5-gen2";
-			reg = <0x3400>;
-			interrupts-extended = <&spmi_bus 0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
-			#address-cells = <1>;
-			#size-cells = <0>;
-			#thermal-sensor-cells = <1>;
-			status = "disabled";
-		};
-
-		pmk8280_rtc: rtc@6100 {
-			compatible = "qcom,pmk8350-rtc";
-			reg = <0x6100>, <0x6200>;
-			reg-names = "rtc", "alarm";
-			interrupts = <0x0 0x62 0x1 IRQ_TYPE_EDGE_RISING>;
-			wakeup-source;
-			status = "disabled";
-		};
-
-		pmk8280_sdam_6: nvram@8500 {
-			compatible = "qcom,spmi-sdam";
-			reg = <0x8500>;
-			#address-cells = <1>;
-			#size-cells = <1>;
-			ranges = <0 0x8500 0x100>;
-			status = "disabled";
-		};
-	};
+/* SID 0 */
+#define PMIC_LABEL pmk8280
+#include "pmk8350.dtsi"
 
+&spmi_bus {
 	pmc8280_1: pmic@1 {
 		compatible = "qcom,pm8350", "qcom,spmi-pmic";
 		reg = <0x1 SPMI_USID>;
-- 
2.30.2

[PATCH v2 12/22] arm64: dts: qcom: pm8350: allow overriding SID and label

[Dmitry Baryshkov]

The platform can use several instances of PM8350 PMIC. Allow overriding
the SID and node labels inside pm8350.dtsi.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350.dtsi          | 29 ++++++++++++++-----
 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  3 ++
 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  6 ++++
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350.dtsi b/arch/arm64/boot/dts/qcom/pm8350.dtsi
index 9fb963df4b41..b7b14b5fe67d 100644
--- a/arch/arm64/boot/dts/qcom/pm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350.dtsi
@@ -6,12 +6,23 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
+/* (Sadly) this PMIC can be configured to be at different SIDs */
+#ifndef PMIC_SID
+	#define PMIC_SID 1
+#endif
+
+#ifndef PMIC_LABEL
+	#define PMIC_LABEL pm8350
+#endif
+
+#include "pmic-dyn-header.dtsi"
+
 / {
 	thermal-zones {
-		pm8350-thermal {
+		NODE(thermal) {
 			polling-delay-passive = <100>;
 			polling-delay = <0>;
-			thermal-sensors = <&pm8350_temp_alarm>;
+			thermal-sensors = <&LABEL(temp_alarm)>;
 
 			trips {
 				trip0 {
@@ -31,27 +42,29 @@ crit {
 };
 
 &spmi_bus {
-	pm8350: pmic@1 {
+	PMIC_LABEL: pmic@PMIC_SID {
 		compatible = "qcom,pm8350", "qcom,spmi-pmic";
-		reg = <0x1 SPMI_USID>;
+		reg = <PMIC_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		pm8350_temp_alarm: temp-alarm@a00 {
+		LABEL(temp_alarm): temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
-			interrupts = <0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts = <PMIC_SID 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
 			#thermal-sensor-cells = <0>;
 		};
 
-		pm8350_gpios: gpio@8800 {
+		LABEL(gpios): gpio@8800 {
 			compatible = "qcom,pm8350-gpio", "qcom,spmi-gpio";
 			reg = <0x8800>;
 			gpio-controller;
-			gpio-ranges = <&pm8350_gpios 0 0 10>;
+			gpio-ranges = <&LABEL(gpios) 0 0 10>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 		};
 	};
 };
+
+#include "pmic-dyn-footer.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
index 1c81269f0783..29b10e7fb8c3 100644
--- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
@@ -9,5 +9,8 @@
 #undef _LABEL
 #undef __LABEL
 
+#undef NODE
+
 #undef PMIC_SID
 #undef PMIC_LABEL
+#undef PMIC_NODE
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
index 75f0448568bd..bb41c9387aba 100644
--- a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
@@ -9,6 +9,12 @@
  * pmic-dyn-footer.dtsi.
  */
 
+#ifndef PMIC_NODE
+	#define PMIC_NODE PMIC_LABEL
+#endif
+
 #define LABEL(name) _LABEL(PMIC_LABEL, name)
 #define _LABEL(pmic, name) __LABEL(pmic, name)
 #define __LABEL(pmic, name) pmic ## _ ## name
+
+#define NODE(name) PMIC_NODE ##-##  name
-- 
2.30.2

[PATCH v2 13/22] arm64: dts: qcom: pm8350: use interrupts-extended for IRQ specification

[Dmitry Baryshkov]

As stated in the commit 2d5cab9232ba ("arm64: dts: qcom: sc8280xp-pmics:
Specify interrupt parent explicitly"), we should not use bare interrupts
for our PMIC devices. Instead interrupts-extended should be used. Change
pm8350.dtsi to use interrupts-extended propery.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350.dtsi b/arch/arm64/boot/dts/qcom/pm8350.dtsi
index b7b14b5fe67d..b05bc01e3ef5 100644
--- a/arch/arm64/boot/dts/qcom/pm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350.dtsi
@@ -51,7 +51,7 @@ PMIC_LABEL: pmic@PMIC_SID {
 		LABEL(temp_alarm): temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
-			interrupts = <PMIC_SID 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts-extended = <&spmi_bus PMIC_SID 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
 			#thermal-sensor-cells = <0>;
 		};
 
-- 
2.30.2

[PATCH v2 14/22] arm64: dts: qcom: sc8280xp*: use pm8350.dtsi

[Dmitry Baryshkov]

Employ PMIC_SID/PMIC_LABEL and switch sc8280xp-pmics to use pm8350.dtsi
in order to reduce duplication and possible discrepancies.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 100 ++-----------------
 1 file changed, 9 insertions(+), 91 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 6c183b6c9207..69f79ed87f62 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -7,79 +7,21 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8280_1_thermal: pm8280-1-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-			thermal-sensors = <&pm8280_1_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "critical";
-				};
-			};
-		};
-
-		pm8280_2_thermal: pm8280-2-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-			thermal-sensors = <&pm8280_2_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "critical";
-				};
-			};
-		};
-	};
-};
-
 /* SID 0 */
 #define PMIC_LABEL pmk8280
 #include "pmk8350.dtsi"
 
-&spmi_bus {
-	pmc8280_1: pmic@1 {
-		compatible = "qcom,pm8350", "qcom,spmi-pmic";
-		reg = <0x1 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
+#define PMIC_SID 1
+#define PMIC_LABEL pmc8280_1
+#define PMIC_NODE pmc82801
+#include "pm8350.dtsi"
 
-		pm8280_1_temp_alarm: temp-alarm@a00 {
-			compatible = "qcom,spmi-temp-alarm";
-			reg = <0xa00>;
-			interrupts-extended = <&spmi_bus 0x1 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
-			#thermal-sensor-cells = <0>;
-		};
-
-		pmc8280_1_gpios: gpio@8800 {
-			compatible = "qcom,pm8350-gpio", "qcom,spmi-gpio";
-			reg = <0x8800>;
-			gpio-controller;
-			gpio-ranges = <&pmc8280_1_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
+#define PMIC_SID 3
+#define PMIC_LABEL pmc8280_2
+#define PMIC_NODE pmc82802
+#include "pm8350.dtsi"
 
+&spmi_bus {
 	pmc8280c: pmic@2 {
 		compatible = "qcom,pm8350c", "qcom,spmi-pmic";
 		reg = <0x2 SPMI_USID>;
@@ -108,30 +50,6 @@ pmc8280c_lpg: pwm {
 		};
 	};
 
-	pmc8280_2: pmic@3 {
-		compatible = "qcom,pm8350", "qcom,spmi-pmic";
-		reg = <0x3 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pm8280_2_temp_alarm: temp-alarm@a00 {
-			compatible = "qcom,spmi-temp-alarm";
-			reg = <0xa00>;
-			interrupts-extended = <&spmi_bus 0x3 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
-			#thermal-sensor-cells = <0>;
-		};
-
-		pmc8280_2_gpios: gpio@8800 {
-			compatible = "qcom,pm8350-gpio", "qcom,spmi-gpio";
-			reg = <0x8800>;
-			gpio-controller;
-			gpio-ranges = <&pmc8280_2_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-
 	pmr735a: pmic@4 {
 		compatible = "qcom,pmr735a", "qcom,spmi-pmic";
 		reg = <0x4 SPMI_USID>;
-- 
2.30.2

[PATCH v2 15/22] arm64: dts: qcom: pm8350c: move thermal zone declaration to the top

[Dmitry Baryshkov]

Other PMIC files have thermal zone at the top of the file. Move the
pm8350c's thermal zone to the top. While we are at it, drop unused
labels.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350c.dtsi | 48 +++++++++++++--------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index f28e71487d5c..ee25c07ce31d 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -6,6 +6,30 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
+/ {
+	thermal-zones {
+		pm8350c-thermal {
+			polling-delay-passive = <100>;
+			polling-delay = <0>;
+			thermal-sensors = <&pm8350c_temp_alarm>;
+
+			trips {
+				trip0 {
+					temperature = <95000>;
+					hysteresis = <0>;
+					type = "passive";
+				};
+
+				pm8350c-crit {
+					temperature = <115000>;
+					hysteresis = <0>;
+					type = "critical";
+				};
+			};
+		};
+	};
+};
+
 &spmi_bus {
 	pm8350c: pmic@2 {
 		compatible = "qcom,pm8350c", "qcom,spmi-pmic";
@@ -37,27 +61,3 @@ pm8350c_pwm: pwm {
 		};
 	};
 };
-
-/ {
-	thermal-zones {
-		pm8350c_thermal: pm8350c-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-			thermal-sensors = <&pm8350c_temp_alarm>;
-
-			trips {
-				pm8350c_trip0: trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				pm8350c_crit: pm8350c-crit {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "critical";
-				};
-			};
-		};
-	};
-};
-- 
2.30.2

[PATCH v2 16/22] arm64: dts: qcom: pm8350c: allow overriding SID and label

[Dmitry Baryshkov]

The platform can use several instances of PM8350C PMIC. Allow overriding
the SID and node labels inside pm8350c.dtsi.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350c.dtsi | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index ee25c07ce31d..ddfc8fd6bb3e 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -6,12 +6,23 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
+/* (Sadly) this PMIC can be configured to be at different SIDs */
+#ifndef PMIC_SID
+	#define PMIC_SID 2
+#endif
+
+#ifndef PMIC_LABEL
+	#define PMIC_LABEL pm8350c
+#endif
+
+#include "pmic-dyn-header.dtsi"
+
 / {
 	thermal-zones {
 		pm8350c-thermal {
 			polling-delay-passive = <100>;
 			polling-delay = <0>;
-			thermal-sensors = <&pm8350c_temp_alarm>;
+			thermal-sensors = <&LABEL(temp_alarm)>;
 
 			trips {
 				trip0 {
@@ -31,33 +42,35 @@ pm8350c-crit {
 };
 
 &spmi_bus {
-	pm8350c: pmic@2 {
+	pmic@PMIC_SID {
 		compatible = "qcom,pm8350c", "qcom,spmi-pmic";
-		reg = <0x2 SPMI_USID>;
+		reg = <PMIC_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		pm8350c_temp_alarm: temp-alarm@a00 {
+		LABEL(temp_alarm): temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
 			interrupts = <0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
 			#thermal-sensor-cells = <0>;
 		};
 
-		pm8350c_gpios: gpio@8800 {
+		LABEL(gpios): gpio@8800 {
 			compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio";
 			reg = <0x8800>;
 			gpio-controller;
-			gpio-ranges = <&pm8350c_gpios 0 0 9>;
+			gpio-ranges = <&LABEL(gpios) 0 0 9>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 		};
 
-		pm8350c_pwm: pwm {
+		LABEL(pwm): pwm {
 			compatible = "qcom,pm8350c-pwm";
 			#pwm-cells = <2>;
 			status = "disabled";
 		};
 	};
 };
+
+#include "pmic-dyn-footer.dtsi"
-- 
2.30.2

[PATCH v2 17/22] arm64: dts: qcom: pm8350c: use interrupts-extended for IRQ specification

[Dmitry Baryshkov]

As stated in the commit 2d5cab9232ba ("arm64: dts: qcom: sc8280xp-pmics:
Specify interrupt parent explicitly"), we should not use bare interrupts
for our PMIC devices. Instead interrupts-extended should be used. Change
pm8350.dtsi to use interrupts-extended propery.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8350c.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index ddfc8fd6bb3e..eef36900dee6 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -51,7 +51,7 @@ pmic@PMIC_SID {
 		LABEL(temp_alarm): temp-alarm@a00 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0xa00>;
-			interrupts = <0x2 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts-extended = <&spmi_bus PMIC_SID 0xa 0x0 IRQ_TYPE_EDGE_BOTH>;
 			#thermal-sensor-cells = <0>;
 		};
 
-- 
2.30.2

[PATCH v2 18/22] arm64: dts: qcom: sc8280xp*: use pm8350c.dtsi

[Dmitry Baryshkov]

Employ PMIC_SID/_LABEL and switch sc8280xp-pmics to use pm8350c.dtsi in
order to reduce duplication and possible discrepancies.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts     |  4 +--
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |  4 +--
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi  | 32 +++----------------
 3 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index d7b537284990..18f821d99434 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -24,7 +24,7 @@ aliases {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pmc8280c_lpg 3 1000000>;
+		pwms = <&pmc8280c_pwm 3 1000000>;
 		enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>;
 		power-supply = <&vreg_edp_bl>;
 
@@ -555,7 +555,7 @@ &pcie4_phy {
 	status = "okay";
 };
 
-&pmc8280c_lpg {
+&pmc8280c_pwm {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 590400985055..c74d32f7931e 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -54,7 +54,7 @@ wcd938x: audio-codec {
 
 	backlight: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pmc8280c_lpg 3 1000000>;
+		pwms = <&pmc8280c_pwm 3 1000000>;
 		enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>;
 		power-supply = <&vreg_edp_bl>;
 
@@ -725,7 +725,7 @@ &pcie4_phy {
 	status = "okay";
 };
 
-&pmc8280c_lpg {
+&pmc8280c_pwm {
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 69f79ed87f62..3d2914d13fd2 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -16,40 +16,16 @@
 #define PMIC_NODE pmc82801
 #include "pm8350.dtsi"
 
+#define PMIC_SID 2
+#define PMIC_LABEL pmc8280c
+#include "pm8350c.dtsi"
+
 #define PMIC_SID 3
 #define PMIC_LABEL pmc8280_2
 #define PMIC_NODE pmc82802
 #include "pm8350.dtsi"
 
 &spmi_bus {
-	pmc8280c: pmic@2 {
-		compatible = "qcom,pm8350c", "qcom,spmi-pmic";
-		reg = <0x2 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmc8280c_gpios: gpio@8800 {
-			compatible = "qcom,pm8350c-gpio", "qcom,spmi-gpio";
-			reg = <0x8800>;
-			gpio-controller;
-			gpio-ranges = <&pmc8280c_gpios 0 0 9>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-
-		pmc8280c_lpg: pwm {
-			compatible = "qcom,pm8350c-pwm";
-
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			#pwm-cells = <2>;
-
-			status = "disabled";
-		};
-	};
-
 	pmr735a: pmic@4 {
 		compatible = "qcom,pmr735a", "qcom,spmi-pmic";
 		reg = <0x4 SPMI_USID>;
-- 
2.30.2

[PATCH v2 19/22] arm64: dts: qcom: sc8280xp*: use pmr735a.dtsi

[Dmitry Baryshkov]

Stop duplicating declarations of pmr735a PMICs, use existing file to
prevent possible future issues.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index 3d2914d13fd2..7a1298a71791 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -25,21 +25,5 @@
 #define PMIC_NODE pmc82802
 #include "pm8350.dtsi"
 
-&spmi_bus {
-	pmr735a: pmic@4 {
-		compatible = "qcom,pmr735a", "qcom,spmi-pmic";
-		reg = <0x4 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmr735a_gpios: gpio@8800 {
-			compatible = "qcom,pmr735a-gpio", "qcom,spmi-gpio";
-			reg = <0x8800>;
-			gpio-controller;
-			gpio-ranges = <&pmr735a_gpios 0 0 4>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-};
+/* SID 4 */
+#include "pmr735a.dtsi"
-- 
2.30.2

[PATCH v2 20/22] arm64: dts: qcom: pm8150: convert to use dynamic SID/LABEL

[Dmitry Baryshkov]

SA8450p-based platforms have 4 instances of pm8150. Convert pm8150.dtsi
to use pmic-dyn-header.dtsi in order to support dynamic and label
assignment.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8150.dtsi          | 53 ++++++++++++-------
 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  1 +
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
index db90c55fa2cf..77bb325e425b 100644
--- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
@@ -9,13 +9,28 @@
 #include <dt-bindings/spmi/spmi.h>
 #include <dt-bindings/iio/qcom,spmi-vadc.h>
 
+/* (Sadly) this PMIC can be configured to be at different SIDs */
+#ifndef PMIC_SID
+	#define PMIC_SID 0
+#endif
+
+#ifndef PMIC_SID1
+	#define PMIC_SID1 1
+#endif
+
+#ifndef PMIC_LABEL
+	#define PMIC_LABEL pm8150
+#endif
+
+#include "pmic-dyn-header.dtsi"
+
 / {
 	thermal-zones {
-		pm8150-thermal {
+		NODE(thermal) {
 			polling-delay-passive = <100>;
 			polling-delay = <0>;
 
-			thermal-sensors = <&pm8150_temp>;
+			thermal-sensors = <&LABEL(temp)>;
 
 			trips {
 				trip0 {
@@ -41,9 +56,9 @@ trip2 {
 };
 
 &spmi_bus {
-	pm8150_0: pmic@0 {
+	pmic@0 {
 		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0x0 SPMI_USID>;
+		reg = <PMIC_SID SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
@@ -55,7 +70,7 @@ pon: pon@800 {
 
 			pon_pwrkey: pwrkey {
 				compatible = "qcom,pm8941-pwrkey";
-				interrupts = <0x0 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
 				debounce = <15625>;
 				bias-pull-up;
 				linux,code = <KEY_POWER>;
@@ -65,7 +80,7 @@ pon_pwrkey: pwrkey {
 
 			pon_resin: resin {
 				compatible = "qcom,pm8941-resin";
-				interrupts = <0x0 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
 				debounce = <15625>;
 				bias-pull-up;
 
@@ -73,22 +88,22 @@ pon_resin: resin {
 			};
 		};
 
-		pm8150_temp: temp-alarm@2400 {
+		LABEL(temp): temp-alarm@2400 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0x2400>;
-			interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
-			io-channels = <&pm8150_adc ADC5_DIE_TEMP>;
+			interrupts = <PMIC_SID 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
+			io-channels = <&LABEL(adc) ADC5_DIE_TEMP>;
 			io-channel-names = "thermal";
 			#thermal-sensor-cells = <0>;
 		};
 
-		pm8150_adc: adc@3100 {
+		LABEL(adc): adc@3100 {
 			compatible = "qcom,spmi-adc5";
 			reg = <0x3100>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#io-channel-cells = <1>;
-			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 
 			ref-gnd@0 {
 				reg = <ADC5_REF_GND>;
@@ -109,10 +124,10 @@ die-temp@6 {
 			};
 		};
 
-		pm8150_adc_tm: adc-tm@3500 {
+		LABEL(adc_tm): adc-tm@3500 {
 			compatible = "qcom,spmi-adc-tm5";
 			reg = <0x3500>;
-			interrupts = <0x0 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
 			#thermal-sensor-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -123,24 +138,26 @@ rtc@6000 {
 			compatible = "qcom,pm8941-rtc";
 			reg = <0x6000>, <0x6100>;
 			reg-names = "rtc", "alarm";
-			interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
+			interrupts = <PMIC_SID 0x61 0x1 IRQ_TYPE_NONE>;
 		};
 
-		pm8150_gpios: gpio@c000 {
+		LABEL(gpios): gpio@c000 {
 			compatible = "qcom,pm8150-gpio", "qcom,spmi-gpio";
 			reg = <0xc000>;
 			gpio-controller;
-			gpio-ranges = <&pm8150_gpios 0 0 10>;
+			gpio-ranges = <&LABEL(gpios) 0 0 10>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 		};
 	};
 
-	pmic@1 {
+	pmic@PMIC_SID1 {
 		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0x1 SPMI_USID>;
+		reg = <PMIC_SID1 SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};
 };
+
+#include "pmic-dyn-footer.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
index 29b10e7fb8c3..83a2bada48ff 100644
--- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
@@ -12,5 +12,6 @@
 #undef NODE
 
 #undef PMIC_SID
+#undef PMIC_SID1
 #undef PMIC_LABEL
 #undef PMIC_NODE
-- 
2.30.2

Re: [PATCH v2 20/22] arm64: dts: qcom: pm8150: convert to use dynamic SID/LABEL

[Krzysztof Kozlowski]

On 02/04/2023 00:08, Dmitry Baryshkov wrote:
> SA8450p-based platforms have 4 instances of pm8150. Convert pm8150.dtsi
> to use pmic-dyn-header.dtsi in order to support dynamic and label
> assignment.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8150.dtsi          | 53 ++++++++++++-------
>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  1 +
>  2 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> index db90c55fa2cf..77bb325e425b 100644
> --- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> @@ -9,13 +9,28 @@
>  #include <dt-bindings/spmi/spmi.h>
>  #include <dt-bindings/iio/qcom,spmi-vadc.h>
>  
> +/* (Sadly) this PMIC can be configured to be at different SIDs */
> +#ifndef PMIC_SID
> +	#define PMIC_SID 0
> +#endif

No, the DTS code must be simple, no ifndefs for some defines. This means
that sometimes you expect here define, sometimes not. It's not easy to
maintain and understand the code. Define must be simple and always
defined, not sometimes.

> +
> +#ifndef PMIC_SID1
> +	#define PMIC_SID1 1
> +#endif
> +
> +#ifndef PMIC_LABEL
> +	#define PMIC_LABEL pm8150
> +#endif
> +
> +#include "pmic-dyn-header.dtsi"
> +
>  / {
>  	thermal-zones {
> -		pm8150-thermal {
> +		NODE(thermal) {
>  			polling-delay-passive = <100>;
>  			polling-delay = <0>;
>  
> -			thermal-sensors = <&pm8150_temp>;
> +			thermal-sensors = <&LABEL(temp)>;
>  
>  			trips {
>  				trip0 {
> @@ -41,9 +56,9 @@ trip2 {
>  };
>  
>  &spmi_bus {
> -	pm8150_0: pmic@0 {
> +	pmic@0 {
>  		compatible = "qcom,pm8150", "qcom,spmi-pmic";
> -		reg = <0x0 SPMI_USID>;
> +		reg = <PMIC_SID SPMI_USID>;
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> @@ -55,7 +70,7 @@ pon: pon@800 {
>  
>  			pon_pwrkey: pwrkey {
>  				compatible = "qcom,pm8941-pwrkey";
> -				interrupts = <0x0 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts = <PMIC_SID 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
>  				debounce = <15625>;
>  				bias-pull-up;
>  				linux,code = <KEY_POWER>;
> @@ -65,7 +80,7 @@ pon_pwrkey: pwrkey {
>  
>  			pon_resin: resin {
>  				compatible = "qcom,pm8941-resin";
> -				interrupts = <0x0 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
> +				interrupts = <PMIC_SID 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
>  				debounce = <15625>;
>  				bias-pull-up;
>  
> @@ -73,22 +88,22 @@ pon_resin: resin {
>  			};
>  		};
>  
> -		pm8150_temp: temp-alarm@2400 {
> +		LABEL(temp): temp-alarm@2400 {

NAK for all defines creating labels.


Best regards,
Krzysztof

[PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Dmitry Baryshkov]

Supporting SIDs greater than 9 required additional handling in order to
properly generatae hex values. Apply this customization to pm8150.dtsi.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8150.dtsi          | 16 ++++++++--------
 arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  6 ++++++
 arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  6 ++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
index 77bb325e425b..37cc99e5d1a6 100644
--- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
@@ -58,7 +58,7 @@ trip2 {
 &spmi_bus {
 	pmic@0 {
 		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <PMIC_SID SPMI_USID>;
+		reg = <PMIC_SID_HEX SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 
@@ -70,7 +70,7 @@ pon: pon@800 {
 
 			pon_pwrkey: pwrkey {
 				compatible = "qcom,pm8941-pwrkey";
-				interrupts = <PMIC_SID 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID_HEX 0x8 0x0 IRQ_TYPE_EDGE_BOTH>;
 				debounce = <15625>;
 				bias-pull-up;
 				linux,code = <KEY_POWER>;
@@ -80,7 +80,7 @@ pon_pwrkey: pwrkey {
 
 			pon_resin: resin {
 				compatible = "qcom,pm8941-resin";
-				interrupts = <PMIC_SID 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
+				interrupts = <PMIC_SID_HEX 0x8 0x1 IRQ_TYPE_EDGE_BOTH>;
 				debounce = <15625>;
 				bias-pull-up;
 
@@ -91,7 +91,7 @@ pon_resin: resin {
 		LABEL(temp): temp-alarm@2400 {
 			compatible = "qcom,spmi-temp-alarm";
 			reg = <0x2400>;
-			interrupts = <PMIC_SID 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
+			interrupts = <PMIC_SID_HEX 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
 			io-channels = <&LABEL(adc) ADC5_DIE_TEMP>;
 			io-channel-names = "thermal";
 			#thermal-sensor-cells = <0>;
@@ -103,7 +103,7 @@ LABEL(adc): adc@3100 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#io-channel-cells = <1>;
-			interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID_HEX 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 
 			ref-gnd@0 {
 				reg = <ADC5_REF_GND>;
@@ -127,7 +127,7 @@ die-temp@6 {
 		LABEL(adc_tm): adc-tm@3500 {
 			compatible = "qcom,spmi-adc-tm5";
 			reg = <0x3500>;
-			interrupts = <PMIC_SID 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
+			interrupts = <PMIC_SID_HEX 0x35 0x0 IRQ_TYPE_EDGE_RISING>;
 			#thermal-sensor-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -138,7 +138,7 @@ rtc@6000 {
 			compatible = "qcom,pm8941-rtc";
 			reg = <0x6000>, <0x6100>;
 			reg-names = "rtc", "alarm";
-			interrupts = <PMIC_SID 0x61 0x1 IRQ_TYPE_NONE>;
+			interrupts = <PMIC_SID_HEX 0x61 0x1 IRQ_TYPE_NONE>;
 		};
 
 		LABEL(gpios): gpio@c000 {
@@ -154,7 +154,7 @@ LABEL(gpios): gpio@c000 {
 
 	pmic@PMIC_SID1 {
 		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <PMIC_SID1 SPMI_USID>;
+		reg = <PMIC_SID1_HEX SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
index 83a2bada48ff..f3743ef3aa13 100644
--- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
@@ -11,6 +11,12 @@
 
 #undef NODE
 
+#undef HEX
+#undef _HEX
+
+#undef PMIC_SID_HEX
+#undef PMIC_SID1_HEX
+
 #undef PMIC_SID
 #undef PMIC_SID1
 #undef PMIC_LABEL
diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
index bb41c9387aba..640d1bf5ce8e 100644
--- a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi
@@ -18,3 +18,9 @@
 #define __LABEL(pmic, name) pmic ## _ ## name
 
 #define NODE(name) PMIC_NODE ##-##  name
+
+#define HEX(sid) _HEX(sid)
+#define _HEX(sid) 0x## sid
+
+#define PMIC_SID_HEX HEX(PMIC_SID)
+#define PMIC_SID1_HEX HEX(PMIC_SID1)
-- 
2.30.2

Re: [PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Krzysztof Kozlowski]

On 02/04/2023 00:08, Dmitry Baryshkov wrote:
> Supporting SIDs greater than 9 required additional handling in order to
> properly generatae hex values. Apply this customization to pm8150.dtsi.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8150.dtsi          | 16 ++++++++--------
>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  6 ++++++
>  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  6 ++++++
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 

> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> index 83a2bada48ff..f3743ef3aa13 100644
> --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> @@ -11,6 +11,12 @@
>  
>  #undef NODE
>  
> +#undef HEX
> +#undef _HEX
> +
> +#undef PMIC_SID_HEX
> +#undef PMIC_SID1_HEX
> +
>  #undef PMIC_SID
>  #undef PMIC_SID1
>  #undef PMIC_LABEL

Same comment as for previous patches - all undefs must be gone.

Maybe I should not have acked all these changes customized SID ("include
sid into defines") because it looks like it opened can of worms.

Best regards,
Krzysztof

Re: [PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Konrad Dybcio]

On 2.04.2023 11:51, Krzysztof Kozlowski wrote:
> On 02/04/2023 00:08, Dmitry Baryshkov wrote:
>> Supporting SIDs greater than 9 required additional handling in order to
>> properly generatae hex values. Apply this customization to pm8150.dtsi.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>  arch/arm64/boot/dts/qcom/pm8150.dtsi          | 16 ++++++++--------
>>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  6 ++++++
>>  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  6 ++++++
>>  3 files changed, 20 insertions(+), 8 deletions(-)
>>
> 
>> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
>> index 83a2bada48ff..f3743ef3aa13 100644
>> --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
>> @@ -11,6 +11,12 @@
>>  
>>  #undef NODE
>>  
>> +#undef HEX
>> +#undef _HEX
>> +
>> +#undef PMIC_SID_HEX
>> +#undef PMIC_SID1_HEX
All decimal numbers can be represented as hex numbers..
Is there any point to keeping them separate?

Konrad
>> +
>>  #undef PMIC_SID
>>  #undef PMIC_SID1
>>  #undef PMIC_LABEL
> 
> Same comment as for previous patches - all undefs must be gone.
> 
> Maybe I should not have acked all these changes customized SID ("include
> sid into defines") because it looks like it opened can of worms.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Dmitry Baryshkov]

On Mon, 3 Apr 2023 at 13:35, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 2.04.2023 11:51, Krzysztof Kozlowski wrote:
> > On 02/04/2023 00:08, Dmitry Baryshkov wrote:
> >> Supporting SIDs greater than 9 required additional handling in order to
> >> properly generatae hex values. Apply this customization to pm8150.dtsi.
> >>
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> ---
> >>  arch/arm64/boot/dts/qcom/pm8150.dtsi          | 16 ++++++++--------
> >>  arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi |  6 ++++++
> >>  arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi |  6 ++++++
> >>  3 files changed, 20 insertions(+), 8 deletions(-)
> >>
> >
> >> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> >> index 83a2bada48ff..f3743ef3aa13 100644
> >> --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi
> >> @@ -11,6 +11,12 @@
> >>
> >>  #undef NODE
> >>
> >> +#undef HEX
> >> +#undef _HEX
> >> +
> >> +#undef PMIC_SID_HEX
> >> +#undef PMIC_SID1_HEX
> All decimal numbers can be represented as hex numbers..
> Is there any point to keeping them separate?

Yes, to have pmic@c rather than pmic@12 for USID = 12 = 0xc.

>
> Konrad
> >> +
> >>  #undef PMIC_SID
> >>  #undef PMIC_SID1
> >>  #undef PMIC_LABEL
> >
> > Same comment as for previous patches - all undefs must be gone.

This means that we can not include two copies of the same PMIC (which
do have on both platforms).

> >
> > Maybe I should not have acked all these changes customized SID ("include
> > sid into defines") because it looks like it opened can of worms.

I think this can of worms is still better than imperfect duplication.


-- 
With best wishes
Dmitry

Re: [PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Krzysztof Kozlowski]

On 03/04/2023 13:45, Dmitry Baryshkov wrote:
>> Konrad
>>>> +
>>>>  #undef PMIC_SID
>>>>  #undef PMIC_SID1
>>>>  #undef PMIC_LABEL
>>>
>>> Same comment as for previous patches - all undefs must be gone.
> 
> This means that we can not include two copies of the same PMIC (which
> do have on both platforms).

Consider spi15 and spi16:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi?h=v6.3-rc5&id=7e364e56293bb98cae1b55fd835f5991c4e96e7d#n1045

Do you see it written as #include "qcom-sm8250-spi.dtsi" with
parametrizing the reg/unit address, interrupts etc?

No. Neither PMIC should be. It is not a special device.

Best regards,
Krzysztof

Re: [PATCH v2 21/22] arch: arm64: dts: qcom: pm8150: support SID greater that 9

[Dmitry Baryshkov]

On Mon, 3 Apr 2023 at 15:57, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 03/04/2023 13:45, Dmitry Baryshkov wrote:
> >> Konrad
> >>>> +
> >>>>  #undef PMIC_SID
> >>>>  #undef PMIC_SID1
> >>>>  #undef PMIC_LABEL
> >>>
> >>> Same comment as for previous patches - all undefs must be gone.
> >
> > This means that we can not include two copies of the same PMIC (which
> > do have on both platforms).
>
> Consider spi15 and spi16:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi?h=v6.3-rc5&id=7e364e56293bb98cae1b55fd835f5991c4e96e7d#n1045
>
> Do you see it written as #include "qcom-sm8250-spi.dtsi" with
> parametrizing the reg/unit address, interrupts etc?
>
> No. Neither PMIC should be. It is not a special device.

I think there should be balance. PMICs are complex structures.
Possibly schema will help here once it is in a more enforced mode.


-- 
With best wishes
Dmitry

[PATCH v2 22/22] arm64: dts: qcom sa8540p-pmics: switch to pm8150.dtsi

[Dmitry Baryshkov]

sa8540-pmics.dtsi contains 4 stripped down versions of pm8150 PMIC
declarations. Change it to use pm8150.dtsi directly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi | 96 ++++-----------------
 1 file changed, 19 insertions(+), 77 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi b/arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi
index 1221be89b3de..201297b6bb19 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p-pmics.dtsi
@@ -7,80 +7,22 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-&spmi_bus {
-	pmm8540a: pmic@0 {
-		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0x0 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		rtc@6000 {
-			compatible = "qcom,pm8941-rtc";
-			reg = <0x6000>, <0x6100>;
-			reg-names = "rtc", "alarm";
-			interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
-			wakeup-source;
-		};
-
-		pmm8540a_gpios: gpio@c000 {
-			compatible = "qcom,pm8150-gpio", "qcom,spmi-gpio";
-			reg = <0xc000>;
-			gpio-controller;
-			gpio-ranges = <&pmm8540a_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-
-	pmm8540c: pmic@4 {
-		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0x4 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmm8540c_gpios: gpio@c000 {
-			compatible = "qcom,pm8150-gpio", "qcom,spmi-gpio";
-			reg = <0xc000>;
-			gpio-controller;
-			gpio-ranges = <&pmm8540c_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-
-	pmm8540e: pmic@8 {
-		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0x8 SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmm8540e_gpios: gpio@c000 {
-			compatible = "qcom,pm8150-gpio", "qcom,spmi-gpio";
-			reg = <0xc000>;
-			gpio-controller;
-			gpio-ranges = <&pmm8540e_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-
-	pmm8540g: pmic@c {
-		compatible = "qcom,pm8150", "qcom,spmi-pmic";
-		reg = <0xc SPMI_USID>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		pmm8540g_gpios: gpio@c000 {
-			compatible = "qcom,pm8150-gpio", "qcom,spmi-gpio";
-			reg = <0xc000>;
-			gpio-controller;
-			gpio-ranges = <&pmm8540g_gpios 0 0 10>;
-			#gpio-cells = <2>;
-			interrupt-controller;
-			#interrupt-cells = <2>;
-		};
-	};
-};
+#define PMIC_SID 0
+#define PMIC_SID1 1
+#define PMIC_LABEL pmm8540a
+#include "pm8150.dtsi"
+
+#define PMIC_SID 4
+#define PMIC_SID1 5
+#define PMIC_LABEL pmm8540c
+#include "pm8150.dtsi"
+
+#define PMIC_SID 8
+#define PMIC_SID1 9
+#define PMIC_LABEL pmm8540e
+#include "pm8150.dtsi"
+
+#define PMIC_SID c
+#define PMIC_SID1 d
+#define PMIC_LABEL pmm8540g
+#include "pm8150.dtsi"
-- 
2.30.2

Re: [PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[Krzysztof Kozlowski]

On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> The sc8280xp platform uses its own copy of PMIC declarations. This can
> easily end up with the issues that are fixed in the main PMIC include
> file, but are not fixed for sc8280xp (and vice versa). For example
> commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON
> compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the
> PON device, while sc8280xp-pmic.dtsi still has the incorrect
> "qcom,pm8998-pon".
> 
> Another example is pm8280_2_temp_alarm device, which uses interrupts
> tied to SID 2, while having SID 3. This can be easily left unnoticed.
> 
> Employ a small amount of C preprocessor magic to make
> sc8280xp-pmics.dtsi use standard PMIC include files

Preprocessor magic is disliked in DTS. We allow only simple defines, no
undefs. Sometimes some nodes or strings could be concatenated, but in
obvious way. You should not parametrize it and have different, generated
labels in DTS based on something coming external to that DTS.

Best regards,
Krzysztof

Re: [PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[Konrad Dybcio]

On 2.04.2023 11:55, Krzysztof Kozlowski wrote:
> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>> The sc8280xp platform uses its own copy of PMIC declarations. This can
>> easily end up with the issues that are fixed in the main PMIC include
>> file, but are not fixed for sc8280xp (and vice versa). For example
>> commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON
>> compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the
>> PON device, while sc8280xp-pmic.dtsi still has the incorrect
>> "qcom,pm8998-pon".
>>
>> Another example is pm8280_2_temp_alarm device, which uses interrupts
>> tied to SID 2, while having SID 3. This can be easily left unnoticed.
>>
>> Employ a small amount of C preprocessor magic to make
>> sc8280xp-pmics.dtsi use standard PMIC include files
> 
> Preprocessor magic is disliked in DTS. We allow only simple defines, no
> undefs. Sometimes some nodes or strings could be concatenated, but in
> obvious way. You should not parametrize it and have different, generated
> labels in DTS based on something coming external to that DTS.
This again begs the question, is it time we start moving parts of the
dts code to be autogenerated?

Should we keep a separate file for each SID?

Or should we consider the SPMI 'interrupts' implementation flawed and
work towards one that does not require a SID to be specified within?

Currently it's:

interrupts = <USID PERIPH_ADDR>>8 IRQ_WITHIN_PERIPH IRQ_TYPE>;

So the first two cells are effectively useless and can be retrieved
from the parent node and the reg property.

Getting rid of that would solve a decent chunk of problems that this
patchset concerns.

Konrad

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[Dmitry Baryshkov]

On Mon, 3 Apr 2023 at 13:44, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 2.04.2023 11:55, Krzysztof Kozlowski wrote:
> > On 02/04/2023 00:07, Dmitry Baryshkov wrote:
> >> The sc8280xp platform uses its own copy of PMIC declarations. This can
> >> easily end up with the issues that are fixed in the main PMIC include
> >> file, but are not fixed for sc8280xp (and vice versa). For example
> >> commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON
> >> compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the
> >> PON device, while sc8280xp-pmic.dtsi still has the incorrect
> >> "qcom,pm8998-pon".
> >>
> >> Another example is pm8280_2_temp_alarm device, which uses interrupts
> >> tied to SID 2, while having SID 3. This can be easily left unnoticed.
> >>
> >> Employ a small amount of C preprocessor magic to make
> >> sc8280xp-pmics.dtsi use standard PMIC include files
> >
> > Preprocessor magic is disliked in DTS. We allow only simple defines, no
> > undefs. Sometimes some nodes or strings could be concatenated, but in
> > obvious way. You should not parametrize it and have different, generated
> > labels in DTS based on something coming external to that DTS.
> This again begs the question, is it time we start moving parts of the
> dts code to be autogenerated?
>
> Should we keep a separate file for each SID?
>
> Or should we consider the SPMI 'interrupts' implementation flawed and
> work towards one that does not require a SID to be specified within?
>
> Currently it's:
>
> interrupts = <USID PERIPH_ADDR>>8 IRQ_WITHIN_PERIPH IRQ_TYPE>;
>
> So the first two cells are effectively useless and can be retrieved
> from the parent node and the reg property.
>
> Getting rid of that would solve a decent chunk of problems that this
> patchset concerns.

While I agree with the USID part, the PERIPH_ADDR part is not always
like that, see pm8916.dtsi / audio-codec@f000.

I'll give it a thought, but it is probably not in the forthcoming future.


-- 
With best wishes
Dmitry

Re: [PATCH v2 00/22] arm64: dts: qcom: remove duplication in PMIC declarations

[Krzysztof Kozlowski]

On 03/04/2023 12:44, Konrad Dybcio wrote:
> 
> 
> On 2.04.2023 11:55, Krzysztof Kozlowski wrote:
>> On 02/04/2023 00:07, Dmitry Baryshkov wrote:
>>> The sc8280xp platform uses its own copy of PMIC declarations. This can
>>> easily end up with the issues that are fixed in the main PMIC include
>>> file, but are not fixed for sc8280xp (and vice versa). For example
>>> commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON
>>> compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the
>>> PON device, while sc8280xp-pmic.dtsi still has the incorrect
>>> "qcom,pm8998-pon".
>>>
>>> Another example is pm8280_2_temp_alarm device, which uses interrupts
>>> tied to SID 2, while having SID 3. This can be easily left unnoticed.
>>>
>>> Employ a small amount of C preprocessor magic to make
>>> sc8280xp-pmics.dtsi use standard PMIC include files
>>
>> Preprocessor magic is disliked in DTS. We allow only simple defines, no
>> undefs. Sometimes some nodes or strings could be concatenated, but in
>> obvious way. You should not parametrize it and have different, generated
>> labels in DTS based on something coming external to that DTS.
> This again begs the question, is it time we start moving parts of the
> dts code to be autogenerated?

Do we auto-generate C-code? Just a bit, but in general no. There are
exceptions but coding style is here clear:
https://elixir.bootlin.com/linux/v6.1-rc2/source/Documentation/process/coding-style.rst#L828

Pre-processor generated code should be narrowed to some cases or simpler
structures.

For DTS we actually are even stricter.


Best regards,
Krzysztof