diff mbox series

[v2,1/7] arm64: dts: qcom: sm8450: add spmi node

Message ID 20221229103212.984324-1-konrad.dybcio@linaro.org
State New
Headers
Series [v2,1/7] arm64: dts: qcom: sm8450: add spmi node |

Commit Message

Konrad Dybcio Dec. 29, 2022, 10:32 a.m. UTC 
  From: Vinod Koul <vkoul@kernel.org>

Add the spmi bus as found in the SM8450 SoC

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
[Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
v1 -> v2:
No changes

 arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Krzysztof Kozlowski Dec. 29, 2022, 10:42 a.m. UTC | #1 
On 29/12/2022 11:32, Konrad Dybcio wrote:
> From: Vinod Koul <vkoul@kernel.org>
> 
> Add the spmi bus as found in the SM8450 SoC
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> v1 -> v2:
> No changes
> 
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 570475040d95..b9b59c5223eb 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
>  			#clock-cells = <0>;
>  		};
>  
> +		spmi_bus: spmi@c42d000 {

Hmm looks different than reg.

> +			compatible = "qcom,spmi-pmic-arb";
> +			reg = <0 0x0c400000 0 0x00003000>,
> +			      <0 0x0c500000 0 0x00400000>,
> +			      <0 0x0c440000 0 0x00080000>,
> +			      <0 0x0c4c0000 0 0x00010000>,
> +			      <0 0x0c42d000 0 0x00010000>;
x

Best regards,
Krzysztof
Konrad Dybcio Dec. 29, 2022, 10:45 a.m. UTC | #2 
On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
> On 29/12/2022 11:32, Konrad Dybcio wrote:
>> From: Vinod Koul <vkoul@kernel.org>
>>
>> Add the spmi bus as found in the SM8450 SoC
>>
>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> v1 -> v2:
>> No changes
>>
>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 570475040d95..b9b59c5223eb 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
>>  			#clock-cells = <0>;
>>  		};
>>  
>> +		spmi_bus: spmi@c42d000 {
> 
> Hmm looks different than reg.
> 
>> +			compatible = "qcom,spmi-pmic-arb";
>> +			reg = <0 0x0c400000 0 0x00003000>,
>> +			      <0 0x0c500000 0 0x00400000>,
>> +			      <0 0x0c440000 0 0x00080000>,
>> +			      <0 0x0c4c0000 0 0x00010000>,
>> +			      <0 0x0c42d000 0 0x00010000>;
> x
Hm, my guess would be that Vinod chose to put the "cnfg" reg
instead of "core" in the unit address, as 8450 has 2 SPMI bus
hosts and they both share the core reg, so it would have been
impossible to have two spmi@core nodes..

Konrad
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 29, 2022, 10:57 a.m. UTC | #3 
On 29/12/2022 11:45, Konrad Dybcio wrote:
> 
> 
> On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
>> On 29/12/2022 11:32, Konrad Dybcio wrote:
>>> From: Vinod Koul <vkoul@kernel.org>
>>>
>>> Add the spmi bus as found in the SM8450 SoC
>>>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>> v1 -> v2:
>>> No changes
>>>
>>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> index 570475040d95..b9b59c5223eb 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
>>>  			#clock-cells = <0>;
>>>  		};
>>>  
>>> +		spmi_bus: spmi@c42d000 {
>>
>> Hmm looks different than reg.
>>
>>> +			compatible = "qcom,spmi-pmic-arb";
>>> +			reg = <0 0x0c400000 0 0x00003000>,
>>> +			      <0 0x0c500000 0 0x00400000>,
>>> +			      <0 0x0c440000 0 0x00080000>,
>>> +			      <0 0x0c4c0000 0 0x00010000>,
>>> +			      <0 0x0c42d000 0 0x00010000>;
>> x
> Hm, my guess would be that Vinod chose to put the "cnfg" reg
> instead of "core" in the unit address, as 8450 has 2 SPMI bus
> hosts and they both share the core reg, so it would have been
> impossible to have two spmi@core nodes..

Eh? SM8450 has 2 SPMI hosts both using 0x0c400000? How does that work?
Usually address can be mapped only once.

Where is the second SPMI? I cannot find it in linux-next.


Best regards,
Krzysztof
Konrad Dybcio Dec. 29, 2022, 11:04 a.m. UTC | #4 
On 29.12.2022 11:57, Krzysztof Kozlowski wrote:
> On 29/12/2022 11:45, Konrad Dybcio wrote:
>>
>>
>> On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
>>> On 29/12/2022 11:32, Konrad Dybcio wrote:
>>>> From: Vinod Koul <vkoul@kernel.org>
>>>>
>>>> Add the spmi bus as found in the SM8450 SoC
>>>>
>>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>>>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>> v1 -> v2:
>>>> No changes
>>>>
>>>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> index 570475040d95..b9b59c5223eb 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
>>>>  			#clock-cells = <0>;
>>>>  		};
>>>>  
>>>> +		spmi_bus: spmi@c42d000 {
>>>
>>> Hmm looks different than reg.
>>>
>>>> +			compatible = "qcom,spmi-pmic-arb";
>>>> +			reg = <0 0x0c400000 0 0x00003000>,
>>>> +			      <0 0x0c500000 0 0x00400000>,
>>>> +			      <0 0x0c440000 0 0x00080000>,
>>>> +			      <0 0x0c4c0000 0 0x00010000>,
>>>> +			      <0 0x0c42d000 0 0x00010000>;
>>> x
>> Hm, my guess would be that Vinod chose to put the "cnfg" reg
>> instead of "core" in the unit address, as 8450 has 2 SPMI bus
>> hosts and they both share the core reg, so it would have been
>> impossible to have two spmi@core nodes..
> 
> Eh? SM8450 has 2 SPMI hosts both using 0x0c400000? How does that work?
> Usually address can be mapped only once.
No idea either!

> 
> Where is the second SPMI? I cannot find it in linux-next.
It's only there on downstream and I'm not sure how useful it is
really, only some debug subdevice is attached to it.. Perhaps
we could ignore its existence for now and I could use the core
reg in unit address for spmi0?

Konrad
> 
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 29, 2022, 11:31 a.m. UTC | #5 
On 29/12/2022 12:04, Konrad Dybcio wrote:
> 
> 
> On 29.12.2022 11:57, Krzysztof Kozlowski wrote:
>> On 29/12/2022 11:45, Konrad Dybcio wrote:
>>>
>>>
>>> On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
>>>> On 29/12/2022 11:32, Konrad Dybcio wrote:
>>>>> From: Vinod Koul <vkoul@kernel.org>
>>>>>
>>>>> Add the spmi bus as found in the SM8450 SoC
>>>>>
>>>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>>>>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>> ---
>>>>> v1 -> v2:
>>>>> No changes
>>>>>
>>>>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>>>>>  1 file changed, 22 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> index 570475040d95..b9b59c5223eb 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
>>>>>  			#clock-cells = <0>;
>>>>>  		};
>>>>>  
>>>>> +		spmi_bus: spmi@c42d000 {
>>>>
>>>> Hmm looks different than reg.
>>>>
>>>>> +			compatible = "qcom,spmi-pmic-arb";
>>>>> +			reg = <0 0x0c400000 0 0x00003000>,
>>>>> +			      <0 0x0c500000 0 0x00400000>,
>>>>> +			      <0 0x0c440000 0 0x00080000>,
>>>>> +			      <0 0x0c4c0000 0 0x00010000>,
>>>>> +			      <0 0x0c42d000 0 0x00010000>;
>>>> x
>>> Hm, my guess would be that Vinod chose to put the "cnfg" reg
>>> instead of "core" in the unit address, as 8450 has 2 SPMI bus
>>> hosts and they both share the core reg, so it would have been
>>> impossible to have two spmi@core nodes..
>>
>> Eh? SM8450 has 2 SPMI hosts both using 0x0c400000? How does that work?
>> Usually address can be mapped only once.
> No idea either!
> 
>>
>> Where is the second SPMI? I cannot find it in linux-next.
> It's only there on downstream and I'm not sure how useful it is
> really, only some debug subdevice is attached to it.. Perhaps
> we could ignore its existence for now and I could use the core
> reg in unit address for spmi0?

I see it indeed in downstream. core, chnls and obsrvr IO are the same.
There is quite of debug devices attached.

There is a comment in PMIC arbiter code to use a IO mapping allowing
simultaneous mappings, so this is actually valid.

Anyway, DT expects unit address to match first reg, so if we want to
have second PMIC, we need to change the order of reg entries.

We can ignore this problem till we add second PMIC...

Best regards,
Krzysztof
Bjorn Andersson Dec. 29, 2022, 4:12 p.m. UTC | #6 
On Thu, Dec 29, 2022 at 11:57:58AM +0100, Krzysztof Kozlowski wrote:
> On 29/12/2022 11:45, Konrad Dybcio wrote:
> > 
> > 
> > On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
> >> On 29/12/2022 11:32, Konrad Dybcio wrote:
> >>> From: Vinod Koul <vkoul@kernel.org>
> >>>
> >>> Add the spmi bus as found in the SM8450 SoC
> >>>
> >>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> >>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> >>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >>> ---
> >>> v1 -> v2:
> >>> No changes
> >>>
> >>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
> >>>  1 file changed, 22 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> >>> index 570475040d95..b9b59c5223eb 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> >>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
> >>>  			#clock-cells = <0>;
> >>>  		};
> >>>  
> >>> +		spmi_bus: spmi@c42d000 {
> >>
> >> Hmm looks different than reg.
> >>
> >>> +			compatible = "qcom,spmi-pmic-arb";
> >>> +			reg = <0 0x0c400000 0 0x00003000>,
> >>> +			      <0 0x0c500000 0 0x00400000>,
> >>> +			      <0 0x0c440000 0 0x00080000>,
> >>> +			      <0 0x0c4c0000 0 0x00010000>,
> >>> +			      <0 0x0c42d000 0 0x00010000>;
> >> x
> > Hm, my guess would be that Vinod chose to put the "cnfg" reg
> > instead of "core" in the unit address, as 8450 has 2 SPMI bus
> > hosts and they both share the core reg, so it would have been
> > impossible to have two spmi@core nodes..
> 
> Eh? SM8450 has 2 SPMI hosts both using 0x0c400000? How does that work?
> Usually address can be mapped only once.
> 

The SPMI controller does something like multi-master. The driver expects
the same region to be mapped multiple times and qcom,channel is used to
select which one each instance should operate on.

Regards,
Bjorn

> Where is the second SPMI? I cannot find it in linux-next.
> 
> 
> Best regards,
> Krzysztof
>
Vinod Koul Dec. 30, 2022, 5:16 a.m. UTC | #7 
On 29-12-22, 10:12, Bjorn Andersson wrote:
> On Thu, Dec 29, 2022 at 11:57:58AM +0100, Krzysztof Kozlowski wrote:
> > On 29/12/2022 11:45, Konrad Dybcio wrote:
> > > 
> > > 
> > > On 29.12.2022 11:42, Krzysztof Kozlowski wrote:
> > >> On 29/12/2022 11:32, Konrad Dybcio wrote:
> > >>> From: Vinod Koul <vkoul@kernel.org>
> > >>>
> > >>> Add the spmi bus as found in the SM8450 SoC
> > >>>
> > >>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > >>> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> > >>> [Konrad: 0x0 -> 0, move #cells down, make reg-names a vertical list]
> > >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > >>> ---
> > >>> v1 -> v2:
> > >>> No changes
> > >>>
> > >>>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
> > >>>  1 file changed, 22 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > >>> index 570475040d95..b9b59c5223eb 100644
> > >>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > >>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > >>> @@ -2715,6 +2715,28 @@ aoss_qmp: power-controller@c300000 {
> > >>>  			#clock-cells = <0>;
> > >>>  		};
> > >>>  
> > >>> +		spmi_bus: spmi@c42d000 {
> > >>
> > >> Hmm looks different than reg.
> > >>
> > >>> +			compatible = "qcom,spmi-pmic-arb";
> > >>> +			reg = <0 0x0c400000 0 0x00003000>,
> > >>> +			      <0 0x0c500000 0 0x00400000>,
> > >>> +			      <0 0x0c440000 0 0x00080000>,
> > >>> +			      <0 0x0c4c0000 0 0x00010000>,
> > >>> +			      <0 0x0c42d000 0 0x00010000>;
> > >> x
> > > Hm, my guess would be that Vinod chose to put the "cnfg" reg
> > > instead of "core" in the unit address, as 8450 has 2 SPMI bus
> > > hosts and they both share the core reg, so it would have been
> > > impossible to have two spmi@core nodes..
> > 
> > Eh? SM8450 has 2 SPMI hosts both using 0x0c400000? How does that work?
> > Usually address can be mapped only once.
> > 
> 
> The SPMI controller does something like multi-master. The driver expects
> the same region to be mapped multiple times and qcom,channel is used to
> select which one each instance should operate on.

Right, this one adds same as downstream. I agree in future we should
revisit this and decide how we should model this. For now I am more
inclined to get this piece closed, it been more than a year :-( lets not
make it two!
Bjorn Andersson Jan. 11, 2023, 5:09 a.m. UTC | #8 
On Thu, 29 Dec 2022 11:32:06 +0100, Konrad Dybcio wrote:
> From: Vinod Koul <vkoul@kernel.org>
> 
> Add the spmi bus as found in the SM8450 SoC
> 
> 

Applied, thanks!

[1/7] arm64: dts: qcom: sm8450: add spmi node
      commit: f891f86e47c3208986b0985ca1fbc94647ba2ad0
[2/7] arm64: dts: qcom: sm8450-nagara: Include PMIC DTSIs
      commit: 25deb75e99bc57a7860cef2688b032d0e2f979dc
[3/7] arm64: dts: qcom: sm8450-nagara: Add GPIO line names for PMIC GPIOs
      commit: 4c5ab70d11ba591e28d4b07e50847084141c2374
[4/7] arm64: dts: qcom: sm8450-nagara: Add GPIO keys
      commit: 7b2557697890a947e178d4dc20848b479e384123
[5/7] arm64: dts: qcom: sm8450-nagara: Set up camera regulators
      commit: 40430a7c485b5463247b28691ad6a4fc5e280235
[6/7] arm64: dts: qcom: sm8450-nagara: Enable PMIC RESIN+PON
      commit: e9090691e48d2ceabec70448ac893637fbf0e27e
[7/7] arm64: dts: qcom: sm8450-nagara: Configure SLG51000 PMIC
      commit: 0d89bfbcd6d4c2691f5d70b8f2938aeb7774e7f6

Best regards,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 570475040d95..b9b59c5223eb 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2715,6 +2715,28 @@  aoss_qmp: power-controller@c300000 {
 			#clock-cells = <0>;
 		};
 
+		spmi_bus: spmi@c42d000 {
+			compatible = "qcom,spmi-pmic-arb";
+			reg = <0 0x0c400000 0 0x00003000>,
+			      <0 0x0c500000 0 0x00400000>,
+			      <0 0x0c440000 0 0x00080000>,
+			      <0 0x0c4c0000 0 0x00010000>,
+			      <0 0x0c42d000 0 0x00010000>;
+			reg-names = "core",
+				    "chnls",
+				    "obsrvr",
+				    "intr",
+				    "cnfg";
+			interrupt-names = "periph_irq";
+			interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,ee = <0>;
+			qcom,channel = <0>;
+			interrupt-controller;
+			#interrupt-cells = <4>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+		};
+
 		ipcc: mailbox@ed18000 {
 			compatible = "qcom,sm8450-ipcc", "qcom,ipcc";
 			reg = <0 0x0ed18000 0 0x1000>;