diff mbox series

[V12,3/3] arm64: dts: ipq6018: add pwm node

Message ID	20230925065915.3467964-4-quic_devipriy@quicinc.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5; From: Devi Priya <quic_devipriy@quicinc.com> To: <thierry.reding@gmail.com>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <agross@kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <ndesaulniers@google.com>, <trix@redhat.com>, <baruch@tkos.co.il>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <llvm@lists.linux.dev> CC: <linux-pwm@vger.kernel.org>, <u.kleine-koenig@pengutronix.de>, <nathan@kernel.org> Subject: [PATCH V12 3/3] arm64: dts: ipq6018: add pwm node Date: Mon, 25 Sep 2023 12:29:15 +0530 Message-ID: <20230925065915.3467964-4-quic_devipriy@quicinc.com> In-Reply-To: <20230925065915.3467964-1-quic_devipriy@quicinc.com> References: <20230925065915.3467964-1-quic_devipriy@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Add PWM support for IPQ chipsets \| [V12,0/3] Add PWM support for IPQ chipsets [V12,1/3] pwm: driver for qualcomm ipq6018 pwm block [V12,2/3] dt-bindings: pwm: add IPQ6018 binding [V12,3/3] arm64: dts: ipq6018: add pwm node

Commit Message

Devi Priya Sept. 25, 2023, 6:59 a.m. UTC

  Describe the PWM block on IPQ6018.

The PWM is in the TCSR area. Make &tcsr "simple-mfd" compatible, and add
&pwm as child of &tcsr.

Add also ipq6018 specific compatible string.

Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
---
v12: 

  No change

v11:

  No change

v10:

  No change

v9:

  Add 'ranges' property (Rob)

v8:

  Add size cell to 'reg' (Rob)

v7:

  Use 'reg' instead of 'offset' (Rob)

  Add qcom,tcsr-ipq6018 (Rob)

  Drop clock-names (Bjorn)

v6:

  Make the PWM node child of TCSR (Rob Herring)

  Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)

v5: Use qcom,pwm-regs for TCSR phandle instead of direct regs

v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)

 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Sept. 25, 2023, 7:13 a.m. UTC | #1

On 25/09/2023 08:59, Devi Priya wrote:
> Describe the PWM block on IPQ6018.
> 
> The PWM is in the TCSR area. Make &tcsr "simple-mfd" compatible, and add
> &pwm as child of &tcsr.
> 
> Add also ipq6018 specific compatible string.
> 
> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
> v12: 
> 
>   No change
> 
> v11:
> 
>   No change
> 
> v10:
> 
>   No change
> 
> v9:
> 
>   Add 'ranges' property (Rob)
> 
> v8:
> 
>   Add size cell to 'reg' (Rob)
> 
> v7:
> 
>   Use 'reg' instead of 'offset' (Rob)
> 
>   Add qcom,tcsr-ipq6018 (Rob)
> 
>   Drop clock-names (Bjorn)
> 
> v6:
> 
>   Make the PWM node child of TCSR (Rob Herring)
> 
>   Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
> 
> v5: Use qcom,pwm-regs for TCSR phandle instead of direct regs
> 
> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> 
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 47b8b1d6730a..cadd2c583526 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -398,8 +398,21 @@ tcsr_mutex: hwlock@1905000 {
>  		};
>  
>  		tcsr: syscon@1937000 {
> -			compatible = "qcom,tcsr-ipq6018", "syscon";
> +			compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof

Devi Priya Sept. 29, 2023, 11:47 a.m. UTC | #2

On 9/25/2023 12:29 PM, Devi Priya wrote:
> Describe the PWM block on IPQ6018.
> 
> The PWM is in the TCSR area. Make &tcsr "simple-mfd" compatible, and add
> &pwm as child of &tcsr.
> 
> Add also ipq6018 specific compatible string.
> 
> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
> ---
> v12:
> 
>    No change
> 
> v11:
> 
>    No change
> 
> v10:
> 
>    No change
> 
> v9:
> 
>    Add 'ranges' property (Rob)
> 
> v8:
> 
>    Add size cell to 'reg' (Rob)
> 
> v7:
> 
>    Use 'reg' instead of 'offset' (Rob)
> 
>    Add qcom,tcsr-ipq6018 (Rob)
> 
>    Drop clock-names (Bjorn)
> 
> v6:
> 
>    Make the PWM node child of TCSR (Rob Herring)
> 
>    Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
> 
> v5: Use qcom,pwm-regs for TCSR phandle instead of direct regs
> 
> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
> 
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 47b8b1d6730a..cadd2c583526 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -398,8 +398,21 @@ tcsr_mutex: hwlock@1905000 {
>   		};
>   
>   		tcsr: syscon@1937000 {
> -			compatible = "qcom,tcsr-ipq6018", "syscon";
> +			compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
>   			reg = <0x0 0x01937000 0x0 0x21000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0x0 0x0 0x01937000 0x21000>;
> +
Hi Krzysztof,
Referring to 
https://lore.kernel.org/all/20220909091056.128949-1-krzysztof.kozlowski@linaro.org/, 
it seems that the TCSR block should
not have any child nodes. Could you pls provide your suggestions on pwm
being added as the child node?

Thanks,
Devi Priya
> +			 pwm: pwm@a010 {
> +				compatible = "qcom,ipq6018-pwm";
> +				reg = <0xa010 0x20>;
> +				clocks = <&gcc GCC_ADSS_PWM_CLK>;
> +				assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
> +				assigned-clock-rates = <100000000>;
> +				#pwm-cells = <2>;
> +				status = "disabled";
> +			};
>   		};
>   
>   		usb2: usb@70f8800 {

Krzysztof Kozlowski Sept. 30, 2023, 3:35 p.m. UTC | #3

On 29/09/2023 13:47, Devi Priya wrote:
> 
> 
> On 9/25/2023 12:29 PM, Devi Priya wrote:
>> Describe the PWM block on IPQ6018.
>>
>> The PWM is in the TCSR area. Make &tcsr "simple-mfd" compatible, and add
>> &pwm as child of &tcsr.
>>
>> Add also ipq6018 specific compatible string.
>>
>> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
>> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>> v12:
>>
>>    No change
>>
>> v11:
>>
>>    No change
>>
>> v10:
>>
>>    No change
>>
>> v9:
>>
>>    Add 'ranges' property (Rob)
>>
>> v8:
>>
>>    Add size cell to 'reg' (Rob)
>>
>> v7:
>>
>>    Use 'reg' instead of 'offset' (Rob)
>>
>>    Add qcom,tcsr-ipq6018 (Rob)
>>
>>    Drop clock-names (Bjorn)
>>
>> v6:
>>
>>    Make the PWM node child of TCSR (Rob Herring)
>>
>>    Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
>>
>> v5: Use qcom,pwm-regs for TCSR phandle instead of direct regs
>>
>> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
>>
>>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> index 47b8b1d6730a..cadd2c583526 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> @@ -398,8 +398,21 @@ tcsr_mutex: hwlock@1905000 {
>>   		};
>>   
>>   		tcsr: syscon@1937000 {
>> -			compatible = "qcom,tcsr-ipq6018", "syscon";
>> +			compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
>>   			reg = <0x0 0x01937000 0x0 0x21000>;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0x0 0x0 0x01937000 0x21000>;
>> +
> Hi Krzysztof,
> Referring to 
> https://lore.kernel.org/all/20220909091056.128949-1-krzysztof.kozlowski@linaro.org/, 
> it seems that the TCSR block should
> not have any child nodes. Could you pls provide your suggestions on pwm
> being added as the child node?

If you are sure that TCSR contains PWM and all registers are there, then
feel free to add proper binding. Sending untested patch is not the way
to go.

Best regards,
Krzysztof

Devi Priya Sept. 30, 2023, 5:13 p.m. UTC | #4

On 9/30/2023 9:05 PM, Krzysztof Kozlowski wrote:
> On 29/09/2023 13:47, Devi Priya wrote:
>>
>>
>> On 9/25/2023 12:29 PM, Devi Priya wrote:
>>> Describe the PWM block on IPQ6018.
>>>
>>> The PWM is in the TCSR area. Make &tcsr "simple-mfd" compatible, and add
>>> &pwm as child of &tcsr.
>>>
>>> Add also ipq6018 specific compatible string.
>>>
>>> Co-developed-by: Baruch Siach <baruch.siach@siklu.com>
>>> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
>>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>>> ---
>>> v12:
>>>
>>>     No change
>>>
>>> v11:
>>>
>>>     No change
>>>
>>> v10:
>>>
>>>     No change
>>>
>>> v9:
>>>
>>>     Add 'ranges' property (Rob)
>>>
>>> v8:
>>>
>>>     Add size cell to 'reg' (Rob)
>>>
>>> v7:
>>>
>>>     Use 'reg' instead of 'offset' (Rob)
>>>
>>>     Add qcom,tcsr-ipq6018 (Rob)
>>>
>>>     Drop clock-names (Bjorn)
>>>
>>> v6:
>>>
>>>     Make the PWM node child of TCSR (Rob Herring)
>>>
>>>     Add assigned-clocks/assigned-clock-rates (Uwe Kleine-König)
>>>
>>> v5: Use qcom,pwm-regs for TCSR phandle instead of direct regs
>>>
>>> v3: s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
>>>
>>>    arch/arm64/boot/dts/qcom/ipq6018.dtsi | 15 ++++++++++++++-
>>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>>> index 47b8b1d6730a..cadd2c583526 100644
>>> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>>> @@ -398,8 +398,21 @@ tcsr_mutex: hwlock@1905000 {
>>>    		};
>>>    
>>>    		tcsr: syscon@1937000 {
>>> -			compatible = "qcom,tcsr-ipq6018", "syscon";
>>> +			compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
>>>    			reg = <0x0 0x01937000 0x0 0x21000>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			ranges = <0x0 0x0 0x01937000 0x21000>;
>>> +
>> Hi Krzysztof,
>> Referring to
>> https://lore.kernel.org/all/20220909091056.128949-1-krzysztof.kozlowski@linaro.org/,
>> it seems that the TCSR block should
>> not have any child nodes. Could you pls provide your suggestions on pwm
>> being added as the child node?
> 
> If you are sure that TCSR contains PWM and all registers are there, then
> feel free to add proper binding. Sending untested patch is not the way
> to go.

Sure, okay

Thanks,
Devi Priya
> 
> Best regards,
> Krzysztof
>

diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 47b8b1d6730a..cadd2c583526 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -398,8 +398,21 @@  tcsr_mutex: hwlock@1905000 {
 		};
 
 		tcsr: syscon@1937000 {
-			compatible = "qcom,tcsr-ipq6018", "syscon";
+			compatible = "qcom,tcsr-ipq6018", "syscon", "simple-mfd";
 			reg = <0x0 0x01937000 0x0 0x21000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0x0 0x01937000 0x21000>;
+
+			 pwm: pwm@a010 {
+				compatible = "qcom,ipq6018-pwm";
+				reg = <0xa010 0x20>;
+				clocks = <&gcc GCC_ADSS_PWM_CLK>;
+				assigned-clocks = <&gcc GCC_ADSS_PWM_CLK>;
+				assigned-clock-rates = <100000000>;
+				#pwm-cells = <2>;
+				status = "disabled";
+			};
 		};
 
 		usb2: usb@70f8800 {